[PATCH] D137153: [X86] Support -march=sierraforest, grandridge, graniterapids.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.

Drop trailing period from subject.




Comment at: clang/test/Preprocessor/predefined-arch-macros.c:1792
+// RUN: %clang -march=graniterapids -m32 -E -dM %s -o - 2>&1 \
+// RUN: -target i386-unknown-linux \
+// RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_GNR_M32

Prefer `--target=` to legacy `-target ` for new tests.

Consider refactoring the long `CHECK` lines to use delta encoding:

```
RUN: ... | FileCheck %s --check-prefixes=A
RUN: ... | FileCheck %s --check-prefixes=A,B
RUN: ... | FileCheck %s --check-prefixes=A,B,C
```

for better maintenance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137153

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

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D137181#3916558 , @goldstein.w.n 
wrote:

> In D137181#3916547 , @owenpan wrote:
>
>> Yes, if there is a header guard. Otherwise, set `PPLevel` to `PPBranchLevel 
>> + 1`.
>
> That fails alot of the tests for me.
>
> maybe you did something different?

Here is what I did:

  $ git diff UnwrappedLineParser.cpp
  diff --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
  index 25d9018fa109..ab3b9c53ee54 100644
  --- a/clang/lib/Format/UnwrappedLineParser.cpp
  +++ b/clang/lib/Format/UnwrappedLineParser.cpp
  @@ -197,6 +197,7 @@ public:
   PreBlockLine = std::move(Parser.Line);
   Parser.Line = std::make_unique();
   Parser.Line->Level = PreBlockLine->Level;
  +Parser.Line->PPLevel = PreBlockLine->PPLevel;
   Parser.Line->InPPDirective = PreBlockLine->InPPDirective;
   Parser.Line->InMacroBody = PreBlockLine->InMacroBody;
 }
  @@ -1274,6 +1275,10 @@ void UnwrappedLineParser::parsePPDefine() {
 addUnwrappedLine();
 ++Line->Level;
 Line->InMacroBody = true;
  +  if (Style.IndentPPDirectives != FormatStyle::PPDIS_None) {
  +Line->PPLevel =
  +IncludeGuard == IG_Defined ? PPBranchLevel : PPBranchLevel + 1;
  +  }
   
 // Errors during a preprocessor directive can only affect the layout of the
 // preprocessor directive, and thus we ignore them. An alternative approach


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181

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

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.h:66
+  /// #endifPPLevel still at : 0
+  int PPLevel;
+

goldstein.w.n wrote:
> owenpan wrote:
> > You need to initialize `PPLevel` in the constructor. FWIW I'd use 
> > `unsigned` to be consistent with `Level` above.
> Will initialize. It's `int` because we do a less than zero check in the 
> Formatter.
> It's `int` because we do a less than zero check in the Formatter.

If you set `PPLevel` to `PPBranchLevel` when there is no header guard, and to 
`PPBranchLevel + 1`otherwise, `PPLevel` can never be negative.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181

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

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

[Noah Goldstein via Phabricator via cfe-commits]

goldstein.w.n added a comment.

In D137181#3916547 , @owenpan wrote:

> In D137181#3916521 , @goldstein.w.n 
> wrote:
>
>> I think we can remove the places I set `InitialPPLevel` and `OldPPLevel` but 
>> anything else I remove causes at least one test to fail.
>>
>> What did you do? Just set `PPLevel = PPBranchLevel`?
>
> Yes, if there is a header guard. Otherwise, set `PPLevel` to `PPBranchLevel + 
> 1`.

That fails alot of the tests for me.

I have a patch attached (applyable on this) maybe you did something 
different?F25244239: v1-0001-Remove-all-other-defs.patch 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181

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

[PATCH] D134974: [OpenMP] Add map clause to the LIT test on use_device_addr clause

[Animesh Kumar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0f8e7b43291c: [OpenMP] Add map clause to the LIT test on 
use_device_addr clause (authored by animeshk-amd).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134974

Files:
  clang/test/OpenMP/target_data_use_device_addr_codegen.cpp

Index: clang/test/OpenMP/target_data_use_device_addr_codegen.cpp
===
--- clang/test/OpenMP/target_data_use_device_addr_codegen.cpp
+++ clang/test/OpenMP/target_data_use_device_addr_codegen.cpp
@@ -11,13 +11,13 @@
 #ifndef HEADER
 #define HEADER
 
-// CHECK-DAG: [[SIZES1:@.+]] = private unnamed_addr constant [5 x i64] zeroinitializer
+// CHECK-DAG: [[SIZES1:@.+]] = private unnamed_addr constant [6 x i64] [i64 4, i64 16, i64 4, i64 4, i64 0, i64 4]
 // 64 = 0x40 = OMP_MAP_RETURN_PARAM
-// CHECK-DAG: [[MAPTYPES1:@.+]] = private unnamed_addr constant [5 x i64] [i64 64, i64 64, i64 64, i64 64, i64 64]
-// CHECK-DAG: [[SIZES2:@.+]] = private unnamed_addr constant [5 x i64] zeroinitializer
+// CHECK-DAG: [[MAPTYPES1:@.+]] = private unnamed_addr constant [6 x i64] [i64 67, i64 67, i64 3, i64 67, i64 67, i64 67]
+// CHECK-DAG: [[SIZES2:@.+]] = private unnamed_addr constant [6 x i64] [i64 0, i64 4, i64 16, i64 4, i64 4, i64 0]
 // 0 = OMP_MAP_NONE
 // 281474976710720 = 0x10040 = OMP_MAP_MEMBER_OF | OMP_MAP_RETURN_PARAM
-// CHECK-DAG: [[MAPTYPES2:@.+]] = private unnamed_addr constant [5 x i64] [i64 0, i64 281474976710720, i64 281474976710720, i64 281474976710720, i64 281474976710720]
+// CHECK-DAG: [[MAPTYPES2:@.+]] = private unnamed_addr constant [6 x i64] [i64 0, i64 281474976710723, i64 281474976710739, i64 281474976710739, i64 281474976710675, i64 281474976710723]
 struct S {
   int a = 0;
   int *ptr = 
@@ -25,7 +25,7 @@
   int arr[4];
   S() {}
   void foo() {
-#pragma omp target data use_device_addr(a, ptr [3:4], ref, ptr[0], arr[:a])
+#pragma omp target data map(tofrom: a, ptr [3:4], ref, ptr[0], arr[:a]) use_device_addr(a, ptr [3:4], ref, ptr[0], arr[:a])
 ++a, ++*ptr, ++ref, ++arr[0];
   }
 };
@@ -38,7 +38,7 @@
   float vla[(int)a];
   S s;
   s.foo();
-#pragma omp target data use_device_addr(a, ptr [3:4], ref, ptr[0], arr[:(int)a], vla[0])
+#pragma omp target data map(tofrom: a, ptr [3:4], ref, ptr[0], arr[:(int)a], vla[0]) use_device_addr(a, ptr [3:4], ref, ptr[0], arr[:(int)a], vla[0])
   ++a, ++*ptr, ++ref, ++arr[0], ++vla[0];
   return a;
 }
@@ -48,47 +48,68 @@
 // CHECK: [[PTR_ADDR:%.+]] = alloca ptr,
 // CHECK: [[REF_ADDR:%.+]] = alloca ptr,
 // CHECK: [[ARR_ADDR:%.+]] = alloca [4 x float],
-// CHECK: [[BPTRS:%.+]] = alloca [5 x ptr],
-// CHECK: [[PTRS:%.+]] = alloca [5 x ptr],
+// CHECK: [[BPTRS:%.+]] = alloca [6 x ptr],
+// CHECK: [[PTRS:%.+]] = alloca [6 x ptr],
+// CHECK: [[MAP_PTRS:%.+]] = alloca [6 x ptr],
+// CHECK: [[SIZES:%.+]] = alloca [6 x i64],
 // CHECK: [[VLA_ADDR:%.+]] = alloca float, i64 %{{.+}},
 // CHECK: [[PTR:%.+]] = load ptr, ptr [[PTR_ADDR]],
-// CHECK: [[REF:%.+]] = load ptr, ptr [[REF_ADDR]],
-// CHECK: [[ARR:%.+]] = getelementptr inbounds [4 x float], ptr [[ARR_ADDR]], i64 0, i64 0
-// CHECK: [[BPTR0:%.+]] = getelementptr inbounds [5 x ptr], ptr [[BPTRS]], i32 0, i32 0
+// CHECK-NEXT: [[P4:%.+]] = load ptr, ptr [[PTR_ADDR]], align 8
+// CHECK-NEXT: [[ARR_IDX:%.+]] = getelementptr inbounds float, ptr [[P4]], i64 3
+// CHECK: [[P5:%.+]] = load ptr, ptr [[PTR_ADDR]], align 8
+// CHECK-NEXT: [[P6:%.+]] = load ptr, ptr [[PTR_ADDR]], align 8
+// CHECK-NEXT: [[ARR_IDX1:%.+]] = getelementptr inbounds float, ptr [[P6]], i64 0
+// CHECK: [[P7:%.+]] = load ptr, ptr [[REF_ADDR]],
+// CHECK-NEXT: [[REF:%.+]] = load ptr, ptr [[REF_ADDR]],
+// CHECK-NEXT: [[ARR_IDX2:%.+]] = getelementptr inbounds [4 x float], ptr [[ARR_ADDR]], i64 0, i64 0
+// CHECK: [[P10:%.+]] = mul nuw i64 {{.+}}, 4
+// CHECK-NEXT: [[ARR_IDX5:%.+]] = getelementptr inbounds float, ptr [[VLA_ADDR]], i64 0
+// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 8 [[SIZES]], ptr align 8 [[SIZES1]], i64 48, i1 false)
+// CHECK: [[BPTR0:%.+]] = getelementptr inbounds [6 x ptr], ptr [[BPTRS]], i32 0, i32 0
 // CHECK: store ptr [[A_ADDR]], ptr [[BPTR0]],
-// CHECK: [[PTR0:%.+]] = getelementptr inbounds [5 x ptr], ptr [[PTRS]], i32 0, i32 0
+// CHECK: [[PTR0:%.+]] = getelementptr inbounds [6 x ptr], ptr [[PTRS]], i32 0, i32 0
 // CHECK: store ptr [[A_ADDR]], ptr [[PTR0]],
-// CHECK: [[BPTR1:%.+]] = getelementptr inbounds [5 x ptr], ptr [[BPTRS]], i32 0, i32 1
+// CHECK: [[BPTR1:%.+]] = getelementptr inbounds [6 x ptr], ptr [[BPTRS]], i32 0, i32 1
 // CHECK: store ptr [[PTR]], ptr [[BPTR1]],
-// CHECK: [[PTR1:%.+]] = getelementptr inbounds [5 x ptr], ptr [[PTRS]], i32 0, i32 1
-// CHECK: store ptr [[PTR]], ptr [[PTR1]],
-// CHECK: [[BPTR2:%.+]] = getelementptr inbounds [5 x ptr], ptr [[BPTRS]], i32 0, i32 2
-// CHECK: store ptr [[REF]], ptr 

[clang] 0f8e7b4 - [OpenMP] Add map clause to the LIT test on use_device_addr clause

[Animesh Kumar via cfe-commits]

Author: Animesh Kumar
Date: 2022-11-09T12:23:39+05:30
New Revision: 0f8e7b43291ce5881cde6f3d34041c578390dda4

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

LOG: [OpenMP] Add map clause to the LIT test on use_device_addr clause

As per the OpenMP Spec, "A list item in a use_device_addr clause
must have a corresponding list item in the device data environment"
. Therefore a `map` clause is added which will make sure that the
respective list items are mapped to the device data environment
before the `use_device_addr` clause is specified. The CHECK lines
are also modified based on this change.

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

Added: 


Modified: 
clang/test/OpenMP/target_data_use_device_addr_codegen.cpp

Removed: 




diff  --git a/clang/test/OpenMP/target_data_use_device_addr_codegen.cpp 
b/clang/test/OpenMP/target_data_use_device_addr_codegen.cpp
index 5522dcbb54ce5..0b9bc0959f3f4 100644
--- a/clang/test/OpenMP/target_data_use_device_addr_codegen.cpp
+++ b/clang/test/OpenMP/target_data_use_device_addr_codegen.cpp
@@ -11,13 +11,13 @@
 #ifndef HEADER
 #define HEADER
 
-// CHECK-DAG: [[SIZES1:@.+]] = private unnamed_addr constant [5 x i64] 
zeroinitializer
+// CHECK-DAG: [[SIZES1:@.+]] = private unnamed_addr constant [6 x i64] [i64 4, 
i64 16, i64 4, i64 4, i64 0, i64 4]
 // 64 = 0x40 = OMP_MAP_RETURN_PARAM
-// CHECK-DAG: [[MAPTYPES1:@.+]] = private unnamed_addr constant [5 x i64] [i64 
64, i64 64, i64 64, i64 64, i64 64]
-// CHECK-DAG: [[SIZES2:@.+]] = private unnamed_addr constant [5 x i64] 
zeroinitializer
+// CHECK-DAG: [[MAPTYPES1:@.+]] = private unnamed_addr constant [6 x i64] [i64 
67, i64 67, i64 3, i64 67, i64 67, i64 67]
+// CHECK-DAG: [[SIZES2:@.+]] = private unnamed_addr constant [6 x i64] [i64 0, 
i64 4, i64 16, i64 4, i64 4, i64 0]
 // 0 = OMP_MAP_NONE
 // 281474976710720 = 0x10040 = OMP_MAP_MEMBER_OF | OMP_MAP_RETURN_PARAM
-// CHECK-DAG: [[MAPTYPES2:@.+]] = private unnamed_addr constant [5 x i64] [i64 
0, i64 281474976710720, i64 281474976710720, i64 281474976710720, i64 
281474976710720]
+// CHECK-DAG: [[MAPTYPES2:@.+]] = private unnamed_addr constant [6 x i64] [i64 
0, i64 281474976710723, i64 281474976710739, i64 281474976710739, i64 
281474976710675, i64 281474976710723]
 struct S {
   int a = 0;
   int *ptr = 
@@ -25,7 +25,7 @@ struct S {
   int arr[4];
   S() {}
   void foo() {
-#pragma omp target data use_device_addr(a, ptr [3:4], ref, ptr[0], arr[:a])
+#pragma omp target data map(tofrom: a, ptr [3:4], ref, ptr[0], arr[:a]) 
use_device_addr(a, ptr [3:4], ref, ptr[0], arr[:a])
 ++a, ++*ptr, ++ref, ++arr[0];
   }
 };
@@ -38,7 +38,7 @@ int main() {
   float vla[(int)a];
   S s;
   s.foo();
-#pragma omp target data use_device_addr(a, ptr [3:4], ref, ptr[0], 
arr[:(int)a], vla[0])
+#pragma omp target data map(tofrom: a, ptr [3:4], ref, ptr[0], arr[:(int)a], 
vla[0]) use_device_addr(a, ptr [3:4], ref, ptr[0], arr[:(int)a], vla[0])
   ++a, ++*ptr, ++ref, ++arr[0], ++vla[0];
   return a;
 }
@@ -48,47 +48,68 @@ int main() {
 // CHECK: [[PTR_ADDR:%.+]] = alloca ptr,
 // CHECK: [[REF_ADDR:%.+]] = alloca ptr,
 // CHECK: [[ARR_ADDR:%.+]] = alloca [4 x float],
-// CHECK: [[BPTRS:%.+]] = alloca [5 x ptr],
-// CHECK: [[PTRS:%.+]] = alloca [5 x ptr],
+// CHECK: [[BPTRS:%.+]] = alloca [6 x ptr],
+// CHECK: [[PTRS:%.+]] = alloca [6 x ptr],
+// CHECK: [[MAP_PTRS:%.+]] = alloca [6 x ptr],
+// CHECK: [[SIZES:%.+]] = alloca [6 x i64],
 // CHECK: [[VLA_ADDR:%.+]] = alloca float, i64 %{{.+}},
 // CHECK: [[PTR:%.+]] = load ptr, ptr [[PTR_ADDR]],
-// CHECK: [[REF:%.+]] = load ptr, ptr [[REF_ADDR]],
-// CHECK: [[ARR:%.+]] = getelementptr inbounds [4 x float], ptr [[ARR_ADDR]], 
i64 0, i64 0
-// CHECK: [[BPTR0:%.+]] = getelementptr inbounds [5 x ptr], ptr [[BPTRS]], i32 
0, i32 0
+// CHECK-NEXT: [[P4:%.+]] = load ptr, ptr [[PTR_ADDR]], align 8
+// CHECK-NEXT: [[ARR_IDX:%.+]] = getelementptr inbounds float, ptr [[P4]], i64 
3
+// CHECK: [[P5:%.+]] = load ptr, ptr [[PTR_ADDR]], align 8
+// CHECK-NEXT: [[P6:%.+]] = load ptr, ptr [[PTR_ADDR]], align 8
+// CHECK-NEXT: [[ARR_IDX1:%.+]] = getelementptr inbounds float, ptr [[P6]], 
i64 0
+// CHECK: [[P7:%.+]] = load ptr, ptr [[REF_ADDR]],
+// CHECK-NEXT: [[REF:%.+]] = load ptr, ptr [[REF_ADDR]],
+// CHECK-NEXT: [[ARR_IDX2:%.+]] = getelementptr inbounds [4 x float], ptr 
[[ARR_ADDR]], i64 0, i64 0
+// CHECK: [[P10:%.+]] = mul nuw i64 {{.+}}, 4
+// CHECK-NEXT: [[ARR_IDX5:%.+]] = getelementptr inbounds float, ptr 
[[VLA_ADDR]], i64 0
+// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 8 [[SIZES]], ptr 
align 8 [[SIZES1]], i64 48, i1 false)
+// CHECK: [[BPTR0:%.+]] = getelementptr inbounds [6 x ptr], ptr [[BPTRS]], i32 
0, i32 0
 // CHECK: store ptr [[A_ADDR]], ptr [[BPTR0]],
-// CHECK: [[PTR0:%.+]] = 

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:112
   : Line(Line), TokenSource(TokenSource), ResetToken(ResetToken),
-PreviousLineLevel(Line.Level), PreviousTokenSource(TokenSource),
-Token(nullptr), PreviousToken(nullptr) {
+PreviousLineLevel(Line.Level), PreviousLinePPLevel(Line.PPLevel),
+PreviousTokenSource(TokenSource), Token(nullptr),

goldstein.w.n wrote:
> owenpan wrote:
> > Do you need to add `PreviousLinePPLevel` here? If yes, can you add test 
> > cases for it? Otherwise, you don't need to make any changes to 
> > `ScopedMacroState`.
> It's needed.
> 
> Without:
> ```
> Expected equality of these values:
>   Expected.str()
> Which is: "#ifndef foo\n#define foo\nif (emacs) {\n#ifdef is\n #define 
> lit   \\\n if (af) { \\\n return duh(); \\\n 
> }\n#endif\n}\n#endif"
>   format(test::messUp(Code), ObjCStyle)
> Which is: "#ifndef foo\n#define foo\nif (emacs) {\n#ifdef is\n #define 
> lit\\\n  if (af) { \\\n  return duh(); \\\n  
> }\n#endif\n}\n#endif"
> With diff:
> @@ -3,8 +3,8 @@
>  if (emacs) {
>  #ifdef is
> - #define lit   \\
> - if (af) { \\
> - return duh(); \\
> - }
> + #define lit\\
> +  if (af) { \\
> +  return duh(); \\
> +  }
>  #endif
>  }
> ```
I don't have to touch `ScopedMacroState`. Perhaps it has something to do with 
the way you set/adjust/use `PPLevel`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181

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

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D137181#3916521 , @goldstein.w.n 
wrote:

> I think we can remove the places I set `InitialPPLevel` and `OldPPLevel` but 
> anything else I remove causes at least one test to fail.
>
> What did you do? Just set `PPLevel = PPBranchLevel`?

Yes, if there is a header guard. Otherwise, set `PPLevel` to `PPBranchLevel + 
1`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181

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

[PATCH] D137153: [X86] Support -march=sierraforest, grandridge, graniterapids.

[Phoebe Wang via Phabricator via cfe-commits]

pengfei accepted this revision.
pengfei 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/D137153/new/

https://reviews.llvm.org/D137153

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

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

[Noah Goldstein via Phabricator via cfe-commits]

goldstein.w.n added a comment.

In D137181#3916505 , @owenpan wrote:

> In D137181#3916488 , @goldstein.w.n 
> wrote:
>
>> In D137181#3916474 , @owenpan 
>> wrote:
>>
>>> You can still simply the changes to UnwrappedLineParser.cpp a lot. In fact, 
>>> instead of adjusting `PPLevel` at various places, you only need to set it 
>>> once when `InMacroBody` is set.
>>
>> I don't think thats right, it needs to stay updated with the true macro 
>> depth (even though we only use `PPLevel` for formatting `InMacroBody` lines).
>
> My experiment from a few days ago was successful and also passes all the new 
> tests you just added here. If your experience told you otherwise, then 
> perhaps there are some missing test cases.

What did you do? Just set `PPLevel = PPBranchLevel`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181

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

[PATCH] D137669: clang/cmake: Require pre-built test dependencies for stand-alone builds

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

I don't have a strong opinion. I'm happy as long as there is *anything* telling 
me "tests were disabled because ...".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137669

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

[PATCH] D137153: [X86] Support -march=sierraforest, grandridge, graniterapids.

[Freddy, Ye via Phabricator via cfe-commits]

FreddyYe added inline comments.



Comment at: llvm/include/llvm/Support/X86TargetParser.def:94
+X86_CPU_SUBTYPE(AMDFAM19H_ZNVER4,"znver4")
+X86_CPU_SUBTYPE(INTEL_COREI7_SIERRAFOREST,   "sierraforest")
+X86_CPU_SUBTYPE(INTEL_COREI7_GRANITERAPIDS,  "graniterapids")

pengfei wrote:
> FreddyYe wrote:
> > FreddyYe wrote:
> > > FYI: gcc is going to land the [[ 
> > > https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603547.html | 
> > > sierraforest patch ]]. They are going to add such E-core based CPUs to 
> > > CPU_TYPE list but not this one, like the previous atom series e.g. 
> > > tremont, silvermont and so on. It's functionally correct, so I'm ok to 
> > > this change. WDYT
> > > FYI: gcc is going to land the [[ 
> > > https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603547.html | 
> > > sierraforest patch ]]. They are going to add such E-core based CPUs to 
> > > CPU_TYPE list but not this one, like the previous atom series e.g. 
> > > tremont, silvermont and so on. It's functionally correct, so I'm ok to 
> > > this change. WDYT
> > 
> > ping.
> Is this to match with GCC?
Yes.



Comment at: llvm/lib/Target/X86/X86.td:949
+  FeatureAMXINT8,
+  FeatureAMXBF16,
+  FeatureBF16];

skan wrote:
> Why is this differnt from FeaturesGraniteRapids in X86TargetParser.cpp?
Good catch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137153

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

[PATCH] D137153: [X86] Support -march=sierraforest, grandridge, graniterapids.

[Freddy, Ye via Phabricator via cfe-commits]

FreddyYe updated this revision to Diff 474155.
FreddyYe marked an inline comment as done.
FreddyYe added a comment.

Address comments. THX fore review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137153

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Basic/Targets/X86.cpp
  clang/test/CodeGen/attr-target-mv.c
  clang/test/CodeGen/target-builtin-noerror.c
  clang/test/Driver/x86-march.c
  clang/test/Misc/target-invalid-cpu-note.c
  clang/test/Preprocessor/predefined-arch-macros.c
  compiler-rt/lib/builtins/cpu_model.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/Support/X86TargetParser.def
  llvm/include/llvm/Support/X86TargetParser.h
  llvm/lib/Support/Host.cpp
  llvm/lib/Support/X86TargetParser.cpp
  llvm/lib/Target/X86/X86.td
  llvm/test/CodeGen/X86/cpus-intel.ll

Index: llvm/test/CodeGen/X86/cpus-intel.ll
===
--- llvm/test/CodeGen/X86/cpus-intel.ll
+++ llvm/test/CodeGen/X86/cpus-intel.ll
@@ -19,6 +19,9 @@
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=lakemont 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=raptorlake 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=meteorlake 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=sierraforest 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=grandridge 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=graniterapids 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=nocona 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=core2 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
@@ -56,6 +59,9 @@
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=knm 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=raptorlake 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=meteorlake 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=sierraforest 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=grandridge 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=graniterapids 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 
 define void @foo() {
   ret void
Index: llvm/lib/Target/X86/X86.td
===
--- llvm/lib/Target/X86/X86.td
+++ llvm/lib/Target/X86/X86.td
@@ -943,6 +943,12 @@
   list SPRFeatures =
 !listconcat(ICXFeatures, SPRAdditionalFeatures);
 
+  // Graniterapids
+  list GNRAdditionalFeatures = [FeatureAMXFP16,
+  FeaturePREFETCHI];
+  list GNRFeatures =
+!listconcat(SPRFeatures, GNRAdditionalFeatures);
+
   // Atom
   list AtomFeatures = [FeatureX87,
  FeatureCX8,
@@ -1050,6 +1056,19 @@
   list ADLFeatures =
 !listconcat(TRMFeatures, ADLAdditionalFeatures);
 
+  // Sierraforest
+  list SRFAdditionalFeatures = [FeatureCMPCCXADD,
+  FeatureAVXIFMA,
+  FeatureAVXNECONVERT,
+  FeatureAVXVNNIINT8];
+  list SRFFeatures =
+!listconcat(ADLFeatures, SRFAdditionalFeatures);
+
+  // Grandridge
+  list GRRAdditionalFeatures = [FeatureRAOINT];
+  list GRRFeatures =
+!listconcat(SRFFeatures, GRRAdditionalFeatures);
+
   // Knights Landing
   list KNLFeatures = [FeatureX87,
 FeatureCX8,
@@ -1441,6 +1460,10 @@
 ProcessorFeatures.GLPTuning>;
 def : ProcModel<"tremont", SLMModel, ProcessorFeatures.TRMFeatures,
 ProcessorFeatures.TRMTuning>;
+def : ProcModel<"sierraforest", AlderlakePModel, ProcessorFeatures.SRFFeatures,
+ProcessorFeatures.TRMTuning>;
+def : ProcModel<"grandridge", AlderlakePModel, ProcessorFeatures.GRRFeatures,
+ProcessorFeatures.TRMTuning>;
 
 // "Arrandale" along with corei3 and corei5
 foreach P = ["nehalem", "corei7"] in {
@@ -1506,6 +1529,8 @@
 

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D137181#3916488 , @goldstein.w.n 
wrote:

> In D137181#3916474 , @owenpan wrote:
>
>> You can still simply the changes to UnwrappedLineParser.cpp a lot. In fact, 
>> instead of adjusting `PPLevel` at various places, you only need to set it 
>> once when `InMacroBody` is set.
>
> I don't think thats right, it needs to stay updated with the true macro depth 
> (even though we only use `PPLevel` for formatting `InMacroBody` lines).

My experiment from a few days ago was successful and also passes all the new 
tests you just added here. If your experience told you otherwise, then perhaps 
there are some missing test cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181

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

[PATCH] D137669: clang/cmake: Require pre-built test dependencies for stand-alone builds

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

In D137669#3916493 , @mgorny wrote:

> Could you issue a warning though? It's really annoying when targets disappear 
> like that and you have to guess which of the checks failed.

What about just making it an error if LLVM_INCLUDE_TESTS is set, but it kind 
find all the tools?  I also don't like the silent disabling of features, but 
I'm not sure how visible the warning would be to users.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137669

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

[libunwind] 4db6871 - [libunwind] Check corrupted return address in unwind_phase2 when CET is enabled.

[via cfe-commits]

Author: jinge90
Date: 2022-11-09T14:27:07+08:00
New Revision: 4db687155bc12f31b5ed122ba1086c5f04838a24

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

LOG: [libunwind] Check corrupted return address in unwind_phase2 when CET is 
enabled.

If CET shadow stack is enabled, we count the number of stack frames skipped
and adjust CET shadow stack based on the number in libunwind unwind_phase2.
At the same time, we can enhance security via comparing the return address in
normal stack against counterpart in CET shadow stack, if they don't match,
it means the return address stored in normal stack has been corrupted and we
will return _URC_FATAL_PHASE2_ERROR in that case.

Reviewed By: MaskRay
Differential Revision: https://reviews.llvm.org/D136667

Signed-off-by: jinge90 

Added: 


Modified: 
libunwind/src/UnwindLevel1.c

Removed: 




diff  --git a/libunwind/src/UnwindLevel1.c b/libunwind/src/UnwindLevel1.c
index 7160680467e42..f984a8bb6adec 100644
--- a/libunwind/src/UnwindLevel1.c
+++ b/libunwind/src/UnwindLevel1.c
@@ -50,6 +50,7 @@
 __unw_resume((cursor));
\
   } while (0)
 #elif defined(_LIBUNWIND_TARGET_I386)
+#define __cet_ss_step_size 4
 #define __unw_phase2_resume(cursor, fn)
\
   do { 
\
 _LIBUNWIND_POP_CET_SSP((fn));  
\
@@ -61,6 +62,7 @@
  "d"(cetJumpAddress)); 
\
   } while (0)
 #elif defined(_LIBUNWIND_TARGET_X86_64)
+#define __cet_ss_step_size 8
 #define __unw_phase2_resume(cursor, fn)
\
   do { 
\
 _LIBUNWIND_POP_CET_SSP((fn));  
\
@@ -177,6 +179,9 @@ unwind_phase2(unw_context_t *uc, unw_cursor_t *cursor, 
_Unwind_Exception *except
   // uc is initialized by __unw_getcontext in the parent frame. The first stack
   // frame walked is unwind_phase2.
   unsigned framesWalked = 1;
+#ifdef _LIBUNWIND_USE_CET
+  unsigned long shadowStackTop = _get_ssp();
+#endif
   // Walk each frame until we reach where search phase said to stop.
   while (true) {
 
@@ -228,6 +233,20 @@ unwind_phase2(unw_context_t *uc, unw_cursor_t *cursor, 
_Unwind_Exception *except
 }
 #endif
 
+// In CET enabled environment, we check return address stored in normal stack
+// against return address stored in CET shadow stack, if the 2 addresses don't
+// match, it means return address in normal stack has been corrupted, we return
+// _URC_FATAL_PHASE2_ERROR.
+#ifdef _LIBUNWIND_USE_CET
+if (shadowStackTop != 0) {
+  unw_word_t retInNormalStack;
+  __unw_get_reg(cursor, UNW_REG_IP, );
+  unsigned long retInShadowStack = *(
+  unsigned long *)(shadowStackTop + __cet_ss_step_size * framesWalked);
+  if (retInNormalStack != retInShadowStack)
+return _URC_FATAL_PHASE2_ERROR;
+}
+#endif
 ++framesWalked;
 // If there is a personality routine, tell it we are unwinding.
 if (frameInfo.handler != 0) {



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

[PATCH] D137153: [X86] Support -march=sierraforest, grandridge, graniterapids.

[Kan Shengchen via Phabricator via cfe-commits]

skan added inline comments.



Comment at: llvm/lib/Target/X86/X86.td:949
+  FeatureAMXINT8,
+  FeatureAMXBF16,
+  FeatureBF16];

Why is this differnt from FeaturesGraniteRapids in X86TargetParser.cpp?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137153

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

[PATCH] D137669: clang/cmake: Require pre-built test dependencies for stand-alone builds

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

Could you issue a warning though? It's really annoying when targets disappear 
like that and you have to guess which of the checks failed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137669

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

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

[Noah Goldstein via Phabricator via cfe-commits]

goldstein.w.n added a comment.

In D137181#3916474 , @owenpan wrote:

> You can still simply the changes to UnwrappedLineParser.cpp a lot. In fact, 
> instead of adjusting `PPLevel` at various places, you only need to set it 
> once when `InMacroBody` is set.

I don't think thats right, it needs to stay updated with the true macro depth 
(even though we only use `PPLevel` for formatting `InMacroBody` lines).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181

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

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

[Noah Goldstein via Phabricator via cfe-commits]

goldstein.w.n added inline comments.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:92-94
+  if (Line.Level >= IndentForLevel.size())
+IndentForLevel.resize(Line.Level + 1, UnknownIndent ? -1 : Indent);
+}

owenpan wrote:
> Please run git-clang-format.
Will do for next submissions (thought `arc` did that automatically on 
precommit).



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:112
   : Line(Line), TokenSource(TokenSource), ResetToken(ResetToken),
-PreviousLineLevel(Line.Level), PreviousTokenSource(TokenSource),
-Token(nullptr), PreviousToken(nullptr) {
+PreviousLineLevel(Line.Level), PreviousLinePPLevel(Line.PPLevel),
+PreviousTokenSource(TokenSource), Token(nullptr),

owenpan wrote:
> Do you need to add `PreviousLinePPLevel` here? If yes, can you add test cases 
> for it? Otherwise, you don't need to make any changes to `ScopedMacroState`.
It's needed.

Without:
```
Expected equality of these values:
  Expected.str()
Which is: "#ifndef foo\n#define foo\nif (emacs) {\n#ifdef is\n #define lit  
 \\\n if (af) { \\\n return duh(); \\\n 
}\n#endif\n}\n#endif"
  format(test::messUp(Code), ObjCStyle)
Which is: "#ifndef foo\n#define foo\nif (emacs) {\n#ifdef is\n #define lit  
  \\\n  if (af) { \\\n  return duh(); \\\n  
}\n#endif\n}\n#endif"
With diff:
@@ -3,8 +3,8 @@
 if (emacs) {
 #ifdef is
- #define lit   \\
- if (af) { \\
- return duh(); \\
- }
+ #define lit\\
+  if (af) { \\
+  return duh(); \\
+  }
 #endif
 }
```



Comment at: clang/lib/Format/UnwrappedLineParser.h:66
+  /// #endifPPLevel still at : 0
+  int PPLevel;
+

owenpan wrote:
> You need to initialize `PPLevel` in the constructor. FWIW I'd use `unsigned` 
> to be consistent with `Level` above.
Will initialize. It's `int` because we do a less than zero check in the 
Formatter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181

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

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D91950#3915777 , 
@HazardyKnusperkeks wrote:

> @MyDeveloperDay @owenpan What shall we do?
>
> 1. Abandon this
> 2. Commit it in our name
> 3. Use the email from this commit: 
> https://github.com/Wandalen/game_chess/commit/a84bba03667643ded0cfe7aff2907c6546a6a192
>  I'm pretty sure this is the correct person

IMO we should either commit it with her name only or also include the email you 
found.


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

https://reviews.llvm.org/D91950

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

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

You can still simply the changes to UnwrappedLineParser.cpp a lot. In fact, 
instead of adjusting `PPLevel` at various places, you only need to set it once 
when `InMacroBody` is set.




Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:92-94
+  if (Line.Level >= IndentForLevel.size())
+IndentForLevel.resize(Line.Level + 1, UnknownIndent ? -1 : Indent);
+}

Please run git-clang-format.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:112
   : Line(Line), TokenSource(TokenSource), ResetToken(ResetToken),
-PreviousLineLevel(Line.Level), PreviousTokenSource(TokenSource),
-Token(nullptr), PreviousToken(nullptr) {
+PreviousLineLevel(Line.Level), PreviousLinePPLevel(Line.PPLevel),
+PreviousTokenSource(TokenSource), Token(nullptr),

Do you need to add `PreviousLinePPLevel` here? If yes, can you add test cases 
for it? Otherwise, you don't need to make any changes to `ScopedMacroState`.



Comment at: clang/lib/Format/UnwrappedLineParser.h:66
+  /// #endifPPLevel still at : 0
+  int PPLevel;
+

You need to initialize `PPLevel` in the constructor. FWIW I'd use `unsigned` to 
be consistent with `Level` above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181

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

[PATCH] D137650: [clangd] Implement hover for C++ string literals

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added a comment.

Seems like I have made things too complex :(

Should work for C, C++, and other languages now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137650

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

[PATCH] D137650: [clangd] Implement hover for C++ string literals

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 474149.
v1nh1shungry added a comment.

Improve the patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137650

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1300,7 +1300,6 @@
   "auto x = ^42.0i;",
   "auto x = ^42;",
   "auto x = ^nullptr;",
-  "auto x = ^\"asdf\";",
   };
 
   for (const auto  : Tests) {
@@ -1326,6 +1325,11 @@
  HI.Type = "char";
  HI.Value = "65 (0x41)";
}},
+  {"auto s = ^[[\"Hello, world!\"]]; // string literal",
+   [](HoverInfo ) {
+ HI.Name = "String Literal";
+ HI.Type = "const char[14]";
+   }},
   {
   R"cpp(// Local variable
 int main() {
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -804,12 +804,23 @@
 llvm::Optional getHoverContents(const Expr *E, ParsedAST ,
const PrintingPolicy ,
const SymbolIndex *Index) {
+  HoverInfo HI;
+
   // There's not much value in hovering over "42" and getting a hover card
   // saying "42 is an int", similar for other literals.
-  if (isLiteral(E))
+  if (isLiteral(E)) {
+// Generate hover info for string literals showing
+// its character's type and length
+if (const StringLiteral *SL = dyn_cast(E)) {
+  HoverInfo::PrintedType PT;
+  PT.Type = SL->getType().getAsString(PP);
+  HI.Type = PT;
+  HI.Name = "String Literal";
+  return HI;
+}
 return llvm::None;
+  }
 
-  HoverInfo HI;
   // For `this` expr we currently generate hover with pointee type.
   if (const CXXThisExpr *CTE = dyn_cast(E))
 return getThisExprHoverContents(CTE, AST.getASTContext(), PP);


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1300,7 +1300,6 @@
   "auto x = ^42.0i;",
   "auto x = ^42;",
   "auto x = ^nullptr;",
-  "auto x = ^\"asdf\";",
   };
 
   for (const auto  : Tests) {
@@ -1326,6 +1325,11 @@
  HI.Type = "char";
  HI.Value = "65 (0x41)";
}},
+  {"auto s = ^[[\"Hello, world!\"]]; // string literal",
+   [](HoverInfo ) {
+ HI.Name = "String Literal";
+ HI.Type = "const char[14]";
+   }},
   {
   R"cpp(// Local variable
 int main() {
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -804,12 +804,23 @@
 llvm::Optional getHoverContents(const Expr *E, ParsedAST ,
const PrintingPolicy ,
const SymbolIndex *Index) {
+  HoverInfo HI;
+
   // There's not much value in hovering over "42" and getting a hover card
   // saying "42 is an int", similar for other literals.
-  if (isLiteral(E))
+  if (isLiteral(E)) {
+// Generate hover info for string literals showing
+// its character's type and length
+if (const StringLiteral *SL = dyn_cast(E)) {
+  HoverInfo::PrintedType PT;
+  PT.Type = SL->getType().getAsString(PP);
+  HI.Type = PT;
+  HI.Name = "String Literal";
+  return HI;
+}
 return llvm::None;
+  }
 
-  HoverInfo HI;
   // For `this` expr we currently generate hover with pointee type.
   if (const CXXThisExpr *CTE = dyn_cast(E))
 return getThisExprHoverContents(CTE, AST.getASTContext(), PP);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137650: [clangd] Implement hover for C++ string literals

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 474147.
v1nh1shungry added a comment.

Support different C++ standards


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137650

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1300,7 +1300,6 @@
   "auto x = ^42.0i;",
   "auto x = ^42;",
   "auto x = ^nullptr;",
-  "auto x = ^\"asdf\";",
   };
 
   for (const auto  : Tests) {
@@ -1326,6 +1325,11 @@
  HI.Type = "char";
  HI.Value = "65 (0x41)";
}},
+  {"auto s = ^[[\"Hello, world!\"]]; // string literal",
+   [](HoverInfo ) {
+ HI.Name = "String Literal";
+ HI.Type = "const char[14]";
+   }},
   {
   R"cpp(// Local variable
 int main() {
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -777,6 +777,80 @@
   return HI;
 }
 
+llvm::Optional
+getStringLiteralHoverContents(const StringLiteral *SL,
+  const LangOptions ) {
+  HoverInfo HI;
+
+  HI.Name = "String Literal";
+
+  // In C++ string literals have `const` qualifier, but in C they don't.
+  std::string Qualifier;
+
+  std::string CharType;
+  // C++
+  if (LangOpts.CPlusPlus) {
+Qualifier = "const";
+if (LangOpts.CPlusPlus20 || LangOpts.CPlusPlus2b) {
+  switch (SL->getKind()) {
+  case StringLiteral::Ordinary:
+CharType = "char";
+break;
+  case StringLiteral::Wide:
+CharType = "wchar_t";
+break;
+  case StringLiteral::UTF8:
+CharType = "char8_t";
+break;
+  case StringLiteral::UTF16:
+CharType = "char16_t";
+break;
+  case StringLiteral::UTF32:
+CharType = "char32_t";
+break;
+  }
+} else if (LangOpts.CPlusPlus11 || LangOpts.CPlusPlus14 ||
+   LangOpts.CPlusPlus17) {
+  switch (SL->getKind()) {
+  case StringLiteral::Ordinary:
+  case StringLiteral::UTF8:
+CharType = "char";
+break;
+  case StringLiteral::Wide:
+CharType = "wchar_t";
+break;
+  case StringLiteral::UTF16:
+CharType = "char16_t";
+break;
+  case StringLiteral::UTF32:
+CharType = "char32_t";
+break;
+  }
+} else {
+  switch (SL->getKind()) {
+  case StringLiteral::Ordinary:
+CharType = "char";
+break;
+  case StringLiteral::Wide:
+CharType = "wchar_t";
+break;
+  default:
+llvm_unreachable("Unsupported until C++11");
+  }
+}
+  } else {
+// TODO: For other languages
+return llvm::None;
+  }
+
+  HoverInfo::PrintedType Type;
+  Type.Type =
+  llvm::formatv("{0} {1}[{2}]", Qualifier, CharType, SL->getLength() + 1);
+  HI.Type = Type;
+
+  return HI;
+}
+
 bool isLiteral(const Expr *E) {
   // Unfortunately there's no common base Literal classes inherits from
   // (apart from Expr), therefore these exclusions.
@@ -806,8 +880,14 @@
const SymbolIndex *Index) {
   // There's not much value in hovering over "42" and getting a hover card
   // saying "42 is an int", similar for other literals.
-  if (isLiteral(E))
+  if (isLiteral(E)) {
+// Generate hover info for string literals showing
+// its character's type and length
+if (const StringLiteral *SL = dyn_cast(E)) {
+  return getStringLiteralHoverContents(SL, AST.getLangOpts());
+}
 return llvm::None;
+  }
 
   HoverInfo HI;
   // For `this` expr we currently generate hover with pointee type.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137153: [X86] Support -march=sierraforest, grandridge, graniterapids.

[Freddy, Ye via Phabricator via cfe-commits]

FreddyYe updated this revision to Diff 474146.
FreddyYe marked 2 inline comments as done.
FreddyYe added a comment.

Address comments. THX for review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137153

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Basic/Targets/X86.cpp
  clang/test/CodeGen/attr-target-mv.c
  clang/test/CodeGen/target-builtin-noerror.c
  clang/test/Driver/x86-march.c
  clang/test/Misc/target-invalid-cpu-note.c
  clang/test/Preprocessor/predefined-arch-macros.c
  compiler-rt/lib/builtins/cpu_model.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/Support/X86TargetParser.def
  llvm/include/llvm/Support/X86TargetParser.h
  llvm/lib/Support/Host.cpp
  llvm/lib/Support/X86TargetParser.cpp
  llvm/lib/Target/X86/X86.td
  llvm/test/CodeGen/X86/cpus-intel.ll

Index: llvm/test/CodeGen/X86/cpus-intel.ll
===
--- llvm/test/CodeGen/X86/cpus-intel.ll
+++ llvm/test/CodeGen/X86/cpus-intel.ll
@@ -19,6 +19,9 @@
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=lakemont 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=raptorlake 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=meteorlake 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=sierraforest 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=grandridge 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=graniterapids 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=nocona 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=core2 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
@@ -56,6 +59,9 @@
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=knm 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=raptorlake 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=meteorlake 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=sierraforest 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=grandridge 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=graniterapids 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 
 define void @foo() {
   ret void
Index: llvm/lib/Target/X86/X86.td
===
--- llvm/lib/Target/X86/X86.td
+++ llvm/lib/Target/X86/X86.td
@@ -943,6 +943,14 @@
   list SPRFeatures =
 !listconcat(ICXFeatures, SPRAdditionalFeatures);
 
+  // Graniterapids
+  list GNRAdditionalFeatures = [FeatureAMXTILE,
+  FeatureAMXINT8,
+  FeatureAMXBF16,
+  FeatureBF16];
+  list GNRFeatures =
+!listconcat(SPRFeatures, GNRAdditionalFeatures);
+
   // Atom
   list AtomFeatures = [FeatureX87,
  FeatureCX8,
@@ -1050,6 +1058,19 @@
   list ADLFeatures =
 !listconcat(TRMFeatures, ADLAdditionalFeatures);
 
+  // Sierraforest
+  list SRFAdditionalFeatures = [FeatureCMPCCXADD,
+  FeatureAVXIFMA,
+  FeatureAVXNECONVERT,
+  FeatureAVXVNNIINT8];
+  list SRFFeatures =
+!listconcat(ADLFeatures, SRFAdditionalFeatures);
+
+  // Grandridge
+  list GRRAdditionalFeatures = [FeatureRAOINT];
+  list GRRFeatures =
+!listconcat(SRFFeatures, GRRAdditionalFeatures);
+
   // Knights Landing
   list KNLFeatures = [FeatureX87,
 FeatureCX8,
@@ -1441,6 +1462,10 @@
 ProcessorFeatures.GLPTuning>;
 def : ProcModel<"tremont", SLMModel, ProcessorFeatures.TRMFeatures,
 ProcessorFeatures.TRMTuning>;
+def : ProcModel<"sierraforest", AlderlakePModel, ProcessorFeatures.SRFFeatures,
+ProcessorFeatures.TRMTuning>;
+def : ProcModel<"grandridge", AlderlakePModel, ProcessorFeatures.GRRFeatures,
+ProcessorFeatures.TRMTuning>;
 
 // 

[PATCH] D137153: [X86] Support -march=sierraforest, grandridge, graniterapids.

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: llvm/include/llvm/Support/X86TargetParser.def:94
+X86_CPU_SUBTYPE(AMDFAM19H_ZNVER4,"znver4")
+X86_CPU_SUBTYPE(INTEL_COREI7_SIERRAFOREST,   "sierraforest")
+X86_CPU_SUBTYPE(INTEL_COREI7_GRANITERAPIDS,  "graniterapids")

FreddyYe wrote:
> FreddyYe wrote:
> > FYI: gcc is going to land the [[ 
> > https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603547.html | 
> > sierraforest patch ]]. They are going to add such E-core based CPUs to 
> > CPU_TYPE list but not this one, like the previous atom series e.g. tremont, 
> > silvermont and so on. It's functionally correct, so I'm ok to this change. 
> > WDYT
> > FYI: gcc is going to land the [[ 
> > https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603547.html | 
> > sierraforest patch ]]. They are going to add such E-core based CPUs to 
> > CPU_TYPE list but not this one, like the previous atom series e.g. tremont, 
> > silvermont and so on. It's functionally correct, so I'm ok to this change. 
> > WDYT
> 
> ping.
Is this to match with GCC?



Comment at: llvm/lib/Support/X86TargetParser.cpp:389
+  // Graniterapids microarchitecture based processors.
+  { {"graniterapids"}, CK_Graniterapids, FEATURE_AVX512VP2INTERSECT, 
FeaturesGraniteRapids },
   // Knights Landing processor.

Is `VP2INTERSECT` supported by `graniterapids`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137153

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

[PATCH] D137618: [Clang] Fix behavior of -ffp-model option when overriden

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf updated this revision to Diff 474142.
qiucf marked an inline comment as done.
qiucf added a reviewer: michele.scandale.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137618

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/ffp-model.c
  clang/test/Driver/fp-model.c


Index: clang/test/Driver/fp-model.c
===
--- clang/test/Driver/fp-model.c
+++ clang/test/Driver/fp-model.c
@@ -66,6 +66,11 @@
 // RUN:   | FileCheck --check-prefix=WARN10 %s
 // WARN10: warning: overriding '-ffp-model=strict' option with 
'-fdenormal-fp-math=preserve-sign,preserve-sign' [-Woverriding-t-option]
 
+// RUN: %clang -### -ffp-model=fast -ffp-model=strict -c %s 2>&1 | FileCheck \
+// RUN:   --check-prefix=WARN11 %s
+// WARN11: warning: overriding '-ffp-model=fast' option with 
'-ffp-model=strict' [-Woverriding-t-option]
+// WARN11-NOT: warning: overriding '-ffp-model=strict' option with 
'-ffp-model=strict' [-Woverriding-t-option]
+
 // RUN: %clang -### -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NOROUND %s
 // CHECK-NOROUND: "-cc1"
@@ -107,6 +112,13 @@
 // CHECK-FPM-STRICT: "-frounding-math"
 // CHECK-FPM-STRICT: "-ffp-exception-behavior=strict"
 
+// RUN: %clang -### -nostdinc -ffp-model=strict -ffp-model=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NO-EXCEPT %s
+// RUN: %clang -### -nostdinc -ffp-model=strict -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NO-EXCEPT %s
+// RUN: %clang -### -nostdinc -ffp-model=strict -Ofast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NO-EXCEPT %s
+// CHECK-NO-EXCEPT-NOT: "-ffp-exception-behavior=strict"
 
 // RUN: %clang -### -nostdinc -ffp-exception-behavior=strict -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FEB-STRICT %s
Index: clang/test/CodeGen/ffp-model.c
===
--- clang/test/CodeGen/ffp-model.c
+++ clang/test/CodeGen/ffp-model.c
@@ -36,9 +36,9 @@
 
   // CHECK-STRICT-FAST: load float, ptr
   // CHECK-STRICT-FAST: load float, ptr
-  // CHECK-STRICT-FAST: call fast float 
@llvm.experimental.constrained.fmul.f32(float {{.*}}, float {{.*}}, {{.*}})
+  // CHECK-STRICT-FAST: fmul fast float {{.*}}, {{.*}}
   // CHECK-STRICT-FAST: load float, ptr
-  // CHECK-STRICT-FAST: call fast float 
@llvm.experimental.constrained.fadd.f32(float {{.*}}, float {{.*}}, {{.*}}
+  // CHECK-STRICT-FAST: fadd fast float {{.*}}, {{.*}}
 
   // CHECK-FAST1: load float, ptr
   // CHECK-FAST1: load float, ptr
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3031,6 +3031,7 @@
   SignedZeros = false;
   TrappingMath = false;
   RoundingFPMath = false;
+  FPExceptionBehavior = "";
   // If fast-math is set then set the fp-contract mode to fast.
   FPContract = "fast";
   SeenFfastMathOption = true;
@@ -3071,10 +3072,12 @@
   else {
 StrictFPModel = false;
 FPModel = "";
-D.Diag(clang::diag::warn_drv_overriding_flag_option)
-<< "-ffp-model=strict" <<
-((A->getNumValues() == 0) ?  A->getSpelling()
-: Args.MakeArgString(A->getSpelling() + A->getValue()));
+auto RHS = (A->getNumValues() == 0)
+   ? A->getSpelling()
+   : Args.MakeArgString(A->getSpelling() + A->getValue());
+if (RHS != "-ffp-model=strict")
+  D.Diag(clang::diag::warn_drv_overriding_flag_option)
+  << "-ffp-model=strict" << RHS;
   }
 }
 


Index: clang/test/Driver/fp-model.c
===
--- clang/test/Driver/fp-model.c
+++ clang/test/Driver/fp-model.c
@@ -66,6 +66,11 @@
 // RUN:   | FileCheck --check-prefix=WARN10 %s
 // WARN10: warning: overriding '-ffp-model=strict' option with '-fdenormal-fp-math=preserve-sign,preserve-sign' [-Woverriding-t-option]
 
+// RUN: %clang -### -ffp-model=fast -ffp-model=strict -c %s 2>&1 | FileCheck \
+// RUN:   --check-prefix=WARN11 %s
+// WARN11: warning: overriding '-ffp-model=fast' option with '-ffp-model=strict' [-Woverriding-t-option]
+// WARN11-NOT: warning: overriding '-ffp-model=strict' option with '-ffp-model=strict' [-Woverriding-t-option]
+
 // RUN: %clang -### -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NOROUND %s
 // CHECK-NOROUND: "-cc1"
@@ -107,6 +112,13 @@
 // CHECK-FPM-STRICT: "-frounding-math"
 // CHECK-FPM-STRICT: "-ffp-exception-behavior=strict"
 
+// RUN: %clang -### -nostdinc -ffp-model=strict -ffp-model=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NO-EXCEPT %s
+// RUN: %clang -### -nostdinc -ffp-model=strict -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NO-EXCEPT %s

[PATCH] D137153: [X86] Support -march=sierraforest, grandridge, graniterapids.

[Freddy, Ye via Phabricator via cfe-commits]

FreddyYe updated this revision to Diff 474140.
FreddyYe added a comment.

Added ZHAOXIN_FAM7H.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137153

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Basic/Targets/X86.cpp
  clang/test/CodeGen/attr-target-mv.c
  clang/test/CodeGen/target-builtin-noerror.c
  clang/test/Driver/x86-march.c
  clang/test/Misc/target-invalid-cpu-note.c
  clang/test/Preprocessor/predefined-arch-macros.c
  compiler-rt/lib/builtins/cpu_model.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/Support/X86TargetParser.def
  llvm/include/llvm/Support/X86TargetParser.h
  llvm/lib/Support/Host.cpp
  llvm/lib/Support/X86TargetParser.cpp
  llvm/lib/Target/X86/X86.td
  llvm/test/CodeGen/X86/cpus-intel.ll

Index: llvm/test/CodeGen/X86/cpus-intel.ll
===
--- llvm/test/CodeGen/X86/cpus-intel.ll
+++ llvm/test/CodeGen/X86/cpus-intel.ll
@@ -19,6 +19,9 @@
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=lakemont 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=raptorlake 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=meteorlake 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=sierraforest 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=grandridge 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=graniterapids 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=nocona 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=core2 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
@@ -56,6 +59,9 @@
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=knm 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=raptorlake 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=meteorlake 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=sierraforest 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=grandridge 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=graniterapids 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 
 define void @foo() {
   ret void
Index: llvm/lib/Target/X86/X86.td
===
--- llvm/lib/Target/X86/X86.td
+++ llvm/lib/Target/X86/X86.td
@@ -943,6 +943,14 @@
   list SPRFeatures =
 !listconcat(ICXFeatures, SPRAdditionalFeatures);
 
+  // Graniterapids
+  list GNRAdditionalFeatures = [FeatureAMXTILE,
+  FeatureAMXINT8,
+  FeatureAMXBF16,
+  FeatureBF16];
+  list GNRFeatures =
+!listconcat(SPRFeatures, GNRAdditionalFeatures);
+
   // Atom
   list AtomFeatures = [FeatureX87,
  FeatureCX8,
@@ -1050,6 +1058,19 @@
   list ADLFeatures =
 !listconcat(TRMFeatures, ADLAdditionalFeatures);
 
+  // Sierraforest
+  list SRFAdditionalFeatures = [FeatureCMPCCXADD,
+  FeatureAVXIFMA,
+  FeatureAVXNECONVERT,
+  FeatureAVXVNNIINT8];
+  list SRFFeatures =
+!listconcat(ADLFeatures, SRFAdditionalFeatures);
+
+  // Grandridge
+  list GRRAdditionalFeatures = [FeatureRAOINT];
+  list GRRFeatures =
+!listconcat(SRFFeatures, GRRAdditionalFeatures);
+
   // Knights Landing
   list KNLFeatures = [FeatureX87,
 FeatureCX8,
@@ -1441,6 +1462,10 @@
 ProcessorFeatures.GLPTuning>;
 def : ProcModel<"tremont", SLMModel, ProcessorFeatures.TRMFeatures,
 ProcessorFeatures.TRMTuning>;
+def : ProcModel<"sierraforest", AlderlakePModel, ProcessorFeatures.SRFFeatures,
+ProcessorFeatures.TRMTuning>;
+def : ProcModel<"grandridge", AlderlakePModel, ProcessorFeatures.GRRFeatures,
+ProcessorFeatures.TRMTuning>;
 
 // "Arrandale" along with corei3 and corei5
 foreach P = ["nehalem", 

[PATCH] D137153: [X86] Support -march=sierraforest, grandridge, graniterapids.

[Freddy, Ye via Phabricator via cfe-commits]

FreddyYe added a comment.

gcc has landed related patch: Sierraforest 
,
 Graniterapids 

 and Grandrdige 
.
 We can follow their CPU list order now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137153

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

[PATCH] D137153: [X86] Support -march=sierraforest, grandridge, graniterapids.

[Freddy, Ye via Phabricator via cfe-commits]

FreddyYe added inline comments.



Comment at: llvm/lib/Target/X86/X86.td:1528
+ProcessorFeatures.TRMTuning>;
+def : ProcModel<"grandridge", SLMModel, ProcessorFeatures.GRRFeatures,
+ProcessorFeatures.TRMTuning>;

RKSimon wrote:
> SLMModel explicitly doesn't have support for AVX/AVX2 - I think you should 
> investigate forking SLMModel and creating a AlderlakeEModel for all of these
> 
> Also, should these be here or moved up below tremont and the other atom cores?
Yeah, SLMModel's coverage is not right here. From view of coverage, 
AlderlakePModel is better here. Maybe we could use AlderlakePModel first? When 
sierraforest and grandridge's instruction perf info is more detailed or we can 
test from a real machine, we can then add a new Model and replaced here. Same 
for SRFTuning and GRRTuning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137153

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

[PATCH] D137153: [WIP][X86] Support -march=sierraforest, grandridge, graniterapids.

[Freddy, Ye via Phabricator via cfe-commits]

FreddyYe updated this revision to Diff 474138.
FreddyYe added a comment.

Rebase and add missing supports in cpu_model.c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137153

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Basic/Targets/X86.cpp
  clang/test/CodeGen/attr-target-mv.c
  clang/test/CodeGen/target-builtin-noerror.c
  clang/test/Driver/x86-march.c
  clang/test/Misc/target-invalid-cpu-note.c
  clang/test/Preprocessor/predefined-arch-macros.c
  compiler-rt/lib/builtins/cpu_model.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/Support/X86TargetParser.def
  llvm/include/llvm/Support/X86TargetParser.h
  llvm/lib/Support/Host.cpp
  llvm/lib/Support/X86TargetParser.cpp
  llvm/lib/Target/X86/X86.td
  llvm/test/CodeGen/X86/cpus-intel.ll

Index: llvm/test/CodeGen/X86/cpus-intel.ll
===
--- llvm/test/CodeGen/X86/cpus-intel.ll
+++ llvm/test/CodeGen/X86/cpus-intel.ll
@@ -19,6 +19,9 @@
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=lakemont 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=raptorlake 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=meteorlake 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=sierraforest 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=grandridge 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=graniterapids 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=nocona 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=core2 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
@@ -56,6 +59,9 @@
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=knm 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=raptorlake 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=meteorlake 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=sierraforest 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=grandridge 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=graniterapids 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 
 define void @foo() {
   ret void
Index: llvm/lib/Target/X86/X86.td
===
--- llvm/lib/Target/X86/X86.td
+++ llvm/lib/Target/X86/X86.td
@@ -943,6 +943,14 @@
   list SPRFeatures =
 !listconcat(ICXFeatures, SPRAdditionalFeatures);
 
+  // Graniterapids
+  list GNRAdditionalFeatures = [FeatureAMXTILE,
+  FeatureAMXINT8,
+  FeatureAMXBF16,
+  FeatureBF16];
+  list GNRFeatures =
+!listconcat(SPRFeatures, GNRAdditionalFeatures);
+
   // Atom
   list AtomFeatures = [FeatureX87,
  FeatureCX8,
@@ -1050,6 +1058,19 @@
   list ADLFeatures =
 !listconcat(TRMFeatures, ADLAdditionalFeatures);
 
+  // Sierraforest
+  list SRFAdditionalFeatures = [FeatureCMPCCXADD,
+  FeatureAVXIFMA,
+  FeatureAVXNECONVERT,
+  FeatureAVXVNNIINT8];
+  list SRFFeatures =
+!listconcat(ADLFeatures, SRFAdditionalFeatures);
+
+  // Grandridge
+  list GRRAdditionalFeatures = [FeatureRAOINT];
+  list GRRFeatures =
+!listconcat(SRFFeatures, GRRAdditionalFeatures);
+
   // Knights Landing
   list KNLFeatures = [FeatureX87,
 FeatureCX8,
@@ -1441,6 +1462,10 @@
 ProcessorFeatures.GLPTuning>;
 def : ProcModel<"tremont", SLMModel, ProcessorFeatures.TRMFeatures,
 ProcessorFeatures.TRMTuning>;
+def : ProcModel<"sierraforest", AlderlakePModel, ProcessorFeatures.SRFFeatures,
+ProcessorFeatures.TRMTuning>;
+def : ProcModel<"grandridge", AlderlakePModel, ProcessorFeatures.GRRFeatures,
+ProcessorFeatures.TRMTuning>;
 
 // "Arrandale" along with corei3 and 

[PATCH] D137583: [lldb] Fix simple template names and template params with scope qualifiers

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a subscriber: aaron.ballman.
dblaikie added a comment.

@aaron.ballman does this seem OK? (this was based on my suggestion in the 
related review linked in the description)

It probably needs tests in clang too - not sure if there's an opportunity to 
use a unit test to simplify access to the printing policy & exercise this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137583

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

[PATCH] D137536: [NFC] Replace use of PPC64 macro into powerpc64 in intrinsic headers

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf abandoned this revision.
qiucf added a comment.

I think rG7aa90b21b453d1ca52fdfccfd7e01e61d9e5b1f1 
 has 
already done that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137536

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

[PATCH] D137609: [C++20] [Modules] Remove unmaintained header modules

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a reviewer: bruno.
ChuanqiXu added a comment.



In D137609#3915632 , @iains wrote:

> I think that (if this change is approved) there will be also some 
> simplifications possible in the driver (since the mode that produces a 
> wrapper header for multiple command-line headers is different from the mode 
> where multiple command line headers would each produce a single C++ standard 
> header unit) ..

Yes, of course. I'll take care of it.

In D137609#3915495 , @Bigcheese wrote:

> I'm not aware of anyone using this mode, but please wait for responses from 
> Google and Meta people to verify that.

Sure.


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

https://reviews.llvm.org/D137609

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

[PATCH] D137153: [WIP][X86] Support -march=sierraforest, grandridge, graniterapids.

[Freddy, Ye via Phabricator via cfe-commits]

FreddyYe updated this revision to Diff 474132.
FreddyYe marked an inline comment as done.
FreddyYe added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137153

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Basic/Targets/X86.cpp
  clang/test/CodeGen/attr-target-mv.c
  clang/test/CodeGen/target-builtin-noerror.c
  clang/test/Driver/x86-march.c
  clang/test/Misc/target-invalid-cpu-note.c
  clang/test/Preprocessor/predefined-arch-macros.c
  compiler-rt/lib/builtins/cpu_model.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/Support/X86TargetParser.def
  llvm/include/llvm/Support/X86TargetParser.h
  llvm/lib/Support/Host.cpp
  llvm/lib/Support/X86TargetParser.cpp
  llvm/lib/Target/X86/X86.td
  llvm/test/CodeGen/X86/cpus-intel.ll

Index: llvm/test/CodeGen/X86/cpus-intel.ll
===
--- llvm/test/CodeGen/X86/cpus-intel.ll
+++ llvm/test/CodeGen/X86/cpus-intel.ll
@@ -17,6 +17,9 @@
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=yonah 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=prescott 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=lakemont 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=sierraforest 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=grandridge 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=graniterapids 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=nocona 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=core2 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
@@ -52,6 +55,9 @@
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=tremont 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=knl 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=knm 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=sierraforest 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=grandridge 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=graniterapids 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 
 define void @foo() {
   ret void
Index: llvm/lib/Target/X86/X86.td
===
--- llvm/lib/Target/X86/X86.td
+++ llvm/lib/Target/X86/X86.td
@@ -943,6 +943,14 @@
   list SPRFeatures =
 !listconcat(ICXFeatures, SPRAdditionalFeatures);
 
+  // Graniterapids
+  list GNRAdditionalFeatures = [FeatureAMXTILE,
+  FeatureAMXINT8,
+  FeatureAMXBF16,
+  FeatureBF16];
+  list GNRFeatures =
+!listconcat(SPRFeatures, GNRAdditionalFeatures);
+
   // Atom
   list AtomFeatures = [FeatureX87,
  FeatureCX8,
@@ -1050,6 +1058,19 @@
   list ADLFeatures =
 !listconcat(TRMFeatures, ADLAdditionalFeatures);
 
+  // Sierraforest
+  list SRFAdditionalFeatures = [FeatureCMPCCXADD,
+  FeatureAVXIFMA,
+  FeatureAVXNECONVERT,
+  FeatureAVXVNNIINT8];
+  list SRFFeatures =
+!listconcat(ADLFeatures, SRFAdditionalFeatures);
+
+  // Grandridge
+  list GRRAdditionalFeatures = [FeatureRAOINT];
+  list GRRFeatures =
+!listconcat(SRFFeatures, GRRAdditionalFeatures);
+
   // Knights Landing
   list KNLFeatures = [FeatureX87,
 FeatureCX8,
@@ -1441,6 +1462,10 @@
 ProcessorFeatures.GLPTuning>;
 def : ProcModel<"tremont", SLMModel, ProcessorFeatures.TRMFeatures,
 ProcessorFeatures.TRMTuning>;
+def : ProcModel<"sierraforest", AlderlakePModel, ProcessorFeatures.SRFFeatures,
+ProcessorFeatures.TRMTuning>;
+def : ProcModel<"grandridge", AlderlakePModel, ProcessorFeatures.GRRFeatures,
+ProcessorFeatures.TRMTuning>;
 
 // "Arrandale" along with corei3 and corei5
 

[PATCH] D137550: [clangd] Fix the code action `RemoveUsingNamespace`

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added a comment.

@tom-anders Thanks for reviewing and for your suggestions!

I think your suggestion is excellent, but for now, it is too complex for me to 
implement.

If this patch is okay, could you please help me commit it? Thanks again!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137550

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

[PATCH] D137550: [clangd] Fix the code action `RemoveUsingNamespace`

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 474127.
v1nh1shungry added a comment.

Improve the patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137550

Files:
  clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
  clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp


Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -249,6 +249,21 @@
 ns::Foo foo;
 foo + 10;
   }
+)cpp"},
+  {// Does not qualify user-defined literals
+   R"cpp(
+  namespace ns {
+  long double operator "" _w(long double);
+  }
+  using namespace n^s;
+  int main() { 1.5_w; }
+)cpp",
+   R"cpp(
+  namespace ns {
+  long double operator "" _w(long double);
+  }
+  
+  int main() { 1.5_w; }
 )cpp"}};
   for (auto C : Cases)
 EXPECT_EQ(C.second, apply(C.first)) << C.first;
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -155,12 +155,21 @@
 if (!visibleContext(T->getDeclContext())
  ->Equals(TargetDirective->getNominatedNamespace()))
   return;
-// Avoid adding qualifiers before operators, e.g.
+// Avoid adding qualifiers before operators
+// and user-defined literals, e.g.
 //   using namespace std;
+//   auto s = "foo"s; // Must not changed to auto s = "foo" std::s;
 //   cout << "foo"; // Must not changed to std::cout std:: << "foo"
-// FIXME: User-defined literals are not handled
-if (T->isInIdentifierNamespace(
-Decl::IdentifierNamespace::IDNS_NonMemberOperator))
+// FIXME: Add a using-directive for user-defined literals
+// declared in an inline namespace, e.g.
+//   using namespace s^td;
+//   int main() { cout << "foo"s; }
+// change to
+//   using namespace std::literals;
+//   int main() { std::cout << "foo"s; }
+if (auto kind = T->getDeclName().getNameKind();
+kind == DeclarationName::NameKind::CXXOperatorName ||
+kind == DeclarationName::NameKind::CXXLiteralOperatorName)
   return;
   }
   SourceLocation Loc = Ref.NameLoc;


Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -249,6 +249,21 @@
 ns::Foo foo;
 foo + 10;
   }
+)cpp"},
+  {// Does not qualify user-defined literals
+   R"cpp(
+  namespace ns {
+  long double operator "" _w(long double);
+  }
+  using namespace n^s;
+  int main() { 1.5_w; }
+)cpp",
+   R"cpp(
+  namespace ns {
+  long double operator "" _w(long double);
+  }
+  
+  int main() { 1.5_w; }
 )cpp"}};
   for (auto C : Cases)
 EXPECT_EQ(C.second, apply(C.first)) << C.first;
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -155,12 +155,21 @@
 if (!visibleContext(T->getDeclContext())
  ->Equals(TargetDirective->getNominatedNamespace()))
   return;
-// Avoid adding qualifiers before operators, e.g.
+// Avoid adding qualifiers before operators
+// and user-defined literals, e.g.
 //   using namespace std;
+//   auto s = "foo"s; // Must not changed to auto s = "foo" std::s;
 //   cout << "foo"; // Must not changed to std::cout std:: << "foo"
-// FIXME: User-defined literals are not handled
-if (T->isInIdentifierNamespace(
-Decl::IdentifierNamespace::IDNS_NonMemberOperator))
+// FIXME: Add a using-directive for user-defined literals
+// declared in an inline namespace, e.g.
+//   using namespace s^td;
+//   int main() { cout << "foo"s; }
+// change to
+//   using namespace std::literals;
+//   int main() { std::cout 

[PATCH] D137473: [vfs] Allow root paths relative to the directory of the vfsoverlay YAML file

[Ben Barham via Phabricator via cfe-commits]

bnbarham added inline comments.



Comment at: llvm/include/llvm/Support/VirtualFileSystem.h:660
 ///   'use-external-names': 
+///   'root-relative': 
 ///   'overlay-relative': 

haowei wrote:
> bnbarham wrote:
> > phosek wrote:
> > > Could we make this just a boolean akin to `overlay-relative` since there 
> > > are only two options (default to `false`)?
> > I personally prefer being explicit here, `overlay-relative` is fairly 
> > confusing as it is.
> > 
> > `overlay-relative` isn't about allowing relative paths, but instead means 
> > that *all* external paths should be prefixed with the directory of the 
> > overlay. To put another way, external paths can be relative whether this is 
> > true/false, `overlay-relative` just *always* prepends the overlay path.
> > 
> > Could you add a comment to make it clear that this has no interaction with 
> > `overlay-relative`? If you want to add a comment to `overlay-relative` with 
> > something like the above that would also be appreciated :)
> I also think `overlay-relative` is a bit misleading and it should be an enum 
> instead of a boolean option. And it should only work if the external paths 
> are relative instead of blindly prepend the overlay dir to every external 
> paths. But changing this will be a breaking change so I prefer to avoid it in 
> this patch.
> 
> I updated the descriptions.
FWIW it was added explicitly to always prepend, it's used for reproducers where 
the overlay may already have absolute paths. I was just trying to explain what 
`overlay-relative` does.



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1903
+assert(!FullPath.empty() && "YAML file directory must exist");
+sys::fs::make_absolute(FS->getYAMLFileDir(), Name);
+Name = canonicalize(Name);

haowei wrote:
> bnbarham wrote:
> > IMO both this and CWD should be using the base FS instead. VFS didn't have 
> > a CWD previously, but now that it does it doesn't really make sense to use 
> > the process wide CWD. Especially since `-working-directory` doesn't change 
> > it.
> Could you clarify a bit more about the "base FS" please? I am still quite new 
> to the LLVM VFS system.  Which API should I use to get the appropriate 
> working directory instead of the the process wide CWD?
> 
> I avoided changing the default behavior (relative to the process's current 
> working directory) as I am a bit concerned breaking other people's use cases. 
> 
> I looked up a bit, the value "--working-directory" will be writen into 
> "VFS->setCurrentWorkingDirectory()" Do you think it is a better idea to use 
> this value instead of the process current working directory? Though it would 
> still be a behavior change for users rely on process current working 
> directories. 
> Could you clarify a bit more about the "base FS" please?

By "base FS" I just meant the filesystem that's passed down when creating the 
RedirectingFileSystem, ie. `ExternalFS` in `getVFSFromYAML`.

> Which API should I use to get the appropriate working directory instead of 
> the the process wide CWD?

`ExternalFS->getCurrentWorkingDirectory()` and `ExternalFS->makeAbsolute` is 
what I'd expect to be used here.

> Do you think it is a better idea to use this value instead of the process 
> current working directory? Though it would still be a behavior change for 
> users rely on process current working directories.

Yes, I think it's surprising that it currently *isn't* using it. It's just a 
hold over from when the VFS didn't have the concept of CWD. I'd be happy with 
accepting this patch as is, with a separate patch after that changes to using 
the VFS CWD.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137473

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

[PATCH] D137650: [clangd] Implement hover for C++ string literals

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry updated this revision to Diff 474117.
v1nh1shungry added a comment.

Improve the patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137650

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1300,7 +1300,6 @@
   "auto x = ^42.0i;",
   "auto x = ^42;",
   "auto x = ^nullptr;",
-  "auto x = ^\"asdf\";",
   };
 
   for (const auto  : Tests) {
@@ -1326,6 +1325,11 @@
  HI.Type = "char";
  HI.Value = "65 (0x41)";
}},
+  {"auto s = ^[[\"Hello, world!\"]]; // string literal",
+   [](HoverInfo ) {
+ HI.Name = "String Literal";
+ HI.Type = "const char[14]";
+   }},
   {
   R"cpp(// Local variable
 int main() {
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -777,6 +777,51 @@
   return HI;
 }
 
+llvm::Optional
+getStringLiteralHoverContents(const StringLiteral *SL,
+  const LangOptions ) {
+  HoverInfo HI;
+
+  HI.Name = "String Literal";
+
+  // In C++ string literals have `const` qualifier, but in C they don't.
+  std::string Qualifier;
+
+  std::string CharType;
+  // C++
+  // TODO: Support different C++ standards
+  if (LangOpts.CPlusPlus) {
+Qualifier = "const";
+switch (SL->getKind()) {
+case StringLiteral::Ordinary:
+  CharType = "char";
+  break;
+case StringLiteral::Wide:
+  CharType = "wchar_t";
+  break;
+case StringLiteral::UTF8:
+  CharType = "char8_t";
+  break;
+case StringLiteral::UTF16:
+  CharType = "char16_t";
+  break;
+case StringLiteral::UTF32:
+  CharType = "char32_t";
+  break;
+}
+  } else {
+// TODO: For other languages
+return llvm::None;
+  }
+
+  HoverInfo::PrintedType Type;
+  Type.Type =
+  llvm::formatv("{0} {1}[{2}]", Qualifier, CharType, SL->getLength() + 1);
+  HI.Type = Type;
+
+  return HI;
+}
+
 bool isLiteral(const Expr *E) {
   // Unfortunately there's no common base Literal classes inherits from
   // (apart from Expr), therefore these exclusions.
@@ -806,8 +851,14 @@
const SymbolIndex *Index) {
   // There's not much value in hovering over "42" and getting a hover card
   // saying "42 is an int", similar for other literals.
-  if (isLiteral(E))
+  if (isLiteral(E)) {
+// Generate hover info for string literals showing
+// its character's type and length
+if (const StringLiteral *SL = dyn_cast(E)) {
+  return getStringLiteralHoverContents(SL, AST.getLangOpts());
+}
 return llvm::None;
+  }
 
   HoverInfo HI;
   // For `this` expr we currently generate hover with pointee type.


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1300,7 +1300,6 @@
   "auto x = ^42.0i;",
   "auto x = ^42;",
   "auto x = ^nullptr;",
-  "auto x = ^\"asdf\";",
   };
 
   for (const auto  : Tests) {
@@ -1326,6 +1325,11 @@
  HI.Type = "char";
  HI.Value = "65 (0x41)";
}},
+  {"auto s = ^[[\"Hello, world!\"]]; // string literal",
+   [](HoverInfo ) {
+ HI.Name = "String Literal";
+ HI.Type = "const char[14]";
+   }},
   {
   R"cpp(// Local variable
 int main() {
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -777,6 +777,51 @@
   return HI;
 }
 
+llvm::Optional
+getStringLiteralHoverContents(const StringLiteral *SL,
+  const LangOptions ) {
+  HoverInfo HI;
+
+  HI.Name = "String Literal";
+
+  // In C++ string literals have `const` qualifier, but in C they don't.
+  std::string Qualifier;
+
+  std::string CharType;
+  // C++
+  // TODO: Support different C++ standards
+  if (LangOpts.CPlusPlus) {
+Qualifier = "const";
+switch (SL->getKind()) {
+case StringLiteral::Ordinary:
+  CharType = "char";
+  break;
+case StringLiteral::Wide:
+  CharType = "wchar_t";
+  break;
+case StringLiteral::UTF8:
+  CharType = "char8_t";
+  break;
+case StringLiteral::UTF16:
+  CharType = "char16_t";
+  break;
+case StringLiteral::UTF32:
+  CharType = "char32_t";
+  break;
+}
+  } else {
+// TODO: For other languages
+ 

[PATCH] D137473: [vfs] Allow root paths relative to the directory of the vfsoverlay YAML file

[Haowei Wu via Phabricator via cfe-commits]

haowei added inline comments.



Comment at: clang/test/VFS/Inputs/root-relative-overlay.yaml:4
+  'case-sensitive': false,
+  'overlay-relative': true,
+  'root-relative': 'yaml-dir',

bnbarham wrote:
> bnbarham wrote:
> > I'd prefer a test without `overlay-relative` set to make it clear they 
> > don't depend on each other.
> There's also unit tests in `llvm/unittests/Support/VirtualFileSystemTest.cpp` 
> that you could add to.
I need some time to patch this unit test for testing the new option.
Existing functions in this unit test file does not support defining the 
YAMLFilePath.

I will update this patch when I finish.



Comment at: clang/test/VFS/Inputs/root-relative-overlay.yaml:4
+  'case-sensitive': false,
+  'overlay-relative': true,
+  'root-relative': 'yaml-dir',

haowei wrote:
> bnbarham wrote:
> > bnbarham wrote:
> > > I'd prefer a test without `overlay-relative` set to make it clear they 
> > > don't depend on each other.
> > There's also unit tests in 
> > `llvm/unittests/Support/VirtualFileSystemTest.cpp` that you could add to.
> I need some time to patch this unit test for testing the new option.
> Existing functions in this unit test file does not support defining the 
> YAMLFilePath.
> 
> I will update this patch when I finish.
I updated the yaml file and add unit tests with overlay-relative set to true 
and false.



Comment at: llvm/include/llvm/Support/VirtualFileSystem.h:660
 ///   'use-external-names': 
+///   'root-relative': 
 ///   'overlay-relative': 

bnbarham wrote:
> phosek wrote:
> > Could we make this just a boolean akin to `overlay-relative` since there 
> > are only two options (default to `false`)?
> I personally prefer being explicit here, `overlay-relative` is fairly 
> confusing as it is.
> 
> `overlay-relative` isn't about allowing relative paths, but instead means 
> that *all* external paths should be prefixed with the directory of the 
> overlay. To put another way, external paths can be relative whether this is 
> true/false, `overlay-relative` just *always* prepends the overlay path.
> 
> Could you add a comment to make it clear that this has no interaction with 
> `overlay-relative`? If you want to add a comment to `overlay-relative` with 
> something like the above that would also be appreciated :)
I also think `overlay-relative` is a bit misleading and it should be an enum 
instead of a boolean option. And it should only work if the external paths are 
relative instead of blindly prepend the overlay dir to every external paths. 
But changing this will be a breaking change so I prefer to avoid it in this 
patch.

I updated the descriptions.



Comment at: llvm/include/llvm/Support/VirtualFileSystem.h:752
+  enum class RootRelativeKind {
+/// The roots are relative to the current working directory.
+CWD,

bnbarham wrote:
> `to the current working directory when the overlay is created.` is maybe a 
> little clearer to me
Added into the comments.



Comment at: llvm/include/llvm/Support/VirtualFileSystem.h:755
+/// The roots are relative to the directory where the YAML file locates.
+YAMLDir
+  };

bnbarham wrote:
> Any thoughts on something like `OverlayDir` instead?
I think OverlayDir is better.



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1903
+assert(!FullPath.empty() && "YAML file directory must exist");
+sys::fs::make_absolute(FS->getYAMLFileDir(), Name);
+Name = canonicalize(Name);

bnbarham wrote:
> IMO both this and CWD should be using the base FS instead. VFS didn't have a 
> CWD previously, but now that it does it doesn't really make sense to use the 
> process wide CWD. Especially since `-working-directory` doesn't change it.
Could you clarify a bit more about the "base FS" please? I am still quite new 
to the LLVM VFS system.  Which API should I use to get the appropriate working 
directory instead of the the process wide CWD?

I avoided changing the default behavior (relative to the process's current 
working directory) as I am a bit concerned breaking other people's use cases. 

I looked up a bit, the value "--working-directory" will be writen into 
"VFS->setCurrentWorkingDirectory()" Do you think it is a better idea to use 
this value instead of the process current working directory? Though it would 
still be a behavior change for users rely on process current working 
directories. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137473

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

[PATCH] D137473: [vfs] Allow root paths relative to the directory of the vfsoverlay YAML file

[Haowei Wu via Phabricator via cfe-commits]

haowei updated this revision to Diff 474115.
haowei marked 2 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137473

Files:
  clang/test/VFS/Inputs/root-relative-overlay.yaml
  clang/test/VFS/relative-path.c
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp

Index: llvm/lib/Support/VirtualFileSystem.cpp
===
--- llvm/lib/Support/VirtualFileSystem.cpp
+++ llvm/lib/Support/VirtualFileSystem.cpp
@@ -1482,12 +1482,12 @@
   return Combined;
 }
 
-void RedirectingFileSystem::setExternalContentsPrefixDir(StringRef PrefixDir) {
-  ExternalContentsPrefixDir = PrefixDir.str();
+void RedirectingFileSystem::setOverlayFileDir(StringRef Dir) {
+  OverlayFileDir = Dir.str();
 }
 
-StringRef RedirectingFileSystem::getExternalContentsPrefixDir() const {
-  return ExternalContentsPrefixDir;
+StringRef RedirectingFileSystem::getOverlayFileDir() const {
+  return OverlayFileDir;
 }
 
 void RedirectingFileSystem::setFallthrough(bool Fallthrough) {
@@ -1621,6 +1621,20 @@
 return None;
   }
 
+  Optional
+  parseRootRelativeKind(yaml::Node *N) {
+SmallString<12> Storage;
+StringRef Value;
+if (!parseScalarString(N, Value, Storage))
+  return None;
+if (Value.equals_insensitive("cwd")) {
+  return RedirectingFileSystem::RootRelativeKind::CWD;
+} else if (Value.equals_insensitive("overlay-dir")) {
+  return RedirectingFileSystem::RootRelativeKind::OverlayDir;
+}
+return None;
+  }
+
   struct KeyStatus {
 bool Required;
 bool Seen = false;
@@ -1828,7 +1842,7 @@
 
 SmallString<256> FullPath;
 if (FS->IsRelativeOverlay) {
-  FullPath = FS->getExternalContentsPrefixDir();
+  FullPath = FS->getOverlayFileDir();
   assert(!FullPath.empty() &&
  "External contents prefix directory must exist");
   llvm::sys::path::append(FullPath, Value);
@@ -1884,6 +1898,15 @@
   } else if (sys::path::is_absolute(Name,
 sys::path::Style::windows_backslash)) {
 path_style = sys::path::Style::windows_backslash;
+  } else if (FS->RootRelative ==
+ RedirectingFileSystem::RootRelativeKind::OverlayDir) {
+SmallString<256> FullPath = FS->getOverlayFileDir();
+assert(!FullPath.empty() && "Overlay file directory must exist");
+sys::fs::make_absolute(FS->getOverlayFileDir(), Name);
+Name = canonicalize(Name);
+path_style = sys::path::is_absolute(Name, sys::path::Style::posix)
+ ? sys::path::Style::posix
+ : sys::path::Style::windows_backslash;
   } else {
 // Relative VFS root entries are made absolute to the current working
 // directory, then we can determine the path style from that.
@@ -1962,6 +1985,7 @@
 KeyStatusPair("version", true),
 KeyStatusPair("case-sensitive", false),
 KeyStatusPair("use-external-names", false),
+KeyStatusPair("root-relative", false),
 KeyStatusPair("overlay-relative", false),
 KeyStatusPair("fallthrough", false),
 KeyStatusPair("redirecting-with", false),
@@ -2051,6 +2075,13 @@
   error(I.getValue(), "expected valid redirect kind");
   return false;
 }
+  } else if (Key == "root-relative") {
+if (auto Kind = parseRootRelativeKind(I.getValue())) {
+  FS->RootRelative = *Kind;
+} else {
+  error(I.getValue(), "expected valid root-relative kind");
+  return false;
+}
   } else {
 llvm_unreachable("key missing from Keys");
   }
@@ -2100,13 +2131,13 @@
 // Example:
 //-ivfsoverlay dummy.cache/vfs/vfs.yaml
 // yields:
-//  FS->ExternalContentsPrefixDir => //dummy.cache/vfs
+//  FS->YAMLFileDir => //dummy.cache/vfs
 //
 SmallString<256> OverlayAbsDir = sys::path::parent_path(YAMLFilePath);
 std::error_code EC = llvm::sys::fs::make_absolute(OverlayAbsDir);
 assert(!EC && "Overlay dir final path must be absolute");
 (void)EC;
-FS->setExternalContentsPrefixDir(OverlayAbsDir);
+FS->setOverlayFileDir(OverlayAbsDir);
   }
 
   if (!P.parse(Root, FS.get()))
Index: llvm/include/llvm/Support/VirtualFileSystem.h
===
--- llvm/include/llvm/Support/VirtualFileSystem.h
+++ llvm/include/llvm/Support/VirtualFileSystem.h
@@ -651,17 +651,28 @@
 /// \endverbatim
 ///
 /// The roots may be absolute or relative. If relative they will be made
-/// absolute against the current working directory.
+/// absolute against either current working directory or the directory where
+/// the Overlay YAML file is located, depending on the 'root-relative'
+/// configuration.
 ///
 /// All configuration options are 

[PATCH] D137650: [clangd] Implement hover for C++ string literals

[v1nh1shungry via Phabricator via cfe-commits]

v1nh1shungry added a comment.

@tom-anders Thanks for reviewing and for your suggestions!




Comment at: clang-tools-extra/clangd/Hover.cpp:785
+
+  // TODO: Show string literal's contents
+  HI.Name = "String Literal";

tom-anders wrote:
> Is it really useful to show the contents inside the Hover? You already see 
> the contents of the string literal anyway, so I don't think this adds any 
> value
I added this comment only because others show the contents. I think you're 
right.



Comment at: clang-tools-extra/clangd/Hover.cpp:814
+  } else {
+// TODO: For other languages
+return llvm::None;

tom-anders wrote:
> Hmm so what's stopping us from adding support for C here?
Because I'm not that familiar with C's data types and standards.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137650

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

[PATCH] D137677: [include-cleaner] add macro symbols and implicit refs to HTML report

[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.

Demo: https://gistpreview.github.io/?fec9b77c726cfb3cc7c424b197e3f68c


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137677

Files:
  clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
  clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
  clang-tools-extra/include-cleaner/lib/Types.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
@@ -47,6 +47,13 @@
 
 class HTMLReportAction : public clang::ASTFrontendAction {
   RecordedAST AST;
+  RecordedPP PP;
+
+  void ExecuteAction() override {
+auto  = getCompilerInstance().getPreprocessor();
+P.addPPCallbacks(PP.record(P));
+ASTFrontendAction::ExecuteAction();
+  }
 
   std::unique_ptr CreateASTConsumer(CompilerInstance ,
  StringRef File) override {
@@ -62,7 +69,7 @@
   exit(1);
 }
 writeHTMLReport(AST.Ctx->getSourceManager().getMainFileID(), AST.Roots,
-*AST.Ctx, OS);
+PP.MacroReferences, *AST.Ctx, OS);
   }
 };
 
Index: clang-tools-extra/include-cleaner/lib/Types.cpp
===
--- clang-tools-extra/include-cleaner/lib/Types.cpp
+++ clang-tools-extra/include-cleaner/lib/Types.cpp
@@ -21,7 +21,7 @@
   return OS << ND->getNameAsString();
 return OS << S.declaration().getDeclKindName();
   case Symbol::Macro:
-return OS << S.macro().Name;
+return OS << S.macro().Name->getName();
   case Symbol::Standard:
 return OS << S.standard().scope() << S.standard().name();
   }
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
@@ -15,11 +15,12 @@
 
 #include "AnalysisInternal.h"
 #include "clang-include-cleaner/Analysis.h"
+#include "clang-include-cleaner/Types.h"
 #include "clang/AST/ASTContext.h"
-#include "clang/AST/Decl.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
+#include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace clang::include_cleaner {
@@ -38,6 +39,7 @@
   }
   .ref { text-decoration: underline; color: #008; }
   .sel { position: relative; cursor: pointer; }
+  .ref.implicit { background-color: #ff8; }
   #hover {
 background-color: #aaccff; border: 1px solid black;
 z-index: 1;
@@ -47,6 +49,8 @@
   }
   #hover p, #hover pre { margin: 0; }
   #hover section header { font-weight: bold; }
+  #hover section.implicit { background-color: #bbb; }
+  #hover section.ambiguous { background-color: #caf; }
   #hover section:not(:first-child) { margin-top: 1em; }
 )css";
 
@@ -75,18 +79,67 @@
 )js";
 
 // Print the declaration tersely, but enough to identify e.g. which overload.
-std::string printDecl(const NamedDecl ) {
+std::string printSymbol(const Symbol ) {
   std::string S;
   llvm::raw_string_ostream OS(S);
-  PrintingPolicy PP = ND.getASTContext().getPrintingPolicy();
-  PP.FullyQualifiedName = true;
-  PP.TerseOutput = true;
-  PP.SuppressInitializers = true;
-  ND.print(OS, PP);
+  switch (Sym.kind()) {
+  case Symbol::Declaration: {
+const auto  = Sym.declaration();
+PrintingPolicy PP = D.getASTContext().getPrintingPolicy();
+PP.FullyQualifiedName = true;
+PP.TerseOutput = true;
+PP.SuppressInitializers = true;
+D.print(OS, PP);
+break;
+  }
+  case Symbol::Macro:
+OS << "#define " << Sym.macro().Name->getName() << " ...";
+break;
+  case Symbol::Standard:
+OS << "(defined in stdlib)";
+break;
+  }
   llvm::erase_value(S, '\n');
   return S;
 }
 
+// Categorize the symbol, like FunctionDecl or Macro
+llvm::StringRef describeSymbol(const Symbol ) {
+  switch (Sym.kind()) {
+  case Symbol::Declaration:
+return Sym.declaration().getDeclKindName();
+  case Symbol::Macro:
+return "Macro";
+  case Symbol::Standard:
+return "Standard symbol";
+  }
+  llvm_unreachable("unhandled symbol kind");
+}
+
+// Categorize the symbol, like FunctionDecl or Macro
+std::string locateSymbol(const Symbol , const SourceManager ) {
+  switch (Sym.kind()) {
+  case Symbol::Declaration:
+return Sym.declaration().getLocation().printToString(SM);
+  case Symbol::Macro:
+return Sym.macro().Definition.printToString(SM);
+  case Symbol::Standard:
+return ("stdlib header " + Sym.standard().header().name()).str();
+  }
+  

[PATCH] D137674: [clangd] Make system-include-extractor.test more resilient in the face of paths with special characters

[Nathan Ridge via Phabricator via cfe-commits]

nridge abandoned this revision.
nridge added a comment.

Great minds :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137674

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

[PATCH] D133757: [clangd] Perform system include extraction inside CommandMangler

[Alan Zhao via Phabricator via cfe-commits]

ayzhao added a comment.

In D133757#3916040 , @sammccall wrote:

> In D133757#3916028 , @nridge wrote:
>
>> In D133757#3915995 , @ayzhao wrote:
>>
>>> This change is causing system-include-extractor.test 
>>> 
>>>  to fail on Chrome: https://crbug.com/1382508
>>>
>>> The test fails on this assertion 
>>> 
>>>  if the path to the LLVM repository contains the plus sign (+). The actual 
>>> output replaces the plus sign with the URL escape code %2B, but the still 
>>> expects the plus sign in the output.
>>
>> Thanks for the report, I sent out D137674  
>> to work around this.
>
> Oops, I landed a slightly worse version in 
> 96d771c4707df9b3153ea5a8451f2bbbe17c8917 
>  before 
> seeing your message. @ayzhao it should be fixed at head.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133757

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

[PATCH] D137674: [clangd] Make system-include-extractor.test more resilient in the face of paths with special characters

[Alan Zhao via Phabricator via cfe-commits]

ayzhao accepted this revision.
ayzhao added a comment.

Thanks for the quick fix! I patched this in and I can confirm it fixes the 
issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137674

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

[PATCH] D133757: [clangd] Perform system include extraction inside CommandMangler

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D133757#3916028 , @nridge wrote:

> In D133757#3915995 , @ayzhao wrote:
>
>> This change is causing system-include-extractor.test 
>> 
>>  to fail on Chrome: https://crbug.com/1382508
>>
>> The test fails on this assertion 
>> 
>>  if the path to the LLVM repository contains the plus sign (+). The actual 
>> output replaces the plus sign with the URL escape code %2B, but the still 
>> expects the plus sign in the output.
>
> Thanks for the report, I sent out D137674  
> to work around this.

Oops, I landed a slightly worse version in 
96d771c4707df9b3153ea5a8451f2bbbe17c8917 
 before 
seeing your message. @ayzhao it should be fixed at head.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133757

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

[PATCH] D137674: [clangd] Make system-include-extractor.test more resilient in the face of paths with special characters

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

LG, I committed 
https://github.com/llvm/llvm-project/commit/96d771c4707df9b3153ea5a8451f2bbbe17c8917
 but was too lazy to add the comment, feel free to land this or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137674

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

[clang-tools-extra] 96d771c - [clangd] Avoid escaping issues in system-include-extractor.test

[via cfe-commits]

Author: Sam McCall
Date: 2022-11-08T23:48:15+01:00
New Revision: 96d771c4707df9b3153ea5a8451f2bbbe17c8917

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

LOG: [clangd] Avoid escaping issues in system-include-extractor.test

Added: 


Modified: 
clang-tools-extra/clangd/test/system-include-extractor.test

Removed: 




diff  --git a/clang-tools-extra/clangd/test/system-include-extractor.test 
b/clang-tools-extra/clangd/test/system-include-extractor.test
index ba6aaf6efb9d..cb894623a3b6 100644
--- a/clang-tools-extra/clangd/test/system-include-extractor.test
+++ b/clang-tools-extra/clangd/test/system-include-extractor.test
@@ -63,7 +63,7 @@
 # CHECK:   "method": "textDocument/publishDiagnostics",
 # CHECK-NEXT:   "params": {
 # CHECK-NEXT: "diagnostics": [],
-# CHECK-NEXT: "uri": "file://INPUT_DIR/the-file.cpp",
+# CHECK-NEXT: "uri": "file://{{.*}}/the-file.cpp",
 ---
 {"jsonrpc":"2.0","id":1,"method":"shutdown"}
 ---



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

[PATCH] D133757: [clangd] Perform system include extraction inside CommandMangler

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

In D133757#3915995 , @ayzhao wrote:

> This change is causing system-include-extractor.test 
> 
>  to fail on Chrome: https://crbug.com/1382508
>
> The test fails on this assertion 
> 
>  if the path to the LLVM repository contains the plus sign (+). The actual 
> output replaces the plus sign with the URL escape code %2B, but the still 
> expects the plus sign in the output.

Thanks for the report, I sent out D137674  to 
work around this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133757

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

[PATCH] D137674: [clangd] Make system-include-extractor.test more resilient in the face of paths with special characters

[Nathan Ridge via Phabricator via cfe-commits]

nridge created this revision.
nridge added a reviewer: ayzhao.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
nridge requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137674

Files:
  clang-tools-extra/clangd/test/system-include-extractor.test


Index: clang-tools-extra/clangd/test/system-include-extractor.test
===
--- clang-tools-extra/clangd/test/system-include-extractor.test
+++ clang-tools-extra/clangd/test/system-include-extractor.test
@@ -63,7 +63,9 @@
 # CHECK:   "method": "textDocument/publishDiagnostics",
 # CHECK-NEXT:   "params": {
 # CHECK-NEXT: "diagnostics": [],
-# CHECK-NEXT: "uri": "file://INPUT_DIR/the-file.cpp",
+# Double-check that the diagnostics pertain to the intended file.
+# We don't match the path portion exactly to avoid issues around URL escaping.
+# CHECK-NEXT: "uri": "file://{{.*}}/the-file.cpp",
 ---
 {"jsonrpc":"2.0","id":1,"method":"shutdown"}
 ---


Index: clang-tools-extra/clangd/test/system-include-extractor.test
===
--- clang-tools-extra/clangd/test/system-include-extractor.test
+++ clang-tools-extra/clangd/test/system-include-extractor.test
@@ -63,7 +63,9 @@
 # CHECK:   "method": "textDocument/publishDiagnostics",
 # CHECK-NEXT:   "params": {
 # CHECK-NEXT: "diagnostics": [],
-# CHECK-NEXT: "uri": "file://INPUT_DIR/the-file.cpp",
+# Double-check that the diagnostics pertain to the intended file.
+# We don't match the path portion exactly to avoid issues around URL escaping.
+# CHECK-NEXT: "uri": "file://{{.*}}/the-file.cpp",
 ---
 {"jsonrpc":"2.0","id":1,"method":"shutdown"}
 ---
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137659: [Driver] Refactor err_drv_unsupported_option_argument call sites to use llvm::opt::Arg::getSpelling

[Fangrui Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8c2c62282fca: [Driver] Refactor 
err_drv_unsupported_option_argument call sites to use llvm… (authored by 
MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137659

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/lib/Driver/ToolChains/Arch/Mips.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/X86.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Hexagon.cpp
  clang/lib/Driver/ToolChains/MSP430.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/mingw-cfguard.c
  flang/lib/Frontend/CompilerInvocation.cpp

Index: flang/lib/Frontend/CompilerInvocation.cpp
===
--- flang/lib/Frontend/CompilerInvocation.cpp
+++ flang/lib/Frontend/CompilerInvocation.cpp
@@ -688,7 +688,7 @@
   fpContractMode = LangOptions::FPM_Fast;
 else {
   diags.Report(clang::diag::err_drv_unsupported_option_argument)
-  << a->getOption().getName() << val;
+  << a->getSpelling() << val;
   return false;
 }
 
Index: clang/test/Driver/mingw-cfguard.c
===
--- clang/test/Driver/mingw-cfguard.c
+++ clang/test/Driver/mingw-cfguard.c
@@ -25,4 +25,4 @@
 // GUARD_NOCHECKS-NOT: "--no-guard-cf"
 
 // RUN: %clang --target=x86_64-w64-windows-gnu -### %s -mguard=xxx 2>&1 | FileCheck -check-prefix=GUARD_UNKNOWN %s
-// GUARD_UNKNOWN: error: unsupported argument 'xxx' to option '--mguard='
+// GUARD_UNKNOWN: error: unsupported argument 'xxx' to option '-mguard='
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1711,7 +1711,7 @@
 StringRef Value = A->getValue();
 if (Value != "simple" && Value != "mangled")
   Diags.Report(diag::err_drv_unsupported_option_argument)
-  << A->getOption().getName() << A->getValue();
+  << A->getSpelling() << A->getValue();
 Opts.setDebugSimpleTemplateNames(
 StringRef(A->getValue()) == "simple"
 ? codegenoptions::DebugTemplateNamesKind::Simple
Index: clang/lib/Driver/ToolChains/MSP430.cpp
===
--- clang/lib/Driver/ToolChains/MSP430.cpp
+++ clang/lib/Driver/ToolChains/MSP430.cpp
@@ -101,7 +101,7 @@
 Features.push_back("+hwmultf5");
   } else {
 D.Diag(clang::diag::err_drv_unsupported_option_argument)
-<< HWMultArg->getOption().getName() << HWMult;
+<< HWMultArg->getSpelling() << HWMult;
   }
 }
 
Index: clang/lib/Driver/ToolChains/Hexagon.cpp
===
--- clang/lib/Driver/ToolChains/Hexagon.cpp
+++ clang/lib/Driver/ToolChains/Hexagon.cpp
@@ -40,7 +40,7 @@
 StringRef Val = A->getValue();
 if (!Val.equals_insensitive("64b") && !Val.equals_insensitive("128b"))
   D.Diag(diag::err_drv_unsupported_option_argument)
-  << A->getOption().getName() << Val;
+  << A->getSpelling() << Val;
   }
 }
 
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -729,7 +729,7 @@
 Args.MakeArgString("--compress-debug-sections=" + Twine(Value)));
   } else {
 D.Diag(diag::err_drv_unsupported_option_argument)
-<< A->getOption().getName() << Value;
+<< A->getSpelling() << Value;
   }
 }
   }
Index: clang/lib/Driver/ToolChains/Flang.cpp
===
--- clang/lib/Driver/ToolChains/Flang.cpp
+++ clang/lib/Driver/ToolChains/Flang.cpp
@@ -104,7 +104,7 @@
   // Clang's "fast-honor-pragmas" option is not supported because it is
   // non-standard
   D.Diag(diag::err_drv_unsupported_option_argument)
-  << A->getOption().getName() << Val;
+  << A->getSpelling() << Val;
   }
 
   for (const Arg *A : Args) {
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -296,7 +296,7 @@
   CmdArgs.push_back(Args.MakeArgString("--compress-debug-sections=" + V));
 else
   TC.getDriver().Diag(diag::err_drv_unsupported_option_argument)
-  

[clang] 8c2c622 - [Driver] Refactor err_drv_unsupported_option_argument call sites to use llvm::opt::Arg::getSpelling

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2022-11-08T14:39:09-08:00
New Revision: 8c2c62282fca2f3dbfa338427aa97f702e67f86b

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

LOG: [Driver] Refactor err_drv_unsupported_option_argument call sites to use 
llvm::opt::Arg::getSpelling

For `-foo=bar`, getSpelling return `-foo=` which is exactly what we need from
the diagnostic. Drop `-` from the err_drv_unsupported_option_argument template.
This change makes `--` long option diagnostics more convenient.

Reviewed By: lenary

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

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/lib/Driver/Driver.cpp
clang/lib/Driver/SanitizerArgs.cpp
clang/lib/Driver/ToolChains/Arch/AArch64.cpp
clang/lib/Driver/ToolChains/Arch/ARM.cpp
clang/lib/Driver/ToolChains/Arch/Mips.cpp
clang/lib/Driver/ToolChains/Arch/RISCV.cpp
clang/lib/Driver/ToolChains/Arch/X86.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/CommonArgs.cpp
clang/lib/Driver/ToolChains/Flang.cpp
clang/lib/Driver/ToolChains/Gnu.cpp
clang/lib/Driver/ToolChains/Hexagon.cpp
clang/lib/Driver/ToolChains/MSP430.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/Driver/mingw-cfguard.c
flang/lib/Frontend/CompilerInvocation.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 44c277b3c3d8..12c246b32cfd 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -19,7 +19,7 @@ def err_drv_unsupported_opt_for_target : Error<
 def err_drv_unsupported_opt_for_language_mode : Error<
   "unsupported option '%0' for language mode '%1'">;
 def err_drv_unsupported_option_argument : Error<
-  "unsupported argument '%1' to option '-%0'">;
+  "unsupported argument '%1' to option '%0'">;
 def err_drv_unknown_stdin_type : Error<
   "-E or -x required when input is from standard input">;
 def err_drv_unknown_stdin_type_clang_cl : Error<

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 27e15b983db6..a45c9e7f4957 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -703,7 +703,7 @@ static driver::LTOKind parseLTOMode(Driver , const 
llvm::opt::ArgList ,
 
   if (LTOMode == LTOK_Unknown) {
 D.Diag(diag::err_drv_unsupported_option_argument)
-<< A->getOption().getName() << A->getValue();
+<< A->getSpelling() << A->getValue();
 return LTOK_None;
   }
   return LTOMode;
@@ -735,7 +735,7 @@ Driver::OpenMPRuntimeKind Driver::getOpenMPRuntime(const 
ArgList ) const {
   if (RT == OMPRT_Unknown) {
 if (A)
   Diag(diag::err_drv_unsupported_option_argument)
-  << A->getOption().getName() << A->getValue();
+  << A->getSpelling() << A->getValue();
 else
   // FIXME: We could use a nicer diagnostic here.
   Diag(diag::err_drv_unsupported_opt) << "-fopenmp";

diff  --git a/clang/lib/Driver/SanitizerArgs.cpp 
b/clang/lib/Driver/SanitizerArgs.cpp
index a710081fa2d4..e88cfaed1c4e 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -249,7 +249,7 @@ static SanitizerMask parseSanitizeTrapArgs(const Driver ,
 SanitizerSet S;
 S.Mask = InvalidValues;
 D.Diag(diag::err_drv_unsupported_option_argument)
-<< Arg->getOption().getName() << toString(S);
+<< Arg->getSpelling() << toString(S);
   }
   TrappingKinds |= expandSanitizerGroups(Add) & ~TrapRemove;
 } else if (Arg->getOption().matches(options::OPT_fno_sanitize_trap_EQ)) {
@@ -604,7 +604,7 @@ SanitizerArgs::SanitizerArgs(const ToolChain ,
 SetToDiagnose.Mask |= KindsToDiagnose;
 if (DiagnoseErrors)
   D.Diag(diag::err_drv_unsupported_option_argument)
-  << Arg->getOption().getName() << toString(SetToDiagnose);
+  << Arg->getSpelling() << toString(SetToDiagnose);
 DiagnosedUnrecoverableKinds |= KindsToDiagnose;
   }
   RecoverableKinds |= expandSanitizerGroups(Add);
@@ -619,7 +619,7 @@ SanitizerArgs::SanitizerArgs(const ToolChain ,
 SetToDiagnose.Mask |= KindsToDiagnose;
 if (DiagnoseErrors)
   D.Diag(diag::err_drv_unsupported_option_argument)
-  << Arg->getOption().getName() << toString(SetToDiagnose);
+  << Arg->getSpelling() << toString(SetToDiagnose);
 DiagnosedAlwaysRecoverableKinds |= KindsToDiagnose;
   }
   RecoverableKinds &= ~expandSanitizerGroups(Remove);
@@ -951,7 +951,7 @@ SanitizerArgs::SanitizerArgs(const ToolChain ,
   auto parsedAsanDtorKind = 

[PATCH] D133757: [clangd] Perform system include extraction inside CommandMangler

[Alan Zhao via Phabricator via cfe-commits]

ayzhao added a comment.

This change is causing system-include-extractor.test 

 to fail on Chrome: https://crbug.com/1382508

The test fails on this assertion 

 if the path to the LLVM repository contains the plus sign (+). The actual 
output replaces the plus sign with the URL escape code %2B, but the still 
expects the plus sign in the output.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133757

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

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 474080.
cor3ntin marked 4 inline comments as done.
cor3ntin added a comment.

Address Aaron's comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137244

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/SemaLambda.h
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Sema/ScopeInfo.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/cxx20-decomposition.cpp

Index: clang/test/SemaCXX/cxx20-decomposition.cpp
===
--- clang/test/SemaCXX/cxx20-decomposition.cpp
+++ clang/test/SemaCXX/cxx20-decomposition.cpp
@@ -1,5 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify -Wunused-variable %s
 
 template 
 constexpr bool is_same = false;
@@ -80,10 +79,21 @@
 namespace std {
 
 template 
-struct tuple_size {
+struct tuple_size;
+
+template 
+struct tuple_size : tuple_size{};
+
+template 
+requires requires { tuple_size::value; }
+struct tuple_size : tuple_size{};
+
+template <>
+struct tuple_size {
   static constexpr unsigned long value = 2;
 };
 
+
 template 
 struct tuple_element;
 
@@ -139,3 +149,37 @@
 };
   }
 }
+
+namespace ODRUseTests {
+  struct P { int a; int b; };
+  void GH57826() {
+const auto [a, b] = P{1, 2}; //expected-note 2{{'b' declared here}} \
+ //expected-note 3{{'a' declared here}}
+(void)[&](auto c) { return b + [] {
+return a;
+}() ; }(0);
+(void)[&](auto c) { return b + [](auto) {
+return a;
+}(0) ; }(0);
+(void)[=](auto c) { return b + [](auto) {
+return a;
+}(0) ; }(0);
+(void)[,](auto c) { return b + [](auto) {
+return a;
+}(0) ; }(0);
+(void)[,](auto c) { return b + [a](auto) {
+return a;
+}(0) ; }(0);
+(void)[](auto c) { return b + [](auto) { // expected-error 2{{variable 'b' cannot be implicitly captured}} \
+ // expected-note 2{{lambda expression begins here}} \
+ // expected-note 4{{capture 'b'}}
+return a;
+}(0) ; }(0); // expected-note {{in instantiation}}
+(void)[](auto c) { return b + [](auto) {   // expected-note 3{{lambda expression begins here}} \
+ // expected-note 6{{capture 'a'}} \
+ // expected-note 6{{default capture}} \
+ // expected-note {{in instantiation}}
+return a;  // expected-error 3{{variable 'a' cannot be implicitly captured}}
+}(0) ; }(0); // expected-note 2{{in instantiation}}
+  }
+}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -13323,9 +13323,8 @@
 }
 
 // Transform the captured variable.
-VarDecl *CapturedVar
-  = cast_or_null(getDerived().TransformDecl(C->getLocation(),
- C->getCapturedVar()));
+ValueDecl *CapturedVar = cast_or_null(
+getDerived().TransformDecl(C->getLocation(), C->getCapturedVar()));
 if (!CapturedVar || CapturedVar->isInvalidDecl()) {
   Invalid = true;
   continue;
Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -62,7 +62,7 @@
 static inline Optional
 getStackIndexOfNearestEnclosingCaptureReadyLambda(
 ArrayRef FunctionScopes,
-VarDecl *VarToCapture) {
+ValueDecl *VarToCapture) {
   // Label failure to capture.
   const Optional NoLambdaIsCaptureReady;
 
@@ -172,7 +172,7 @@
 
 Optional clang::getStackIndexOfNearestEnclosingCaptureCapableLambda(
 ArrayRef FunctionScopes,
-VarDecl *VarToCapture, Sema ) {
+ValueDecl *VarToCapture, Sema ) {
 
   const Optional NoLambdaIsCaptureCapable;
 
@@ -1231,11 +1231,7 @@
 if (Var->isInvalidDecl())
   continue;
 
-VarDecl *Underlying;
-if (auto *BD = dyn_cast(Var))
-  Underlying = dyn_cast(BD->getDecomposedDecl());
-else
-  Underlying = cast(Var);
+VarDecl *Underlying = Var->getPotentiallyDecomposedVarDecl();
 
 if (!Underlying->hasLocalStorage()) {
   Diag(C->Loc, diag::err_capture_non_automatic_variable) << C->Id;
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -8223,7 +8223,7 @@
   

[PATCH] D137669: clang/cmake: Require pre-built test dependencies for stand-alone builds

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added subscribers: smeenai, mstorsjo.
mstorsjo added a comment.

This does, somewhat, coincide with what I'm trying to do in D131052 
. There, I don't point out the binaries for 
`FileCheck` and similar, but point out preexisting native `llvm-tblgen`, 
`clang-tblgen` etc (to avoid needing to build them in a nested native cmake 
when cross compiling) - but if they exist, they would probably be in the same 
directory. Would it make sense to try to settle on a common variable name for 
both of these? Or is there a case where we specifically need to be able to 
differentiate between the two?

CC @smeenai

(I haven't gotten around to refreshing that patch yet, unfortunately.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137669

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

[PATCH] D136723: [include-cleaner] Record main-file macro occurences and includes

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

(and sorry, it seems I was sitting on a pile of comments I thought I'd sent, 
LMK if I should follow up on them)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136723

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

[PATCH] D136723: [include-cleaner] Record main-file macro occurences and includes

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D136723#3915853 , @maryammo wrote:

> This commit causes build failure on  
> `https://lab.llvm.org/buildbot/#/builders/121/builds/24947`  : 
> I was able to reproduce the failure and by reverting this commit locally it 
> passed, can you please take a look? @sammccall

It looks like this was already fixed in
https://github.com/llvm/llvm-project/commit/d19ba74dee0b9ab553bd8a6ef5b67ff349f4bf13




Comment at: 
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h:81
+llvm::StringMap> BySpelling;
+llvm::DenseMap> ByFile;
+  } Includes;

kadircet wrote:
> it feels like having a set of includes corresponding to the same fileentry, 
> rather than a single one, feels subtle enough to deserve a comment.
> 
> the only case i can think of is, two logically different files resolving to 
> same physical file (i.e. symlinks). these will actually have completely 
> different spellings, and probably it'll make things hard when deciding which 
> include to "keep" or "insert". are there other cases where we can have 
> multiple includes corresponding to the same fileentry?
Sure:
```
#include "foo.h"
#include "bar.h"
#include "foo.h"
```

it isn't useful, but people write it (by mistake).
Maybe we want to clean it up, maybe we don't, but I don't see much reason to 
model it wrong on purpose - it doesn't really simplify anything, and may 
further confuse a confusing situation.

I can add a comment - what do you want it to say?
"There may be more than one #include of the same file" just seems to restate 
the obvious interpretation of the type here. (And this is the header, so it's 
not an ideal place to dump a bunch of implementation comments)



Comment at: 
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h:54
+struct RecordedPP {
+  // The callback (when installed into clang) tracks macros/includes in this.
+  std::unique_ptr record(const Preprocessor );

kadircet wrote:
> s/this/the main file/
"this" as in the current object, as the data flow here isn't obvious. Changed 
to `*this`, clearer?



Comment at: 
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h:75
+//  - for a logical file like , we check Spelled
+llvm::SmallVector match(Header H) const;
+

kadircet wrote:
> hokein wrote:
> > sammccall wrote:
> > > in the prototype I reimplemented this function in clangd, but I expect we 
> > > can just reuse the RecordedIncludes class instead, copying from clangd's 
> > > includes list is cheap.
> > > 
> > > (That's one argument for putting this in a different header, which I can 
> > > do already if you prefer)
> > that's an interesting idea, but we need to do it carefully because of 
> > `FileEntry*`, the `Include` structure has a filed of `FileEntry*`, it is 
> > not feasible for clangd to propagate it (clangd's `Inclusion` doesn't have 
> > it, instead it uses a `HeaderID` which is based on the `fs::UniqueID` to 
> > identify a physical file).
> > 
> > But I think this is not something we should worry about at the moment, and 
> > the current interfaces look quite good.
> right, let's keep it to the minimum now. we can extend/move-around later on
Good point. We could give Include a UniqueID instead I think. Let's defer this 
though.



Comment at: 
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h:75
+//  - for a logical file like , we check Spelled
+llvm::SmallVector match(Header H) const;
+

sammccall wrote:
> kadircet wrote:
> > hokein wrote:
> > > sammccall wrote:
> > > > in the prototype I reimplemented this function in clangd, but I expect 
> > > > we can just reuse the RecordedIncludes class instead, copying from 
> > > > clangd's includes list is cheap.
> > > > 
> > > > (That's one argument for putting this in a different header, which I 
> > > > can do already if you prefer)
> > > that's an interesting idea, but we need to do it carefully because of 
> > > `FileEntry*`, the `Include` structure has a filed of `FileEntry*`, it is 
> > > not feasible for clangd to propagate it (clangd's `Inclusion` doesn't 
> > > have it, instead it uses a `HeaderID` which is based on the 
> > > `fs::UniqueID` to identify a physical file).
> > > 
> > > But I think this is not something we should worry about at the moment, 
> > > and the current interfaces look quite good.
> > right, let's keep it to the minimum now. we can extend/move-around later on
> Good point. We could give Include a UniqueID instead I think. Let's defer 
> this though.
> that's an interesting idea, but we need to do it carefully because of 
> `FileEntry*`, the `Include` structure has a filed of `FileEntry*`, it is not 
> feasible for clangd to propagate it (clangd's `Inclusion` doesn't have it, 
> instead it uses a `HeaderID` which is based on the 

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

[Noah Goldstein via Phabricator via cfe-commits]

goldstein.w.n added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.h:47
 
+  /// The preprocessor indent level of the \c UnwrappedLine.
+  int PPLevel;

sstwcw wrote:
> Please elaborate what preprocessor indent level means.
Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181

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

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

[Noah Goldstein via Phabricator via cfe-commits]

goldstein.w.n updated this revision to Diff 474079.
goldstein.w.n added a comment.



1. Updating D137181 : [clang-format] Don't 
use 'PPIndentWidth' inside multi-line macros #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

Elaborate in comments for PPLevel


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181

Files:
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5040,6 +5040,254 @@
"  int y = 0;\n"
"}",
style);
+
+  style.IndentPPDirectives = FormatStyle::PPDIS_None;
+  verifyFormat("#ifdef X\n"
+   "#define Y  \\\n"
+   "switch (Y) {   \\\n"
+   "case 0:\\\n"
+   "break; \\\n"
+   "case 1:\\\n"
+   "break; \\\n"
+   "}  \\\n"
+   "Z();\n"
+   "#endif",
+   style);
+  verifyFormat("#ifdef foo\n"
+   "#define bar() \\\n"
+   "if (A) {  \\\n"
+   "B();  \\\n"
+   "} \\\n"
+   "C();\n"
+   "#endif",
+   style);
+  verifyFormat("if (emacs) {\n"
+   "#ifdef is\n"
+   "#define lit   \\\n"
+   "if (af) { \\\n"
+   "return duh(); \\\n"
+   "}\n"
+   "#endif\n"
+   "}",
+   style);
+  verifyFormat("#if abc\n"
+   "#ifdef foo\n"
+   "#define bar()\\\n"
+   "if (A) { \\\n"
+   "if (B) { \\\n"
+   "C(); \\\n"
+   "}\\\n"
+   "}\\\n"
+   "D();\n"
+   "#endif\n"
+   "#endif",
+   style);
+  verifyFormat("#ifndef foo\n"
+   "#define foo\n"
+   "if (emacs) {\n"
+   "#ifdef is\n"
+   "#define lit   \\\n"
+   "if (af) { \\\n"
+   "return duh(); \\\n"
+   "}\n"
+   "#endif\n"
+   "}\n"
+   "#endif",
+   style);
+
+  style.PPIndentWidth = 2;
+  verifyFormat("#ifdef foo\n"
+   "#define bar() \\\n"
+   "if (A) {  \\\n"
+   "B();  \\\n"
+   "} \\\n"
+   "C();\n"
+   "#endif",
+   style);
+  style.IndentWidth = 8;
+  verifyFormat("#ifdef foo\n"
+   "#define bar()\\\n"
+   "if (A) { \\\n"
+   "B(); \\\n"
+   "}\\\n"
+   "C();\n"
+   "#endif",
+   style);
+
+  style.IndentWidth = 4;
+  style.PPIndentWidth = 1;
+
+  style.IndentPPDirectives = FormatStyle::PPDIS_AfterHash;
+  verifyFormat("#ifdef X\n"
+   "# define Y  \\\n"
+   " switch (Y) {   \\\n"
+   " case 0:\\\n"
+   " break; \\\n"
+   " case 1:\\\n"
+   " break; \\\n"
+   " }  \\\n"
+   " Z();\n"
+   "#endif",
+   style);
+  verifyFormat("#ifdef foo\n"
+   "# define bar() \\\n"
+   " if (A) {  \\\n"
+   " B();  \\\n"
+   " } \\\n"
+   " C();\n"
+   "#endif",
+   style);
+  verifyFormat("if (emacs) {\n"
+   "#ifdef is\n"
+   "# define lit   \\\n"
+   " if (af) { \\\n"
+   " return duh(); \\\n"
+   " }\n"
+   "#endif\n"
+   "}",
+   style);
+  verifyFormat("#if abc\n"
+   "# ifdef foo\n"
+   "#  define bar()\\\n"
+   "  if (A) { \\\n"
+   "  if (B) { \\\n"
+   "  C(); \\\n"
+   "  }\\\n"
+   "  }\\\n"
+   "  D();\n"
+   "# endif\n"
+   

[PATCH] D137669: clang/cmake: Require pre-built test dependencies for stand-alone builds

[Tom Stellard via Phabricator via cfe-commits]

tstellar created this revision.
tstellar added reviewers: phosek, mgorny, Ericson2314.
Herald added a project: All.
tstellar requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

If the FileCheck, count, and not tools are not found in
LLVM_TOOLS_BINARY_DIR, then tests will be disabled.  CMake will
no longer try to build these tool from the llvm sources.

In theory, since stand-alone builds are meant to be done without
access to the llvm sources, this should be a no-op, but this
change will affect anyone trying to do stand-alone builds with
the full monorepo sources.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137669

Files:
  clang/CMakeLists.txt


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -86,34 +86,26 @@
   set(LLVM_UTILS_PROVIDED ON)
 endif()
 
-if(EXISTS ${LLVM_MAIN_SRC_DIR}/utils/lit/lit.py)
-  # Note: path not really used, except for checking if lit was found
-  set(LLVM_LIT ${LLVM_MAIN_SRC_DIR}/utils/lit/lit.py)
-  if(EXISTS ${LLVM_MAIN_SRC_DIR}/utils/llvm-lit)
-add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/llvm-lit utils/llvm-lit)
+# Seek installed Lit.
+find_program(LLVM_LIT
+ NAMES llvm-lit lit.py lit
+ PATHS "${LLVM_MAIN_SRC_DIR}/utils/lit"
+ DOC "Path to lit.py")
+
+if (LLVM_LIT AND LLVM_UTILS_PROVIDED)
+  if(EXISTS ${LLVM_MAIN_SRC_DIR}/utils/lit/lit.py)
+# Note: path not really used, except for checking if lit was found
+if(EXISTS ${LLVM_MAIN_SRC_DIR}/utils/llvm-lit)
+  add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/llvm-lit utils/llvm-lit)
+endif()
+set(UNITTEST_DIR ${LLVM_MAIN_SRC_DIR}/utils/unittest)
+if(EXISTS ${UNITTEST_DIR}/googletest/include/gtest/gtest.h
+AND NOT EXISTS 
${LLVM_LIBRARY_DIR}/${CMAKE_STATIC_LIBRARY_PREFIX}gtest${CMAKE_STATIC_LIBRARY_SUFFIX}
+AND EXISTS ${UNITTEST_DIR}/CMakeLists.txt)
+  add_subdirectory(${UNITTEST_DIR} utils/unittest)
+endif()
   endif()
-  if(NOT LLVM_UTILS_PROVIDED)
-add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/FileCheck utils/FileCheck)
-add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/count utils/count)
-add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/not utils/not)
-set(LLVM_UTILS_PROVIDED ON)
-set(CLANG_TEST_DEPS FileCheck count not)
-  endif()
-  set(UNITTEST_DIR ${LLVM_MAIN_SRC_DIR}/utils/unittest)
-  if(EXISTS ${UNITTEST_DIR}/googletest/include/gtest/gtest.h
-  AND NOT EXISTS 
${LLVM_LIBRARY_DIR}/${CMAKE_STATIC_LIBRARY_PREFIX}gtest${CMAKE_STATIC_LIBRARY_SUFFIX}
-  AND EXISTS ${UNITTEST_DIR}/CMakeLists.txt)
-add_subdirectory(${UNITTEST_DIR} utils/unittest)
-  endif()
-else()
-  # Seek installed Lit.
-  find_program(LLVM_LIT
-   NAMES llvm-lit lit.py lit
-   PATHS "${LLVM_MAIN_SRC_DIR}/utils/lit"
-   DOC "Path to lit.py")
-endif()
 
-if(LLVM_LIT)
   # Define the default arguments to use with 'lit', and an option for the 
user
   # to override.
   set(LIT_ARGS_DEFAULT "-sv")


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -86,34 +86,26 @@
   set(LLVM_UTILS_PROVIDED ON)
 endif()
 
-if(EXISTS ${LLVM_MAIN_SRC_DIR}/utils/lit/lit.py)
-  # Note: path not really used, except for checking if lit was found
-  set(LLVM_LIT ${LLVM_MAIN_SRC_DIR}/utils/lit/lit.py)
-  if(EXISTS ${LLVM_MAIN_SRC_DIR}/utils/llvm-lit)
-add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/llvm-lit utils/llvm-lit)
+# Seek installed Lit.
+find_program(LLVM_LIT
+ NAMES llvm-lit lit.py lit
+ PATHS "${LLVM_MAIN_SRC_DIR}/utils/lit"
+ DOC "Path to lit.py")
+
+if (LLVM_LIT AND LLVM_UTILS_PROVIDED)
+  if(EXISTS ${LLVM_MAIN_SRC_DIR}/utils/lit/lit.py)
+# Note: path not really used, except for checking if lit was found
+if(EXISTS ${LLVM_MAIN_SRC_DIR}/utils/llvm-lit)
+  add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/llvm-lit utils/llvm-lit)
+endif()
+set(UNITTEST_DIR ${LLVM_MAIN_SRC_DIR}/utils/unittest)
+if(EXISTS ${UNITTEST_DIR}/googletest/include/gtest/gtest.h
+AND NOT EXISTS ${LLVM_LIBRARY_DIR}/${CMAKE_STATIC_LIBRARY_PREFIX}gtest${CMAKE_STATIC_LIBRARY_SUFFIX}
+AND EXISTS ${UNITTEST_DIR}/CMakeLists.txt)
+  add_subdirectory(${UNITTEST_DIR} utils/unittest)
+endif()
   endif()
-  if(NOT LLVM_UTILS_PROVIDED)
-add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/FileCheck utils/FileCheck)
-add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/count utils/count)
-

[PATCH] D136723: [include-cleaner] Record main-file macro occurences and includes

[Maryam Moghadas via Phabricator via cfe-commits]

maryammo added a comment.

This commit causes build failure on  
`https://lab.llvm.org/buildbot/#/builders/121/builds/24947`  :

  [43/635] Building CXX object 
tools/clang/tools/extra/include-cleaner/lib/CMakeFiles/obj.clangIncludeCleaner.dir/Types.cpp.o
  FAILED: 
tools/clang/tools/extra/include-cleaner/lib/CMakeFiles/obj.clangIncludeCleaner.dir/Types.cpp.o
 
  CCACHE_CPP2=yes CCACHE_HASHDIR=yes /usr/bin/ccache /usr/lib64/ccache/c++  
-DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS 
-D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
-Itools/clang/tools/extra/include-cleaner/lib 
-I/home/buildbots/ppc64le-clang-multistage-test/clang-ppc64le-multistage/llvm/clang-tools-extra/include-cleaner/lib
 
-I/home/buildbots/ppc64le-clang-multistage-test/clang-ppc64le-multistage/llvm/clang/include
 -Itools/clang/include -Iinclude 
-I/home/buildbots/ppc64le-clang-multistage-test/clang-ppc64le-multistage/llvm/llvm/include
 
-I/home/buildbots/ppc64le-clang-multistage-test/clang-ppc64le-multistage/llvm/clang-tools-extra/include-cleaner/include
 -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden 
-Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings 
-Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long 
-Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-class-memaccess 
-Wno-noexcept-type -Wdelete-non-virtual-dtor -Wno-comment 
-Wno-misleading-indentation -fdiagnostics-color -ffunction-sections 
-fdata-sections -fno-common -Woverloaded-virtual -fno-strict-aliasing -O3 
-DNDEBUG-fno-exceptions -fno-rtti -UNDEBUG -std=c++17 -MD -MT 
tools/clang/tools/extra/include-cleaner/lib/CMakeFiles/obj.clangIncludeCleaner.dir/Types.cpp.o
 -MF 
tools/clang/tools/extra/include-cleaner/lib/CMakeFiles/obj.clangIncludeCleaner.dir/Types.cpp.o.d
 -o 
tools/clang/tools/extra/include-cleaner/lib/CMakeFiles/obj.clangIncludeCleaner.dir/Types.cpp.o
 -c 
/home/buildbots/ppc64le-clang-multistage-test/clang-ppc64le-multistage/llvm/clang-tools-extra/include-cleaner/lib/Types.cpp
  In file included from 
/home/buildbots/ppc64le-clang-multistage-test/clang-ppc64le-multistage/llvm/clang-tools-extra/include-cleaner/lib/Types.cpp:9:
  
/home/buildbots/ppc64le-clang-multistage-test/clang-ppc64le-multistage/llvm/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h:84:10:
 error: declaration of ‘clang::include_cleaner::Symbol 
clang::include_cleaner::SymbolReference::Symbol’ [-fpermissive]
 Symbol Symbol;
^~
  
/home/buildbots/ppc64le-clang-multistage-test/clang-ppc64le-multistage/llvm/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h:51:8:
 error: changes meaning of ‘Symbol’ from ‘struct 
clang::include_cleaner::Symbol’ [-fpermissive]
   struct Symbol {
  ^~
  
/home/buildbots/ppc64le-clang-multistage-test/clang-ppc64le-multistage/llvm/clang-tools-extra/include-cleaner/lib/Types.cpp:
 In function ‘llvm::raw_ostream& 
clang::include_cleaner::operator<<(llvm::raw_ostream&, const 
clang::include_cleaner::SymbolReference&)’:
  
/home/buildbots/ppc64le-clang-multistage-test/clang-ppc64le-multistage/llvm/clang-tools-extra/include-cleaner/lib/Types.cpp:51:51:
 warning: ‘*’ in boolean context, suggest ‘&&’ instead [-Wint-in-bool-context]
  /*Width=*/CHAR_BIT *


I was able to reproduce the failure and by reverting this commit locally it 
passed, can you please take a look? @sammccall


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136723

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

[PATCH] D137659: [Driver] Refactor err_drv_unsupported_option_argument call sites to use llvm::opt::Arg::getSpelling

[Sam Elliott via Phabricator via cfe-commits]

lenary accepted this revision.
lenary added a comment.
This revision is now accepted and ready to land.

Thanks for doing this cleanup!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137659

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

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

[Manas Gupta via Phabricator via cfe-commits]

manas added a comment.

Ping


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] D134859: [clang][Interp] Implement basic support for floating point values

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added inline comments.



Comment at: clang/lib/AST/Interp/Opcodes.td:502
+  let Types = [AluTypeClass];
+  let Args = [ArgFltSemantics];
+  let HasGroup = 1;

tbaeder wrote:
> sepavloff wrote:
> > tbaeder wrote:
> > > jcranmer-intel wrote:
> > > > Integer-to-floating point conversion is dependent on rounding 
> > > > mode--consider `(float)UINT_MAX`.
> > > This test succeeds here, whether I use `-frounding-math` or not:
> > > 
> > > ```
> > > constexpr float f = (float)4294967295;
> > > static_assert(f == (float)4.2949673E+9);
> > > ```
> > > How can I test this behavior?
> > You can use `#pragma STDC FENV_ROUND`. See 
> > `clang/test/AST/const-fpfeatures.c` as an example.
> Why does it work here to compare both `f` and `f2` to the same value? 
> https://godbolt.org/z/zdsf1sK7r
`FENV_ROUND` is a new feature of C2x, so it doesn't look like it's properly 
handled in gcc or clang yet. (It's for sure not handled in clang--the code to 
convert a floating-point literal into an `APFloat` hardcodes default rounding 
mode in its call.)

Dynamic rounding mode shows you the difference: https://godbolt.org/z/856xM8KTh


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

https://reviews.llvm.org/D134859

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

[PATCH] D137473: [vfs] Allow root paths relative to the directory of the vfsoverlay YAML file

[Ben Barham via Phabricator via cfe-commits]

bnbarham added inline comments.



Comment at: clang/test/VFS/Inputs/root-relative-overlay.yaml:4
+  'case-sensitive': false,
+  'overlay-relative': true,
+  'root-relative': 'yaml-dir',

bnbarham wrote:
> I'd prefer a test without `overlay-relative` set to make it clear they don't 
> depend on each other.
There's also unit tests in `llvm/unittests/Support/VirtualFileSystemTest.cpp` 
that you could add to.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137473

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

[PATCH] D91949: [clang-format] Add BeforeStructInitialization option in BraceWrapping configuration

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks abandoned this revision.
HazardyKnusperkeks added a comment.

More than one year silence.


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

https://reviews.llvm.org/D91949

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

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a subscriber: owenpan.
HazardyKnusperkeks added a comment.

@MyDeveloperDay @owenpan What shall we do?

1. Abandon this
2. Commit it in our name
3. Use the email from this commit: 
https://github.com/Wandalen/game_chess/commit/a84bba03667643ded0cfe7aff2907c6546a6a192
 I'm pretty sure this is the correct person


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

https://reviews.llvm.org/D91950

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

[PATCH] D137473: [vfs] Allow root paths relative to the directory of the vfsoverlay YAML file

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In D137473#3915497 , @bnbarham wrote:

> This seems reasonable to me in general. @dexonsmith in case you have any 
> thoughts.

SGTM! (I haven't reviewed in detail but I figure @bnbarham is on it...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137473

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

[PATCH] D137409: [clang-format][NFCish] Alphabetical sort Format.(h|pp)

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:23323
  "WhitespaceSensitiveMacros: ['FOO', 'BAR']")));
-  std::vector NonDefaultWhiteSpaceMacros{"FOO", "BAR"};
+  std::vector NonDefaultWhiteSpaceMacros =
+  Style9->WhitespaceSensitiveMacros;

MyDeveloperDay wrote:
> related?
Sadly yes.
Because `WhiteSensitiveMacros` were not handled in `operator==` we basically 
tested nothing here.
When I wrote the tests I assumed (and still think it should be that way) that 
`WhiteSensitiveMacros` would only contain what is posted in the 
`.clang-format`. But that's not the case, the vector gets filled with whatever 
`getLLVMStyle()` does and then overwrites them in order, extending when there 
is not enough room.

That will hold for all `std::vector`s and so.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137409

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

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

[Ben Boeckel via Phabricator via cfe-commits]

ben.boeckel added a comment.

FWIW, I was able to make CMake's module test suite pass with this patch on top 
of you MyP1689 branch on your Github fork. I also added some diffs to help 
clean up output files. I'll try and get it to work with the replacement patches 
as well, but I just want this to be a working checkpoint.


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

https://reviews.llvm.org/D134267

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

[PATCH] D137206: [clang][modules][deps] Including module maps are affecting

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese accepted this revision.
Bigcheese 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/D137206/new/

https://reviews.llvm.org/D137206

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

[PATCH] D137512: [clang] Add Swift support for MIPS

[Arnold Schwaighofer via Phabricator via cfe-commits]

aschwaighofer added a comment.

This looks good to me. Can you rebase your patch so that it applies on top of 
github.com:apple/llvm-project.git main?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137512

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

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/AST/DeclCXX.cpp:3234-3237
+  if (auto *Var = llvm::dyn_cast(this))
+return Var;
+  if (auto *BD = llvm::dyn_cast(this))
+return llvm::dyn_cast(BD->getDecomposedDecl());

aaron.ballman wrote:
> No need to use the fully-qualified name (we weren't using it for the calls to 
> `isa` either).
My IDE completion being weird, sorry about that :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137244

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

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

In D137244#3915633 , @aaron.ballman 
wrote:

> Thanks for this! Can you also add a release note for the changes?

Thanks for the Review! 
As mentioned this fixes an unreleased feature, hence why I don't think a 
release note entry is necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137244

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

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thanks for this! Can you also add a release note for the changes?




Comment at: clang/include/clang/AST/Decl.h:673
 
+class VarDecl;
+

This should be moved up to around line 77 or so (and kept in alphabetical 
order).



Comment at: clang/lib/AST/DeclCXX.cpp:3234-3237
+  if (auto *Var = llvm::dyn_cast(this))
+return Var;
+  if (auto *BD = llvm::dyn_cast(this))
+return llvm::dyn_cast(BD->getDecomposedDecl());

No need to use the fully-qualified name (we weren't using it for the calls to 
`isa` either).



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:91
 bool CheckDefaultArgumentVisitor::VisitDeclRefExpr(const DeclRefExpr *DRE) {
-  const NamedDecl *Decl = DRE->getDecl();
+  const ValueDecl *Decl = dyn_cast(DRE->getDecl());
+

1) We should rename `Decl` to something that doesn't shadow the name of a type, 
since we're here.
2) Can `Decl` be null? You're assuming it can't be below with the `isa` call, 
so this could switch to `cast`.



Comment at: clang/lib/Sema/SemaExpr.cpp:18384-18387
+  if (Underlying) {
+if (Underlying->hasLocalStorage() && Diagnose)
   diagnoseUncapturableValueReferenceOrBinding(S, Loc, Var);
   }





Comment at: clang/lib/Sema/SemaExpr.cpp:19481-19483
+  const bool RefersToEnclosingScope =
+  (SemaRef.CurContext != VD->getDeclContext() &&
+   VD->getDeclContext()->isFunctionOrMethod() && VD->hasLocalStorage());





Comment at: clang/lib/Sema/SemaExpr.cpp:19485
+  if (RefersToEnclosingScope) {
+LambdaScopeInfo *const LSI =
+SemaRef.getCurLambda(/*IgnoreNonLambdaCapturingScope=*/true);





Comment at: clang/lib/Sema/SemaExpr.cpp:19489-19492
+  // If a variable could potentially be odr-used, defer marking it so
+  // until we finish analyzing the full expression for any
+  // lvalue-to-rvalue
+  // or discarded value conversions that would obviate odr-use.

Can re-flow this comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137244

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

[PATCH] D137609: [C++20] [Modules] Remove unmaintained header modules

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

I think that (if this change is approved) there will be also some 
simplifications possible in the driver (since the mode that produces a wrapper 
header for multiple command-line headers is different from the mode where 
multiple command line headers would each produce a single C++ standard header 
unit) ..


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

https://reviews.llvm.org/D137609

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

[PATCH] D137617: [AArch64] Allow users-facing feature names in clang target attributes

[Dave Green 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 rGf0e6c403c2d3: [AArch64] Allow users-facing feature names in 
clang target attributes (authored by dmgreen).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D137617?vs=473903=474060#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137617

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/test/CodeGen/aarch64-targetattr.c
  clang/test/Sema/aarch64-fp16-target.c

Index: clang/test/Sema/aarch64-fp16-target.c
===
--- clang/test/Sema/aarch64-fp16-target.c
+++ clang/test/Sema/aarch64-fp16-target.c
@@ -10,13 +10,18 @@
   vabdh_f16(f16, f16);
 }
 
+__attribute__((target("fp16")))
+void fp16(float16_t f16) {
+  vabdh_f16(f16, f16);
+}
+
 __attribute__((target("arch=armv8-a+fp16")))
 void test_fp16_arch(float16_t f16) {
 vabdh_f16(f16, f16);
 }
 
 __attribute__((target("+fp16")))
-void test_fp16(float16_t f16) {
+void test_plusfp16(float16_t f16) {
 vabdh_f16(f16, f16);
 }
 
Index: clang/test/CodeGen/aarch64-targetattr.c
===
--- clang/test/CodeGen/aarch64-targetattr.c
+++ clang/test/CodeGen/aarch64-targetattr.c
@@ -75,6 +75,20 @@
 __attribute__((target("cpu=neoverse-n1,tune=cortex-a710,arch=armv8.6-a+sve2,branch-protection=standard")))
 void allplusbranchprotection() {}
 
+// These tests check that the user facing and internal llvm name are both accepted.
+// CHECK-LABEL: @plusnoneon() #18
+__attribute__((target("+noneon")))
+void plusnoneon() {}
+// CHECK-LABEL: @plusnosimd() #18
+__attribute__((target("+nosimd")))
+void plusnosimd() {}
+// CHECK-LABEL: @noneon() #18
+__attribute__((target("no-neon")))
+void noneon() {}
+// CHECK-LABEL: @nosimd() #18
+__attribute__((target("no-simd")))
+void nosimd() {}
+
 // CHECK: attributes #0 = { {{.*}} "target-features"="+v8.1a,+v8.2a,+v8a" }
 // CHECK: attributes #1 = { {{.*}} "target-features"="+sve,+v8.1a,+v8.2a,+v8a" }
 // CHECK: attributes #2 = { {{.*}} "target-features"="+sve2,+v8.1a,+v8.2a,+v8a" }
@@ -92,3 +106,4 @@
 // CHECK: attributes #15 = { {{.*}} "target-features"="+fullfp16" }
 // CHECK: attributes #16 = { {{.*}} "target-cpu"="neoverse-n1" "target-features"="+crc,+crypto,+dotprod,+fp-armv8,+fullfp16,+lse,+neon,+ras,+rcpc,+rdm,+spe,+ssbs,+sve2,+v8.1a,+v8.2a,+v8.3a,+v8.4a,+v8.5a,+v8.6a,+v8a" "tune-cpu"="cortex-a710" }
 // CHECK: attributes #17 = { {{.*}} "branch-target-enforcement"="true" {{.*}} "target-cpu"="neoverse-n1" "target-features"="+crc,+crypto,+dotprod,+fp-armv8,+fullfp16,+lse,+neon,+ras,+rcpc,+rdm,+spe,+ssbs,+sve2,+v8.1a,+v8.2a,+v8.3a,+v8.4a,+v8.5a,+v8.6a,+v8a" "tune-cpu"="cortex-a710" }
+// CHECK: attributes #18 = { {{.*}} "target-features"="-neon" }
Index: clang/lib/Basic/Targets/AArch64.cpp
===
--- clang/lib/Basic/Targets/AArch64.cpp
+++ clang/lib/Basic/Targets/AArch64.cpp
@@ -744,7 +744,10 @@
   else
 // Pushing the original feature string to give a sema error later on
 // when they get checked.
-Features.push_back(Feature.str());
+if (Feature.startswith("no"))
+  Features.push_back("-" + Feature.drop_front(2).str());
+else
+  Features.push_back("+" + Feature.str());
 }
   };
 
@@ -792,13 +795,25 @@
 Ret.Duplicate = "tune=";
   else
 Ret.Tune = Feature.split("=").second.trim();
-} else if (Feature.startswith("no-"))
-  Ret.Features.push_back("-" + Feature.split("-").second.str());
-else if (Feature.startswith("+")) {
+} else if (Feature.startswith("+")) {
   SplitAndAddFeatures(Feature, Ret.Features);
+} else if (Feature.startswith("no-")) {
+  StringRef FeatureName =
+  llvm::AArch64::getArchExtFeature(Feature.split("-").second);
+  if (!FeatureName.empty())
+Ret.Features.push_back("-" + FeatureName.drop_front(1).str());
+  else
+Ret.Features.push_back("-" + Feature.split("-").second.str());
+} else {
+  // Try parsing the string to the internal target feature name. If it is
+  // invalid, add the original string (which could already be an internal
+  // name). These should be checked later by isValidFeatureName.
+  StringRef FeatureName = llvm::AArch64::getArchExtFeature(Feature);
+  if (!FeatureName.empty())
+Ret.Features.push_back(FeatureName.str());
+  else
+Ret.Features.push_back("+" + Feature.str());
 }
-else
-  Ret.Features.push_back("+" + Feature.str());
   }
   return Ret;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] f0e6c40 - [AArch64] Allow users-facing feature names in clang target attributes

[David Green via cfe-commits]

Author: David Green
Date: 2022-11-08T19:30:26Z
New Revision: f0e6c403c2d399e4fd821aa5a7e4a20534494c71

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

LOG: [AArch64] Allow users-facing feature names in clang target attributes

D133848 added support for the GCC format of target("..") attributes. The
supported formats to match gcc are:
//  "arch=" - parsed to features as per -march=..
//  "cpu=" - parsed to features as per -mcpu=.., with CPU set to 
//  "tune=" - TuneCPU set to 
//  "+feature", "+nofeature" - Add (or remove) feature.

We also support the existing formats, previously accepted by clang, for
compatibility with the existing code and intrinsics code:
//  "feature", "no-feature" - Add (or remove) feature.

The clang formats would accept and use internal feature names
("fullfp16"/"neon"/"sve") as opposed to the user facing names
("fp16"/"simd"/"sve"). Usually they use the same names, but can be
different for cases like fp, fullfp16 and mte (among others).

This patch makes the clang format also except the user facing names, by
parsing the features through getArchExtFeature. There is a fallback if
the name is not recognized (like "fullfp16"), where we add the existing
string which should then be checked later for consistency. This allows
the internal names to be used as before, so long as they are recognized
as internal names. (Note that we currently don't have an implementation
of isValidFeatureName. The backend will currently give an error like
"'-sid' is not a recognized feature for this target (ignoring feature)."
This should be improved in a later patch once an implementation of
isValidFeatureName in clang is present).

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

Added: 


Modified: 
clang/lib/Basic/Targets/AArch64.cpp
clang/test/CodeGen/aarch64-targetattr.c
clang/test/Sema/aarch64-fp16-target.c

Removed: 




diff  --git a/clang/lib/Basic/Targets/AArch64.cpp 
b/clang/lib/Basic/Targets/AArch64.cpp
index a2cdfda3842b0..c070462bc5d1f 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -744,7 +744,10 @@ ParsedTargetAttr 
AArch64TargetInfo::parseTargetAttr(StringRef Features) const {
   else
 // Pushing the original feature string to give a sema error later on
 // when they get checked.
-Features.push_back(Feature.str());
+if (Feature.startswith("no"))
+  Features.push_back("-" + Feature.drop_front(2).str());
+else
+  Features.push_back("+" + Feature.str());
 }
   };
 
@@ -792,13 +795,25 @@ ParsedTargetAttr 
AArch64TargetInfo::parseTargetAttr(StringRef Features) const {
 Ret.Duplicate = "tune=";
   else
 Ret.Tune = Feature.split("=").second.trim();
-} else if (Feature.startswith("no-"))
-  Ret.Features.push_back("-" + Feature.split("-").second.str());
-else if (Feature.startswith("+")) {
+} else if (Feature.startswith("+")) {
   SplitAndAddFeatures(Feature, Ret.Features);
+} else if (Feature.startswith("no-")) {
+  StringRef FeatureName =
+  llvm::AArch64::getArchExtFeature(Feature.split("-").second);
+  if (!FeatureName.empty())
+Ret.Features.push_back("-" + FeatureName.drop_front(1).str());
+  else
+Ret.Features.push_back("-" + Feature.split("-").second.str());
+} else {
+  // Try parsing the string to the internal target feature name. If it is
+  // invalid, add the original string (which could already be an internal
+  // name). These should be checked later by isValidFeatureName.
+  StringRef FeatureName = llvm::AArch64::getArchExtFeature(Feature);
+  if (!FeatureName.empty())
+Ret.Features.push_back(FeatureName.str());
+  else
+Ret.Features.push_back("+" + Feature.str());
 }
-else
-  Ret.Features.push_back("+" + Feature.str());
   }
   return Ret;
 }

diff  --git a/clang/test/CodeGen/aarch64-targetattr.c 
b/clang/test/CodeGen/aarch64-targetattr.c
index 0f1f7da54625e..bac132f7ef342 100644
--- a/clang/test/CodeGen/aarch64-targetattr.c
+++ b/clang/test/CodeGen/aarch64-targetattr.c
@@ -75,6 +75,20 @@ void all() {}
 
__attribute__((target("cpu=neoverse-n1,tune=cortex-a710,arch=armv8.6-a+sve2,branch-protection=standard")))
 void allplusbranchprotection() {}
 
+// These tests check that the user facing and internal llvm name are both 
accepted.
+// CHECK-LABEL: @plusnoneon() #18
+__attribute__((target("+noneon")))
+void plusnoneon() {}
+// CHECK-LABEL: @plusnosimd() #18
+__attribute__((target("+nosimd")))
+void plusnosimd() {}
+// CHECK-LABEL: @noneon() #18
+__attribute__((target("no-neon")))
+void noneon() {}
+// CHECK-LABEL: @nosimd() #18
+__attribute__((target("no-simd")))
+void nosimd() {}
+
 // CHECK: 

[PATCH] D137268: [clang][Headers] Do not define varargs macros for __need___va_list

[Adhemerval Zanella via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9e956995db1f: [clang][Headers] Do not define varargs macros 
for __need___va_list (authored by zatrazz).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137268

Files:
  clang/lib/Headers/stdarg.h
  compiler-rt/test/sanitizer_common/TestCases/Linux/signal_send.cpp


Index: compiler-rt/test/sanitizer_common/TestCases/Linux/signal_send.cpp
===
--- compiler-rt/test/sanitizer_common/TestCases/Linux/signal_send.cpp
+++ compiler-rt/test/sanitizer_common/TestCases/Linux/signal_send.cpp
@@ -5,6 +5,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
Index: clang/lib/Headers/stdarg.h
===
--- clang/lib/Headers/stdarg.h
+++ clang/lib/Headers/stdarg.h
@@ -8,8 +8,16 @@
  */
 
 #ifndef __STDARG_H
-#define __STDARG_H
 
+#ifndef __GNUC_VA_LIST
+#define __GNUC_VA_LIST
+typedef __builtin_va_list __gnuc_va_list;
+#endif
+
+#ifdef __need___va_list
+#undef __need___va_list
+#else
+#define __STDARG_H
 #ifndef _VA_LIST
 typedef __builtin_va_list va_list;
 #define _VA_LIST
@@ -29,9 +37,6 @@
 #define va_copy(dest, src)  __builtin_va_copy(dest, src)
 #endif
 
-#ifndef __GNUC_VA_LIST
-#define __GNUC_VA_LIST 1
-typedef __builtin_va_list __gnuc_va_list;
-#endif
-
 #endif /* __STDARG_H */
+
+#endif /* not __STDARG_H */


Index: compiler-rt/test/sanitizer_common/TestCases/Linux/signal_send.cpp
===
--- compiler-rt/test/sanitizer_common/TestCases/Linux/signal_send.cpp
+++ compiler-rt/test/sanitizer_common/TestCases/Linux/signal_send.cpp
@@ -5,6 +5,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
Index: clang/lib/Headers/stdarg.h
===
--- clang/lib/Headers/stdarg.h
+++ clang/lib/Headers/stdarg.h
@@ -8,8 +8,16 @@
  */
 
 #ifndef __STDARG_H
-#define __STDARG_H
 
+#ifndef __GNUC_VA_LIST
+#define __GNUC_VA_LIST
+typedef __builtin_va_list __gnuc_va_list;
+#endif
+
+#ifdef __need___va_list
+#undef __need___va_list
+#else
+#define __STDARG_H
 #ifndef _VA_LIST
 typedef __builtin_va_list va_list;
 #define _VA_LIST
@@ -29,9 +37,6 @@
 #define va_copy(dest, src)  __builtin_va_copy(dest, src)
 #endif
 
-#ifndef __GNUC_VA_LIST
-#define __GNUC_VA_LIST 1
-typedef __builtin_va_list __gnuc_va_list;
-#endif
-
 #endif /* __STDARG_H */
+
+#endif /* not __STDARG_H */
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 9e95699 - [clang][Headers] Do not define varargs macros for __need___va_list

[Adhemerval Zanella via cfe-commits]

Author: Adhemerval Zanella
Date: 2022-11-08T16:29:35-03:00
New Revision: 9e956995db1fc7e792e3dfb3a465a52626195557

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

LOG: [clang][Headers] Do not define varargs macros for __need___va_list

The glibc uses the define to avoid namespace polution on headers
that requires variadic argument, where the inclusion of stdarg.h is
required to obtain the va_list definition.

For such cases only __gnuc_va_list is required.

Reviewed By: MaskRay

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

Added: 


Modified: 
clang/lib/Headers/stdarg.h
compiler-rt/test/sanitizer_common/TestCases/Linux/signal_send.cpp

Removed: 




diff  --git a/clang/lib/Headers/stdarg.h b/clang/lib/Headers/stdarg.h
index dc7becff670f4..4fbfe0985a160 100644
--- a/clang/lib/Headers/stdarg.h
+++ b/clang/lib/Headers/stdarg.h
@@ -8,8 +8,16 @@
  */
 
 #ifndef __STDARG_H
-#define __STDARG_H
 
+#ifndef __GNUC_VA_LIST
+#define __GNUC_VA_LIST
+typedef __builtin_va_list __gnuc_va_list;
+#endif
+
+#ifdef __need___va_list
+#undef __need___va_list
+#else
+#define __STDARG_H
 #ifndef _VA_LIST
 typedef __builtin_va_list va_list;
 #define _VA_LIST
@@ -29,9 +37,6 @@ typedef __builtin_va_list va_list;
 #define va_copy(dest, src)  __builtin_va_copy(dest, src)
 #endif
 
-#ifndef __GNUC_VA_LIST
-#define __GNUC_VA_LIST 1
-typedef __builtin_va_list __gnuc_va_list;
-#endif
-
 #endif /* __STDARG_H */
+
+#endif /* not __STDARG_H */

diff  --git a/compiler-rt/test/sanitizer_common/TestCases/Linux/signal_send.cpp 
b/compiler-rt/test/sanitizer_common/TestCases/Linux/signal_send.cpp
index 84084b9291a70..f5dcc4bc3208c 100644
--- a/compiler-rt/test/sanitizer_common/TestCases/Linux/signal_send.cpp
+++ b/compiler-rt/test/sanitizer_common/TestCases/Linux/signal_send.cpp
@@ -5,6 +5,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 



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

[PATCH] D137659: [Driver] Refactor err_drv_unsupported_option_argument call sites to use llvm::opt::Arg::getSpelling

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added a reviewer: lenary.
Herald added subscribers: StephenFan, frasercrmck, luismarques, apazos, 
sameer.abuasal, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, 
MartinMosbeck, rogfer01, atanasyan, edward-jones, zzheng, jrtc27, niosHD, 
sabuasal, simoncook, johnrusso, rbar, asb, sdardis.
Herald added projects: Flang, All.
MaskRay requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, jdoerfert.
Herald added a project: clang.

For `-foo=bar`, getSpelling return `-foo=` which is exactly what we need from
the diagnostic. Drop `-` from the err_drv_unsupported_option_argument template.
This change makes `--` long option diagnostics more convenient.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137659

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/lib/Driver/ToolChains/Arch/Mips.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/X86.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Hexagon.cpp
  clang/lib/Driver/ToolChains/MSP430.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/mingw-cfguard.c
  flang/lib/Frontend/CompilerInvocation.cpp

Index: flang/lib/Frontend/CompilerInvocation.cpp
===
--- flang/lib/Frontend/CompilerInvocation.cpp
+++ flang/lib/Frontend/CompilerInvocation.cpp
@@ -688,7 +688,7 @@
   fpContractMode = LangOptions::FPM_Fast;
 else {
   diags.Report(clang::diag::err_drv_unsupported_option_argument)
-  << a->getOption().getName() << val;
+  << a->getSpelling() << val;
   return false;
 }
 
Index: clang/test/Driver/mingw-cfguard.c
===
--- clang/test/Driver/mingw-cfguard.c
+++ clang/test/Driver/mingw-cfguard.c
@@ -25,4 +25,4 @@
 // GUARD_NOCHECKS-NOT: "--no-guard-cf"
 
 // RUN: %clang --target=x86_64-w64-windows-gnu -### %s -mguard=xxx 2>&1 | FileCheck -check-prefix=GUARD_UNKNOWN %s
-// GUARD_UNKNOWN: error: unsupported argument 'xxx' to option '--mguard='
+// GUARD_UNKNOWN: error: unsupported argument 'xxx' to option '-mguard='
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1711,7 +1711,7 @@
 StringRef Value = A->getValue();
 if (Value != "simple" && Value != "mangled")
   Diags.Report(diag::err_drv_unsupported_option_argument)
-  << A->getOption().getName() << A->getValue();
+  << A->getSpelling() << A->getValue();
 Opts.setDebugSimpleTemplateNames(
 StringRef(A->getValue()) == "simple"
 ? codegenoptions::DebugTemplateNamesKind::Simple
Index: clang/lib/Driver/ToolChains/MSP430.cpp
===
--- clang/lib/Driver/ToolChains/MSP430.cpp
+++ clang/lib/Driver/ToolChains/MSP430.cpp
@@ -101,7 +101,7 @@
 Features.push_back("+hwmultf5");
   } else {
 D.Diag(clang::diag::err_drv_unsupported_option_argument)
-<< HWMultArg->getOption().getName() << HWMult;
+<< HWMultArg->getSpelling() << HWMult;
   }
 }
 
Index: clang/lib/Driver/ToolChains/Hexagon.cpp
===
--- clang/lib/Driver/ToolChains/Hexagon.cpp
+++ clang/lib/Driver/ToolChains/Hexagon.cpp
@@ -40,7 +40,7 @@
 StringRef Val = A->getValue();
 if (!Val.equals_insensitive("64b") && !Val.equals_insensitive("128b"))
   D.Diag(diag::err_drv_unsupported_option_argument)
-  << A->getOption().getName() << Val;
+  << A->getSpelling() << Val;
   }
 }
 
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -729,7 +729,7 @@
 Args.MakeArgString("--compress-debug-sections=" + Twine(Value)));
   } else {
 D.Diag(diag::err_drv_unsupported_option_argument)
-<< A->getOption().getName() << Value;
+<< A->getSpelling() << Value;
   }
 }
   }
Index: clang/lib/Driver/ToolChains/Flang.cpp
===
--- clang/lib/Driver/ToolChains/Flang.cpp
+++ clang/lib/Driver/ToolChains/Flang.cpp
@@ -104,7 +104,7 @@
   // Clang's "fast-honor-pragmas" option is not supported because it is
   // non-standard
   D.Diag(diag::err_drv_unsupported_option_argument)
-  << A->getOption().getName() << Val;
+  << 

[PATCH] D137198: [clang][modules][deps] Parent module maps are affecting

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese accepted this revision.
Bigcheese 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/D137198/new/

https://reviews.llvm.org/D137198

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

[clang-tools-extra] c333b92 - [include-cleaner] Missing changes from 38cccb906603

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-11-08T20:18:10+01:00
New Revision: c333b92b72b40d1f39d89ab94f764baeb8bb0cab

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

LOG: [include-cleaner] Missing changes from 38cccb906603

Added: 


Modified: 
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
clang-tools-extra/include-cleaner/lib/Analysis.cpp
clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp

Removed: 




diff  --git 
a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h 
b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
index 4c5c90e769ae..62277199871b 100644
--- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
+++ b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
@@ -28,7 +28,7 @@ namespace include_cleaner {
 /// that symbol may be provided by several headers.
 /// FIXME: Provide signals about the providing headers so the caller can filter
 /// and rank the results.
-using UsedSymbolCB = llvm::function_ref 
Providers)>;
 
 /// Find and report all references to symbols in a region of code.
@@ -44,8 +44,9 @@ using UsedSymbolCB = llvm::function_ref ASTRoots,
-  llvm::ArrayRef MacroRefs, UsedSymbolCB CB);
+void walkUsed(llvm::ArrayRef ASTRoots,
+  llvm::ArrayRef MacroRefs, const SourceManager &,
+  UsedSymbolCB CB);
 
 } // namespace include_cleaner
 } // namespace clang

diff  --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp 
b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
index b4dfd129c60d..f7c8bdeee0bf 100644
--- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -28,8 +28,9 @@ toHeader(llvm::ArrayRef Headers) {
 }
 } // namespace
 
-void walkUsed(const SourceManager , llvm::ArrayRef ASTRoots,
-  llvm::ArrayRef MacroRefs, UsedSymbolCB CB) {
+void walkUsed(llvm::ArrayRef ASTRoots,
+  llvm::ArrayRef MacroRefs,
+  const SourceManager , UsedSymbolCB CB) {
   tooling::stdlib::Recognizer Recognizer;
   for (auto *Root : ASTRoots) {
 auto  = Root->getASTContext().getSourceManager();

diff  --git a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp 
b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
index bd794037e85a..49652ad35023 100644
--- a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -50,9 +50,9 @@ TEST(WalkUsed, Basic) {
 
   auto  = AST.sourceManager();
   llvm::DenseMap> OffsetToProviders;
-  walkUsed(SM, TopLevelDecls, /*MacroRefs=*/{},
-   [&](SymbolReference SymRef, llvm::ArrayRef Providers) {
- auto [FID, Offset] = SM.getDecomposedLoc(SymRef.RefLocation);
+  walkUsed(TopLevelDecls, /*MacroRefs=*/{}, SM,
+   [&](const SymbolReference , llvm::ArrayRef Providers) {
+ auto [FID, Offset] = SM.getDecomposedLoc(Ref.RefLocation);
  EXPECT_EQ(FID, SM.getMainFileID());
  OffsetToProviders.try_emplace(Offset, Providers.vec());
});
@@ -89,11 +89,12 @@ TEST(WalkUsed, MacroRefs) {
   Symbol Answer =
   Macro{("ANSWER"), SM.getComposedLoc(HdrID, Hdr.point())};
   llvm::DenseMap> OffsetToProviders;
-  walkUsed(SM, /*ASTRoots=*/{}, /*MacroRefs=*/
+  walkUsed(/*ASTRoots=*/{}, /*MacroRefs=*/
{SymbolReference{SM.getComposedLoc(SM.getMainFileID(), 
Main.point()),
 Answer, RefType::Explicit}},
-   [&](SymbolReference SymRef, llvm::ArrayRef Providers) {
- auto [FID, Offset] = SM.getDecomposedLoc(SymRef.RefLocation);
+   SM,
+   [&](const SymbolReference , llvm::ArrayRef Providers) {
+ auto [FID, Offset] = SM.getDecomposedLoc(Ref.RefLocation);
  EXPECT_EQ(FID, SM.getMainFileID());
  OffsetToProviders.try_emplace(Offset, Providers.vec());
});



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

[PATCH] D137644: [include-cleaner] pass through recorded macro refs in walkUsed

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rG38cccb906603: [include-cleaner] pass through recorded macro 
refs in walkUsed (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137644

Files:
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
  clang-tools-extra/include-cleaner/lib/Analysis.cpp
  clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -50,7 +50,7 @@
 
   auto  = AST.sourceManager();
   llvm::DenseMap> OffsetToProviders;
-  walkUsed(TopLevelDecls,
+  walkUsed(SM, TopLevelDecls, /*MacroRefs=*/{},
[&](SymbolReference SymRef, llvm::ArrayRef Providers) {
  auto [FID, Offset] = SM.getDecomposedLoc(SymRef.RefLocation);
  EXPECT_EQ(FID, SM.getMainFileID());
@@ -68,5 +68,40 @@
   Pair(Code.point("vconstructor"), UnorderedElementsAre(VectorSTL;
 }
 
+TEST(WalkUsed, MacroRefs) {
+  llvm::Annotations Hdr(R"cpp(
+#define ^ANSWER 42
+  )cpp");
+  llvm::Annotations Main(R"cpp(
+#include "hdr.h"
+int x = ^ANSWER;
+  )cpp");
+
+  SourceManagerForFile SMF("main.cpp", Main.code());
+  auto  = SMF.get();
+  const FileEntry *HdrFile =
+  SM.getFileManager().getVirtualFile("hdr.h", Hdr.code().size(), 0);
+  SM.overrideFileContents(HdrFile,
+  llvm::MemoryBuffer::getMemBuffer(Hdr.code().str()));
+  FileID HdrID = SM.createFileID(HdrFile, SourceLocation(), SrcMgr::C_User);
+
+  IdentifierTable Idents;
+  Symbol Answer =
+  Macro{("ANSWER"), SM.getComposedLoc(HdrID, Hdr.point())};
+  llvm::DenseMap> OffsetToProviders;
+  walkUsed(SM, /*ASTRoots=*/{}, /*MacroRefs=*/
+   {SymbolReference{SM.getComposedLoc(SM.getMainFileID(), Main.point()),
+Answer, RefType::Explicit}},
+   [&](SymbolReference SymRef, llvm::ArrayRef Providers) {
+ auto [FID, Offset] = SM.getDecomposedLoc(SymRef.RefLocation);
+ EXPECT_EQ(FID, SM.getMainFileID());
+ OffsetToProviders.try_emplace(Offset, Providers.vec());
+   });
+
+  EXPECT_THAT(
+  OffsetToProviders,
+  UnorderedElementsAre(Pair(Main.point(), UnorderedElementsAre(HdrFile;
+}
+
 } // namespace
 } // namespace clang::include_cleaner
Index: clang-tools-extra/include-cleaner/lib/Analysis.cpp
===
--- clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -26,9 +26,10 @@
   });
   return Result;
 }
-
 } // namespace
-void walkUsed(llvm::ArrayRef ASTRoots, UsedSymbolCB CB) {
+
+void walkUsed(const SourceManager , llvm::ArrayRef ASTRoots,
+  llvm::ArrayRef MacroRefs, UsedSymbolCB CB) {
   tooling::stdlib::Recognizer Recognizer;
   for (auto *Root : ASTRoots) {
 auto  = Root->getASTContext().getSourceManager();
@@ -45,7 +46,14 @@
 return CB({Loc, Symbol(ND), RT}, {Header(FE)});
 });
   }
-  // FIXME: Handle references of macros.
+  for (const SymbolReference  : MacroRefs) {
+assert(MacroRef.Target.kind() == Symbol::Macro);
+// FIXME: Handle IWYU pragmas, non self-contained files.
+// FIXME: Handle macro locations.
+if (auto *FE = SM.getFileEntryForID(
+SM.getFileID(MacroRef.Target.macro().Definition)))
+  CB(MacroRef, {Header(FE)});
+  }
 }
 
 } // namespace clang::include_cleaner
Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
===
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
@@ -35,7 +35,7 @@
 ///
 /// The AST traversal is rooted at ASTRoots - typically top-level declarations
 /// of a single source file.
-/// FIXME: Handle macro uses.
+/// The references to macros must be recorded separately and provided.
 ///
 /// This is the main entrypoint of the include-cleaner library, and can be used:
 ///  - to diagnose missing includes: a referenced symbol is provided by
@@ -44,7 +44,8 @@
 ///the headers for any referenced symbol
 /// FIXME: Take in an include structure to improve location to header mappings
 /// (e.g. IWYU pragmas).
-void walkUsed(llvm::ArrayRef ASTRoots, UsedSymbolCB CB);
+void walkUsed(const SourceManager &, llvm::ArrayRef ASTRoots,
+  llvm::ArrayRef MacroRefs, UsedSymbolCB CB);
 
 } // namespace include_cleaner
 } // namespace clang
___
cfe-commits mailing list

[clang-tools-extra] 38cccb9 - [include-cleaner] pass through recorded macro refs in walkUsed

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-11-08T20:10:39+01:00
New Revision: 38cccb90660347939dd7bfdd57ba66760cba39c2

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

LOG: [include-cleaner] pass through recorded macro refs in walkUsed

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

Added: 


Modified: 
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
clang-tools-extra/include-cleaner/lib/Analysis.cpp
clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp

Removed: 




diff  --git 
a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h 
b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
index a75fd9ad8166..4c5c90e769ae 100644
--- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
+++ b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
@@ -35,7 +35,7 @@ using UsedSymbolCB = llvm::function_ref ASTRoots, UsedSymbolCB CB);
+void walkUsed(const SourceManager &, llvm::ArrayRef ASTRoots,
+  llvm::ArrayRef MacroRefs, UsedSymbolCB CB);
 
 } // namespace include_cleaner
 } // namespace clang

diff  --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp 
b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
index 95c6c6a2b8b9..b4dfd129c60d 100644
--- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -26,9 +26,10 @@ toHeader(llvm::ArrayRef Headers) {
   });
   return Result;
 }
-
 } // namespace
-void walkUsed(llvm::ArrayRef ASTRoots, UsedSymbolCB CB) {
+
+void walkUsed(const SourceManager , llvm::ArrayRef ASTRoots,
+  llvm::ArrayRef MacroRefs, UsedSymbolCB CB) {
   tooling::stdlib::Recognizer Recognizer;
   for (auto *Root : ASTRoots) {
 auto  = Root->getASTContext().getSourceManager();
@@ -45,7 +46,14 @@ void walkUsed(llvm::ArrayRef ASTRoots, UsedSymbolCB 
CB) {
 return CB({Loc, Symbol(ND), RT}, {Header(FE)});
 });
   }
-  // FIXME: Handle references of macros.
+  for (const SymbolReference  : MacroRefs) {
+assert(MacroRef.Target.kind() == Symbol::Macro);
+// FIXME: Handle IWYU pragmas, non self-contained files.
+// FIXME: Handle macro locations.
+if (auto *FE = SM.getFileEntryForID(
+SM.getFileID(MacroRef.Target.macro().Definition)))
+  CB(MacroRef, {Header(FE)});
+  }
 }
 
 } // namespace clang::include_cleaner

diff  --git a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp 
b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
index cb7bc1ae6a43..bd794037e85a 100644
--- a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -50,7 +50,7 @@ TEST(WalkUsed, Basic) {
 
   auto  = AST.sourceManager();
   llvm::DenseMap> OffsetToProviders;
-  walkUsed(TopLevelDecls,
+  walkUsed(SM, TopLevelDecls, /*MacroRefs=*/{},
[&](SymbolReference SymRef, llvm::ArrayRef Providers) {
  auto [FID, Offset] = SM.getDecomposedLoc(SymRef.RefLocation);
  EXPECT_EQ(FID, SM.getMainFileID());
@@ -68,5 +68,40 @@ TEST(WalkUsed, Basic) {
   Pair(Code.point("vconstructor"), UnorderedElementsAre(VectorSTL;
 }
 
+TEST(WalkUsed, MacroRefs) {
+  llvm::Annotations Hdr(R"cpp(
+#define ^ANSWER 42
+  )cpp");
+  llvm::Annotations Main(R"cpp(
+#include "hdr.h"
+int x = ^ANSWER;
+  )cpp");
+
+  SourceManagerForFile SMF("main.cpp", Main.code());
+  auto  = SMF.get();
+  const FileEntry *HdrFile =
+  SM.getFileManager().getVirtualFile("hdr.h", Hdr.code().size(), 0);
+  SM.overrideFileContents(HdrFile,
+  llvm::MemoryBuffer::getMemBuffer(Hdr.code().str()));
+  FileID HdrID = SM.createFileID(HdrFile, SourceLocation(), SrcMgr::C_User);
+
+  IdentifierTable Idents;
+  Symbol Answer =
+  Macro{("ANSWER"), SM.getComposedLoc(HdrID, Hdr.point())};
+  llvm::DenseMap> OffsetToProviders;
+  walkUsed(SM, /*ASTRoots=*/{}, /*MacroRefs=*/
+   {SymbolReference{SM.getComposedLoc(SM.getMainFileID(), 
Main.point()),
+Answer, RefType::Explicit}},
+   [&](SymbolReference SymRef, llvm::ArrayRef Providers) {
+ auto [FID, Offset] = SM.getDecomposedLoc(SymRef.RefLocation);
+ EXPECT_EQ(FID, SM.getMainFileID());
+ OffsetToProviders.try_emplace(Offset, Providers.vec());
+   });
+
+  EXPECT_THAT(
+  OffsetToProviders,
+  UnorderedElementsAre(Pair(Main.point(), UnorderedElementsAre(HdrFile;
+}
+
 } // namespace
 } // namespace clang::include_cleaner



___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D137197: [clang][modules][deps] Transitive module maps are not affecting

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese accepted this revision.
Bigcheese added a comment.
This revision is now accepted and ready to land.

Yeah, I agree with this. lgtm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137197

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

[PATCH] D137071: [clang][Interp] Implement missing compound assign operators

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Precommit CI looks to have potentially found something interesting here.




Comment at: clang/test/AST/Interp/literals.cpp:553
+  static_assert(IntRem(2, 1) == 0, "");
+  static_assert(IntRem(9, 7) == 2, "");
+

aaron.ballman wrote:
> ```
> static_assert(IntRem(9, 0) == 12, ""); // Not constexpr
> static_assert(IntRem(__INT_MIN__, -1) == 12, ""); // Not constexpr
> ```
Missed the test for __INT_MIN__ and -1?


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

https://reviews.llvm.org/D137071

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

[PATCH] D137650: [clangd] Implement hover for C++ string literals

[Tom Praschan via Phabricator via cfe-commits]

tom-anders added inline comments.



Comment at: clang-tools-extra/clangd/Hover.cpp:785
+
+  // TODO: Show string literal's contents
+  HI.Name = "String Literal";

Is it really useful to show the contents inside the Hover? You already see the 
contents of the string literal anyway, so I don't think this adds any value



Comment at: clang-tools-extra/clangd/Hover.cpp:788
+
+  // In C++ string literals have `const` qualifier, but in C it don't.
+  std::string Qualifier;

(nit)



Comment at: clang-tools-extra/clangd/Hover.cpp:814
+  } else {
+// TODO: For other languages
+return llvm::None;

Hmm so what's stopping us from adding support for C here?



Comment at: clang-tools-extra/clangd/Hover.cpp:820
+  Type.Type =
+  Qualifier + CharType + "[" + std::to_string(SL->getLength() + 1) + "]";
+  HI.Type = Type;

I think you should prefer `llvm::formatv` here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137650

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

[PATCH] D137232: [clang][Interp] Support inc/dec operators on pointers

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/Interp/Opcodes.td:421-425
+// [Pointer] -> []
+def IncPtr : Opcode {
+  let HasGroup = 0;
+}
+// [Pointer] -> []

Don't both of these result in a `Pointer`? And should they have a `Types` field?



Comment at: clang/test/AST/Interp/arrays.cpp:216
 
-  static_assert(getNextElem(E, 1) == 3);
-#endif
+  static_assert(getNextElem(E, 1) == 3, "");
+

I'd like test cases where the pointer arithmetic has run off the beginning/end 
of the object (forming the invalid pointer is UB per 
http://eel.is/c++draft/expr.add#4.3 even if you never dereference the pointer).


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

https://reviews.llvm.org/D137232

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

[PATCH] D137192: [clang][serialization] NFCI: Avoid re-reading input file info

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese accepted this revision.
Bigcheese added a comment.
This revision is now accepted and ready to land.

lgtm, thanks for fixing this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137192

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

[PATCH] D137550: [clangd] Fix the code action `RemoveUsingNamespace`

[Tom Praschan via Phabricator via cfe-commits]

tom-anders accepted this revision.
tom-anders added a comment.
This revision is now accepted and ready to land.

Would be cool if in the future we could instead transform something like

  using namespace std;
  int main() {
 auto t = 5ms;
  }

into

  using namespace std::chrono_literals;
  int main() {
 auto t = 5ms;
  }

But I think your fix certainly improved the behavior for now, so LGTM!




Comment at: 
clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp:253
+)cpp"},
+  {// Does qualify user-defined literals
+   R"cpp(






Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137550

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

[clang] 98fa954 - Revert "[Assignment Tracking][7/*] Add assignment tracking functionality to clang"

[via cfe-commits]

Author: OCHyams
Date: 2022-11-08T18:43:05Z
New Revision: 98fa95492f3bbd5befdeb36c88a3ac5ef2740b4e

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

LOG: Revert "[Assignment Tracking][7/*] Add assignment tracking functionality 
to clang"

This reverts commit 28f9636edd7d4e5f3eba1e3d0d37ec53d616481f.

Bot failure: https://lab.llvm.org/buildbot/#/builders/109/builds/50251

Added: 


Modified: 
clang/lib/CodeGen/BackendUtil.cpp

Removed: 
clang/test/CodeGen/assignment-tracking/assignment-tracking.cpp
clang/test/CodeGen/assignment-tracking/flag.cpp
clang/test/CodeGen/assignment-tracking/memcpy-fragment.cpp
clang/test/CodeGen/assignment-tracking/nested-scope.cpp
clang/test/Driver/assignment-tracking-opts.c



diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index eb444c829381f..f5c125da10da6 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -30,7 +30,6 @@
 #include "llvm/CodeGen/SchedulerRegistry.h"
 #include "llvm/CodeGen/TargetSubtargetInfo.h"
 #include "llvm/IR/DataLayout.h"
-#include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/IRPrintingPasses.h"
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/Module.h"
@@ -831,13 +830,6 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
   SI.registerCallbacks(PIC, );
   PassBuilder PB(TM.get(), PTO, PGOOpt, );
 
-  if (CodeGenOpts.EnableAssignmentTracking) {
-PB.registerPipelineStartEPCallback(
-[&](ModulePassManager , OptimizationLevel Level) {
-  MPM.addPass(AssignmentTrackingPass());
-});
-  }
-
   // Enable verify-debuginfo-preserve-each for new PM.
   DebugifyEachInstrumentation Debugify;
   DebugInfoPerPass DebugInfoBeforePass;

diff  --git a/clang/test/CodeGen/assignment-tracking/assignment-tracking.cpp 
b/clang/test/CodeGen/assignment-tracking/assignment-tracking.cpp
deleted file mode 100644
index 244ecdf8971d9..0
--- a/clang/test/CodeGen/assignment-tracking/assignment-tracking.cpp
+++ /dev/null
@@ -1,96 +0,0 @@
-// RUN: %clang_cc1 -triple x86_64-none-linux-gnu -debug-info-kind=standalone 
-O0 \
-// RUN: -emit-llvm  -fexperimental-assignment-tracking %s -o - 
  \
-// RUN: | FileCheck %s --implicit-check-not="call void @llvm.dbg"
-
-// Based on llvm/test/DebugInfo/Generic/track-assignments.ll - check that using
-// -Xclang -fexperimental-assignment-tracking results in emitting (or, as it is
-// set up currently, telling llvm to create) assignment tracking metadata.
-//
-// See the original test for more info.
-
-struct Inner { int A, B; };
-struct Outer { Inner A, B; };
-struct Large { int A[10]; };
-struct LCopyCtor { int A[4]; LCopyCtor(); LCopyCtor(LCopyCtor const &); };
-int Value, Index, Cond;
-Inner InnerA, InnerB;
-Large L;
-
-void zeroInit() { int Z[3] = {0, 0, 0}; }
-// CHECK-LABEL: define dso_local void @_Z8zeroInitv
-// CHECK:   %Z = alloca [3 x i32], align 4, !DIAssignID ![[ID_0:[0-9]+]]
-// CHECK-NEXT:  call void @llvm.dbg.assign(metadata i1 undef, metadata 
![[VAR_0:[0-9]+]], metadata !DIExpression(), metadata ![[ID_0]], metadata [3 x 
i32]* %Z, metadata !DIExpression())
-// CHECK:@llvm.memset.p0i8.i64{{.*}}, !DIAssignID ![[ID_1:[0-9]+]]
-// CHECK-NEXT:   call void @llvm.dbg.assign(metadata i8 0, metadata 
![[VAR_0]], metadata !DIExpression(), metadata ![[ID_1]], metadata i8* %0, 
metadata !DIExpression())
-
-void memcpyInit() { int A[4] = {0, 1, 2, 3}; }
-// CHECK-LABEL: define dso_local void @_Z10memcpyInitv
-// CHECK:   %A = alloca [4 x i32], align 16, !DIAssignID ![[ID_2:[0-9]+]]
-// CHECK-NEXT:  call void @llvm.dbg.assign(metadata i1 undef, metadata 
![[VAR_1:[0-9]+]], metadata !DIExpression(), metadata ![[ID_2]], metadata [4 x 
i32]* %A, metadata !DIExpression())
-// CHECK:@llvm.memcpy.p0i8.p0i8.i64{{.*}}, !DIAssignID ![[ID_3:[0-9]+]]
-// CHECK-NEXT:   call void @llvm.dbg.assign(metadata i1 undef, metadata 
![[VAR_1]], metadata !DIExpression(), metadata ![[ID_3]], metadata i8* %0, 
metadata !DIExpression())
-
-void setField() {
-  Outer O;
-  O.A.B = Value;
-}
-// CHECK-LABEL: define dso_local void @_Z8setFieldv
-// CHECK:   %O = alloca %struct.Outer, align 4, !DIAssignID 
![[ID_4:[0-9]+]]
-// CHECK-NEXT:  call void @llvm.dbg.assign(metadata i1 undef, metadata 
![[VAR_2:[0-9]+]], metadata !DIExpression(), metadata ![[ID_4]], metadata 
%struct.Outer* %O, metadata !DIExpression())
-// CHECK:   store i32 %0, i32* %B, align 4,{{.*}}!DIAssignID 
![[ID_5:[0-9]+]]
-// CHECK-NEXT:  call void @llvm.dbg.assign(metadata i32 %0, metadata 
![[VAR_2]], metadata !DIExpression(DW_OP_LLVM_fragment, 32, 32), metadata 
![[ID_5]], metadata i32* %B, metadata !DIExpression())
-
-void unknownOffset() {
-  int A[2];
-  A[Index] = Value;
-}
-// 

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

[Aaron Gokaslan via Phabricator via cfe-commits]

Skylion007 added a comment.

Okay, now I am getting what I believe to be segfaults:

  #0 0x564383482be4 PrintStackTraceSignalHandler(void*) Signals.cpp:0:0
  #1 0x564383480464 SignalHandler(int) Signals.cpp:0:0
  #2 0x7f7c275c9420 __restore_rt 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x14420)
  #3 0x5643804b0ea5 
clang::tidy::performance::UnnecessaryCopyOnLastUseCheck::check(clang::ast_matchers::MatchFinder::MatchResult
 const&) (.cold) UnnecessaryCopyOnLastUseCheck.cpp:0:0
  #4 0x56438262bba1 clang::ast_matchers::internal::(anonymous 
namespace)::MatchASTVisitor::MatchVisitor::visitMatch(clang::ast_matchers::BoundNodes
 const&) ASTMatchFinder.cpp:0:0
  #5 0x5643826818df 
clang::ast_matchers::internal::BoundNodesTreeBuilder::visitMatches(clang::ast_matchers::internal::BoundNodesTreeBuilder::Visitor*)
 (/home/aaron/git/llvm-project/build/bin/clang-tidy+0x2d258df)

It worked before this PR, now it just crashes on a lot of real world codebases 
including just trying to run it for a few files on LLVM's own codebase.


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] D137399: [clang][Interp] Emit negated integers directly as constants

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

This is missing test coverage (possibly around integer promotion behavior as 
well).




Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1525
+return true;
+  APSInt V = APSInt(IL->getValue(), /*IsUnsigned=*/false);
+  return this->emitConst(-V, E);

Are you sure about `IsUnsigned`? http://eel.is/c++draft/expr.unary.op#8 reads 
to me like `-0ULL` will remain unsigned as a result.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137399

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

[PATCH] D137473: [vfs] Allow root paths relative to the directory of the vfsoverlay YAML file

[Ben Barham via Phabricator via cfe-commits]

bnbarham added a subscriber: dexonsmith.
bnbarham added a comment.

This seems reasonable to me in general. @dexonsmith in case you have any 
thoughts.




Comment at: clang/test/VFS/Inputs/root-relative-overlay.yaml:4
+  'case-sensitive': false,
+  'overlay-relative': true,
+  'root-relative': 'yaml-dir',

I'd prefer a test without `overlay-relative` set to make it clear they don't 
depend on each other.



Comment at: llvm/include/llvm/Support/VirtualFileSystem.h:660
 ///   'use-external-names': 
+///   'root-relative': 
 ///   'overlay-relative': 

phosek wrote:
> Could we make this just a boolean akin to `overlay-relative` since there are 
> only two options (default to `false`)?
I personally prefer being explicit here, `overlay-relative` is fairly confusing 
as it is.

`overlay-relative` isn't about allowing relative paths, but instead means that 
*all* external paths should be prefixed with the directory of the overlay. To 
put another way, external paths can be relative whether this is true/false, 
`overlay-relative` just *always* prepends the overlay path.

Could you add a comment to make it clear that this has no interaction with 
`overlay-relative`? If you want to add a comment to `overlay-relative` with 
something like the above that would also be appreciated :)



Comment at: llvm/include/llvm/Support/VirtualFileSystem.h:752
+  enum class RootRelativeKind {
+/// The roots are relative to the current working directory.
+CWD,

`to the current working directory when the overlay is created.` is maybe a 
little clearer to me



Comment at: llvm/include/llvm/Support/VirtualFileSystem.h:755
+/// The roots are relative to the directory where the YAML file locates.
+YAMLDir
+  };

Any thoughts on something like `OverlayDir` instead?



Comment at: llvm/include/llvm/Support/VirtualFileSystem.h:926
+  /// is set. This will also be prefixed to each 'roots->name' if RootRelative
+  /// is set to RootRelativeKind::YAMLDir.
+  std::string YAMLFileDir;

*and the path is relative*



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1903
+assert(!FullPath.empty() && "YAML file directory must exist");
+sys::fs::make_absolute(FS->getYAMLFileDir(), Name);
+Name = canonicalize(Name);

IMO both this and CWD should be using the base FS instead. VFS didn't have a 
CWD previously, but now that it does it doesn't really make sense to use the 
process wide CWD. Especially since `-working-directory` doesn't change it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137473

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

[PATCH] D137609: [C++20] [Modules] Remove unmaintained header modules

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese added a comment.

I'm not aware of anyone using this mode, but please wait for responses from 
Google and Meta people to verify that.


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

https://reviews.llvm.org/D137609

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

[PATCH] D113359: [Libomptarget][WIP] Introduce VGPU Plugin

[Deepak Eachempati via Phabricator via cfe-commits]

dreachem added subscribers: dhruvachak, dreachem.
dreachem added a comment.
Herald added a subscriber: MaskRay.
Herald added a project: All.

@jdoerfert @tianshilei1992 @atmnpatel @dhruvachak

Is the target to get this merged in for LLVM 16? Does the VGPU implementation 
provide a way to support OMPT callbacks for various constructs (parallel, 
worksharing, barriers, etc.)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113359

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

[PATCH] D125860: [clang] Only use major version in resource dir

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

@tbaeder That CI failure seems unrelated.  Maybe just commit early in the day 
so you have time to deal with the bot failures (if any).


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

https://reviews.llvm.org/D125860

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