[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

2022-11-24 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 477862.
tbaeder marked 10 inline comments as done.
tbaeder added a comment.

Remove some questionable (but unused) `Floating` API that didn't take 
floating-point semantics into account.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134859/new/

https://reviews.llvm.org/D134859

Files:
  clang/lib/AST/CMakeLists.txt
  clang/lib/AST/Interp/Boolean.h
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/Context.cpp
  clang/lib/AST/Interp/Descriptor.cpp
  clang/lib/AST/Interp/Floating.cpp
  clang/lib/AST/Interp/Floating.h
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/Interp.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpStack.h
  clang/lib/AST/Interp/Opcodes.td
  clang/lib/AST/Interp/PrimType.h
  clang/lib/AST/Interp/Primitives.h
  clang/test/AST/Interp/const-fpfeatures.cpp
  clang/test/AST/Interp/floats.cpp
  clang/test/SemaCXX/rounding-math.cpp

Index: clang/test/SemaCXX/rounding-math.cpp
===
--- clang/test/SemaCXX/rounding-math.cpp
+++ clang/test/SemaCXX/rounding-math.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-linux -verify=norounding -Wno-unknown-pragmas %s
 // RUN: %clang_cc1 -triple x86_64-linux -verify=rounding %s -frounding-math -Wno-unknown-pragmas
+// RUN: %clang_cc1 -triple x86_64-linux -verify=rounding %s -frounding-math -fexperimental-new-constant-interpreter -Wno-unknown-pragmas
 // rounding-no-diagnostics
 
 #define fold(x) (__builtin_constant_p(x) ? (x) : (x))
Index: clang/test/AST/Interp/floats.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/floats.cpp
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
+// RUN: %clang_cc1 -verify=ref %s
+
+constexpr int i = 2;
+constexpr float f = 1.0f;
+static_assert(f == 1.0f, "");
+
+constexpr float f2 = 1u * f;
+static_assert(f2 == 1.0f, "");
+
+constexpr float f3 = 1.5;
+constexpr int i3 = f3;
+static_assert(i3 == 1);
+
+constexpr bool b3 = f3;
+static_assert(b3);
+
+
+static_assert(1.0f + 3u == 4, "");
+static_assert(4.0f / 1.0f == 4, "");
+static_assert(10.0f * false == 0, "");
+
+constexpr float floats[] = {1.0f, 2.0f, 3.0f, 4.0f};
+
+constexpr float m = 5.0f / 0.0f; // ref-error {{must be initialized by a constant expression}} \
+ // ref-note {{division by zero}} \
+ // expected-error {{must be initialized by a constant expression}} \
+ // expected-note {{division by zero}}
+
+static_assert(~2.0f == 3, ""); // ref-error {{invalid argument type 'float' to unary expression}} \
+   // expected-error {{invalid argument type 'float' to unary expression}}
+
+/// Initialized by a double.
+constexpr float df = 0.0;
+/// The other way around.
+constexpr double fd = 0.0f;
+
+static_assert(0.0f == -0.0f, "");
+
+const int k = 3 * (1.0f / 3.0f);
+static_assert(k == 1, "");
+
+constexpr bool b = 1.0;
+static_assert(b, "");
+
+constexpr double db = true;
+static_assert(db == 1.0, "");
+
+constexpr float fa[] = {1.0f, 2.0, 1, false};
+constexpr float da[] = {1.0f, 2.0, 1, false};
Index: clang/test/AST/Interp/const-fpfeatures.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/const-fpfeatures.cpp
@@ -0,0 +1,72 @@
+// RUN: %clang_cc1 -S -emit-llvm -triple i386-linux -std=c++2a -Wno-unknown-pragmas %s -o - | FileCheck %s
+// RUN: %clang_cc1 -S -emit-llvm -triple i386-linux -fexperimental-new-constant-interpreter -std=c++2a -Wno-unknown-pragmas %s -o - | FileCheck %s
+
+
+#pragma STDC FENV_ROUND FE_UPWARD
+
+float F1u = 1.0F + 0x0.02p0F;
+float F2u = 1.0F + 0x0.01p0F;
+float F3u = 0x1.01p0;
+// CHECK: @F1u = {{.*}} float 0x3FF02000
+// CHECK: @F2u = {{.*}} float 0x3FF02000
+// CHECK: @F3u = {{.*}} float 0x3FF02000
+
+float FI1u = 0xU;
+// CHECK: @FI1u = {{.*}} float 0x41F0
+
+#pragma STDC FENV_ROUND FE_DOWNWARD
+
+float F1d = 1.0F + 0x0.02p0F;
+float F2d = 1.0F + 0x0.01p0F;
+float F3d = 0x1.01p0;
+
+// CHECK: @F1d = {{.*}} float 0x3FF02000
+// CHECK: @F2d = {{.*}} float 1.00e+00
+// CHECK: @F3d = {{.*}} float 1.00e+00
+
+
+float FI1d = 0xU;
+// CHECK: @FI1d = {{.*}} float 0x41EFE000
+
+// nextUp(1.F) == 0x1.02p0F
+
+constexpr float add_round_down(float x, float y) {
+  #pragma STDC FENV_ROUND FE_DOWNWARD
+  float res = x;
+  res += y;
+  return res;
+}
+
+constexpr float add_round_up(float x, float y) {
+  #pragma STDC FENV_ROUND FE_UPWARD
+  float res = x;
+  res += y;
+  return res;
+}
+
+float V1 = add_round_down(1.0F, 0x0.01p0F);
+float V2 = add_round_up(1.0F, 0x0.01p0F);
+// CHECK: @V1 = {{.*}} float 1.00e+00
+// CHECK: @V2 = {{.*}} float 0x3FF02000
+

[PATCH] D138655: [clang-tidy] Fix `cppcoreguidelines-init-variables` for invalid vardecl

2022-11-24 Thread gehry via Phabricator via cfe-commits
Sockke updated this revision to Diff 477861.
Sockke added a comment.

Added test.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138655/new/

https://reviews.llvm.org/D138655

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables-clang-diagnostic-error.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables-clang-diagnostic-error.cpp
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables-clang-diagnostic-error.cpp
@@ -0,0 +1,17 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables %t -fix-errors --
+
+#include "unknown.h"
+// CHECK-MESSAGES: :[[@LINE-1]]:10: error: 'unknown.h' file not found 
[clang-diagnostic-error]
+
+namespace std {
+template 
+struct vector {
+  vector();
+};
+
+void test() {
+  int a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'a' is not initialized 
[cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int a = 0;{{$}}
+  std::vector arr;
+}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -60,6 +60,8 @@
   const ASTContext  = *Result.Context;
   const SourceManager  = Context.getSourceManager();
 
+  if (MatchedDecl->isInvalidDecl())
+return;
   // We want to warn about cases where the type name
   // comes from a macro like this:
   //


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables-clang-diagnostic-error.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables-clang-diagnostic-error.cpp
@@ -0,0 +1,17 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables %t -fix-errors --
+
+#include "unknown.h"
+// CHECK-MESSAGES: :[[@LINE-1]]:10: error: 'unknown.h' file not found [clang-diagnostic-error]
+
+namespace std {
+template 
+struct vector {
+  vector();
+};
+
+void test() {
+  int a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'a' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int a = 0;{{$}}
+  std::vector arr;
+}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -60,6 +60,8 @@
   const ASTContext  = *Result.Context;
   const SourceManager  = Context.getSourceManager();
 
+  if (MatchedDecl->isInvalidDecl())
+return;
   // We want to warn about cases where the type name
   // comes from a macro like this:
   //
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D136694: [clang][Interp] Check that constructor calls initialize all record fields

2022-11-24 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added a comment.

Ping


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136694/new/

https://reviews.llvm.org/D136694

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D137235: [clang][Interp] Fix ImplicitValueInitExprs for pointer types

2022-11-24 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137235/new/

https://reviews.llvm.org/D137235

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D136815: [clang][Interp] Unify visiting variable declarations

2022-11-24 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added a comment.

Ping


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136815/new/

https://reviews.llvm.org/D136815

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138117: [clang][docs] Correct floating point option explanations

2022-11-24 Thread KAWASHIMA Takahiro via Phabricator via cfe-commits
kawashima-fj added a comment.

ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138117/new/

https://reviews.llvm.org/D138117

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D127462: [Clang] Begin implementing Plan 9 C extensions

2022-11-24 Thread Keegan Saunders via Phabricator via cfe-commits
ksaunders added a comment.

> Just to check -- do you think (some of) these features are something you wish 
> to propose to WG14 for adoption into C? e.g., are you aiming to get multiple 
> compilers to implement Plan 9 extensions to demonstrate to WG14 that this is 
> existing practice in C compilers?

A lot of the Plan 9 extensions were actually adopted by C99 like compound 
literals and anonymous structures. Although I find these additional extensions 
interesting and useful, I don't think that they belong in C and they should 
remain as non-standard extensions. My interests lie in compiling existing code 
with Clang which utilizes these extensions, rather than encouraging new code to 
utilize them.

There was actually a proposal to add Plan 9 extensions into the Linux kernel, 
but Linus rejected it. I personally share his opinion that the silent type 
conversion that the Plan 9 compilers introduce can be problematic. But on the 
other hand, they are also very powerful when used judiciously. It's on the LKML 
here if you're interested: https://lkml.org/lkml/2019/1/9/1127

> Does this work for accessing r->FirstLock but give an ambiguous lookup for 
> r->hold? Or do you only allow one member of the underlying canonical type?

Good question. The compilers only allow one member of the underlying type. So 
you'll get an error saying you've declared `Lock` twice.

> Also, why does it require a typedef name?

I am not sure, but I presume it is because of cases like this:

  struct A
  {
  int a;
  };
  
  struct B
  {
  int b;
  };
  
  typedef struct B A;
  
  struct Example
  {
  struct A;
  A;
  };

So it's not clear when you do `example->A` which member you are referring to. 
If you restrict it to typedef names then you have no ambiguity of this kind.

> Ah, interesting. So this is another case where multiple members of the same 
> type would be a problem. Does this only find structure/union members, or does 
> this also work for other members? e.g. void size(size_t *) being called with 
> lock(r)? And if it works for other members... what does it do for bit-field 
> members which share an allocation unit?

It only searches for //unnamed// structure and union members, so non-record 
members like bit-fields are not used for resolution. That's a good test case I 
should add as well, yeah.

> Thanks for the extra details!

Thank you for your interest :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127462/new/

https://reviews.llvm.org/D127462

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D127910: [Clang][AArch64] Add SME C intrinsics for load and store

2022-11-24 Thread Bryan Chan via Phabricator via cfe-commits
bryanpkc commandeered this revision.
bryanpkc added a reviewer: sagarkulkarni19.
bryanpkc added a comment.

In D127910#3885699 , @david-arm wrote:

> Hi @sagarkulkarni19, just a gentle ping to see if you are still planning to 
> do more work on this patch?

@david-arm we are going to update this patch very soon. Sorry for the delays.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127910/new/

https://reviews.llvm.org/D127910

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138579: [AArch64] Assembly support for FEAT_LRCPC3

2022-11-24 Thread Thorsten via Phabricator via cfe-commits
tschuett added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64.td:510
 
+def FeatureRCPC3 : SubtargetFeature<"rcpc3", "HasRCPC3",
+"true", "Enable Armv8.9-A RCPC instructions added to A64 and Advanced SIMD 
and floating-point instruction set (FEAT_LRCPC3)",

Nice feature, but the text is quite long with a lot of `and` s


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138579/new/

https://reviews.llvm.org/D138579

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D137205: [clang-tidy] Add performance-unnecessary-copy-on-last-use check

2022-11-24 Thread Fabian Keßler via Phabricator via cfe-commits
Febbe marked 2 inline comments as done.
Febbe added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyOnLastUseCheck.cpp:123
+  hasLHS(ignoringParenImpCasts(declRefExpr(equalsNode(DeclRef)),
+  Context);
+  return !Matches.empty();

Febbe wrote:
> njames93 wrote:
> > Matching over the entire context seems pretty and a huge drain on 
> > performance, would it not make sense to just match inside the function 
> > declaration.
> > Side note maybe a RecursiveASTVisitor would make more sense here in terms 
> > of performance.
> > 
> > Same goes for isInLambdaCapture.
> How can I reduce the Context?
> 
> Also, in which terms, the RecursiveASTVisitor would make more sense here?
Replaced with RecursiveASTVisitor.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-on-last-use.cpp:1
+// RUN: %check_clang_tidy %s -std=c++17 
performance-unnecessary-copy-on-last-use %t
+// RUN: %check_clang_tidy %s -std=c++11 
performance-unnecessary-copy-on-last-use %t

njames93 wrote:
> Running this check explicitly in c++11/17 implies you expect different 
> diagnostics, if so you can use the `--check-suffixes` flag to enable checking 
> configurations. Search for other tests which use that if you're unsure 
Not necessarily, I just want the check to work with c++11 and after the changes 
to the language after c++17 (e.g. guaranteed copy elision). 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137205/new/

https://reviews.llvm.org/D137205

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138630: [modules] Fix marking `ObjCMethodDecl::isOverriding` when there a no overrides.

2022-11-24 Thread Volodymyr Sapsai via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0314ba3acbab: [modules] Fix marking 
`ObjCMethodDecl::isOverriding` when there are no… (authored by vsapsai).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138630/new/

https://reviews.llvm.org/D138630

Files:
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/Modules/override.m

Index: clang/test/Modules/override.m
===
--- /dev/null
+++ clang/test/Modules/override.m
@@ -0,0 +1,69 @@
+// UNSUPPORTED: -aix
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fsyntax-only -I%t/include %t/test.m \
+// RUN:-fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache -fmodule-name=CheckOverride
+
+// Test that if we have the same method in a different module, it's not an
+// override as it is the same method and it has the same DeclContext but a
+// different object in the memory.
+
+
+//--- include/CheckOverride.h
+@interface NSObject
+@end
+
+@interface CheckOverrideInterfaceOnly: NSObject
+- (void)potentialOverrideInterfaceOnly;
+@end
+
+@interface CheckOverrideCategoryOnly: NSObject
+@end
+@interface CheckOverrideCategoryOnly(CategoryOnly)
+- (void)potentialOverrideCategoryOnly;
+@end
+
+@interface CheckOverrideImplementationOfInterface: NSObject
+- (void)potentialOverrideImplementationOfInterface;
+@end
+
+@interface CheckOverrideImplementationOfCategory: NSObject
+@end
+@interface CheckOverrideImplementationOfCategory(CategoryImpl)
+- (void)potentialOverrideImplementationOfCategory;
+@end
+
+//--- include/Redirect.h
+// Ensure CheckOverride is imported as the module despite all `-fmodule-name` flags.
+#import 
+
+//--- include/module.modulemap
+module CheckOverride {
+  header "CheckOverride.h"
+}
+module Redirect {
+  header "Redirect.h"
+  export *
+}
+
+//--- test.m
+#import 
+#import 
+
+@implementation CheckOverrideImplementationOfInterface
+- (void)potentialOverrideImplementationOfInterface {}
+@end
+
+@implementation CheckOverrideImplementationOfCategory
+- (void)potentialOverrideImplementationOfCategory {}
+@end
+
+void triggerOverrideCheck(CheckOverrideInterfaceOnly *intfOnly,
+  CheckOverrideCategoryOnly *catOnly,
+  CheckOverrideImplementationOfInterface *intfImpl,
+  CheckOverrideImplementationOfCategory *catImpl) {
+  [intfOnly potentialOverrideInterfaceOnly];
+  [catOnly potentialOverrideCategoryOnly];
+  [intfImpl potentialOverrideImplementationOfInterface];
+  [catImpl potentialOverrideImplementationOfCategory];
+}
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4438,6 +4438,11 @@
 ResultTypeCompatibilityKind RTC) {
   if (!ObjCMethod)
 return;
+  auto IsMethodInCurrentClass = [CurrentClass](const ObjCMethodDecl *M) {
+// Checking canonical decl works across modules.
+return M->getClassInterface()->getCanonicalDecl() ==
+   CurrentClass->getCanonicalDecl();
+  };
   // Search for overridden methods and merge information down from them.
   OverrideSearch overrides(*this, ObjCMethod);
   // Keep track if the method overrides any method in the class's base classes,
@@ -4449,8 +4454,7 @@
   for (ObjCMethodDecl *overridden : overrides) {
 if (!hasOverriddenMethodsInBaseOrProtocol) {
   if (isa(overridden->getDeclContext()) ||
-  CurrentClass != overridden->getClassInterface() ||
-  overridden->isOverriding()) {
+  !IsMethodInCurrentClass(overridden) || overridden->isOverriding()) {
 CheckObjCMethodDirectOverrides(ObjCMethod, overridden);
 hasOverriddenMethodsInBaseOrProtocol = true;
   } else if (isa(ObjCMethod->getDeclContext())) {
@@ -4475,7 +4479,7 @@
   OverrideSearch overrides(*this, overridden);
   for (ObjCMethodDecl *SuperOverridden : overrides) {
 if (isa(SuperOverridden->getDeclContext()) ||
-CurrentClass != SuperOverridden->getClassInterface()) {
+!IsMethodInCurrentClass(SuperOverridden)) {
   CheckObjCMethodDirectOverrides(ObjCMethod, SuperOverridden);
   hasOverriddenMethodsInBaseOrProtocol = true;
   overridden->setOverriding(true);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 0314ba3 - [modules] Fix marking `ObjCMethodDecl::isOverriding` when there are no overrides.

2022-11-24 Thread Volodymyr Sapsai via cfe-commits

Author: Volodymyr Sapsai
Date: 2022-11-24T14:26:02-08:00
New Revision: 0314ba3acbabd8dc6d39b6d49798d22b6dc33802

URL: 
https://github.com/llvm/llvm-project/commit/0314ba3acbabd8dc6d39b6d49798d22b6dc33802
DIFF: 
https://github.com/llvm/llvm-project/commit/0314ba3acbabd8dc6d39b6d49798d22b6dc33802.diff

LOG: [modules] Fix marking `ObjCMethodDecl::isOverriding` when there are no 
overrides.

Incorrect `isOverriding` flag triggers the assertion
`!Overridden.empty()` in `ObjCMethodDecl::getOverriddenMethods` when a
method is marked as overriding but we cannot find any overrides.

When a method is declared in a category and defined in implementation,
we don't treat it as an override because it is the same method with
a separate declaration and a definition. But with modules we can find
a method declaration both in a modular category and a non-modular category
with different memory addresses. Thus we erroneously conclude the method
is overriding. Fix by comparing canonical declarations that are the same
for equal entities coming from different modules.

rdar://92845511

Differential Revision: https://reviews.llvm.org/D138630

Added: 
clang/test/Modules/override.m

Modified: 
clang/lib/Sema/SemaDeclObjC.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp
index 97dff49862bb5..0cd764552b932 100644
--- a/clang/lib/Sema/SemaDeclObjC.cpp
+++ b/clang/lib/Sema/SemaDeclObjC.cpp
@@ -4438,6 +4438,11 @@ void Sema::CheckObjCMethodOverrides(ObjCMethodDecl 
*ObjCMethod,
 ResultTypeCompatibilityKind RTC) {
   if (!ObjCMethod)
 return;
+  auto IsMethodInCurrentClass = [CurrentClass](const ObjCMethodDecl *M) {
+// Checking canonical decl works across modules.
+return M->getClassInterface()->getCanonicalDecl() ==
+   CurrentClass->getCanonicalDecl();
+  };
   // Search for overridden methods and merge information down from them.
   OverrideSearch overrides(*this, ObjCMethod);
   // Keep track if the method overrides any method in the class's base classes,
@@ -4449,8 +4454,7 @@ void Sema::CheckObjCMethodOverrides(ObjCMethodDecl 
*ObjCMethod,
   for (ObjCMethodDecl *overridden : overrides) {
 if (!hasOverriddenMethodsInBaseOrProtocol) {
   if (isa(overridden->getDeclContext()) ||
-  CurrentClass != overridden->getClassInterface() ||
-  overridden->isOverriding()) {
+  !IsMethodInCurrentClass(overridden) || overridden->isOverriding()) {
 CheckObjCMethodDirectOverrides(ObjCMethod, overridden);
 hasOverriddenMethodsInBaseOrProtocol = true;
   } else if (isa(ObjCMethod->getDeclContext())) {
@@ -4475,7 +4479,7 @@ void Sema::CheckObjCMethodOverrides(ObjCMethodDecl 
*ObjCMethod,
   OverrideSearch overrides(*this, overridden);
   for (ObjCMethodDecl *SuperOverridden : overrides) {
 if (isa(SuperOverridden->getDeclContext()) ||
-CurrentClass != SuperOverridden->getClassInterface()) {
+!IsMethodInCurrentClass(SuperOverridden)) {
   CheckObjCMethodDirectOverrides(ObjCMethod, SuperOverridden);
   hasOverriddenMethodsInBaseOrProtocol = true;
   overridden->setOverriding(true);

diff  --git a/clang/test/Modules/override.m b/clang/test/Modules/override.m
new file mode 100644
index 0..247c2be76ee14
--- /dev/null
+++ b/clang/test/Modules/override.m
@@ -0,0 +1,69 @@
+// UNSUPPORTED: -aix
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fsyntax-only -I%t/include %t/test.m \
+// RUN:-fmodules -fimplicit-module-maps 
-fmodules-cache-path=%t/modules.cache -fmodule-name=CheckOverride
+
+// Test that if we have the same method in a 
diff erent module, it's not an
+// override as it is the same method and it has the same DeclContext but a
+// 
diff erent object in the memory.
+
+
+//--- include/CheckOverride.h
+@interface NSObject
+@end
+
+@interface CheckOverrideInterfaceOnly: NSObject
+- (void)potentialOverrideInterfaceOnly;
+@end
+
+@interface CheckOverrideCategoryOnly: NSObject
+@end
+@interface CheckOverrideCategoryOnly(CategoryOnly)
+- (void)potentialOverrideCategoryOnly;
+@end
+
+@interface CheckOverrideImplementationOfInterface: NSObject
+- (void)potentialOverrideImplementationOfInterface;
+@end
+
+@interface CheckOverrideImplementationOfCategory: NSObject
+@end
+@interface CheckOverrideImplementationOfCategory(CategoryImpl)
+- (void)potentialOverrideImplementationOfCategory;
+@end
+
+//--- include/Redirect.h
+// Ensure CheckOverride is imported as the module despite all `-fmodule-name` 
flags.
+#import 
+
+//--- include/module.modulemap
+module CheckOverride {
+  header "CheckOverride.h"
+}
+module Redirect {
+  header "Redirect.h"
+  export *
+}
+
+//--- test.m
+#import 
+#import 
+
+@implementation CheckOverrideImplementationOfInterface

[PATCH] D137205: [clang-tidy] Add performance-unnecessary-copy-on-last-use check

2022-11-24 Thread Fabian Keßler via Phabricator via cfe-commits
Febbe updated this revision to Diff 477846.
Febbe marked an inline comment as done.
Febbe added a comment.
Herald added subscribers: kadircet, arphaman.

Replaces match clauses with `RecursiveASTVisistor`s

- doubled performance
- fixed also a bug in template spezialisations


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137205/new/

https://reviews.llvm.org/D137205

Files:
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/UnnecessaryCopyOnLastUseCheck.cpp
  clang-tools-extra/clang-tidy/performance/UnnecessaryCopyOnLastUseCheck.h
  clang-tools-extra/clangd/TidyProvider.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-copy-on-last-use.rst
  
clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-on-last-use.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-on-last-use.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-on-last-use.cpp
@@ -0,0 +1,270 @@
+// RUN: %check_clang_tidy %s -std=c++17 performance-unnecessary-copy-on-last-use %t
+// RUN: %check_clang_tidy %s -std=c++11 performance-unnecessary-copy-on-last-use %t
+// CHECK-FIXES: #include 
+
+struct Movable {
+  Movable() = default;
+  Movable(Movable const &) = default;
+  Movable(Movable &&) = default;
+  Movable =(Movable const &) = default;
+  Movable =(Movable &&) = default;
+  ~Movable();
+
+  void memberUse() {}
+};
+
+struct Copyable {
+  Copyable() = default;
+  Copyable(Copyable const &) = default;
+  Copyable(Copyable &&) = default;
+  Copyable =(Copyable const &) = default;
+  Copyable =(Copyable &&) = default;
+  ~Copyable() = default; 
+
+  void memberUse() {}
+};
+// static_assert(!std::is_trivially_copyable_v, "Movable must not be trivially copyable");
+
+void valueReceiver(Movable Mov);
+void constRefReceiver(Movable const );
+
+void valueTester() {
+  Movable Mov{};
+  valueReceiver(Mov);
+  valueReceiver(Mov);
+  // CHECK-MESSAGES: [[@LINE-1]]:17: warning: Parameter 'Mov' is copied on last use, consider moving it instead. [performance-unnecessary-copy-on-last-use]
+  // CHECK-FIXES: valueReceiver(std::move(Mov));
+  Mov = Movable{};
+  valueReceiver(Mov);
+  // CHECK-MESSAGES: [[@LINE-1]]:17: warning: Parameter 'Mov' is copied on last use, consider moving it instead. [performance-unnecessary-copy-on-last-use]
+  // CHECK-FIXES: valueReceiver(std::move(Mov));
+}
+
+void testUsageInBranch(bool Splitter) {
+  Movable Mov{};
+
+  valueReceiver(Mov);
+  if(Splitter){
+Mov.memberUse();
+  } else {
+Mov = Movable{};
+  }
+  valueReceiver(Mov);
+  // CHECK-MESSAGES: [[@LINE-1]]:17: warning: Parameter 'Mov' is copied on last use, consider moving it instead. [performance-unnecessary-copy-on-last-use]
+  // CHECK-FIXES: valueReceiver(std::move(Mov));
+
+  if(Splitter){
+Mov = Movable{};
+  } else {
+Mov = Movable{};
+  }
+  valueReceiver(Mov);
+  Mov.memberUse();
+}
+
+void testExplicitCopy() {
+  Movable Mov{};
+  constRefReceiver(Movable{Mov});
+  // CHECK-MESSAGES: [[@LINE-1]]:28: warning: Parameter 'Mov' is copied on last use, consider moving it instead. [performance-unnecessary-copy-on-last-use]
+  // CHECK-FIXES: constRefReceiver(Movable{std::move(Mov)});
+}
+
+Movable testReturn() {
+  Movable Mov{};
+  return Mov; // no warning, copy elision
+}
+
+Movable testReturn2(Movable && Mov, bool F) {
+  return F? Mov: Movable{}; 
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: Parameter 'Mov' is copied on last use, consider moving it instead. [performance-unnecessary-copy-on-last-use] 
+  // CHECK-FIXES: return F? std::move(Mov): Movable{};
+}
+
+void rValReferenceTester(Movable&& Mov) {
+  valueReceiver(Mov);
+  valueReceiver(Mov);
+  // CHECK-MESSAGES: [[@LINE-1]]:17: warning: Parameter 'Mov' is copied on last use, consider moving it instead. [performance-unnecessary-copy-on-last-use]
+  // CHECK-FIXES: valueReceiver(std::move(Mov));
+  Mov = Movable{};
+  valueReceiver(Mov);
+  // CHECK-MESSAGES: [[@LINE-1]]:17: warning: Parameter 'Mov' is copied on last use, consider moving it instead. [performance-unnecessary-copy-on-last-use]
+  // CHECK-FIXES: valueReceiver(std::move(Mov));
+}
+
+void referenceTester(Movable& Mov) {
+  valueReceiver(Mov);
+  valueReceiver(Mov);
+  Mov = Movable{};
+  valueReceiver(Mov);
+}
+
+void pointerTester(Movable* Mov) {
+  valueReceiver(*Mov);
+  valueReceiver(*Mov);
+  *Mov = Movable{};
+  valueReceiver(*Mov);
+}
+
+// Replacements in expansions from macros or of their parameters are buggy, so we don't fix them.
+// Todo (future): The source location of macro parameters might be fixed in the future
+#define FUN(Mov) valueReceiver((Mov))
+void 

[PATCH] D138274: Add version to all LLVM cmake package

2022-11-24 Thread Mariusz Ceier via Phabricator via cfe-commits
mceier added inline comments.



Comment at: clang/cmake/modules/ClangConfig.cmake.in:6
+set(LLVM_VERSION 
${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH})
+find_package(LLVM @LLVM_VERSION@ EXACT REQUIRED CONFIG
  HINTS "@CLANG_CONFIG_LLVM_CMAKE_DIR@")

I think instead of  `@LLVM_VERSION@` it should be `${LLVM_VERSION}` since 
`@LLVM_VERSION@` can be something like `16.0.0gitfce7a7aa` when 
LLVM_VERSION_SUFFIX is set and that value is incorrect according to cmake 
find_package:

>  find_package called with invalid argument "16.0.0gitfce7a7aa"

This error message is produced by cmake when configuring standalone build of 
lldb:

> CMake Error at /usr/lib/llvm/16/lib64/cmake/clang/ClangConfig.cmake:10 
> (find_package):
> find_package called with invalid argument "16.0.0gitfce7a7aa"
> Call Stack (most recent call first):
> cmake/modules/LLDBStandalone.cmake:10 (find_package)
> CMakeLists.txt:30 (include)





Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138274/new/

https://reviews.llvm.org/D138274

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138681: [AVR] Fix broken bitcast for aliases in non-zero address space

2022-11-24 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson accepted this revision.
arichardson added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138681/new/

https://reviews.llvm.org/D138681

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138681: [AVR] Fix broken bitcast for aliases in non-zero address space

2022-11-24 Thread Ayke via Phabricator via cfe-commits
aykevl updated this revision to Diff 477828.
aykevl added a comment.

- now _actually_ add that `CHECK` line


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138681/new/

https://reviews.llvm.org/D138681

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/avr/alias-avr.c


Index: clang/test/CodeGen/avr/alias-avr.c
===
--- clang/test/CodeGen/avr/alias-avr.c
+++ clang/test/CodeGen/avr/alias-avr.c
@@ -6,3 +6,8 @@
 
 // CHECK: @multiply ={{.*}} alias i16 (i16, i16), ptr addrspace(1) @mul
 int multiply(int x, int y) __attribute__((alias("mul")));
+
+// Make sure the correct address space is used when creating an alias that 
needs
+// a pointer cast.
+// CHECK: @smallmul = alias i8 (i16, i16), ptr addrspace(1) @mul
+char smallmul(int a, int b) __attribute__((alias("mul")));
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4002,7 +4002,8 @@
 // (If function is requested for a definition, we always need to create a 
new
 // function, not just return a bitcast.)
 if (!IsForDefinition)
-  return llvm::ConstantExpr::getBitCast(Entry, Ty->getPointerTo());
+  return llvm::ConstantExpr::getBitCast(
+  Entry, Ty->getPointerTo(Entry->getAddressSpace()));
   }
 
   // This function doesn't have a complete type (for example, the return


Index: clang/test/CodeGen/avr/alias-avr.c
===
--- clang/test/CodeGen/avr/alias-avr.c
+++ clang/test/CodeGen/avr/alias-avr.c
@@ -6,3 +6,8 @@
 
 // CHECK: @multiply ={{.*}} alias i16 (i16, i16), ptr addrspace(1) @mul
 int multiply(int x, int y) __attribute__((alias("mul")));
+
+// Make sure the correct address space is used when creating an alias that needs
+// a pointer cast.
+// CHECK: @smallmul = alias i8 (i16, i16), ptr addrspace(1) @mul
+char smallmul(int a, int b) __attribute__((alias("mul")));
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4002,7 +4002,8 @@
 // (If function is requested for a definition, we always need to create a new
 // function, not just return a bitcast.)
 if (!IsForDefinition)
-  return llvm::ConstantExpr::getBitCast(Entry, Ty->getPointerTo());
+  return llvm::ConstantExpr::getBitCast(
+  Entry, Ty->getPointerTo(Entry->getAddressSpace()));
   }
 
   // This function doesn't have a complete type (for example, the return
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138681: [AVR] Fix broken bitcast for aliases in non-zero address space

2022-11-24 Thread Ayke via Phabricator via cfe-commits
aykevl updated this revision to Diff 477827.
aykevl added a comment.

- add `CHECK:` line that I forgot


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138681/new/

https://reviews.llvm.org/D138681

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/avr/alias-avr.c


Index: clang/test/CodeGen/avr/alias-avr.c
===
--- clang/test/CodeGen/avr/alias-avr.c
+++ clang/test/CodeGen/avr/alias-avr.c
@@ -6,3 +6,7 @@
 
 // CHECK: @multiply ={{.*}} alias i16 (i16, i16), ptr addrspace(1) @mul
 int multiply(int x, int y) __attribute__((alias("mul")));
+
+// Make sure the correct address space is used when creating an alias that 
needs
+// a pointer cast.
+char smallmul(int a, int b) __attribute__((alias("mul")));
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4002,7 +4002,8 @@
 // (If function is requested for a definition, we always need to create a 
new
 // function, not just return a bitcast.)
 if (!IsForDefinition)
-  return llvm::ConstantExpr::getBitCast(Entry, Ty->getPointerTo());
+  return llvm::ConstantExpr::getBitCast(
+  Entry, Ty->getPointerTo(Entry->getAddressSpace()));
   }
 
   // This function doesn't have a complete type (for example, the return


Index: clang/test/CodeGen/avr/alias-avr.c
===
--- clang/test/CodeGen/avr/alias-avr.c
+++ clang/test/CodeGen/avr/alias-avr.c
@@ -6,3 +6,7 @@
 
 // CHECK: @multiply ={{.*}} alias i16 (i16, i16), ptr addrspace(1) @mul
 int multiply(int x, int y) __attribute__((alias("mul")));
+
+// Make sure the correct address space is used when creating an alias that needs
+// a pointer cast.
+char smallmul(int a, int b) __attribute__((alias("mul")));
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4002,7 +4002,8 @@
 // (If function is requested for a definition, we always need to create a new
 // function, not just return a bitcast.)
 if (!IsForDefinition)
-  return llvm::ConstantExpr::getBitCast(Entry, Ty->getPointerTo());
+  return llvm::ConstantExpr::getBitCast(
+  Entry, Ty->getPointerTo(Entry->getAddressSpace()));
   }
 
   // This function doesn't have a complete type (for example, the return
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138681: [AVR] Fix broken bitcast for aliases in non-zero address space

2022-11-24 Thread Ayke via Phabricator via cfe-commits
aykevl created this revision.
aykevl added reviewers: benshi001, dylanmckay, rjmccall, MaskRay.
Herald added subscribers: jeroen.dobbelaere, StephenFan, Jim, arichardson.
Herald added a project: All.
aykevl requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This was triggered by some code in picolibc. The minimal version looks like 
this:

  double infinity(void) {
 return 5;
  }
  
  extern long double infinityl() __attribute__((__alias__("infinity")));

These two declarations have a different type (not because of the 'long double', 
which is also 'double' in IR, but because infinityl has no `(void)` and is 
therefore a variadic function). This led to a crash in the bitcast which 
assumed address space 0.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138681

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/avr/alias-avr.c


Index: clang/test/CodeGen/avr/alias-avr.c
===
--- clang/test/CodeGen/avr/alias-avr.c
+++ clang/test/CodeGen/avr/alias-avr.c
@@ -6,3 +6,7 @@
 
 // CHECK: @multiply ={{.*}} alias i16 (i16, i16), ptr addrspace(1) @mul
 int multiply(int x, int y) __attribute__((alias("mul")));
+
+// Make sure the correct address space is used when creating an alias that 
needs
+// a pointer cast.
+char smallmul(int a, int b) __attribute__((alias("mul")));
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4002,7 +4002,8 @@
 // (If function is requested for a definition, we always need to create a 
new
 // function, not just return a bitcast.)
 if (!IsForDefinition)
-  return llvm::ConstantExpr::getBitCast(Entry, Ty->getPointerTo());
+  return llvm::ConstantExpr::getBitCast(
+  Entry, Ty->getPointerTo(Entry->getAddressSpace()));
   }
 
   // This function doesn't have a complete type (for example, the return


Index: clang/test/CodeGen/avr/alias-avr.c
===
--- clang/test/CodeGen/avr/alias-avr.c
+++ clang/test/CodeGen/avr/alias-avr.c
@@ -6,3 +6,7 @@
 
 // CHECK: @multiply ={{.*}} alias i16 (i16, i16), ptr addrspace(1) @mul
 int multiply(int x, int y) __attribute__((alias("mul")));
+
+// Make sure the correct address space is used when creating an alias that needs
+// a pointer cast.
+char smallmul(int a, int b) __attribute__((alias("mul")));
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4002,7 +4002,8 @@
 // (If function is requested for a definition, we always need to create a new
 // function, not just return a bitcast.)
 if (!IsForDefinition)
-  return llvm::ConstantExpr::getBitCast(Entry, Ty->getPointerTo());
+  return llvm::ConstantExpr::getBitCast(
+  Entry, Ty->getPointerTo(Entry->getAddressSpace()));
   }
 
   // This function doesn't have a complete type (for example, the return
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138630: [modules] Fix marking `ObjCMethodDecl::isOverriding` when there a no overrides.

2022-11-24 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added a comment.

Thanks for the quick review! During the commit I will also fix the typo s/there 
a no overrides/there are no overrides/




Comment at: clang/test/Modules/override.m:30-34
+@interface CheckOverrideImplementationOfCategory: NSObject
+@end
+@interface CheckOverrideImplementationOfCategory(CategoryImpl)
+- (void)potentialOverrideImplementationOfCategory;
+@end

Actually, only this interface triggers the assertion. Added the rest for 
completeness and for better coverage.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138630/new/

https://reviews.llvm.org/D138630

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D131963: [libc++] Add custom clang-tidy checks

2022-11-24 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment.
Herald added a reviewer: njames93.

Let's make those checks optional until we can figure out the packaging stuff, 
and land this (with green CI). This is going to become way too stale otherwise.




Comment at: clang/cmake/modules/ClangConfigVersion.cmake.in:1
+set(PACKAGE_VERSION "@PACKAGE_VERSION@")
+

This has been added in `main` now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131963/new/

https://reviews.llvm.org/D131963

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138677: [Lex] Fix suggested spelling of /usr/bin/../include/foo

2022-11-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: clang/lib/Lex/HeaderSearch.cpp:1932
+  
+  llvm::SmallString<32> FilePath(File.begin(), File.end());
+  path::remove_dots(FilePath, /*remove_dot_dot=*/true);

nit: you can change the method signature to take in a SmallString directly. 
there's an implicit constructor and any caller that already has a copy that 
they're throwing away can pass it here instead.



Comment at: clang/lib/Lex/HeaderSearch.cpp:1932
+  
+  llvm::SmallString<32> FilePath(File.begin(), File.end());
+  path::remove_dots(FilePath, /*remove_dot_dot=*/true);

kadircet wrote:
> nit: you can change the method signature to take in a SmallString directly. 
> there's an implicit constructor and any caller that already has a copy that 
> they're throwing away can pass it here instead.
this is same as `llvm::SmallString<32> FilePath(File);`



Comment at: clang/lib/Lex/HeaderSearch.cpp:1950
   if (DI == DE) {
-// Dir is a prefix of File, up to '.' components and choice of path
-// separators.
+// Dir is a prefix of File, up to choice of path // separators.
 unsigned PrefixLength = NI - path::begin(File);

looks like reflow went wrong here, drop `//`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138677/new/

https://reviews.llvm.org/D138677

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 5a90edb - [clang-fuzzer] Add missing dependency

2022-11-24 Thread Arthur Eubanks via cfe-commits

Author: Arthur Eubanks
Date: 2022-11-24T09:20:47-08:00
New Revision: 5a90edb3c825b43f7af31eab3284687ee08474ad

URL: 
https://github.com/llvm/llvm-project/commit/5a90edb3c825b43f7af31eab3284687ee08474ad
DIFF: 
https://github.com/llvm/llvm-project/commit/5a90edb3c825b43f7af31eab3284687ee08474ad.diff

LOG: [clang-fuzzer] Add missing dependency

Added: 


Modified: 
clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt

Removed: 




diff  --git a/clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt 
b/clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt
index b00baee3c4f2..0c5bf984c0e6 100644
--- a/clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt
+++ b/clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt
@@ -4,6 +4,7 @@ set(LLVM_LINK_COMPONENTS
   Core
   ExecutionEngine
   IPO
+  IRPrinter
   IRReader
   MC
   MCJIT



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 66b1f6b - Reland [clang-fuzzer] Use new pass manager for optimizing IR

2022-11-24 Thread Arthur Eubanks via cfe-commits

Author: Arthur Eubanks
Date: 2022-11-24T09:12:39-08:00
New Revision: 66b1f6bba51536ff8f75c84adf1df63894016b7d

URL: 
https://github.com/llvm/llvm-project/commit/66b1f6bba51536ff8f75c84adf1df63894016b7d
DIFF: 
https://github.com/llvm/llvm-project/commit/66b1f6bba51536ff8f75c84adf1df63894016b7d.diff

LOG: Reland [clang-fuzzer] Use new pass manager for optimizing IR

With CMakeLists.txt fix.

Added: 


Modified: 
clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp

Removed: 




diff  --git a/clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt 
b/clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt
index 9ceb1d3318283..b00baee3c4f2d 100644
--- a/clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt
+++ b/clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt
@@ -8,6 +8,7 @@ set(LLVM_LINK_COMPONENTS
   MC
   MCJIT
   Object
+  Passes
   RuntimeDyld
   SelectionDAG
   Support

diff  --git a/clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp 
b/clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
index aa38b5e4aa26e..5c9066c148d61 100644
--- a/clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
+++ b/clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
@@ -30,34 +30,28 @@
 #include "llvm/ExecutionEngine/SectionMemoryManager.h"
 #include "llvm/IR/IRPrintingPasses.h"
 #include "llvm/IR/LLVMContext.h"
-#include "llvm/IR/LegacyPassManager.h"
-#include "llvm/IR/LegacyPassNameParser.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Verifier.h"
+#include "llvm/IRPrinter/IRPrintingPasses.h"
 #include "llvm/IRReader/IRReader.h"
 #include "llvm/MC/TargetRegistry.h"
-#include "llvm/Pass.h"
-#include "llvm/PassRegistry.h"
+#include "llvm/Passes/OptimizationLevel.h"
+#include "llvm/Passes/PassBuilder.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Target/TargetMachine.h"
-#include "llvm/Transforms/IPO.h"
-#include "llvm/Transforms/IPO/PassManagerBuilder.h"
-#include "llvm/Transforms/Vectorize.h"
 
 using namespace llvm;
 
-static codegen::RegisterCodeGenFlags CGF;
-
 // Define a type for the functions that are compiled and executed
 typedef void (*LLVMFunc)(int*, int*, int*, int);
 
 // Helper function to parse command line args and find the optimization level
-static void getOptLevel(const std::vector ,
-  CodeGenOpt::Level ) {
+static CodeGenOpt::Level
+getOptLevel(const std::vector ) {
   // Find the optimization level from the command line args
-  OLvl = CodeGenOpt::Default;
+  CodeGenOpt::Level OLvl = CodeGenOpt::Default;
   for (auto  : ExtraArgs) {
 if (A[0] == '-' && A[1] == 'O') {
   switch(A[2]) {
@@ -71,6 +65,7 @@ static void getOptLevel(const std::vector 
,
   }
 }
   }
+  return OLvl;
 }
 
 static void ErrorAndExit(std::string message) {
@@ -80,16 +75,45 @@ static void ErrorAndExit(std::string message) {
 
 // Helper function to add optimization passes to the TargetMachine at the 
 // specified optimization level, OptLevel
-static void AddOptimizationPasses(legacy::PassManagerBase ,
-  CodeGenOpt::Level OptLevel,
-  unsigned SizeLevel) {
-  // Create and initialize a PassManagerBuilder
-  PassManagerBuilder Builder;
-  Builder.OptLevel = OptLevel;
-  Builder.SizeLevel = SizeLevel;
-  Builder.Inliner = createFunctionInliningPass(OptLevel, SizeLevel, false);
-  Builder.LoopVectorize = true;
-  Builder.populateModulePassManager(MPM);
+static void RunOptimizationPasses(raw_ostream , Module ,
+  CodeGenOpt::Level OptLevel) {
+  llvm::OptimizationLevel OL;
+  switch (OptLevel) {
+  case CodeGenOpt::None:
+OL = OptimizationLevel::O0;
+break;
+  case CodeGenOpt::Less:
+OL = OptimizationLevel::O1;
+break;
+  case CodeGenOpt::Default:
+OL = OptimizationLevel::O2;
+break;
+  case CodeGenOpt::Aggressive:
+OL = OptimizationLevel::O3;
+break;
+  }
+
+  LoopAnalysisManager LAM;
+  FunctionAnalysisManager FAM;
+  CGSCCAnalysisManager CGAM;
+  ModuleAnalysisManager MAM;
+
+  PassBuilder PB;
+
+  PB.registerModuleAnalyses(MAM);
+  PB.registerCGSCCAnalyses(CGAM);
+  PB.registerFunctionAnalyses(FAM);
+  PB.registerLoopAnalyses(LAM);
+  PB.crossRegisterProxies(LAM, FAM, CGAM, MAM);
+
+  ModulePassManager MPM;
+  if (OL == OptimizationLevel::O0)
+MPM = PB.buildO0DefaultPipeline(OL);
+  else
+MPM = PB.buildPerModuleDefaultPipeline(OL);
+  MPM.addPass(PrintModulePass(OS));
+
+  MPM.run(M, MAM);
 }
 
 // Mimics the opt tool to run an optimization pass over the provided IR
@@ -120,24 +144,10 @@ static std::string OptLLVM(const std::string , 
CodeGenOpt::Level OLvl) {
   codegen::setFunctionAttributes(codegen::getCPUStr(),
  codegen::getFeaturesStr(), *M);
 
-  legacy::PassManager Passes;
-  
-  

[PATCH] D137213: [clang][modules] NFCI: Pragma diagnostic mappings: write/read FileID instead of SourceLocation

2022-11-24 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment.

I just realized @jansvoboda11 is probably out on holiday this week (IIRC, Apple 
usually gets this week off). Since this was committed almost a month ago, I'm 
guessing this isn't enough of a blocker that we need to revert rather than wait 
until next week (and there are some commits on top that rely on this!). But 
probably reverting the stack would be an option if it's critical.

In the meantime, I'll try to help.




Comment at: clang/lib/Serialization/ASTReader.cpp:6343
  "Invalid data, missing pragma diagnostic states");
-  SourceLocation Loc = ReadSourceLocation(F, Record[Idx++]);
-  auto IDAndOffset = SourceMgr.getDecomposedLoc(Loc);
-  assert(IDAndOffset.first.isValid() && "invalid FileID for transition");
-  assert(IDAndOffset.second == 0 && "not a start location for a FileID");
+  FileID FID = ReadFileID(F, Record, Idx);
+  assert(FID.isValid() && "invalid FileID for transition");

eaeltsin wrote:
> This doesn't work as before, likely because ReadFileID doesn't do 
> TranslateSourceLocation.
> 
> Our tests fail.
> 
> I tried calling TranslateSourceLocation here and the tests passed:
> ```
>   SourceLocation Loc = Diag.SourceMgr->getComposedLoc(FID, 0);
>   SourceLocation Loc2 = TranslateSourceLocation(F, Loc);
>   auto IDAndOffset = SourceMgr.getDecomposedLoc(Loc2);
> 
>   // Note that we don't need to set up Parent/ParentOffset here, because
>   // we won't be changing the diagnostic state within imported FileIDs
>   // (other than perhaps appending to the main source file, which has no
>   // parent).
>   auto  = Diag.DiagStatesByLoc.Files[IDAndOffset.first];
> ```
> 
> Sorry I don't know the codebase, so this fix is definitely ugly :) But it 
> shows the problem.
> 
I don't think that's the issue, since `ReadFileID()` calls `TranslateFileID`, 
which should seems like it should be equivalent.

However, I notice that the post-increment for `Idx` got dropped! Can you try 
replacing the line of code with the following and see if that fixes your tests 
(without any extra TranslateSourceLocation logic)?
```
lang=c++
FileID FID = ReadFileID(F, Record, Idx++);
```

If so, maybe you can contribute that fix with a reduced testcase? I suggest 
adding me, @vsapsai, @Bigcheese, and @jansvoboda11 as reviewers.

@alexfh, maybe you can check if this fixes your tests as well?

(If this is the issue, it's a bit surprising we don't have existing tests 
covering this case... and embarrassing I missed it when reviewing initially!)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137213/new/

https://reviews.llvm.org/D137213

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] d75bd5e - Revert "[clang-fuzzer] Use new pass manager for optimizing IR"

2022-11-24 Thread Arthur Eubanks via cfe-commits

Author: Arthur Eubanks
Date: 2022-11-24T08:57:19-08:00
New Revision: d75bd5e8bb62eddaccc256eef4e10130d6018ec8

URL: 
https://github.com/llvm/llvm-project/commit/d75bd5e8bb62eddaccc256eef4e10130d6018ec8
DIFF: 
https://github.com/llvm/llvm-project/commit/d75bd5e8bb62eddaccc256eef4e10130d6018ec8.diff

LOG: Revert "[clang-fuzzer] Use new pass manager for optimizing IR"

This reverts commit a46a746cfa08a72f9e9188451ed5cac2f77d5237.

Breaks bots, e.g. https://lab.llvm.org/buildbot#builders/121/builds/25511.

Added: 


Modified: 
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp

Removed: 




diff  --git a/clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp 
b/clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
index 5c9066c148d61..aa38b5e4aa26e 100644
--- a/clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
+++ b/clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
@@ -30,28 +30,34 @@
 #include "llvm/ExecutionEngine/SectionMemoryManager.h"
 #include "llvm/IR/IRPrintingPasses.h"
 #include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/LegacyPassManager.h"
+#include "llvm/IR/LegacyPassNameParser.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Verifier.h"
-#include "llvm/IRPrinter/IRPrintingPasses.h"
 #include "llvm/IRReader/IRReader.h"
 #include "llvm/MC/TargetRegistry.h"
-#include "llvm/Passes/OptimizationLevel.h"
-#include "llvm/Passes/PassBuilder.h"
+#include "llvm/Pass.h"
+#include "llvm/PassRegistry.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Target/TargetMachine.h"
+#include "llvm/Transforms/IPO.h"
+#include "llvm/Transforms/IPO/PassManagerBuilder.h"
+#include "llvm/Transforms/Vectorize.h"
 
 using namespace llvm;
 
+static codegen::RegisterCodeGenFlags CGF;
+
 // Define a type for the functions that are compiled and executed
 typedef void (*LLVMFunc)(int*, int*, int*, int);
 
 // Helper function to parse command line args and find the optimization level
-static CodeGenOpt::Level
-getOptLevel(const std::vector ) {
+static void getOptLevel(const std::vector ,
+  CodeGenOpt::Level ) {
   // Find the optimization level from the command line args
-  CodeGenOpt::Level OLvl = CodeGenOpt::Default;
+  OLvl = CodeGenOpt::Default;
   for (auto  : ExtraArgs) {
 if (A[0] == '-' && A[1] == 'O') {
   switch(A[2]) {
@@ -65,7 +71,6 @@ getOptLevel(const std::vector ) {
   }
 }
   }
-  return OLvl;
 }
 
 static void ErrorAndExit(std::string message) {
@@ -75,45 +80,16 @@ static void ErrorAndExit(std::string message) {
 
 // Helper function to add optimization passes to the TargetMachine at the 
 // specified optimization level, OptLevel
-static void RunOptimizationPasses(raw_ostream , Module ,
-  CodeGenOpt::Level OptLevel) {
-  llvm::OptimizationLevel OL;
-  switch (OptLevel) {
-  case CodeGenOpt::None:
-OL = OptimizationLevel::O0;
-break;
-  case CodeGenOpt::Less:
-OL = OptimizationLevel::O1;
-break;
-  case CodeGenOpt::Default:
-OL = OptimizationLevel::O2;
-break;
-  case CodeGenOpt::Aggressive:
-OL = OptimizationLevel::O3;
-break;
-  }
-
-  LoopAnalysisManager LAM;
-  FunctionAnalysisManager FAM;
-  CGSCCAnalysisManager CGAM;
-  ModuleAnalysisManager MAM;
-
-  PassBuilder PB;
-
-  PB.registerModuleAnalyses(MAM);
-  PB.registerCGSCCAnalyses(CGAM);
-  PB.registerFunctionAnalyses(FAM);
-  PB.registerLoopAnalyses(LAM);
-  PB.crossRegisterProxies(LAM, FAM, CGAM, MAM);
-
-  ModulePassManager MPM;
-  if (OL == OptimizationLevel::O0)
-MPM = PB.buildO0DefaultPipeline(OL);
-  else
-MPM = PB.buildPerModuleDefaultPipeline(OL);
-  MPM.addPass(PrintModulePass(OS));
-
-  MPM.run(M, MAM);
+static void AddOptimizationPasses(legacy::PassManagerBase ,
+  CodeGenOpt::Level OptLevel,
+  unsigned SizeLevel) {
+  // Create and initialize a PassManagerBuilder
+  PassManagerBuilder Builder;
+  Builder.OptLevel = OptLevel;
+  Builder.SizeLevel = SizeLevel;
+  Builder.Inliner = createFunctionInliningPass(OptLevel, SizeLevel, false);
+  Builder.LoopVectorize = true;
+  Builder.populateModulePassManager(MPM);
 }
 
 // Mimics the opt tool to run an optimization pass over the provided IR
@@ -144,10 +120,24 @@ static std::string OptLLVM(const std::string , 
CodeGenOpt::Level OLvl) {
   codegen::setFunctionAttributes(codegen::getCPUStr(),
  codegen::getFeaturesStr(), *M);
 
+  legacy::PassManager Passes;
+  
+  Passes.add(new TargetLibraryInfoWrapperPass(ModuleTriple));
+  Passes.add(createTargetTransformInfoWrapperPass(TM->getTargetIRAnalysis()));
+
+  LLVMTargetMachine  = static_cast(*TM);
+  Passes.add(LTM.createPassConfig(Passes));
+
+  Passes.add(createVerifierPass());
+
+  AddOptimizationPasses(Passes, OLvl, 0);
+
   // Add a pass that writes the 

[PATCH] D138679: [clang][CodeGen] Add default attributes to __clang_call_terminate

2022-11-24 Thread John Brawn via Phabricator via cfe-commits
john.brawn created this revision.
john.brawn added reviewers: rsmith, rjmccall, lebedev.ri, miyuki.
Herald added a project: All.
john.brawn requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When generating __clang_call_terminate use SetLLVMFunctionAttributes to set the 
default function attributes, like we do for all the other functions generated 
by clang. This fixes a problem where target features from the command line 
weren't being applied to this function.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138679

Files:
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/arm-generated-fn-attr.cpp
  clang/test/CodeGenCXX/dllimport-runtime-fns.cpp
  clang/test/CodeGenCXX/exceptions.cpp
  clang/test/CodeGenCXX/runtime-dllstorage.cpp

Index: clang/test/CodeGenCXX/runtime-dllstorage.cpp
===
--- clang/test/CodeGenCXX/runtime-dllstorage.cpp
+++ clang/test/CodeGenCXX/runtime-dllstorage.cpp
@@ -116,7 +116,7 @@
 
 // CHECK-IA-DAG: @_ZTH1t = dso_local alias void (), ptr @__tls_init
 // CHECK-IA-DAG: declare dso_local i32 @__gxx_personality_v0(...)
-// CHECK-IA-DAG: define linkonce_odr hidden void @__clang_call_terminate(ptr %0)
+// CHECK-IA-DAG: define linkonce_odr hidden void @__clang_call_terminate(ptr noundef %0)
 
 // CHECK-DYNAMIC-IA-DAG: declare dllimport i32 @__cxa_thread_atexit(ptr, ptr, ptr)
 // CHECK-DYNAMIC-IA-DAG: declare dllimport i32 @__cxa_atexit(ptr, ptr, ptr)
Index: clang/test/CodeGenCXX/exceptions.cpp
===
--- clang/test/CodeGenCXX/exceptions.cpp
+++ clang/test/CodeGenCXX/exceptions.cpp
@@ -633,4 +633,4 @@
 
 }
 
-// CHECK98: attributes [[NI_NR_NUW]] = { noinline noreturn nounwind }
+// CHECK98: attributes [[NI_NR_NUW]] = { noinline noreturn nounwind {{.*}} }
Index: clang/test/CodeGenCXX/dllimport-runtime-fns.cpp
===
--- clang/test/CodeGenCXX/dllimport-runtime-fns.cpp
+++ clang/test/CodeGenCXX/dllimport-runtime-fns.cpp
@@ -28,14 +28,14 @@
 // MSVC: declare dso_local void @__std_terminate()
 
 // _ZSt9terminatev and __cxa_begin_catch should be marked dllimport.
-// ITANIUM-LABEL: define linkonce_odr hidden void @__clang_call_terminate(ptr %0)
+// ITANIUM-LABEL: define linkonce_odr hidden void @__clang_call_terminate(ptr noundef %0)
 // ITANIUM: call ptr @__cxa_begin_catch({{.*}})
 // ITANIUM: call void @_ZSt9terminatev()
 // ITANIUM: declare dllimport ptr @__cxa_begin_catch(ptr)
 // ITANIUM: declare dllimport void @_ZSt9terminatev()
 
 // .. not for mingw.
-// GNU-LABEL: define linkonce_odr hidden void @__clang_call_terminate(ptr %0)
+// GNU-LABEL: define linkonce_odr hidden void @__clang_call_terminate(ptr noundef %0)
 // GNU: call ptr @__cxa_begin_catch({{.*}})
 // GNU: call void @_ZSt9terminatev()
 // GNU: declare dso_local ptr @__cxa_begin_catch(ptr)
Index: clang/test/CodeGenCXX/arm-generated-fn-attr.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/arm-generated-fn-attr.cpp
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -triple thumbv8.1m.main-none-eabi -target-feature +pacbti -fcxx-exceptions -fexceptions -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,CHECK-PACBTI
+// RUN: %clang_cc1 -triple thumbv8.1m.main-none-eabi -target-feature -pacbti -fcxx-exceptions -fexceptions -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,CHECK-NOPACBTI
+
+// Check that functions generated by clang have the correct attributes
+
+class Example {
+public:
+  Example();
+  int fn();
+};
+
+// Initialization of var1 causes __cxx_global_var_init and __tls_init to be generated
+thread_local Example var1;
+extern thread_local Example var2;
+extern void fn();
+
+int testfn() noexcept {
+  // Calling fn in a noexcept function causes __clang_call_terminate to be generated
+  fn();
+  // Use of var1 and var2 causes TLS wrapper functions to be generated
+  return var1.fn() + var2.fn();
+}
+
+// CHECK: define {{.*}} @__cxx_global_var_init() [[ATTR1:#[0-9]+]]
+// CHECK: define {{.*}} @__clang_call_terminate({{.*}}) [[ATTR2:#[0-9]+]]
+// CHECK: define {{.*}} @_ZTW4var1() [[ATTR3:#[0-9]+]]
+// CHECK: define {{.*}} @_ZTW4var2() [[ATTR3]]
+// CHECK: define {{.*}} @__tls_init() [[ATTR1]]
+
+// CHECK-PACBTI: attributes [[ATTR1]] = { {{.*}}"target-features"="+armv8.1-m.main,+pacbti,+thumb-mode"{{.*}} }
+// CHECK-PACBTI: attributes [[ATTR2]] = { {{.*}}"target-features"="+armv8.1-m.main,+pacbti,+thumb-mode"{{.*}} }
+// CHECK-PACBTI: attributes [[ATTR3]] = { {{.*}}"target-features"="+armv8.1-m.main,+pacbti,+thumb-mode"{{.*}} }
+
+// CHECK-NOPACBTI: attributes [[ATTR1]] = { {{.*}}"target-features"="+armv8.1-m.main,+thumb-mode,-pacbti"{{.*}} }
+// CHECK-NOPACBTI: attributes [[ATTR2]] = { {{.*}}"target-features"="+armv8.1-m.main,+thumb-mode,-pacbti"{{.*}} }
+// CHECK-NOPACBTI: attributes [[ATTR3]] 

[PATCH] D138678: [include-cleaner] Capture private headers in PragmaIncludes.

2022-11-24 Thread Viktoriia Bakalova via Phabricator via cfe-commits
VitaNuo created this revision.
Herald added a project: All.
VitaNuo requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Save file IDs of IWYU private headers and report them as private.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138678

Files:
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
  clang-tools-extra/include-cleaner/lib/Record.cpp
  clang-tools-extra/include-cleaner/unittests/RecordTest.cpp


Index: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -368,11 +368,18 @@
   Inputs.Code = R"cpp(
 #include "public.h"
   )cpp";
-  Inputs.ExtraFiles["public.h"] = "#include \"private.h\"";
+  Inputs.ExtraFiles["public.h"] = R"cpp(
+#include "private.h";
+#include "private2.h";
+  )cpp";
   Inputs.ExtraFiles["private.h"] = R"cpp(
 // IWYU pragma: private, include "public2.h"
 class Private {};
   )cpp";
+  Inputs.ExtraFiles["private2.h"] = R"cpp(
+// IWYU pragma: private
+class Private {};
+  )cpp";
   TestAST Processed = build();
   auto PrivateFE = Processed.fileManager().getFile("private.h");
   assert(PrivateFE);
@@ -380,6 +387,10 @@
   auto PublicFE = Processed.fileManager().getFile("public.h");
   assert(PublicFE);
   EXPECT_EQ(PI.getPublic(PublicFE.get()), ""); // no mapping.
+
+  auto Private2FE = Processed.fileManager().getFile("private2.h");
+  assert(Private2FE);
+  EXPECT_TRUE(PI.isPrivate(Private2FE.get()));
 }
 
 TEST_F(PragmaIncludeTest, IWYUExport) {
Index: clang-tools-extra/include-cleaner/lib/Record.cpp
===
--- clang-tools-extra/include-cleaner/lib/Record.cpp
+++ clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -263,6 +263,8 @@
 assert(ExportStack.back().Block);
 ExportStack.pop_back();
   }
+} else if (Pragma->startswith("private")) {
+  Out->IWYUPrivate.insert(SM.getFileEntryForID(CommentFID)->getUniqueID());
 }
 
 if (InMainFile) {
@@ -346,6 +348,10 @@
   return !NonSelfContainedFiles.contains(FE->getUniqueID());
 }
 
+bool PragmaIncludes::isPrivate(const FileEntry *FE) const {
+  return IWYUPrivate.contains(FE->getUniqueID());
+}
+
 std::unique_ptr RecordedAST::record() {
   class Recorder : public ASTConsumer {
 RecordedAST *Out;
Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
===
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
@@ -73,6 +73,10 @@
   /// Returns true if the given file is a self-contained file.
   bool isSelfContained(const FileEntry *File) const;
 
+  /// Returns true if the given file is marked with the IWYU private pragma
+  /// without public mapping.
+  bool isPrivate(const FileEntry *File) const;
+
 private:
   class RecordPragma;
   /// 1-based Line numbers for the #include directives of the main file that
@@ -88,6 +92,10 @@
   llvm::DenseMap
   IWYUPublic;
 
+  /// Contains all files marked with the IWYU private pragma that
+  /// do not have public header mapping
+  llvm::DenseSet IWYUPrivate;
+
   /// A reverse map from the underlying header to its exporter headers.
   //
   //  There's no way to get a FileEntry from a UniqueID, especially when it


Index: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -368,11 +368,18 @@
   Inputs.Code = R"cpp(
 #include "public.h"
   )cpp";
-  Inputs.ExtraFiles["public.h"] = "#include \"private.h\"";
+  Inputs.ExtraFiles["public.h"] = R"cpp(
+#include "private.h";
+#include "private2.h";
+  )cpp";
   Inputs.ExtraFiles["private.h"] = R"cpp(
 // IWYU pragma: private, include "public2.h"
 class Private {};
   )cpp";
+  Inputs.ExtraFiles["private2.h"] = R"cpp(
+// IWYU pragma: private
+class Private {};
+  )cpp";
   TestAST Processed = build();
   auto PrivateFE = Processed.fileManager().getFile("private.h");
   assert(PrivateFE);
@@ -380,6 +387,10 @@
   auto PublicFE = Processed.fileManager().getFile("public.h");
   assert(PublicFE);
   EXPECT_EQ(PI.getPublic(PublicFE.get()), ""); // no mapping.
+
+  auto Private2FE = Processed.fileManager().getFile("private2.h");
+  assert(Private2FE);
+  EXPECT_TRUE(PI.isPrivate(Private2FE.get()));
 }
 
 TEST_F(PragmaIncludeTest, IWYUExport) {
Index: clang-tools-extra/include-cleaner/lib/Record.cpp
===
--- 

[PATCH] D138677: [Lex] Fix suggested spelling of /usr/bin/../include/foo

2022-11-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added a project: All.
sammccall requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Since D60873  we remove dotdots from the 
search path entries, but not the
filenames we're matching against, so do the latter too.

Since this also removes (single) dots, drop the logic to skip over them.
(Some of this was already dead, some is newly dead).

See D138676  for motivation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138677

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/unittests/Lex/HeaderSearchTest.cpp


Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -150,6 +150,14 @@
 "z");
 }
 
+TEST_F(HeaderSearchTest, BothDotDots) {
+  addSearchDir("/x/../y/");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/x/../y/z",
+   /*WorkingDir=*/"",
+   /*MainFile=*/""),
+"z");
+}
+
 TEST_F(HeaderSearchTest, IncludeFromSameDirectory) {
   EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/y/z/t.h",
/*WorkingDir=*/"",
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1928,6 +1928,10 @@
 llvm::StringRef File, llvm::StringRef WorkingDir, llvm::StringRef MainFile,
 bool *IsSystem) {
   using namespace llvm::sys;
+  
+  llvm::SmallString<32> FilePath(File.begin(), File.end());
+  path::remove_dots(FilePath, /*remove_dot_dot=*/true);
+  File = FilePath;
 
   unsigned BestPrefixLength = 0;
   // Checks whether `Dir` is a strict path prefix of `File`. If so and that's
@@ -1941,19 +1945,9 @@
 Dir = DirPath;
 for (auto NI = path::begin(File), NE = path::end(File),
   DI = path::begin(Dir), DE = path::end(Dir);
- /*termination condition in loop*/; ++NI, ++DI) {
-  // '.' components in File are ignored.
-  while (NI != NE && *NI == ".")
-++NI;
-  if (NI == NE)
-break;
-
-  // '.' components in Dir are ignored.
-  while (DI != DE && *DI == ".")
-++DI;
+ NI != NE; ++NI, ++DI) {
   if (DI == DE) {
-// Dir is a prefix of File, up to '.' components and choice of path
-// separators.
+// Dir is a prefix of File, up to choice of path // separators.
 unsigned PrefixLength = NI - path::begin(File);
 if (PrefixLength > BestPrefixLength) {
   BestPrefixLength = PrefixLength;


Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -150,6 +150,14 @@
 "z");
 }
 
+TEST_F(HeaderSearchTest, BothDotDots) {
+  addSearchDir("/x/../y/");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/x/../y/z",
+   /*WorkingDir=*/"",
+   /*MainFile=*/""),
+"z");
+}
+
 TEST_F(HeaderSearchTest, IncludeFromSameDirectory) {
   EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/y/z/t.h",
/*WorkingDir=*/"",
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1928,6 +1928,10 @@
 llvm::StringRef File, llvm::StringRef WorkingDir, llvm::StringRef MainFile,
 bool *IsSystem) {
   using namespace llvm::sys;
+  
+  llvm::SmallString<32> FilePath(File.begin(), File.end());
+  path::remove_dots(FilePath, /*remove_dot_dot=*/true);
+  File = FilePath;
 
   unsigned BestPrefixLength = 0;
   // Checks whether `Dir` is a strict path prefix of `File`. If so and that's
@@ -1941,19 +1945,9 @@
 Dir = DirPath;
 for (auto NI = path::begin(File), NE = path::end(File),
   DI = path::begin(Dir), DE = path::end(Dir);
- /*termination condition in loop*/; ++NI, ++DI) {
-  // '.' components in File are ignored.
-  while (NI != NE && *NI == ".")
-++NI;
-  if (NI == NE)
-break;
-
-  // '.' components in Dir are ignored.
-  while (DI != DE && *DI == ".")
-++DI;
+ NI != NE; ++NI, ++DI) {
   if (DI == DE) {
-// Dir is a prefix of File, up to '.' components and choice of path
-// separators.
+// Dir is a prefix of File, up to choice of path // separators.
 unsigned PrefixLength = NI - path::begin(File);
 if 

[clang] a46a746 - [clang-fuzzer] Use new pass manager for optimizing IR

2022-11-24 Thread Arthur Eubanks via cfe-commits

Author: Arthur Eubanks
Date: 2022-11-24T08:39:31-08:00
New Revision: a46a746cfa08a72f9e9188451ed5cac2f77d5237

URL: 
https://github.com/llvm/llvm-project/commit/a46a746cfa08a72f9e9188451ed5cac2f77d5237
DIFF: 
https://github.com/llvm/llvm-project/commit/a46a746cfa08a72f9e9188451ed5cac2f77d5237.diff

LOG: [clang-fuzzer] Use new pass manager for optimizing IR

Added: 


Modified: 
clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp

Removed: 




diff  --git a/clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp 
b/clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
index aa38b5e4aa26e..5c9066c148d61 100644
--- a/clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
+++ b/clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
@@ -30,34 +30,28 @@
 #include "llvm/ExecutionEngine/SectionMemoryManager.h"
 #include "llvm/IR/IRPrintingPasses.h"
 #include "llvm/IR/LLVMContext.h"
-#include "llvm/IR/LegacyPassManager.h"
-#include "llvm/IR/LegacyPassNameParser.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Verifier.h"
+#include "llvm/IRPrinter/IRPrintingPasses.h"
 #include "llvm/IRReader/IRReader.h"
 #include "llvm/MC/TargetRegistry.h"
-#include "llvm/Pass.h"
-#include "llvm/PassRegistry.h"
+#include "llvm/Passes/OptimizationLevel.h"
+#include "llvm/Passes/PassBuilder.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Target/TargetMachine.h"
-#include "llvm/Transforms/IPO.h"
-#include "llvm/Transforms/IPO/PassManagerBuilder.h"
-#include "llvm/Transforms/Vectorize.h"
 
 using namespace llvm;
 
-static codegen::RegisterCodeGenFlags CGF;
-
 // Define a type for the functions that are compiled and executed
 typedef void (*LLVMFunc)(int*, int*, int*, int);
 
 // Helper function to parse command line args and find the optimization level
-static void getOptLevel(const std::vector ,
-  CodeGenOpt::Level ) {
+static CodeGenOpt::Level
+getOptLevel(const std::vector ) {
   // Find the optimization level from the command line args
-  OLvl = CodeGenOpt::Default;
+  CodeGenOpt::Level OLvl = CodeGenOpt::Default;
   for (auto  : ExtraArgs) {
 if (A[0] == '-' && A[1] == 'O') {
   switch(A[2]) {
@@ -71,6 +65,7 @@ static void getOptLevel(const std::vector 
,
   }
 }
   }
+  return OLvl;
 }
 
 static void ErrorAndExit(std::string message) {
@@ -80,16 +75,45 @@ static void ErrorAndExit(std::string message) {
 
 // Helper function to add optimization passes to the TargetMachine at the 
 // specified optimization level, OptLevel
-static void AddOptimizationPasses(legacy::PassManagerBase ,
-  CodeGenOpt::Level OptLevel,
-  unsigned SizeLevel) {
-  // Create and initialize a PassManagerBuilder
-  PassManagerBuilder Builder;
-  Builder.OptLevel = OptLevel;
-  Builder.SizeLevel = SizeLevel;
-  Builder.Inliner = createFunctionInliningPass(OptLevel, SizeLevel, false);
-  Builder.LoopVectorize = true;
-  Builder.populateModulePassManager(MPM);
+static void RunOptimizationPasses(raw_ostream , Module ,
+  CodeGenOpt::Level OptLevel) {
+  llvm::OptimizationLevel OL;
+  switch (OptLevel) {
+  case CodeGenOpt::None:
+OL = OptimizationLevel::O0;
+break;
+  case CodeGenOpt::Less:
+OL = OptimizationLevel::O1;
+break;
+  case CodeGenOpt::Default:
+OL = OptimizationLevel::O2;
+break;
+  case CodeGenOpt::Aggressive:
+OL = OptimizationLevel::O3;
+break;
+  }
+
+  LoopAnalysisManager LAM;
+  FunctionAnalysisManager FAM;
+  CGSCCAnalysisManager CGAM;
+  ModuleAnalysisManager MAM;
+
+  PassBuilder PB;
+
+  PB.registerModuleAnalyses(MAM);
+  PB.registerCGSCCAnalyses(CGAM);
+  PB.registerFunctionAnalyses(FAM);
+  PB.registerLoopAnalyses(LAM);
+  PB.crossRegisterProxies(LAM, FAM, CGAM, MAM);
+
+  ModulePassManager MPM;
+  if (OL == OptimizationLevel::O0)
+MPM = PB.buildO0DefaultPipeline(OL);
+  else
+MPM = PB.buildPerModuleDefaultPipeline(OL);
+  MPM.addPass(PrintModulePass(OS));
+
+  MPM.run(M, MAM);
 }
 
 // Mimics the opt tool to run an optimization pass over the provided IR
@@ -120,24 +144,10 @@ static std::string OptLLVM(const std::string , 
CodeGenOpt::Level OLvl) {
   codegen::setFunctionAttributes(codegen::getCPUStr(),
  codegen::getFeaturesStr(), *M);
 
-  legacy::PassManager Passes;
-  
-  Passes.add(new TargetLibraryInfoWrapperPass(ModuleTriple));
-  Passes.add(createTargetTransformInfoWrapperPass(TM->getTargetIRAnalysis()));
-
-  LLVMTargetMachine  = static_cast(*TM);
-  Passes.add(LTM.createPassConfig(Passes));
-
-  Passes.add(createVerifierPass());
-
-  AddOptimizationPasses(Passes, OLvl, 0);
-
   // Add a pass that writes the optimized IR to an output stream
   std::string outString;
   raw_string_ostream OS(outString);
-  Passes.add(createPrintModulePass(OS, "", 

[PATCH] D138676: [include-cleaner] HTMLReport shows headers that would be inserted

2022-11-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 477809.
sammccall added a comment.

fix bold/italic of header-to-insert on symbol refs


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138676/new/

https://reviews.llvm.org/D138676

Files:
  clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
  clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
  clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp

Index: clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
===
--- clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
+++ clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
@@ -71,8 +71,10 @@
<< ": " << EC.message() << "\n";
   exit(1);
 }
-writeHTMLReport(AST.Ctx->getSourceManager().getMainFileID(), PP.Includes,
-AST.Roots, PP.MacroReferences, *AST.Ctx, , OS);
+writeHTMLReport(
+AST.Ctx->getSourceManager().getMainFileID(), PP.Includes, AST.Roots,
+PP.MacroReferences, *AST.Ctx,
+getCompilerInstance().getPreprocessor().getHeaderSearchInfo(), , OS);
   }
 };
 
Index: clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
===
--- clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
+++ clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
@@ -18,6 +18,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
 #include "llvm/Support/ScopedPrinter.h"
@@ -29,7 +30,7 @@
 constexpr llvm::StringLiteral CSS = R"css(
   body { margin: 0; }
   pre { line-height: 1.5em; counter-reset: line; margin: 0; }
-  pre .line { counter-increment: line; }
+  pre .line:not(.added) { counter-increment: line; }
   pre .line::before {
 content: counter(line);
 display: inline-block;
@@ -37,6 +38,7 @@
 text-align: right;
 width: 3em; padding-right: 0.5em; margin-right: 0.5em;
   }
+  pre .line.added::before { content: '+' }
   .ref, .inc { text-decoration: underline; color: #008; }
   .sel { position: relative; cursor: pointer; }
   .ref.implicit { background-color: #ff8; }
@@ -52,6 +54,7 @@
   #hover .target.implicit, .provides .implicit { background-color: #bbb; }
   #hover .target.ambiguous, .provides .ambiguous { background-color: #caf; }
   .missing, .unused { background-color: #faa !important; }
+  .inserted { background-color: #bea !important; }
   .semiused { background-color: #888 !important; }
   #hover th { color: #008; text-align: right; padding-right: 0.5em; }
   #hover .target:not(:first-child) {
@@ -59,6 +62,8 @@
 padding-top: 1em;
 border-top: 1px solid #444;
   }
+  .ref.missing #hover .insert { font-weight: bold; }
+  .ref:not(.missing) #hover .insert { font-style: italic; }
 )css";
 
 constexpr llvm::StringLiteral JS = R"js(
@@ -128,6 +133,7 @@
   llvm::raw_ostream 
   const ASTContext 
   const SourceManager 
+  HeaderSearch 
   const RecordedPP::RecordedIncludes 
   const PragmaIncludes *PI;
   FileID MainFile;
@@ -142,6 +148,7 @@
 SmallVector Headers;
 SmallVector Includes;
 bool Satisfied = false; // Is the include present?
+std::string Insert; // If we had no includes, what would we insert?
   };
   std::vector Targets;
   // Points within the main file that reference a Target.
@@ -157,6 +164,7 @@
   };
   std::vector Refs;
   llvm::DenseMap> IncludeRefs;
+  llvm::StringMap> Insertion;
 
   llvm::StringRef includeType(const Include *I) {
 auto  = IncludeRefs[I];
@@ -169,8 +177,24 @@
 return "semiused";
   }
 
+  std::string spellHeader(const Header ) {
+switch (H.kind()) {
+case Header::Physical: {
+  bool IsSystem = false;
+  std::string Path = HS.suggestPathToFileForDiagnostics(
+  H.physical(), MainFE->tryGetRealPathName(), );
+  return IsSystem ? "<" + Path + ">" : "\"" + Path + "\"";
+}
+case Header::Standard:
+  return H.standard().name().str();
+case Header::Verbatim:
+  return H.verbatim().str();
+}
+llvm_unreachable("Unknown Header kind");
+  }
+
   Target makeTarget(const SymbolReference ) {
-Target T{SR.Target, SR.RT, {}, {}, {}};
+Target T{SR.Target, SR.RT, {}, {}, {}, {}, {}};
 
 // Duplicates logic from walkUsed(), which doesn't expose SymbolLocations.
 // FIXME: use locateDecl and friends once implemented.
@@ -200,16 +224,21 @@
 llvm::sort(T.Includes);
 T.Includes.erase(std::unique(T.Includes.begin(), T.Includes.end()),
  T.Includes.end());
+
+if (!T.Headers.empty())
+  // FIXME: library should tell us which header to use.
+  T.Insert = spellHeader(T.Headers.front());
 
 return T;
   }
 
 public:
-  Reporter(llvm::raw_ostream , ASTContext ,
+  Reporter(llvm::raw_ostream , ASTContext , 

[PATCH] D138676: [include-cleaner] HTMLReport shows headers that would be inserted

2022-11-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added a subscriber: mgrang.
Herald added a project: All.
sammccall requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Demo: 
https://htmlpreview.github.io/?https://gist.githubusercontent.com/sam-mccall/9730f933a2cf2e003365520b6636f731/raw/7911d8251ceab7c244e0510285105027cd0a9403/PathMapping.cpp.html

Header insertion doesn't actually work that well (not this patch's fault):

- we don't have ranking of locations/headers yet, so inserted header is pretty 
random
- on my system, we get a lot of absolute "/usr/bin/../include/..." paths. This 
is a HeaderSearch bug introduced in D60873  
that I'll send a fix for


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138676

Files:
  clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
  clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
  clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp

Index: clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
===
--- clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
+++ clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
@@ -71,8 +71,10 @@
<< ": " << EC.message() << "\n";
   exit(1);
 }
-writeHTMLReport(AST.Ctx->getSourceManager().getMainFileID(), PP.Includes,
-AST.Roots, PP.MacroReferences, *AST.Ctx, , OS);
+writeHTMLReport(
+AST.Ctx->getSourceManager().getMainFileID(), PP.Includes, AST.Roots,
+PP.MacroReferences, *AST.Ctx,
+getCompilerInstance().getPreprocessor().getHeaderSearchInfo(), , OS);
   }
 };
 
Index: clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
===
--- clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
+++ clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
@@ -18,6 +18,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
 #include "llvm/Support/ScopedPrinter.h"
@@ -29,7 +30,7 @@
 constexpr llvm::StringLiteral CSS = R"css(
   body { margin: 0; }
   pre { line-height: 1.5em; counter-reset: line; margin: 0; }
-  pre .line { counter-increment: line; }
+  pre .line:not(.added) { counter-increment: line; }
   pre .line::before {
 content: counter(line);
 display: inline-block;
@@ -37,6 +38,7 @@
 text-align: right;
 width: 3em; padding-right: 0.5em; margin-right: 0.5em;
   }
+  pre .line.added::before { content: '+' }
   .ref, .inc { text-decoration: underline; color: #008; }
   .sel { position: relative; cursor: pointer; }
   .ref.implicit { background-color: #ff8; }
@@ -52,6 +54,7 @@
   #hover .target.implicit, .provides .implicit { background-color: #bbb; }
   #hover .target.ambiguous, .provides .ambiguous { background-color: #caf; }
   .missing, .unused { background-color: #faa !important; }
+  .inserted { background-color: #bea !important; }
   .semiused { background-color: #888 !important; }
   #hover th { color: #008; text-align: right; padding-right: 0.5em; }
   #hover .target:not(:first-child) {
@@ -59,6 +62,8 @@
 padding-top: 1em;
 border-top: 1px solid #444;
   }
+  .missing #hover .insert { font-weight: bold; }
+  .satisfied #hover .insert { font-style: italic; }
 )css";
 
 constexpr llvm::StringLiteral JS = R"js(
@@ -128,6 +133,7 @@
   llvm::raw_ostream 
   const ASTContext 
   const SourceManager 
+  HeaderSearch 
   const RecordedPP::RecordedIncludes 
   const PragmaIncludes *PI;
   FileID MainFile;
@@ -142,6 +148,7 @@
 SmallVector Headers;
 SmallVector Includes;
 bool Satisfied = false; // Is the include present?
+std::string Insert; // If we had no includes, what would we insert?
   };
   std::vector Targets;
   // Points within the main file that reference a Target.
@@ -157,6 +164,7 @@
   };
   std::vector Refs;
   llvm::DenseMap> IncludeRefs;
+  llvm::StringMap> Insertion;
 
   llvm::StringRef includeType(const Include *I) {
 auto  = IncludeRefs[I];
@@ -169,8 +177,24 @@
 return "semiused";
   }
 
+  std::string spellHeader(const Header ) {
+switch (H.kind()) {
+case Header::Physical: {
+  bool IsSystem = false;
+  std::string Path = HS.suggestPathToFileForDiagnostics(
+  H.physical(), MainFE->tryGetRealPathName(), );
+  return IsSystem ? "<" + Path + ">" : "\"" + Path + "\"";
+}
+case Header::Standard:
+  return H.standard().name().str();
+case Header::Verbatim:
+  return H.verbatim().str();
+}
+llvm_unreachable("Unknown Header kind");
+  }
+
   Target makeTarget(const SymbolReference ) {
-Target T{SR.Target, SR.RT, {}, {}, {}};
+Target T{SR.Target, SR.RT, {}, {}, {}, 

[PATCH] D60873: [clang][HeaderSuggestion] Handle the case of dotdot with an absolute path

2022-11-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.
Herald added projects: clang-tools-extra, All.

I think this introduced a bug: if your search path has 
`/usr/bin/../include/c++/v1`, then you'll end up with `FileEntry`s with `Name` 
like `/usr/bin/../include/c++/v1/__utility/move.h`. Calling the `FileEntry` 
overload of `suggestPathToFileForDiagnostics` won't return the relative path, 
because it will strip dots from the dir path but not the fileentry's Name.

Fix is probably to strip dots from both, I guess.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60873/new/

https://reviews.llvm.org/D60873

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D137642: [X86][CodeGen] Fix crash in hotpatch

2022-11-24 Thread Sylvain Audi via Phabricator via cfe-commits
saudi added a comment.

Ping!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137642/new/

https://reviews.llvm.org/D137642

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138583: Create unused non-trivially-destructible check in clang-tidy

2022-11-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

In D138583#3949604 , @ankineri wrote:

> I do agree that having it as part of `-Wunused-variable` is better on many 
> aspects, but I have a few concerns about it.
> The proposed check can be applied to any type including STL ones such as 
> `std::string`, all as per codebase owner's choice. Adding this attribute to 
> STL classes will probably be really hard as it will break many builds.
> Having `warn_unused absl::Status` (and a couple of others) is probably a good 
> idea anyway, but is it feasible to apply it to STL?

My guess is that libc++ and libstdc++ would accept such a patch *where it's 
safe*, since clang and gcc respectively support the attribute, but you'd have 
to check with them.

---

Since the standard library is mostly templates, one issue you're going to run 
into with say `std::vector` is that it's only `warn_unused` because `int` 
is `warn_unused`, and this is hard to express with attributes.
`std::string` is actually the same since it's an alias for 
`std::basic_string`. While maybe nobody uses 
`std::basic_string`, it's a bit of a grey area.

Fundamentally this is all stuff that such a clang-tidy check would have to 
address as well, and IMO the tools you have to address it with clang + 
attributes are much better than in a clang-tidy check.
(If it solves std::string and std::vector, then that might even be enough to 
provide some hacky `warn_unused_recursive` attribute that is equivalent to 
`warn_unused` if all type template parameters are warn-unused).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138583/new/

https://reviews.llvm.org/D138583

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138583: Create unused non-trivially-destructible check in clang-tidy

2022-11-24 Thread Andrei via Phabricator via cfe-commits
ankineri added a comment.

I do agree that having it as part of `-Wunused-variable` is better on many 
aspects, but I have a few concerns about it.
The proposed check can be applied to any type including STL ones such as 
`std::string`, all as per codebase owner's choice. Adding this attribute to STL 
classes will probably be really hard as it will break many builds.
Having `warn_unused absl::Status` (and a couple of others) is probably a good 
idea anyway, but is it feasible to apply it to STL?

In other words, this check is:

- Configurable per build (which types to check for)
- Applicable to types of libraries not owned by the developer
- Likely breaking far fewer build targets
- Is decoupled from trivially-destructible objects (i.e. with 
`-Wunused-variable` types like `int` fall into the same category as 
`[[warn_unused]]`-marked ones).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138583/new/

https://reviews.llvm.org/D138583

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D137258: [clang] Optimize storage and lookup of analyzer options

2022-11-24 Thread Balázs Benics via Phabricator via cfe-commits
steakhal accepted this revision.
steakhal added a comment.
This revision is now accepted and ready to land.

BTW the change itself looks good to me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137258/new/

https://reviews.llvm.org/D137258

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138552: [docs] Document that the modules can improve the linking time

2022-11-24 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment.

>> I'd be curious to learn how linking time can be reduced significantly by 
>> deploying modules.

Oh, today I found the conclusion "significant decrease in link time time" was 
not responsible. Since I didn't recognize that our projects uses thinlto and 
thinlto cache. So there are too many factors and I can't say it reduces the 
linktime directly. And I'll try to do more precise analysis today.

And it would be great if you are interested. If you want to play with it. I 
think you can get started to apply the patch https://reviews.llvm.org/D138666 
(this patch is for fun only.) to clang. Then you can find a sample project. The 
project would be best to have long link time and large and repeatedly included 
headers. Our project is close-sourced so I can't share it.

Then you can put the headers into a module unit:

  // m.cppm
  module;
  #include "headers"
  // ...
  export module m;

Then for each .cc file, you can replace the headers with the modules by:

  import m;

Note that if there are uses of macro in the source files, you need to extract 
them into a standalone header which contains macro definitions only. And let 
the source files include the standalone headers. Because the named modules 
can't export macros. (Or we can hack it in the compiler for simplicity). 
Finally remember to compile `m.cppm` to `m.o` and link it. (If there are bugs 
you can try to file an issue)

Then I think you are able to analysis the changes.

(Never mind if you don't want to do so many things)

In D138552#3947272 , @dblaikie wrote:

> I'm not sure I follow this - the comparison doesn't seem equal. If you wanted 
> the modules-like code in non-modules, you could get it by having 
> `notDirectlyUsed` declared-but-not-defined in the header file, and defined in 
> the corresponding .cpp file, yeah?

Yeah.

> This seems comparing different things... As mentioned, header files can be 
> refactored to reduce the number of symbols as well.

It is true too.

It looks like my writing is misleading. Sorry for confusing. My point here is: 
with modules, the not directly used symbols wouldn't show up in the importee.  
And **usually**there would be many not directly used symbols in the header 
files. We need a clean re-design and some effort to move these not directly 
used items into .cpp files. It is not trivial in real world projects.

And another problem here is, without LTO, the function definitions in other TU 
can't be inlined. But now, the function definitions in the module interface can 
be imported to the importee as AvaialableExternally linkage with optimization. 
The AvaialableExternally linkage is a special linkage which aims for the IPO. 
And the AvaialableExternally entities would be removed in the middle end after 
inlining. (I know there are arguments to remove the function definition in the 
module file.) So if we move not directly used things in .cpp files, the 
performance will be hurt. But it is not the case for modules (at least for now).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138552/new/

https://reviews.llvm.org/D138552

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138668: Correct typos

2022-11-24 Thread Balázs Benics via Phabricator via cfe-commits
steakhal accepted this revision.
steakhal added a comment.

Looks good.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138668/new/

https://reviews.llvm.org/D138668

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D137725: [OpenMP][OMPIRBuilder] Mirgrate getName from clang to OMPIRBuilder

2022-11-24 Thread Jan Sjödin via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2aa338f68e1e: [OpenMP][OMPIRBuilder] Mirgrate getName from 
clang to OMPIRBuilder (authored by jsjodin).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137725/new/

https://reviews.llvm.org/D137725

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -3963,6 +3963,12 @@
   return OS.str().str();
 }
 
+std::string
+OpenMPIRBuilder::createPlatformSpecificName(ArrayRef Parts) const {
+  return OpenMPIRBuilder::getNameWithSeparators(Parts, Config.firstSeparator(),
+Config.separator());
+}
+
 GlobalVariable *
 OpenMPIRBuilder::getOrCreateInternalVariable(Type *Ty, const StringRef ,
  unsigned AddressSpace) {
Index: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -92,34 +92,61 @@
   /// directive is present or not.
   Optional HasRequiresUnifiedSharedMemory;
 
+  /// First separator used between the initial two parts of a name.
+  Optional FirstSeparator;
+  /// Separator used between all of the rest consecutive parts of s name
+  Optional Separator;
+
   OpenMPIRBuilderConfig() {}
   OpenMPIRBuilderConfig(bool IsEmbedded, bool IsTargetCodegen,
 bool HasRequiresUnifiedSharedMemory)
   : IsEmbedded(IsEmbedded), IsTargetCodegen(IsTargetCodegen),
 HasRequiresUnifiedSharedMemory(HasRequiresUnifiedSharedMemory) {}
 
-  // Convenience getter functions that assert if the value is not present.
-  bool isEmbedded() {
+  // Getters functions that assert if the required values are not present.
+  bool isEmbedded() const {
 assert(IsEmbedded.has_value() && "IsEmbedded is not set");
 return IsEmbedded.value();
   }
 
-  bool isTargetCodegen() {
+  bool isTargetCodegen() const {
 assert(IsTargetCodegen.has_value() && "IsTargetCodegen is not set");
 return IsTargetCodegen.value();
   }
 
-  bool hasRequiresUnifiedSharedMemory() {
+  bool hasRequiresUnifiedSharedMemory() const {
 assert(HasRequiresUnifiedSharedMemory.has_value() &&
"HasUnifiedSharedMemory is not set");
 return HasRequiresUnifiedSharedMemory.value();
   }
 
+  // Returns the FirstSeparator if set, otherwise use the default
+  // separator depending on isTargetCodegen
+  StringRef firstSeparator() const {
+if (FirstSeparator.has_value())
+  return FirstSeparator.value();
+if (isTargetCodegen())
+  return "_";
+return ".";
+  }
+
+  // Returns the Separator if set, otherwise use the default
+  // separator depending on isTargetCodegen
+  StringRef separator() const {
+if (Separator.has_value())
+  return Separator.value();
+if (isTargetCodegen())
+  return "$";
+return ".";
+  }
+
   void setIsEmbedded(bool Value) { IsEmbedded = Value; }
   void setIsTargetCodegen(bool Value) { IsTargetCodegen = Value; }
   void setHasRequiresUnifiedSharedMemory(bool Value) {
 HasRequiresUnifiedSharedMemory = Value;
   }
+  void setFirstSeparator(StringRef FS) { FirstSeparator = FS; }
+  void setSeparator(StringRef S) { Separator = S; }
 };
 
 /// An interface to create LLVM-IR for OpenMP directives.
@@ -150,6 +177,16 @@
   /// Type used throughout for insertion points.
   using InsertPointTy = IRBuilder<>::InsertPoint;
 
+  /// Get the create a name using the platform specific separators.
+  /// \param Parts parts of the final name that needs separation
+  /// The created name has a first separator between the first and second part
+  /// and a second separator between all other parts.
+  /// E.g. with FirstSeparator "$" and Separator "." and
+  /// parts: "p1", "p2", "p3", "p4"
+  /// The resulting name is "p1$p2.p3.p4"
+  /// The separators are retrieved from the OpenMPIRBuilderConfig.
+  std::string createPlatformSpecificName(ArrayRef Parts) const;
+
   /// Callback type for variable finalization (think destructors).
   ///
   /// \param CodeGenIP is the insertion point at which the finalization code
Index: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -889,7 +889,7 @@
 }
 
 CGOpenMPRuntimeGPU::CGOpenMPRuntimeGPU(CodeGenModule )
-: CGOpenMPRuntime(CGM, "_", 

[clang] 2aa338f - [OpenMP][OMPIRBuilder] Mirgrate getName from clang to OMPIRBuilder

2022-11-24 Thread Jan Sjodin via cfe-commits

Author: Jan Sjodin
Date: 2022-11-24T10:11:13-05:00
New Revision: 2aa338f68e1ee414680f0f08939dd8015b97d014

URL: 
https://github.com/llvm/llvm-project/commit/2aa338f68e1ee414680f0f08939dd8015b97d014
DIFF: 
https://github.com/llvm/llvm-project/commit/2aa338f68e1ee414680f0f08939dd8015b97d014.diff

LOG: [OpenMP][OMPIRBuilder] Mirgrate getName from clang to OMPIRBuilder

This change moves the getName function from clang and moves the separator class
members from CGOpenMPRuntime into OMPIRBuilder. Also enusre all the getters
in the config class are const.

Reviewed By: jdoerfert

Differential Revision: https://reviews.llvm.org/D137725

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/CodeGen/CGOpenMPRuntime.h
clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 0552fb31bff97..dfbb1df5046e4 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -1057,14 +1057,11 @@ static FieldDecl *addFieldToRecordDecl(ASTContext , 
DeclContext *DC,
   return Field;
 }
 
-CGOpenMPRuntime::CGOpenMPRuntime(CodeGenModule , StringRef FirstSeparator,
- StringRef Separator)
-: CGM(CGM), FirstSeparator(FirstSeparator), Separator(Separator),
-  OMPBuilder(CGM.getModule()), OffloadEntriesInfoManager() {
+CGOpenMPRuntime::CGOpenMPRuntime(CodeGenModule )
+: CGM(CGM), OMPBuilder(CGM.getModule()), OffloadEntriesInfoManager() {
   KmpCriticalNameTy = llvm::ArrayType::get(CGM.Int32Ty, /*NumElements*/ 8);
   llvm::OpenMPIRBuilderConfig Config(CGM.getLangOpts().OpenMPIsDevice, false,
  hasRequiresUnifiedSharedMemory());
-
   // Initialize Types used in OpenMPIRBuilder from OMPKinds.def
   OMPBuilder.initialize();
   OMPBuilder.setConfig(Config);
@@ -1088,14 +1085,7 @@ void CGOpenMPRuntime::clear() {
 }
 
 std::string CGOpenMPRuntime::getName(ArrayRef Parts) const {
-  SmallString<128> Buffer;
-  llvm::raw_svector_ostream OS(Buffer);
-  StringRef Sep = FirstSeparator;
-  for (StringRef Part : Parts) {
-OS << Sep << Part;
-Sep = Separator;
-  }
-  return std::string(OS.str());
+  return OMPBuilder.createPlatformSpecificName(Parts);
 }
 
 static llvm::Function *

diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.h 
b/clang/lib/CodeGen/CGOpenMPRuntime.h
index b70708a46eeb3..c441afa6be9bc 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.h
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.h
@@ -306,15 +306,10 @@ class CGOpenMPRuntime {
 
 protected:
   CodeGenModule 
-  StringRef FirstSeparator, Separator;
 
   /// An OpenMP-IR-Builder instance.
   llvm::OpenMPIRBuilder OMPBuilder;
 
-  /// Constructor allowing to redefine the name separator for the variables.
-  explicit CGOpenMPRuntime(CodeGenModule , StringRef FirstSeparator,
-   StringRef Separator);
-
   /// Helper to emit outlined function for 'target' directive.
   /// \param D Directive to emit.
   /// \param ParentName Name of the function that encloses the target region.
@@ -691,8 +686,7 @@ class CGOpenMPRuntime {
   Address DependenciesArray);
 
 public:
-  explicit CGOpenMPRuntime(CodeGenModule )
-  : CGOpenMPRuntime(CGM, ".", ".") {}
+  explicit CGOpenMPRuntime(CodeGenModule );
   virtual ~CGOpenMPRuntime() {}
   virtual void clear();
 

diff  --git a/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
index 4bfc462efaeaa..85837317a3dd1 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -889,7 +889,7 @@ unsigned 
CGOpenMPRuntimeGPU::getDefaultLocationReserved2Flags() const {
 }
 
 CGOpenMPRuntimeGPU::CGOpenMPRuntimeGPU(CodeGenModule )
-: CGOpenMPRuntime(CGM, "_", "$") {
+: CGOpenMPRuntime(CGM) {
   llvm::OpenMPIRBuilderConfig Config(CGM.getLangOpts().OpenMPIsDevice, true,
  hasRequiresUnifiedSharedMemory());
   OMPBuilder.setConfig(Config);

diff  --git a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h 
b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
index fe9c95c4f6c55..37069dc86ffea 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -92,34 +92,61 @@ class OpenMPIRBuilderConfig {
   /// directive is present or not.
   Optional HasRequiresUnifiedSharedMemory;
 
+  /// First separator used between the initial two parts of a name.
+  Optional FirstSeparator;
+  /// Separator used between all of the rest consecutive parts of s name
+  Optional Separator;
+
   OpenMPIRBuilderConfig() {}
   OpenMPIRBuilderConfig(bool IsEmbedded, bool IsTargetCodegen,
 bool HasRequiresUnifiedSharedMemory)
 

[PATCH] D138655: [clang-tidy] Fix `cppcoreguidelines-init-variables` for invalid vardecl

2022-11-24 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment.

Will be good idea to add test for this situation.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138655/new/

https://reviews.llvm.org/D138655

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D90568: [clang] Add [is|set]Nested methods to NamespaceDecl

2022-11-24 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

My bots look happy at least. Thanks for the quick fix!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90568/new/

https://reviews.llvm.org/D90568

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138668: Correct typos

2022-11-24 Thread Sprite via Phabricator via cfe-commits
Sprite created this revision.
Sprite added a project: LLVM.
Herald added subscribers: Moerafaat, zero9178, steakhal, bzcheeseman, kosarev, 
sdasgup3, carlosgalvezp, wenzhicui, wrengr, cota, teijeong, rdzhabarov, 
tatianashp, msifontes, jurahul, Kayjukh, grosul1, martong, Joonsoo, kerbowa, 
liufengdb, aartbik, mgester, arpith-jacob, csigg, antiagainst, shauheen, 
rriddle, mehdi_amini, pengfei, hiraditya, jvesely.
Herald added a reviewer: jpienaar.
Herald added a reviewer: NoQ.
Herald added a reviewer: njames93.
Herald added a project: All.
Sprite requested review of this revision.
Herald added a reviewer: nicolasvasilache.
Herald added subscribers: cfe-commits, llvm-commits, stephenneuendorffer, 
nicolasvasilache.
Herald added projects: clang, MLIR, clang-tools-extra.

Just found some typos while reading the llvm/circt project.

- compliment -> complement
- emitsd -> emits


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138668

Files:
  clang-tools-extra/clang-tidy/abseil/DurationRewriter.cpp
  clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
  clang/test/CodeGen/X86/x86-GCC-inline-asm-Y-constraints.c
  llvm/docs/AMDGPUDwarfExtensionsForHeterogeneousDebugging.rst
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/CodeGen/SplitKit.h
  llvm/test/MC/X86/x86-GCC-inline-asm-Y-constraints.ll
  mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td

Index: mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td
===
--- mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td
+++ mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td
@@ -690,7 +690,7 @@
 %shape' = shape.const_shape [1, 2] : !shape.shape
 ```
 
-This operation is the compliment of `shape_of` wrt ValueShape values.
+This operation is the complement of `shape_of` wrt ValueShape values.
   }];
 
   let arguments = (ins AnyTypeOf<[1DTensorOf<[AnyInteger, Index]>, Shape_ValueShapeType]>:$arg);
Index: llvm/test/MC/X86/x86-GCC-inline-asm-Y-constraints.ll
===
--- llvm/test/MC/X86/x86-GCC-inline-asm-Y-constraints.ll
+++ llvm/test/MC/X86/x86-GCC-inline-asm-Y-constraints.ll
@@ -1,5 +1,5 @@
 ; RUN: llc -mtriple=x86_64-apple-darwin -mcpu skx < %s | FileCheck %s
-; This test compliments the .c test under clang/test/CodeGen/. We check 
+; This test complements the .c test under clang/test/CodeGen/. We check 
 ; if the inline asm constraints are respected in the generated code.
 
 ; Function Attrs: nounwind
Index: llvm/lib/CodeGen/SplitKit.h
===
--- llvm/lib/CodeGen/SplitKit.h
+++ llvm/lib/CodeGen/SplitKit.h
@@ -487,7 +487,7 @@
 
   /// overlapIntv - Indicate that all instructions in range should use the open
   /// interval if End does not have tied-def usage of the register and in this
-  /// case compliment interval is used. Let the complement interval be live.
+  /// case complement interval is used. Let the complement interval be live.
   ///
   /// This doubles the register pressure, but is sometimes required to deal with
   /// register uses after the last valid split point.
Index: llvm/lib/CodeGen/CodeGenPrepare.cpp
===
--- llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -4023,11 +4023,11 @@
 auto IVInc = getIVIncrement(PN, );
 if (!IVInc)
   return None;
-// TODO: The result of the intrinsics above is two-compliment. However when
+// TODO: The result of the intrinsics above is two-complement. However when
 // IV inc is expressed as add or sub, iv.next is potentially a poison value.
 // If it has nuw or nsw flags, we need to make sure that these flags are
 // inferrable at the point of memory instruction. Otherwise we are replacing
-// well-defined two-compliment computation with poison. Currently, to avoid
+// well-defined two-complement computation with poison. Currently, to avoid
 // potentially complex analysis needed to prove this, we reject such cases.
 if (auto *OIVInc = dyn_cast(IVInc->first))
   if (OIVInc->hasNoSignedWrap() || OIVInc->hasNoUnsignedWrap())
Index: llvm/docs/AMDGPUDwarfExtensionsForHeterogeneousDebugging.rst
===
--- llvm/docs/AMDGPUDwarfExtensionsForHeterogeneousDebugging.rst
+++ llvm/docs/AMDGPUDwarfExtensionsForHeterogeneousDebugging.rst
@@ -222,7 +222,7 @@
 infinite precision offsets to allow it to correctly track a series of positive
 and negative offsets that may transiently overflow or underflow, but end up in
 range. This is simple for the arithmetic operations as they are defined in terms
-of two's compliment arithmetic on a base type of a fixed size. Therefore, the
+of two's complement arithmetic on a base type of a fixed size. Therefore, the
 offset operation define that integer overflow is ill-formed. This is in contrast
 to 

[PATCH] D112621: [analyzer][solver] Introduce reasoning for not equal to operator

2022-11-24 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment.

In D112621#3915822 , @manas wrote:

> Ping

`Analysis/constant-folding.c` seems to fail.
Please run the `check-clang-analysis` build target to see what fails and 
investigate it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112621/new/

https://reviews.llvm.org/D112621

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138649: [include-cleaner] Show details for #include directives (used/unused)

2022-11-24 Thread Sam McCall via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3e658abd4147: [include-cleaner] Show details for #include 
directives (used/unused) (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138649/new/

https://reviews.llvm.org/D138649

Files:
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
  clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
  clang-tools-extra/include-cleaner/lib/Record.cpp

Index: clang-tools-extra/include-cleaner/lib/Record.cpp
===
--- clang-tools-extra/include-cleaner/lib/Record.cpp
+++ clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -377,6 +377,13 @@
   BySpellingIt->second.push_back(Index);
   if (I.Resolved)
 ByFile[I.Resolved].push_back(Index);
+  ByLine[I.Line] = Index;
+}
+
+const Include *
+RecordedPP::RecordedIncludes::atLine(unsigned OneBasedIndex) const {
+  auto It = ByLine.find(OneBasedIndex);
+  return (It == ByLine.end()) ? nullptr : [It->second];
 }
 
 llvm::SmallVector
Index: clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
===
--- clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
+++ clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
@@ -37,7 +37,7 @@
 text-align: right;
 width: 3em; padding-right: 0.5em; margin-right: 0.5em;
   }
-  .ref { text-decoration: underline; color: #008; }
+  .ref, .inc { text-decoration: underline; color: #008; }
   .sel { position: relative; cursor: pointer; }
   .ref.implicit { background-color: #ff8; }
   #hover {
@@ -49,9 +49,10 @@
 padding: 0.5em;
   }
   #hover p, #hover pre { margin: 0; }
-  #hover .target.implicit { background-color: #bbb; }
-  #hover .target.ambiguous { background-color: #caf; }
+  #hover .target.implicit, .provides .implicit { background-color: #bbb; }
+  #hover .target.ambiguous, .provides .ambiguous { background-color: #caf; }
   .missing, .unused { background-color: #faa !important; }
+  .semiused { background-color: #888 !important; }
   #hover th { color: #008; text-align: right; padding-right: 0.5em; }
   #hover .target:not(:first-child) {
 margin-top: 1em;
@@ -95,6 +96,22 @@
   llvm_unreachable("unhandled symbol kind");
 }
 
+// Return detailed symbol description (declaration), if we have any.
+std::string printDetails(const Symbol ) {
+  std::string S;
+  if (Sym.kind() == Symbol::Declaration) {
+// Print the declaration of the symbol, e.g. to disambiguate overloads.
+const auto  = Sym.declaration();
+PrintingPolicy PP = D.getASTContext().getPrintingPolicy();
+PP.FullyQualifiedName = true;
+PP.TerseOutput = true;
+PP.SuppressInitializers = true;
+llvm::raw_string_ostream SS(S);
+D.print(SS, PP);
+  }
+  return S;
+}
+
 llvm::StringRef refType(RefType T) {
   switch (T) {
   case RefType::Explicit:
@@ -139,6 +156,18 @@
 }
   };
   std::vector Refs;
+  llvm::DenseMap> IncludeRefs;
+
+  llvm::StringRef includeType(const Include *I) {
+auto  = IncludeRefs[I];
+if (List.empty())
+  return "unused";
+if (llvm::any_of(List, [&](unsigned I) {
+  return Targets[Refs[I].TargetIndex].Type == RefType::Explicit;
+}))
+  return "used";
+return "semiused";
+  }
 
   Target makeTarget(const SymbolReference ) {
 Target T{SR.Target, SR.RT, {}, {}, {}};
@@ -194,6 +223,8 @@
 
 Refs.push_back({Offset, SR.RT == RefType::Implicit, Targets.size()});
 Targets.push_back(makeTarget(SR));
+for (const auto *I : Targets.back().Includes)
+  IncludeRefs[I].push_back(Targets.size() - 1);
   }
 
   void write() {
@@ -202,6 +233,11 @@
 OS << "\n";
 OS << "" << CSS << "\n";
 OS << "" << JS << "\n";
+for (auto  : Includes.all()) {
+  OS << "";
+  writeInclude(Inc);
+  OS << "\n";
+}
 for (unsigned I = 0; I < Targets.size(); ++I) {
   OS << "";
   writeTarget(Targets[I]);
@@ -260,6 +296,45 @@
 OS << ">";
   }
 
+  void writeInclude(const Include ) {
+OS << "";
+if (Inc.Resolved) {
+  OS << "Resolved";
+  escapeString(Inc.Resolved->getName());
+  OS << "\n";
+}
+// We show one ref for each symbol: first by (RefType != Explicit, Sequence)
+llvm::DenseMap FirstRef;
+for (unsigned RefIndex : IncludeRefs[]) {
+  const Target  = Targets[Refs[RefIndex].TargetIndex];
+  auto I = FirstRef.try_emplace(T.Sym, RefIndex);
+  if (!I.second && T.Type == RefType::Explicit &&
+  Targets[Refs[I.first->second].TargetIndex].Type != RefType::Explicit)
+I.first->second = RefIndex;
+}
+std::vector> Sorted = {FirstRef.begin(),
+   FirstRef.end()};
+llvm::stable_sort(Sorted, llvm::less_second{});
+for (auto &[S, RefIndex] : Sorted) {
+  auto  = 

[clang-tools-extra] 3e658ab - [include-cleaner] Show details for #include directives (used/unused)

2022-11-24 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2022-11-24T15:36:44+01:00
New Revision: 3e658abd41475640f7460e132513d20e8e332ae7

URL: 
https://github.com/llvm/llvm-project/commit/3e658abd41475640f7460e132513d20e8e332ae7
DIFF: 
https://github.com/llvm/llvm-project/commit/3e658abd41475640f7460e132513d20e8e332ae7.diff

LOG: [include-cleaner] Show details for #include directives (used/unused)

Differential Revision: https://reviews.llvm.org/D138649

Added: 


Modified: 
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
clang-tools-extra/include-cleaner/lib/Record.cpp

Removed: 




diff  --git 
a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h 
b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
index d6a6a6aa417c..a569f219939d 100644
--- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
+++ b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
@@ -147,11 +147,15 @@ struct RecordedPP {
 ///  - for a logical file like , we check Spelled
 llvm::SmallVector match(Header H) const;
 
+/// Finds the include written on the specified line.
+const Include *atLine(unsigned OneBasedIndex) const;
+
   private:
 std::vector All;
 // Lookup structures for match(), values are index into All.
 llvm::StringMap> BySpelling;
 llvm::DenseMap> ByFile;
+llvm::DenseMap ByLine;
   } Includes;
 };
 

diff  --git a/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp 
b/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
index 477fea6e3fc0..e28a951592c4 100644
--- a/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
+++ b/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
@@ -37,7 +37,7 @@ constexpr llvm::StringLiteral CSS = R"css(
 text-align: right;
 width: 3em; padding-right: 0.5em; margin-right: 0.5em;
   }
-  .ref { text-decoration: underline; color: #008; }
+  .ref, .inc { text-decoration: underline; color: #008; }
   .sel { position: relative; cursor: pointer; }
   .ref.implicit { background-color: #ff8; }
   #hover {
@@ -49,9 +49,10 @@ constexpr llvm::StringLiteral CSS = R"css(
 padding: 0.5em;
   }
   #hover p, #hover pre { margin: 0; }
-  #hover .target.implicit { background-color: #bbb; }
-  #hover .target.ambiguous { background-color: #caf; }
+  #hover .target.implicit, .provides .implicit { background-color: #bbb; }
+  #hover .target.ambiguous, .provides .ambiguous { background-color: #caf; }
   .missing, .unused { background-color: #faa !important; }
+  .semiused { background-color: #888 !important; }
   #hover th { color: #008; text-align: right; padding-right: 0.5em; }
   #hover .target:not(:first-child) {
 margin-top: 1em;
@@ -95,6 +96,22 @@ llvm::StringRef describeSymbol(const Symbol ) {
   llvm_unreachable("unhandled symbol kind");
 }
 
+// Return detailed symbol description (declaration), if we have any.
+std::string printDetails(const Symbol ) {
+  std::string S;
+  if (Sym.kind() == Symbol::Declaration) {
+// Print the declaration of the symbol, e.g. to disambiguate overloads.
+const auto  = Sym.declaration();
+PrintingPolicy PP = D.getASTContext().getPrintingPolicy();
+PP.FullyQualifiedName = true;
+PP.TerseOutput = true;
+PP.SuppressInitializers = true;
+llvm::raw_string_ostream SS(S);
+D.print(SS, PP);
+  }
+  return S;
+}
+
 llvm::StringRef refType(RefType T) {
   switch (T) {
   case RefType::Explicit:
@@ -139,6 +156,18 @@ class Reporter {
 }
   };
   std::vector Refs;
+  llvm::DenseMap> IncludeRefs;
+
+  llvm::StringRef includeType(const Include *I) {
+auto  = IncludeRefs[I];
+if (List.empty())
+  return "unused";
+if (llvm::any_of(List, [&](unsigned I) {
+  return Targets[Refs[I].TargetIndex].Type == RefType::Explicit;
+}))
+  return "used";
+return "semiused";
+  }
 
   Target makeTarget(const SymbolReference ) {
 Target T{SR.Target, SR.RT, {}, {}, {}};
@@ -194,6 +223,8 @@ class Reporter {
 
 Refs.push_back({Offset, SR.RT == RefType::Implicit, Targets.size()});
 Targets.push_back(makeTarget(SR));
+for (const auto *I : Targets.back().Includes)
+  IncludeRefs[I].push_back(Targets.size() - 1);
   }
 
   void write() {
@@ -202,6 +233,11 @@ class Reporter {
 OS << "\n";
 OS << "" << CSS << "\n";
 OS << "" << JS << "\n";
+for (auto  : Includes.all()) {
+  OS << "";
+  writeInclude(Inc);
+  OS << "\n";
+}
 for (unsigned I = 0; I < Targets.size(); ++I) {
   OS << "";
   writeTarget(Targets[I]);
@@ -260,6 +296,45 @@ class Reporter {
 OS << ">";
   }
 
+  void writeInclude(const Include ) {
+OS << "";
+if (Inc.Resolved) {
+  OS << "Resolved";
+  escapeString(Inc.Resolved->getName());
+  OS << "\n";
+}
+// We show one ref for each symbol: first by 

[PATCH] D138649: [include-cleaner] Show details for #include directives (used/unused)

2022-11-24 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

The demo is really nice!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138649/new/

https://reviews.llvm.org/D138649

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138489: [tsan] Add tsan support for loongarch64

2022-11-24 Thread Lu Weining via Phabricator via cfe-commits
SixWeining added inline comments.



Comment at: compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp:1543
+"r"(__fn), "r"(__arg), "r"(nr_clone), "i"(__NR_exit)
+  : "memory");
+  return res;

tangyouling wrote:
> SixWeining wrote:
> > Shall we list $t0-$t8 here? Ref D137396.
> > Shall we list $t0-$t8 here? Ref D137396.
> 
> I will add the missing $t0-$t8.
As this inline asm is almost at the end of the function, it's sure `$t*` would 
not be used any more. So I think we could not add them to the clobber list. 
Just keep current approach.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138489/new/

https://reviews.llvm.org/D138489

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D137652: Remove mandatory define of optional features macros for OpenCL C 3.0

2022-11-24 Thread Sven van Haastregt via Phabricator via cfe-commits
svenvh added inline comments.



Comment at: clang/include/clang/Basic/OpenCLExtensions.def:123
+OPENCL_OPTIONALCOREFEATURE(__opencl_c_work_group_collective_functions, false, 
300, OCL_C_30)
+OPENCL_OPTIONALCOREFEATURE(__opencl_c_int64, false, 300, OCL_C_30)
 

I am wondering why those features weren't added together with the other OpenCL 
3.0 features; there wasn't any discussion around that in D95776.  Perhaps it's 
because these don't affect the compiler behaviour directly? (but then neither 
does e.g. `__opencl_c_atomic_order_acq_rel`)  Wondering if @Anastasia has any 
insights here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137652/new/

https://reviews.llvm.org/D137652

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] 0cb2dd5 - [include-cleaner] Make Symbol (and Macro) hashable.

2022-11-24 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2022-11-24T15:15:50+01:00
New Revision: 0cb2dd5322f494769a7c31c8ed8aab930919f5f3

URL: 
https://github.com/llvm/llvm-project/commit/0cb2dd5322f494769a7c31c8ed8aab930919f5f3
DIFF: 
https://github.com/llvm/llvm-project/commit/0cb2dd5322f494769a7c31c8ed8aab930919f5f3.diff

LOG: [include-cleaner] Make Symbol (and Macro) hashable.

For now, we decided not to add operator< or handle other variants.
(If we do so in future we may want to extract a base class).

Differential Revision: https://reviews.llvm.org/D138648

Added: 


Modified: 
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h

Removed: 




diff  --git 
a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h 
b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
index 29a3aa7ea7f73..405ded21561c3 100644
--- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
+++ b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
@@ -64,9 +64,11 @@ struct Symbol {
   struct Macro macro() const { return std::get(Storage); }
 
 private:
-  // FIXME: Add support for macros.
   // Order must match Kind enum!
   std::variant Storage;
+
+  Symbol(decltype(Storage) Sentinel) : Storage(std::move(Sentinel)) {}
+  friend llvm::DenseMapInfo;
 };
 llvm::raw_ostream <<(llvm::raw_ostream &, const Symbol &);
 
@@ -137,4 +139,36 @@ llvm::raw_ostream <<(llvm::raw_ostream &, const 
Include &);
 } // namespace include_cleaner
 } // namespace clang
 
+namespace llvm {
+
+template <> struct DenseMapInfo {
+  using Outer = clang::include_cleaner::Symbol;
+  using Base = DenseMapInfo;
+
+  static inline Outer getEmptyKey() { return {Base::getEmptyKey()}; }
+  static inline Outer getTombstoneKey() { return {Base::getTombstoneKey()}; }
+  static unsigned getHashValue(const Outer ) {
+return Base::getHashValue(Val.Storage);
+  }
+  static bool isEqual(const Outer , const Outer ) {
+return Base::isEqual(LHS.Storage, RHS.Storage);
+  }
+};
+template <> struct DenseMapInfo {
+  using Outer = clang::include_cleaner::Macro;
+  using Base = DenseMapInfo;
+
+  static inline Outer getEmptyKey() { return {nullptr, Base::getEmptyKey()}; }
+  static inline Outer getTombstoneKey() {
+return {nullptr, Base::getTombstoneKey()};
+  }
+  static unsigned getHashValue(const Outer ) {
+return Base::getHashValue(Val.Definition);
+  }
+  static bool isEqual(const Outer , const Outer ) {
+return Base::isEqual(LHS.Definition, RHS.Definition);
+  }
+};
+} // namespace llvm
+
 #endif



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138648: [include-cleaner] Make Symbol (and Macro) hashable.

2022-11-24 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0cb2dd5322f4: [include-cleaner] Make Symbol (and Macro) 
hashable. (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D138648?vs=477723=477768#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138648/new/

https://reviews.llvm.org/D138648

Files:
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h


Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
===
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
@@ -64,9 +64,11 @@
   struct Macro macro() const { return std::get(Storage); }
 
 private:
-  // FIXME: Add support for macros.
   // Order must match Kind enum!
   std::variant Storage;
+
+  Symbol(decltype(Storage) Sentinel) : Storage(std::move(Sentinel)) {}
+  friend llvm::DenseMapInfo;
 };
 llvm::raw_ostream <<(llvm::raw_ostream &, const Symbol &);
 
@@ -137,4 +139,36 @@
 } // namespace include_cleaner
 } // namespace clang
 
+namespace llvm {
+
+template <> struct DenseMapInfo {
+  using Outer = clang::include_cleaner::Symbol;
+  using Base = DenseMapInfo;
+
+  static inline Outer getEmptyKey() { return {Base::getEmptyKey()}; }
+  static inline Outer getTombstoneKey() { return {Base::getTombstoneKey()}; }
+  static unsigned getHashValue(const Outer ) {
+return Base::getHashValue(Val.Storage);
+  }
+  static bool isEqual(const Outer , const Outer ) {
+return Base::isEqual(LHS.Storage, RHS.Storage);
+  }
+};
+template <> struct DenseMapInfo {
+  using Outer = clang::include_cleaner::Macro;
+  using Base = DenseMapInfo;
+
+  static inline Outer getEmptyKey() { return {nullptr, Base::getEmptyKey()}; }
+  static inline Outer getTombstoneKey() {
+return {nullptr, Base::getTombstoneKey()};
+  }
+  static unsigned getHashValue(const Outer ) {
+return Base::getHashValue(Val.Definition);
+  }
+  static bool isEqual(const Outer , const Outer ) {
+return Base::isEqual(LHS.Definition, RHS.Definition);
+  }
+};
+} // namespace llvm
+
 #endif


Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
===
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
@@ -64,9 +64,11 @@
   struct Macro macro() const { return std::get(Storage); }
 
 private:
-  // FIXME: Add support for macros.
   // Order must match Kind enum!
   std::variant Storage;
+
+  Symbol(decltype(Storage) Sentinel) : Storage(std::move(Sentinel)) {}
+  friend llvm::DenseMapInfo;
 };
 llvm::raw_ostream <<(llvm::raw_ostream &, const Symbol &);
 
@@ -137,4 +139,36 @@
 } // namespace include_cleaner
 } // namespace clang
 
+namespace llvm {
+
+template <> struct DenseMapInfo {
+  using Outer = clang::include_cleaner::Symbol;
+  using Base = DenseMapInfo;
+
+  static inline Outer getEmptyKey() { return {Base::getEmptyKey()}; }
+  static inline Outer getTombstoneKey() { return {Base::getTombstoneKey()}; }
+  static unsigned getHashValue(const Outer ) {
+return Base::getHashValue(Val.Storage);
+  }
+  static bool isEqual(const Outer , const Outer ) {
+return Base::isEqual(LHS.Storage, RHS.Storage);
+  }
+};
+template <> struct DenseMapInfo {
+  using Outer = clang::include_cleaner::Macro;
+  using Base = DenseMapInfo;
+
+  static inline Outer getEmptyKey() { return {nullptr, Base::getEmptyKey()}; }
+  static inline Outer getTombstoneKey() {
+return {nullptr, Base::getTombstoneKey()};
+  }
+  static unsigned getHashValue(const Outer ) {
+return Base::getHashValue(Val.Definition);
+  }
+  static bool isEqual(const Outer , const Outer ) {
+return Base::isEqual(LHS.Definition, RHS.Definition);
+  }
+};
+} // namespace llvm
+
 #endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] a72609c - [Format] Don't crash on mismatched brackets

2022-11-24 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2022-11-24T15:14:06+01:00
New Revision: a72609cabef4c5f5afa3811575a3963830cb13dd

URL: 
https://github.com/llvm/llvm-project/commit/a72609cabef4c5f5afa3811575a3963830cb13dd
DIFF: 
https://github.com/llvm/llvm-project/commit/a72609cabef4c5f5afa3811575a3963830cb13dd.diff

LOG: [Format] Don't crash on mismatched brackets

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTestCSharp.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 7f16d04d898b9..889dfa5fd7a63 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -2837,7 +2837,7 @@ static bool isFunctionDeclarationName(bool IsCpp, const 
FormatToken ,
 if (!Current.is(TT_StartOfName) || Current.NestingLevel != 0)
   return false;
 for (; Next; Next = Next->Next) {
-  if (Next->is(TT_TemplateOpener)) {
+  if (Next->is(TT_TemplateOpener) && Next->MatchingParen) {
 Next = Next->MatchingParen;
   } else if (Next->is(tok::coloncolon)) {
 Next = Next->Next;

diff  --git a/clang/unittests/Format/FormatTestCSharp.cpp 
b/clang/unittests/Format/FormatTestCSharp.cpp
index a18f57ec57144..1928a0d2da632 100644
--- a/clang/unittests/Format/FormatTestCSharp.cpp
+++ b/clang/unittests/Format/FormatTestCSharp.cpp
@@ -1600,5 +1600,9 @@ TEST_F(FormatTestCSharp, ShortFunctions) {
Style);
 }
 
+TEST_F(FormatTestCSharp, BrokenBrackets) {
+  EXPECT_NE("", format("int where b <")); // reduced from crasher
+}
+
 } // namespace format
 } // end namespace clang



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138583: Create unused non-trivially-destructible check in clang-tidy

2022-11-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Extending --warn-unused to other types seems preferable in a few ways:

- clang warnings are more accessible than clang-tidy checks
- avoids duplicating implementation
- places description of the types along with the types, instead of in 
clang-tidy config
- avoids divergent ux between sources of warnings

In fact it looks like an attribute for this already exists: it can be spelled 
`__attribute__((warn_unused))` or `[[gnu::warn_unused]]`

  struct __attribute__((warn_unused)) S { ~S(); };
  void foo() { S s; }



  $ clang -fsyntax-only ~/test.cc -Wall
  /usr/local/google/home/sammccall/test.cc:4:5: warning: unused variable 's' 
[-Wunused-variable]
S s;


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138583/new/

https://reviews.llvm.org/D138583

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D90568: [clang] Add [is|set]Nested methods to NamespaceDecl

2022-11-24 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

In D90568#3949134 , @thakis wrote:

> This breaks the build: http://45.33.8.238/linux/92294/step_4.txt
>
> Please take a look and revert for now if it takes a while to fix.

Apologies it took 2 times, It should all be good now, lmk if there's any other 
issues.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90568/new/

https://reviews.llvm.org/D90568

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D132952: [Sema] disable -Wvla for function array parameters

2022-11-24 Thread Yingchi Long via Phabricator via cfe-commits
inclyc abandoned this revision.
inclyc added a comment.

Prefer https://reviews.llvm.org/D137343


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132952/new/

https://reviews.llvm.org/D132952

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D136156: [Clang][Diagnostic] Add hidden-reinterpret-cast diagnostic warning

2022-11-24 Thread Tom Weaver via Phabricator via cfe-commits
TWeaver added a comment.

Hello all,

First of all, thank you so much for your reviews, comments and time.

Secondly, I apologise for the late reply.

I've be thinking about your statements and I have to somewhat agree. This 
doesn't catch undefined behaviour, at least by its self. It may help catch 
undefined behaviour as a result of 'bad-code', but it's not going to catch UB 
as a result of the cast in every case. The clang-tidy checks are fine but they 
don't actually drill down to the functionality I'm looking for, aka, "this 
functional cast is a reinterpret_cast - beware!" (missing from this patch is a 
suggestion to write 'reinterpret_cast(...)' instead).

Also, I'm aware that there's a warning for picking up old style c casts in the 
compiler, enabled by providing '-Wold-style-cast' on the command line. Are old 
style c casts not also language features? If that's the case why is its 
diagnostic embedded in the compiler and not in clang-tidy?

I ask this because there's an argument to be made that C++ style functional 
casts are just as dangerous as old-style casts. They can produce reinterpret 
casts when used in the right (or wrong) context. Would it be possible to at 
least add a diagnostic for detecting any functional style casts instead?

$ clang.exe .\old-style-test.cpp -Wold-style-cast
.\old-style-test.cpp:3:11: warning: use of old-style cast [-Wold-style-cast]

  int y = (double)x;
  ^  

Thanks again for your time


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136156/new/

https://reviews.llvm.org/D136156

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D129156: Add -fpass-plugin option to Flang

2022-11-24 Thread Rainer Orth via Phabricator via cfe-commits
ro added a comment.

In D129156#3949194 , @awarzynski 
wrote:

> 



> Sorry that you are hitting this - things like this happen, sadly. I think 
> that the easiest to resolve it would be to tweak the expected error so that 
> it works on Solaris as well as other popular platforms. Given that you might 
> be the only person able to test on Solaris, would you be able to upload 
> something for  a review? Thanks!

No worries, especially given that this seems to be the first test ever to check 
the `Could not load library` error message.

I've just submitted a patch that works on Linux and Solaris (all that I can 
easily test): D138663 .


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129156/new/

https://reviews.llvm.org/D129156

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138651: [CUDA][HIP] Don't diagnose use for __bf16

2022-11-24 Thread Pierre van Houtryve via Phabricator via cfe-commits
Pierre-vh updated this revision to Diff 477760.
Pierre-vh added a comment.

Not all targets have bf16 and AuxTarget may not be available all the time so I 
changed the condition slightly


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138651/new/

https://reviews.llvm.org/D138651

Files:
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCUDA/amdgpu-bf16.cu


Index: clang/test/SemaCUDA/amdgpu-bf16.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/amdgpu-bf16.cu
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -triple amdgcn-amd-amdhsa -aux-triple 
x86_64-unknown-linux-gnu -verify %s
+// expected-no-diagnostics
+
+// If AMDGPU is the main target and X86 the aux target, ensure we
+// don't complain about unsupported BF16 types in x86 code.
+
+#include "Inputs/cuda.h"
+
+__device__ void devicefn() {
+}
+
+__bf16 hostfn(__bf16 a) {
+  return a;
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -1518,7 +1518,9 @@
 break;
   case DeclSpec::TST_half:Result = Context.HalfTy; break;
   case DeclSpec::TST_BFloat16:
-if (!S.Context.getTargetInfo().hasBFloat16Type())
+// Likewise, CUDA host and device may have different __bf16 support.
+if (!S.Context.getTargetInfo().hasBFloat16Type() && !S.getLangOpts().CUDA 
&&
+!(S.getLangOpts().OpenMP && S.getLangOpts().OpenMPIsDevice))
   S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported)
 << "__bf16";
 Result = Context.BFloat16Ty;
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2174,6 +2174,10 @@
   if (Target->hasBFloat16Type()) {
 Width = Target->getBFloat16Width();
 Align = Target->getBFloat16Align();
+  } else if (AuxTarget && AuxTarget->hasBFloat16Type() &&
+ (getLangOpts().OpenMP || getLangOpts().OpenMPIsDevice)) {
+Width = AuxTarget->getBFloat16Width();
+Align = AuxTarget->getBFloat16Align();
   }
   break;
 case BuiltinType::Float16:


Index: clang/test/SemaCUDA/amdgpu-bf16.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/amdgpu-bf16.cu
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -triple amdgcn-amd-amdhsa -aux-triple x86_64-unknown-linux-gnu -verify %s
+// expected-no-diagnostics
+
+// If AMDGPU is the main target and X86 the aux target, ensure we
+// don't complain about unsupported BF16 types in x86 code.
+
+#include "Inputs/cuda.h"
+
+__device__ void devicefn() {
+}
+
+__bf16 hostfn(__bf16 a) {
+  return a;
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -1518,7 +1518,9 @@
 break;
   case DeclSpec::TST_half:Result = Context.HalfTy; break;
   case DeclSpec::TST_BFloat16:
-if (!S.Context.getTargetInfo().hasBFloat16Type())
+// Likewise, CUDA host and device may have different __bf16 support.
+if (!S.Context.getTargetInfo().hasBFloat16Type() && !S.getLangOpts().CUDA &&
+!(S.getLangOpts().OpenMP && S.getLangOpts().OpenMPIsDevice))
   S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported)
 << "__bf16";
 Result = Context.BFloat16Ty;
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2174,6 +2174,10 @@
   if (Target->hasBFloat16Type()) {
 Width = Target->getBFloat16Width();
 Align = Target->getBFloat16Align();
+  } else if (AuxTarget && AuxTarget->hasBFloat16Type() &&
+ (getLangOpts().OpenMP || getLangOpts().OpenMPIsDevice)) {
+Width = AuxTarget->getBFloat16Width();
+Align = AuxTarget->getBFloat16Align();
   }
   break;
 case BuiltinType::Float16:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138559: Record macro references in #ifdef clause.

2022-11-24 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6ebd0aa42066: [include-cleaner] Record macro references in 
#ifdef clause. (authored by VitaNuo, committed by hokein).

Changed prior to commit:
  https://reviews.llvm.org/D138559?vs=477722=477759#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138559/new/

https://reviews.llvm.org/D138559

Files:
  clang-tools-extra/include-cleaner/lib/Record.cpp
  clang-tools-extra/include-cleaner/unittests/RecordTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -7,12 +7,13 @@
 //===--===//
 
 #include "clang-include-cleaner/Record.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Testing/TestAST.h"
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
-#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Testing/Support/Annotations.h"
 #include "gmock/gmock.h"
@@ -217,6 +218,44 @@
   EXPECT_THAT(ExpOffsets, ElementsAreArray(MainFile.points("exp")));
 }
 
+TEST_F(RecordPPTest, CapturesConditionalMacroRefs) {
+  llvm::Annotations MainFile(R"cpp(
+#define X 1
+
+#ifdef ^X
+#endif
+
+#if defined(^X)
+#endif
+
+#ifndef ^X
+#endif
+
+#ifdef Y
+#elifdef ^X
+#endif
+
+#ifndef ^X
+#elifndef ^X
+#endif
+  )cpp");
+
+  Inputs.Code = MainFile.code();
+  Inputs.ExtraArgs.push_back("-std=c++2b");
+  auto AST = build();
+
+  std::vector RefOffsets;
+  SourceManager  = AST.sourceManager();
+  for (const auto  : Recorded.MacroReferences) {
+auto [FID, Off] = SM.getDecomposedLoc(Ref.RefLocation);
+ASSERT_EQ(FID, SM.getMainFileID());
+EXPECT_EQ(Ref.RT, RefType::Ambiguous);
+EXPECT_EQ("X", Ref.Target.macro().Name->getName());
+RefOffsets.push_back(Off);
+  }
+  EXPECT_THAT(RefOffsets, ElementsAreArray(MainFile.points()));
+}
+
 // Matches an Include* on the specified line;
 MATCHER_P(line, N, "") { return arg->Line == (unsigned)N; }
 
Index: clang-tools-extra/include-cleaner/lib/Record.cpp
===
--- clang-tools-extra/include-cleaner/lib/Record.cpp
+++ clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -83,14 +83,54 @@
   recordMacroRef(MacroName, *MI);
   }
 
+  void Ifdef(SourceLocation Loc, const Token ,
+ const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Ifndef(SourceLocation Loc, const Token ,
+  const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Elifdef(SourceLocation Loc, const Token ,
+   const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Elifndef(SourceLocation Loc, const Token ,
+const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Defined(const Token , const MacroDefinition ,
+   SourceRange Range) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
 private:
-  void recordMacroRef(const Token , const MacroInfo ) {
+  void recordMacroRef(const Token , const MacroInfo ,
+  RefType RT = RefType::Explicit) {
 if (MI.isBuiltinMacro())
   return; // __FILE__ is not a reference.
-Recorded.MacroReferences.push_back(
-SymbolReference{Tok.getLocation(),
-Macro{Tok.getIdentifierInfo(), MI.getDefinitionLoc()},
-RefType::Explicit});
+Recorded.MacroReferences.push_back(SymbolReference{
+Tok.getLocation(),
+Macro{Tok.getIdentifierInfo(), MI.getDefinitionLoc()}, RT});
   }
 
   bool Active = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] 6ebd0aa - [include-cleaner] Record macro references in #ifdef clause.

2022-11-24 Thread Haojian Wu via cfe-commits

Author: Viktoriia Bakalova
Date: 2022-11-24T14:48:25+01:00
New Revision: 6ebd0aa42066dd1879217b260704571e440ce5ef

URL: 
https://github.com/llvm/llvm-project/commit/6ebd0aa42066dd1879217b260704571e440ce5ef
DIFF: 
https://github.com/llvm/llvm-project/commit/6ebd0aa42066dd1879217b260704571e440ce5ef.diff

LOG: [include-cleaner] Record macro references in #ifdef clause.

Records macro references in #ifdef clauses as ambiguous.

Reviewed By: hokein

Differential Revision: https://reviews.llvm.org/D138559

Added: 


Modified: 
clang-tools-extra/include-cleaner/lib/Record.cpp
clang-tools-extra/include-cleaner/unittests/RecordTest.cpp

Removed: 




diff  --git a/clang-tools-extra/include-cleaner/lib/Record.cpp 
b/clang-tools-extra/include-cleaner/lib/Record.cpp
index 16956b45198b6..618ec89e82ab7 100644
--- a/clang-tools-extra/include-cleaner/lib/Record.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -83,14 +83,54 @@ class PPRecorder : public PPCallbacks {
   recordMacroRef(MacroName, *MI);
   }
 
+  void Ifdef(SourceLocation Loc, const Token ,
+ const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Ifndef(SourceLocation Loc, const Token ,
+  const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Elifdef(SourceLocation Loc, const Token ,
+   const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Elifndef(SourceLocation Loc, const Token ,
+const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Defined(const Token , const MacroDefinition ,
+   SourceRange Range) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
 private:
-  void recordMacroRef(const Token , const MacroInfo ) {
+  void recordMacroRef(const Token , const MacroInfo ,
+  RefType RT = RefType::Explicit) {
 if (MI.isBuiltinMacro())
   return; // __FILE__ is not a reference.
-Recorded.MacroReferences.push_back(
-SymbolReference{Tok.getLocation(),
-Macro{Tok.getIdentifierInfo(), MI.getDefinitionLoc()},
-RefType::Explicit});
+Recorded.MacroReferences.push_back(SymbolReference{
+Tok.getLocation(),
+Macro{Tok.getIdentifierInfo(), MI.getDefinitionLoc()}, RT});
   }
 
   bool Active = false;

diff  --git a/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp 
b/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
index efe8e405102bc..fb4e0632fd242 100644
--- a/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -7,12 +7,13 @@
 
//===--===//
 
 #include "clang-include-cleaner/Record.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Testing/TestAST.h"
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
-#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Testing/Support/Annotations.h"
 #include "gmock/gmock.h"
@@ -217,6 +218,44 @@ TEST_F(RecordPPTest, CapturesMacroRefs) {
   EXPECT_THAT(ExpOffsets, ElementsAreArray(MainFile.points("exp")));
 }
 
+TEST_F(RecordPPTest, CapturesConditionalMacroRefs) {
+  llvm::Annotations MainFile(R"cpp(
+#define X 1
+
+#ifdef ^X
+#endif
+
+#if defined(^X)
+#endif
+
+#ifndef ^X
+#endif
+
+#ifdef Y
+#elifdef ^X
+#endif
+
+#ifndef ^X
+#elifndef ^X
+#endif
+  )cpp");
+
+  Inputs.Code = MainFile.code();
+  Inputs.ExtraArgs.push_back("-std=c++2b");
+  auto AST = build();
+
+  std::vector RefOffsets;
+  SourceManager  = AST.sourceManager();
+  for (const auto  : Recorded.MacroReferences) {
+auto [FID, Off] = SM.getDecomposedLoc(Ref.RefLocation);
+ASSERT_EQ(FID, SM.getMainFileID());
+EXPECT_EQ(Ref.RT, RefType::Ambiguous);
+EXPECT_EQ("X", Ref.Target.macro().Name->getName());
+RefOffsets.push_back(Off);
+  }
+  EXPECT_THAT(RefOffsets, ElementsAreArray(MainFile.points()));
+}
+
 // Matches an Include* on the specified line;
 MATCHER_P(line, N, "") { return arg->Line == 

[PATCH] D138649: [include-cleaner] Show details for #include directives (used/unused)

2022-11-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Demo 
https://htmlpreview.github.io/?https://gist.githubusercontent.com/sam-mccall/24f7bcf835052d4ddcf6dad4f26a500c/raw/db4791b0b80f5c67c6d25c9090bf8166e362c556/PathMapping.cpp.html


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138649/new/

https://reviews.llvm.org/D138649

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138579: [AArch64] Assembly support for FEAT_LRCPC3

2022-11-24 Thread Sam Elliott via Phabricator via cfe-commits
lenary added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:8572
+  def STLR_x_64 : BaseLRCPC3IntegerLoadStore<0b11, 0b10, (outs GPR64sp:$Rn_wb) 
  , (ins GPR64:$Rt, GPR64sp:$Rn), "stlr" , "\t$Rt, [$Rn, #-8]!", "$Rn = 
$Rn_wb">; /* PUSH register */
+  def LDAPR_w_32: BaseLRCPC3IntegerLoadStore<0b10, 0b11, (outs GPR32:$Rt, 
GPR64sp:$Rn_wb), (ins GPR64sp:$Rn)   , "ldapr", "\t$Rt, [$Rn], #4"  , 
"$Rn = $Rn_wb">; /* POP register */
+  def LDAPR_x_64: BaseLRCPC3IntegerLoadStore<0b11, 0b11, (outs GPR64:$Rt, 
GPR64sp:$Rn_wb), (ins GPR64sp:$Rn)   , "ldapr", "\t$Rt, [$Rn], #8"  , 
"$Rn = $Rn_wb">; /* POP register */

Additional comment: can you change the order of `(outs` here, as most writeback 
instructions seem to make their writeback register the first of the outs. A 
good example is `LDRXpost` in the fully-expanded tablegen.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138579/new/

https://reviews.llvm.org/D138579

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D129156: Add -fpass-plugin option to Flang

2022-11-24 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski added a comment.

In D129156#3949156 , @ro wrote:

> This introduced a new failure on Solaris:
>
>   FAIL: Flang :: Driver/pass-plugin-not-found.f90
>
> Running the failing command manually shows:
>
>   error: unable to load plugin 'X.Y': 'Could not load library 'X.Y': ld.so.1: 
> flang-new: X.Y: open failed: No such file or directory'
>
> while the test currently expects
>
>   error: unable to load plugin 'X.Y': 'Could not load library 'X.Y': X.Y: 
> cannot open shared object file: No such file or directory'
>
> This expectation is unjustified, I believe: the message is produced by 
> `PassPlugin::Load` -> `sys::DynamicLibrary::getPermanentLibrary` -> 
> `HandleSet::DLOpen` -> `::dlerror`.  Obviously the output of the last is 
> system-dependent; I don't think we can put any requirements on it (maybe not 
> even the exact wording of the `No such file or directory` part.

Sorry that you are hitting this - things like this happen, sadly. I think that 
the easiest to resolve it would be to tweak the expected error so that it works 
on Solaris as well as other popular platforms. Given that you might be the only 
person able to test on Solaris, would you be able to upload something for  a 
review? Thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129156/new/

https://reviews.llvm.org/D129156

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138651: [CUDA][HIP] Don't diagnose use for __bf16

2022-11-24 Thread Pierre van Houtryve via Phabricator via cfe-commits
Pierre-vh planned changes to this revision.
Pierre-vh added a comment.

Need to fix a test crash


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138651/new/

https://reviews.llvm.org/D138651

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138583: Create unused non-trivially-destructible check in clang-tidy

2022-11-24 Thread Andrei via Phabricator via cfe-commits
ankineri requested review of this revision.
ankineri added a comment.

`-Wunused-variable` does not detect unused non-trivially-destructible objects 
because they may be used for RAII, i.e.

  {
scoped_lock lock(_mutex);
critical_section();
  }

`lock` here is not an unused variable because its destructor side-effects are 
its usage. However if replaced with say `absl::Status` or even `std::string` it 
will be more or less obvious that the variable is unused, but the warning will 
still not be triggered (as both `absl::Status` and `std::string` have 
nontrivial destructors). This is where this checks is useful: for 
non-trivially-destructible types known not to be used in RAII.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138583/new/

https://reviews.llvm.org/D138583

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D129156: Add -fpass-plugin option to Flang

2022-11-24 Thread Rainer Orth via Phabricator via cfe-commits
ro added a comment.

This introduced a new failure on Solaris:

  FAIL: Flang :: Driver/pass-plugin-not-found.f90

Running the failing command manually shows:

  error: unable to load plugin 'X.Y': 'Could not load library 'X.Y': ld.so.1: 
flang-new: X.Y: open failed: No such file or directory'

while the test currently expects

  error: unable to load plugin 'X.Y': 'Could not load library 'X.Y': X.Y: 
cannot open shared object file: No such file or directory'

This expectation is unjustified, I believe: the message is produced by 
`PassPlugin::Load` -> `sys::DynamicLibrary::getPermanentLibrary` -> 
`HandleSet::DLOpen` -> `::dlerror`.  Obviously the output of the last is 
system-dependent; I don't think we can put any requirements on it (maybe not 
even the exact wording of the `No such file or directory` part.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129156/new/

https://reviews.llvm.org/D129156

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138559: Record macro references in #ifdef clause.

2022-11-24 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

Thanks, LGTM. I will commit it for you.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138559/new/

https://reviews.llvm.org/D138559

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D90568: [clang] Add [is|set]Nested methods to NamespaceDecl

2022-11-24 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

This breaks the build: http://45.33.8.238/linux/92294/step_4.txt

Please take a look and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90568/new/

https://reviews.llvm.org/D90568

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D108230: [analyzer] Ignore single element arrays in getStaticSize() conditionally

2022-11-24 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment.

I proposed the D138657  and D138659 
 patches for getting rid of this flag.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108230/new/

https://reviews.llvm.org/D108230

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138659: [analyzer] Deprecate FAM analyzer-config, recommend -fstrict-flex-arrays instead

2022-11-24 Thread Balázs Benics via Phabricator via cfe-commits
steakhal created this revision.
steakhal added reviewers: NoQ, xazax.hun.
Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp, donat.nagy, 
Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware.
Herald added a reviewer: Szelethus.
Herald added a project: All.
steakhal requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

By default, clang assumes that all trailing array objects could be a
FAM. So, an array of undefined size, size 0, size 1, or even size 42 is
considered as FAMs for optimizations at least.

One needs to override the default behavior by supplying the
`-fstrict-flex-arrays=` flag, with `N > 0` value to reduce the set of
FAM candidates. Value `3` is the most restrictive and `0` is the most
permissive on this scale.

0: all trailing arrays are FAMs
1: only incomplete, zero and one-element arrays are FAMs
2: only incomplete, zero-element arrays are FAMs
3: only incomplete arrays are FAMs

If the user is happy with considering single-element arrays as FAMs, they
just need to remove the
`consider-single-element-arrays-as-flexible-array-members` from the
command line.
Otherwise, if they don't want to recognize such cases as FAMs, they
should specify `-fstrict-flex-arrays` anyway, which will be picked up by
CSA.

Any use of the deprecated analyzer-config value will trigger a warning
explaining what to use instead.
The `-analyzer-config-help` is updated accordingly.

Depends on D138657 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138659

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/test/Analysis/deprecated-flags-and-options.cpp
  clang/test/Analysis/flexible-array-members.c

Index: clang/test/Analysis/flexible-array-members.c
===
--- clang/test/Analysis/flexible-array-members.c
+++ clang/test/Analysis/flexible-array-members.c
@@ -1,27 +1,30 @@
+// -fstrict-flex-arrays=2 means that only undefined or zero element arrays are considered as FAMs.
+
 // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c90 \
-// RUN:-analyzer-config consider-single-element-arrays-as-flexible-array-members=false
+// RUN:-fstrict-flex-arrays=2
 // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c99 \
-// RUN:-analyzer-config consider-single-element-arrays-as-flexible-array-members=false
+// RUN:-fstrict-flex-arrays=2
 // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c11 \
-// RUN:-analyzer-config consider-single-element-arrays-as-flexible-array-members=false
+// RUN:-fstrict-flex-arrays=2
 // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c17 \
-// RUN:-analyzer-config consider-single-element-arrays-as-flexible-array-members=false
+// RUN:-fstrict-flex-arrays=2
 
 // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++98 -x c++ \
-// RUN:-analyzer-config consider-single-element-arrays-as-flexible-array-members=false
+// RUN:-fstrict-flex-arrays=2
 // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++03 -x c++ \
-// RUN:-analyzer-config consider-single-element-arrays-as-flexible-array-members=false
+// RUN:-fstrict-flex-arrays=2
 // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++11 -x c++ \
-// RUN:-analyzer-config consider-single-element-arrays-as-flexible-array-members=false
+// RUN:-fstrict-flex-arrays=2
 // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++14 -x c++ \
-// RUN:-analyzer-config consider-single-element-arrays-as-flexible-array-members=false
+// RUN:-fstrict-flex-arrays=2
 // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++17 -x c++ \
-// RUN:-analyzer-config consider-single-element-arrays-as-flexible-array-members=false
+// RUN:-fstrict-flex-arrays=2
 
+// By default, -fstrict-flex-arrays=0, which means that even single element arrays are considered as FAMs.
 // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu -analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c17 \
-// RUN:-analyzer-config consider-single-element-arrays-as-flexible-array-members=true -DSINGLE_ELEMENT_FAMS
+// RUN:-DSINGLE_ELEMENT_FAMS
 // RUN: 

[PATCH] D90568: [clang] Add [is|set]Nested methods to NamespaceDecl

2022-11-24 Thread Nathan James via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG15e76eed0c76: [clang] Add [is|set]Nested methods to 
NamespaceDecl (authored by njames93).

Changed prior to commit:
  https://reviews.llvm.org/D90568?vs=477467=477750#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90568/new/

https://reviews.llvm.org/D90568

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/HLSLExternalSemaSource.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/AST/ast-dump-decl.cpp
  clang/test/AST/ast-dump-namespace-json.cpp
  clang/unittests/Sema/ExternalSemaSourceTest.cpp

Index: clang/unittests/Sema/ExternalSemaSourceTest.cpp
===
--- clang/unittests/Sema/ExternalSemaSourceTest.cpp
+++ clang/unittests/Sema/ExternalSemaSourceTest.cpp
@@ -121,7 +121,7 @@
   CurrentSema->getPreprocessor().getIdentifierInfo(CorrectTo);
   NamespaceDecl *NewNamespace =
   NamespaceDecl::Create(Context, DestContext, false, Typo.getBeginLoc(),
-Typo.getLoc(), ToIdent, nullptr);
+Typo.getLoc(), ToIdent, nullptr, false);
   DestContext->addDecl(NewNamespace);
   TypoCorrection Correction(ToIdent);
   Correction.addCorrectionDecl(NewNamespace);
Index: clang/test/AST/ast-dump-namespace-json.cpp
===
--- clang/test/AST/ast-dump-namespace-json.cpp
+++ clang/test/AST/ast-dump-namespace-json.cpp
@@ -170,6 +170,7 @@
 // CHECK-NEXT: }
 // CHECK-NEXT:},
 // CHECK-NEXT:"name": "quux"
+// CHECK-NEXT:"isNested": true
 // CHECK-NEXT:   }
 // CHECK-NEXT:  ]
 // CHECK-NEXT: }
@@ -195,7 +196,7 @@
 // CHECK-NEXT:"tokLen": 1
 // CHECK-NEXT:   }
 // CHECK-NEXT:  },
-// CHECK-NEXT:  "name": "quux",
+// CHECK-NEXT:  "name": "quux"
 // CHECK-NEXT:  "inner": [
 // CHECK-NEXT:   {
 // CHECK-NEXT:"id": "0x{{.*}}",
@@ -220,7 +221,8 @@
 // CHECK-NEXT: }
 // CHECK-NEXT:},
 // CHECK-NEXT:"name": "frobble",
-// CHECK-NEXT:"isInline": true
+// CHECK-NEXT:"isInline": true,
+// CHECK-NEXT:"isNested": true
 // CHECK-NEXT:   }
 // CHECK-NEXT:  ]
 // CHECK-NEXT: }
Index: clang/test/AST/ast-dump-decl.cpp
===
--- clang/test/AST/ast-dump-decl.cpp
+++ clang/test/AST/ast-dump-decl.cpp
@@ -53,6 +53,23 @@
 }
 // CHECK:  NamespaceDecl{{.*}} TestNamespaceDeclInline inline
 
+namespace TestNestedNameSpace::Nested {
+}
+// CHECK:  NamespaceDecl{{.*}} TestNestedNameSpace
+// CHECK:  NamespaceDecl{{.*}} Nested nested{{\s*$}}
+
+namespace TestMultipleNested::SecondLevelNested::Nested {
+}
+// CHECK:  NamespaceDecl{{.*}} TestMultipleNested
+// CHECK:  NamespaceDecl{{.*}} SecondLevelNested nested
+// CHECK:  NamespaceDecl{{.*}} Nested nested{{\s*$}}
+
+namespace TestInlineNested::inline SecondLevel::inline Nested {
+}
+// CHECK:  NamespaceDecl{{.*}} TestInlineNested
+// CHECK:  NamespaceDecl{{.*}} SecondLevel inline nested
+// CHECK:  NamespaceDecl{{.*}} Nested inline nested{{\s*$}}
+
 namespace testUsingDirectiveDecl {
   namespace A {
   }
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1252,6 +1252,7 @@
   VisitRedeclarable(D);
   VisitNamedDecl(D);
   Record.push_back(D->isInline());
+  Record.push_back(D->isNested());
   Record.AddSourceLocation(D->getBeginLoc());
   Record.AddSourceLocation(D->getRBraceLoc());
 
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -1745,6 +1745,7 @@
   RedeclarableResult Redecl = VisitRedeclarable(D);
   VisitNamedDecl(D);
   D->setInline(Record.readInt());
+  D->setNested(Record.readInt());
   D->LocStart = readSourceLocation();
   D->RBraceLoc = readSourceLocation();
 
@@ -1758,7 +1759,7 @@
   } else {
 // Link this namespace back to the first declaration, which has already
 // been deserialized.
-D->AnonOrFirstNamespaceAndInline.setPointer(D->getFirstDecl());
+D->AnonOrFirstNamespaceAndFlags.setPointer(D->getFirstDecl());
   }
 
   mergeRedeclarable(D, Redecl);
@@ -2784,8 +2785,8 @@
 // We cannot have loaded any redeclarations of this declaration yet, so
 // 

[clang] 15e76ee - [clang] Add [is|set]Nested methods to NamespaceDecl

2022-11-24 Thread Nathan James via cfe-commits

Author: Nathan James
Date: 2022-11-24T12:44:35Z
New Revision: 15e76eed0c7662f8a4bce849a58637070d3b0a75

URL: 
https://github.com/llvm/llvm-project/commit/15e76eed0c7662f8a4bce849a58637070d3b0a75
DIFF: 
https://github.com/llvm/llvm-project/commit/15e76eed0c7662f8a4bce849a58637070d3b0a75.diff

LOG: [clang] Add [is|set]Nested methods to NamespaceDecl

Adds support for NamespaceDecl to inform if its part of a nested namespace.
This flag only corresponds to the inner namespaces in a nested namespace 
declaration.
In this example:
namespace  {}
Only  and  will be classified as nested.

This flag isn't meant for assisting in building the AST, more for static 
analysis and refactorings.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D90568

Added: 


Modified: 
clang/include/clang/AST/Decl.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/ASTImporter.cpp
clang/lib/AST/DeclCXX.cpp
clang/lib/AST/ItaniumMangle.cpp
clang/lib/AST/JSONNodeDumper.cpp
clang/lib/AST/TextNodeDumper.cpp
clang/lib/Parse/ParseDeclCXX.cpp
clang/lib/Sema/HLSLExternalSemaSource.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTWriterDecl.cpp
clang/test/AST/ast-dump-decl.cpp
clang/test/AST/ast-dump-namespace-json.cpp
clang/unittests/Sema/ExternalSemaSourceTest.cpp

Removed: 




diff  --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 1e05d1e425a28..d391d98fd32de 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -542,6 +542,9 @@ class LabelDecl : public NamedDecl {
 class NamespaceDecl : public NamedDecl, public DeclContext,
   public Redeclarable
 {
+
+  enum Flags : unsigned { F_Inline = 1 << 0, F_Nested = 1 << 1 };
+
   /// The starting location of the source range, pointing
   /// to either the namespace or the inline keyword.
   SourceLocation LocStart;
@@ -553,11 +556,12 @@ class NamespaceDecl : public NamedDecl, public 
DeclContext,
   /// this namespace or to the first namespace in the chain (the latter case
   /// only when this is not the first in the chain), along with a
   /// boolean value indicating whether this is an inline namespace.
-  llvm::PointerIntPair AnonOrFirstNamespaceAndInline;
+  llvm::PointerIntPair
+  AnonOrFirstNamespaceAndFlags;
 
   NamespaceDecl(ASTContext , DeclContext *DC, bool Inline,
 SourceLocation StartLoc, SourceLocation IdLoc,
-IdentifierInfo *Id, NamespaceDecl *PrevDecl);
+IdentifierInfo *Id, NamespaceDecl *PrevDecl, bool Nested);
 
   using redeclarable_base = Redeclarable;
 
@@ -569,10 +573,10 @@ class NamespaceDecl : public NamedDecl, public 
DeclContext,
   friend class ASTDeclReader;
   friend class ASTDeclWriter;
 
-  static NamespaceDecl *Create(ASTContext , DeclContext *DC,
-   bool Inline, SourceLocation StartLoc,
-   SourceLocation IdLoc, IdentifierInfo *Id,
-   NamespaceDecl *PrevDecl);
+  static NamespaceDecl *Create(ASTContext , DeclContext *DC, bool Inline,
+   SourceLocation StartLoc, SourceLocation IdLoc,
+   IdentifierInfo *Id, NamespaceDecl *PrevDecl,
+   bool Nested);
 
   static NamespaceDecl *CreateDeserialized(ASTContext , unsigned ID);
 
@@ -601,12 +605,33 @@ class NamespaceDecl : public NamedDecl, public 
DeclContext,
 
   /// Returns true if this is an inline namespace declaration.
   bool isInline() const {
-return AnonOrFirstNamespaceAndInline.getInt();
+return AnonOrFirstNamespaceAndFlags.getInt() & F_Inline;
   }
 
   /// Set whether this is an inline namespace declaration.
   void setInline(bool Inline) {
-AnonOrFirstNamespaceAndInline.setInt(Inline);
+unsigned F = AnonOrFirstNamespaceAndFlags.getInt();
+if (Inline)
+  AnonOrFirstNamespaceAndFlags.setInt(F | F_Inline);
+else
+  AnonOrFirstNamespaceAndFlags.setInt(F & ~F_Inline);
+  }
+
+  /// Returns true if this is a nested namespace declaration.
+  /// \code
+  /// namespace outer::nested { }
+  /// \endcode
+  bool isNested() const {
+return AnonOrFirstNamespaceAndFlags.getInt() & F_Nested;
+  }
+
+  /// Set whether this is a nested namespace declaration.
+  void setNested(bool Nested) {
+unsigned F = AnonOrFirstNamespaceAndFlags.getInt();
+if (Nested)
+  AnonOrFirstNamespaceAndFlags.setInt(F | F_Nested);
+else
+  AnonOrFirstNamespaceAndFlags.setInt(F & ~F_Nested);
   }
 
   /// Returns true if the inline qualifier for \c Name is redundant.
@@ -635,11 +660,11 @@ class NamespaceDecl : public NamedDecl, public 
DeclContext,
   /// Retrieve the anonymous namespace nested inside this namespace,
   /// if any.
   

[PATCH] D138657: [analyzer] Consider single-elem arrays as FAMs by default

2022-11-24 Thread Balázs Benics via Phabricator via cfe-commits
steakhal created this revision.
steakhal added reviewers: NoQ, xazax.hun.
Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp, donat.nagy, 
Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware.
Herald added a reviewer: Szelethus.
Herald added a project: All.
steakhal requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

According to my measurement in https://reviews.llvm.org/D108230#3933232,
it seems like there is no drawback to enabling this analyzer-config by default.

Actually, enabling this by default would make it consistent with the
codegen of clang, which according to `-fstrict-flex-arrays`, assumes
by default that all trailing arrays could be FAMs, let them be of size
undefined, zero, one, or anything else.

Speaking of `-fstrict-flex-arrays`, in the next patch I'll deprecate
the analyzer-config FAM option in favor of that flag. That way, CSA will
always be in sync with what the codegen will think of FAMs.

So, if a new codebase sets `-fstrict-flex-arrays` to some value above 0,
CSA will also make sure that only arrays of the right size will be
considered as FAMs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138657

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/flexible-array-members.c


Index: clang/test/Analysis/flexible-array-members.c
===
--- clang/test/Analysis/flexible-array-members.c
+++ clang/test/Analysis/flexible-array-members.c
@@ -1,13 +1,22 @@
-// RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu 
-analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c90
-// RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu 
-analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c99
-// RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu 
-analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c11
-// RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu 
-analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c17
-
-// RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu 
-analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++98 -x c++
-// RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu 
-analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++03 -x c++
-// RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu 
-analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++11 -x c++
-// RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu 
-analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++14 -x c++
-// RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu 
-analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++17 -x c++
+// RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu 
-analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c90 \
+// RUN:-analyzer-config 
consider-single-element-arrays-as-flexible-array-members=false
+// RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu 
-analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c99 \
+// RUN:-analyzer-config 
consider-single-element-arrays-as-flexible-array-members=false
+// RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu 
-analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c11 \
+// RUN:-analyzer-config 
consider-single-element-arrays-as-flexible-array-members=false
+// RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu 
-analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c17 \
+// RUN:-analyzer-config 
consider-single-element-arrays-as-flexible-array-members=false
+
+// RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu 
-analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++98 -x c++ \
+// RUN:-analyzer-config 
consider-single-element-arrays-as-flexible-array-members=false
+// RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu 
-analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++03 -x c++ \
+// RUN:-analyzer-config 
consider-single-element-arrays-as-flexible-array-members=false
+// RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu 
-analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++11 -x c++ \
+// RUN:-analyzer-config 
consider-single-element-arrays-as-flexible-array-members=false
+// RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu 
-analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++14 -x c++ \
+// RUN:-analyzer-config 
consider-single-element-arrays-as-flexible-array-members=false
+// RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu 
-analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c++17 -x c++ \
+// RUN:-analyzer-config 
consider-single-element-arrays-as-flexible-array-members=false
 
 // RUN: %clang_analyze_cc1 -triple x86_64-linux-gnu 
-analyzer-checker=core,unix,debug.ExprInspection %s -verify -std=c17 \
 // RUN:-analyzer-config 

[PATCH] D138648: [include-cleaner] Make Symbol (and Macro) hashable.

2022-11-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

thanks, lgtm!




Comment at: 
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h:147
+  using Outer = clang::include_cleaner::Symbol;
+  constexpr static auto Member = ::Storage;
+  using Base = DenseMapInfo;

probably left over?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138648/new/

https://reviews.llvm.org/D138648

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138583: Create unused non-trivially-destructible check in clang-tidy

2022-11-24 Thread Nathan James via Phabricator via cfe-commits
njames93 requested changes to this revision.
njames93 added a comment.
This revision now requires changes to proceed.

Clang already has a warning `-Wunused-variable` that is designed for this 
specific purpose. So unless this is bringing anything enhanced functionality I 
don't see the need for this check.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138583/new/

https://reviews.llvm.org/D138583

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138579: [AArch64] Assembly support for FEAT_LRCPC3

2022-11-24 Thread Dave Green via Phabricator via cfe-commits
dmgreen added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:11772
+  : I,
+Sched<[]> {
+  bits<5> Rt;

One extra nit: Can we add a scheduling description? These sound like they can 
use WriteAtomic.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138579/new/

https://reviews.llvm.org/D138579

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138655: [clang-tidy] Fix `cppcoreguidelines-init-variables` for invalid vardecl

2022-11-24 Thread gehry via Phabricator via cfe-commits
Sockke created this revision.
Sockke added a reviewer: aaron.ballman.
Herald added subscribers: carlosgalvezp, shchenz, kbarton, xazax.hun, nemanjai.
Herald added a reviewer: njames93.
Herald added a project: All.
Sockke requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

https://godbolt.org/z/n4cK4fo3o
The checker missed a check for invalid vardecl and this could cause a false 
positive.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138655

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp


Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -60,6 +60,8 @@
   const ASTContext  = *Result.Context;
   const SourceManager  = Context.getSourceManager();
 
+  if (MatchedDecl->isInvalidDecl())
+return;
   // We want to warn about cases where the type name
   // comes from a macro like this:
   //


Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -60,6 +60,8 @@
   const ASTContext  = *Result.Context;
   const SourceManager  = Context.getSourceManager();
 
+  if (MatchedDecl->isInvalidDecl())
+return;
   // We want to warn about cases where the type name
   // comes from a macro like this:
   //
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D70401: [RISCV] Complete RV32E/ilp32e implementation

2022-11-24 Thread Wang Pengcheng via Phabricator via cfe-commits
pcwang-thead added a comment.

In D70401#3948829 , @StephenFan wrote:

> In D70401#3873874 , @pcwang-thead 
> wrote:
>
>> In D70401#3873347 , @luojia wrote:
>>
>>> Hello! Any further updates to this patch? It seems like all the inline 
>>> comments have been resolved.
>>
>> We have done some works in this patch to make it compatible with GCC, it can 
>> be combined with GNU toolchain now.
>>
>> But as what have been discussed[1, 2], we may proceed with this patch when 
>> RV32E/ilp32e is ratified.
>>
>> 1. https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/269
>> 2. https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/257
>
> RV32E/ilp32e has been 
> ratified(https://github.com/riscv-non-isa/riscv-elf-psabi-doc). Do you plan 
> to proceed with this patch? :)

I will follow the proceeding of spec and finish this patch, but I don't think 
they have been ratified.
There are some changes about RV32E/RV64E 
, but I think they are 
still **proposal**.
And, there are still some issues we need to fix in the psabi:

- Add the LP64E ABI, to support RV64E 

- ilp32e issue 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70401/new/

https://reviews.llvm.org/D70401

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D137340: [clang-tidy] Add misc-use-anonymous-namespace check

2022-11-24 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment.

Another week, another ping @njames93 @aaron.ballman

I have addressed all comments since 3 weeks ago, and have not received any 
objections. I intend to land this patch next week (1st December) if I don't 
receive any further feedback.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137340/new/

https://reviews.llvm.org/D137340

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D137531: [clang] Add the check of membership in decltype for the issue #58674

2022-11-24 Thread Liming Liu via Phabricator via cfe-commits
lime updated this revision to Diff 477737.
lime added a comment.

Undo unrelated format changes.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137531/new/

https://reviews.llvm.org/D137531

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/decltype.cpp

Index: clang/test/SemaCXX/decltype.cpp
===
--- clang/test/SemaCXX/decltype.cpp
+++ clang/test/SemaCXX/decltype.cpp
@@ -101,6 +101,44 @@
   template void foo(decltype(T(LP1{ .p1 = g1, .p1.x[1] = 'x' }))) {}
 }
 
+namespace GH58674 {
+  struct Foo {
+float value_;
+struct nested {
+  float value_;
+};
+  };
+
+  template 
+  struct TemplateFoo {
+float value_;
+  };
+
+  float bar;
+
+  template 
+  struct Animal{};
+
+  template 
+  class Cat : Animal {
+using okay = decltype(Foo::value_);
+using also_okay = decltype(bar);
+using okay2 = decltype(Foo::nested::value_);
+using okay3 = decltype(TemplateFoo::value_);
+  public:
+void meow() {
+  using okay = decltype(Foo::value_);
+  using also_okay = decltype(bar);
+  using okay2 = decltype(Foo::nested::value_);
+  using okay3 = decltype(TemplateFoo::value_);
+}
+  };
+
+  void baz() {
+  Cat{}.meow();
+  }
+}
+
 template
 class conditional {
 };
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -2692,20 +2692,34 @@
   // to get this right here so that we don't end up making a
   // spuriously dependent expression if we're inside a dependent
   // instance method.
+  //
+  // We also don't need to do this if R resolved to a member in another
+  // class, which can happen in an unevaluated operand:
+  //
+  // C++ [expr.prim.id]p3.3:
+  //   If that id-expression denotes a non-static data member and it
+  //   appears in an unevaluated operand.
   if (!R.empty() && (*R.begin())->isCXXClassMember()) {
-bool MightBeImplicitMember;
-if (!IsAddressOfOperand)
-  MightBeImplicitMember = true;
-else if (!SS.isEmpty())
-  MightBeImplicitMember = false;
-else if (R.isOverloadedResult())
-  MightBeImplicitMember = false;
-else if (R.isUnresolvableResult())
-  MightBeImplicitMember = true;
-else
-  MightBeImplicitMember = isa(R.getFoundDecl()) ||
-  isa(R.getFoundDecl()) ||
-  isa(R.getFoundDecl());
+bool MightBeImplicitMember = true, CheckField = true;
+if (IsAddressOfOperand) {
+  MightBeImplicitMember = SS.isEmpty() && !R.isOverloadedResult();
+  CheckField = !R.isUnresolvableResult();
+}
+if (MightBeImplicitMember && CheckField) {
+  if (R.isSingleResult() &&
+  isa(R.getFoundDecl())) {
+auto Class = cast((*R.begin())->getDeclContext());
+for (auto Curr = S->getLookupEntity(); Curr && !Curr->isFileContext();
+ Curr = Curr->getParent()) {
+  if (auto ThisClass = dyn_cast_if_present(Curr)) {
+if ((MightBeImplicitMember = ThisClass->Equals(Class) ||
+ ThisClass->isDerivedFrom(Class)))
+  break;
+  }
+}
+  } else if (IsAddressOfOperand)
+MightBeImplicitMember = false;
+}
 
 if (MightBeImplicitMember)
   return BuildPossibleImplicitMemberExpr(SS, TemplateKWLoc,
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -304,6 +304,9 @@
 - GNU attributes being applied prior to standard attributes would be handled
   improperly, which was corrected to match the behaviour exhibited by GCC.
   `Issue 58229 `_
+- Fix an issue about ``decltype`` in the members of class templates derived from
+  templates with related parameters.
+  `Issue 58674 `_
 
 Improvements to Clang's diagnostics
 ^^^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D137531: [clang] Add the check of membership in decltype for the issue #58674

2022-11-24 Thread Liming Liu via Phabricator via cfe-commits
lime updated this revision to Diff 477735.
lime added a comment.

Rebase.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137531/new/

https://reviews.llvm.org/D137531

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/decltype.cpp

Index: clang/test/SemaCXX/decltype.cpp
===
--- clang/test/SemaCXX/decltype.cpp
+++ clang/test/SemaCXX/decltype.cpp
@@ -101,6 +101,44 @@
   template void foo(decltype(T(LP1{ .p1 = g1, .p1.x[1] = 'x' }))) {}
 }
 
+namespace GH58674 {
+  struct Foo {
+float value_;
+struct nested {
+  float value_;
+};
+  };
+
+  template 
+  struct TemplateFoo {
+float value_;
+  };
+
+  float bar;
+
+  template 
+  struct Animal{};
+
+  template 
+  class Cat : Animal {
+using okay = decltype(Foo::value_);
+using also_okay = decltype(bar);
+using okay2 = decltype(Foo::nested::value_);
+using okay3 = decltype(TemplateFoo::value_);
+  public:
+void meow() {
+  using okay = decltype(Foo::value_);
+  using also_okay = decltype(bar);
+  using okay2 = decltype(Foo::nested::value_);
+  using okay3 = decltype(TemplateFoo::value_);
+}
+  };
+
+  void baz() {
+  Cat{}.meow();
+  }
+}
+
 template
 class conditional {
 };
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -2692,20 +2692,34 @@
   // to get this right here so that we don't end up making a
   // spuriously dependent expression if we're inside a dependent
   // instance method.
+  //
+  // We also don't need to do this if R resolved to a member in another
+  // class, which can happen in an unevaluated operand:
+  //
+  // C++ [expr.prim.id]p3.3:
+  //   If that id-expression denotes a non-static data member and it
+  //   appears in an unevaluated operand.
   if (!R.empty() && (*R.begin())->isCXXClassMember()) {
-bool MightBeImplicitMember;
-if (!IsAddressOfOperand)
-  MightBeImplicitMember = true;
-else if (!SS.isEmpty())
-  MightBeImplicitMember = false;
-else if (R.isOverloadedResult())
-  MightBeImplicitMember = false;
-else if (R.isUnresolvableResult())
-  MightBeImplicitMember = true;
-else
-  MightBeImplicitMember = isa(R.getFoundDecl()) ||
-  isa(R.getFoundDecl()) ||
-  isa(R.getFoundDecl());
+bool MightBeImplicitMember = true, CheckField = true;
+if (IsAddressOfOperand) {
+  MightBeImplicitMember = SS.isEmpty() && !R.isOverloadedResult();
+  CheckField = !R.isUnresolvableResult();
+}
+if (MightBeImplicitMember && CheckField) {
+  if (R.isSingleResult() &&
+  isa(R.getFoundDecl())) {
+auto Class = cast((*R.begin())->getDeclContext());
+for (auto Curr = S->getLookupEntity(); Curr && !Curr->isFileContext();
+ Curr = Curr->getParent()) {
+  if (auto ThisClass = dyn_cast_if_present(Curr)) {
+if ((MightBeImplicitMember = ThisClass->Equals(Class) ||
+ ThisClass->isDerivedFrom(Class)))
+  break;
+  }
+}
+  } else if (IsAddressOfOperand)
+MightBeImplicitMember = false;
+}
 
 if (MightBeImplicitMember)
   return BuildPossibleImplicitMemberExpr(SS, TemplateKWLoc,
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -304,6 +304,9 @@
 - GNU attributes being applied prior to standard attributes would be handled
   improperly, which was corrected to match the behaviour exhibited by GCC.
   `Issue 58229 `_
+- Fix an issue about ``decltype`` in the members of class templates derived from
+  templates with related parameters.
+  `Issue 58674 `_
 
 Improvements to Clang's diagnostics
 ^^^
@@ -815,8 +818,8 @@
 - Introduced the new function ``clang_CXXMethod_isMoveAssignmentOperator``,
   which identifies whether a method cursor is a move-assignment
   operator.
-- ``clang_Cursor_getNumTemplateArguments``, ``clang_Cursor_getTemplateArgumentKind``, 
-  ``clang_Cursor_getTemplateArgumentType``, ``clang_Cursor_getTemplateArgumentValue`` and 
+- ``clang_Cursor_getNumTemplateArguments``, ``clang_Cursor_getTemplateArgumentKind``,
+  ``clang_Cursor_getTemplateArgumentType``, ``clang_Cursor_getTemplateArgumentValue`` and
   ``clang_Cursor_getTemplateArgumentUnsignedValue`` now work on struct, class,
   and partial template specialization cursors in addition to function cursors.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138651: [CUDA][HIP] Don't diagnose use for __bf16

2022-11-24 Thread Pierre van Houtryve via Phabricator via cfe-commits
Pierre-vh updated this revision to Diff 477734.
Pierre-vh added a comment.

Add newline at end of file


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138651/new/

https://reviews.llvm.org/D138651

Files:
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCUDA/amdgpu-bf16.cu


Index: clang/test/SemaCUDA/amdgpu-bf16.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/amdgpu-bf16.cu
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -triple amdgcn-amd-amdhsa -aux-triple 
x86_64-unknown-linux-gnu -verify %s
+// expected-no-diagnostics
+
+// If AMDGPU is the main target and X86 the aux target, ensure we
+// don't complain about unsupported BF16 types in x86 code.
+
+#include "Inputs/cuda.h"
+
+__device__ void devicefn() {
+}
+
+__bf16 hostfn(__bf16 a) {
+  return a;
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -1518,7 +1518,9 @@
 break;
   case DeclSpec::TST_half:Result = Context.HalfTy; break;
   case DeclSpec::TST_BFloat16:
-if (!S.Context.getTargetInfo().hasBFloat16Type())
+// Likewise, CUDA host and device may have different __bf16 support.
+if (!S.Context.getTargetInfo().hasBFloat16Type() && !S.getLangOpts().CUDA 
&&
+!(S.getLangOpts().OpenMP && S.getLangOpts().OpenMPIsDevice))
   S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported)
 << "__bf16";
 Result = Context.BFloat16Ty;
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2171,9 +2171,15 @@
   Align = Target->getLongFractAlign();
   break;
 case BuiltinType::BFloat16:
-  if (Target->hasBFloat16Type()) {
+  if (Target->hasBFloat16Type() || !getLangOpts().OpenMP ||
+  !getLangOpts().OpenMPIsDevice) {
 Width = Target->getBFloat16Width();
 Align = Target->getBFloat16Align();
+  } else {
+assert(getLangOpts().OpenMP && getLangOpts().OpenMPIsDevice &&
+   "Expected OpenMP device compilation.");
+Width = AuxTarget->getBFloat16Width();
+Align = AuxTarget->getBFloat16Align();
   }
   break;
 case BuiltinType::Float16:


Index: clang/test/SemaCUDA/amdgpu-bf16.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/amdgpu-bf16.cu
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -triple amdgcn-amd-amdhsa -aux-triple x86_64-unknown-linux-gnu -verify %s
+// expected-no-diagnostics
+
+// If AMDGPU is the main target and X86 the aux target, ensure we
+// don't complain about unsupported BF16 types in x86 code.
+
+#include "Inputs/cuda.h"
+
+__device__ void devicefn() {
+}
+
+__bf16 hostfn(__bf16 a) {
+  return a;
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -1518,7 +1518,9 @@
 break;
   case DeclSpec::TST_half:Result = Context.HalfTy; break;
   case DeclSpec::TST_BFloat16:
-if (!S.Context.getTargetInfo().hasBFloat16Type())
+// Likewise, CUDA host and device may have different __bf16 support.
+if (!S.Context.getTargetInfo().hasBFloat16Type() && !S.getLangOpts().CUDA &&
+!(S.getLangOpts().OpenMP && S.getLangOpts().OpenMPIsDevice))
   S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported)
 << "__bf16";
 Result = Context.BFloat16Ty;
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2171,9 +2171,15 @@
   Align = Target->getLongFractAlign();
   break;
 case BuiltinType::BFloat16:
-  if (Target->hasBFloat16Type()) {
+  if (Target->hasBFloat16Type() || !getLangOpts().OpenMP ||
+  !getLangOpts().OpenMPIsDevice) {
 Width = Target->getBFloat16Width();
 Align = Target->getBFloat16Align();
+  } else {
+assert(getLangOpts().OpenMP && getLangOpts().OpenMPIsDevice &&
+   "Expected OpenMP device compilation.");
+Width = AuxTarget->getBFloat16Width();
+Align = AuxTarget->getBFloat16Align();
   }
   break;
 case BuiltinType::Float16:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138579: [AArch64] Assembly support for FEAT_LRCPC3

2022-11-24 Thread Sam Elliott via Phabricator via cfe-commits
lenary accepted this revision.
lenary added a comment.
This revision is now accepted and ready to land.

Some comment nits that you can fixup on commit.




Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:3913
+// are post-indexed, and the immediate values are not inside the [] brackets 
and
+// thus not verified by GPR64sp0 parser.
+def STLRW0  : InstAlias<"stlr\t$Rt, [$Rn, #0]" , (STLRW   GPR32: $Rt, 
GPR64sp:$Rn)>;

Maybe?

Maybe also clarify that `GPR64sp0` is not appropriate because it parses and 
discards the optional zero.



Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:8556
+  //   size   opcopc2
+  def STILPwo : BaseLRCPC3IntegerLoadStorePair<0b10, 0b00, 0b, (outs 
GPR64sp:$Rn_wb)   , (ins GPR32:$Rt, GPR32:$Rt2, 
GPR64sp:$Rn), "stilp" , "\t$Rt, $Rt2, [$Rn, #-8]!",  "$Rn = $Rn_wb">; /* PUSH 
register pair */
+  def STILPxo : BaseLRCPC3IntegerLoadStorePair<0b11, 0b00, 0b, (outs 
GPR64sp:$Rn_wb)   , (ins GPR64:$Rt, GPR64:$Rt2, 
GPR64sp:$Rn), "stilp" , "\t$Rt, $Rt2, [$Rn, #-16]!", "$Rn = $Rn_wb">; /* PUSH 
register pair */

nit: Please remove this comment, it doesn't correspond to anything in the spec, 
and I don't think it's helpful.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138579/new/

https://reviews.llvm.org/D138579

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138630: [modules] Fix marking `ObjCMethodDecl::isOverriding` when there a no overrides.

2022-11-24 Thread Egor Zhdan via Phabricator via cfe-commits
egorzhdan accepted this revision.
egorzhdan added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138630/new/

https://reviews.llvm.org/D138630

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138651: [CUDA][HIP] Don't diagnose use for __bf16

2022-11-24 Thread Pierre van Houtryve via Phabricator via cfe-commits
Pierre-vh created this revision.
Pierre-vh added reviewers: arsenm, rjmccall, tra.
Herald added subscribers: kosarev, mattd, kerbowa, pengfei, tpr, yaxunl, 
jvesely.
Herald added a project: All.
Pierre-vh requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1, wdng.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

e0fb01e97b6b7d2fe66b17b36eeb98aa78c6e3bb 
 caused 
issues in some of our HIP projects. Builds were failing because "__bf16" wasn't 
allowed on the target. This is because in those cases, the main target is 
AMDGPU (which doesn't have bf16), and the aux target is X86 (which has bf16).

This implements a fix similar to D57369  but 
for bf16 which prevents Clang from diagnosing uses of bf16 when compiling 
heterogenous applications.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138651

Files:
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCUDA/amdgpu-bf16.cu


Index: clang/test/SemaCUDA/amdgpu-bf16.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/amdgpu-bf16.cu
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -triple amdgcn-amd-amdhsa -aux-triple 
x86_64-unknown-linux-gnu -verify %s
+// expected-no-diagnostics
+
+// If AMDGPU is the main target and X86 the aux target, ensure we
+// don't complain about unsupported BF16 types in x86 code.
+
+#include "Inputs/cuda.h"
+
+__device__ void devicefn() {
+}
+
+__bf16 hostfn(__bf16 a) {
+  return a;
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -1518,7 +1518,9 @@
 break;
   case DeclSpec::TST_half:Result = Context.HalfTy; break;
   case DeclSpec::TST_BFloat16:
-if (!S.Context.getTargetInfo().hasBFloat16Type())
+// Likewise, CUDA host and device may have different __bf16 support.
+if (!S.Context.getTargetInfo().hasBFloat16Type() && !S.getLangOpts().CUDA 
&&
+!(S.getLangOpts().OpenMP && S.getLangOpts().OpenMPIsDevice))
   S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported)
 << "__bf16";
 Result = Context.BFloat16Ty;
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2171,9 +2171,15 @@
   Align = Target->getLongFractAlign();
   break;
 case BuiltinType::BFloat16:
-  if (Target->hasBFloat16Type()) {
+  if (Target->hasBFloat16Type() || !getLangOpts().OpenMP ||
+  !getLangOpts().OpenMPIsDevice) {
 Width = Target->getBFloat16Width();
 Align = Target->getBFloat16Align();
+  } else {
+assert(getLangOpts().OpenMP && getLangOpts().OpenMPIsDevice &&
+   "Expected OpenMP device compilation.");
+Width = AuxTarget->getBFloat16Width();
+Align = AuxTarget->getBFloat16Align();
   }
   break;
 case BuiltinType::Float16:


Index: clang/test/SemaCUDA/amdgpu-bf16.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/amdgpu-bf16.cu
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -triple amdgcn-amd-amdhsa -aux-triple x86_64-unknown-linux-gnu -verify %s
+// expected-no-diagnostics
+
+// If AMDGPU is the main target and X86 the aux target, ensure we
+// don't complain about unsupported BF16 types in x86 code.
+
+#include "Inputs/cuda.h"
+
+__device__ void devicefn() {
+}
+
+__bf16 hostfn(__bf16 a) {
+  return a;
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -1518,7 +1518,9 @@
 break;
   case DeclSpec::TST_half:Result = Context.HalfTy; break;
   case DeclSpec::TST_BFloat16:
-if (!S.Context.getTargetInfo().hasBFloat16Type())
+// Likewise, CUDA host and device may have different __bf16 support.
+if (!S.Context.getTargetInfo().hasBFloat16Type() && !S.getLangOpts().CUDA &&
+!(S.getLangOpts().OpenMP && S.getLangOpts().OpenMPIsDevice))
   S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported)
 << "__bf16";
 Result = Context.BFloat16Ty;
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2171,9 +2171,15 @@
   Align = Target->getLongFractAlign();
   break;
 case BuiltinType::BFloat16:
-  if (Target->hasBFloat16Type()) {
+  if (Target->hasBFloat16Type() || !getLangOpts().OpenMP ||
+  !getLangOpts().OpenMPIsDevice) {
 Width = Target->getBFloat16Width();
 Align = Target->getBFloat16Align();
+  } 

[PATCH] D138649: [include-cleaner] Show details for #include directives (used/unused)

2022-11-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added a project: All.
sammccall requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138649

Files:
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
  clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
  clang-tools-extra/include-cleaner/lib/Record.cpp

Index: clang-tools-extra/include-cleaner/lib/Record.cpp
===
--- clang-tools-extra/include-cleaner/lib/Record.cpp
+++ clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -337,6 +337,13 @@
   BySpellingIt->second.push_back(Index);
   if (I.Resolved)
 ByFile[I.Resolved].push_back(Index);
+  ByLine[I.Line] = Index;
+}
+
+const Include *
+RecordedPP::RecordedIncludes::atLine(unsigned OneBasedIndex) const {
+  auto It = ByLine.find(OneBasedIndex);
+  return (It == ByLine.end()) ? nullptr : [It->second];
 }
 
 llvm::SmallVector
Index: clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
===
--- clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
+++ clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
@@ -37,7 +37,7 @@
 text-align: right;
 width: 3em; padding-right: 0.5em; margin-right: 0.5em;
   }
-  .ref { text-decoration: underline; color: #008; }
+  .ref, .inc { text-decoration: underline; color: #008; }
   .sel { position: relative; cursor: pointer; }
   .ref.implicit { background-color: #ff8; }
   #hover {
@@ -49,9 +49,10 @@
 padding: 0.5em;
   }
   #hover p, #hover pre { margin: 0; }
-  #hover .target.implicit { background-color: #bbb; }
-  #hover .target.ambiguous { background-color: #caf; }
+  #hover .target.implicit, .provides .implicit { background-color: #bbb; }
+  #hover .target.ambiguous, .provides .ambiguous { background-color: #caf; }
   .missing, .unused { background-color: #faa !important; }
+  .semiused { background-color: #888 !important; }
   #hover th { color: #008; text-align: right; padding-right: 0.5em; }
   #hover .target:not(:first-child) {
 margin-top: 1em;
@@ -95,6 +96,22 @@
   llvm_unreachable("unhandled symbol kind");
 }
 
+// Return detailed symbol description (declaration), if we have any.
+std::string printDetails(const Symbol ) {
+  std::string S;
+  if (Sym.kind() == Symbol::Declaration) {
+// Print the declaration of the symbol, e.g. to disambiguate overloads.
+const auto  = Sym.declaration();
+PrintingPolicy PP = D.getASTContext().getPrintingPolicy();
+PP.FullyQualifiedName = true;
+PP.TerseOutput = true;
+PP.SuppressInitializers = true;
+llvm::raw_string_ostream SS(S);
+D.print(SS, PP);
+  }
+  return S;
+}
+
 llvm::StringRef refType(RefType T) {
   switch (T) {
   case RefType::Explicit:
@@ -139,6 +156,18 @@
 }
   };
   std::vector Refs;
+  llvm::DenseMap> IncludeRefs;
+
+  llvm::StringRef includeType(const Include *I) {
+auto  = IncludeRefs[I];
+if (List.empty())
+  return "unused";
+if (llvm::any_of(List, [&](unsigned I) {
+  return Targets[Refs[I].TargetIndex].Type == RefType::Explicit;
+}))
+  return "used";
+return "semiused";
+  }
 
   Target makeTarget(const SymbolReference ) {
 Target T{SR.Target, SR.RT, {}, {}, {}};
@@ -194,6 +223,8 @@
 
 Refs.push_back({Offset, SR.RT == RefType::Implicit, Targets.size()});
 Targets.push_back(makeTarget(SR));
+for (const auto *I : Targets.back().Includes)
+  IncludeRefs[I].push_back(Targets.size() - 1);
   }
 
   void write() {
@@ -202,6 +233,11 @@
 OS << "\n";
 OS << "" << CSS << "\n";
 OS << "" << JS << "\n";
+for (auto  : Includes.all()) {
+  OS << "";
+  writeInclude(Inc);
+  OS << "\n";
+}
 for (unsigned I = 0; I < Targets.size(); ++I) {
   OS << "";
   writeTarget(Targets[I]);
@@ -260,6 +296,45 @@
 OS << ">";
   }
 
+  void writeInclude(const Include ) {
+OS << "";
+if (Inc.Resolved) {
+  OS << "Resolved";
+  escapeString(Inc.Resolved->getName());
+  OS << "\n";
+}
+// We show one ref for each symbol: first by (RefType != Explicit, Sequence)
+llvm::DenseMap FirstRef;
+for (unsigned RefIndex : IncludeRefs[]) {
+  const Target  = Targets[Refs[RefIndex].TargetIndex];
+  auto I = FirstRef.try_emplace(T.Sym, RefIndex);
+  if (!I.second && T.Type == RefType::Explicit &&
+  Targets[Refs[I.first->second].TargetIndex].Type != RefType::Explicit)
+I.first->second = RefIndex;
+}
+std::vector> Sorted = {FirstRef.begin(),
+   FirstRef.end()};
+llvm::stable_sort(Sorted, llvm::less_second{});
+for (auto &[S, RefIndex] : Sorted) {
+  auto  = Targets[Refs[RefIndex].TargetIndex];
+  OS << "Provides";
+  std::string Details = 

[PATCH] D138648: [include-cleaner] Make Symbol (and Macro) hashable.

2022-11-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added a project: All.
sammccall requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

For now, we decided not to add operator< or handle other variants.
(If we do so in future we may want to extract a base class).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138648

Files:
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h


Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
===
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
@@ -67,6 +67,9 @@
   // FIXME: Add support for macros.
   // Order must match Kind enum!
   std::variant Storage;
+
+  Symbol(decltype(Storage) Sentinel) : Storage(std::move(Sentinel)) {}
+  friend llvm::DenseMapInfo;
 };
 llvm::raw_ostream <<(llvm::raw_ostream &, const Symbol &);
 
@@ -137,4 +140,37 @@
 } // namespace include_cleaner
 } // namespace clang
 
+namespace llvm {
+
+template <> struct DenseMapInfo {
+  using Outer = clang::include_cleaner::Symbol;
+  constexpr static auto Member = ::Storage;
+  using Base = DenseMapInfo;
+
+  static inline Outer getEmptyKey() { return {Base::getEmptyKey()}; }
+  static inline Outer getTombstoneKey() { return {Base::getTombstoneKey()}; }
+  static unsigned getHashValue(const Outer ) {
+return Base::getHashValue(Val.Storage);
+  }
+  static bool isEqual(const Outer , const Outer ) {
+return Base::isEqual(LHS.Storage, RHS.Storage);
+  }
+};
+template <> struct DenseMapInfo {
+  using Outer = clang::include_cleaner::Macro;
+  using Base = DenseMapInfo;
+
+  static inline Outer getEmptyKey() { return {nullptr, Base::getEmptyKey()}; }
+  static inline Outer getTombstoneKey() {
+return {nullptr, Base::getTombstoneKey()};
+  }
+  static unsigned getHashValue(const Outer ) {
+return Base::getHashValue(Val.Definition);
+  }
+  static bool isEqual(const Outer , const Outer ) {
+return Base::isEqual(LHS.Definition, RHS.Definition);
+  }
+};
+} // namespace llvm
+
 #endif


Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
===
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
@@ -67,6 +67,9 @@
   // FIXME: Add support for macros.
   // Order must match Kind enum!
   std::variant Storage;
+
+  Symbol(decltype(Storage) Sentinel) : Storage(std::move(Sentinel)) {}
+  friend llvm::DenseMapInfo;
 };
 llvm::raw_ostream <<(llvm::raw_ostream &, const Symbol &);
 
@@ -137,4 +140,37 @@
 } // namespace include_cleaner
 } // namespace clang
 
+namespace llvm {
+
+template <> struct DenseMapInfo {
+  using Outer = clang::include_cleaner::Symbol;
+  constexpr static auto Member = ::Storage;
+  using Base = DenseMapInfo;
+
+  static inline Outer getEmptyKey() { return {Base::getEmptyKey()}; }
+  static inline Outer getTombstoneKey() { return {Base::getTombstoneKey()}; }
+  static unsigned getHashValue(const Outer ) {
+return Base::getHashValue(Val.Storage);
+  }
+  static bool isEqual(const Outer , const Outer ) {
+return Base::isEqual(LHS.Storage, RHS.Storage);
+  }
+};
+template <> struct DenseMapInfo {
+  using Outer = clang::include_cleaner::Macro;
+  using Base = DenseMapInfo;
+
+  static inline Outer getEmptyKey() { return {nullptr, Base::getEmptyKey()}; }
+  static inline Outer getTombstoneKey() {
+return {nullptr, Base::getTombstoneKey()};
+  }
+  static unsigned getHashValue(const Outer ) {
+return Base::getHashValue(Val.Definition);
+  }
+  static bool isEqual(const Outer , const Outer ) {
+return Base::isEqual(LHS.Definition, RHS.Definition);
+  }
+};
+} // namespace llvm
+
 #endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D70401: [RISCV] Complete RV32E/ilp32e implementation

2022-11-24 Thread luxufan via Phabricator via cfe-commits
StephenFan added a comment.

In D70401#3873874 , @pcwang-thead 
wrote:

> In D70401#3873347 , @luojia wrote:
>
>> Hello! Any further updates to this patch? It seems like all the inline 
>> comments have been resolved.
>
> We have done some works in this patch to make it compatible with GCC, it can 
> be combined with GNU toolchain now.
>
> But as what have been discussed[1, 2], we may proceed with this patch when 
> RV32E/ilp32e is ratified.
>
> 1. https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/269
> 2. https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/257

RV32E/ilp32e has been 
ratified(https://github.com/riscv-non-isa/riscv-elf-psabi-doc). Do you plan to 
proceed with this patch? :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70401/new/

https://reviews.llvm.org/D70401

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138559: Record macro references in #ifdef clause.

2022-11-24 Thread Viktoriia Bakalova via Phabricator via cfe-commits
VitaNuo updated this revision to Diff 477722.
VitaNuo added a comment.

Simplify test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138559/new/

https://reviews.llvm.org/D138559

Files:
  clang-tools-extra/include-cleaner/lib/Record.cpp
  clang-tools-extra/include-cleaner/unittests/RecordTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -7,12 +7,13 @@
 //===--===//
 
 #include "clang-include-cleaner/Record.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Testing/TestAST.h"
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
-#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Testing/Support/Annotations.h"
 #include "gmock/gmock.h"
@@ -218,6 +219,44 @@
   EXPECT_THAT(ExpOffsets, ElementsAreArray(MainFile.points("exp")));
 }
 
+TEST_F(RecordPPTest, CapturesConditionalMacroRefs) {
+  llvm::Annotations MainFile(R"cpp(
+#define X 1
+
+#ifdef ^X
+#endif
+
+#if defined(^X)
+#endif
+
+#ifndef ^X
+#endif
+
+#ifdef Y
+#elifdef ^X
+#endif
+
+#ifndef ^X
+#elifndef ^X
+#endif
+  )cpp");
+
+  Inputs.Code = MainFile.code();
+  Inputs.ExtraArgs.push_back("-std=c++2b");
+  auto AST = build();
+
+  std::vector RefOffsets;
+  SourceManager  = AST.sourceManager();
+  for (const auto  : Recorded.MacroReferences) {
+auto [FID, Off] = SM.getDecomposedLoc(Ref.RefLocation);
+ASSERT_EQ(FID, SM.getMainFileID());
+EXPECT_EQ(Ref.RT, RefType::Ambiguous);
+EXPECT_EQ("X", Ref.Target.macro().Name->getName());
+RefOffsets.push_back(Off);
+  }
+  EXPECT_THAT(RefOffsets, ElementsAreArray(MainFile.points()));
+}
+
 // Matches an Include* on the specified line;
 MATCHER_P(line, N, "") { return arg->Line == (unsigned)N; }
 
Index: clang-tools-extra/include-cleaner/lib/Record.cpp
===
--- clang-tools-extra/include-cleaner/lib/Record.cpp
+++ clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -83,14 +83,54 @@
   recordMacroRef(MacroName, *MI);
   }
 
+  void Ifdef(SourceLocation Loc, const Token ,
+ const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Ifndef(SourceLocation Loc, const Token ,
+  const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Elifdef(SourceLocation Loc, const Token ,
+   const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Elifndef(SourceLocation Loc, const Token ,
+const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Defined(const Token , const MacroDefinition ,
+   SourceRange Range) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
 private:
-  void recordMacroRef(const Token , const MacroInfo ) {
+  void recordMacroRef(const Token , const MacroInfo ,
+  RefType RT = RefType::Explicit) {
 if (MI.isBuiltinMacro())
   return; // __FILE__ is not a reference.
-Recorded.MacroReferences.push_back(
-SymbolReference{Tok.getLocation(),
-Macro{Tok.getIdentifierInfo(), MI.getDefinitionLoc()},
-RefType::Explicit});
+Recorded.MacroReferences.push_back(SymbolReference{
+Tok.getLocation(),
+Macro{Tok.getIdentifierInfo(), MI.getDefinitionLoc()}, RT});
   }
 
   bool Active = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138559: Record macro references in #ifdef clause.

2022-11-24 Thread Viktoriia Bakalova via Phabricator via cfe-commits
VitaNuo added inline comments.



Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:249
+
+  Inputs.Code = MainFile.code();
+  auto AST = build();

hokein wrote:
> The `elifndef` and `elifdef` is a C++2b extension feature, so 
> `Inputs.ExtraArgs.push_back("-std=c++2b");` to get rid of the 
> `[-Wc++2b-extensions]` warning in the testcase.
Thanks!



Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:248
+  SourceManager  = AST.sourceManager();
+  ASSERT_THAT(Recorded.MacroReferences, Not(IsEmpty()));
+

hokein wrote:
> VitaNuo wrote:
> > hokein wrote:
> > > nit: this can be removed, as the EXPECT_THAT on line 258 covers this.
> > Sorry, not sure what this comment refers to. Can it be that the line 
> > numbering changed due to my newer patch, and this comment does not show up 
> > in the correct place anymore?
> yeah, the comment was attached to the old snapshot. it is the line 254 now (` 
> ASSERT_THAT(Recorded.MacroReferences, Not(IsEmpty()));`).
Ok,removed the check.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138559/new/

https://reviews.llvm.org/D138559

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138559: Record macro references in #ifdef clause.

2022-11-24 Thread Viktoriia Bakalova via Phabricator via cfe-commits
VitaNuo updated this revision to Diff 477721.
VitaNuo added a comment.

Add -std=c++2b argument to avoid compiler warnings.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138559/new/

https://reviews.llvm.org/D138559

Files:
  clang-tools-extra/include-cleaner/lib/Record.cpp
  clang-tools-extra/include-cleaner/unittests/RecordTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -7,12 +7,13 @@
 //===--===//
 
 #include "clang-include-cleaner/Record.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Testing/TestAST.h"
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
-#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Testing/Support/Annotations.h"
 #include "gmock/gmock.h"
@@ -218,6 +219,49 @@
   EXPECT_THAT(ExpOffsets, ElementsAreArray(MainFile.points("exp")));
 }
 
+TEST_F(RecordPPTest, CapturesConditionalMacroRefs) {
+  llvm::Annotations MainFile(R"cpp(
+#define X 1
+
+#ifdef ^X
+#endif
+
+#if defined(^X)
+#endif
+
+#ifndef ^X
+#endif
+
+int main() {
+  #ifdef Y
+  #elifdef ^X
+  #else 
+  #endif
+
+  #ifndef ^X
+  #elifndef ^X
+  #else
+  #endif
+  return 0;
+}
+  )cpp");
+
+  Inputs.Code = MainFile.code();
+  Inputs.ExtraArgs.push_back("-std=c++2b");
+  auto AST = build();
+
+  std::vector RefOffsets;
+  SourceManager  = AST.sourceManager();
+  for (const auto  : Recorded.MacroReferences) {
+auto [FID, Off] = SM.getDecomposedLoc(Ref.RefLocation);
+ASSERT_EQ(FID, SM.getMainFileID());
+EXPECT_EQ(Ref.RT, RefType::Ambiguous);
+EXPECT_EQ("X", Ref.Target.macro().Name->getName());
+RefOffsets.push_back(Off);
+  }
+  EXPECT_THAT(RefOffsets, ElementsAreArray(MainFile.points()));
+}
+
 // Matches an Include* on the specified line;
 MATCHER_P(line, N, "") { return arg->Line == (unsigned)N; }
 
Index: clang-tools-extra/include-cleaner/lib/Record.cpp
===
--- clang-tools-extra/include-cleaner/lib/Record.cpp
+++ clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -83,14 +83,54 @@
   recordMacroRef(MacroName, *MI);
   }
 
+  void Ifdef(SourceLocation Loc, const Token ,
+ const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Ifndef(SourceLocation Loc, const Token ,
+  const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Elifdef(SourceLocation Loc, const Token ,
+   const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Elifndef(SourceLocation Loc, const Token ,
+const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Defined(const Token , const MacroDefinition ,
+   SourceRange Range) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
 private:
-  void recordMacroRef(const Token , const MacroInfo ) {
+  void recordMacroRef(const Token , const MacroInfo ,
+  RefType RT = RefType::Explicit) {
 if (MI.isBuiltinMacro())
   return; // __FILE__ is not a reference.
-Recorded.MacroReferences.push_back(
-SymbolReference{Tok.getLocation(),
-Macro{Tok.getIdentifierInfo(), MI.getDefinitionLoc()},
-RefType::Explicit});
+Recorded.MacroReferences.push_back(SymbolReference{
+Tok.getLocation(),
+Macro{Tok.getIdentifierInfo(), MI.getDefinitionLoc()}, RT});
   }
 
   bool Active = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138559: Record macro references in #ifdef clause.

2022-11-24 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:235
+
+int main() {
+  #ifdef Y

nit: we can get rid of the main function, it is not needed.



Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:238
+  #elifdef ^X
+  #else 
+  #endif

nit: this `#else` line can be removed.



Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:243
+  #elifndef ^X
+  #else
+  #endif

The same, can be removed.



Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:249
+
+  Inputs.Code = MainFile.code();
+  auto AST = build();

The `elifndef` and `elifdef` is a C++2b extension feature, so 
`Inputs.ExtraArgs.push_back("-std=c++2b");` to get rid of the 
`[-Wc++2b-extensions]` warning in the testcase.



Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:248
+  SourceManager  = AST.sourceManager();
+  ASSERT_THAT(Recorded.MacroReferences, Not(IsEmpty()));
+

VitaNuo wrote:
> hokein wrote:
> > nit: this can be removed, as the EXPECT_THAT on line 258 covers this.
> Sorry, not sure what this comment refers to. Can it be that the line 
> numbering changed due to my newer patch, and this comment does not show up in 
> the correct place anymore?
yeah, the comment was attached to the old snapshot. it is the line 254 now (` 
ASSERT_THAT(Recorded.MacroReferences, Not(IsEmpty()));`).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138559/new/

https://reviews.llvm.org/D138559

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138559: Record macro references in #ifdef clause.

2022-11-24 Thread Viktoriia Bakalova via Phabricator via cfe-commits
VitaNuo updated this revision to Diff 477716.
VitaNuo added a comment.

Formatting.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138559/new/

https://reviews.llvm.org/D138559

Files:
  clang-tools-extra/include-cleaner/lib/Record.cpp
  clang-tools-extra/include-cleaner/unittests/RecordTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -7,12 +7,13 @@
 //===--===//
 
 #include "clang-include-cleaner/Record.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Testing/TestAST.h"
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
-#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Testing/Support/Annotations.h"
 #include "gmock/gmock.h"
@@ -218,6 +219,49 @@
   EXPECT_THAT(ExpOffsets, ElementsAreArray(MainFile.points("exp")));
 }
 
+TEST_F(RecordPPTest, CapturesConditionalMacroRefs) {
+  llvm::Annotations MainFile(R"cpp(
+#define X 1
+
+#ifdef ^X
+#endif
+
+#if defined(^X)
+#endif
+
+#ifndef ^X
+#endif
+
+int main() {
+  #ifdef Y
+  #elifdef ^X
+  #else 
+  #endif
+
+  #ifndef ^X
+  #elifndef ^X
+  #else
+  #endif
+  return 0;
+}
+  )cpp");
+
+  Inputs.Code = MainFile.code();
+  auto AST = build();
+
+  std::vector RefOffsets;
+  SourceManager  = AST.sourceManager();
+  ASSERT_THAT(Recorded.MacroReferences, Not(IsEmpty()));
+  for (const auto  : Recorded.MacroReferences) {
+auto [FID, Off] = SM.getDecomposedLoc(Ref.RefLocation);
+ASSERT_EQ(FID, SM.getMainFileID());
+EXPECT_EQ(Ref.RT, RefType::Ambiguous);
+EXPECT_EQ("X", Ref.Target.macro().Name->getName());
+RefOffsets.push_back(Off);
+  }
+  EXPECT_THAT(RefOffsets, ElementsAreArray(MainFile.points()));
+}
+
 // Matches an Include* on the specified line;
 MATCHER_P(line, N, "") { return arg->Line == (unsigned)N; }
 
Index: clang-tools-extra/include-cleaner/lib/Record.cpp
===
--- clang-tools-extra/include-cleaner/lib/Record.cpp
+++ clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -83,14 +83,54 @@
   recordMacroRef(MacroName, *MI);
   }
 
+  void Ifdef(SourceLocation Loc, const Token ,
+ const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Ifndef(SourceLocation Loc, const Token ,
+  const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Elifdef(SourceLocation Loc, const Token ,
+   const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Elifndef(SourceLocation Loc, const Token ,
+const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Defined(const Token , const MacroDefinition ,
+   SourceRange Range) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
 private:
-  void recordMacroRef(const Token , const MacroInfo ) {
+  void recordMacroRef(const Token , const MacroInfo ,
+  RefType RT = RefType::Explicit) {
 if (MI.isBuiltinMacro())
   return; // __FILE__ is not a reference.
-Recorded.MacroReferences.push_back(
-SymbolReference{Tok.getLocation(),
-Macro{Tok.getIdentifierInfo(), MI.getDefinitionLoc()},
-RefType::Explicit});
+Recorded.MacroReferences.push_back(SymbolReference{
+Tok.getLocation(),
+Macro{Tok.getIdentifierInfo(), MI.getDefinitionLoc()}, RT});
   }
 
   bool Active = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138559: Record macro references in #ifdef clause.

2022-11-24 Thread Viktoriia Bakalova via Phabricator via cfe-commits
VitaNuo added inline comments.



Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:248
+  SourceManager  = AST.sourceManager();
+  ASSERT_THAT(Recorded.MacroReferences, Not(IsEmpty()));
+

hokein wrote:
> nit: this can be removed, as the EXPECT_THAT on line 258 covers this.
Sorry, not sure what this comment refers to. Can it be that the line numbering 
changed due to my newer patch, and this comment does not show up in the correct 
place anymore?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138559/new/

https://reviews.llvm.org/D138559

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138559: Record macro references in #ifdef clause.

2022-11-24 Thread Viktoriia Bakalova via Phabricator via cfe-commits
VitaNuo added inline comments.



Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:95
 private:
-  void recordMacroRef(const Token , const MacroInfo ) {
+  void recordMacroRef(const Token , const MacroInfo , RefType RT) {
 if (MI.isBuiltinMacro())

hokein wrote:
> nit: we can set a default value (`RefType::Explicit`) for the RT parameter, 
> then we don't need to pass the `RefType::Explicit` in all callsites.
Right, didn't know default parameter values were possible in C++, thanks.



Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:103
 private:
-  void recordMacroRef(const Token , const MacroInfo ) {
+  void recordMacroRef(const Token , const MacroInfo , RefType RT = 
RefType::Explicit) {
 if (MI.isBuiltinMacro())

hokein wrote:
> this line exceeds the max 80 character limit, clang-format should fix it.
Formatting should be fine now.



Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:228
+  llvm::Annotations MainFile(R"cpp(
+#include "header.h"
+

hokein wrote:
> I think we can simplify the testcase further -- the header.h is not needed, 
> we can use the macro `X` in all tests (`#ifdef`, `#ifndef` etc)
> 
Ok. I was trying to add some variety, but you're right - it's not the job of 
this test to check that things are included properly.



Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:233
+#ifdef ^X
+ #define Y 2
+#endif

hokein wrote:
> the `#define Y 2` can be removed, our test doesn't care about it. just use
> 
> ```
> #ifdef X
> #endif 
> ```
> is enough.
Ok, I'll remove all the fluff.



Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:255
+EXPECT_EQ(Ref.RT, RefType::Ambiguous);
+EXPECT_EQ(RefNames[I], Ref.Target.macro().Name->getName());  
+RefOffsets.push_back(Off);

hokein wrote:
> we're using the index of the array `Recorded.MacroRefrences` to access 
> another array, it is fine currently (because both arrays has the same size), 
> but we will get an out-of-bound-access issue if we add a case `#if 
> defined(X)..`to the `MainFile`.
> 
> If we just use a single macro `X` for all testcases, then the `RefdNames` is 
> not needed, we check ` EXPECT_EQ("X", Ref.Target.macro().Name->getName());`.
> 
> 
Ok, I've reduced everything to X.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138559/new/

https://reviews.llvm.org/D138559

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138559: Record macro references in #ifdef clause.

2022-11-24 Thread Viktoriia Bakalova via Phabricator via cfe-commits
VitaNuo updated this revision to Diff 477711.
VitaNuo added a comment.

Address review comments. Format and simplify code.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138559/new/

https://reviews.llvm.org/D138559

Files:
  clang-tools-extra/include-cleaner/lib/Record.cpp
  clang-tools-extra/include-cleaner/unittests/RecordTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -7,12 +7,13 @@
 //===--===//
 
 #include "clang-include-cleaner/Record.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Testing/TestAST.h"
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
-#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Testing/Support/Annotations.h"
 #include "gmock/gmock.h"
@@ -218,6 +219,49 @@
   EXPECT_THAT(ExpOffsets, ElementsAreArray(MainFile.points("exp")));
 }
 
+TEST_F(RecordPPTest, CapturesConditionalMacroRefs) {
+  llvm::Annotations MainFile(R"cpp(
+#define X 1
+
+#ifdef ^X
+#endif
+
+#if defined(^X)
+#endif
+
+#ifndef ^X
+#endif
+
+int main() {
+  #ifdef Y
+  #elifdef ^X
+  #else 
+  #endif
+
+  #ifndef ^X
+  #elifndef ^X
+  #else
+  #endif
+  return 0;
+}
+  )cpp");
+
+  Inputs.Code = MainFile.code();
+  auto AST = build();
+
+  std::vector RefOffsets;
+  SourceManager  = AST.sourceManager();
+  ASSERT_THAT(Recorded.MacroReferences, Not(IsEmpty()));
+  for (const auto& Ref : Recorded.MacroReferences) {
+auto [FID, Off] = SM.getDecomposedLoc(Ref.RefLocation);
+ASSERT_EQ(FID, SM.getMainFileID());
+EXPECT_EQ(Ref.RT, RefType::Ambiguous);
+EXPECT_EQ("X", Ref.Target.macro().Name->getName());
+RefOffsets.push_back(Off);
+  }
+  EXPECT_THAT(RefOffsets, ElementsAreArray(MainFile.points()));
+}
+
 // Matches an Include* on the specified line;
 MATCHER_P(line, N, "") { return arg->Line == (unsigned)N; }
 
Index: clang-tools-extra/include-cleaner/lib/Record.cpp
===
--- clang-tools-extra/include-cleaner/lib/Record.cpp
+++ clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -83,14 +83,54 @@
   recordMacroRef(MacroName, *MI);
   }
 
+  void Ifdef(SourceLocation Loc, const Token ,
+ const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Ifndef(SourceLocation Loc, const Token ,
+  const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Elifdef(SourceLocation Loc, const Token ,
+   const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Elifndef(SourceLocation Loc, const Token ,
+const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Defined(const Token , const MacroDefinition ,
+   SourceRange Range) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
 private:
-  void recordMacroRef(const Token , const MacroInfo ) {
+  void recordMacroRef(const Token , const MacroInfo ,
+  RefType RT = RefType::Explicit) {
 if (MI.isBuiltinMacro())
   return; // __FILE__ is not a reference.
-Recorded.MacroReferences.push_back(
-SymbolReference{Tok.getLocation(),
-Macro{Tok.getIdentifierInfo(), MI.getDefinitionLoc()},
-RefType::Explicit});
+Recorded.MacroReferences.push_back(SymbolReference{
+Tok.getLocation(),
+Macro{Tok.getIdentifierInfo(), MI.getDefinitionLoc()}, RT});
   }
 
   bool Active = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] 0780811 - [include-cleaner] Remove an unused local variable, NFC.

2022-11-24 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2022-11-24T11:05:34+01:00
New Revision: 078081171c4e916926e14e6204713f131f9ffb28

URL: 
https://github.com/llvm/llvm-project/commit/078081171c4e916926e14e6204713f131f9ffb28
DIFF: 
https://github.com/llvm/llvm-project/commit/078081171c4e916926e14e6204713f131f9ffb28.diff

LOG: [include-cleaner] Remove an unused local variable, NFC.

Added: 


Modified: 
clang-tools-extra/include-cleaner/unittests/RecordTest.cpp

Removed: 




diff  --git a/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp 
b/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
index 37eabb9dbfb28..efe8e405102bc 100644
--- a/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -199,7 +199,6 @@ TEST_F(RecordPPTest, CapturesMacroRefs) {
 
   std::vector RefOffsets;
   std::vector ExpOffsets; // Expansion locs of refs in macro locs.
-  std::vector RefMacroLocs;
   for (const auto  : Recorded.MacroReferences) {
 if (Ref.Target == OrigX) {
   auto [FID, Off] = SM.getDecomposedLoc(Ref.RefLocation);



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138559: Record macro references in #ifdef clause.

2022-11-24 Thread Viktoriia Bakalova via Phabricator via cfe-commits
VitaNuo updated this revision to Diff 477707.
VitaNuo added a comment.

Add support for #if defined(X), #elifdef, #elifndef


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138559/new/

https://reviews.llvm.org/D138559

Files:
  clang-tools-extra/include-cleaner/lib/Record.cpp
  clang-tools-extra/include-cleaner/unittests/RecordTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -7,12 +7,13 @@
 //===--===//
 
 #include "clang-include-cleaner/Record.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Testing/TestAST.h"
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
-#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Testing/Support/Annotations.h"
 #include "gmock/gmock.h"
@@ -218,6 +219,71 @@
   EXPECT_THAT(ExpOffsets, ElementsAreArray(MainFile.points("exp")));
 }
 
+TEST_F(RecordPPTest, CapturesConditionalMacroRefs) {
+  llvm::Annotations Header(R"cpp(
+#define Z 2
+#define CPU
+  )cpp");
+
+  llvm::Annotations MainFile(R"cpp(
+#include "header.h"
+
+#define X 1
+#define RAM
+
+#ifdef ^X
+ #define Y 2
+#endif
+
+#if defined(^X)
+ #define Q 3
+#endif
+
+#ifndef ^Z
+ #define W 4
+#endif
+
+int main() {
+  int result;
+  #ifdef GPU
+result = 1;
+  #elifdef ^RAM
+result = 2;
+  #else 
+result = 3;  
+  #endif
+
+  int result2;
+  #ifndef ^X
+result2 = 1;
+  #elifndef ^CPU
+result2 = 2;
+  #else
+result2 = 3;
+  #endif
+  return 0;
+}
+  )cpp");
+
+  Inputs.Code = MainFile.code();
+  Inputs.ExtraFiles["header.h"] = Header.code();
+  auto AST = build();
+
+  std::vector RefOffsets;
+  std::vector RefNames = {"X", "X", "Z", "RAM", "X", "CPU"};
+  SourceManager  = AST.sourceManager();
+  ASSERT_THAT(Recorded.MacroReferences, Not(IsEmpty()));
+  for (auto I = 0u; I < Recorded.MacroReferences.size(); I++) {
+const auto  = Recorded.MacroReferences[I];
+auto [FID, Off] = SM.getDecomposedLoc(Ref.RefLocation);
+ASSERT_EQ(FID, SM.getMainFileID());
+EXPECT_EQ(Ref.RT, RefType::Ambiguous);
+EXPECT_EQ(RefNames[I], Ref.Target.macro().Name->getName());
+RefOffsets.push_back(Off);
+  }
+  EXPECT_THAT(RefOffsets, ElementsAreArray(MainFile.points()));
+}
+
 // Matches an Include* on the specified line;
 MATCHER_P(line, N, "") { return arg->Line == (unsigned)N; }
 
Index: clang-tools-extra/include-cleaner/lib/Record.cpp
===
--- clang-tools-extra/include-cleaner/lib/Record.cpp
+++ clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -83,14 +83,54 @@
   recordMacroRef(MacroName, *MI);
   }
 
+  void Ifdef(SourceLocation Loc, const Token ,
+ const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Ifndef(SourceLocation Loc, const Token ,
+  const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Elifdef(SourceLocation Loc, const Token ,
+   const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Elifndef(SourceLocation Loc, const Token ,
+const MacroDefinition ) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
+  void Defined(const Token , const MacroDefinition ,
+   SourceRange Range) override {
+if (!Active)
+  return;
+if (const auto *MI = MD.getMacroInfo())
+  recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
+  }
+
 private:
-  void recordMacroRef(const Token , const MacroInfo ) {
+  void recordMacroRef(const Token , const MacroInfo ,
+  RefType RT = RefType::Explicit) {
 if (MI.isBuiltinMacro())
   return; // __FILE__ is not a reference.
-Recorded.MacroReferences.push_back(
-SymbolReference{Tok.getLocation(),
-Macro{Tok.getIdentifierInfo(), MI.getDefinitionLoc()},
-RefType::Explicit});
+Recorded.MacroReferences.push_back(SymbolReference{
+

[PATCH] D137149: Use PassGate from LLVMContext if any otherwise global one

2022-11-24 Thread Evgeniy via Phabricator via cfe-commits
ebrevnov updated this revision to Diff 477706.
ebrevnov added a comment.
Herald added a project: Flang.

Fixed build error in flang/lib/Frontend/FrontendActions.cpp


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137149/new/

https://reviews.llvm.org/D137149

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  flang/lib/Frontend/FrontendActions.cpp
  llvm/include/llvm/IR/OptBisect.h
  llvm/include/llvm/Passes/StandardInstrumentations.h
  llvm/lib/Analysis/CallGraphSCCPass.cpp
  llvm/lib/Analysis/LoopPass.cpp
  llvm/lib/Analysis/RegionPass.cpp
  llvm/lib/IR/LLVMContextImpl.cpp
  llvm/lib/IR/OptBisect.cpp
  llvm/lib/IR/Pass.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/LTO/ThinLTOCodeGenerator.cpp
  llvm/lib/Passes/PassBuilderBindings.cpp
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/tools/opt/NewPMDriver.cpp
  llvm/unittests/IR/LegacyPassManagerTest.cpp
  llvm/unittests/IR/PassManagerTest.cpp

Index: llvm/unittests/IR/PassManagerTest.cpp
===
--- llvm/unittests/IR/PassManagerTest.cpp
+++ llvm/unittests/IR/PassManagerTest.cpp
@@ -826,7 +826,7 @@
   FunctionAnalysisManager FAM;
   FunctionPassManager FPM;
   PassInstrumentationCallbacks PIC;
-  StandardInstrumentations SI(/*DebugLogging*/ true);
+  StandardInstrumentations SI(M->getContext(), /*DebugLogging*/ true);
   SI.registerCallbacks(PIC, );
   FAM.registerPass([&] { return PassInstrumentationAnalysis(); });
   FAM.registerPass([&] { return DominatorTreeAnalysis(); });
@@ -872,7 +872,7 @@
   FunctionAnalysisManager FAM;
   FunctionPassManager FPM;
   PassInstrumentationCallbacks PIC;
-  StandardInstrumentations SI(/*DebugLogging*/ true);
+  StandardInstrumentations SI(M->getContext(), /*DebugLogging*/ true);
   SI.registerCallbacks(PIC, );
   FAM.registerPass([&] { return PassInstrumentationAnalysis(); });
   FAM.registerPass([&] { return DominatorTreeAnalysis(); });
@@ -937,7 +937,7 @@
   FunctionAnalysisManager FAM;
   FunctionPassManager FPM;
   PassInstrumentationCallbacks PIC;
-  StandardInstrumentations SI(/*DebugLogging*/ true);
+  StandardInstrumentations SI(M->getContext(), /*DebugLogging*/ true);
   SI.registerCallbacks(PIC, );
   FAM.registerPass([&] { return PassInstrumentationAnalysis(); });
   FAM.registerPass([&] { return DominatorTreeAnalysis(); });
Index: llvm/unittests/IR/LegacyPassManagerTest.cpp
===
--- llvm/unittests/IR/LegacyPassManagerTest.cpp
+++ llvm/unittests/IR/LegacyPassManagerTest.cpp
@@ -359,10 +359,8 @@
 struct CustomOptPassGate : public OptPassGate {
   bool Skip;
   CustomOptPassGate(bool Skip) : Skip(Skip) { }
-  bool shouldRunPass(const Pass *P, StringRef IRDescription) override {
-if (P->getPassKind() == PT_Module)
-  return !Skip;
-return OptPassGate::shouldRunPass(P, IRDescription);
+  bool shouldRunPass(const StringRef PassName, StringRef IRDescription) override {
+return !Skip;
   }
   bool isEnabled() const override { return true; }
 };
Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -354,8 +354,8 @@
   PrintPassOptions PrintPassOpts;
   PrintPassOpts.Verbose = DebugPM == DebugLogging::Verbose;
   PrintPassOpts.SkipAnalyses = DebugPM == DebugLogging::Quiet;
-  StandardInstrumentations SI(DebugPM != DebugLogging::None, VerifyEachPass,
-  PrintPassOpts);
+  StandardInstrumentations SI(M.getContext(), DebugPM != DebugLogging::None,
+  VerifyEachPass, PrintPassOpts);
   SI.registerCallbacks(PIC, );
   DebugifyEachInstrumentation Debugify;
   DebugifyStatsMap DIStatsMap;
Index: llvm/lib/Passes/StandardInstrumentations.cpp
===
--- llvm/lib/Passes/StandardInstrumentations.cpp
+++ llvm/lib/Passes/StandardInstrumentations.cpp
@@ -767,27 +767,35 @@
   return ShouldRun;
 }
 
-void OptBisectInstrumentation::registerCallbacks(
+bool OptPassGateInstrumentation::shouldRun(StringRef PassName, Any IR) {
+  if (isIgnored(PassName))
+return true;
+
+  bool ShouldRun =
+  Context.getOptPassGate().shouldRunPass(PassName, getIRName(IR));
+  if (!ShouldRun && !this->HasWrittenIR && !OptBisectPrintIRPath.empty()) {
+// FIXME: print IR if limit is higher than number of opt-bisect
+// invocations
+this->HasWrittenIR = true;
+const Module *M = unwrapModule(IR, /*Force=*/true);
+assert((M && >getContext() == ) && "Missing/Mismatching Module");
+std::error_code EC;
+raw_fd_ostream OS(OptBisectPrintIRPath, EC);
+if (EC)
+  report_fatal_error(errorCodeToError(EC));
+M->print(OS, nullptr);
+  }
+  return ShouldRun;
+}
+
+void OptPassGateInstrumentation::registerCallbacks(
 PassInstrumentationCallbacks ) {
-  if 

[PATCH] D138559: Record macro references in #ifdef clause.

2022-11-24 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:87
+  void Ifdef(SourceLocation Loc, const Token ,
+ const MacroDefinition ) override {
+if (!Active)

the indentation doesn't look right, running clang-format should fix it.



Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:103
 private:
-  void recordMacroRef(const Token , const MacroInfo ) {
+  void recordMacroRef(const Token , const MacroInfo , RefType RT = 
RefType::Explicit) {
 if (MI.isBuiltinMacro())

this line exceeds the max 80 character limit, clang-format should fix it.



Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:228
+  llvm::Annotations MainFile(R"cpp(
+#include "header.h"
+

I think we can simplify the testcase further -- the header.h is not needed, we 
can use the macro `X` in all tests (`#ifdef`, `#ifndef` etc)




Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:233
+#ifdef ^X
+ #define Y 2
+#endif

the `#define Y 2` can be removed, our test doesn't care about it. just use

```
#ifdef X
#endif 
```
is enough.



Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:248
+  SourceManager  = AST.sourceManager();
+  ASSERT_THAT(Recorded.MacroReferences, Not(IsEmpty()));
+

nit: this can be removed, as the EXPECT_THAT on line 258 covers this.



Comment at: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp:255
+EXPECT_EQ(Ref.RT, RefType::Ambiguous);
+EXPECT_EQ(RefNames[I], Ref.Target.macro().Name->getName());  
+RefOffsets.push_back(Off);

we're using the index of the array `Recorded.MacroRefrences` to access another 
array, it is fine currently (because both arrays has the same size), but we 
will get an out-of-bound-access issue if we add a case `#if defined(X)..`to the 
`MainFile`.

If we just use a single macro `X` for all testcases, then the `RefdNames` is 
not needed, we check ` EXPECT_EQ("X", Ref.Target.macro().Name->getName());`.




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138559/new/

https://reviews.llvm.org/D138559

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


  1   2   >