[PATCH] D98493: [WoA][MSVC] Use default linker setting in MSVC-compatible driver

[Maxim Kuvyrkov via Phabricator via cfe-commits]

maxim-kuvyrkov added a comment.

Hi @phosek ,

The only upstream bot that was broken by this change was clang-ppc64le-rhel, 
and I have notified the bot owner of the problem and said that I'm working on 
this within 15 minutes of the breakage.  I posted a tentative fix for the 
testsuite several hours later.

I assume you have reverted this patch because you are seeing breakage in one of 
your internal bots.  If that's the case then some details on the breakage would 
be useful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98493

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

[PATCH] D98923: [Driver] Pass -fexperimental-strict-floating-point to cc1 if it is specified

[Jim Lin via Phabricator via cfe-commits]

Jim created this revision.
Jim added a reviewer: kpn.
Jim requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

In my case, it has to enable strict floating-point by driver for default 
unsupported target.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98923

Files:
  clang/lib/Driver/ToolChains/Clang.cpp


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5386,6 +5386,9 @@
   if (Args.hasArg(options::OPT_fexperimental_new_constant_interpreter))
 CmdArgs.push_back("-fexperimental-new-constant-interpreter");
 
+  if (Args.hasArg(options::OPT_fexperimental_strict_floating_point))
+CmdArgs.push_back("-fexperimental-strict-floating-point");
+
   if (Arg *A = Args.getLastArg(options::OPT_fbracket_depth_EQ)) {
 CmdArgs.push_back("-fbracket-depth");
 CmdArgs.push_back(A->getValue());


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5386,6 +5386,9 @@
   if (Args.hasArg(options::OPT_fexperimental_new_constant_interpreter))
 CmdArgs.push_back("-fexperimental-new-constant-interpreter");
 
+  if (Args.hasArg(options::OPT_fexperimental_strict_floating_point))
+CmdArgs.push_back("-fexperimental-strict-floating-point");
+
   if (Arg *A = Args.getLastArg(options::OPT_fbracket_depth_EQ)) {
 CmdArgs.push_back("-fbracket-depth");
 CmdArgs.push_back(A->getValue());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98493: [WoA][MSVC] Use default linker setting in MSVC-compatible driver

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

They're not part of buildbot but they're not internal, you can see the log 
here: 
https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket.appspot.com/8852352747956966400/+/u/clang/test/stdout.

I noticed D98862 , but it still hasn't landed 
and given that it's been 24 hours since your change landed and anyone who is 
using `CLANG_DEFAULT_LINKER` is currently broken (I for example use this option 
in my local build and tests have been failing for me locally as well), I think 
it's it better to revert this change and then reland it once your fix is ready.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98493

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

[PATCH] D98824: [Tooling] Handle compilation databases containing commands with double dashes

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D98824#2634715 , @jnykiel wrote:

> I have addressed some of your comments. The second revision is good enough to 
> get the tests to pass - the `injectResourceDir` change was necessary for the 
> one clang-tidy test that started failing after leaving the `--` in. I don't 
> feel familiar enough with clangd specifically to attempt comprehensive fixes 
> there. The clangd issue report specifically mentions `-fsyntax-only` and 
> `-resource-dir` as well - so while not comprehensive, the fix may be good for 
> that too.

There are a lot more argument adjusters that look at the compile flags and they 
don't stop at `--` though :/ I don't think it is feasible to special case `--` 
in all of those loops and doesn't happen enough in practice (i.e. we see a 
filename that starts with `-resource-dir` or `-fsyntax-only` etc.) to justify a 
more complex solution. So as mentioned in the comments let's ignore these for 
now.




Comment at: clang/lib/Tooling/ArgumentsAdjusters.cpp:48
+
+  if (Arg == "--")
+break;

we should perform this either in all or none. thinking about this again, there 
are a lot more things that can go wrong but they happen pretty rarely so let's 
just drop this change and replace the push_back below with 
`getInsertArgumentAdjuster("-fsyntax-only")(AdjustedArgs, "");`

copying over args once more isn't crucial here as this happen only once before 
parsing a translation unit, which by far dominates the latency.



Comment at: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp:248
+if (InsertDoubleDash)
+  Result.CommandLine.push_back("--");
 Result.CommandLine.push_back(std::string(Filename));

the condition should be a function of the `Filename`, not the compile flag we 
are using. as the original command might not contain the `--` but the filename 
might still be starting with a `-` or `/` here, right?

So i'd suggest `Filename.startswith("-") || (ClangCLMode && 
Filename.startswith("/"))`



Comment at: clang/lib/Tooling/Tooling.cpp:439
+  for (StringRef Arg : Args) {
+if (Arg == "--")
+  break;

similar to comment above, let's just ignore this.



Comment at: clang/lib/Tooling/Tooling.cpp:448
+  resourceDir.append(CompilerInvocation::GetResourcesPath(Argv0, MainAddr));
+  Args = getInsertArgumentAdjuster(CommandLineArguments(1, resourceDir),
+   ArgumentInsertPosition::END)(Args, "");

nit:
```
Args = getInsertArgumentAdjuster(("-resource-dir=" + 
CompilerInvocation::GetResourcesPath(Argv0, MainAddr)).c_str())(Args, "");
```

There's an overload of `getInsertArgumentAdjuster` that takes in a `const char*`


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

https://reviews.llvm.org/D98824

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

[PATCH] D98862: [clang] Update unit-tests after linker selection fix for *-msvc targets

[Maxim Kuvyrkov via Phabricator via cfe-commits]

maxim-kuvyrkov updated this revision to Diff 331784.
maxim-kuvyrkov added a comment.

Added more fixes to OpenMP/linking.c and Driver/msvc-link.c .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98862

Files:
  clang/test/Driver/Xlinker-args.c
  clang/test/Driver/cl-inputs.c
  clang/test/Driver/cl-link-at-file.c
  clang/test/Driver/cl-link.c
  clang/test/Driver/msvc-link.c
  clang/test/OpenMP/linking.c

Index: clang/test/OpenMP/linking.c
===
--- clang/test/OpenMP/linking.c
+++ clang/test/OpenMP/linking.c
@@ -81,7 +81,7 @@
 // CHECK-LD-OVERRIDE-64: "-lgomp" "-lrt"
 // CHECK-LD-OVERRIDE-64: "-lpthread" "-lc"
 //
-// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: %clang -no-canonical-prefixes -fuse-ld=link %s -### -o %t.o 2>&1 \
 // RUN: -fopenmp=libomp -target x86_64-msvc-win32 -rtlib=platform \
 // RUN:   | FileCheck --check-prefix=CHECK-MSVC-LINK-64 %s
 // CHECK-MSVC-LINK-64: link.exe
@@ -95,7 +95,7 @@
 // SIMD-ONLY11-NOT: libomp
 // SIMD-ONLY11-NOT: libgomp
 //
-// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: %clang -no-canonical-prefixes %s -fuse-ld=link -### -o %t.o 2>&1 \
 // RUN: -fopenmp=libiomp5 -target x86_64-msvc-win32 -rtlib=platform \
 // RUN:   | FileCheck --check-prefix=CHECK-MSVC-ILINK-64 %s
 
Index: clang/test/Driver/msvc-link.c
===
--- clang/test/Driver/msvc-link.c
+++ clang/test/Driver/msvc-link.c
@@ -1,4 +1,4 @@
-// RUN: %clang -target i686-pc-windows-msvc -### %s 2>&1 | FileCheck --check-prefix=BASIC %s
+// RUN: %clang -target i686-pc-windows-msvc -fuse-ld=link -### %s 2>&1 | FileCheck --check-prefix=BASIC %s
 // BASIC: link.exe"
 // BASIC: "-out:a.exe"
 // BASIC: "-defaultlib:libcmt"
@@ -6,7 +6,7 @@
 // BASIC: "-nologo"
 // BASIC-NOT: "-Brepro"
 
-// RUN: %clang -target i686-pc-windows-msvc -shared -o a.dll -### %s 2>&1 | FileCheck --check-prefix=DLL %s
+// RUN: %clang -target i686-pc-windows-msvc -shared -o a.dll -fuse-ld=link -### %s 2>&1 | FileCheck --check-prefix=DLL %s
 // DLL: link.exe"
 // DLL: "-out:a.dll"
 // DLL: "-defaultlib:libcmt"
@@ -19,13 +19,13 @@
 // LIBPATH: "-libpath:/usr/lib"
 // LIBPATH: "-nologo"
 
-// RUN: %clang_cl /Brepro -### -- %s 2>&1 | FileCheck --check-prefix=REPRO %s
+// RUN: %clang_cl /Brepro -fuse-ld=link -### -- %s 2>&1 | FileCheck --check-prefix=REPRO %s
 // REPRO: link.exe"
 // REPRO: "-out:msvc-link.exe"
 // REPRO: "-nologo"
 // REPRO: "-Brepro"
 
-// RUN: %clang_cl /Brepro- -### -- %s 2>&1 | FileCheck --check-prefix=NOREPRO %s
+// RUN: %clang_cl /Brepro- -fuse-ld=link -### -- %s 2>&1 | FileCheck --check-prefix=NOREPRO %s
 // NOREPRO: link.exe"
 // NOREPRO: "-out:msvc-link.exe"
 // NOREPRO: "-nologo"
Index: clang/test/Driver/cl-link.c
===
--- clang/test/Driver/cl-link.c
+++ clang/test/Driver/cl-link.c
@@ -2,14 +2,14 @@
 // be interpreted as a command-line option, e.g. on Mac where %s is commonly
 // under /Users.
 
-// RUN: %clang_cl /Tc%s -### /link foo bar baz 2>&1 | FileCheck --check-prefix=LINK %s
-// RUN: %clang_cl /Tc%s -### /linkfoo bar baz 2>&1 | FileCheck --check-prefix=LINK %s
+// RUN: %clang_cl /Tc%s -fuse-ld=link -### /link foo bar baz 2>&1 | FileCheck --check-prefix=LINK %s
+// RUN: %clang_cl /Tc%s -fuse-ld=link -### /linkfoo bar baz 2>&1 | FileCheck --check-prefix=LINK %s
 // LINK: link.exe
 // LINK: "foo"
 // LINK: "bar"
 // LINK: "baz"
 
-// RUN: %clang_cl -m32 -arch:IA32 --target=i386-pc-win32 /Tc%s -### -fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN %s
+// RUN: %clang_cl -m32 -arch:IA32 --target=i386-pc-win32 /Tc%s -fuse-ld=link -### -fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN %s
 // ASAN: link.exe
 // ASAN: "-debug"
 // ASAN: "-incremental:no"
@@ -19,7 +19,7 @@
 // ASAN: "-wholearchive:{{.*}}clang_rt.asan_cxx-i386.lib"
 // ASAN: "{{.*}}cl-link{{.*}}.obj"
 
-// RUN: %clang_cl -m32 -arch:IA32 --target=i386-pc-win32 /MD /Tc%s -### -fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN-MD %s
+// RUN: %clang_cl -m32 -arch:IA32 --target=i386-pc-win32 /MD /Tc%s -fuse-ld=link -### -fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN-MD %s
 // ASAN-MD: link.exe
 // ASAN-MD: "-debug"
 // ASAN-MD: "-incremental:no"
@@ -29,13 +29,13 @@
 // ASAN-MD: "-wholearchive:{{.*}}clang_rt.asan_dynamic_runtime_thunk-i386.lib"
 // ASAN-MD: "{{.*}}cl-link{{.*}}.obj"
 
-// RUN: %clang_cl /LD -### /Tc%s 2>&1 | FileCheck --check-prefix=DLL %s
-// RUN: %clang_cl /LDd -### /Tc%s 2>&1 | FileCheck --check-prefix=DLL %s
+// RUN: %clang_cl /LD -fuse-ld=link -### /Tc%s 2>&1 | FileCheck --check-prefix=DLL %s
+// RUN: %clang_cl /LDd -fuse-ld=link -### /Tc%s 2>&1 | FileCheck --check-prefix=DLL %s
 // DLL: link.exe
 // "-dll"
 
-// RUN: %clang_cl -m32 -arch:IA32 --target=i386-pc-win32 /LD /Tc%s -### -fsanitize=address 2>&1 |

[PATCH] D98493: [WoA][MSVC] Use default linker setting in MSVC-compatible driver

[Maxim Kuvyrkov via Phabricator via cfe-commits]

maxim-kuvyrkov added a comment.

In D98493#2636616 , @phosek wrote:

> They're not part of buildbot but they're not internal, you can see the log 
> here: 
> https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket.appspot.com/8852352747956966400/+/u/clang/test/stdout.
>
> I noticed D98862 , but it still hasn't 
> landed and given that it's been 24 hours since your change landed and anyone 
> who is using `CLANG_DEFAULT_LINKER` is currently broken (I for example use 
> this option in my local build and tests have been failing for me locally as 
> well), I think it's it better to revert this change and then reland it once 
> your fix is ready.

Would you please test D98862  (which I've just 
updated) on top of this patch and see that completely fixes your bots?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98493

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

[PATCH] D98862: [clang] Update unit-tests after linker selection fix for *-msvc targets

[Maxim Kuvyrkov via Phabricator via cfe-commits]

maxim-kuvyrkov added a comment.

Hi @amyk , thanks for testing -- I've updated two more places, which I missed 
on the first go.  Would you please test the new version?

Also, D98862  was reverted so you need to 
apply it manually.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98862

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

[PATCH] D98433: [clang] [C++2b] [P1102] Accept lambdas without parameter list ().

[Marek Kurdej via Phabricator via cfe-commits]

curdeius updated this revision to Diff 331795.
curdeius added a comment.

- Accept but warn in pre-C++2b mode when lambda is missing '()' and have 
lambda-specifiers.
- Fix tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98433

Files:
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/Parse/ParseExprCXX.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.lambda/p4-1y.cpp
  clang/test/FixIt/fixit-c++11.cpp
  clang/test/Parser/cxx-concepts-requires-clause.cpp
  clang/test/Parser/cxx0x-lambda-expressions.cpp
  clang/test/Parser/cxx1z-constexpr-lambdas.cpp
  clang/test/Parser/cxx2a-template-lambdas.cpp
  clang/test/Parser/cxx2b-lambdas.cpp
  clang/test/SemaOpenCLCXX/address-space-lambda.cl
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -63,7 +63,7 @@
 
  C++2b (tentatively C++23)
  -std=c++2b
- No
+ Partial
 
 
 
@@ -1276,7 +1276,7 @@
 
   Make () in lambdas optional in all cases
   https://wg21.link/p1102r2";>P1102R2
-  No
+  Clang 13
 
 
 
Index: clang/test/SemaOpenCLCXX/address-space-lambda.cl
===
--- clang/test/SemaOpenCLCXX/address-space-lambda.cl
+++ clang/test/SemaOpenCLCXX/address-space-lambda.cl
@@ -61,7 +61,10 @@
   [&] () __global {} (); //expected-error{{no matching function for call to object of type '(lambda at}} expected-note{{candidate function not viable: 'this' object is in default address space, but method expects object in address space '__global'}}
   [&] () __private {} (); //expected-error{{no matching function for call to object of type '(lambda at}} expected-note{{candidate function not viable: 'this' object is in default address space, but method expects object in address space '__private'}}
 
-  [&] __private {} (); //expected-error{{lambda requires '()' before attribute specifier}} expected-error{{expected body of lambda expression}}
+  [&] __private {} (); // expected-error{{no matching function for call to object of type '(lambda at}} expected-note{{candidate function not viable: 'this' object is in default address space, but method expects object in address space '__private'}}
+#if __cplusplus <= 202002L
+// expected-warning@-2{{lambda without '()' before attribute specifier is a C++2b extension}}
+#endif
 
   [&] () mutable __private {} ();
   [&] () __private mutable {} (); //expected-error{{expected body of lambda expression}}
Index: clang/test/Parser/cxx2b-lambdas.cpp
===
--- /dev/null
+++ clang/test/Parser/cxx2b-lambdas.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -std=c++2b %s -verify
+
+auto L = [] {};
+auto LL1 = [] () {};
+auto LL2 = [] () mutable {};
+auto LL3 = [] () constexpr {};
+
+auto L0 = [] constexpr {};
+auto L1 = [] mutable {};
+auto L2 = [] noexcept {};
+auto L3 = [] constexpr mutable {};
+auto L4 = [] mutable constexpr {};
+auto L5 = [] constexpr mutable noexcept {};
+auto L6 = [s = 1] mutable {};
+auto L7 = [s = 1] constexpr mutable noexcept {};
+auto L8 = [] -> bool { return true; };
+auto L9 = [] { return true; };
+auto L10 = [] noexcept { return true; };
+auto L11 = [] -> bool { return true; };
+auto L12 = [] consteval {};
+auto L13 = [] requires requires() { true; }
+{};
+auto L15 = [] [[maybe_unused]]{};
+
+auto XL0 = [] mutable constexpr mutable {};// expected-error{{cannot appear multiple times}}
+auto XL1 = [] constexpr mutable constexpr {};  // expected-error{{cannot appear multiple times}}
+auto XL2 = []) constexpr mutable constexpr {}; // expected-error{{expected body}}
+auto XL3 = []( constexpr mutable constexpr {}; // expected-error{{invalid storage class specifier}} \
+   // expected-error{{function parameter cannot be constexpr}} \
+   // expected-error{{C++ requires}} \
+   // expected-error{{expected ')'}} \
+   // expected-note{{to match this '('}} \
+   // expected-error{{expected body}} \
+   // expected-warning{{duplicate 'constexpr'}}
Index: clang/test/Parser/cxx2a-template-lambdas.cpp
===
--- clang/test/Parser/cxx2a-template-lambdas.cpp
+++ clang/test/Parser/cxx2a-template-lambdas.cpp
@@ -1,3 +1,4 @@
+// RUN: %clang_cc1 -std=c++2b %s -verify
 // RUN: %clang_cc1 -std=c++2a %s -verify
 
 auto L0 = []<> { }; //expected-error {{cannot be empty}}
Index: clang/test/Parser/cxx1z-constexpr-lambdas.cpp
===
--- clang/test/Parser/cxx1z-constexpr-lambdas.cpp
+++ clang/test/

[PATCH] D98918: [clang][lit] Allow test cases to use the compiler that are used to compile Clang

[Balázs Benics via Phabricator via cfe-commits]

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

Oh, wait a minute. The `config.host_cxx` was already there.
Where is the symmetry :D It looks much better now, thanks.

Regarding D83660 , I'm really looking forward 
to it. It's a shame that it takes so long to land it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98918

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

[PATCH] D98493: [WoA][MSVC] Use default linker setting in MSVC-compatible driver

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

In D98493#2636648 , @maxim-kuvyrkov 
wrote:

> In D98493#2636616 , @phosek wrote:
>
>> They're not part of buildbot but they're not internal, you can see the log 
>> here: 
>> https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket.appspot.com/8852352747956966400/+/u/clang/test/stdout.
>>
>> I noticed D98862 , but it still hasn't 
>> landed and given that it's been 24 hours since your change landed and anyone 
>> who is using `CLANG_DEFAULT_LINKER` is currently broken (I for example use 
>> this option in my local build and tests have been failing for me locally as 
>> well), I think it's it better to revert this change and then reland it once 
>> your fix is ready.
>
> Would you please test D98862  (which I've 
> just updated) on top of this patch and see that completely fixes your bots?

It's passing for me locally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98493

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

[PATCH] D98862: [clang] Update unit-tests after linker selection fix for *-msvc targets

[Petr Hosek via Phabricator via cfe-commits]

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

I've tested this locally and it's working for us.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98862

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

[PATCH] D98918: [clang][lit] Allow test cases to use the compiler that are used to compile Clang

[Ella Ma via Phabricator via cfe-commits]

OikawaKirie added a comment.

In D98918#2636735 , @steakhal wrote:

> 



> Oh, wait a minute. The `config.host_cxx` was already there.
> Where is the symmetry :D It looks much better now, thanks.

Sounds not bad if it matters to you :P

> Regarding D83660 , I'm really looking 
> forward to it. It's a shame that it takes so long to land it.

Please commit this patch on my behalf (Ella Ma ), so 
that I can continue with D83660  with your 
mocked solver.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98918

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

[PATCH] D98935: [WoA][MSVC] Use default linker setting in MSVC-compatible driver [take 2]

[Maxim Kuvyrkov via Phabricator via cfe-commits]

maxim-kuvyrkov created this revision.
maxim-kuvyrkov added reviewers: asl, phosek.
maxim-kuvyrkov requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

At the moment "link.exe" is hard-coded as default linker in MSVC.cpp,
so there's no way to use LLD as default linker for MSVC driver.

This patch adds checking of CLANG_DEFAULT_LINKER to MSVC.cpp and
updates unit-tests that expect link.exe linker to explicitly select it
via -fuse-ld=link, so that buildbots and other builds that set
-DCLANG_DEFAULT_LINKER=foobar don't fail these tests.

This is a squash of

- https://reviews.llvm.org/D98493 (MSVC.cpp change) and
- https://reviews.llvm.org/D98862 (unit-tests change)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98935

Files:
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/test/Driver/Xlinker-args.c
  clang/test/Driver/cl-inputs.c
  clang/test/Driver/cl-link-at-file.c
  clang/test/Driver/cl-link.c
  clang/test/Driver/msvc-link.c
  clang/test/OpenMP/linking.c

Index: clang/test/OpenMP/linking.c
===
--- clang/test/OpenMP/linking.c
+++ clang/test/OpenMP/linking.c
@@ -81,7 +81,7 @@
 // CHECK-LD-OVERRIDE-64: "-lgomp" "-lrt"
 // CHECK-LD-OVERRIDE-64: "-lpthread" "-lc"
 //
-// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: %clang -no-canonical-prefixes -fuse-ld=link %s -### -o %t.o 2>&1 \
 // RUN: -fopenmp=libomp -target x86_64-msvc-win32 -rtlib=platform \
 // RUN:   | FileCheck --check-prefix=CHECK-MSVC-LINK-64 %s
 // CHECK-MSVC-LINK-64: link.exe
@@ -95,7 +95,7 @@
 // SIMD-ONLY11-NOT: libomp
 // SIMD-ONLY11-NOT: libgomp
 //
-// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: %clang -no-canonical-prefixes %s -fuse-ld=link -### -o %t.o 2>&1 \
 // RUN: -fopenmp=libiomp5 -target x86_64-msvc-win32 -rtlib=platform \
 // RUN:   | FileCheck --check-prefix=CHECK-MSVC-ILINK-64 %s
 
Index: clang/test/Driver/msvc-link.c
===
--- clang/test/Driver/msvc-link.c
+++ clang/test/Driver/msvc-link.c
@@ -1,4 +1,4 @@
-// RUN: %clang -target i686-pc-windows-msvc -### %s 2>&1 | FileCheck --check-prefix=BASIC %s
+// RUN: %clang -target i686-pc-windows-msvc -fuse-ld=link -### %s 2>&1 | FileCheck --check-prefix=BASIC %s
 // BASIC: link.exe"
 // BASIC: "-out:a.exe"
 // BASIC: "-defaultlib:libcmt"
@@ -6,7 +6,7 @@
 // BASIC: "-nologo"
 // BASIC-NOT: "-Brepro"
 
-// RUN: %clang -target i686-pc-windows-msvc -shared -o a.dll -### %s 2>&1 | FileCheck --check-prefix=DLL %s
+// RUN: %clang -target i686-pc-windows-msvc -shared -o a.dll -fuse-ld=link -### %s 2>&1 | FileCheck --check-prefix=DLL %s
 // DLL: link.exe"
 // DLL: "-out:a.dll"
 // DLL: "-defaultlib:libcmt"
@@ -19,13 +19,13 @@
 // LIBPATH: "-libpath:/usr/lib"
 // LIBPATH: "-nologo"
 
-// RUN: %clang_cl /Brepro -### -- %s 2>&1 | FileCheck --check-prefix=REPRO %s
+// RUN: %clang_cl /Brepro -fuse-ld=link -### -- %s 2>&1 | FileCheck --check-prefix=REPRO %s
 // REPRO: link.exe"
 // REPRO: "-out:msvc-link.exe"
 // REPRO: "-nologo"
 // REPRO: "-Brepro"
 
-// RUN: %clang_cl /Brepro- -### -- %s 2>&1 | FileCheck --check-prefix=NOREPRO %s
+// RUN: %clang_cl /Brepro- -fuse-ld=link -### -- %s 2>&1 | FileCheck --check-prefix=NOREPRO %s
 // NOREPRO: link.exe"
 // NOREPRO: "-out:msvc-link.exe"
 // NOREPRO: "-nologo"
Index: clang/test/Driver/cl-link.c
===
--- clang/test/Driver/cl-link.c
+++ clang/test/Driver/cl-link.c
@@ -2,14 +2,14 @@
 // be interpreted as a command-line option, e.g. on Mac where %s is commonly
 // under /Users.
 
-// RUN: %clang_cl /Tc%s -### /link foo bar baz 2>&1 | FileCheck --check-prefix=LINK %s
-// RUN: %clang_cl /Tc%s -### /linkfoo bar baz 2>&1 | FileCheck --check-prefix=LINK %s
+// RUN: %clang_cl /Tc%s -fuse-ld=link -### /link foo bar baz 2>&1 | FileCheck --check-prefix=LINK %s
+// RUN: %clang_cl /Tc%s -fuse-ld=link -### /linkfoo bar baz 2>&1 | FileCheck --check-prefix=LINK %s
 // LINK: link.exe
 // LINK: "foo"
 // LINK: "bar"
 // LINK: "baz"
 
-// RUN: %clang_cl -m32 -arch:IA32 --target=i386-pc-win32 /Tc%s -### -fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN %s
+// RUN: %clang_cl -m32 -arch:IA32 --target=i386-pc-win32 /Tc%s -fuse-ld=link -### -fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN %s
 // ASAN: link.exe
 // ASAN: "-debug"
 // ASAN: "-incremental:no"
@@ -19,7 +19,7 @@
 // ASAN: "-wholearchive:{{.*}}clang_rt.asan_cxx-i386.lib"
 // ASAN: "{{.*}}cl-link{{.*}}.obj"
 
-// RUN: %clang_cl -m32 -arch:IA32 --target=i386-pc-win32 /MD /Tc%s -### -fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN-MD %s
+// RUN: %clang_cl -m32 -arch:IA32 --target=i386-pc-win32 /MD /Tc%s -fuse-ld=link -### -fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN-MD %s
 // ASAN-MD: link.exe
 // ASAN-MD: "-debug"
 // ASAN-MD: "-incremental:no"
@@ -29,13 +29,13 @@
 // ASAN-MD: "-wholear

[PATCH] D98862: [clang] Update unit-tests after linker selection fix for *-msvc targets

[Maxim Kuvyrkov via Phabricator via cfe-commits]

maxim-kuvyrkov abandoned this revision.
maxim-kuvyrkov added a comment.

This is now squashed into https://reviews.llvm.org/D98935


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98862

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

[PATCH] D97371: [clang][parser] Remove questionable ProhibitAttributes() call in objc parsing

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 331817.

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

https://reviews.llvm.org/D97371

Files:
  clang/lib/Parse/ParseStmt.cpp
  clang/test/CodeGenObjC/attr-nomerge.m


Index: clang/test/CodeGenObjC/attr-nomerge.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/attr-nomerge.m
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-exceptions -o - %s | FileCheck %s
+
+// Test that the nomerge attribute is applied to function calls
+// in @try, @catch and @finally
+void opaque(void);
+void opaque2(void);
+void opaque3(void);
+
+@class C;
+
+int main(int argc, const char * argv[]) {
+  __attribute__((nomerge)) @try {
+opaque();
+  } @catch(C *c) {
+opaque2();
+  } @finally {
+opaque3();
+  }
+return 0;
+}
+
+// CHECK: call void @opaque() #[[ATTR0:[0-9]+]]
+// CHECK-DAG: call void @opaque2() #[[ATTR0]]
+// CHECK-DAG: call void @opaque3() #[[ATTR0]]
+// CHECK-DAG: attributes #[[ATTR0]] = {{{.*}}nomerge{{.*}}}
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -172,7 +172,6 @@
   switch (Kind) {
   case tok::at: // May be a @try or @throw statement
 {
-  ProhibitAttributes(Attrs); // TODO: is it correct?
   AtLoc = ConsumeToken();  // consume @
   return ParseObjCAtStatement(AtLoc, StmtCtx);
 }


Index: clang/test/CodeGenObjC/attr-nomerge.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/attr-nomerge.m
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-exceptions -o - %s | FileCheck %s
+
+// Test that the nomerge attribute is applied to function calls
+// in @try, @catch and @finally
+void opaque(void);
+void opaque2(void);
+void opaque3(void);
+
+@class C;
+
+int main(int argc, const char * argv[]) {
+  __attribute__((nomerge)) @try {
+opaque();
+  } @catch(C *c) {
+opaque2();
+  } @finally {
+opaque3();
+  }
+return 0;
+}
+
+// CHECK: call void @opaque() #[[ATTR0:[0-9]+]]
+// CHECK-DAG: call void @opaque2() #[[ATTR0]]
+// CHECK-DAG: call void @opaque3() #[[ATTR0]]
+// CHECK-DAG: attributes #[[ATTR0]] = {{{.*}}nomerge{{.*}}}
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -172,7 +172,6 @@
   switch (Kind) {
   case tok::at: // May be a @try or @throw statement
 {
-  ProhibitAttributes(Attrs); // TODO: is it correct?
   AtLoc = ConsumeToken();  // consume @
   return ParseObjCAtStatement(AtLoc, StmtCtx);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98237: [clang-format] Option for empty lines after an access modifier.

[Max Sagebaum via Phabricator via cfe-commits]

Max_S updated this revision to Diff 331818.
Max_S added a comment.

I added more tests for the interaction between MaxEmptyLinesToKeep, 
EmptyLineBeforeAccessModifier and EmptyLineAfterAccessModifier. During these 
tests I recognized, that EmptyLineBeforeAccessModifier = Always,Leave will 
(should adhere) to MaxEmptyLinesToKeep. So the option does not force the new 
line it only applies it if it is missing. I changed the logic for 
EmptyLineAfterAccessModifier accordingly.

Four of the new tests fail, two of them are probably bugs in the implementation 
of EmptyLineBeforeAccessModifier.

The test in line 9354 is set to:

  Style.MaxEmptyLinesToKeep = 0u; // Both force one new line
  Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Always;
  Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Always;
  EXPECT_EQ(NL_B_1_A_1_I_1, format(NL_B_3_A_3_I_3, Style));

So both options should force here the newline, that are removed by 
MaxEmptyLinesToKeep. EmptyLineBeforeAccessModifier removes all lines, so it 
seems that MaxEmptyLinesToKeep has the higher rank here. Is that correct 
behavior?

The test in line 9368 is set to:

  Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Leave;
  Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Leave;
  Style.MaxEmptyLinesToKeep = 1u; 
  EXPECT_EQ(NL_B_1_A_1_I_1, format(NL_B_3_A_3_I_3, Style));

So both options should leave the new lines in place but should adhere to 
MaxEmptyLinesToKeep. EmptyLineBeforeAccessModifier keeps all three lines, so it 
seems that MaxEmptyLinesToKeep has the lower rank here. Is that correct 
behavior?

The tests in line 9462 and 9466 require some additional implementation. 
EmptyLineAfterAccessModifier is adding the three lines since it is configured 
to do so, but EmptyLineBeforeAccessModifier would remove these lines. Since 
EmptyLineBeforeAccessModifier can only access the old file and not the new one. 
It does not know that three lines have been written and probably could not 
remove them.

I would like to implement it in such a way, that EmptyLineAfterAccessModifier 
looks ahead and skips its logic if the next token is a also an access modifier 
such that the logic of EmptyLineBeforeAccessModifier takes precedence. I could 
not find a way to get the next line. Is this possible somehow? Does there exist 
some helper functionality?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98237

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9179,6 +9179,297 @@
Style);
 }
 
+TEST_F(FormatTest, FormatsAfterAccessModifiers) {
+  char const *const NL_After_Never = "struct foo {\n"
+ "private:\n"
+ "  void f() {}\n"
+ "\n"
+ "private:\n"
+ "  int i;\n"
+ "\n"
+ "protected:\n"
+ "  int j;\n"
+ "};\n";
+  char const *const NL_After_Always = "struct foo {\n"
+  "private:\n"
+  "\n"
+  "  void f() {}\n"
+  "\n"
+  "private:\n"
+  "\n"
+  "  int i;\n"
+  "\n"
+  "protected:\n"
+  "\n"
+  "  int j;\n"
+  "};\n";
+
+  char const *const NL_After_Always3Times = "struct foo {\n"
+"private:\n"
+"\n\n\n"
+"  void f() {}\n"
+"\n"
+"private:\n"
+"\n\n\n"
+"  int i;\n"
+"\n"
+"protected:\n"
+"\n\n\n"
+"  int j;\n"
+"};\n";
+
+  char const *const NL_After_Never_Before_Never = "struct foo {\n"
+ 

[PATCH] D98237: [clang-format] Option for empty lines after an access modifier.

[Max Sagebaum via Phabricator via cfe-commits]

Max_S added inline comments.



Comment at: clang/include/clang/Format/Format.h:1914
+ELAAMS_Leave,
+/// Always add empty line after access modifiers.
+/// \code

HazardyKnusperkeks wrote:
> It does not always add, it does enforces one empty line, or am I mistaken?
It adds it if it is missing, but leaves additional lines. All in all 
MaxEmptyLinesToKeep has precedence here. If MaxEmptyLinesToKeep is zero, then 
no line is added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98237

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

[PATCH] D98856: Always emit error for wrong interfaces to scalable vectors, unless cmdline flag is passed.

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen updated this revision to Diff 331820.
sdesmalen added a comment.

Rebased after D98736  already fixed one of the 
tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98856

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  llvm/include/llvm/CodeGen/ValueTypes.h
  llvm/include/llvm/Support/TypeSize.h
  llvm/lib/Analysis/InstructionSimplify.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
  llvm/lib/CodeGen/ValueTypes.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/TypeSize.cpp
  llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp

Index: llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
===
--- llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
+++ llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
@@ -180,7 +180,8 @@
   // Check convenience size scaling methods.
   EXPECT_EQ(v2i32.getSizeInBits() * 2, v4i32.getSizeInBits());
   EXPECT_EQ(2 * nxv2i32.getSizeInBits(), nxv4i32.getSizeInBits());
-  EXPECT_EQ(nxv2f64.getSizeInBits() / 2, nxv2i32.getSizeInBits());
+  EXPECT_EQ(nxv2f64.getSizeInBits().divideCoefficientBy(2),
+nxv2i32.getSizeInBits());
 }
 
 } // end anonymous namespace
Index: llvm/lib/Support/TypeSize.cpp
===
--- /dev/null
+++ llvm/lib/Support/TypeSize.cpp
@@ -0,0 +1,39 @@
+//===- TypeSize.cpp - Wrapper around type sizes--*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "llvm/Support/TypeSize.h"
+#include "llvm/Support/CommandLine.h"
+
+using namespace llvm;
+
+cl::opt llvm::TypeSizeClOpt::ScalableErrorAsWarning(
+"treat-scalable-fixed-error-as-warning", cl::Hidden, cl::init(false),
+cl::desc("Treat issues where a fixed-width property is requested from a "
+ "scalable type as a warning, instead of an error."),
+cl::ZeroOrMore);
+
+TypeSize::operator TypeSize::ScalarTy() const {
+  auto Error = []() {
+report_fatal_error("Cannot convert a scalable size to a fixed-width size!");
+  };
+#ifdef STRICT_FIXED_SIZE_VECTORS
+  if (isScalable())
+Error();
+#else
+  if (isScalable()) {
+if (llvm::TypeSizeClOpt::ScalableErrorAsWarning) {
+  WithColor::warning() << "Compiler has made implicit assumption that "
+  "TypeSize is not scalable. This may or may not "
+  "lead to broken code.\n";
+  return getKnownMinValue();
+} else
+  Error();
+  }
+  return getFixedValue();
+#endif
+}
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -185,6 +185,7 @@
   TrigramIndex.cpp
   Triple.cpp
   Twine.cpp
+  TypeSize.cpp
   Unicode.cpp
   UnicodeCaseFold.cpp
   VersionTuple.cpp
Index: llvm/lib/CodeGen/ValueTypes.cpp
===
--- llvm/lib/CodeGen/ValueTypes.cpp
+++ llvm/lib/CodeGen/ValueTypes.cpp
@@ -14,6 +14,31 @@
 #include "llvm/Support/TypeSize.h"
 using namespace llvm;
 
+unsigned EVT::getVectorNumElements() const {
+  auto Error = []() {
+report_fatal_error(
+"Cannot return the exact number of elements of a scalable vector!");
+  };
+#ifdef STRICT_FIXED_SIZE_VECTORS
+  if (isScalableVector())
+Error();
+#else
+  assert(isVector() && "Invalid vector type!");
+  if (isScalableVector()) {
+if (llvm::TypeSizeClOpt::ScalableErrorAsWarning)
+  WithColor::warning()
+  << "Possible incorrect use of EVT::getVectorNumElements() for "
+ "scalable vector. Scalable flag may be dropped, use "
+ "EVT::getVectorElementCount() instead\n";
+else
+  Error();
+  }
+#endif
+  if (isSimple())
+return V.getVectorNumElements();
+  return getExtendedVectorNumElements();
+}
+
 EVT EVT::changeExtendedTypeToInteger() const {
   assert(isExtended() && "Type is not extended!");
   LLVMContext &Context = LLVMTy->getContext();
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -4778,8 +4778,8 @@
 assert(VT.isVector() && "This DAG node is restricted to vector types.");
 assert(Operand.getValueType().bitsLE(VT) &&
"The input must be the same size or smaller than the result.");
-assert(VT.getVectorNumElements() <
- Operand.getValueType().getVectorNumElements() &&
+assert(VT.getVectorMinNumElements() <
+   Opera

[PATCH] D98891: Automate common diagnostic checking for statement attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 331823.
aaron.ballman set the repository for this revision to rG LLVM Github Monorepo.
aaron.ballman removed a project: clang.
aaron.ballman removed a subscriber: jdoerfert.
aaron.ballman added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Triggering another rebuild, perhaps the CI only works when the review is not 
private?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98891

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Sema/ParsedAttr.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/ParsedAttr.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.fallthrough/p1.cpp
  clang/test/Parser/stmt-attributes.c
  clang/test/Sema/c2x-fallthrough.c
  clang/test/SemaCXX/switch-implicit-fallthrough.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -1828,6 +1828,22 @@
 
 } // end anonymous namespace
 
+static bool isSupportedPragmaClangAttributeSubject(const Record &Subject) {
+  // FIXME: #pragma clang attribute does not currently support statement
+  // attributes, so test whether the subject is one that appertains to a
+  // declaration node. However, it may be reasonable for support for statement
+  // attributes to be added.
+  if (Subject.isSubClassOf("DeclNode") || Subject.isSubClassOf("DeclBase") ||
+  Subject.getName() == "DeclBase")
+return true;
+
+  if (Subject.isSubClassOf("SubsetSubject"))
+return isSupportedPragmaClangAttributeSubject(
+*Subject.getValueAsDef("Base"));
+
+  return false;
+}
+
 static bool doesDeclDeriveFrom(const Record *D, const Record *Base) {
   const Record *CurrentBase = D->getValueAsOptionalDef(BaseFieldName);
   if (!CurrentBase)
@@ -1949,13 +1965,15 @@
 return false;
   const Record *SubjectObj = Attribute.getValueAsDef("Subjects");
   std::vector Subjects = SubjectObj->getValueAsListOfDefs("Subjects");
-  if (Subjects.empty())
-return false;
+  bool HasAtLeastOneValidSubject = false;
   for (const auto *Subject : Subjects) {
+if (!isSupportedPragmaClangAttributeSubject(*Subject))
+  continue;
 if (SubjectsToRules.find(Subject) == SubjectsToRules.end())
   return false;
+HasAtLeastOneValidSubject = true;
   }
-  return true;
+  return HasAtLeastOneValidSubject;
 }
 
 static std::string GenerateTestExpression(ArrayRef LangOpts) {
@@ -2001,6 +2019,8 @@
   const Record *SubjectObj = Attr.getValueAsDef("Subjects");
   std::vector Subjects = SubjectObj->getValueAsListOfDefs("Subjects");
   for (const auto *Subject : Subjects) {
+if (!isSupportedPragmaClangAttributeSubject(*Subject))
+  continue;
 auto It = SubjectsToRules.find(Subject);
 assert(It != SubjectsToRules.end() &&
"This attribute is unsupported by #pragma clang attribute");
@@ -3503,7 +3523,7 @@
 return;
 
   const Record *SubjectObj = Attr.getValueAsDef("Subjects");
-  std::vector Subjects = SubjectObj->getValueAsListOfDefs("Subjects");
+  std::vector Subjects = SubjectObj->getValueAsListOfDefs("Subjects");
 
   // If the list of subjects is empty, it is assumed that the attribute
   // appertains to everything.
@@ -3512,42 +3532,99 @@
 
   bool Warn = SubjectObj->getValueAsDef("Diag")->getValueAsBit("Warn");
 
-  // Otherwise, generate an appertainsTo check specific to this attribute which
-  // checks all of the given subjects against the Decl passed in.
-  //
-  // If D is null, that means the attribute was not applied to a declaration
-  // at all (for instance because it was applied to a type), or that the caller
-  // has determined that the check should fail (perhaps prior to the creation
-  // of the declaration).
-  OS << "bool diagAppertainsToDecl(Sema &S, ";
-  OS << "const ParsedAttr &Attr, const Decl *D) const override {\n";
-  OS << "  if (";
-  for (auto I = Subjects.begin(), E = Subjects.end(); I != E; ++I) {
-// If the subject has custom code associated with it, use the generated
-// function for it. The function cannot be inlined into this check (yet)
-// because it requires the subject to be of a specific type, and were that
-// information inlined here, it would not support an attribute with multiple
-// custom subjects.
-if ((*I)->isSubClassOf("SubsetSubject")) {
-  OS << "!" << functionNameForCustomAppertainsTo(**I) << "(D)";
-} else {
-  OS << "!isa<" << GetSubjectWithSuffix(*I) << ">(D)";
+  // Split the subjects into declaration subjects and statement subjects.
+  // FIXME: subset subjects are added to the declaration list until there are
+  // enough statement attribut

[clang] 4f750f6 - [SystemZ][z/OS] Distinguish between text and binary files on z/OS

[Abhina Sreeskantharajan via cfe-commits]

Author: Abhina Sreeskantharajan
Date: 2021-03-19T08:09:57-04:00
New Revision: 4f750f6ebc412869ce6bb28331313a9c9a9d9af7

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

LOG: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

This patch consists of the initial changes to help distinguish between text and 
binary content correctly on z/OS. I would like to get feedback from Windows 
users on setting OF_None for all ToolOutputFiles. This seems to have been done 
as an optimization to prevent CRLF translation on Windows in the past.

Reviewed By: zibi

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

Added: 


Modified: 
clang/lib/Frontend/CompilerInstance.cpp
clang/lib/Frontend/FrontendActions.cpp
clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
clang/tools/arcmt-test/arcmt-test.cpp
llvm/include/llvm/Support/FileSystem.h
llvm/include/llvm/Support/MemoryBuffer.h
llvm/lib/IRReader/IRReader.cpp
llvm/lib/Support/MemoryBuffer.cpp
llvm/lib/Support/Path.cpp
llvm/lib/Support/ToolOutputFile.cpp
llvm/lib/TableGen/Main.cpp
llvm/utils/FileCheck/FileCheck.cpp

Removed: 




diff  --git a/clang/lib/Frontend/CompilerInstance.cpp 
b/clang/lib/Frontend/CompilerInstance.cpp
index d40240b5b527..284b20cb400a 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -814,15 +814,18 @@ CompilerInstance::createOutputFileImpl(StringRef 
OutputPath, bool Binary,
 TempPath += OutputExtension;
 TempPath += ".tmp";
 int fd;
-std::error_code EC =
-llvm::sys::fs::createUniqueFile(TempPath, fd, TempPath);
+std::error_code EC = llvm::sys::fs::createUniqueFile(
+TempPath, fd, TempPath,
+Binary ? llvm::sys::fs::OF_None : llvm::sys::fs::OF_Text);
 
 if (CreateMissingDirectories &&
 EC == llvm::errc::no_such_file_or_directory) {
   StringRef Parent = llvm::sys::path::parent_path(OutputPath);
   EC = llvm::sys::fs::create_directories(Parent);
   if (!EC) {
-EC = llvm::sys::fs::createUniqueFile(TempPath, fd, TempPath);
+EC = llvm::sys::fs::createUniqueFile(TempPath, fd, TempPath,
+ Binary ? llvm::sys::fs::OF_None
+: llvm::sys::fs::OF_Text);
   }
 }
 

diff  --git a/clang/lib/Frontend/FrontendActions.cpp 
b/clang/lib/Frontend/FrontendActions.cpp
index 38b6f753134c..4e5043b6c75b 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -795,7 +795,7 @@ void PreprocessOnlyAction::ExecuteAction() {
 void PrintPreprocessedAction::ExecuteAction() {
   CompilerInstance &CI = getCompilerInstance();
   // Output file may need to be set to 'Binary', to avoid converting Unix style
-  // line feeds () to Microsoft style line feeds ().
+  // line feeds () to Microsoft style line feeds () on Windows.
   //
   // Look to see what type of line endings the file uses. If there's a
   // CRLF, then we won't open the file up in binary mode. If there is
@@ -807,30 +807,35 @@ void PrintPreprocessedAction::ExecuteAction() {
   // all of their source code on a single line. However, that is still a
   // concern, so if we scan for too long, we'll just assume the file should
   // be opened in binary mode.
-  bool BinaryMode = true;
-  const SourceManager& SM = CI.getSourceManager();
-  if (llvm::Optional Buffer =
-  SM.getBufferOrNone(SM.getMainFileID())) {
-const char *cur = Buffer->getBufferStart();
-const char *end = Buffer->getBufferEnd();
-const char *next = (cur != end) ? cur + 1 : end;
-
-// Limit ourselves to only scanning 256 characters into the source
-// file.  This is mostly a sanity check in case the file has no
-// newlines whatsoever.
-if (end - cur > 256) end = cur + 256;
-
-while (next < end) {
-  if (*cur == 0x0D) {  // CR
-if (*next == 0x0A)  // CRLF
-  BinaryMode = false;
-
-break;
-  } else if (*cur == 0x0A)  // LF
-break;
-
-  ++cur;
-  ++next;
+
+  bool BinaryMode = false;
+  if (llvm::Triple(LLVM_HOST_TRIPLE).isOSWindows()) {
+BinaryMode = true;
+const SourceManager &SM = CI.getSourceManager();
+if (llvm::Optional Buffer =
+SM.getBufferOrNone(SM.getMainFileID())) {
+  const char *cur = Buffer->getBufferStart();
+  const char *end = Buffer->getBufferEnd();
+  const char *next = (cur != end) ? cur + 1 : end;
+
+  // Limit ourselves to only scanning 256 characters into the source
+  // file.  This is mostly a sanity check in case the file has no
+  // newlines whatsoever.
+  if (end - cur > 256)
+end = cur + 256;
+
+  while (next < end) {

[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

[Abhina Sree 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 rG4f750f6ebc41: [SystemZ][z/OS] Distinguish between text and 
binary files on z/OS (authored by abhina.sreeskantharajan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

Files:
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/tools/arcmt-test/arcmt-test.cpp
  llvm/include/llvm/Support/FileSystem.h
  llvm/include/llvm/Support/MemoryBuffer.h
  llvm/lib/IRReader/IRReader.cpp
  llvm/lib/Support/MemoryBuffer.cpp
  llvm/lib/Support/Path.cpp
  llvm/lib/Support/ToolOutputFile.cpp
  llvm/lib/TableGen/Main.cpp
  llvm/utils/FileCheck/FileCheck.cpp

Index: llvm/utils/FileCheck/FileCheck.cpp
===
--- llvm/utils/FileCheck/FileCheck.cpp
+++ llvm/utils/FileCheck/FileCheck.cpp
@@ -821,7 +821,9 @@
 
   // Read the expected strings from the check file.
   ErrorOr> CheckFileOrErr =
-  MemoryBuffer::getFileOrSTDIN(CheckFilename);
+  MemoryBuffer::getFileOrSTDIN(CheckFilename, /*FileSize*/ -1,
+   /*RequiresNullTerminator*/ true,
+   /*IsText*/ true);
   if (std::error_code EC = CheckFileOrErr.getError()) {
 errs() << "Could not open check file '" << CheckFilename
<< "': " << EC.message() << '\n';
@@ -843,7 +845,9 @@
 
   // Open the file to check and add it to SourceMgr.
   ErrorOr> InputFileOrErr =
-  MemoryBuffer::getFileOrSTDIN(InputFilename);
+  MemoryBuffer::getFileOrSTDIN(InputFilename, /*FileSize*/ -1,
+   /*RequiresNullTerminator*/ true,
+   /*IsText*/ true);
   if (InputFilename == "-")
 InputFilename = ""; // Overwrite for improved diagnostic messages
   if (std::error_code EC = InputFileOrErr.getError()) {
Index: llvm/lib/TableGen/Main.cpp
===
--- llvm/lib/TableGen/Main.cpp
+++ llvm/lib/TableGen/Main.cpp
@@ -70,7 +70,7 @@
 return reportError(argv0, "the option -d must be used together with -o\n");
 
   std::error_code EC;
-  ToolOutputFile DepOut(DependFilename, EC, sys::fs::OF_None);
+  ToolOutputFile DepOut(DependFilename, EC, sys::fs::OF_Text);
   if (EC)
 return reportError(argv0, "error opening " + DependFilename + ":" +
   EC.message() + "\n");
@@ -93,7 +93,7 @@
 
   Records.startTimer("Parse, build records");
   ErrorOr> FileOrErr =
-  MemoryBuffer::getFileOrSTDIN(InputFilename);
+  MemoryBuffer::getFileOrSTDIN(InputFilename, -1, true, true);
   if (std::error_code EC = FileOrErr.getError())
 return reportError(argv0, "Could not open input file '" + InputFilename +
   "': " + EC.message() + "\n");
@@ -137,13 +137,14 @@
 // Only updates the real output file if there are any differences.
 // This prevents recompilation of all the files depending on it if there
 // aren't any.
-if (auto ExistingOrErr = MemoryBuffer::getFile(OutputFilename))
+if (auto ExistingOrErr =
+MemoryBuffer::getFile(OutputFilename, -1, true, false, true))
   if (std::move(ExistingOrErr.get())->getBuffer() == Out.str())
 WriteFile = false;
   }
   if (WriteFile) {
 std::error_code EC;
-ToolOutputFile OutFile(OutputFilename, EC, sys::fs::OF_None);
+ToolOutputFile OutFile(OutputFilename, EC, sys::fs::OF_Text);
 if (EC)
   return reportError(argv0, "error opening " + OutputFilename + ": " +
 EC.message() + "\n");
Index: llvm/lib/Support/ToolOutputFile.cpp
===
--- llvm/lib/Support/ToolOutputFile.cpp
+++ llvm/lib/Support/ToolOutputFile.cpp
@@ -11,6 +11,7 @@
 //===--===//
 
 #include "llvm/Support/ToolOutputFile.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Signals.h"
 using namespace llvm;
@@ -45,7 +46,12 @@
 EC = std::error_code();
 return;
   }
-  OSHolder.emplace(Filename, EC, Flags);
+
+  // On Windows, we set the OF_None flag even for text files to avoid
+  // CRLF translation.
+  OSHolder.emplace(
+  Filename, EC,
+  llvm::Triple(LLVM_HOST_TRIPLE).isOSWindows() ? sys::fs::OF_None : Flags);
   OS = OSHolder.getPointer();
   // If open fails, no cleanup is needed.
   if (EC)
Index: llvm/lib/Support/Path.cpp
===
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -167,8 +167,8 @@
 static std::error_code
 createUniqueEntity(const Twine &Model, int &ResultFD,
SmallVe

[PATCH] D96110: [X86] Pass to transform tdpbf16ps intrinsics to scalar operation.

[Pengfei 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/D96110/new/

https://reviews.llvm.org/D96110

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

[PATCH] D95460: [flang][driver] Add forced form flags and -ffixed-line-length

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

Thank you for you reply Joachim,

I see 3 options here:

**OPTION 1:** make Clang accept these options
This would be as simple as removing the `FlangOnlyOption` from the 
corresponding TableGen definitions in Options.td. However, since we added help 
texts to their definitions, these options would be displayed together with 
other options when using `clang --help`. Given that these options are _not 
supported_ by `clang`, I think that we should avoid this upstream. However, 
this could be a quick and easy fix for you downstream. More on `clang -help` in 
this RFC .

**OPTION 2:** tweak the driver
From your description, it seems like it would make more sense to tweak the 
driver so that it can work in .e.g _linker_ mode. In such a mode, it would skip 
the compilation phase and ignore the compilation flags altogether. IMHO this 
would be the most sound approach here, but it may require a bit of work. I 
would start by asking on cfe-dev - it's likely that more projects use `clang` 
as a linker driver. Perhaps it's already possible?

**OPTION 3:** tweak ` -Wno-unknown-argument`
I would expect ` -Wno-unknown-argument` to work here as you initially 
suggested. I'm not familiar with its semantics or e.g. DiagnosticDriverKinds.td 

 (there's a lot in there!). But sometimes you can tweak these things with a 
one-line change and then stuff auto-magically works. I can take a look at some 
point next week. It might be a low hanging fruit or a dead end.

What are you thoughts?

Best,
Andrzej


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95460

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

[PATCH] D98943: [clang][deps] NFC: Extract ModuleID struct

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith.
Herald added a subscriber: mgrang.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch extracts the `ModuleName` and `ContextHash` members of 
`ClangModuleDep`, `FullDependencies` and `ModuleDeps` into a single struct 
`ModuleID`. This makes it easier to understand how the full dependency graph 
works.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98943

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -222,16 +222,16 @@
   return llvm::json::Array(Strings);
 }
 
-static llvm::json::Array toJSONSorted(std::vector V) {
-  llvm::sort(V, [](const ClangModuleDep &A, const ClangModuleDep &B) {
+static llvm::json::Array toJSONSorted(std::vector V) {
+  llvm::sort(V, [](const ModuleID &A, const ModuleID &B) {
 return std::tie(A.ModuleName, A.ContextHash) <
std::tie(B.ModuleName, B.ContextHash);
   });
 
   llvm::json::Array Ret;
-  for (const ClangModuleDep &CMD : V)
+  for (const ModuleID &MID : V)
 Ret.push_back(llvm::json::Object(
-{{"module-name", CMD.ModuleName}, {"context-hash", CMD.ContextHash}}));
+{{"module-name", MID.ModuleName}, {"context-hash", MID.ContextHash}}));
   return Ret;
 }
 
@@ -244,26 +244,25 @@
 
 InputDeps ID;
 ID.FileName = std::string(Input);
-ID.ContextHash = std::move(FD.ContextHash);
+ID.ContextHash = std::move(FD.ID.ContextHash);
 ID.FileDeps = std::move(FD.FileDeps);
 ID.ModuleDeps = std::move(FD.ClangModuleDeps);
 
 std::unique_lock ul(Lock);
 for (const ModuleDeps &MD : FDR.DiscoveredModules) {
-  auto I = Modules.find({MD.ContextHash, MD.ModuleName, 0});
+  auto I = Modules.find({MD.ID, 0});
   if (I != Modules.end()) {
 I->first.InputIndex = std::min(I->first.InputIndex, InputIndex);
 continue;
   }
-  Modules.insert(
-  I, {{MD.ContextHash, MD.ModuleName, InputIndex}, std::move(MD)});
+  Modules.insert(I, {{MD.ID, InputIndex}, std::move(MD)});
 }
 
 if (FullCommandLine)
   ID.AdditonalCommandLine = FD.getAdditionalCommandLine(
-  [&](ClangModuleDep CMD) { return lookupPCMPath(CMD); },
-  [&](ClangModuleDep CMD) -> const ModuleDeps & {
-return lookupModuleDeps(CMD);
+  [&](ModuleID MID) { return lookupPCMPath(MID); },
+  [&](ModuleID MID) -> const ModuleDeps & {
+return lookupModuleDeps(MID);
   });
 
 Inputs.push_back(std::move(ID));
@@ -271,13 +270,13 @@
 
   void printFullOutput(raw_ostream &OS) {
 // Sort the modules by name to get a deterministic order.
-std::vector ModuleNames;
+std::vector ModuleIDs;
 for (auto &&M : Modules)
-  ModuleNames.push_back(M.first);
-llvm::sort(ModuleNames,
-   [](const ContextModulePair &A, const ContextModulePair &B) {
- return std::tie(A.ModuleName, A.InputIndex) <
-std::tie(B.ModuleName, B.InputIndex);
+  ModuleIDs.push_back(M.first);
+llvm::sort(ModuleIDs,
+   [](const IndexedModuleID &A, const IndexedModuleID &B) {
+ return std::tie(A.ID.ModuleName, A.InputIndex) <
+std::tie(B.ID.ModuleName, B.InputIndex);
});
 
 llvm::sort(Inputs, [](const InputDeps &A, const InputDeps &B) {
@@ -287,20 +286,20 @@
 using namespace llvm::json;
 
 Array OutModules;
-for (auto &&ModName : ModuleNames) {
-  auto &MD = Modules[ModName];
+for (auto &&ModID : ModuleIDs) {
+  auto &MD = Modules[ModID];
   Object O{
-  {"name", MD.ModuleName},
-  {"context-hash", MD.ContextHash},
+  {"name", MD.ID.ModuleName},
+  {"context-hash", MD.ID.ContextHash},
   {"file-deps", toJSONSorted(MD.FileDeps)},
   {"clang-module-deps", toJSONSorted(MD.ClangModuleDeps)},
   {"clang-modulemap-file", MD.ClangModuleMapFile},
   {"command-line",
FullCommandLine
? MD.getFullCommandLine(
- [&](ClangModuleDep CMD) { return lookupPCMPath(CMD); },
- [&](ClangModuleDep CMD) -> const ModuleDeps & {
-   return lookupModuleDeps(CMD);
+ [&](ModuleID MID) { return lookupPCMPath(MID); },
+ [&](ModuleID MID) -> const ModuleDeps & {
+   ret

[clang] fa4e729 - Automate common diagnostic checking for statement attributes

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2021-03-19T08:35:38-04:00
New Revision: fa4e72971e05e3c923e11a31e2025361e3425a8b

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

LOG: Automate common diagnostic checking for statement attributes

Clang currently automates a fair amount of diagnostic checking for
declaration attributes based on the declarations in Attr.td. It checks
for things like subject appertainment, number of arguments, language
options, etc. This patch uses the same machinery to perform diagnostic
checking on statement attributes.

Added: 


Modified: 
clang/include/clang/Basic/Attr.td
clang/include/clang/Sema/ParsedAttr.h
clang/include/clang/Sema/Sema.h
clang/lib/Sema/ParsedAttr.cpp
clang/lib/Sema/SemaAttr.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/Sema/SemaStmtAttr.cpp
clang/lib/Sema/SemaType.cpp
clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.fallthrough/p1.cpp
clang/test/Parser/stmt-attributes.c
clang/test/Sema/c2x-fallthrough.c
clang/test/SemaCXX/switch-implicit-fallthrough.cpp
clang/utils/TableGen/ClangAttrEmitter.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index 6b50894512cd..c7b68856aab0 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1183,9 +1183,9 @@ def OpenCLKernel : InheritableAttr {
 
 def OpenCLUnrollHint : StmtAttr {
   let Spellings = [GNU<"opencl_unroll_hint">];
-//  let Subjects = SubjectList<[ForStmt, CXXForRangeStmt, WhileStmt, DoStmt],
-// ErrorDiag, "'for', 'while', and 'do' 
statements">;
-  let Args = [UnsignedArgument<"UnrollHint">];
+  let Subjects = SubjectList<[ForStmt, CXXForRangeStmt, WhileStmt, DoStmt],
+ ErrorDiag, "'for', 'while', and 'do' statements">;
+  let Args = [UnsignedArgument<"UnrollHint", /*opt*/1>];
   let Documentation = [OpenCLUnrollHintDocs];
 }
 
@@ -1326,7 +1326,10 @@ def FallThrough : StmtAttr {
   let Spellings = [CXX11<"", "fallthrough", 201603>,
C2x<"", "fallthrough", 201904>,
CXX11<"clang", "fallthrough">, GCC<"fallthrough">];
-//  let Subjects = [NullStmt];
+  // The attribute only applies to a NullStmt, but we have special fix-it
+  // behavior if applied to a case label.
+  let Subjects = SubjectList<[NullStmt, SwitchCase], ErrorDiag,
+ "empty statements">;
   let Documentation = [FallthroughDocs];
 }
 
@@ -1344,7 +1347,8 @@ def NoMerge : DeclOrStmtAttr {
   let Spellings = [Clang<"nomerge">];
   let Documentation = [NoMergeDocs];
   let InheritEvenIfAlreadyPresent = 1;
-  let Subjects = SubjectList<[Function], ErrorDiag, "functions and 
statements">;
+  let Subjects = SubjectList<[Function, Stmt], ErrorDiag,
+ "functions and statements">;
   let SimpleHandler = 1;
 }
 
@@ -3467,6 +3471,7 @@ def LoopHint : Attr {
   }];
 
   let Documentation = [LoopHintDocs, UnrollHintDocs];
+  let HasCustomParsing = 1;
 }
 
 def CapturedRecord : InheritableAttr {

diff  --git a/clang/include/clang/Sema/ParsedAttr.h 
b/clang/include/clang/Sema/ParsedAttr.h
index 0d731d9150a8..a3d82fcd84f7 100644
--- a/clang/include/clang/Sema/ParsedAttr.h
+++ b/clang/include/clang/Sema/ParsedAttr.h
@@ -39,6 +39,7 @@ class IdentifierInfo;
 class LangOptions;
 class ParsedAttr;
 class Sema;
+class Stmt;
 class TargetInfo;
 
 struct ParsedAttrInfo {
@@ -80,6 +81,11 @@ struct ParsedAttrInfo {
 const Decl *D) const {
 return true;
   }
+  /// Check if this attribute appertains to St, and issue a diagnostic if not.
+  virtual bool diagAppertainsToStmt(Sema &S, const ParsedAttr &Attr,
+const Stmt *St) const {
+return true;
+  }
   /// Check if this attribute is allowed by the language we are compiling, and
   /// issue a diagnostic if not.
   virtual bool diagLangOpts(Sema &S, const ParsedAttr &Attr) const {
@@ -592,6 +598,7 @@ class ParsedAttr final
   unsigned getMaxArgs() const;
   bool hasVariadicArg() const;
   bool diagnoseAppertainsTo(class Sema &S, const Decl *D) const;
+  bool diagnoseAppertainsTo(class Sema &S, const Stmt *St) const;
   bool appliesToDecl(const Decl *D, attr::SubjectMatchRule MatchRule) const;
   void getMatchRules(const LangOptions &LangOpts,
  SmallVectorImpl>

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index b144587650eb..6fae208f74e7 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -4260,6 +4260,13 @@ class Sema final {
 
   void checkUnusedDeclAttributes(Declarator &D);
 
+  /// Handles semantic checking for features that are common to all attributes,
+  /// such 

[PATCH] D98891: Automate common diagnostic checking for statement attributes

[Aaron Ballman via Phabricator via cfe-commits]

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

Accepting so I can close (the review was only created so I could run the CI and 
wasn't meant to be public in the first place).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98891

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

[PATCH] D98891: Automate common diagnostic checking for statement attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

This was committed in fa4e72971e05e3c923e11a31e2025361e3425a8b 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98891

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

[PATCH] D98341: [analyzer][solver] Prevent infeasible states (PR49490)

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

The assertion `areFeasible(Constraints)` triggered on trunk.
More details in the Bugzilla ticket 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98341

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

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

Ah, I wanted to give it a go, but the bots caught an assertion failure for the 
parent revision of this. See the details at the Bugzilla ticket 
.

What is more concerning is that this patch resolves that assertion failure. I 
guess it implies it can not be NFC?
Regardless, I'm gonna wait to get that issue fixed before I have a deeper look.




Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:191
+using IntTypes = ::testing::Types;
+TYPED_TEST_CASE(RangeSetTest, IntTypes);

You should probably test `_ExtInt`s as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86465

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

[PATCH] D98514: [RGT] Fix ASTMatchersTest so all assertions are executed

[Paul Robinson via Phabricator via cfe-commits]

probinson added a reviewer: gribozavr.
probinson added a comment.

+gribozavr who has been in this file most recently


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98514

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

[PATCH] D98935: [WoA][MSVC] Use default linker setting in MSVC-compatible driver [take 2]

[Maxim Kuvyrkov via Phabricator via cfe-commits]

maxim-kuvyrkov updated this revision to Diff 331849.
maxim-kuvyrkov added a comment.

NFC: Make clang-format happy


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98935

Files:
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/test/Driver/Xlinker-args.c
  clang/test/Driver/cl-inputs.c
  clang/test/Driver/cl-link-at-file.c
  clang/test/Driver/cl-link.c
  clang/test/Driver/msvc-link.c
  clang/test/OpenMP/linking.c

Index: clang/test/OpenMP/linking.c
===
--- clang/test/OpenMP/linking.c
+++ clang/test/OpenMP/linking.c
@@ -81,7 +81,7 @@
 // CHECK-LD-OVERRIDE-64: "-lgomp" "-lrt"
 // CHECK-LD-OVERRIDE-64: "-lpthread" "-lc"
 //
-// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: %clang -no-canonical-prefixes -fuse-ld=link %s -### -o %t.o 2>&1 \
 // RUN: -fopenmp=libomp -target x86_64-msvc-win32 -rtlib=platform \
 // RUN:   | FileCheck --check-prefix=CHECK-MSVC-LINK-64 %s
 // CHECK-MSVC-LINK-64: link.exe
@@ -95,7 +95,7 @@
 // SIMD-ONLY11-NOT: libomp
 // SIMD-ONLY11-NOT: libgomp
 //
-// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: %clang -no-canonical-prefixes %s -fuse-ld=link -### -o %t.o 2>&1 \
 // RUN: -fopenmp=libiomp5 -target x86_64-msvc-win32 -rtlib=platform \
 // RUN:   | FileCheck --check-prefix=CHECK-MSVC-ILINK-64 %s
 
Index: clang/test/Driver/msvc-link.c
===
--- clang/test/Driver/msvc-link.c
+++ clang/test/Driver/msvc-link.c
@@ -1,4 +1,4 @@
-// RUN: %clang -target i686-pc-windows-msvc -### %s 2>&1 | FileCheck --check-prefix=BASIC %s
+// RUN: %clang -target i686-pc-windows-msvc -fuse-ld=link -### %s 2>&1 | FileCheck --check-prefix=BASIC %s
 // BASIC: link.exe"
 // BASIC: "-out:a.exe"
 // BASIC: "-defaultlib:libcmt"
@@ -6,7 +6,7 @@
 // BASIC: "-nologo"
 // BASIC-NOT: "-Brepro"
 
-// RUN: %clang -target i686-pc-windows-msvc -shared -o a.dll -### %s 2>&1 | FileCheck --check-prefix=DLL %s
+// RUN: %clang -target i686-pc-windows-msvc -shared -o a.dll -fuse-ld=link -### %s 2>&1 | FileCheck --check-prefix=DLL %s
 // DLL: link.exe"
 // DLL: "-out:a.dll"
 // DLL: "-defaultlib:libcmt"
@@ -19,13 +19,13 @@
 // LIBPATH: "-libpath:/usr/lib"
 // LIBPATH: "-nologo"
 
-// RUN: %clang_cl /Brepro -### -- %s 2>&1 | FileCheck --check-prefix=REPRO %s
+// RUN: %clang_cl /Brepro -fuse-ld=link -### -- %s 2>&1 | FileCheck --check-prefix=REPRO %s
 // REPRO: link.exe"
 // REPRO: "-out:msvc-link.exe"
 // REPRO: "-nologo"
 // REPRO: "-Brepro"
 
-// RUN: %clang_cl /Brepro- -### -- %s 2>&1 | FileCheck --check-prefix=NOREPRO %s
+// RUN: %clang_cl /Brepro- -fuse-ld=link -### -- %s 2>&1 | FileCheck --check-prefix=NOREPRO %s
 // NOREPRO: link.exe"
 // NOREPRO: "-out:msvc-link.exe"
 // NOREPRO: "-nologo"
Index: clang/test/Driver/cl-link.c
===
--- clang/test/Driver/cl-link.c
+++ clang/test/Driver/cl-link.c
@@ -2,14 +2,14 @@
 // be interpreted as a command-line option, e.g. on Mac where %s is commonly
 // under /Users.
 
-// RUN: %clang_cl /Tc%s -### /link foo bar baz 2>&1 | FileCheck --check-prefix=LINK %s
-// RUN: %clang_cl /Tc%s -### /linkfoo bar baz 2>&1 | FileCheck --check-prefix=LINK %s
+// RUN: %clang_cl /Tc%s -fuse-ld=link -### /link foo bar baz 2>&1 | FileCheck --check-prefix=LINK %s
+// RUN: %clang_cl /Tc%s -fuse-ld=link -### /linkfoo bar baz 2>&1 | FileCheck --check-prefix=LINK %s
 // LINK: link.exe
 // LINK: "foo"
 // LINK: "bar"
 // LINK: "baz"
 
-// RUN: %clang_cl -m32 -arch:IA32 --target=i386-pc-win32 /Tc%s -### -fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN %s
+// RUN: %clang_cl -m32 -arch:IA32 --target=i386-pc-win32 /Tc%s -fuse-ld=link -### -fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN %s
 // ASAN: link.exe
 // ASAN: "-debug"
 // ASAN: "-incremental:no"
@@ -19,7 +19,7 @@
 // ASAN: "-wholearchive:{{.*}}clang_rt.asan_cxx-i386.lib"
 // ASAN: "{{.*}}cl-link{{.*}}.obj"
 
-// RUN: %clang_cl -m32 -arch:IA32 --target=i386-pc-win32 /MD /Tc%s -### -fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN-MD %s
+// RUN: %clang_cl -m32 -arch:IA32 --target=i386-pc-win32 /MD /Tc%s -fuse-ld=link -### -fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN-MD %s
 // ASAN-MD: link.exe
 // ASAN-MD: "-debug"
 // ASAN-MD: "-incremental:no"
@@ -29,13 +29,13 @@
 // ASAN-MD: "-wholearchive:{{.*}}clang_rt.asan_dynamic_runtime_thunk-i386.lib"
 // ASAN-MD: "{{.*}}cl-link{{.*}}.obj"
 
-// RUN: %clang_cl /LD -### /Tc%s 2>&1 | FileCheck --check-prefix=DLL %s
-// RUN: %clang_cl /LDd -### /Tc%s 2>&1 | FileCheck --check-prefix=DLL %s
+// RUN: %clang_cl /LD -fuse-ld=link -### /Tc%s 2>&1 | FileCheck --check-prefix=DLL %s
+// RUN: %clang_cl /LDd -fuse-ld=link -### /Tc%s 2>&1 | FileCheck --check-prefix=DLL %s
 // DLL: link.exe
 // "-dll"
 
-// RUN: %clang_cl -m32 -arch:IA32 --target=i386-pc-win32 /LD /Tc%s -### -fsanitize=address 

[PATCH] D98935: [WoA][MSVC] Use default linker setting in MSVC-compatible driver [take 2]

[Maxim Kuvyrkov via Phabricator via cfe-commits]

maxim-kuvyrkov accepted this revision.
maxim-kuvyrkov added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: mstorsjo.

This was reviewed in https://reviews.llvm.org/D98493 and 
https://reviews.llvm.org/D98862.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98935

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

[PATCH] D98935: [WoA][MSVC] Use default linker setting in MSVC-compatible driver [take 2]

[Maxim Kuvyrkov 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 rG2049fe58903b: [WoA][MSVC] Use default linker setting in 
MSVC-compatible driver [take 2] (authored by maxim-kuvyrkov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98935

Files:
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/test/Driver/Xlinker-args.c
  clang/test/Driver/cl-inputs.c
  clang/test/Driver/cl-link-at-file.c
  clang/test/Driver/cl-link.c
  clang/test/Driver/msvc-link.c
  clang/test/OpenMP/linking.c

Index: clang/test/OpenMP/linking.c
===
--- clang/test/OpenMP/linking.c
+++ clang/test/OpenMP/linking.c
@@ -81,7 +81,7 @@
 // CHECK-LD-OVERRIDE-64: "-lgomp" "-lrt"
 // CHECK-LD-OVERRIDE-64: "-lpthread" "-lc"
 //
-// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: %clang -no-canonical-prefixes -fuse-ld=link %s -### -o %t.o 2>&1 \
 // RUN: -fopenmp=libomp -target x86_64-msvc-win32 -rtlib=platform \
 // RUN:   | FileCheck --check-prefix=CHECK-MSVC-LINK-64 %s
 // CHECK-MSVC-LINK-64: link.exe
@@ -95,7 +95,7 @@
 // SIMD-ONLY11-NOT: libomp
 // SIMD-ONLY11-NOT: libgomp
 //
-// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: %clang -no-canonical-prefixes %s -fuse-ld=link -### -o %t.o 2>&1 \
 // RUN: -fopenmp=libiomp5 -target x86_64-msvc-win32 -rtlib=platform \
 // RUN:   | FileCheck --check-prefix=CHECK-MSVC-ILINK-64 %s
 
Index: clang/test/Driver/msvc-link.c
===
--- clang/test/Driver/msvc-link.c
+++ clang/test/Driver/msvc-link.c
@@ -1,4 +1,4 @@
-// RUN: %clang -target i686-pc-windows-msvc -### %s 2>&1 | FileCheck --check-prefix=BASIC %s
+// RUN: %clang -target i686-pc-windows-msvc -fuse-ld=link -### %s 2>&1 | FileCheck --check-prefix=BASIC %s
 // BASIC: link.exe"
 // BASIC: "-out:a.exe"
 // BASIC: "-defaultlib:libcmt"
@@ -6,7 +6,7 @@
 // BASIC: "-nologo"
 // BASIC-NOT: "-Brepro"
 
-// RUN: %clang -target i686-pc-windows-msvc -shared -o a.dll -### %s 2>&1 | FileCheck --check-prefix=DLL %s
+// RUN: %clang -target i686-pc-windows-msvc -shared -o a.dll -fuse-ld=link -### %s 2>&1 | FileCheck --check-prefix=DLL %s
 // DLL: link.exe"
 // DLL: "-out:a.dll"
 // DLL: "-defaultlib:libcmt"
@@ -19,13 +19,13 @@
 // LIBPATH: "-libpath:/usr/lib"
 // LIBPATH: "-nologo"
 
-// RUN: %clang_cl /Brepro -### -- %s 2>&1 | FileCheck --check-prefix=REPRO %s
+// RUN: %clang_cl /Brepro -fuse-ld=link -### -- %s 2>&1 | FileCheck --check-prefix=REPRO %s
 // REPRO: link.exe"
 // REPRO: "-out:msvc-link.exe"
 // REPRO: "-nologo"
 // REPRO: "-Brepro"
 
-// RUN: %clang_cl /Brepro- -### -- %s 2>&1 | FileCheck --check-prefix=NOREPRO %s
+// RUN: %clang_cl /Brepro- -fuse-ld=link -### -- %s 2>&1 | FileCheck --check-prefix=NOREPRO %s
 // NOREPRO: link.exe"
 // NOREPRO: "-out:msvc-link.exe"
 // NOREPRO: "-nologo"
Index: clang/test/Driver/cl-link.c
===
--- clang/test/Driver/cl-link.c
+++ clang/test/Driver/cl-link.c
@@ -2,14 +2,14 @@
 // be interpreted as a command-line option, e.g. on Mac where %s is commonly
 // under /Users.
 
-// RUN: %clang_cl /Tc%s -### /link foo bar baz 2>&1 | FileCheck --check-prefix=LINK %s
-// RUN: %clang_cl /Tc%s -### /linkfoo bar baz 2>&1 | FileCheck --check-prefix=LINK %s
+// RUN: %clang_cl /Tc%s -fuse-ld=link -### /link foo bar baz 2>&1 | FileCheck --check-prefix=LINK %s
+// RUN: %clang_cl /Tc%s -fuse-ld=link -### /linkfoo bar baz 2>&1 | FileCheck --check-prefix=LINK %s
 // LINK: link.exe
 // LINK: "foo"
 // LINK: "bar"
 // LINK: "baz"
 
-// RUN: %clang_cl -m32 -arch:IA32 --target=i386-pc-win32 /Tc%s -### -fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN %s
+// RUN: %clang_cl -m32 -arch:IA32 --target=i386-pc-win32 /Tc%s -fuse-ld=link -### -fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN %s
 // ASAN: link.exe
 // ASAN: "-debug"
 // ASAN: "-incremental:no"
@@ -19,7 +19,7 @@
 // ASAN: "-wholearchive:{{.*}}clang_rt.asan_cxx-i386.lib"
 // ASAN: "{{.*}}cl-link{{.*}}.obj"
 
-// RUN: %clang_cl -m32 -arch:IA32 --target=i386-pc-win32 /MD /Tc%s -### -fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN-MD %s
+// RUN: %clang_cl -m32 -arch:IA32 --target=i386-pc-win32 /MD /Tc%s -fuse-ld=link -### -fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN-MD %s
 // ASAN-MD: link.exe
 // ASAN-MD: "-debug"
 // ASAN-MD: "-incremental:no"
@@ -29,13 +29,13 @@
 // ASAN-MD: "-wholearchive:{{.*}}clang_rt.asan_dynamic_runtime_thunk-i386.lib"
 // ASAN-MD: "{{.*}}cl-link{{.*}}.obj"
 
-// RUN: %clang_cl /LD -### /Tc%s 2>&1 | FileCheck --check-prefix=DLL %s
-// RUN: %clang_cl /LDd -### /Tc%s 2>&1 | FileCheck --check-prefix=DLL %s
+// RUN: %clang_cl /LD -fuse-ld=link -### /Tc%s 2>&1 | FileCheck --check-prefix=DLL %s
+// RUN: %clang_cl /LDd -fuse-ld=link -### /Tc%s 2>&1 | Fil

[clang] 2049fe5 - [WoA][MSVC] Use default linker setting in MSVC-compatible driver [take 2]

[Maxim Kuvyrkov via cfe-commits]

Author: Maxim Kuvyrkov
Date: 2021-03-19T13:38:03Z
New Revision: 2049fe58903b68f66872a18e608f40e5233b55fb

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

LOG: [WoA][MSVC] Use default linker setting in MSVC-compatible driver [take 2]

At the moment "link.exe" is hard-coded as default linker in MSVC.cpp,
so there's no way to use LLD as default linker for MSVC driver.

This patch adds checking of CLANG_DEFAULT_LINKER to MSVC.cpp and
updates unit-tests that expect link.exe linker to explicitly select it
via -fuse-ld=link, so that buildbots and other builds that set
-DCLANG_DEFAULT_LINKER=foobar don't fail these tests.

This is a squash of
- https://reviews.llvm.org/D98493 (MSVC.cpp change) and
- https://reviews.llvm.org/D98862 (unit-tests change)

Reviewed By: maxim-kuvyrkov

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/MSVC.cpp
clang/test/Driver/Xlinker-args.c
clang/test/Driver/cl-inputs.c
clang/test/Driver/cl-link-at-file.c
clang/test/Driver/cl-link.c
clang/test/Driver/msvc-link.c
clang/test/OpenMP/linking.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/MSVC.cpp 
b/clang/lib/Driver/ToolChains/MSVC.cpp
index 96de02378ca2..877919e11464 100644
--- a/clang/lib/Driver/ToolChains/MSVC.cpp
+++ b/clang/lib/Driver/ToolChains/MSVC.cpp
@@ -11,6 +11,7 @@
 #include "Darwin.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/Version.h"
+#include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/DriverDiagnostic.h"
@@ -577,7 +578,10 @@ void visualstudio::Linker::ConstructJob(Compilation &C, 
const JobAction &JA,
   // translate 'lld' into 'lld-link', and in the case of the regular msvc
   // linker, we need to use a special search algorithm.
   llvm::SmallString<128> linkPath;
-  StringRef Linker = Args.getLastArgValue(options::OPT_fuse_ld_EQ, "link");
+  StringRef Linker
+= Args.getLastArgValue(options::OPT_fuse_ld_EQ, CLANG_DEFAULT_LINKER);
+  if (Linker.empty())
+Linker = "link";
   if (Linker.equals_lower("lld"))
 Linker = "lld-link";
 

diff  --git a/clang/test/Driver/Xlinker-args.c 
b/clang/test/Driver/Xlinker-args.c
index a44957cd8aef..cb045a1d40ac 100644
--- a/clang/test/Driver/Xlinker-args.c
+++ b/clang/test/Driver/Xlinker-args.c
@@ -17,7 +17,7 @@
 // LINUX: "--no-demangle" "-e" "_start" "one" "two" "three" "four" "-z" "five" 
"-r" {{.*}} "-T" "a.lds"
 
 // Check that we forward '-Xlinker' and '-Wl,' on Windows.
-// RUN: %clang -target i686-pc-win32 -### \
+// RUN: %clang -target i686-pc-win32 -fuse-ld=link -### \
 // RUN:   -Xlinker one -Wl,two %s 2>&1 | \
 // RUN:   FileCheck -check-prefix=WIN %s
 // WIN: link.exe

diff  --git a/clang/test/Driver/cl-inputs.c b/clang/test/Driver/cl-inputs.c
index 59455a0aa5e5..8eb44517ee16 100644
--- a/clang/test/Driver/cl-inputs.c
+++ b/clang/test/Driver/cl-inputs.c
@@ -50,16 +50,16 @@
 // RUN: %clang_cl -### /Tc - 2>&1 | FileCheck -check-prefix=STDINTc %s
 // STDINTc: "-x" "c"
 
-// RUN: env LIB=%S/Inputs/cl-libs %clang_cl -### -- %s cl-test.lib 2>&1 | 
FileCheck -check-prefix=LIBINPUT %s
+// RUN: env LIB=%S/Inputs/cl-libs %clang_cl -fuse-ld=link -### -- %s 
cl-test.lib 2>&1 | FileCheck -check-prefix=LIBINPUT %s
 // LIBINPUT: link.exe"
 // LIBINPUT: "cl-test.lib"
 
-// RUN: env LIB=%S/Inputs/cl-libs %clang_cl -### -- %s cl-test2.lib 2>&1 | 
FileCheck -check-prefix=LIBINPUT2 %s
+// RUN: env LIB=%S/Inputs/cl-libs %clang_cl -fuse-ld=link -### -- %s 
cl-test2.lib 2>&1 | FileCheck -check-prefix=LIBINPUT2 %s
 // LIBINPUT2: error: no such file or directory: 'cl-test2.lib'
 // LIBINPUT2: link.exe"
 // LIBINPUT2-NOT: "cl-test2.lib"
 
-// RUN: %clang_cl -### -- %s /nonexisting.lib 2>&1 | FileCheck 
-check-prefix=LIBINPUT3 %s
+// RUN: %clang_cl -fuse-ld=link -### -- %s /nonexisting.lib 2>&1 | FileCheck 
-check-prefix=LIBINPUT3 %s
 // LIBINPUT3: error: no such file or directory: '/nonexisting.lib'
 // LIBINPUT3: link.exe"
 // LIBINPUT3-NOT: "/nonexisting.lib"

diff  --git a/clang/test/Driver/cl-link-at-file.c 
b/clang/test/Driver/cl-link-at-file.c
index 50ae07fadf5b..4e665f89b74e 100644
--- a/clang/test/Driver/cl-link-at-file.c
+++ b/clang/test/Driver/cl-link-at-file.c
@@ -7,7 +7,7 @@
 
 // RUN: echo /link bar.lib baz.lib > %t.args
 // RUN: touch %t.obj
-// RUN: %clang_cl -### @%t.args -- %t.obj 2>&1 | FileCheck %s 
-check-prefix=ARGS
+// RUN: %clang_cl -fuse-ld=link -### @%t.args -- %t.obj 2>&1 | FileCheck %s 
-check-prefix=ARGS
 // If the "/link" option captures all remaining args beyond its response file,
 // it will also capture "--" and our input argument. In this case, Clang will
 // be clueless and will emit "argument unused" warnings. If PR17239 is properly

diff  --gi

[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

I did not follow the discussion closely but we (CodeChecker team) might have a 
similar problem.
Consider this: https://godbolt.org/z/835P38

  int do_bifurcation(int p) { return p < 0; }
  
  int b(int x, int y) {
int tmp = 13 / y;  // y can't be 0.
(void)tmp;
  
int p0 = do_bifurcation(x);  // There is a path where p0 is 0.
  
int div = p0 * y; // So, div also becomes 0 on that path.
return 1 / div;
  }

However, the bugreport tells us that you do a division by zero, which was 
initialized a line above.

Do you think it is a related issue @NoQ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97183

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

[clang] a9fc44c - [TableGen] Improve handling of template arguments

[Paul C. Anagnostopoulos via cfe-commits]

Author: Paul C. Anagnostopoulos
Date: 2021-03-19T09:57:53-04:00
New Revision: a9fc44c5573208859c2550382755098d750fc47d

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

LOG: [TableGen] Improve handling of template arguments

This requires changes to TableGen files and some C++ files due to
incompatible multiclass template arguments that slipped through
before the improved handling.

Added: 
llvm/test/TableGen/template-args.td

Modified: 
clang/utils/TableGen/MveEmitter.cpp
llvm/docs/TableGen/ProgRef.rst
llvm/include/llvm/TableGen/Record.h
llvm/lib/TableGen/Record.cpp
llvm/lib/TableGen/TGParser.cpp
llvm/lib/TableGen/TGParser.h
llvm/test/TableGen/self-reference-typeerror.td

Removed: 




diff  --git a/clang/utils/TableGen/MveEmitter.cpp 
b/clang/utils/TableGen/MveEmitter.cpp
index e9ae08ac4c05..091af2dc52a1 100644
--- a/clang/utils/TableGen/MveEmitter.cpp
+++ b/clang/utils/TableGen/MveEmitter.cpp
@@ -1272,6 +1272,13 @@ Result::Ptr EmitterBase::getCodeForDagArg(DagInit *D, 
unsigned ArgNum,
 return it->second;
   }
 
+  // Sometimes the Arg is a bit. Prior to multiclass template argument
+  // checking, integers would sneak through the bit declaration,
+  // but now they really are bits.
+  if (auto *BI = dyn_cast(Arg))
+return std::make_shared(getScalarType("u32"),
+  BI->getValue());
+
   if (auto *II = dyn_cast(Arg))
 return std::make_shared(getScalarType("u32"),
   II->getValue());
@@ -1287,7 +1294,11 @@ Result::Ptr EmitterBase::getCodeForDagArg(DagInit *D, 
unsigned ArgNum,
 }
   }
 
-  PrintFatalError("bad dag argument type for code generation");
+  PrintError("bad DAG argument type for code generation");
+  PrintNote("DAG: " + D->getAsString());
+  if (TypedInit *Typed = dyn_cast(Arg))
+PrintNote("argument type: " + Typed->getType()->getAsString());
+  PrintFatalNote("argument number " + Twine(ArgNum) + ": " + 
Arg->getAsString());
 }
 
 Result::Ptr EmitterBase::getCodeForArg(unsigned ArgNum, const Type *ArgType,

diff  --git a/llvm/docs/TableGen/ProgRef.rst b/llvm/docs/TableGen/ProgRef.rst
index c60bffef3ed2..9799e29a63e6 100644
--- a/llvm/docs/TableGen/ProgRef.rst
+++ b/llvm/docs/TableGen/ProgRef.rst
@@ -299,7 +299,7 @@ wide range of records conveniently and compactly.
 :token:`ClassID`
 Specifying a class name in a type context indicates
 that the type of the defined value must
-be a subclass of the specified class.  This is useful in conjunction with
+be a subclass of the specified class. This is useful in conjunction with
 the ``list`` type; for example, to constrain the elements of the list to a
 common base class (e.g., a ``list`` can only contain definitions
 derived from the ``Register`` class).
@@ -554,19 +554,22 @@ classes and records can inherit.
TemplateArgDecl: `Type` `TokIdentifier` ["=" `Value`]
 
 A class can be parameterized by a list of "template arguments," whose values
-can be used in the class's record body.  These template arguments are
+can be used in the class's record body. These template arguments are
 specified each time the class is inherited by another class or record.
 
 If a template argument is not assigned a default value with ``=``, it is
 uninitialized (has the "value" ``?``) and must be specified in the template
-argument list when the class is inherited. If an argument is assigned a
-default value, then it need not be specified in the argument list. The
-template argument default values are evaluated from left to right.
+argument list when the class is inherited (required argument). If an
+argument is assigned a default value, then it need not be specified in the
+argument list (optional argument). In the declaration, all required template
+arguments must precede any optional arguments. The template argument default
+values are evaluated from left to right.
 
 The :token:`RecordBody` is defined below. It can include a list of
-superclasses from which the current class inherits, along with field 
definitions
-and other statements. When a class ``C`` inherits from another class ``D``,
-the fields of ``D`` are effectively merged into the fields of ``C``.
+superclasses from which the current class inherits, along with field
+definitions and other statements. When a class ``C`` inherits from another
+class ``D``, the fields of ``D`` are effectively merged into the fields of
+``C``.
 
 A given class can only be defined once. A ``class`` statement is
 considered to define the class if *any* of the following are true (the
@@ -605,7 +608,7 @@ of the fields of the class or record.
RecordBody: `ParentClassList` `Body`
ParentClassList: [":" `ParentClassListNE`]
ParentClassLi

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

[Whisperity via Phabricator via cfe-commits]

whisperity added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp:347
+// Checks whether ArgType converts implicitly to ParamType.
+static bool areTypesCompatible(QualType ArgType, QualType ParamType,
+   const ASTContext &Ctx) {

aaron.ballman wrote:
> whisperity wrote:
> > aaron.ballman wrote:
> > > whisperity wrote:
> > > > whisperity wrote:
> > > > > aaron.ballman wrote:
> > > > > > It seems like we're doing an awful lot of the same work as 
> > > > > > `ASTContext::typesAreCompatible()` and type compatibility rules are 
> > > > > > pretty complex, so I worry about this implementation being 
> > > > > > different than the `ASTContext` implementation. Have you explored 
> > > > > > whether we can reuse more of the logic from `ASTContext` here, or 
> > > > > > are they doing fundamentally different kinds of type compatibility 
> > > > > > checks?
> > > > > No, I didn't know that function even existed. This check must be 
> > > > > older than that function.
> > > > Actually, no, that function is pretty old... However, that function, 
> > > > and all the function it subsequently calls, require a **non-const** 
> > > > `ASTContext`. I have changed `ASTContext`:
> > > > 
> > > > ```
> > > > ╰─ git diff --cached --stat
> > > >  clang/include/clang/AST/ASTContext.h   
> > > >   | 67 
> > > > ---
> > > >  clang/lib/AST/ASTContext.cpp   
> > > >   | 72 
> > > > +---
> > > >  2 files changed, 73 insertions(+), 66 deletions(-)
> > > > ```
> > > > 
> > > > making related member functions and internal static functions take 
> > > > `const ASTContext &`/`*`.
> > > > 
> > > > This, by itself, did not break any of the tests of `check-clang 
> > > > check-clang-unit check-clang-tools check-clang-extra-unit`!
> > > Doubtful -- `typesAreCompatible()` is critical for checking the semantics 
> > > of assignment in C, overloading in C++, etc. It may have simply been 
> > > overlooked when writing this check. Given the complexities of type 
> > > checking, my intuition is that we should be leaning on `ASTContext` for 
> > > as much of this functionality as we can get away with. That will also get 
> > > us nice "extras" like caring about address spaces, ARC, etc which are 
> > > what got me worried when I started looking at this implementation.
> > @aaron.ballman Changing the function to be 
> > 
> > ```
> > return Ctx.typesAreCompatible(ArgType, ParamType);
> > ```
> > 
> > will make the checker miss the test case about `T`/`const T&` mixup.
> > 
> > ```
> > void value_const_reference(int ll, const int& kk);
> > void const_ref_value_swapped() {
> >   const int& kk = 42;
> >   const int& ll = 42;
> >   value_const_reference(kk, ll);
> >   // error: CHECK-MESSAGES: expected string not found in input:
> >   // warning: 1st argument 'kk' (passed to 'll') looks like it 
> > might be swapped with the 2nd, 'll' (passed to 'kk')
> > }
> > ```
> > 
> > 
> > 
> > Setting `bool CompareUnqualified = true` (3rd argument to 
> > `typesAreCompatible`) doesn't help either.
> > Which is valid from `typesAreCompatible`'s perspective... that function 
> > answer the question, applied to the context of the above test: //"Is 
> > `ll = kk;` valid?"//, which is obviously **false** as both are 
> > `const T&`s.
> Yeah, I expect there to be a delta between the work this check is doing and 
> the existing work done by `typesAreCompatible()`. However, given the 
> complexity of type compatibility checking, I'd say it's better for us to try 
> to refactor the ASTContext functionality so that we can share as much of the 
> implementation as plausible rather than duplicate some really difficult logic 
> in the tidy check.
I talked with @aaron.ballman in private about this a little, and unfortunately, 
the route to call `typesAreCompatible()` is no dice. First things first, in C++ 
mode, that function just early returns (essentially) `==` on the type. 
Otherwise, it would call `QualType ASTContext::mergeTypes(QualType, QualType, 
...)`. Okay, let's delve into this thing. Now, first things first, somewhere 
down the line if you happen to give it something C++-specific (e.g. 
`LValueReferenceType`), it will just assert into your face. (Because C++ things 
are only `compatible` if the types are equal, most likely. In C++ mode, 
`mergeType` shouldn't be, and doesn't get, called.)

But there are even more issues with this function. Put simply, it does not 
perform **any** "implicit conversions". Sic!, I'm putting it in between quotes, 
but the fact is that if one type is `const` and the other isn't, it will just 
say //"Nope, these are not compatible."//.
So modelling the whole idea of //"is p

[PATCH] D98948: [analyzer][solver] Fix infeasible constraints (PR49642)

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko created this revision.
vsavchenko added reviewers: steakhal, NoQ, xazax.hun, ASDenysPetrov.
Herald added subscribers: martong, Charusso, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware.
vsavchenko requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Additionally, this patch puts an assertion checking for feasible
constraints in every place where constraints are assigned to states.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98948

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/test/Analysis/PR49642.c

Index: clang/test/Analysis/PR49642.c
===
--- /dev/null
+++ clang/test/Analysis/PR49642.c
@@ -0,0 +1,20 @@
+// RUN: %clang --analyze -Xclang -analyzer-checker=core %s
+
+typedef ssize_t;
+b;
+
+unsigned c;
+int write(int, const void *, unsigned long);
+
+a() {
+  d();
+  while (c > 0) {
+b = write(0, d, c);
+if (b)
+  c -= b;
+b < 1;
+  }
+  if (c && c) {
+// ^ no-crash
+  }
+}
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -448,12 +448,12 @@
   EquivalenceClass Other);
 
   /// Return a set of class members for the given state.
-  LLVM_NODISCARD inline SymbolSet getClassMembers(ProgramStateRef State);
+  LLVM_NODISCARD inline SymbolSet getClassMembers(ProgramStateRef State) const;
   /// Return true if the current class is trivial in the given state.
-  LLVM_NODISCARD inline bool isTrivial(ProgramStateRef State);
+  LLVM_NODISCARD inline bool isTrivial(ProgramStateRef State) const;
   /// Return true if the current class is trivial and its only member is dead.
   LLVM_NODISCARD inline bool isTriviallyDead(ProgramStateRef State,
- SymbolReaper &Reaper);
+ SymbolReaper &Reaper) const;
 
   LLVM_NODISCARD static inline ProgramStateRef
   markDisequal(BasicValueFactory &BV, RangeSet::Factory &F,
@@ -521,7 +521,7 @@
ProgramStateRef State, SymbolSet Members,
EquivalenceClass Other,
SymbolSet OtherMembers);
-  static inline void
+  static inline bool
   addToDisequalityInfo(DisequalityMapTy &Info, ConstraintRangeTy &Constraints,
BasicValueFactory &BV, RangeSet::Factory &F,
ProgramStateRef State, EquivalenceClass First,
@@ -535,6 +535,15 @@
 // Constraint functions
 //===--===//
 
+LLVM_NODISCARD LLVM_ATTRIBUTE_UNUSED bool
+areFeasible(ConstraintRangeTy Constraints) {
+  return llvm::none_of(
+  Constraints,
+  [](const std::pair &ClassConstraint) {
+return ClassConstraint.second.isEmpty();
+  });
+}
+
 LLVM_NODISCARD inline const RangeSet *getConstraint(ProgramStateRef State,
 EquivalenceClass Class) {
   return State->get(Class);
@@ -1397,15 +1406,6 @@
 return EquivalenceClass::merge(getBasicVals(), F, State, LHS, RHS);
   }
 
-  LLVM_NODISCARD LLVM_ATTRIBUTE_UNUSED static bool
-  areFeasible(ConstraintRangeTy Constraints) {
-return llvm::none_of(
-Constraints,
-[](const std::pair &ClassConstraint) {
-  return ClassConstraint.second.isEmpty();
-});
-  }
-
   LLVM_NODISCARD ProgramStateRef setConstraint(ProgramStateRef State,
EquivalenceClass Class,
RangeSet Constraint) {
@@ -1574,6 +1574,9 @@
 // Assign new constraints for this class.
 Constraints = CRF.add(Constraints, *this, *NewClassConstraint);
 
+assert(areFeasible(Constraints) && "Constraint manager shouldn't produce "
+   "a state with infeasible constraints");
+
 State = State->set(Constraints);
   }
 
@@ -1644,7 +1647,7 @@
   return State->get_context();
 }
 
-SymbolSet EquivalenceClass::getClassMembers(ProgramStateRef State) {
+SymbolSet EquivalenceClass::getClassMembers(ProgramStateRef State) const {
   if (const SymbolSet *Members = State->get(*this))
 return *Members;
 
@@ -1654,12 +1657,12 @@
   return F.add(F.getEmptySet(), getRepresentativeSymbol());
 }
 
-bool EquivalenceClass::isTrivial(ProgramStateRef State) {
+bool EquivalenceClass::isTrivial(ProgramStateRef State) const {
   return State->get(*this) == nullptr;
 }
 
 bool EquivalenceClass::isTriviallyDead(ProgramStateRef State,
-   SymbolReaper &Reaper) {
+  

[PATCH] D98948: [analyzer][solver] Fix infeasible constraints (PR49642)

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko updated this revision to Diff 331863.
vsavchenko added a comment.

Fix typos in comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98948

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/test/Analysis/PR49642.c

Index: clang/test/Analysis/PR49642.c
===
--- /dev/null
+++ clang/test/Analysis/PR49642.c
@@ -0,0 +1,20 @@
+// RUN: %clang --analyze -Xclang -analyzer-checker=core %s
+
+typedef ssize_t;
+b;
+
+unsigned c;
+int write(int, const void *, unsigned long);
+
+a() {
+  d();
+  while (c > 0) {
+b = write(0, d, c);
+if (b)
+  c -= b;
+b < 1;
+  }
+  if (c && c) {
+// ^ no-crash
+  }
+}
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -448,12 +448,12 @@
   EquivalenceClass Other);
 
   /// Return a set of class members for the given state.
-  LLVM_NODISCARD inline SymbolSet getClassMembers(ProgramStateRef State);
+  LLVM_NODISCARD inline SymbolSet getClassMembers(ProgramStateRef State) const;
   /// Return true if the current class is trivial in the given state.
-  LLVM_NODISCARD inline bool isTrivial(ProgramStateRef State);
+  LLVM_NODISCARD inline bool isTrivial(ProgramStateRef State) const;
   /// Return true if the current class is trivial and its only member is dead.
   LLVM_NODISCARD inline bool isTriviallyDead(ProgramStateRef State,
- SymbolReaper &Reaper);
+ SymbolReaper &Reaper) const;
 
   LLVM_NODISCARD static inline ProgramStateRef
   markDisequal(BasicValueFactory &BV, RangeSet::Factory &F,
@@ -521,7 +521,7 @@
ProgramStateRef State, SymbolSet Members,
EquivalenceClass Other,
SymbolSet OtherMembers);
-  static inline void
+  static inline bool
   addToDisequalityInfo(DisequalityMapTy &Info, ConstraintRangeTy &Constraints,
BasicValueFactory &BV, RangeSet::Factory &F,
ProgramStateRef State, EquivalenceClass First,
@@ -535,6 +535,15 @@
 // Constraint functions
 //===--===//
 
+LLVM_NODISCARD LLVM_ATTRIBUTE_UNUSED bool
+areFeasible(ConstraintRangeTy Constraints) {
+  return llvm::none_of(
+  Constraints,
+  [](const std::pair &ClassConstraint) {
+return ClassConstraint.second.isEmpty();
+  });
+}
+
 LLVM_NODISCARD inline const RangeSet *getConstraint(ProgramStateRef State,
 EquivalenceClass Class) {
   return State->get(Class);
@@ -1397,15 +1406,6 @@
 return EquivalenceClass::merge(getBasicVals(), F, State, LHS, RHS);
   }
 
-  LLVM_NODISCARD LLVM_ATTRIBUTE_UNUSED static bool
-  areFeasible(ConstraintRangeTy Constraints) {
-return llvm::none_of(
-Constraints,
-[](const std::pair &ClassConstraint) {
-  return ClassConstraint.second.isEmpty();
-});
-  }
-
   LLVM_NODISCARD ProgramStateRef setConstraint(ProgramStateRef State,
EquivalenceClass Class,
RangeSet Constraint) {
@@ -1428,7 +1428,7 @@
 getRange(State, DisequalClass).Delete(getBasicVals(), F, *Point);
 
 // If we end up with at least one of the disequal classes to be
-// constrainted with an empty range-set, the state is infeasible.
+// constrained with an empty range-set, the state is infeasible.
 if (UpdatedConstraint.isEmpty())
   return nullptr;
 
@@ -1574,6 +1574,9 @@
 // Assign new constraints for this class.
 Constraints = CRF.add(Constraints, *this, *NewClassConstraint);
 
+assert(areFeasible(Constraints) && "Constraint manager shouldn't produce "
+   "a state with infeasible constraints");
+
 State = State->set(Constraints);
   }
 
@@ -1644,7 +1647,7 @@
   return State->get_context();
 }
 
-SymbolSet EquivalenceClass::getClassMembers(ProgramStateRef State) {
+SymbolSet EquivalenceClass::getClassMembers(ProgramStateRef State) const {
   if (const SymbolSet *Members = State->get(*this))
 return *Members;
 
@@ -1654,12 +1657,12 @@
   return F.add(F.getEmptySet(), getRepresentativeSymbol());
 }
 
-bool EquivalenceClass::isTrivial(ProgramStateRef State) {
+bool EquivalenceClass::isTrivial(ProgramStateRef State) const {
   return State->get(*this) == nullptr;
 }
 
 bool EquivalenceClass::isTriviallyDead(ProgramStateRef State,
-  

[PATCH] D98950: [clang][deps] NFC: Document collector, rename members

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch documents how `ModuleDepCollector{,PP}` work and what their members 
store. Also renames somewhat vague `MainDeps` to `FileDeps` and `Deps` to 
`ModularDeps`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98950

Files:
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -79,7 +79,7 @@
   // We do not want #line markers to affect dependency generation!
   if (Optional Filename =
   SM.getNonBuiltinFilenameForID(SM.getFileID(SM.getExpansionLoc(Loc
-MDC.MainDeps.push_back(
+MDC.FileDeps.push_back(
 std::string(llvm::sys::path::remove_leading_dotslash(*Filename)));
 }
 
@@ -91,7 +91,7 @@
   if (!File && !Imported) {
 // This is a non-modular include that HeaderSearch failed to find. Add it
 // here as `FileChanged` will never see it.
-MDC.MainDeps.push_back(std::string(FileName));
+MDC.FileDeps.push_back(std::string(FileName));
   }
   handleImport(Imported);
 }
@@ -106,9 +106,10 @@
   if (!Imported)
 return;
 
-  MDC.Deps[MDC.ContextHash + Imported->getTopLevelModule()->getFullModuleName()]
+  const Module *TopLevelModule = Imported->getTopLevelModule();
+  MDC.ModularDeps[MDC.ContextHash + TopLevelModule->getFullModuleName()]
   .ImportedByMainFile = true;
-  DirectDeps.insert(Imported->getTopLevelModule());
+  DirectModularDeps.insert(TopLevelModule);
 }
 
 void ModuleDepCollectorPP::EndOfMainFile() {
@@ -116,21 +117,20 @@
   MDC.MainFile = std::string(
   Instance.getSourceManager().getFileEntryForID(MainFileID)->getName());
 
-  for (const Module *M : DirectDeps) {
+  for (const Module *M : DirectModularDeps)
 handleTopLevelModule(M);
-  }
 
-  for (auto &&I : MDC.Deps)
+  for (auto &&I : MDC.ModularDeps)
 MDC.Consumer.handleModuleDependency(I.second);
 
-  for (auto &&I : MDC.MainDeps)
+  for (auto &&I : MDC.FileDeps)
 MDC.Consumer.handleFileDependency(*MDC.Opts, I);
 }
 
 void ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
   assert(M == M->getTopLevelModule() && "Expected top level module!");
 
-  auto ModI = MDC.Deps.insert(
+  auto ModI = MDC.ModularDeps.insert(
   std::make_pair(MDC.ContextHash + M->getFullModuleName(), ModuleDeps{}));
 
   if (!ModI.first->second.ID.ModuleName.empty())
Index: clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
===
--- clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -104,6 +104,10 @@
 
 class ModuleDepCollector;
 
+/// Callback that records textual includes and direct modular includes/imports
+/// during preprocessing. At the end of the main file, it also collects
+/// transitive modular dependencies and passes everything to the
+/// \c DependencyConsumer of the parent \c ModuleDepCollector.
 class ModuleDepCollectorPP final : public PPCallbacks {
 public:
   ModuleDepCollectorPP(CompilerInstance &I, ModuleDepCollector &MDC)
@@ -124,11 +128,18 @@
   void EndOfMainFile() override;
 
 private:
+  /// The compiler instance for the current translation unit.
   CompilerInstance &Instance;
+  /// The parent dependency collector.
   ModuleDepCollector &MDC;
-  llvm::DenseSet DirectDeps;
+  /// Working set of direct modular dependencies.
+  llvm::DenseSet DirectModularDeps;
 
   void handleImport(const Module *Imported);
+
+  /// Traverses the previously collected direct modular dependencies to discover
+  /// transitive modular dependencies and fills the parent \c ModuleDepCollector
+  /// with both.
   void handleTopLevelModule(const Module *M);
   void addAllSubmoduleDeps(const Module *M, ModuleDeps &MD,
llvm::DenseSet &AddedModules);
@@ -136,6 +147,8 @@
 llvm::DenseSet &AddedModules);
 };
 
+/// Collects modular and non-modular dependencies of the main file by attaching
+/// \c ModuleDepCollectorPP to the preprocessor.
 class ModuleDepCollector final : public DependencyCollector {
 public:
   ModuleDepCollector(std::unique_ptr Opts,
@@ -147,12 +160,20 @@
 private:
   friend ModuleDepCollectorPP;
 
+  /// The compiler instance for the current translation unit.
   CompilerInstance &Instance;
+  /// The consumer of collected dependency information.
   DependencyConsumer &Consumer;
+  /// Path to the main source file.
   std::string MainFile;
+  /// The module hash identifying the compilation 

[PATCH] D97080: [flang][driver] Add -fintrinsic-modules-path option

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:292
+  std::string driverPath = llvm::sys::fs::getMainExecutable(nullptr, nullptr);
+  driverPath = driverPath.substr(0, driverPath.size() - 9);
+  return driverPath.append("/../include/flang/");

arnamoy10 wrote:
> bryanpkc wrote:
> > Can you use `llvm::sys::path::remove_filename` here?
> Thank you.  This seems like a better option, but I could not make it work.  
> For the sake of time I am keeping this hard coded manipulation.  It can be 
> improved in a later revision.
It would be really nice to use `llvm::sys::path::remove_filename` instead. 
Perhaps something like this:
```
llvm::SmallString<128> driverPath;
driverPath.assign(llvm::sys::fs::getMainExecutable(nullptr, nullptr));
llvm::sys::path::remove_filename(driverPath);
```
?


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

https://reviews.llvm.org/D97080

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

[PATCH] D98746: [clang][amdgpu] Use implicit code object default

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Great context, thanks guys. I had missed that part of the compiler.

We presently have a dependency edge from clang to the amdgcn target in llvm. 
The drawback I noticed here is it stops clang unconditionally building a 
runtime library for amdgcn. This patch, or a derivative of that, would hide 
that by making the dependency implicit. Unblocks openmp, which is important, 
but somewhat fragile.

Aside from that workaround, there are two subproblems here:

- Reporting deprecated argument and propagating the clang argument through to 
the backend, so that it doesn't have to be prefixed -mllvm by the user
- Clang emitting different IR based on different values for the code object 
format

The first part can be neatly solved as @yaxunl suggested above. Rewrite 
deprecated to new format, only pass either along if one was specified. That'll 
unblock openmp and break nothing. I'll put that change together.

The second is more troublesome. If clang emits different IR for different code 
object formats, then a device library built at clang build time (e.g. the 
openmp runtime) will need to be compiled N times for N different code object 
formats. Unfortunately, that library is already compiled M times for different 
architectures, so we're looking at N*M copies of roughly the same library, and 
logic in the clang driver to pick out the right one. Plus N extra paths to 
test/debug etc.

It would be preferable from an ease of use and distribution perspective if 
clang emits target id independent code and we specialise it later, either in 
the backend (at which point the backend really is the only thing that specifies 
a default), or as a library that is chosen by the driver during 'linking', like 
the device libs oclc_isa_version_906.bc pattern.

That is, we should make the target id implementation in clang more complicated 
by emitting some hook which is filled in later instead of hardcoding the code 
object version choice upfront, as that makes other stuff in the toolchain 
simpler. It would also turn the current work in progress from fragile into 
correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98746

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

[PATCH] D97119: [flang][driver] Add options for -std=f2018

[Arnamoy B via Phabricator via cfe-commits]

arnamoy10 updated this revision to Diff 331871.
arnamoy10 added a comment.

Separating test cases for accepted input option and not accepted input option.


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

https://reviews.llvm.org/D97119

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Frontend/CompilerInvocation.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/std2018.f90
  flang/test/Driver/std2018_wrong.f90

Index: flang/test/Driver/std2018_wrong.f90
===
--- /dev/null
+++ flang/test/Driver/std2018_wrong.f90
@@ -0,0 +1,22 @@
+! REQUIRES: new-flang-driver
+! Ensure argument -std=f2018 works as expected.
+
+!-
+! FRONTEND FLANG DRIVER (flang-new -fc1)
+!-
+! RUN: not %flang_fc1 -std=90 %s  2>&1 | FileCheck %s --check-prefix=WRONG
+
+!-
+! EXPECTED OUTPUT WITH WRONG
+!-
+! WRONG: Only -std=f2018 is allowed currently.
+
+subroutine foo2()
+do 01 m=1,2
+  select case (m)
+  case default
+print*, "default", m
+  case (1)
+print*, "start"
+01end select
+end subroutine
Index: flang/test/Driver/std2018.f90
===
--- /dev/null
+++ flang/test/Driver/std2018.f90
@@ -0,0 +1,29 @@
+! REQUIRES: new-flang-driver
+! Ensure argument -std=f2018 works as expected.
+
+!-
+! FRONTEND FLANG DRIVER (flang-new -fc1)
+!-
+! RUN: %flang_fc1 %s  2>&1 | FileCheck %s --allow-empty --check-prefix=WITHOUT
+! RUN: %flang_fc1 -std=f2018 %s  2>&1 | FileCheck %s --check-prefix=GIVEN
+! RUN: %flang_fc1 -pedantic %s  2>&1 | FileCheck %s --check-prefix=GIVEN
+
+!-
+! EXPECTED OUTPUT WITHOUT
+!-
+! WITHOUT-NOT: A DO loop should terminate with an END DO or CONTINUE
+
+!-
+! EXPECTED OUTPUT WITH
+!-
+! GIVEN: A DO loop should terminate with an END DO or CONTINUE
+
+subroutine foo2()
+do 01 m=1,2
+  select case (m)
+  case default
+print*, "default", m
+  case (1)
+print*, "start"
+01end select
+end subroutine
Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -45,6 +45,8 @@
 ! HELP-NEXT: -IAdd directory to the end of the list of include search paths
 ! HELP-NEXT: -module-dir   Put MODULE files in 
 ! HELP-NEXT: -o   Write output to 
+! HELP-NEXT: -pedantic  Warn on language extensions
+! HELP-NEXT: -std=   Language standard to compile for
 ! HELP-NEXT: -U  Undefine macro 
 ! HELP-NEXT: --version  Print version information
 ! HELP-NEXT: -Xflang   Pass  to the flang compiler
@@ -92,6 +94,8 @@
 ! HELP-FC1-NEXT: -IAdd directory to the end of the list of include search paths
 ! HELP-FC1-NEXT: -module-dir   Put MODULE files in 
 ! HELP-FC1-NEXT: -o   Write output to 
+! HELP-FC1-NEXT: -pedantic  Warn on language extensions
+! HELP-FC1-NEXT: -std=   Language standard to compile for
 ! HELP-FC1-NEXT: -test-io   Run the InputOuputTest action. Use for development and testing only.
 ! HELP-FC1-NEXT: -U  Undefine macro 
 ! HELP-FC1-NEXT: --version  Print version information
Index: flang/test/Driver/driver-help-hidden.f90
===
--- flang/test/Driver/driver-help-hidden.f90
+++ flang/test/Driver/driver-help-hidden.f90
@@ -45,6 +45,8 @@
 ! CHECK-NEXT: -IAdd directory to the end of the list of include search paths
 ! CHECK-NEXT: -module-dir   Put MODULE files in 
 ! CHECK-NEXT: -o  Write output to 
+! CHECK-NEXT: -pedantic  Warn on language extensions
+! CHECK-NEXT: -std=   Language standard to compile for
 ! CHECK-NEXT: -U  Undefine macro 
 ! CHECK-NEXT: --version Print version information
 ! CHECK-NEXT: -Xflang   Pass  to the flang compiler
Index: flang/lib/Frontend/CompilerInvocation.cpp
===
--- flang/lib/Frontend/CompilerInvocation.cpp
+++ flang/lib/Frontend/CompilerInvocation.cpp
@@ -371,6 +371,26 @@
 res.frontendOpts().features_.Enable(
 Fortran::common::LanguageFeature::OpenMP);
   }
+
+  //-fpedantic
+  if (args.hasArg(clang::driver::options::OPT_pedantic)) {
+res.set_EnableConformanceChecks();
+

[PATCH] D98951: [clang][ASTImporter] Add import API for 'const Type *'.

[Balázs Kéri via Phabricator via cfe-commits]

balazske created this revision.
Herald added subscribers: martong, teemperor, gamesh411, Szelethus, dkrupp.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
balazske requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

There was only an `Import` function for `QualType` but not for `Type`.
For correct import of some AST nodes where not `QualType` is used
an import of `Type *` is needed. (It is the case with
`FieldDecl::getCapturedVLAType`.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98951

Files:
  clang/include/clang/AST/ASTImporter.h
  clang/lib/AST/ASTImporter.cpp


Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -8151,28 +8151,37 @@
   return make_error(ImportError::UnsupportedConstruct);
 }
 
-Expected ASTImporter::Import(QualType FromT) {
-  if (FromT.isNull())
-return QualType{};
-
-  const Type *FromTy = FromT.getTypePtr();
+Expected ASTImporter::Import(const Type *FromT) {
+  if (!FromT)
+return FromT;
 
   // Check whether we've already imported this type.
-  llvm::DenseMap::iterator Pos
-= ImportedTypes.find(FromTy);
+  llvm::DenseMap::iterator Pos =
+  ImportedTypes.find(FromT);
   if (Pos != ImportedTypes.end())
-return ToContext.getQualifiedType(Pos->second, FromT.getLocalQualifiers());
+return Pos->second;
 
   // Import the type
   ASTNodeImporter Importer(*this);
-  ExpectedType ToTOrErr = Importer.Visit(FromTy);
+  ExpectedType ToTOrErr = Importer.Visit(FromT);
   if (!ToTOrErr)
 return ToTOrErr.takeError();
 
   // Record the imported type.
-  ImportedTypes[FromTy] = (*ToTOrErr).getTypePtr();
+  ImportedTypes[FromT] = ToTOrErr->getTypePtr();
+
+  return ToTOrErr->getTypePtr();
+}
+
+Expected ASTImporter::Import(QualType FromT) {
+  if (FromT.isNull())
+return QualType{};
+
+  Expected ToTyOrErr = Import(FromT.getTypePtr());
+  if (!ToTyOrErr)
+return ToTyOrErr.takeError();
 
-  return ToContext.getQualifiedType(*ToTOrErr, FromT.getLocalQualifiers());
+  return ToContext.getQualifiedType(*ToTyOrErr, FromT.getLocalQualifiers());
 }
 
 Expected ASTImporter::Import(TypeSourceInfo *FromTSI) {
Index: clang/include/clang/AST/ASTImporter.h
===
--- clang/include/clang/AST/ASTImporter.h
+++ clang/include/clang/AST/ASTImporter.h
@@ -344,6 +344,12 @@
 Import(ExprWithCleanups::CleanupObject From);
 
 /// Import the given type from the "from" context into the "to"
+/// context.
+///
+/// \returns The equivalent type in the "to" context, or the import error.
+llvm::Expected Import(const Type *FromT);
+
+/// Import the given qualified type from the "from" context into the "to"
 /// context. A null type is imported as a null type (no error).
 ///
 /// \returns The equivalent type in the "to" context, or the import error.


Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -8151,28 +8151,37 @@
   return make_error(ImportError::UnsupportedConstruct);
 }
 
-Expected ASTImporter::Import(QualType FromT) {
-  if (FromT.isNull())
-return QualType{};
-
-  const Type *FromTy = FromT.getTypePtr();
+Expected ASTImporter::Import(const Type *FromT) {
+  if (!FromT)
+return FromT;
 
   // Check whether we've already imported this type.
-  llvm::DenseMap::iterator Pos
-= ImportedTypes.find(FromTy);
+  llvm::DenseMap::iterator Pos =
+  ImportedTypes.find(FromT);
   if (Pos != ImportedTypes.end())
-return ToContext.getQualifiedType(Pos->second, FromT.getLocalQualifiers());
+return Pos->second;
 
   // Import the type
   ASTNodeImporter Importer(*this);
-  ExpectedType ToTOrErr = Importer.Visit(FromTy);
+  ExpectedType ToTOrErr = Importer.Visit(FromT);
   if (!ToTOrErr)
 return ToTOrErr.takeError();
 
   // Record the imported type.
-  ImportedTypes[FromTy] = (*ToTOrErr).getTypePtr();
+  ImportedTypes[FromT] = ToTOrErr->getTypePtr();
+
+  return ToTOrErr->getTypePtr();
+}
+
+Expected ASTImporter::Import(QualType FromT) {
+  if (FromT.isNull())
+return QualType{};
+
+  Expected ToTyOrErr = Import(FromT.getTypePtr());
+  if (!ToTyOrErr)
+return ToTyOrErr.takeError();
 
-  return ToContext.getQualifiedType(*ToTOrErr, FromT.getLocalQualifiers());
+  return ToContext.getQualifiedType(*ToTyOrErr, FromT.getLocalQualifiers());
 }
 
 Expected ASTImporter::Import(TypeSourceInfo *FromTSI) {
Index: clang/include/clang/AST/ASTImporter.h
===
--- clang/include/clang/AST/ASTImporter.h
+++ clang/include/clang/AST/ASTImporter.h
@@ -344,6 +344,12 @@
 Import(ExprWithCleanups::CleanupObject From);
 
 /// Import the given type from the "from" context into 

[clang] 96e675b - [clang][ASTImporter] Add import support for SourceLocExpr.

[Balázs Kéri via cfe-commits]

Author: Balázs Kéri
Date: 2021-03-19T16:33:04+01:00
New Revision: 96e675bdd5c8bfef34135fb50bcc7f570f073639

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

LOG: [clang][ASTImporter] Add import support for SourceLocExpr.

It is possible that imported `SourceLocExpr` can cause not expected behavior 
(if `__builtin_LINE()` is used together with `__LINE__` for example) but still 
it may be worth to import these because some projects use it.

Reviewed By: teemperor

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

Added: 


Modified: 
clang/lib/AST/ASTImporter.cpp
clang/unittests/AST/ASTImporterTest.cpp

Removed: 




diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index d48e173eb3b3..bf3cb4c42873 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -574,6 +574,7 @@ namespace clang {
 
 // Importing expressions
 ExpectedStmt VisitExpr(Expr *E);
+ExpectedStmt VisitSourceLocExpr(SourceLocExpr *E);
 ExpectedStmt VisitVAArgExpr(VAArgExpr *E);
 ExpectedStmt VisitChooseExpr(ChooseExpr *E);
 ExpectedStmt VisitGNUNullExpr(GNUNullExpr *E);
@@ -6483,6 +6484,21 @@ ExpectedStmt ASTNodeImporter::VisitExpr(Expr *E) {
   return make_error(ImportError::UnsupportedConstruct);
 }
 
+ExpectedStmt ASTNodeImporter::VisitSourceLocExpr(SourceLocExpr *E) {
+  Error Err = Error::success();
+  auto BLoc = importChecked(Err, E->getBeginLoc());
+  auto RParenLoc = importChecked(Err, E->getEndLoc());
+  if (Err)
+return std::move(Err);
+  auto ParentContextOrErr = Importer.ImportContext(E->getParentContext());
+  if (!ParentContextOrErr)
+return ParentContextOrErr.takeError();
+
+  return new (Importer.getToContext())
+  SourceLocExpr(Importer.getToContext(), E->getIdentKind(), BLoc, 
RParenLoc,
+*ParentContextOrErr);
+}
+
 ExpectedStmt ASTNodeImporter::VisitVAArgExpr(VAArgExpr *E) {
 
   Error Err = Error::success();

diff  --git a/clang/unittests/AST/ASTImporterTest.cpp 
b/clang/unittests/AST/ASTImporterTest.cpp
index 43464cc0c9ca..8c4b982ec6d5 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -246,6 +246,24 @@ TEST_P(ImportPath, CycleAfterCycle) {
   EXPECT_FALSE(path.hasCycleAtBack());
 }
 
+const internal::VariadicDynCastAllOfMatcher sourceLocExpr;
+
+AST_MATCHER_P(SourceLocExpr, hasBuiltinStr, StringRef, Str) {
+  return Node.getBuiltinStr() == Str;
+}
+
+TEST_P(ImportExpr, ImportSourceLocExpr) {
+  MatchVerifier Verifier;
+  testImport("void declToImport() { (void)__builtin_FILE(); }", Lang_CXX03, "",
+ Lang_CXX03, Verifier,
+ functionDecl(hasDescendant(
+ sourceLocExpr(hasBuiltinStr("__builtin_FILE");
+  testImport("void declToImport() { (void)__builtin_COLUMN(); }", Lang_CXX03,
+ "", Lang_CXX03, Verifier,
+ functionDecl(hasDescendant(
+ sourceLocExpr(hasBuiltinStr("__builtin_COLUMN");
+}
+
 TEST_P(ImportExpr, ImportStringLiteral) {
   MatchVerifier Verifier;
   testImport("void declToImport() { (void)\"foo\"; }", Lang_CXX03, "",



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

[PATCH] D98876: [clang][ASTImporter] Add import support for SourceLocExpr.

[Balázs Kéri via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG96e675bdd5c8: [clang][ASTImporter] Add import support for 
SourceLocExpr. (authored by balazske).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98876

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp


Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -246,6 +246,24 @@
   EXPECT_FALSE(path.hasCycleAtBack());
 }
 
+const internal::VariadicDynCastAllOfMatcher sourceLocExpr;
+
+AST_MATCHER_P(SourceLocExpr, hasBuiltinStr, StringRef, Str) {
+  return Node.getBuiltinStr() == Str;
+}
+
+TEST_P(ImportExpr, ImportSourceLocExpr) {
+  MatchVerifier Verifier;
+  testImport("void declToImport() { (void)__builtin_FILE(); }", Lang_CXX03, "",
+ Lang_CXX03, Verifier,
+ functionDecl(hasDescendant(
+ sourceLocExpr(hasBuiltinStr("__builtin_FILE");
+  testImport("void declToImport() { (void)__builtin_COLUMN(); }", Lang_CXX03,
+ "", Lang_CXX03, Verifier,
+ functionDecl(hasDescendant(
+ sourceLocExpr(hasBuiltinStr("__builtin_COLUMN");
+}
+
 TEST_P(ImportExpr, ImportStringLiteral) {
   MatchVerifier Verifier;
   testImport("void declToImport() { (void)\"foo\"; }", Lang_CXX03, "",
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -574,6 +574,7 @@
 
 // Importing expressions
 ExpectedStmt VisitExpr(Expr *E);
+ExpectedStmt VisitSourceLocExpr(SourceLocExpr *E);
 ExpectedStmt VisitVAArgExpr(VAArgExpr *E);
 ExpectedStmt VisitChooseExpr(ChooseExpr *E);
 ExpectedStmt VisitGNUNullExpr(GNUNullExpr *E);
@@ -6483,6 +6484,21 @@
   return make_error(ImportError::UnsupportedConstruct);
 }
 
+ExpectedStmt ASTNodeImporter::VisitSourceLocExpr(SourceLocExpr *E) {
+  Error Err = Error::success();
+  auto BLoc = importChecked(Err, E->getBeginLoc());
+  auto RParenLoc = importChecked(Err, E->getEndLoc());
+  if (Err)
+return std::move(Err);
+  auto ParentContextOrErr = Importer.ImportContext(E->getParentContext());
+  if (!ParentContextOrErr)
+return ParentContextOrErr.takeError();
+
+  return new (Importer.getToContext())
+  SourceLocExpr(Importer.getToContext(), E->getIdentKind(), BLoc, 
RParenLoc,
+*ParentContextOrErr);
+}
+
 ExpectedStmt ASTNodeImporter::VisitVAArgExpr(VAArgExpr *E) {
 
   Error Err = Error::success();


Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -246,6 +246,24 @@
   EXPECT_FALSE(path.hasCycleAtBack());
 }
 
+const internal::VariadicDynCastAllOfMatcher sourceLocExpr;
+
+AST_MATCHER_P(SourceLocExpr, hasBuiltinStr, StringRef, Str) {
+  return Node.getBuiltinStr() == Str;
+}
+
+TEST_P(ImportExpr, ImportSourceLocExpr) {
+  MatchVerifier Verifier;
+  testImport("void declToImport() { (void)__builtin_FILE(); }", Lang_CXX03, "",
+ Lang_CXX03, Verifier,
+ functionDecl(hasDescendant(
+ sourceLocExpr(hasBuiltinStr("__builtin_FILE");
+  testImport("void declToImport() { (void)__builtin_COLUMN(); }", Lang_CXX03,
+ "", Lang_CXX03, Verifier,
+ functionDecl(hasDescendant(
+ sourceLocExpr(hasBuiltinStr("__builtin_COLUMN");
+}
+
 TEST_P(ImportExpr, ImportStringLiteral) {
   MatchVerifier Verifier;
   testImport("void declToImport() { (void)\"foo\"; }", Lang_CXX03, "",
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -574,6 +574,7 @@
 
 // Importing expressions
 ExpectedStmt VisitExpr(Expr *E);
+ExpectedStmt VisitSourceLocExpr(SourceLocExpr *E);
 ExpectedStmt VisitVAArgExpr(VAArgExpr *E);
 ExpectedStmt VisitChooseExpr(ChooseExpr *E);
 ExpectedStmt VisitGNUNullExpr(GNUNullExpr *E);
@@ -6483,6 +6484,21 @@
   return make_error(ImportError::UnsupportedConstruct);
 }
 
+ExpectedStmt ASTNodeImporter::VisitSourceLocExpr(SourceLocExpr *E) {
+  Error Err = Error::success();
+  auto BLoc = importChecked(Err, E->getBeginLoc());
+  auto RParenLoc = importChecked(Err, E->getEndLoc());
+  if (Err)
+return std::move(Err);
+  auto ParentContextOrErr = Importer.ImportContext(E->getParentContext());
+  if (!ParentContextOrErr)
+return ParentContextOrErr.takeError();
+
+  return new (Importer.getToContext())
+  SourceLocExpr(Importer.getToContext(), E->getIdentKind(), BLoc, RParenLoc,
+*ParentContextOrErr);
+}
+
 Exp

[PATCH] D98824: [Tooling] Handle compilation databases containing commands with double dashes

[Janusz Nykiel via Phabricator via cfe-commits]

jnykiel updated this revision to Diff 331884.
jnykiel edited the summary of this revision.
jnykiel added a comment.

I've addressed your comments (I agree with them, essentially). After 
implementing the insertion of '--' for problematic file names, I had to modify 
some interpolation unit tests to get them to consistently pass on both Windows 
and Unix-like systems - I made it possible to match against a relative 
'non-native' path in a test, as the absolute paths in this unit test fixture 
always start with a drive letter on Windows, thus making it impossible to test 
the '-' cases there, and always start with a '/' on Unix-like systems, making 
the expected string different when run on Windows or Unix-like.


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

https://reviews.llvm.org/D98824

Files:
  clang/lib/Tooling/ArgumentsAdjusters.cpp
  clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/unittests/Tooling/CompilationDatabaseTest.cpp

Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -725,14 +725,14 @@
 protected:
   // Look up the command from a relative path, and return it in string form.
   // The input file is not included in the returned command.
-  std::string getCommand(llvm::StringRef F) {
+  std::string getCommand(llvm::StringRef F, bool MakeNative = true) {
 auto Results =
 inferMissingCompileCommands(std::make_unique(Entries))
-->getCompileCommands(path(F));
+->getCompileCommands(MakeNative ? path(F) : F);
 if (Results.empty())
   return "none";
 // drop the input file argument, so tests don't have to deal with path().
-EXPECT_EQ(Results[0].CommandLine.back(), path(F))
+EXPECT_EQ(Results[0].CommandLine.back(), MakeNative ? path(F) : F)
 << "Last arg should be the file";
 Results[0].CommandLine.pop_back();
 return llvm::join(Results[0].CommandLine, " ");
@@ -812,6 +812,28 @@
   EXPECT_EQ(getCommand("dir/bar.cpp"), "clang -D dir/foo.cpp -Wall");
 }
 
+TEST_F(InterpolateTest, StripDoubleDash) {
+  add("dir/foo.cpp", "-o foo.o -std=c++14 -Wall -- dir/foo.cpp");
+  // input file and output option are removed
+  // -Wall flag isn't
+  // -std option gets re-added as the last argument before the input file
+  // -- is removed as it's not necessary - the new input file doesn't start with
+  // a dash
+  EXPECT_EQ(getCommand("dir/bar.cpp"), "clang -D dir/foo.cpp -Wall -std=c++14");
+}
+
+TEST_F(InterpolateTest, InsertDoubleDash) {
+  add("dir/foo.cpp", "-o foo.o -std=c++14 -Wall");
+  EXPECT_EQ(getCommand("-dir/bar.cpp", false),
+"clang -D dir/foo.cpp -Wall -std=c++14 --");
+}
+
+TEST_F(InterpolateTest, InsertDoubleDashForClangCL) {
+  add("dir/foo.cpp", "clang-cl", "/std:c++14 /W4");
+  EXPECT_EQ(getCommand("/dir/bar.cpp", false),
+"clang-cl -D dir/foo.cpp /W4 /std:c++14 --");
+}
+
 TEST_F(InterpolateTest, Case) {
   add("FOO/BAR/BAZ/SHOUT.cc");
   add("foo/bar/baz/quiet.cc");
@@ -831,7 +853,7 @@
   add("foo.cpp", "clang-cl", "/W4");
 
   // Language flags should be added with CL syntax.
-  EXPECT_EQ(getCommand("foo.h"), "clang-cl -D foo.cpp /W4 /TP");
+  EXPECT_EQ(getCommand("foo.h", false), "clang-cl -D foo.cpp /W4 /TP");
 }
 
 TEST_F(InterpolateTest, DriverModes) {
@@ -839,8 +861,10 @@
   add("bar.cpp", "clang", "--driver-mode=cl");
 
   // --driver-mode overrides should be respected.
-  EXPECT_EQ(getCommand("foo.h"), "clang-cl -D foo.cpp --driver-mode=gcc -x c++-header");
-  EXPECT_EQ(getCommand("bar.h"), "clang -D bar.cpp --driver-mode=cl /TP");
+  EXPECT_EQ(getCommand("foo.h"),
+"clang-cl -D foo.cpp --driver-mode=gcc -x c++-header");
+  EXPECT_EQ(getCommand("bar.h", false),
+"clang -D bar.cpp --driver-mode=cl /TP");
 }
 
 TEST(TransferCompileCommandTest, Smoke) {
Index: clang/lib/Tooling/Tooling.cpp
===
--- clang/lib/Tooling/Tooling.cpp
+++ clang/lib/Tooling/Tooling.cpp
@@ -440,8 +440,10 @@
   return;
 
   // If there's no override in place add our resource dir.
-  Args.push_back("-resource-dir=" +
- CompilerInvocation::GetResourcesPath(Argv0, MainAddr));
+  Args = getInsertArgumentAdjuster(
+  ("-resource-dir=" + CompilerInvocation::GetResourcesPath(Argv0, MainAddr))
+  .c_str(),
+  ArgumentInsertPosition::END)(Args, "");
 }
 
 int ClangTool::run(ToolAction *Action) {
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -177,6 +177,10 @@
Opt.matches(OPT__SLASH_Fo
 continue;
 
+  // ...inclu

[PATCH] D94355: [Passes] Add relative lookup table converter pass

[Gulfem Savrun Yeniceri via Phabricator via cfe-commits]

gulfem added a comment.

@lebedev.ri do you have further comments?
If not, I would like to submit this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94355

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

[PATCH] D88220: [C++20] P1825R0: More implicit moves

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

This change causes

  #include 
  std::shared_ptr x1;
  std::shared_ptr f() {
  std::shared_ptr & r = x1;
  r.reset(new int);
  return r;
  }
  int main() {
  std::shared_ptr x2 = f();
  long n = x2.use_count();
  return n;
  }

when built with `-std=c++20` to erroneously return a `use_count` of 1 instead 
of 2 now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88220

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

[PATCH] D98890: [SystemZ][z/OS] Ignore leading zero width bitfield alignment on z/OS target

[Abhina Sree via Phabricator via cfe-commits]

abhina.sreeskantharajan accepted this revision.
abhina.sreeskantharajan 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/D98890/new/

https://reviews.llvm.org/D98890

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

[PATCH] D98867: [HIP] Fix ROCm detection

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked an inline comment as done.
yaxunl added inline comments.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:271
+}
+if (LatestROCm < FileName)
+  LatestROCm = FileName.str();

tra wrote:
> This will rank `rocm-3.9` higher than `rocm-3.10`. I think you do need to 
> extract and compare the version.
Good catch. Will fix it.


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

https://reviews.llvm.org/D98867

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

[PATCH] D97080: [flang][driver] Add -fintrinsic-modules-path option

[Arnamoy B via Phabricator via cfe-commits]

arnamoy10 updated this revision to Diff 331888.
arnamoy10 added a comment.

Using `llvm::sys::path::remove_filename` as per suggestion.


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

https://reviews.llvm.org/D97080

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Frontend/PreprocessorOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/test/Driver/Inputs/ieee_arithmetic.mod
  flang/test/Driver/Inputs/iso_fortran_env.mod
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/intrinsic_module_path.f90

Index: flang/test/Driver/intrinsic_module_path.f90
===
--- /dev/null
+++ flang/test/Driver/intrinsic_module_path.f90
@@ -0,0 +1,37 @@
+! Ensure argument -fintrinsic-modules-path works as expected.
+! WITHOUT the option, the default location for the module is checked and no error generated.
+! With the option GIVEN, the module with the same name is PREPENDED, and considered over the
+! default one, causing a CHECKSUM error.
+
+! REQUIRES: new-flang-driver
+
+
+!--
+! FLANG DRIVER (flang-new)
+!--
+! RUN: %flang-new -fsyntax-only %s  2>&1 | FileCheck %s --allow-empty --check-prefix=WITHOUT
+! RUN: not %flang-new -fsyntax-only -fintrinsic-modules-path %S/Inputs/ %s  2>&1 | FileCheck %s --check-prefix=GIVEN
+
+!-
+! FRONTEND FLANG DRIVER (flang-new -fc1)
+!-
+! RUN: %flang-new -fc1 %s  2>&1 | FileCheck %s --allow-empty --check-prefix=WITHOUT
+! RUN: not %flang-new -fc1 -fintrinsic-modules-path %S/Inputs/ %s  2>&1 | FileCheck %s --check-prefix=GIVEN
+
+!-
+! EXPECTED OUTPUT WITHOUT
+!-
+! WITHOUT-NOT: 'ieee_arithmetic.mod' was not found
+! WITHOUT-NOT: 'iso_fortran_env.mod' was not found
+
+!-
+! EXPECTED OUTPUT WITH
+!-
+! GIVEN: error: Cannot read module file for module 'ieee_arithmetic': File has invalid checksum
+! GIVEN: error: Cannot read module file for module 'iso_fortran_env': File has invalid checksum
+
+
+program test_intrinsic_module_path
+   use ieee_arithmetic, only: ieee_round_type
+   use iso_fortran_env, only: team_type, event_type, lock_type
+end program
Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -35,6 +35,8 @@
 ! HELP-NEXT: -ffree-formProcess source files in free form
 ! HELP-NEXT: -fimplicit-noneNo implicit typing allowed unless overridden by IMPLICIT statements
 ! HELP-NEXT: -finput-charset= Specify the default character set for source files
+! HELP-NEXT: -fintrinsic-modules-path 
+! HELP-NEXT:Specify where to find the compiled intrinsic modules
 ! HELP-NEXT: -flarge-sizes  Use INTEGER(KIND=8) for the result type in size-related intrinsics
 ! HELP-NEXT: -flogical-abbreviations Enable logical abbreviations
 ! HELP-NEXT: -fno-color-diagnostics Disable colors in diagnostics
@@ -83,6 +85,8 @@
 ! HELP-FC1-NEXT: -fget-symbols-sources   Dump symbols and their source code locations
 ! HELP-FC1-NEXT: -fimplicit-noneNo implicit typing allowed unless overridden by IMPLICIT statements
 ! HELP-FC1-NEXT: -finput-charset= Specify the default character set for source files
+! HELP-FC1-NEXT: -fintrinsic-modules-path 
+! HELP-FC1-NEXT:Specify where to find the compiled intrinsic modules
 ! HELP-FC1-NEXT: -flarge-sizes  Use INTEGER(KIND=8) for the result type in size-related intrinsics
 ! HELP-FC1-NEXT: -flogical-abbreviations Enable logical abbreviations
 ! HELP-FC1-NEXT: -fopenacc  Enable OpenACC
Index: flang/test/Driver/driver-help-hidden.f90
===
--- flang/test/Driver/driver-help-hidden.f90
+++ flang/test/Driver/driver-help-hidden.f90
@@ -35,6 +35,8 @@
 ! CHECK-NEXT: -ffree-formProcess source files in free form
 ! CHECK-NEXT: -fimplicit-noneNo implicit typing allowed unless overridden by IMPLICIT statements
 ! CHECK-NEXT: -finput-charset= Specify the default character set for source files
+! CHECK-NEXT: -fintrinsic-modules-path 
+! CHECK-NEXT:Specify where to find the compiled intrinsic modules
 ! CHECK-NEXT: -flarge-sizes  Use INTEGER(KIND=8) for the result type in size-related intrinsics
 ! CHECK-NEXT: -flogical-abbreviations Enable logical abbreviations
 ! CHECK-NEXT: -fno-color-diagnostics Disable colors in diagnostics
Index: flang/test/Driver/Inputs/iso_fortran_env.mod
===
--- /dev/null
+++ flang/test/Drive

[PATCH] D97080: [flang][driver] Add -fintrinsic-modules-path option

[Arnamoy B via Phabricator via cfe-commits]

arnamoy10 added inline comments.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:292
+  std::string driverPath = llvm::sys::fs::getMainExecutable(nullptr, nullptr);
+  driverPath = driverPath.substr(0, driverPath.size() - 9);
+  return driverPath.append("/../include/flang/");

awarzynski wrote:
> arnamoy10 wrote:
> > bryanpkc wrote:
> > > Can you use `llvm::sys::path::remove_filename` here?
> > Thank you.  This seems like a better option, but I could not make it work.  
> > For the sake of time I am keeping this hard coded manipulation.  It can be 
> > improved in a later revision.
> It would be really nice to use `llvm::sys::path::remove_filename` instead. 
> Perhaps something like this:
> ```
> llvm::SmallString<128> driverPath;
> driverPath.assign(llvm::sys::fs::getMainExecutable(nullptr, nullptr));
> llvm::sys::path::remove_filename(driverPath);
> ```
> ?
This works, thanks for the code!


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

https://reviews.llvm.org/D97080

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

[PATCH] D98867: [HIP] Fix ROCm detection

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 331889.
yaxunl marked an inline comment as done.
yaxunl added a comment.

fix version comparison


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

https://reviews.llvm.org/D98867

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/test/Driver/rocm-detect.hip


Index: clang/test/Driver/rocm-detect.hip
===
--- clang/test/Driver/rocm-detect.hip
+++ clang/test/Driver/rocm-detect.hip
@@ -21,6 +21,20 @@
 // RUN:   --rocm-path=%S/Inputs/rocm %s 2>&1 \
 // RUN:   | FileCheck -check-prefixes=COMMON,NODEFAULTLIBS %s
 
+// Test environment variable ROCM_PATH.
+// RUN: env ROCM_PATH=%S/Inputs/rocm %clang -### -target x86_64-linux-gnu \
+// RUN:   --print-rocm-search-dirs %s 2>&1 \
+// RUN:   | FileCheck -check-prefixes=ROCM-ENV %s
+
+// Test detecting latest /opt/rocm-{release} directory.
+// RUN: rm -rf %T/opt
+// RUN: mkdir -p %T/opt
+// RUN: cp -r %S/Inputs/rocm %T/opt/rocm-3.9.0-1234
+// RUN: cp -r %S/Inputs/rocm %T/opt/rocm-3.10.0
+// RUN: %clang -### -target x86_64-linux-gnu --sysroot=%T \
+// RUN:   --print-rocm-search-dirs %s 2>&1 \
+// RUN:   | FileCheck -check-prefixes=ROCM-REL %s
+
 // Test ROCm installation built by SPACK by invoke clang at 
%T/rocm-spack/llvm-amdgpu-*
 // directory through a soft link.
 
@@ -60,6 +74,11 @@
 
 // COMMON: "-triple" "amdgcn-amd-amdhsa"
 
+// ROCM-ENV: ROCm installation search path: {{.*}}/Inputs/rocm
+
+// ROCM-REL: ROCm installation search path: {{.*}}/opt/rocm
+// ROCM-REL: ROCm installation search path: {{.*}}/opt/rocm-3.10.0
+
 // SPACK: ROCm installation search path (Spack 4.0.0): [[DIR:.*]]
 // SPACK: ROCm installation search path: [[CLANG:.*]]
 // SPACK: ROCm installation search path: [[CLANG]]/lib/clang/{{[0-9.]+}}
Index: clang/lib/Driver/ToolChains/AMDGPU.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -186,6 +186,12 @@
 ROCmSearchDirs.emplace_back(RocmPathArg.str());
 DoPrintROCmSearchDirs();
 return ROCmSearchDirs;
+  } else if (const char *RocmPathEnv = ::getenv("ROCM_PATH")) {
+if (!StringRef(RocmPathEnv).empty()) {
+  ROCmSearchDirs.emplace_back(RocmPathEnv);
+  DoPrintROCmSearchDirs();
+  return ROCmSearchDirs;
+}
   }
 
   // Try to find relative to the compiler binary.
@@ -247,6 +253,45 @@
 
   ROCmSearchDirs.emplace_back(D.SysRoot + "/opt/rocm",
   /*StrictChecking=*/true);
+
+  // Find the latest /opt/rocm-{release} directory.
+  std::error_code EC;
+  std::string LatestROCm;
+  llvm::VersionTuple LatestVer;
+  // Get ROCm version from ROCm directory name.
+  auto GetROCmVersion = [](StringRef DirName) {
+llvm::VersionTuple V;
+std::string VerStr = DirName.drop_front(strlen("rocm-")).str();
+// The ROCm directory name follows the format of
+// rocm-{major}.{minor}.{subMinor}[-{build}]
+auto Loc = StringRef(VerStr).rfind('_');
+if (Loc != StringRef::npos)
+  VerStr[Loc] = '.';
+V.tryParse(VerStr);
+return V;
+  };
+  for (llvm::vfs::directory_iterator
+   File = D.getVFS().dir_begin(D.SysRoot + "/opt", EC),
+   FileEnd;
+   File != FileEnd && !EC; File.increment(EC)) {
+llvm::StringRef FileName = llvm::sys::path::filename(File->path());
+if (!FileName.startswith("rocm-"))
+  continue;
+if (LatestROCm.empty()) {
+  LatestROCm = FileName.str();
+  LatestVer = GetROCmVersion(LatestROCm);
+  continue;
+}
+auto Ver = GetROCmVersion(FileName);
+if (LatestVer < Ver) {
+  LatestROCm = FileName.str();
+  LatestVer = Ver;
+}
+  }
+  if (!LatestROCm.empty())
+ROCmSearchDirs.emplace_back(D.SysRoot + "/opt/" + LatestROCm,
+/*StrictChecking=*/true);
+
   DoPrintROCmSearchDirs();
   return ROCmSearchDirs;
 }


Index: clang/test/Driver/rocm-detect.hip
===
--- clang/test/Driver/rocm-detect.hip
+++ clang/test/Driver/rocm-detect.hip
@@ -21,6 +21,20 @@
 // RUN:   --rocm-path=%S/Inputs/rocm %s 2>&1 \
 // RUN:   | FileCheck -check-prefixes=COMMON,NODEFAULTLIBS %s
 
+// Test environment variable ROCM_PATH.
+// RUN: env ROCM_PATH=%S/Inputs/rocm %clang -### -target x86_64-linux-gnu \
+// RUN:   --print-rocm-search-dirs %s 2>&1 \
+// RUN:   | FileCheck -check-prefixes=ROCM-ENV %s
+
+// Test detecting latest /opt/rocm-{release} directory.
+// RUN: rm -rf %T/opt
+// RUN: mkdir -p %T/opt
+// RUN: cp -r %S/Inputs/rocm %T/opt/rocm-3.9.0-1234
+// RUN: cp -r %S/Inputs/rocm %T/opt/rocm-3.10.0
+// RUN: %clang -### -target x86_64-linux-gnu --sysroot=%T \
+// RUN:   --print-rocm-search-dirs %s 2>&1 \
+// RUN:   | FileCheck -check-prefixes=ROCM-REL %s
+
 // Test ROCm installation built by SPACK by invoke clang at %T/rocm-spack/llvm-amdgpu-*
 // directory through a soft link.
 
@@ -60,6 +74,11 @@
 
 

[PATCH] D88220: [C++20] P1825R0: More implicit moves

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

Confirmed: yikes. @nullptr.cpp, I'm going to open a new pull request to revert 
this, to poke the buildbot, just in case reverting it causes new failures. But 
I certainly don't object if someone wants to be more bold than me in reverting 
quickly.
Here's a reduced test case: https://godbolt.org/z/dz43W4

  struct A {
  A(A&&);
  A(const A&);
  };
  
  extern A global;
  
  A f() {
  A& r = global;
  return r;
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88220

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

[PATCH] D98892: [HWASan] Mention x86_64 aliasing mode in design doc.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse updated this revision to Diff 331895.
morehouse marked an inline comment as done.
morehouse added a comment.

- Expand on lack of 32 bit support.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98892

Files:
  clang/docs/HardwareAssistedAddressSanitizerDesign.rst


Index: clang/docs/HardwareAssistedAddressSanitizerDesign.rst
===
--- clang/docs/HardwareAssistedAddressSanitizerDesign.rst
+++ clang/docs/HardwareAssistedAddressSanitizerDesign.rst
@@ -19,13 +19,17 @@
 sources of AddressSanitizer's memory overhead.
 See the `AddressSanitizer paper`_ for details.
 
-AArch64 has the `Address Tagging`_ (or top-byte-ignore, TBI), a hardware 
feature that allows
-software to use 8 most significant bits of a 64-bit pointer as
+AArch64 has `Address Tagging`_ (or top-byte-ignore, TBI), a hardware feature 
that allows
+software to use the 8 most significant bits of a 64-bit pointer as
 a tag. HWASAN uses `Address Tagging`_
 to implement a memory safety tool, similar to :doc:`AddressSanitizer`,
 but with smaller memory overhead and slightly different (mostly better)
 accuracy guarantees.
 
+Intel's `Linear Address Masking`_ (LAM) also provides address tagging for
+x86_64, though it is not widely available in hardware yet.  For x86_64, HWASAN
+has a limited implementation using page aliasing instead.
+
 Algorithm
 =
 * Every heap/stack/global memory object is forcibly aligned by `TG` bytes
@@ -266,7 +270,15 @@
 will have limited deployability since not all of the code is
 typically instrumented.
 
-The HWASAN's approach is not applicable to 32-bit architectures.
+On x86_64, HWASAN utilizes page aliasing to place tags in userspace address
+bits.  Currently only heap tagging is supported.  The page aliases rely on
+shared memory, which will cause heap memory to be shared between processes if
+the application calls ``fork()``.  Therefore x86_64 is really only safe for
+applications that do not fork.
+
+HWASAN does not currently support 32-bit architectures since they do not
+support `Address Tagging`_ and the address space is too constrained to easily
+implement page aliasing.
 
 
 Related Work
@@ -284,4 +296,4 @@
 .. _SPARC ADI: 
https://lazytyped.blogspot.com/2017/09/getting-started-with-adi.html
 .. _AddressSanitizer paper: 
https://www.usenix.org/system/files/conference/atc12/atc12-final39.pdf
 .. _Address Tagging: 
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0024a/ch12s05s01.html
-
+.. _Linear Address Masking: 
https://software.intel.com/content/www/us/en/develop/download/intel-architecture-instruction-set-extensions-programming-reference.html


Index: clang/docs/HardwareAssistedAddressSanitizerDesign.rst
===
--- clang/docs/HardwareAssistedAddressSanitizerDesign.rst
+++ clang/docs/HardwareAssistedAddressSanitizerDesign.rst
@@ -19,13 +19,17 @@
 sources of AddressSanitizer's memory overhead.
 See the `AddressSanitizer paper`_ for details.
 
-AArch64 has the `Address Tagging`_ (or top-byte-ignore, TBI), a hardware feature that allows
-software to use 8 most significant bits of a 64-bit pointer as
+AArch64 has `Address Tagging`_ (or top-byte-ignore, TBI), a hardware feature that allows
+software to use the 8 most significant bits of a 64-bit pointer as
 a tag. HWASAN uses `Address Tagging`_
 to implement a memory safety tool, similar to :doc:`AddressSanitizer`,
 but with smaller memory overhead and slightly different (mostly better)
 accuracy guarantees.
 
+Intel's `Linear Address Masking`_ (LAM) also provides address tagging for
+x86_64, though it is not widely available in hardware yet.  For x86_64, HWASAN
+has a limited implementation using page aliasing instead.
+
 Algorithm
 =
 * Every heap/stack/global memory object is forcibly aligned by `TG` bytes
@@ -266,7 +270,15 @@
 will have limited deployability since not all of the code is
 typically instrumented.
 
-The HWASAN's approach is not applicable to 32-bit architectures.
+On x86_64, HWASAN utilizes page aliasing to place tags in userspace address
+bits.  Currently only heap tagging is supported.  The page aliases rely on
+shared memory, which will cause heap memory to be shared between processes if
+the application calls ``fork()``.  Therefore x86_64 is really only safe for
+applications that do not fork.
+
+HWASAN does not currently support 32-bit architectures since they do not
+support `Address Tagging`_ and the address space is too constrained to easily
+implement page aliasing.
 
 
 Related Work
@@ -284,4 +296,4 @@
 .. _SPARC ADI: https://lazytyped.blogspot.com/2017/09/getting-started-with-adi.html
 .. _AddressSanitizer paper: https://www.usenix.org/system/files/conference/atc12/atc12-final39.pdf
 .. _Address Tagging: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0024a/ch12s05s01.html
-
+.. _L

[PATCH] D98951: [clang][ASTImporter] Add import API for 'const Type *' (NFC).

[Raphael Isemann via Phabricator via cfe-commits]

teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: whisperity.

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98951

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

[PATCH] D98959: [OpenCL] Add as_size/ptrdiff/intptr/uintptr_t operators

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh created this revision.
svenvh added a reviewer: Anastasia.
svenvh added a project: clang.
Herald added subscribers: ldrumm, yaxunl.
svenvh requested review of this revision.
Herald added a subscriber: cfe-commits.

size_t and friends are built-in scalar data types and s6.4.4.2 of the
OpenCL C Specification says the `as_type()` operator must be available
for these data types.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98959

Files:
  clang/lib/Headers/opencl-c-base.h
  clang/test/SemaOpenCL/as_type.cl


Index: clang/test/SemaOpenCL/as_type.cl
===
--- clang/test/SemaOpenCL/as_type.cl
+++ clang/test/SemaOpenCL/as_type.cl
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 %s -emit-llvm -triple spir-unknown-unknown 
-finclude-default-header -o - -verify -fsyntax-only
+// RUN: %clang_cc1 %s -emit-llvm -DBITS=32 -triple spir-unknown-unknown 
-finclude-default-header -fdeclare-opencl-builtins -o - -verify -fsyntax-only
+// RUN: %clang_cc1 %s -emit-llvm -DBITS=64 -triple spir64-unknown-unknown 
-finclude-default-header -fdeclare-opencl-builtins -o - -verify -fsyntax-only
 
 char3 f1(char16 x) {
   return  __builtin_astype(x, char3); // expected-error{{invalid 
reinterpretation: sizes of 'char3' (vector of 3 'char' values) and '__private 
char16' (vector of 16 'char' values) must match}}
@@ -12,3 +13,15 @@
 char src = 1;
 int dst = as_int(src); // expected-error{{invalid reinterpretation: sizes 
of 'int' and '__private char' must match}}
 }
+
+void target_dependent(int i, long l) {
+  size_t size1 = as_size_t(i);
+#if BITS == 64
+  // expected-error@-2{{sizes of 'size_t' (aka 'unsigned long') and '__private 
int' must match}}
+#endif
+
+  size_t size2 = as_size_t(l);
+#if BITS == 32
+  // expected-error@-2{{sizes of 'size_t' (aka 'unsigned int') and '__private 
long' must match}}
+#endif
+}
Index: clang/lib/Headers/opencl-c-base.h
===
--- clang/lib/Headers/opencl-c-base.h
+++ clang/lib/Headers/opencl-c-base.h
@@ -545,6 +545,11 @@
 #define as_half16(x) __builtin_astype((x), half16)
 #endif // cl_khr_fp16
 
+#define as_size_t(x) __builtin_astype((x), size_t)
+#define as_ptrdiff_t(x) __builtin_astype((x), ptrdiff_t)
+#define as_intptr_t(x) __builtin_astype((x), intptr_t)
+#define as_uintptr_t(x) __builtin_astype((x), uintptr_t)
+
 // OpenCL v1.1 s6.9, v1.2/2.0 s6.10 - Function qualifiers
 
 #define __kernel_exec(X, typen) __kernel \


Index: clang/test/SemaOpenCL/as_type.cl
===
--- clang/test/SemaOpenCL/as_type.cl
+++ clang/test/SemaOpenCL/as_type.cl
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 %s -emit-llvm -triple spir-unknown-unknown -finclude-default-header -o - -verify -fsyntax-only
+// RUN: %clang_cc1 %s -emit-llvm -DBITS=32 -triple spir-unknown-unknown -finclude-default-header -fdeclare-opencl-builtins -o - -verify -fsyntax-only
+// RUN: %clang_cc1 %s -emit-llvm -DBITS=64 -triple spir64-unknown-unknown -finclude-default-header -fdeclare-opencl-builtins -o - -verify -fsyntax-only
 
 char3 f1(char16 x) {
   return  __builtin_astype(x, char3); // expected-error{{invalid reinterpretation: sizes of 'char3' (vector of 3 'char' values) and '__private char16' (vector of 16 'char' values) must match}}
@@ -12,3 +13,15 @@
 char src = 1;
 int dst = as_int(src); // expected-error{{invalid reinterpretation: sizes of 'int' and '__private char' must match}}
 }
+
+void target_dependent(int i, long l) {
+  size_t size1 = as_size_t(i);
+#if BITS == 64
+  // expected-error@-2{{sizes of 'size_t' (aka 'unsigned long') and '__private int' must match}}
+#endif
+
+  size_t size2 = as_size_t(l);
+#if BITS == 32
+  // expected-error@-2{{sizes of 'size_t' (aka 'unsigned int') and '__private long' must match}}
+#endif
+}
Index: clang/lib/Headers/opencl-c-base.h
===
--- clang/lib/Headers/opencl-c-base.h
+++ clang/lib/Headers/opencl-c-base.h
@@ -545,6 +545,11 @@
 #define as_half16(x) __builtin_astype((x), half16)
 #endif // cl_khr_fp16
 
+#define as_size_t(x) __builtin_astype((x), size_t)
+#define as_ptrdiff_t(x) __builtin_astype((x), ptrdiff_t)
+#define as_intptr_t(x) __builtin_astype((x), intptr_t)
+#define as_uintptr_t(x) __builtin_astype((x), uintptr_t)
+
 // OpenCL v1.1 s6.9, v1.2/2.0 s6.10 - Function qualifiers
 
 #define __kernel_exec(X, typen) __kernel \
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98868: [Driver] Add -print-runtime-dir

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm

I'm somewhat surprised this doesn't exist already, but I looked at the -print-* 
options and don't see anything.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98868

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

[PATCH] D98856: Always emit error for wrong interfaces to scalable vectors, unless cmdline flag is passed.

[Christopher Tetreault via Phabricator via cfe-commits]

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

lgtm




Comment at: llvm/lib/CodeGen/ValueTypes.cpp:17
 
+unsigned EVT::getVectorNumElements() const {
+  auto Error = []() {

Out of curiosity, what is the eventual plan for this function? Does it go away, 
or will it just assert if this is a scalable vector?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98856

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

[PATCH] D98856: Always emit error for wrong interfaces to scalable vectors, unless cmdline flag is passed.

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5052-5055
+  if (isa(JA)) {
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("-treat-scalable-fixed-error-as-warning");
+  }

Are there any concerns related to LTO here? Could we live with LTO triggering 
errors for the invalid uses?  Part of me thinks this is reasonable given the 
clang exception is more about ensuring we can continue active development until 
we're ready to press the "it's supported" switch.



Comment at: llvm/include/llvm/Support/TypeSize.h:31-35
+namespace TypeSizeClOpt {
+/// The ScalableErrorAsWarning is a temporary measure to suppress errors from
+/// using the wrong interface.
+extern cl::opt ScalableErrorAsWarning;
+} // namespace TypeSizeClOpt

Why is this need here? Can it just be externs where it's accessed?



Comment at: llvm/lib/CodeGen/ValueTypes.cpp:28
+  if (isScalableVector()) {
+if (llvm::TypeSizeClOpt::ScalableErrorAsWarning)
+  WithColor::warning()

I guess related to my comment for TypeSize.h but I'm wondering if it's better 
to move all error reporting into TypeSize.cpp. For example:
```
if (isScalableVector())
  reportInvalidSizeRequest("EVT::getVectorNumElements()", 
"EVT::getVectorElementCount()")

reportInvalidSizeRequest(string bad_func, string good_fun) {
#ifndef STRICT_FIXED_SIZE_VECTORS
  if (ScalableErrorAsWarning)
warning(don't use badfunc, use good_fun instead)
  else
#endif
Error()
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98856

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

[PATCH] D98951: [clang][ASTImporter] Add import API for 'const Type *' (NFC).

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik accepted this revision.
shafik added a comment.

Can we add a test for the `getCapturedVLAType` case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98951

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

[PATCH] D97119: [flang][driver] Add options for -std=f2018

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.



Comment at: flang/test/Driver/std2018.f90:1
+! REQUIRES: new-flang-driver
+! Ensure argument -std=f2018 works as expected.

Would it be possible to share this test with `f18`?



Comment at: flang/test/Driver/std2018_wrong.f90:14-22
+subroutine foo2()
+do 01 m=1,2
+  select case (m)
+  case default
+print*, "default", m
+  case (1)
+print*, "start"

No input is needed for this test, is it? IIUC, this bit can be deleted to make 
the test leaner.


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

https://reviews.llvm.org/D97119

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

[clang] aafc3f7 - [Driver] Add -print-runtime-dir

[Markus Böck via cfe-commits]

Author: Markus Böck
Date: 2021-03-19T17:48:03+01:00
New Revision: aafc3f7be804d117a632365489a18c3e484a3931

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

LOG: [Driver] Add -print-runtime-dir

This patch adds a new command line option to clang which outputs the directory 
containing clangs runtime libraries to stdout.

The primary use case for this command line flag is for build systems using 
clang-cl. Build systems when using clang-cl invoke the linker, that is either 
link or lld-link in this case, directly instead of invoking the compiler for 
the linking process as is common with the other drivers. This leads to issues 
when runtime libraries of clang, such as sanitizers or profiling, have to be 
linked in as the compiler cannot communicate the link directory to the linker.

Using this flag, build systems would be capable of getting the directory 
containing all of clang's runtime libraries and add it to the linker path.

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

Added: 

clang/test/Driver/Inputs/resource_dir/lib/windows/clang_rt.builtins-x86_64.lib

clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-pc-windows-msvc/clang_rt.builtins.lib

Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Driver/Driver.cpp
clang/test/Driver/immediate-options.c

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index a9b43a8fe620..b7efb7469a23 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3566,6 +3566,8 @@ def print_targets : Flag<["-", "--"], "print-targets">,
   HelpText<"Print the registered targets">;
 def print_rocm_search_dirs : Flag<["-", "--"], "print-rocm-search-dirs">,
   HelpText<"Print the paths used for finding ROCm installation">;
+def print_runtime_dir : Flag<["-", "--"], "print-runtime-dir">,
+  HelpText<"Print the directory pathname containing clangs runtime libraries">;
 def private__bundle : Flag<["-"], "private_bundle">;
 def pthreads : Flag<["-"], "pthreads">;
 defm pthread : BoolOption<"", "pthread",

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index dbd365e7c9bc..e70263e6a295 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -1824,6 +1824,15 @@ bool Driver::HandleImmediateArgs(const Compilation &C) {
 return false;
   }
 
+  if (C.getArgs().hasArg(options::OPT_print_runtime_dir)) {
+if (auto RuntimePath = TC.getRuntimePath()) {
+  llvm::outs() << *RuntimePath << '\n';
+  return false;
+}
+llvm::outs() << TC.getCompilerRTPath() << '\n';
+return false;
+  }
+
   // FIXME: The following handlers should use a callback mechanism, we don't
   // know what the client would like to do.
   if (Arg *A = C.getArgs().getLastArg(options::OPT_print_file_name_EQ)) {

diff  --git 
a/clang/test/Driver/Inputs/resource_dir/lib/windows/clang_rt.builtins-x86_64.lib
 
b/clang/test/Driver/Inputs/resource_dir/lib/windows/clang_rt.builtins-x86_64.lib
new file mode 100644
index ..e69de29bb2d1

diff  --git 
a/clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-pc-windows-msvc/clang_rt.builtins.lib
 
b/clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-pc-windows-msvc/clang_rt.builtins.lib
new file mode 100644
index ..e69de29bb2d1

diff  --git a/clang/test/Driver/immediate-options.c 
b/clang/test/Driver/immediate-options.c
index 723a6fa302f8..c398e0d41c6e 100644
--- a/clang/test/Driver/immediate-options.c
+++ b/clang/test/Driver/immediate-options.c
@@ -17,3 +17,15 @@
 // Allow unspecified output because the value of CLANG_RESOURCE_DIR is unknown.
 // RUN: %clang -print-resource-dir | FileCheck %s 
-check-prefix=PRINT-RESOURCE-DIR
 // PRINT-RESOURCE-DIR: {{.+}}
+
+// Default resource-dir layout
+// RUN: %clang -print-runtime-dir --target=x86_64-pc-windows-msvc \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:  | FileCheck --check-prefix=PRINT-RUNTIME-DIR %s
+// PRINT-RUNTIME-DIR: lib{{/|\\}}windows
+
+// Per target dir layout
+// RUN: %clang -print-runtime-dir --target=x86_64-pc-windows-msvc \
+// RUN:-resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:  | FileCheck --check-prefix=PRINT-RUNTIME-DIR-PER-TARGET %s
+// PRINT-RUNTIME-DIR-PER-TARGET: lib{{/|\\}}x86_64-pc-windows-msvc



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

[PATCH] D98868: [Driver] Add -print-runtime-dir

[Markus Böck 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 rGaafc3f7be804: [Driver] Add -print-runtime-dir (authored by 
zero9178).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98868

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/Inputs/resource_dir/lib/windows/clang_rt.builtins-x86_64.lib
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/x86_64-pc-windows-msvc/clang_rt.builtins.lib
  clang/test/Driver/immediate-options.c


Index: clang/test/Driver/immediate-options.c
===
--- clang/test/Driver/immediate-options.c
+++ clang/test/Driver/immediate-options.c
@@ -17,3 +17,15 @@
 // Allow unspecified output because the value of CLANG_RESOURCE_DIR is unknown.
 // RUN: %clang -print-resource-dir | FileCheck %s 
-check-prefix=PRINT-RESOURCE-DIR
 // PRINT-RESOURCE-DIR: {{.+}}
+
+// Default resource-dir layout
+// RUN: %clang -print-runtime-dir --target=x86_64-pc-windows-msvc \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:  | FileCheck --check-prefix=PRINT-RUNTIME-DIR %s
+// PRINT-RUNTIME-DIR: lib{{/|\\}}windows
+
+// Per target dir layout
+// RUN: %clang -print-runtime-dir --target=x86_64-pc-windows-msvc \
+// RUN:-resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:  | FileCheck --check-prefix=PRINT-RUNTIME-DIR-PER-TARGET %s
+// PRINT-RUNTIME-DIR-PER-TARGET: lib{{/|\\}}x86_64-pc-windows-msvc
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1824,6 +1824,15 @@
 return false;
   }
 
+  if (C.getArgs().hasArg(options::OPT_print_runtime_dir)) {
+if (auto RuntimePath = TC.getRuntimePath()) {
+  llvm::outs() << *RuntimePath << '\n';
+  return false;
+}
+llvm::outs() << TC.getCompilerRTPath() << '\n';
+return false;
+  }
+
   // FIXME: The following handlers should use a callback mechanism, we don't
   // know what the client would like to do.
   if (Arg *A = C.getArgs().getLastArg(options::OPT_print_file_name_EQ)) {
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3566,6 +3566,8 @@
   HelpText<"Print the registered targets">;
 def print_rocm_search_dirs : Flag<["-", "--"], "print-rocm-search-dirs">,
   HelpText<"Print the paths used for finding ROCm installation">;
+def print_runtime_dir : Flag<["-", "--"], "print-runtime-dir">,
+  HelpText<"Print the directory pathname containing clangs runtime libraries">;
 def private__bundle : Flag<["-"], "private_bundle">;
 def pthreads : Flag<["-"], "pthreads">;
 defm pthread : BoolOption<"", "pthread",


Index: clang/test/Driver/immediate-options.c
===
--- clang/test/Driver/immediate-options.c
+++ clang/test/Driver/immediate-options.c
@@ -17,3 +17,15 @@
 // Allow unspecified output because the value of CLANG_RESOURCE_DIR is unknown.
 // RUN: %clang -print-resource-dir | FileCheck %s -check-prefix=PRINT-RESOURCE-DIR
 // PRINT-RESOURCE-DIR: {{.+}}
+
+// Default resource-dir layout
+// RUN: %clang -print-runtime-dir --target=x86_64-pc-windows-msvc \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:  | FileCheck --check-prefix=PRINT-RUNTIME-DIR %s
+// PRINT-RUNTIME-DIR: lib{{/|\\}}windows
+
+// Per target dir layout
+// RUN: %clang -print-runtime-dir --target=x86_64-pc-windows-msvc \
+// RUN:-resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:  | FileCheck --check-prefix=PRINT-RUNTIME-DIR-PER-TARGET %s
+// PRINT-RUNTIME-DIR-PER-TARGET: lib{{/|\\}}x86_64-pc-windows-msvc
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1824,6 +1824,15 @@
 return false;
   }
 
+  if (C.getArgs().hasArg(options::OPT_print_runtime_dir)) {
+if (auto RuntimePath = TC.getRuntimePath()) {
+  llvm::outs() << *RuntimePath << '\n';
+  return false;
+}
+llvm::outs() << TC.getCompilerRTPath() << '\n';
+return false;
+  }
+
   // FIXME: The following handlers should use a callback mechanism, we don't
   // know what the client would like to do.
   if (Arg *A = C.getArgs().getLastArg(options::OPT_print_file_name_EQ)) {
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3566,6 +3566,8 @@
   HelpText<"Print the registered targets">;
 def print_rocm_search_dirs : Flag<

[PATCH] D98237: [clang-format] Option for empty lines after an access modifier.

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:9212
+  StyleWithLine.EmptyLinesAfterAccessModifier = 1u;
+  EXPECT_EQ(test2NL, format(test0NL, StyleWithLine));
+  EXPECT_EQ(test2NL, format(test1NL, StyleWithLine));

Max_S wrote:
> HazardyKnusperkeks wrote:
> > Max_S wrote:
> > > MyDeveloperDay wrote:
> > > > yeah I'm not a fan of this like this... sorry... just write the test 
> > > > out in long form, when it goes wrong I don't have to be a compiler to 
> > > > understand what is going wrong I can just see it.
> > > I can change this, but the current output of the tests is (I forced the 
> > > error):
> > > ```
> > > //llvm-project/clang/unittests/Format/FormatTest.cpp:72: Failure
> > >   Expected: Expected.str()
> > >   Which is: "class Foo {\nprivate:\n\n  int i;\n};"
> > > To be equal to: format(Expected, Style)
> > >   Which is: "class Foo {\nprivate:\n  int i;\n};"
> > > With diff:
> > > @@ -1,5 @@
> > >  class Foo {
> > >  private:
> > > -
> > >int i;
> > >  };
> > > ```
> > > 
> > > Which is actually human readable in this case. Shall I still change it?
> > I'm a fan of it. :)
> > Especially with the demonstration that the string is still expanded.
> I changed all to EXPECT_EQ since the failer output there is better. It shows 
> the variables names and not the arbitrary code in verfyFormat. Hope this is 
> ok.
> ```
> /home/msagebaum/Kaiserslautern/Programms/temp/llvm-project/clang/unittests/Format/FormatTest.cpp:9202:
>  Failure
>   Expected: test2NL
>   Which is: "class Foo {\nprivate:\n\n  int i;\n};"
> To be equal to: format(test1NL)
>   Which is: "class Foo {\nprivate:\n  int i;\n};"
> With diff:
> @@ -1,5 @@
>  class Foo {
>  private:
> -
>int i;
>  };
> 
> 
why would we break with convention?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98237

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

[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Btw, while this change does explain _what_ it does, it doesn't actually say the 
exact reason _why_. Cleanliness? Sure, that's nice... Or is it a case where 
e.g. some translations produce different error messages?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98278

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

[PATCH] D98237: [clang-format] Option for empty lines after an access modifier.

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay requested changes to this revision.
MyDeveloperDay added inline comments.
This revision now requires changes to proceed.



Comment at: clang/unittests/Format/FormatTest.cpp:9392
+ "};\n";
+  EXPECT_EQ(NL_B_3_A_1_I_3, format(NL_B_3_A_3_I_3, Style));
+

I can't read this, I can't tell if you've fat fingered a test, I feel these are 
better done systematically one by one with the text in the verifyFormat("...") 
it makes it easier when they fail to understand what is going on.

Also its ok to have more than one TEST_F if you want to handle the different 
states in small batches

I think you undetersand what NL_B_3_A_0_I_0 means and how it differs form 
NL_B_0_A_3_I_0  but others won't


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98237

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

[PATCH] D98237: [clang-format] Option for empty lines after an access modifier.

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

could you please mark your comments done when they are done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98237

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

[PATCH] D98816: PR49585: Emit the jump destination for a for loop 'continue' from within the scope of the condition variable.

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht added a comment.

We're seeing a crash in the optimizer after this patch, with the following 
logged in assert builds: `assert.h assertion failed at 
llvm/include/llvm/Support/Casting.h:104 in static bool 
llvm::isa_impl_cl::doit(const From 
*) [To = llvm::InvokeInst, From = const llvm::Instruction *]: Val && "isa<> 
used on a null pointer"`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98816

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

[PATCH] D98237: [clang-format] Option for empty lines after an access modifier.

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:2132
+**EmptyLineAfterAccessModifier** (``EmptyLineAfterAccessModifierStyle``)
+  Defines in which cases to put empty line after access modifiers.
+





Comment at: clang/docs/ClangFormatStyleOptions.rst:2159
+Always add empty line after access modifiers if there are none.
+MaxEmptyLinesToKeep is applied also.
+

if MaxEmptyLineToKeep is 0 then surely ELAAMS_Always should win, as this is the 
lesser i.e. if people never uses extra lines except after access modifiers this 
is what they would set? did I missunderstand?

otherwise doesn't MaxEmptyLinesToKeep just nullify the whole option? meaning 
its useless for anyone using it?

I agree in the ELAAMS_Leave  case that MaxEmptyLinesToKeep  should win.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98237

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

[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

[Markus Böck via Phabricator via cfe-commits]

zero9178 added a comment.

In D98278#2637826 , @mstorsjo wrote:

> Btw, while this change does explain _what_ it does, it doesn't actually say 
> the exact reason _why_. Cleanliness? Sure, that's nice... Or is it a case 
> where e.g. some translations produce different error messages?

Now that you mention it, it's indeed not as clear as I thought. But yes, in the 
case of MSVCs STL, the messages from `std::error_codes` which are used by 
various LLVM tools produce different strings then using `strerror` (the C 
function also called by Python) with the same error codes (Specifically, it has 
different casing).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98278

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

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

  enum {
  A,
  B,
  C,
  } ShortEnum1,
  ShortEnum2;

I've seen this before maybe with regard to something else, but can't quite 
recall. (maybe a bug in the bug tracker)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938

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

[PATCH] D98867: [HIP] Fix ROCm detection

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:267-268
+// rocm-{major}.{minor}.{subMinor}[-{build}]
+auto Loc = StringRef(VerStr).rfind('_');
+if (Loc != StringRef::npos)
+  VerStr[Loc] = '.';

You don't need `StringRef` here. `VerStr.rfind()` will do.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:269
+if (Loc != StringRef::npos)
+  VerStr[Loc] = '.';
+V.tryParse(VerStr);

This only deals with a single `.` in the version. It's likely that we will have 
to deal with a patch release at some point, so you may want to replace all 
instances.



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

https://reviews.llvm.org/D98867

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

[PATCH] D97680: [OpenMP] Simplify GPU memory globalization

[Jose Manuel Monsalve Diaz via Phabricator via cfe-commits]

josemonsalve2 added inline comments.



Comment at: clang/test/OpenMP/nvptx_parallel_codegen.cpp:4
-// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown 
-fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-ppc-host.bc -o - -disable-llvm-optzns | FileCheck 
%s --check-prefix CHECK --check-prefix CHECK-64 --check-prefix SEQ
-// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown 
-fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-ppc-host.bc -o - -disable-llvm-optzns 
-fopenmp-cuda-parallel-target-regions | FileCheck %s --check-prefix CHECK 
--check-prefix CHECK-64 --check-prefix PAR
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple i386-unknown-unknown 
-fopenmp-targets=nvptx-nvidia-cuda -emit-llvm-bc %s -o %t-x86-host.bc

Is this flag `-fopenmp-cuda-parallel-target-regions` useful after this change? 
I know it was used to determine something in globalization, and I believe this 
was removed. But is it used for anything else somewhere else or could it be 
removed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97680

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

[PATCH] D97080: [flang][driver] Add -fintrinsic-modules-path option

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:298
+  driverPath.append("/../include/flang/");
+  return driverPath.str().str();
+}

Given this [[ 
https://github.com/llvm/llvm-project/blob/987ee6e3cc1fb672b3ed201e72a5281c2ec88c99/llvm/include/llvm/ADT/SmallString.h#L271-L273
 | conversion operator ]], wouldn't `return std::string(driverPath);` be more 
efficient? 


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

https://reviews.llvm.org/D97080

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

[PATCH] D98902: [Clang][OpenMP][NVPTX] Fixed failure in openmp-offload-gpu.c if the system has CUDA

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D98902#2636308 , @jdoerfert wrote:

> @tra, so you think we should not do this? The user will see a link error late 
> I assume, might be better.

If I compile a `__device__ float foo(float f) { return sin(f); }` and it does 
compile to working GPU code, I If I compile the same code with `-S`, I would 
assume that produced PTX is still compileable with ptxas. After all, it was 
when the source was compiled with `-c`. That will no longer be the case if you 
disable linking with libdevice.

If the user wants to disable linking with the libdevice, there's already 
`-nogpulib` for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98902

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

[PATCH] D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/test/CodeGen/unique-internal-linkage-names-dwarf.c:34-39
+static int go(a) int a;
+{
+  return glob + a;
+}
+
+

hoy wrote:
> dblaikie wrote:
> > Does this need to be down here? Or would the code be a well exercised if it 
> > was up next to the go declaration above?
> Yes, it needs to be here. Otherwise it will just like the function `bar` 
> above that doesn't get a uniquefied name. I think moving the definition up to 
> right after the declaration hides the declaration.
Not sure I follow - do you mean that if the go declaration and go definition 
were next to each other, this test would (mechanically speaking) not validate 
what the patch? Or that it would be less legible, but still mechanically 
correct?

I think it would be (assuming it's still mechanically correct) more legible to 
put the declaration next to the definition - the comment describes why the 
declaration is significant/why the definition is weird, and seeing all that 
together would be clearer to me than spreading it out/having to look further 
away to see what's going on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98799

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

[PATCH] D98948: [analyzer][solver] Fix infeasible constraints (PR49642)

[Balázs Benics via Phabricator via cfe-commits]

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

Aa, get it. Looks good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98948

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

[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D98278#2637866 , @zero9178 wrote:

> In D98278#2637826 , @mstorsjo wrote:
>
>> Btw, while this change does explain _what_ it does, it doesn't actually say 
>> the exact reason _why_. Cleanliness? Sure, that's nice... Or is it a case 
>> where e.g. some translations produce different error messages?
>
> Now that you mention it, it's indeed not as clear as I thought. But yes, in 
> the case of MSVCs STL, the messages from `std::error_codes` which are used by 
> various LLVM tools produce different strings then using `strerror` (the C 
> function also called by Python) with the same error codes (Specifically, it 
> has different casing).

Ok, but would e.g. a case insensitive comparison have worked instead of this?

And didn't the python script have hardcoded strings, specifically for the MSVC 
case? Why weren't they written with the right casing for the case that they're 
supposed to match? I.e. was it an issue with the existing hardcoded strings, or 
did they work in one case but not another one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98278

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

[PATCH] D98278: [test] Add ability to get error messages from CMake for errc substitution

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D98278#2637866 , @zero9178 wrote:

> In D98278#2637826 , @mstorsjo wrote:
>
>> Btw, while this change does explain _what_ it does, it doesn't actually say 
>> the exact reason _why_. Cleanliness? Sure, that's nice... Or is it a case 
>> where e.g. some translations produce different error messages?
>
> Now that you mention it, it's indeed not as clear as I thought. But yes, in 
> the case of MSVCs STL, the messages from `std::error_codes` which are used by 
> various LLVM tools produce different strings then using `strerror` (the C 
> function also called by Python) with the same error codes (Specifically, it 
> has different casing).

Or turning the question another way around: We have a couple different bots 
that build and run the tests, successfully, with MSVC configurations. Are there 
tests that failed for you in your configuration, that succeed in the setup of 
the bots? Or are there other tests that aren't run as part of bots that you're 
fixing? It's all still very vague to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98278

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

[PATCH] D98923: [Driver] Pass -fexperimental-strict-floating-point to cc1 if it is specified

[Kevin P. Neal via Phabricator via cfe-commits]

kpn added a comment.

Test case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98923

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

[PATCH] D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name.

[Hongtao Yu via Phabricator via cfe-commits]

hoy added inline comments.



Comment at: clang/test/CodeGen/unique-internal-linkage-names-dwarf.c:34-39
+static int go(a) int a;
+{
+  return glob + a;
+}
+
+

dblaikie wrote:
> hoy wrote:
> > dblaikie wrote:
> > > Does this need to be down here? Or would the code be a well exercised if 
> > > it was up next to the go declaration above?
> > Yes, it needs to be here. Otherwise it will just like the function `bar` 
> > above that doesn't get a uniquefied name. I think moving the definition up 
> > to right after the declaration hides the declaration.
> Not sure I follow - do you mean that if the go declaration and go definition 
> were next to each other, this test would (mechanically speaking) not validate 
> what the patch? Or that it would be less legible, but still mechanically 
> correct?
> 
> I think it would be (assuming it's still mechanically correct) more legible 
> to put the declaration next to the definition - the comment describes why the 
> declaration is significant/why the definition is weird, and seeing all that 
> together would be clearer to me than spreading it out/having to look further 
> away to see what's going on.
When the `go` declaration and `go` definition were next to each other, the go 
function won't get a uniqufied name at all. The declaration will be overwritten 
by the definition. Only when the declaration is seen by others, such the 
callsite in `baz`, the declaration makes a difference by having the callsite 
use a uniqufied name.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98799

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

[PATCH] D98923: [Driver] Pass -fexperimental-strict-floating-point to cc1 if it is specified

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

Isn't OPT_fexperimental_strict_floating_point marked as a CC1Option in 
Options.td. Can the driver even recognize it?

Can you use -Xclang -fexperimental-strict-floating-point for your use case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98923

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

[PATCH] D98738: [clang-tidy] performance-* checks: Match AllowedTypes against qualified type names when they contain "::".

[Felix Berger via Phabricator via cfe-commits]

flx updated this revision to Diff 331941.
flx added a comment.

Applied changes suggested by ymandel, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98738

Files:
  clang-tools-extra/clang-tidy/utils/Matchers.h
  clang-tools-extra/docs/clang-tidy/checks/performance-for-range-copy.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-value-param.rst
  
clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy-allowed-types.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy-allowed-types.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy-allowed-types.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy-allowed-types.cpp
@@ -1,5 +1,5 @@
 // RUN: %check_clang_tidy %s performance-for-range-copy %t -- \
-// RUN: -config="{CheckOptions: [{key: performance-for-range-copy.AllowedTypes, value: '[Pp]ointer$;[Pp]tr$;[Rr]ef(erence)?$'}]}" \
+// RUN: -config="{CheckOptions: [{key: performance-for-range-copy.AllowedTypes, value: '[Pp]ointer$;[Pp]tr$;[Rr]ef(erence)?$;fully::Qualified'}]}" \
 // RUN: -- -fno-delayed-template-parsing
 
 template 
@@ -63,6 +63,14 @@
 
 typedef SomeComplexTemplate NotTooComplexRef;
 
+namespace fully {
+
+struct Qualified {
+  ~Qualified();
+};
+
+} // namespace fully
+
 void negativeSmartPointer() {
   for (auto P : View>()) {
 auto P2 = P;
@@ -124,3 +132,13 @@
 auto R2 = R;
   }
 }
+
+void negativeFullyQualified() {
+  for (auto Q : View>()) {
+auto Q2 = Q;
+  }
+  using fully::Qualified;
+  for (auto Q : View>()) {
+auto Q2 = Q;
+  }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-value-param.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-value-param.rst
+++ clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-value-param.rst
@@ -66,4 +66,7 @@
 
A semicolon-separated list of names of types allowed to be passed by value.
Regular expressions are accepted, e.g. `[Rr]ef(erence)?$` matches every type
-   with suffix `Ref`, `ref`, `Reference` and `reference`. The default is empty.
+   with suffix `Ref`, `ref`, `Reference` and `reference`. The default is
+   empty. If a name in the list contains the sequence `::` it is matched against
+   the qualified typename (i.e. `namespace::Type`, otherwise it is matched
+   against only the type name (i.e. `Type`).
Index: clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
+++ clang-tools-extra/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
@@ -43,5 +43,7 @@
 
A semicolon-separated list of names of types allowed to be initialized by
copying. Regular expressions are accepted, e.g. `[Rr]ef(erence)?$` matches
-   every type with suffix `Ref`, `ref`, `Reference` and `reference`. The
-   default is empty.
+   every type with suffix `Ref`, `ref`, `Reference` and `reference`. The default
+   is empty. If a name in the list contains the sequence `::` it is matched
+   against the qualified typename (i.e. `namespace::Type`, otherwise it is
+   matched against only the type name (i.e. `Type`).
Index: clang-tools-extra/docs/clang-tidy/checks/performance-for-range-copy.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/performance-for-range-copy.rst
+++ clang-tools-extra/docs/clang-tidy/checks/performance-for-range-copy.rst
@@ -31,4 +31,6 @@
A semicolon-separated list of names of types allowed to be copied in each
iteration. Regular expressions are accepted, e.g. `[Rr]ef(erence)?$` matches
every type with suffix `Ref`, `ref`, `Reference` and `reference`. The default
-   is empty.
+   is empty. If a name in the list contains the sequence `::` it is matched
+   against the qualified typename (i.e. `namespace::Type`, otherwise it is
+   matched against only the type name (i.e. `Type`).
Index: clang-tools-extra/clang-tidy/utils/Matchers.h
===
--- clang-tools-extra/clang-tidy/utils/Matchers.h
+++ clang-tools-extra/clang-tidy/utils/Matchers.h
@@ -49,11 +49,46 @@
   return pointerType(pointee(qualType(isConstQualified(;
 }
 
-AST_MATCHER_P(NamedDecl, matchesAnyListedName, std::vector,
-  NameList) {
-  return llvm::any_of(NameList, [&Node](const std::string &Name) {
-  return llvm::Regex(Name).match(Node.getName());
+class MatchesAnyListedNameMatcher
+: public 

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone created this revision.
Quuxplusone added reviewers: rsmith, nullptr.cpp, aaronpuchert, mizvekov, 
erik.pilkington, sberg, davidstone, david_stone.
Quuxplusone added a project: clang.
Quuxplusone requested review of this revision.
Herald added a subscriber: cfe-commits.

Review D88220  turns out to have some pretty 
severe bugs, but I *think* this patch fixes them.

Paper P1825  is supposed to enable implicit 
move from "non-volatile objects
and rvalue references to non-volatile object types." Instead, what was committed
seems to have enabled implicit move from "non-volatile things of all kinds,
except that if they're rvalue references then they must also refer to 
non-volatile
things." In other words, D88220  accidentally 
enabled implicit move from
lvalue object references (super yikes!) and also from non-object references
(such as references to functions).

These two cases are now fixed and regression-tested.

(For better and worse, D88220  was present in 
trunk for more than a month before anyone noticed the (very major!) bug — my 
thanks to @sberg for reporting it! — so we may be able to take our time fixing 
it, but also it may be another month before someone spots the //next// bug. I 
have no strong opinion on the process here.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98971

Files:
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CXX/class/class.init/class.copy.elision/p3.cpp

Index: clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
===
--- clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
+++ clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
@@ -292,3 +292,108 @@
   return b; // cxx20-error {{calling a private constructor of class 'test_ctor_param_rvalue_ref::B2'}}
 }
 } // namespace test_ctor_param_rvalue_ref
+
+namespace test_lvalue_ref_is_not_moved_from {
+
+struct Target {};
+  // expected-note@-1 {{candidate constructor (the implicit copy constructor) not viable}}
+  // expected-note@-2 {{candidate constructor (the implicit move constructor) not viable}}
+  // cxx11_14_17-note@-3 {{candidate constructor (the implicit copy constructor) not viable}}
+  // cxx11_14_17-note@-4 {{candidate constructor (the implicit move constructor) not viable}}
+
+struct CopyOnly {
+  CopyOnly(CopyOnly&&) = delete; // cxx20-note {{has been explicitly marked deleted here}}
+  CopyOnly(CopyOnly&);
+  operator Target() && = delete; // cxx20-note {{has been explicitly marked deleted here}}
+  operator Target() &;
+};
+
+struct MoveOnly {
+  MoveOnly(MoveOnly&&); // expected-note {{copy constructor is implicitly deleted because}}
+// cxx11_14_17-note@-1 {{copy constructor is implicitly deleted because}}
+  operator Target() &&; // expected-note {{candidate function not viable}}
+// cxx11_14_17-note@-1 {{candidate function not viable}}
+};
+
+extern CopyOnly copyonly;
+extern MoveOnly moveonly;
+
+CopyOnly t1() {
+CopyOnly& r = copyonly;
+return r;
+}
+
+CopyOnly t2() {
+CopyOnly&& r = static_cast(copyonly);
+return r; // cxx20-error {{call to deleted constructor}}
+}
+
+MoveOnly t3() {
+MoveOnly& r = moveonly;
+return r; // expected-error {{call to implicitly-deleted copy constructor}}
+}
+
+MoveOnly t4() {
+MoveOnly&& r = static_cast(moveonly);
+return r; // cxx11_14_17-error {{call to implicitly-deleted copy constructor}}
+}
+
+Target t5() {
+CopyOnly& r = copyonly;
+return r;
+}
+
+Target t6() {
+CopyOnly&& r = static_cast(copyonly);
+return r; // cxx20-error {{invokes a deleted function}}
+}
+
+Target t7() {
+MoveOnly& r = moveonly;
+return r; // expected-error {{no viable conversion}}
+}
+
+Target t8() {
+MoveOnly&& r = static_cast(moveonly);
+return r; // cxx11_14_17-error {{no viable conversion}}
+}
+
+} // namespace test_lvalue_ref_is_not_moved_from
+
+namespace test_rvalue_ref_to_nonobject {
+
+struct CopyOnly {};
+struct MoveOnly {};
+
+struct Target {
+Target(CopyOnly (&)());
+Target(CopyOnly (&&)()) = delete;
+Target(MoveOnly (&)()) = delete; // expected-note {{has been explicitly marked deleted here}}
+  // expected-note@-1 {{has been explicitly marked deleted here}}
+Target(MoveOnly (&&)());
+};
+
+CopyOnly make_copyonly();
+MoveOnly make_moveonly();
+
+Target t1() {
+CopyOnly (&r)() = make_copyonly;
+return r;
+}
+
+Target t2() {
+CopyOnly (&&r)() = static_cast(make_copyonly);
+return r; // OK in all modes; not subject to implicit move
+}
+
+Target t3() {
+MoveOnly (&r)() = make_moveonly;
+return r; // expected-error {{invokes a deleted function}}
+}
+
+Target t4() {
+MoveOnly (&&r)() = static_cast(make_moveonly);
+return r; // expected-error {{invokes a deleted function}}
+}
+
+} // namespace test_rvalue_ref_to_nonobject
Index: clang/l

[PATCH] D88220: [C++20] P1825R0: More implicit moves

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

To whom it may concern, see D98971 . I'm 
(unwisely?) trying a surgical fix rather than trying to revert the month-old 
thing. (Let's please discuss anything D98971 
-specific over there.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88220

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

[PATCH] D98738: [clang-tidy] performance-* checks: Match AllowedTypes against qualified type names when they contain "::".

[Felix Berger via Phabricator via cfe-commits]

flx marked 6 inline comments as done.
flx added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/Matchers.h:52
 
 AST_MATCHER_P(NamedDecl, matchesAnyListedName, std::vector,
   NameList) {

hokein wrote:
> worth some comments on this.
Done in its new location.



Comment at: clang-tools-extra/clang-tidy/utils/Matchers.h:55
   return llvm::any_of(NameList, [&Node](const std::string &Name) {
-  return llvm::Regex(Name).match(Node.getName());
-});
+if (Name.find("::") != std::string::npos) {
+  return llvm::Regex(Name).match(Node.getQualifiedNameAsString());

hokein wrote:
> hokein wrote:
> > If we pass the name as a `llvm::StringRef`, we can use `if 
> > (Name.contains("::"))` which seems a bit clearer to me.
> there is a tricky case where the `::` is a prefix of the Name, e.g.  a global 
> symbol x, the Name is `::x`, and `getQualifiedNameAsString` of symbol x is 
> `x` (without the `::` prefix), then the matcher will not find a match, but we 
> expect this is a match right?
> 
I agree that's a tricky case.

We could prepend "::" to getQualifiedNameAsString() and always match against 
it. Would this work?

Or we could do this only for regex names that have a leading "::".  This would 
avoid extra string operations, and users could still match using "^foo::Bar$" 
to match ::foo::Bar.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98738

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

[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

[Nikita Kniazev via Phabricator via cfe-commits]

nick updated this revision to Diff 331945.
nick retitled this revision from "[clang][AST] Add `CXXBaseSpecifier` matcher 
support" to "[ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)".
nick edited the summary of this revision.
nick added a reviewer: aaron.ballman.
nick added a comment.

Rebased. Removed things reimplemented and landed in D81552 
 + D79063 .


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

https://reviews.llvm.org/D69218

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
  clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp

Index: clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
===
--- clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
+++ clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
@@ -297,6 +297,17 @@
   EXPECT_TRUE(matches("int b[7];", M));
 }
 
+TEST_F(RegistryTest, CXXBaseSpecifier) {
+  // TODO: rewrite with top-level cxxBaseSpecifier matcher when available
+  DeclarationMatcher ClassHasAnyDirectBase =
+  constructMatcher("cxxRecordDecl",
+   constructMatcher("hasDirectBase",
+constructMatcher("cxxBaseSpecifier")))
+  .getTypedMatcher();
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasAnyDirectBase));
+  EXPECT_TRUE(notMatches("class X {};", ClassHasAnyDirectBase));
+}
+
 TEST_F(RegistryTest, CXXCtorInitializer) {
   Matcher CtorDecl = constructMatcher(
   "cxxConstructorDecl",
Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -320,6 +320,15 @@
 varDecl(hasType(pointsTo(ClassX);
 }
 
+TEST(HasType, TakesQualTypeMatcherAndMatchesCXXBaseSpecifier) {
+  TypeMatcher ClassX = hasDeclaration(recordDecl(hasName("X")));
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX =
+  cxxRecordDecl(hasDirectBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class Z {}; class Y : Z {};", ClassHasBaseClassX));
+}
+
 TEST(HasType, TakesDeclMatcherAndMatchesExpr) {
   DeclarationMatcher ClassX = recordDecl(hasName("X"));
   EXPECT_TRUE(
@@ -337,6 +346,15 @@
 notMatches("class X {}; void y() { X *x; }", varDecl(hasType(ClassX;
 }
 
+TEST(HasType, TakesDeclMatcherAndMatchesCXXBaseSpecifier) {
+  DeclarationMatcher ClassX = recordDecl(hasName("X"));
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX =
+  cxxRecordDecl(hasDirectBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class Z {}; class Y : Z {};", ClassHasBaseClassX));
+}
+
 TEST(HasType, MatchesTypedefDecl) {
   EXPECT_TRUE(matches("typedef int X;", typedefDecl(hasType(asString("int");
   EXPECT_TRUE(matches("typedef const int T;",
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1278,6 +1278,35 @@
   ZIsDirectlyDerivedFromX));
 }
 
+TEST_P(ASTMatchersTest, ClassHasBase) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  DeclarationMatcher ClassHasAnyDirectBase =
+  cxxRecordDecl(hasDirectBase(cxxBaseSpecifier()));
+  EXPECT_TRUE(notMatches("class X {};", ClassHasAnyDirectBase));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasAnyDirectBase));
+  EXPECT_TRUE(matches("class X {}; class Y : public virtual X {};",
+  ClassHasAnyDirectBase));
+
+  TypeMatcher ClassX = asString("class X");
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX =
+  cxxRecordDecl(hasDirectBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y {}; class Z : X, Y {};",
+  ClassHasBaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y {}; class Z : Y, X {};",
+  ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class W {}; class Y {}; class Z : W, Y {};",
+ ClassHasBaseClassX));
+  DeclarationMatcher ClassZHasBaseClassX =
+  cxxRecordDecl(hasName("Z"), hasDirectBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {}; class Z : X {};",
+  

[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

[Nikita Kniazev via Phabricator via cfe-commits]

nick updated this revision to Diff 331955.
nick added a comment.

Forgot to remove a duplicated test


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

https://reviews.llvm.org/D69218

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
  clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp

Index: clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
===
--- clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
+++ clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
@@ -297,6 +297,17 @@
   EXPECT_TRUE(matches("int b[7];", M));
 }
 
+TEST_F(RegistryTest, CXXBaseSpecifier) {
+  // TODO: rewrite with top-level cxxBaseSpecifier matcher when available
+  DeclarationMatcher ClassHasAnyDirectBase =
+  constructMatcher("cxxRecordDecl",
+   constructMatcher("hasDirectBase",
+constructMatcher("cxxBaseSpecifier")))
+  .getTypedMatcher();
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasAnyDirectBase));
+  EXPECT_TRUE(notMatches("class X {};", ClassHasAnyDirectBase));
+}
+
 TEST_F(RegistryTest, CXXCtorInitializer) {
   Matcher CtorDecl = constructMatcher(
   "cxxConstructorDecl",
Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -320,6 +320,15 @@
 varDecl(hasType(pointsTo(ClassX);
 }
 
+TEST(HasType, TakesQualTypeMatcherAndMatchesCXXBaseSpecifier) {
+  TypeMatcher ClassX = hasDeclaration(recordDecl(hasName("X")));
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX =
+  cxxRecordDecl(hasDirectBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class Z {}; class Y : Z {};", ClassHasBaseClassX));
+}
+
 TEST(HasType, TakesDeclMatcherAndMatchesExpr) {
   DeclarationMatcher ClassX = recordDecl(hasName("X"));
   EXPECT_TRUE(
@@ -337,6 +346,15 @@
 notMatches("class X {}; void y() { X *x; }", varDecl(hasType(ClassX;
 }
 
+TEST(HasType, TakesDeclMatcherAndMatchesCXXBaseSpecifier) {
+  DeclarationMatcher ClassX = recordDecl(hasName("X"));
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX =
+  cxxRecordDecl(hasDirectBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class Z {}; class Y : Z {};", ClassHasBaseClassX));
+}
+
 TEST(HasType, MatchesTypedefDecl) {
   EXPECT_TRUE(matches("typedef int X;", typedefDecl(hasType(asString("int");
   EXPECT_TRUE(matches("typedef const int T;",
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -4400,6 +4400,13 @@
 return;
   }
 
+  DeclarationMatcher ClassHasAnyDirectBase =
+  cxxRecordDecl(hasDirectBase(cxxBaseSpecifier()));
+  EXPECT_TRUE(notMatches("class X {};", ClassHasAnyDirectBase));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasAnyDirectBase));
+  EXPECT_TRUE(matches("class X {}; class Y : public virtual X {};",
+  ClassHasAnyDirectBase));
+
   EXPECT_TRUE(matches(
   R"cc(
 class Base {};
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -179,6 +179,7 @@
   REGISTER_MATCHER(cxxConstructExpr);
   REGISTER_MATCHER(cxxConstructorDecl);
   REGISTER_MATCHER(cxxConversionDecl);
+  REGISTER_MATCHER(cxxBaseSpecifier);
   REGISTER_MATCHER(cxxCtorInitializer);
   REGISTER_MATCHER(cxxDeductionGuideDecl);
   REGISTER_MATCHER(cxxDefaultArgExpr);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -756,6 +756,7 @@
 const internal::VariadicDynCastAllOfMatcher parmVarDecl;
 const internal::VariadicDynCastAllOfMatcher
 accessSpecDecl;
+const internal::VariadicAllOfMatcher cxxBaseSpecifier;
 const internal::VariadicAllOfMatcher cxxCtorInitializer;
 const internal::VariadicAllOfMatcher templateArgument;
 const internal::VariadicAllOfMatcher templateArgumentLoc;
Index: clang/include/clang/ASTM

[PATCH] D98935: [WoA][MSVC] Use default linker setting in MSVC-compatible driver [take 2]

[Maxim Kuvyrkov via Phabricator via cfe-commits]

maxim-kuvyrkov added a subscriber: tstellar.
maxim-kuvyrkov added a comment.

@tstellar

Hi Tom,

This has passed buildbots, so I'll cherry-pick this to release/12.x to fix 
https://bugs.llvm.org/show_bug.cgi?id=49624


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98935

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

[PATCH] D98816: PR49585: Emit the jump destination for a for loop 'continue' from within the scope of the condition variable.

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht added a comment.

The error message I posted earlier was when using O1 
 + new PM, but it crashes with the 
old one & no optimizations:

  $ cat /tmp/repro.cc
  void a() {
for (; int b = 0;) continue;
  }
  $ clang -c /tmp/repro.cc -o /tmp/repro.o
  Referring to a basic block in another function!
br label %for.inc
  in function _Z1av
  fatal error: error in backend: Broken function found, compilation aborted!
  PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash 
backtrace, preprocessed source, and associated run script.
  Stack dump:
  0.  Program arguments: clang -c /tmp/repro.cc -o /tmp/repro.o
  1.   parser at end of file
  2.  Per-function optimization
  3.  Running pass 'Module Verifier' on function '@_Z1av'
  ...
  $ clang -fno-legacy-pass-manager -O1 -c /tmp/repro.cc -o /tmp/repro.o
  clang: 
/home/rupprecht/src/llvm-project/llvm/include/llvm/Support/Casting.h:104: 
static bool llvm::isa_impl_cl::doit(const From *) [To = llvm::InvokeInst, From = const llvm::Instruction 
*]: Assertion `Val && "isa<> used on a null pointer"' failed.
  PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash 
backtrace, preprocessed source, and associated run script.
  Stack dump:
  0.  Program arguments: bin/clang -fno-legacy-pass-manager -O1 -c 
/tmp/repro.cc -o /tmp/repro.o
  1.   parser at end of file
  2.  Optimizer
  ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98816

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

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:3095
 
-  // ...non-volatile...
-  if (VD->getType().isVolatileQualified())
-return false;
-
-  // C++20 [class.copy.elision]p3:
-  // ...rvalue reference to a non-volatile...
-  if (VD->getType()->isRValueReferenceType() &&
-  (!(CESK & CES_AllowRValueReferenceType) ||
-   VD->getType().getNonReferenceType().isVolatileQualified()))
+  if (VD->getType()->isObjectType()) {
+// C++17 [class.copy.elision]p3:

A drive by fix here would be that we already have a VDType in this context, 
might as well use it even though original for some reason missed it in this 
part.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98971

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

[PATCH] D98816: PR49585: Emit the jump destination for a for loop 'continue' from within the scope of the condition variable.

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht added a comment.

I tried creating an IR repro, but: running `-S -emit-llvm` with the old PM 
crashes before it can generate anything, and running with the new PM seems to 
generate invalid IR (branches to %for.inc, which does not exist). I suspect 
this is not an optimizer bug, but crashing in the optimizer because invalid IR 
has been fed to it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98816

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

[PATCH] D94355: [Passes] Add relative lookup table converter pass

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:390-391
+
+if (!TM.getTargetTriple().isArch64Bit())
+  return false;
+

gulfem wrote:
> lebedev.ri wrote:
> > 1. But all tests are using `x86_64` triple?
> > 2. This is somewhat backwards. if the target wants to disable this, it will 
> > need to override this function with `return false;`.
> 1. Although I used `x86_64 triple`, this optimization can be applied to other 
> 64-bit architectures too, because it not target dependent except 
> `isArch64Bit` and `getCodeModel` check.
> 2. Is there a target that you have in mind that we need to disable this 
> optimization? 
> I thought that it makes sense to enable this optimization by default on all 
> the targets that can support it.
> In case targets want to disable it, they can override it as you said.
> How can we improve the implementation?
> If you have suggestions, I'm happy to incorporate that.
> 
I'm sorry, i do not understand.
Why does `!TM.getTargetTriple().isArch64Bit()` check exist?
To me it reads as "if we aren't compiling for AArch64, don't build rel lookup 
tables".
Am i misreading this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94355

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

[PATCH] D97119: [flang][driver] Add options for -std=f2018

[Arnamoy B via Phabricator via cfe-commits]

arnamoy10 added inline comments.



Comment at: flang/test/Driver/std2018.f90:1
+! REQUIRES: new-flang-driver
+! Ensure argument -std=f2018 works as expected.

awarzynski wrote:
> Would it be possible to share this test with `f18`?
Would you please elaborate on how do want the sharing to happen (same test 
file/different test file)?  I think in a previous comment you mentioned that it 
is sufficient to test the frontend driver for this patch?



Comment at: flang/test/Driver/std2018_wrong.f90:14-22
+subroutine foo2()
+do 01 m=1,2
+  select case (m)
+  case default
+print*, "default", m
+  case (1)
+print*, "start"

awarzynski wrote:
> No input is needed for this test, is it? IIUC, this bit can be deleted to 
> make the test leaner.
Sure, will do.


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

https://reviews.llvm.org/D97119

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

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 331973.
Quuxplusone added a comment.

Per @mizvekov, use `VDType` instead of `VD->getType()` wherever possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98971

Files:
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CXX/class/class.init/class.copy.elision/p3.cpp

Index: clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
===
--- clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
+++ clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
@@ -292,3 +292,108 @@
   return b; // cxx20-error {{calling a private constructor of class 'test_ctor_param_rvalue_ref::B2'}}
 }
 } // namespace test_ctor_param_rvalue_ref
+
+namespace test_lvalue_ref_is_not_moved_from {
+
+struct Target {};
+  // expected-note@-1 {{candidate constructor (the implicit copy constructor) not viable}}
+  // expected-note@-2 {{candidate constructor (the implicit move constructor) not viable}}
+  // cxx11_14_17-note@-3 {{candidate constructor (the implicit copy constructor) not viable}}
+  // cxx11_14_17-note@-4 {{candidate constructor (the implicit move constructor) not viable}}
+
+struct CopyOnly {
+  CopyOnly(CopyOnly&&) = delete; // cxx20-note {{has been explicitly marked deleted here}}
+  CopyOnly(CopyOnly&);
+  operator Target() && = delete; // cxx20-note {{has been explicitly marked deleted here}}
+  operator Target() &;
+};
+
+struct MoveOnly {
+  MoveOnly(MoveOnly&&); // expected-note {{copy constructor is implicitly deleted because}}
+// cxx11_14_17-note@-1 {{copy constructor is implicitly deleted because}}
+  operator Target() &&; // expected-note {{candidate function not viable}}
+// cxx11_14_17-note@-1 {{candidate function not viable}}
+};
+
+extern CopyOnly copyonly;
+extern MoveOnly moveonly;
+
+CopyOnly t1() {
+CopyOnly& r = copyonly;
+return r;
+}
+
+CopyOnly t2() {
+CopyOnly&& r = static_cast(copyonly);
+return r; // cxx20-error {{call to deleted constructor}}
+}
+
+MoveOnly t3() {
+MoveOnly& r = moveonly;
+return r; // expected-error {{call to implicitly-deleted copy constructor}}
+}
+
+MoveOnly t4() {
+MoveOnly&& r = static_cast(moveonly);
+return r; // cxx11_14_17-error {{call to implicitly-deleted copy constructor}}
+}
+
+Target t5() {
+CopyOnly& r = copyonly;
+return r;
+}
+
+Target t6() {
+CopyOnly&& r = static_cast(copyonly);
+return r; // cxx20-error {{invokes a deleted function}}
+}
+
+Target t7() {
+MoveOnly& r = moveonly;
+return r; // expected-error {{no viable conversion}}
+}
+
+Target t8() {
+MoveOnly&& r = static_cast(moveonly);
+return r; // cxx11_14_17-error {{no viable conversion}}
+}
+
+} // namespace test_lvalue_ref_is_not_moved_from
+
+namespace test_rvalue_ref_to_nonobject {
+
+struct CopyOnly {};
+struct MoveOnly {};
+
+struct Target {
+Target(CopyOnly (&)());
+Target(CopyOnly (&&)()) = delete;
+Target(MoveOnly (&)()) = delete; // expected-note {{has been explicitly marked deleted here}}
+  // expected-note@-1 {{has been explicitly marked deleted here}}
+Target(MoveOnly (&&)());
+};
+
+CopyOnly make_copyonly();
+MoveOnly make_moveonly();
+
+Target t1() {
+CopyOnly (&r)() = make_copyonly;
+return r;
+}
+
+Target t2() {
+CopyOnly (&&r)() = static_cast(make_copyonly);
+return r; // OK in all modes; not subject to implicit move
+}
+
+Target t3() {
+MoveOnly (&r)() = make_moveonly;
+return r; // expected-error {{invokes a deleted function}}
+}
+
+Target t4() {
+MoveOnly (&&r)() = static_cast(make_moveonly);
+return r; // expected-error {{invokes a deleted function}}
+}
+
+} // namespace test_rvalue_ref_to_nonobject
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3092,24 +3092,31 @@
   if (VD->hasAttr())
 return false;
 
-  // ...non-volatile...
-  if (VD->getType().isVolatileQualified())
-return false;
-
-  // C++20 [class.copy.elision]p3:
-  // ...rvalue reference to a non-volatile...
-  if (VD->getType()->isRValueReferenceType() &&
-  (!(CESK & CES_AllowRValueReferenceType) ||
-   VD->getType().getNonReferenceType().isVolatileQualified()))
+  if (VDType->isObjectType()) {
+// C++17 [class.copy.elision]p3:
+// ...non-volatile automatic object...
+if (VDType.isVolatileQualified())
+  return false;
+  } else if (VDType->isRValueReferenceType()) {
+// C++20 [class.copy.elision]p3:
+// ...either a non-volatile object or an rvalue reference to a non-volatile object type...
+if (!(CESK & CES_AllowRValueReferenceType))
+  return false;
+if (!VDType.getNonReferenceType()->isObjectType())
+  return false;
+if (VDType.getNonReferenceType().isVolatileQualified())
+  return false;
+  } else {
 return false;

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:3095
 
-  // ...non-volatile...
-  if (VD->getType().isVolatileQualified())
-return false;
-
-  // C++20 [class.copy.elision]p3:
-  // ...rvalue reference to a non-volatile...
-  if (VD->getType()->isRValueReferenceType() &&
-  (!(CESK & CES_AllowRValueReferenceType) ||
-   VD->getType().getNonReferenceType().isVolatileQualified()))
+  if (VD->getType()->isObjectType()) {
+// C++17 [class.copy.elision]p3:

mizvekov wrote:
> A drive by fix here would be that we already have a VDType in this context, 
> might as well use it even though original for some reason missed it in this 
> part.
This whole block is also logically equivalent to the much simpler:
```
if (VDType.isReferenceType()) {
if (!(CESK & CES_AllowRValueReferenceType) || 
!VDType.isRValueReferenceType())
  return false;
VDType = VDType.getNonReferenceType()
}
if (!VDType.isObjectType() || VDType.isVolatileQualified()) 
  return false;
```
But you do have to adjust the comments there and adjust the rest to use VDType 
consistently :)
Also, I think it might be possible to even remove the `!VDType.isObjectType() 
|| ` part from my suggestion above, because it might be the only option left if 
it is not a reference anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98971

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

[PATCH] D97119: [flang][driver] Add options for -std=f2018

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.



Comment at: flang/test/Driver/std2018.f90:1
+! REQUIRES: new-flang-driver
+! Ensure argument -std=f2018 works as expected.

arnamoy10 wrote:
> awarzynski wrote:
> > Would it be possible to share this test with `f18`?
> Would you please elaborate on how do want the sharing to happen (same test 
> file/different test file)?  I think in a previous comment you mentioned that 
> it is sufficient to test the frontend driver for this patch?
This line means that the test is not run when `FLANG_BUILD_NEW_DRIVER` is `Off`:
```
! REQUIRES: new-flang-driver
```
Once this line is removed, the test is effectively shared.

Currently this test won't work with `f18`(i.e. when you remove this line), but 
is should be sufficient to add `-pedantic` and `-std=f2018` as aliases for [[ 
https://github.com/llvm/llvm-project/blob/987ee6e3cc1fb672b3ed201e72a5281c2ec88c99/flang/tools/f18/f18.cpp#L489-L491
 | Mstandard ]].


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

https://reviews.llvm.org/D97119

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

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:3095
 
-  // ...non-volatile...
-  if (VD->getType().isVolatileQualified())
-return false;
-
-  // C++20 [class.copy.elision]p3:
-  // ...rvalue reference to a non-volatile...
-  if (VD->getType()->isRValueReferenceType() &&
-  (!(CESK & CES_AllowRValueReferenceType) ||
-   VD->getType().getNonReferenceType().isVolatileQualified()))
+  if (VD->getType()->isObjectType()) {
+// C++17 [class.copy.elision]p3:

mizvekov wrote:
> mizvekov wrote:
> > A drive by fix here would be that we already have a VDType in this context, 
> > might as well use it even though original for some reason missed it in this 
> > part.
> This whole block is also logically equivalent to the much simpler:
> ```
> if (VDType.isReferenceType()) {
> if (!(CESK & CES_AllowRValueReferenceType) || 
> !VDType.isRValueReferenceType())
>   return false;
> VDType = VDType.getNonReferenceType()
> }
> if (!VDType.isObjectType() || VDType.isVolatileQualified()) 
>   return false;
> ```
> But you do have to adjust the comments there and adjust the rest to use 
> VDType consistently :)
> Also, I think it might be possible to even remove the `!VDType.isObjectType() 
> || ` part from my suggestion above, because it might be the only option left 
> if it is not a reference anyway.
```
  bool isObjectType() const {
// C++ [basic.types]p8:
//   An object type is a (possibly cv-qualified) type that is not a
//   function type, not a reference type, and not a void type.
return !isReferenceType() && !isFunctionType() && !isVoidType();
  }
```
So yeah I think you can just make my suggestion be:
```
if (VDType.isReferenceType()) {
if (!(CESK & CES_AllowRValueReferenceType) || 
!VDType.isRValueReferenceType())
  return false;
VDType = VDType.getNonReferenceType()
}
if (VDType.isVolatileQualified()) 
  return false;
```




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98971

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