date:20220728

[PATCH] D130614: [C++20] [Modules] Merge same concept decls in global module fragment

2022-07-28 Thread Chuanqi Xu via Phabricator via cfe-commits

ChuanqiXu updated this revision to Diff 448246.
ChuanqiXu added a comment.

- Address comment.
- Add Sema::CheckRedefinitionInModule


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

https://reviews.llvm.org/D130614

Files:
  clang/include/clang/Basic/Module.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/Modules/merge-concepts.cppm

Index: clang/test/Modules/merge-concepts.cppm
===
--- /dev/null
+++ clang/test/Modules/merge-concepts.cppm
@@ -0,0 +1,185 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -I%t %t/A.cppm -o %t/A.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -I%t %t/B.cppm -o %t/B.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/Use.cpp -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/Use2.cpp -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/Use3.cpp -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/Use4.cpp -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/C.cppm -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -I%t %t/D.cppm -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -I%t %t/E.cppm -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -I%t %t/F.cppm -verify -fsyntax-only
+//
+// Testing header units for coverity.
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/foo.h -o %t/foo.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t -fmodule-file=%t/foo.pcm %t/Use5.cpp -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t -fmodule-file=%t/foo.pcm %t/Use6.cpp -verify -fsyntax-only
+//
+// Testing with module map modules. It is unclear about the relation ship between Clang modules and
+// C++20 Named Modules. Will them co_exist? Or will them be mutual from each other?
+// The test here is for primarily coverity.
+//
+// RUN: rm -f %t/foo.pcm
+// RUN: %clang_cc1 -std=c++20 -fmodules -fmodules-cache-path=%t -fprebuilt-module-path=%t \
+// RUN:   -fmodule-map-file=%t/module.map %t/Use7.cpp -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -fmodules -fmodules-cache-path=%t -fprebuilt-module-path=%t \
+// RUN:   -fmodule-map-file=%t/module.map %t/Use7.cpp -verify -fsyntax-only
+// Testing module map modules with named modules.
+// RUN: %clang_cc1 -std=c++20 -fmodules -fmodules-cache-path=%t -fmodule-map-file=%t/module.map \
+// RUN:   %t/A.cppm -o %t/A.pcm
+// RUN: %clang_cc1 -std=c++20 -fmodules -fmodules-cache-path=%t -fprebuilt-module-path=%t \
+// RUN:   -fmodule-map-file=%t/module.map %t/Use7.cpp -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -fmodules -fmodules-cache-path=%t -fprebuilt-module-path=%t \
+// RUN:   -fmodule-map-file=%t/module.map %t/Use7.cpp -verify -fsyntax-only
+
+//
+//--- foo.h
+#ifndef FOO_H
+#define FOO_H
+template 
+concept same_as = __is_same(T, U);
+#endif
+
+// The compiler would warn if we include foo_h twice without guard.
+//--- redecl.h
+#ifndef REDECL_H
+#define REDECL_H
+template 
+concept same_as = __is_same(T, U);
+#endif
+
+//--- A.cppm
+module;
+#include "foo.h"
+export module A;
+export using ::same_as;
+
+//--- B.cppm
+module;
+#include "foo.h"
+export module B;
+export using ::same_as;
+
+//--- Use.cpp
+// expected-no-diagnostics
+import A;
+import B;
+
+template  void foo()
+  requires same_as
+{}
+
+//--- Use2.cpp
+// expected-no-diagnostics
+#include "foo.h"
+import A;
+
+template  void foo()
+  requires same_as
+{}
+
+//--- Use3.cpp
+// expected-no-diagnostics
+import A;
+#include "foo.h"
+
+template  void foo()
+  requires same_as
+{}
+
+//--- Use4.cpp
+// expected-no-diagnostics
+import A;
+import B;
+#include "foo.h"
+
+template  void foo()
+  requires same_as
+{}
+
+//--- C.cppm
+// expected-no-diagnostics
+module;
+#include "foo.h"
+export module C;
+import A;
+import B;
+
+template  void foo()
+  requires same_as
+{}
+
+//--- D.cppm
+module;
+#include "foo.h"
+#include "redecl.h"
+export module D;
+export using ::same_as;
+
+// expected-error@* {{redefinition of 'same_as'}}
+// expected-note@* 1+{{previous definition is here}}
+
+//--- E.cppm
+module;
+#include "foo.h"
+export module E;
+export template 
+concept same_as = __is_same(T, U);
+
+// expected-error@* {{redefinition of 'same_as'}}
+// expected-note@* 1+{{previous definition is here}}
+
+//--- F.cppm
+export module F;
+template 
+concept same_as = __is_same(T, U);
+template 
+concept same_as = __is_same(T, U);
+
+// expected-error@* {{redefinition of 'same_as'}}
+// expected-note@* 1+{{previous definition is here}}
+
+//--- Use5.cpp
+// expected-no-diagnostics
+import "foo.h";
+import A;
+
+template  void foo()
+  requires same_as
+{}
+
+//--- Use6.cpp
+// expected-no-diagnostics
+import A;
+import "foo.h";
+
+template  void foo()
+  requires s

[PATCH] D130545: [cmake] Slight fix ups to make robust to the full range of GNUInstallDirs

2022-07-28 Thread Sylvestre Ledru via Phabricator via cfe-commits

sylvestre.ledru added a comment.

Same, it broke apt.llvm.org

It is installing some of the libs in debian/tmp/libgomp.so instead of 
debian/tmp/usr/lib/llvm-15/lib/libgomp.so

I think it should be indeed revert (and in the branch 15 too)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130545

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

[clang] 1dc26b8 - [Driver][PowerPC] Support -mtune=

2022-07-28 Thread Fangrui Song via cfe-commits


Author: Fangrui Song
Date: 2022-07-28T00:34:04-07:00
New Revision: 1dc26b80b872a94c581549a21943756a8c3448a3

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

LOG: [Driver][PowerPC] Support -mtune=

Reviewed By: #powerpc, nemanjai

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

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/Arch/PPC.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/ppc-cpus.c

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 3cab37b21aaf3..c723f6164530b 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3986,7 +3986,7 @@ def module_file_info : Flag<["-"], "module-file-info">, 
Flags<[NoXarchOption,CC1
   HelpText<"Provide information about a particular module file">;
 def mthumb : Flag<["-"], "mthumb">, Group;
 def mtune_EQ : Joined<["-"], "mtune=">, Group,
-  HelpText<"Only supported on X86, RISC-V and SystemZ. Otherwise accepted for 
compatibility with GCC.">;
+  HelpText<"Only supported on AArch64, PowerPC, RISC-V, SystemZ, and X86">;
 def multi__module : Flag<["-"], "multi_module">;
 def multiply__defined__unused : Separate<["-"], "multiply_defined_unused">;
 def multiply__defined : Separate<["-"], "multiply_defined">;

diff  --git a/clang/lib/Driver/ToolChains/Arch/PPC.cpp 
b/clang/lib/Driver/ToolChains/Arch/PPC.cpp
index 7817ec595ceb3..bcaecf4b2d980 100644
--- a/clang/lib/Driver/ToolChains/Arch/PPC.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/PPC.cpp
@@ -107,10 +107,6 @@ const char *ppc::getPPCAsmModeForCPU(StringRef Name) {
 void ppc::getPPCTargetFeatures(const Driver &D, const llvm::Triple &Triple,
const ArgList &Args,
std::vector &Features) {
-  // TODO Handle -mtune=. Suppress -Wunused-command-line-argument as a
-  // longstanding behavior.
-  (void)Args.getLastArg(options::OPT_mtune_EQ);
-
   if (Triple.getSubArch() == llvm::Triple::PPCSubArch_spe)
 Features.push_back("+spe");
 

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index eb71a561a1cd4..e94a4e8145107 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2083,6 +2083,14 @@ void Clang::AddMIPSTargetArgs(const ArgList &Args,
 
 void Clang::AddPPCTargetArgs(const ArgList &Args,
  ArgStringList &CmdArgs) const {
+  if (const Arg *A = Args.getLastArg(options::OPT_mtune_EQ)) {
+CmdArgs.push_back("-tune-cpu");
+if (strcmp(A->getValue(), "native") == 0)
+  CmdArgs.push_back(Args.MakeArgString(llvm::sys::getHostCPUName()));
+else
+  CmdArgs.push_back(A->getValue());
+  }
+
   // Select the ABI to use.
   const char *ABIName = nullptr;
   const llvm::Triple &T = getToolChain().getTriple();

diff  --git a/clang/test/Driver/ppc-cpus.c b/clang/test/Driver/ppc-cpus.c
index 9f23d86e41efa..a38bf87ab6098 100644
--- a/clang/test/Driver/ppc-cpus.c
+++ b/clang/test/Driver/ppc-cpus.c
@@ -19,3 +19,6 @@
 // RUN: %clang -### -c -target powerpc64 %s -mcpu=pwr8 2>&1 | FileCheck %s 
--check-prefix=NO_PPC64
 
 // NO_PPC64-NOT: "-target-cpu" "ppc64"
+
+// RUN: %clang -### -c --target=powerpc64 %s -mcpu=generic -mtune=pwr9 2>&1 | 
FileCheck %s --check-prefix=TUNE
+// TUNE: "-target-cpu" "ppc64" "-tune-cpu" "pwr9"



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

[PATCH] D130526: [Driver][PowerPC] Support -mtune=

2022-07-28 Thread Fangrui Song 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 rG1dc26b80b872: [Driver][PowerPC] Support -mtune= (authored by 
MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130526

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Arch/PPC.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/ppc-cpus.c


Index: clang/test/Driver/ppc-cpus.c
===
--- clang/test/Driver/ppc-cpus.c
+++ clang/test/Driver/ppc-cpus.c
@@ -19,3 +19,6 @@
 // RUN: %clang -### -c -target powerpc64 %s -mcpu=pwr8 2>&1 | FileCheck %s 
--check-prefix=NO_PPC64
 
 // NO_PPC64-NOT: "-target-cpu" "ppc64"
+
+// RUN: %clang -### -c --target=powerpc64 %s -mcpu=generic -mtune=pwr9 2>&1 | 
FileCheck %s --check-prefix=TUNE
+// TUNE: "-target-cpu" "ppc64" "-tune-cpu" "pwr9"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2083,6 +2083,14 @@
 
 void Clang::AddPPCTargetArgs(const ArgList &Args,
  ArgStringList &CmdArgs) const {
+  if (const Arg *A = Args.getLastArg(options::OPT_mtune_EQ)) {
+CmdArgs.push_back("-tune-cpu");
+if (strcmp(A->getValue(), "native") == 0)
+  CmdArgs.push_back(Args.MakeArgString(llvm::sys::getHostCPUName()));
+else
+  CmdArgs.push_back(A->getValue());
+  }
+
   // Select the ABI to use.
   const char *ABIName = nullptr;
   const llvm::Triple &T = getToolChain().getTriple();
Index: clang/lib/Driver/ToolChains/Arch/PPC.cpp
===
--- clang/lib/Driver/ToolChains/Arch/PPC.cpp
+++ clang/lib/Driver/ToolChains/Arch/PPC.cpp
@@ -107,10 +107,6 @@
 void ppc::getPPCTargetFeatures(const Driver &D, const llvm::Triple &Triple,
const ArgList &Args,
std::vector &Features) {
-  // TODO Handle -mtune=. Suppress -Wunused-command-line-argument as a
-  // longstanding behavior.
-  (void)Args.getLastArg(options::OPT_mtune_EQ);
-
   if (Triple.getSubArch() == llvm::Triple::PPCSubArch_spe)
 Features.push_back("+spe");
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3986,7 +3986,7 @@
   HelpText<"Provide information about a particular module file">;
 def mthumb : Flag<["-"], "mthumb">, Group;
 def mtune_EQ : Joined<["-"], "mtune=">, Group,
-  HelpText<"Only supported on X86, RISC-V and SystemZ. Otherwise accepted for 
compatibility with GCC.">;
+  HelpText<"Only supported on AArch64, PowerPC, RISC-V, SystemZ, and X86">;
 def multi__module : Flag<["-"], "multi_module">;
 def multiply__defined__unused : Separate<["-"], "multiply_defined_unused">;
 def multiply__defined : Separate<["-"], "multiply_defined">;


Index: clang/test/Driver/ppc-cpus.c
===
--- clang/test/Driver/ppc-cpus.c
+++ clang/test/Driver/ppc-cpus.c
@@ -19,3 +19,6 @@
 // RUN: %clang -### -c -target powerpc64 %s -mcpu=pwr8 2>&1 | FileCheck %s --check-prefix=NO_PPC64
 
 // NO_PPC64-NOT: "-target-cpu" "ppc64"
+
+// RUN: %clang -### -c --target=powerpc64 %s -mcpu=generic -mtune=pwr9 2>&1 | FileCheck %s --check-prefix=TUNE
+// TUNE: "-target-cpu" "ppc64" "-tune-cpu" "pwr9"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2083,6 +2083,14 @@
 
 void Clang::AddPPCTargetArgs(const ArgList &Args,
  ArgStringList &CmdArgs) const {
+  if (const Arg *A = Args.getLastArg(options::OPT_mtune_EQ)) {
+CmdArgs.push_back("-tune-cpu");
+if (strcmp(A->getValue(), "native") == 0)
+  CmdArgs.push_back(Args.MakeArgString(llvm::sys::getHostCPUName()));
+else
+  CmdArgs.push_back(A->getValue());
+  }
+
   // Select the ABI to use.
   const char *ABIName = nullptr;
   const llvm::Triple &T = getToolChain().getTriple();
Index: clang/lib/Driver/ToolChains/Arch/PPC.cpp
===
--- clang/lib/Driver/ToolChains/Arch/PPC.cpp
+++ clang/lib/Driver/ToolChains/Arch/PPC.cpp
@@ -107,10 +107,6 @@
 void ppc::getPPCTargetFeatures(const Driver &D, const llvm::Triple &Triple,
const ArgList &Args,
std::vector &Features) {
-  // TODO Handle -mtune=. Suppress -Wunused-command-line-argument as a
-  // longstanding behavior.
-  (void)Args.getLastArg(options::OPT_mtune_EQ);
-
   if (Triple.getSubArch() == llvm

[PATCH] D130268: [NFC] Add SmallVector constructor to allow creation of SmallVector from ArrayRef of items convertible to type T

2022-07-28 Thread Nikita Popov via Phabricator via cfe-commits

nikic added a comment.

I think my only concern with this change is that it will also allow some 
implicit ArrayRef constructors. For example, this will permit creating a 
SmallVector from `std::array`, `std::vector`, or just `T` -- but only if 
`ArrayRef` is in scope. This seems somewhat dangerous.

I'm not sure if C++ provides any good way to avoid that, short of explicitly 
marking certain constructors as deleted?

I'm wondering if it might not be better to make this a fully generic overload 
that accepts any range (i.e., anything with begin() and end()). Paradoxically, 
this will likely be less permissive in practice than having explicit 
iterator_range and ArrayRef overloads, because it allows less implicit 
conversions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130268

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

[PATCH] D130614: [C++20] [Modules] Merge same concept decls in global module fragment

2022-07-28 Thread Chuanqi Xu via Phabricator via cfe-commits

ChuanqiXu marked 2 inline comments as done.
ChuanqiXu added a comment.

In D130614#3681881 , @ilya-biryukov 
wrote:

> Thanks! Mostly questions to better understand the spec and clang. Please 
> excuse me if they sound too basic.
> I have read the modules section of the standard, but I couldn't find where it 
> specifies whether these redefinition errors should be present or not. Any 
> guidance of where to look at?

This is primarily at: http://eel.is/c++draft/basic.def.odr#14:

> [basic.def.odr]p14:
> For any definable item D with definitions in multiple translation units,
>
> - if D is a non-inline non-templated function or variable, or
> - if the definitions in different translation units do not satisfy the
>
> following requirements,
>
>   the program is ill-formed; a diagnostic is required only if the definable
>   item is attached to a named module and a prior definition is reachable at
>   the point where a later definition occurs.
>
> - Each such definition shall not be attached to a named module
>
> ([module.unit]).
>
> - Each such definition shall consist of the same sequence of tokens, ...
>
> ... (following off is about the ODR checks)

So except the ODR checking, the redefinition errors should be present if:

- The redefinitions lives in one TU. or,
- Any of the redefinitions get attached to a named modules.

So we can get some examples:

---

  // foo.cpp
  template 
  concept same_as = __is_same(T, U);
  template 
  concept same_as = __is_same(T, U);

This is disallowed. Since there are definitions of `same_as` in the same TU for 
`foo.cpp`. Note that headers won't present a TU. So the following one is 
disallowed too:

  // foo.h
  template 
  concept same_as = __is_same(T, U);
  // foo.cpp
  #include "foo.h"
  #include "foo.h"

The definitions lives in the same TU of `foo.cpp` too. So this is problematic.

---

Note that a named module unit (*.cppm files)  presents a TU. So the following 
one is disallowed:

  C++
  // foo.h
  template 
  concept same_as = __is_same(T, U);
  // foo.cppm
  module;
  #include "foo.h"
  #include "foo.h"
  export module foo;

Since there are two definitions in the same TU of `foo.cppm`.

---

And the following one should be allowed:

  C++
  // foo.h
  template 
  concept same_as = __is_same(T, U);
  // foo.cppm
  module;
  #include "foo.h"
  export module foo;
  export using :: same_as;
  // use.cpp
  #include "foo.h"
  import foo;
  template  void foo()
requires same_as
  {}

This is valid since the two definitions lives in two TUs (use.cpp and foo.cppm) 
and neither of them lives in named modules (the `same_as` definition in 
`foo.cppm` lives in global module instead of a named module).

---

And this one is disallowed

  C++
  // foo.h
  template 
  concept same_as = __is_same(T, U);
  // foo.cppm
  export module foo;
  export template 
  concept same_as = __is_same(T, U);
  // use.cpp
  #include "foo.h"
  import foo;
  template  void foo()
requires same_as
  {}

Although the two definitions live in 2 TUs, but one of them is attached to the 
named module `foo`. So it is problematic.

---

Overall, I think the design intuition as follows:
the biggest different of C++20 Named Modules  between clang modules (or the 
standardized one, header units) is that the C++ Named Modules owns a unique TU, 
while the design of clang header modules (or header units) tries to mimic 
headers as much as they can. However, there are full of legacy codes using 
headers. The C++20 Named modules are hard to use in real projects if they can't 
be compatible with existing headers. So here is the global module fragment, 
which is designed for compatibility.
It might be easier to understand the rules now.

I implemented `Sema::CheckRedefinitionInModule` by the way to mimic the rules. 
Although there is a FIXME, I guess the FIXME itself is not too bad. At least it 
won't make things bad : )

> Could we add a few examples where legitimate redefinition errors do happen?

Yeah, added.

> (private module fragment?

In the above wording, there is a requirement `reachable`. But the declarations 
in the private module fragment is not reachable to the users of the module. So 
it won't be a legitimate redefinition error.

Comment at: clang/lib/Sema/SemaTemplate.cpp:8731

-  auto *OldConcept = dyn_cast(Previous.getRepresentativeDecl());
+  NamedDecl *Old = Previous.getRepresentativeDecl();
+  if (auto *USD = dyn_cast(Old))

ilya-biryukov wrote:
> This follows what `getFoundDecl()` does, but still allows to show errors when 
> lookup results contain multiple entities.
> 
> A question too: is true that concept is always reachable if it has a 
> reachable using decl?
> I wonder whether `hasReachableDefinition` should be called on 
> `UsingShadowDecl` instead?
> This follows what getFoundDecl() does, but still allows to show errors when 
> lookup results contain multiple entities.

Yeah, this is much better.

>

[PATCH] D130569: [Driver] Use libatomic for 32-bit SPARC atomics support on Linux

2022-07-28 Thread Rainer Orth via Phabricator via cfe-commits

ro marked 2 inline comments as done.
ro added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:634
 
+  // LLVM support for atomics on 32-bit SPARC V8+ is incomplete, so
+  // forcibly link with libatomic as a workaround.

MaskRay wrote:
> `// TODO ...` and attach a bug link
It's split between the original bug report and a patch review: I only noticed 
later (when `compiler-rt` started to make use of 64-bit atomics) that those 
aren't implemented by `clang -m32 -mcpu=v9`.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:637
+  if (getToolChain().getTriple().getArch() == llvm::Triple::sparc) {
+CmdArgs.push_back("--as-needed");
+CmdArgs.push_back("-latomic");

MaskRay wrote:
> Such --as-needed usage is a bit fragile if clang driver in the future passes 
> `--as-needed`  in the beginning. Better to use --push-state if you have a 
> not-too-old ld.
Good point: I initially fell into that trap in D130571 when I added 
`--as-needed -latomic --no-as-needed` to `SANITIZER_COMMON_LINK_FLAGS`: it got 
added early to the link line, before the objects using atomics, and had no 
effect.  Only then did Iearn that `target_link_libraries` accepts not only 
library names, but also linker flags.

As for `--push-state`, it was introduced in GNU `ld` 2.25 back in 2014, to it's 
save to assume it's present.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130569

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

[PATCH] D130600: [clang][dataflow] Handle return statements

2022-07-28 Thread Stanislav Gatev via Phabricator via cfe-commits

sgatev added inline comments.



Comment at: 
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:114
+  ///
+  ///  `return` must not be assigned a storage location.
+  void setReturnStorageLocation(StorageLocation &Loc) {

li.zhe.hua wrote:
> samestep wrote:
> > li.zhe.hua wrote:
> > > Fix this as well? A reader shouldn't need to root around in private 
> > > implementation details to understand the requirements for calling a 
> > > function.
> > Could you clarify what you mean? I was simply copying the signature and 
> > docstring from `setThisPointeeStorageLocation`.
> You've marked `return` in backticks. There is no parameter named `return` and 
> it is unclear what `return` refers to. My best guess is that this is a typo 
> of `ReturnLoc`, which is a private data member. So this is a public interface 
> with a requirement that a private data member has some property. This should 
> instead reframe the requirement as properties from the external reader's 
> perspective.
That was my guess initially too, but my next best guess is that `return` in 
backticks stands for the keyword/AST node. In any case, let's make it less 
ambiguous and let's not add requirements based on implementation details. How 
about: `The return value must not be assigned a storage location.`?



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:338-339
+if (Loc == nullptr) {
+  // The outermost context does not set a storage location for `return`, so
+  // in that case we just ignore `return` statements.
+  return;

samestep wrote:
> sgatev wrote:
> > Let's make this a FIXME to set a storage location for the outermost context 
> > too.
> @sgatev I could add a `FIXME` for that, or I could just do it in this same 
> patch; do you have a preference between those two options?
Same patch works!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130600

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

[PATCH] D130569: [Driver] Use libatomic for 32-bit SPARC atomics support on Linux

2022-07-28 Thread Rainer Orth via Phabricator via cfe-commits

ro updated this revision to Diff 448253.
ro marked 2 inline comments as done.
ro added a comment.

- Switch to `--push-state`/`--pop-state`.
- Add testcase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130569

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/linux-ld.c


Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -1007,6 +1007,7 @@
 // CHECK-SPARCV8: "{{.*}}ld{{(.exe)?}}"
 // CHECK-SPARCV8: "-m" "elf32_sparc"
 // CHECK-SPARCV8: "-dynamic-linker" 
"{{(/usr/sparc-unknown-linux-gnu)?}}/lib/ld-linux.so.2"
+// CHECK-SPARCV8: "--push-state" "--as-needed" "-latomic" "--pop-state"
 //
 // RUN: %clang -### %s -no-pie 2>&1 \
 // RUN: --target=sparcel-unknown-linux-gnu \
@@ -1021,6 +1022,7 @@
 // CHECK-SPARCV9: "{{.*}}ld{{(.exe)?}}"
 // CHECK-SPARCV9: "-m" "elf64_sparc"
 // CHECK-SPARCV9: "-dynamic-linker" 
"{{(/usr/sparcv9-unknown-linux-gnu)?}}/lib{{(64)?}}/ld-linux.so.2"
+// CHECK-SPARCV9-NOT: "-latomic"
 
 // Test linker invocation on Android.
 // RUN: %clang -### %s -no-pie 2>&1 \
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -631,6 +631,16 @@
 
   AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
 
+  // LLVM support for atomics on 32-bit SPARC V8+ is incomplete, so
+  // forcibly link with libatomic as a workaround.
+  // TODO: Issue #41880 and D118021.
+  if (getToolChain().getTriple().getArch() == llvm::Triple::sparc) {
+CmdArgs.push_back("--push-state");
+CmdArgs.push_back("--as-needed");
+CmdArgs.push_back("-latomic");
+CmdArgs.push_back("--pop-state");
+  }
+
   if (WantPthread && !isAndroid)
 CmdArgs.push_back("-lpthread");
 


Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -1007,6 +1007,7 @@
 // CHECK-SPARCV8: "{{.*}}ld{{(.exe)?}}"
 // CHECK-SPARCV8: "-m" "elf32_sparc"
 // CHECK-SPARCV8: "-dynamic-linker" "{{(/usr/sparc-unknown-linux-gnu)?}}/lib/ld-linux.so.2"
+// CHECK-SPARCV8: "--push-state" "--as-needed" "-latomic" "--pop-state"
 //
 // RUN: %clang -### %s -no-pie 2>&1 \
 // RUN: --target=sparcel-unknown-linux-gnu \
@@ -1021,6 +1022,7 @@
 // CHECK-SPARCV9: "{{.*}}ld{{(.exe)?}}"
 // CHECK-SPARCV9: "-m" "elf64_sparc"
 // CHECK-SPARCV9: "-dynamic-linker" "{{(/usr/sparcv9-unknown-linux-gnu)?}}/lib{{(64)?}}/ld-linux.so.2"
+// CHECK-SPARCV9-NOT: "-latomic"
 
 // Test linker invocation on Android.
 // RUN: %clang -### %s -no-pie 2>&1 \
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -631,6 +631,16 @@
 
   AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
 
+  // LLVM support for atomics on 32-bit SPARC V8+ is incomplete, so
+  // forcibly link with libatomic as a workaround.
+  // TODO: Issue #41880 and D118021.
+  if (getToolChain().getTriple().getArch() == llvm::Triple::sparc) {
+CmdArgs.push_back("--push-state");
+CmdArgs.push_back("--as-needed");
+CmdArgs.push_back("-latomic");
+CmdArgs.push_back("--pop-state");
+  }
+
   if (WantPthread && !isAndroid)
 CmdArgs.push_back("-lpthread");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130688: [Driver][Sparc] Default to -mcpu=v9 for SparcV8 on Linux

2022-07-28 Thread Rainer Orth via Phabricator via cfe-commits

ro created this revision.
ro added reviewers: glaubitz, MaskRay, jrtc27.
ro added a project: clang.
Herald added subscribers: StephenFan, pengfei, fedor.sergeev, jyknight.
Herald added a project: All.
ro requested review of this revision.

This is the Debian/sparc64 equivalent of D86621 
: since that distro only supports SPARC V9 
CPUs, it should default to `-mcpu=v9`, among others to allow inlining of 
atomics even in 32-bit code.

Tested on `sparc64-pc-linux-gnu`, `sparcv9-sun-solaris2.11`, and 
`x86_64-pc-linux-gnu`.

There's currently one issue, though: while this is the right thing for 
Debian/sparc64 (I don't think Debian/sparc which ran on non-V9 CPUs is 
supported any longer), it's wrong for current Linux distributions for 32-bit 
CPUs (LEON Linux AFAIK, there may be others).  While the driver can 
distringuish between Debian and non-Debian distributions for that, I've found 
no way to do so in the testsuite.  Thus, 
`clang/test/Preprocessor/predefined-arch-macros.c` currently `FAIL`s the 
`CHECK_SPARC` tests: it needs to check for `__sparc_v9__` on Debian, but keep 
`__sparcv8` on others.  Any suggestions on how to handle this?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130688

Files:
  clang/lib/Driver/ToolChains/Arch/Sparc.cpp


Index: clang/lib/Driver/ToolChains/Arch/Sparc.cpp
===
--- clang/lib/Driver/ToolChains/Arch/Sparc.cpp
+++ clang/lib/Driver/ToolChains/Arch/Sparc.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "Sparc.h"
+#include "clang/Driver/Distro.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
@@ -134,7 +135,8 @@
 return std::string(CPUName);
   }
 
-  if (Triple.getArch() == llvm::Triple::sparc && Triple.isOSSolaris())
+  if (Triple.getArch() == llvm::Triple::sparc &&
+  (Triple.isOSSolaris() || Distro(D.getVFS(), Triple).IsDebian()))
 return "v9";
   return "";
 }


Index: clang/lib/Driver/ToolChains/Arch/Sparc.cpp
===
--- clang/lib/Driver/ToolChains/Arch/Sparc.cpp
+++ clang/lib/Driver/ToolChains/Arch/Sparc.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "Sparc.h"
+#include "clang/Driver/Distro.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
@@ -134,7 +135,8 @@
 return std::string(CPUName);
   }
 
-  if (Triple.getArch() == llvm::Triple::sparc && Triple.isOSSolaris())
+  if (Triple.getArch() == llvm::Triple::sparc &&
+  (Triple.isOSSolaris() || Distro(D.getVFS(), Triple).IsDebian()))
 return "v9";
   return "";
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130689: [LLVM] Update C++ standard to 17

2022-07-28 Thread Tobias Hieta via Phabricator via cfe-commits

thieta created this revision.
thieta added reviewers: mehdi_amini, jyknight, jhenderson, hans, tstellar, 
cor3ntin, MaskRay.
Herald added subscribers: ayermolo, StephenFan, mgorny.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added a project: All.
thieta requested review of this revision.
Herald added subscribers: lldb-commits, cfe-commits, yota9.
Herald added projects: clang, LLDB, LLVM.

Also make the soft toolchain requirements hard. This allows
us to use C++17 features in LLVM now. Remember that if we want
to adopt some new feature in a bigger way it should be discussed
and added to the CodingStandard document.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130689

Files:
  bolt/runtime/CMakeLists.txt
  clang/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/CheckCompilerVersion.cmake
  llvm/docs/CodingStandards.rst
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -47,6 +47,18 @@
 Update on required toolchains to build LLVM
 ---
 
+LLVM is now built with C++17 by default. This means C++17 can be used in
+the code base.
+
+The previous "soft" toolchain requirements have now been changed to "hard".
+This means that the the following versions are now required to build LLVM
+and there is no way to supress this error.
+
+* GCC >= 7.1
+* Clang >= 5.0
+* Apple Clang >= 9.3
+* Visual Studio 2019 >= 16.7
+
 Changes to the LLVM IR
 --
 
Index: llvm/docs/CodingStandards.rst
===
--- llvm/docs/CodingStandards.rst
+++ llvm/docs/CodingStandards.rst
@@ -53,7 +53,7 @@
 C++ Standard Versions
 -
 
-Unless otherwise documented, LLVM subprojects are written using standard C++14
+Unless otherwise documented, LLVM subprojects are written using standard C++17
 code and avoid unnecessary vendor-specific extensions.
 
 Nevertheless, we restrict ourselves to features which are available in the
@@ -63,7 +63,7 @@
 Each toolchain provides a good reference for what it accepts:
 
 * Clang: https://clang.llvm.org/cxx_status.html
-* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx14
+* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx17
 * MSVC: https://msdn.microsoft.com/en-us/library/hh567368.aspx
 
 
Index: llvm/cmake/modules/CheckCompilerVersion.cmake
===
--- llvm/cmake/modules/CheckCompilerVersion.cmake
+++ llvm/cmake/modules/CheckCompilerVersion.cmake
@@ -4,21 +4,21 @@
 
 include(CheckCXXSourceCompiles)
 
-set(GCC_MIN 5.1)
+set(GCC_MIN 7.1)
 set(GCC_SOFT_ERROR 7.1)
-set(CLANG_MIN 3.5)
+set(CLANG_MIN 5.0)
 set(CLANG_SOFT_ERROR 5.0)
-set(APPLECLANG_MIN 6.0)
+set(APPLECLANG_MIN 9.3)
 set(APPLECLANG_SOFT_ERROR 9.3)
 
 # https://en.wikipedia.org/wiki/Microsoft_Visual_C#Internal_version_numbering
 # _MSC_VER == 1920 MSVC++ 14.20 Visual Studio 2019 Version 16.0
 # _MSC_VER == 1927 MSVC++ 14.27 Visual Studio 2019 Version 16.7
-set(MSVC_MIN 19.20)
+set(MSVC_MIN 19.27)
 set(MSVC_SOFT_ERROR 19.27)
 
 # Map the above GCC versions to dates: https://gcc.gnu.org/develop.html#timeline
-set(GCC_MIN_DATE 20150422)
+set(LIBSTDCXX_MIN 7)
 set(LIBSTDCXX_SOFT_ERROR 7)
 
 
@@ -79,20 +79,14 @@
 # Test for libstdc++ version of at least 4.8 by checking for _ZNKSt17bad_function_call4whatEv.
 # Note: We should check _GLIBCXX_RELEASE when possible (i.e., for GCC 7.1 and up).
 check_cxx_source_compiles("
-#include 
-#if defined(__GLIBCXX__)
-#if __GLIBCXX__ < ${GCC_MIN_DATE}
-#error Unsupported libstdc++ version
-#endif
-#endif
-#if defined(__GLIBCXX__)
-extern const char _ZNKSt17bad_function_call4whatEv[];
-const char *chk = _ZNKSt17bad_function_call4whatEv;
-#else
-const char *chk = \"\";
-#endif
-int main() { ++chk; return 0; }
-"
+  #include 
+  #if defined(__GLIBCXX__)
+  #if !defined(_GLIBCXX_RELEASE) || _GLIBCXX_RELEASE < ${LIBSTDCXX_MIN}
+  #error Unsupported libstdc++ version
+  #endif
+  #endif
+  int main() { return 0; }
+  "
   LLVM_LIBSTDCXX_MIN)
 if(NOT LLVM_LIBSTDCXX_MIN)
   message(FATAL_ERROR "libstdc++ version must be at least ${GCC_MIN}.")
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -58,7 +58,7 @@
 # Must go after project(..)
 include(GNUInstallDirs)
 
-set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
 set(CMAKE_CXX_STANDARD_REQUIRED YES)
 if (CYGWIN)
   # Cygwin is a bit stricter and lack things like 'strdup', 'stricmp', etc in
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists

[PATCH] D129446: [clang][driver] Find Apple default SDK path

2022-07-28 Thread Tobias Hieta via Phabricator via cfe-commits

thieta added a comment.

Ping - any thoughts on this one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129446

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

2022-07-28 Thread Mehdi AMINI via Phabricator via cfe-commits

mehdi_amini added a comment.
Herald added a subscriber: JDevlieghere.

Nice, LGTM, thanks for driving this!

> Remember that if we want to adopt some new feature in a bigger way it should 
> be discussed and added to the CodingStandard document.

What does it mean exactly? We can't use **anything** C++17 without writing it 
in the coding standards?
I'm not sure it'll be manageable: how do you see this playing out?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

2022-07-28 Thread Tobias Hieta via Phabricator via cfe-commits

thieta added a comment.

In D130689#3684330 , @mehdi_amini 
wrote:

> What does it mean exactly? We can't use **anything** C++17 without writing it 
> in the coding standards?
> I'm not sure it'll be manageable: how do you see this playing out?

Probably poorly worded - what I was trying to convey is that if we want to use 
a C++17 feature that's really impactful on the syntax/readability we should 
probably consider recommending ways to use it in the coding standards, similar 
to our guidelines on using for() loops 
(https://llvm.org/docs/CodingStandards.html#use-range-based-for-loops-wherever-possible)
 or the auto keyword 
(https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

2022-07-28 Thread Chuanqi Xu via Phabricator via cfe-commits

ChuanqiXu added a comment.

In D130689#3684333 , @thieta wrote:

> In D130689#3684330 , @mehdi_amini 
> wrote:
>
>> What does it mean exactly? We can't use **anything** C++17 without writing 
>> it in the coding standards?
>> I'm not sure it'll be manageable: how do you see this playing out?
>
> Probably poorly worded - what I was trying to convey is that if we want to 
> use a C++17 feature that's really impactful on the syntax/readability we 
> should probably consider recommending ways to use it in the coding standards, 
> similar to our guidelines on using for() loops 
> (https://llvm.org/docs/CodingStandards.html#use-range-based-for-loops-wherever-possible)
>  or the auto keyword 
> (https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable).

So it is free that developers want to use some C++17 features in a small amount 
of code, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

2022-07-28 Thread James Henderson via Phabricator via cfe-commits

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

In D130689#3684330 , @mehdi_amini 
wrote:

> Nice, LGTM, thanks for driving this!
>
>> Remember that if we want to adopt some new feature in a bigger way it should 
>> be discussed and added to the CodingStandard document.
>
> What does it mean exactly? We can't use **anything** C++17 without writing it 
> in the coding standards?
> I'm not sure it'll be manageable: how do you see this playing out?

Based on the release note, I don't think that was what was intended, although I 
am curious to understand what //was// intended!

Looks good to me anyway (but get more buy-in, perhaps a new Discourse post too, 
not just an update on the existing thread, since new threads get emailed out to 
more people).

Comment at: llvm/cmake/modules/CheckCompilerVersion.cmake:20

 # Map the above GCC versions to dates: 
https://gcc.gnu.org/develop.html#timeline
+set(LIBSTDCXX_MIN 7)

This comment seems out-of-date now.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

2022-07-28 Thread Daniel Bertalan via Phabricator via cfe-commits

BertalanD added inline comments.



Comment at: llvm/cmake/modules/CheckCompilerVersion.cmake:79-80
 set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -std=c++0x")
 # Test for libstdc++ version of at least 4.8 by checking for 
_ZNKSt17bad_function_call4whatEv.
 # Note: We should check _GLIBCXX_RELEASE when possible (i.e., for GCC 7.1 
and up).
 check_cxx_source_compiles("

This comment should be updated too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

2022-07-28 Thread Mehdi AMINI via Phabricator via cfe-commits

mehdi_amini added a comment.

In D130689#3684333 , @thieta wrote:

> In D130689#3684330 , @mehdi_amini 
> wrote:
>
>> What does it mean exactly? We can't use **anything** C++17 without writing 
>> it in the coding standards?
>> I'm not sure it'll be manageable: how do you see this playing out?
>
> Probably poorly worded - what I was trying to convey is that if we want to 
> use a C++17 feature that's really impactful on the syntax/readability we 
> should probably consider recommending ways to use it in the coding standards, 
> similar to our guidelines on using for() loops 
> (https://llvm.org/docs/CodingStandards.html#use-range-based-for-loops-wherever-possible)
>  or the auto keyword 
> (https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable).

Makes sense, thanks! Let's just update the wording to convey this and this all 
looks good to me :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130081: Add foldings for multi-line comment.

2022-07-28 Thread Haojian Wu via Phabricator via cfe-commits

hokein accepted this revision.
hokein added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:177
 
-// FIXME(kirillbobyrev): Collect comments, PP conditional regions, includes and
-// other code regions (e.g. public/private/protected sections of classes,
-// control flow statement bodies).
+// FIXME( usaxena95): Collect PP conditional regions, includes and other code
+// regions (e.g. public/private/protected sections of classes, control flow

nit: there is an extra space (I'd rather just use `FIXME:`)



Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:224
   if (Tok.Line < Paired->Line) {
-Position Start = offsetToPosition(
-Code,
-OrigStream.tokens()[Tok.OriginalIndex].text().data() - 
Code.data());
-Position End = offsetToPosition(
-Code, OrigStream.tokens()[Paired->OriginalIndex].text().data() -
-  Code.data());
-FoldingRange FR;
-FR.startLine = Start.line;
-FR.startCharacter = Start.character + 1;
-FR.endLine = End.line;
-FR.endCharacter = End.character;
-Result.push_back(FR);
+Position Start = offsetToPosition(Code, 1 + StartOffset(Tok));
+Position End = StartPosition(*Paired);

nit: add a comment explaining the +1 offset  (skipping the bracket).



Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:239
+while (T != Tokens.end() && T->Kind == tok::comment &&
+   StartPosition(*T).line <= LastCommentEnd.line + 1) {
+  LastCommentEnd = EndPosition(*T);

nit: the `<=` seems to do much --I think we care about `= LastCommentEnd.line` 
and `= LastCommentEnd.line + 1` cases, `< LastCommentEnd.line` should be 
impossible, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130081

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

2022-07-28 Thread Sergei Barannikov via Phabricator via cfe-commits

barannikov88 added a comment.

There are a few places (primarily in ADT and Support) that check __cplusplus > 
201402. Do they need to be cleaned up?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

2022-07-28 Thread Tobias Hieta via Phabricator via cfe-commits

thieta updated this revision to Diff 448261.
thieta added a comment.

Address some old comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

Files:
  bolt/runtime/CMakeLists.txt
  clang/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/CheckCompilerVersion.cmake
  llvm/docs/CodingStandards.rst
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -47,6 +47,18 @@
 Update on required toolchains to build LLVM
 ---
 
+LLVM is now built with C++17 by default. This means C++17 can be used in
+the code base.
+
+The previous "soft" toolchain requirements have now been changed to "hard".
+This means that the the following versions are now required to build LLVM
+and there is no way to supress this error.
+
+* GCC >= 7.1
+* Clang >= 5.0
+* Apple Clang >= 9.3
+* Visual Studio 2019 >= 16.7
+
 Changes to the LLVM IR
 --
 
Index: llvm/docs/CodingStandards.rst
===
--- llvm/docs/CodingStandards.rst
+++ llvm/docs/CodingStandards.rst
@@ -53,7 +53,7 @@
 C++ Standard Versions
 -
 
-Unless otherwise documented, LLVM subprojects are written using standard C++14
+Unless otherwise documented, LLVM subprojects are written using standard C++17
 code and avoid unnecessary vendor-specific extensions.
 
 Nevertheless, we restrict ourselves to features which are available in the
@@ -63,7 +63,7 @@
 Each toolchain provides a good reference for what it accepts:
 
 * Clang: https://clang.llvm.org/cxx_status.html
-* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx14
+* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx17
 * MSVC: https://msdn.microsoft.com/en-us/library/hh567368.aspx
 
 
Index: llvm/cmake/modules/CheckCompilerVersion.cmake
===
--- llvm/cmake/modules/CheckCompilerVersion.cmake
+++ llvm/cmake/modules/CheckCompilerVersion.cmake
@@ -4,21 +4,19 @@
 
 include(CheckCXXSourceCompiles)
 
-set(GCC_MIN 5.1)
+set(GCC_MIN 7.1)
 set(GCC_SOFT_ERROR 7.1)
-set(CLANG_MIN 3.5)
+set(CLANG_MIN 5.0)
 set(CLANG_SOFT_ERROR 5.0)
-set(APPLECLANG_MIN 6.0)
+set(APPLECLANG_MIN 9.3)
 set(APPLECLANG_SOFT_ERROR 9.3)
 
 # https://en.wikipedia.org/wiki/Microsoft_Visual_C#Internal_version_numbering
-# _MSC_VER == 1920 MSVC++ 14.20 Visual Studio 2019 Version 16.0
 # _MSC_VER == 1927 MSVC++ 14.27 Visual Studio 2019 Version 16.7
-set(MSVC_MIN 19.20)
+set(MSVC_MIN 19.27)
 set(MSVC_SOFT_ERROR 19.27)
 
-# Map the above GCC versions to dates: https://gcc.gnu.org/develop.html#timeline
-set(GCC_MIN_DATE 20150422)
+set(LIBSTDCXX_MIN 7)
 set(LIBSTDCXX_SOFT_ERROR 7)
 
 
@@ -76,23 +74,15 @@
 set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS})
 set(OLD_CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES})
 set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -std=c++0x")
-# Test for libstdc++ version of at least 4.8 by checking for _ZNKSt17bad_function_call4whatEv.
-# Note: We should check _GLIBCXX_RELEASE when possible (i.e., for GCC 7.1 and up).
 check_cxx_source_compiles("
-#include 
-#if defined(__GLIBCXX__)
-#if __GLIBCXX__ < ${GCC_MIN_DATE}
-#error Unsupported libstdc++ version
-#endif
-#endif
-#if defined(__GLIBCXX__)
-extern const char _ZNKSt17bad_function_call4whatEv[];
-const char *chk = _ZNKSt17bad_function_call4whatEv;
-#else
-const char *chk = \"\";
-#endif
-int main() { ++chk; return 0; }
-"
+  #include 
+  #if defined(__GLIBCXX__)
+  #if !defined(_GLIBCXX_RELEASE) || _GLIBCXX_RELEASE < ${LIBSTDCXX_MIN}
+  #error Unsupported libstdc++ version
+  #endif
+  #endif
+  int main() { return 0; }
+  "
   LLVM_LIBSTDCXX_MIN)
 if(NOT LLVM_LIBSTDCXX_MIN)
   message(FATAL_ERROR "libstdc++ version must be at least ${GCC_MIN}.")
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -58,7 +58,7 @@
 # Must go after project(..)
 include(GNUInstallDirs)
 
-set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
 set(CMAKE_CXX_STANDARD_REQUIRED YES)
 if (CYGWIN)
   # Cygwin is a bit stricter and lack things like 'strdup', 'stricmp', etc in
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -20,7 +20,7 @@
 if(LLDB_BUILT_STANDALONE)
   include(LLDBStandalone)
 
-  set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+  set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
   set(CMAKE_CXX_STANDARD_REQUIRED YES)
   set(CMAKE_CXX_EXTENSIONS NO)
 endif()

[PATCH] D130689: [LLVM] Update C++ standard to 17

2022-07-28 Thread Tobias Hieta via Phabricator via cfe-commits

thieta added a comment.

In D130689#3684360 , @barannikov88 
wrote:

> There are a few places (primarily in ADT and Support) that check __cplusplus 
> > 201402. Do they need to be cleaned up?

Sounds good - but maybe that can be addressed in a separate diff unless they 
make the build fail after this change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

2022-07-28 Thread Tobias Hieta via Phabricator via cfe-commits

thieta added a comment.

In D130689#3684334 , @ChuanqiXu wrote:

> So it is free that developers want to use some C++17 features in a small 
> amount of code, right?

As soon as this land C++ 17 should be free to use everywhere yeah!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

2022-07-28 Thread Tobias Hieta via Phabricator via cfe-commits

thieta updated this revision to Diff 448262.
thieta added a comment.

Fixed unintended indentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

Files:
  bolt/runtime/CMakeLists.txt
  clang/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/CheckCompilerVersion.cmake
  llvm/docs/CodingStandards.rst
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -47,6 +47,18 @@
 Update on required toolchains to build LLVM
 ---
 
+LLVM is now built with C++17 by default. This means C++17 can be used in
+the code base.
+
+The previous "soft" toolchain requirements have now been changed to "hard".
+This means that the the following versions are now required to build LLVM
+and there is no way to supress this error.
+
+* GCC >= 7.1
+* Clang >= 5.0
+* Apple Clang >= 9.3
+* Visual Studio 2019 >= 16.7
+
 Changes to the LLVM IR
 --
 
Index: llvm/docs/CodingStandards.rst
===
--- llvm/docs/CodingStandards.rst
+++ llvm/docs/CodingStandards.rst
@@ -53,7 +53,7 @@
 C++ Standard Versions
 -
 
-Unless otherwise documented, LLVM subprojects are written using standard C++14
+Unless otherwise documented, LLVM subprojects are written using standard C++17
 code and avoid unnecessary vendor-specific extensions.
 
 Nevertheless, we restrict ourselves to features which are available in the
@@ -63,7 +63,7 @@
 Each toolchain provides a good reference for what it accepts:
 
 * Clang: https://clang.llvm.org/cxx_status.html
-* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx14
+* GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx17
 * MSVC: https://msdn.microsoft.com/en-us/library/hh567368.aspx
 
 
Index: llvm/cmake/modules/CheckCompilerVersion.cmake
===
--- llvm/cmake/modules/CheckCompilerVersion.cmake
+++ llvm/cmake/modules/CheckCompilerVersion.cmake
@@ -4,21 +4,19 @@
 
 include(CheckCXXSourceCompiles)
 
-set(GCC_MIN 5.1)
+set(GCC_MIN 7.1)
 set(GCC_SOFT_ERROR 7.1)
-set(CLANG_MIN 3.5)
+set(CLANG_MIN 5.0)
 set(CLANG_SOFT_ERROR 5.0)
-set(APPLECLANG_MIN 6.0)
+set(APPLECLANG_MIN 9.3)
 set(APPLECLANG_SOFT_ERROR 9.3)
 
 # https://en.wikipedia.org/wiki/Microsoft_Visual_C#Internal_version_numbering
-# _MSC_VER == 1920 MSVC++ 14.20 Visual Studio 2019 Version 16.0
 # _MSC_VER == 1927 MSVC++ 14.27 Visual Studio 2019 Version 16.7
-set(MSVC_MIN 19.20)
+set(MSVC_MIN 19.27)
 set(MSVC_SOFT_ERROR 19.27)
 
-# Map the above GCC versions to dates: https://gcc.gnu.org/develop.html#timeline
-set(GCC_MIN_DATE 20150422)
+set(LIBSTDCXX_MIN 7)
 set(LIBSTDCXX_SOFT_ERROR 7)
 
 
@@ -76,22 +74,14 @@
 set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS})
 set(OLD_CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES})
 set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -std=c++0x")
-# Test for libstdc++ version of at least 4.8 by checking for _ZNKSt17bad_function_call4whatEv.
-# Note: We should check _GLIBCXX_RELEASE when possible (i.e., for GCC 7.1 and up).
 check_cxx_source_compiles("
 #include 
 #if defined(__GLIBCXX__)
-#if __GLIBCXX__ < ${GCC_MIN_DATE}
+#if !defined(_GLIBCXX_RELEASE) || _GLIBCXX_RELEASE < ${LIBSTDCXX_MIN}
 #error Unsupported libstdc++ version
 #endif
 #endif
-#if defined(__GLIBCXX__)
-extern const char _ZNKSt17bad_function_call4whatEv[];
-const char *chk = _ZNKSt17bad_function_call4whatEv;
-#else
-const char *chk = \"\";
-#endif
-int main() { ++chk; return 0; }
+int main() { return 0; }
 "
   LLVM_LIBSTDCXX_MIN)
 if(NOT LLVM_LIBSTDCXX_MIN)
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -58,7 +58,7 @@
 # Must go after project(..)
 include(GNUInstallDirs)
 
-set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
 set(CMAKE_CXX_STANDARD_REQUIRED YES)
 if (CYGWIN)
   # Cygwin is a bit stricter and lack things like 'strdup', 'stricmp', etc in
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -20,7 +20,7 @@
 if(LLDB_BUILT_STANDALONE)
   include(LLDBStandalone)
 
-  set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+  set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
   set(CMAKE_CXX_STANDARD_REQUIRED YES)
   set(CMAKE_CXX_EXTENSIONS NO)
 endif()
Index: lld/CMakeLists.txt
===
--- lld/CMakeLists.txt
+++ lld/CMakeLists.txt
@@ -11,7 +11,7 @@
 include(GNUInstallDirs)

[PATCH] D130689: [LLVM] Update C++ standard to 17

2022-07-28 Thread Corentin Jabot via Phabricator via cfe-commits

cor3ntin accepted this revision.
cor3ntin added a comment.

Thanks a lot for working on this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

2022-07-28 Thread H. Vetinari via Phabricator via cfe-commits

h-vetinari added a comment.

It may be worth calling out that this is about C++17 core language and not the 
standard library?

libstdcxx only finished C++17 support in GCC 12, and libcxx is still missing 
various pieces even today (much less for Clang 5).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130511: [pseudo][wip] Eliminate simple-type-specifier ambiguities.

2022-07-28 Thread Haojian Wu via Phabricator via cfe-commits

hokein updated this revision to Diff 448265.
hokein marked an inline comment as done.
hokein added a comment.

address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130511

Files:
  clang-tools-extra/pseudo/include/clang-pseudo/Token.h
  clang-tools-extra/pseudo/lib/cxx/CXX.cpp
  clang-tools-extra/pseudo/lib/cxx/cxx.bnf
  clang-tools-extra/pseudo/test/cxx/nested-name-specifier.cpp


Index: clang-tools-extra/pseudo/test/cxx/nested-name-specifier.cpp
===
--- /dev/null
+++ clang-tools-extra/pseudo/test/cxx/nested-name-specifier.cpp
@@ -0,0 +1,28 @@
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest | FileCheck %s
+
+// Verify that we don't form a complete `::` nested-name-specifier if there is
+// an identifier preceding it. 
+Foo::Foo() {} // No  "Foo ::Foo()" false parse
+// CHECK:  ├─declaration-seq~function-definition := function-declarator 
function-body
+// CHECK-NEXT: │ ├─function-declarator~noptr-declarator := noptr-declarator 
parameters-and-qualifiers
+
+int ::x;
+// CHECK:  declaration~simple-declaration := decl-specifier-seq 
init-declarator-list ;
+// CHECK-NEXT: ├─decl-specifier-seq~INT
+
+void test() {
+  X::Y::Z; // No false qualified-declarator parses "X ::Y::Z" and "X::Y ::Z".
+// CHECK:  statement-seq~statement := 
+// CHECK:  statement~expression-statement := expression ;
+// CHECK:  statement~simple-declaration := decl-specifier-seq ;
+// CHECK-NOT: simple-declaration := decl-specifier-seq init-declarator-list ;
+
+  // FIXME: eliminate the false `a ::c` declaration parse.
+  a::c;
+// CHECK: statement := 
+// CHECK-NEXT: ├─statement~expression-statement := expression ;
+// CHECK-NEXT: │ ├─expression~relational-expression :=
+// CHECK:  └─statement~simple-declaration := 
+// CHECK-NEXT:   ├─simple-declaration := decl-specifier-seq ;
+// CHECK:└─simple-declaration := decl-specifier-seq 
init-declarator-list ;
+}
Index: clang-tools-extra/pseudo/lib/cxx/cxx.bnf
===
--- clang-tools-extra/pseudo/lib/cxx/cxx.bnf
+++ clang-tools-extra/pseudo/lib/cxx/cxx.bnf
@@ -68,7 +68,7 @@
 unqualified-id := ~ decltype-specifier
 unqualified-id := template-id
 qualified-id := nested-name-specifier TEMPLATE_opt unqualified-id
-nested-name-specifier := ::
+nested-name-specifier := :: [guard]
 nested-name-specifier := type-name ::
 nested-name-specifier := namespace-name ::
 nested-name-specifier := decltype-specifier ::
Index: clang-tools-extra/pseudo/lib/cxx/CXX.cpp
===
--- clang-tools-extra/pseudo/lib/cxx/CXX.cpp
+++ clang-tools-extra/pseudo/lib/cxx/CXX.cpp
@@ -312,6 +312,14 @@

IF__CONSTEXPR__L_PAREN__init_statement__condition__R_PAREN__statement,
guardNextTokenNotElse},
 
+  // Implement C++ [basic.lookup.qual.general]:
+  //   If a name, template-id, or decltype-specifier is followed by a
+  //   ::, it shall designate a namespace, class, enumeration, or
+  //   dependent type, and the :: is never interpreted as a complete
+  //   nested-name-specifier.
+  {rule::nested_name_specifier::COLONCOLON,
+   TOKEN_GUARD(coloncolon, Tok.prev().Kind != tok::identifier)},
+
   // The grammar distinguishes (only) user-defined vs plain string 
literals,
   // where the clang lexer distinguishes (only) encoding types.
   {rule::user_defined_string_literal_chunk::STRING_LITERAL,
Index: clang-tools-extra/pseudo/include/clang-pseudo/Token.h
===
--- clang-tools-extra/pseudo/include/clang-pseudo/Token.h
+++ clang-tools-extra/pseudo/include/clang-pseudo/Token.h
@@ -90,6 +90,11 @@
 while (T->Kind == tok::comment);
 return *T;
   }
+  /// Returns the previous token in the stream. this may not be a sentinel.
+  const Token &prev() const {
+assert(Kind != tok::eof);
+return *(this - 1);
+  }
   /// Returns the bracket paired with this one, if any.
   const Token *pair() const { return Pair == 0 ? nullptr : this + Pair; }
 


Index: clang-tools-extra/pseudo/test/cxx/nested-name-specifier.cpp
===
--- /dev/null
+++ clang-tools-extra/pseudo/test/cxx/nested-name-specifier.cpp
@@ -0,0 +1,28 @@
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest | FileCheck %s
+
+// Verify that we don't form a complete `::` nested-name-specifier if there is
+// an identifier preceding it. 
+Foo::Foo() {} // No  "Foo ::Foo()" false parse
+// CHECK:  ââdeclaration-seq~function-definition := function-declarator function-body
+// CHECK-NEXT: â ââfunction-declarator~noptr-declarator := noptr-declarator parameters-and-qualifiers
+
+int ::x;
+// CHECK:  declaration~simple-declaration := decl-specifier-seq ini

[PATCH] D130511: [pseudo][wip] Eliminate simple-type-specifier ambiguities.

2022-07-28 Thread Haojian Wu via Phabricator via cfe-commits

hokein added inline comments.



Comment at: clang-tools-extra/pseudo/lib/cxx/CXX.cpp:168
+bool guardPreviousTokenNotIdentifier(const GuardParams &P) {
+  if (P.LookaheadIndex < 2)
+return true;

sammccall wrote:
> Is LookaheadIndex from another patch?
> I can't find it at head.
> 
> It seems a bit gratuitous here vs P.RHS.front()->startTokenIndex()... In 
> general getting the info from RHS seems cleaner than jumping across by 
> reasoning how many tokens it has
> 
> 
Yeah, the lookaheadIndex is from my other patch: 
https://reviews.llvm.org/D130591

I think using `Tok.prev()` is much better.



Comment at: clang-tools-extra/pseudo/lib/cxx/CXX.cpp:325
+  {(RuleID)Rule::nested_name_specifier_0coloncolon,
+   guardPreviousTokenNotIdentifier},
+

sammccall wrote:
> You could write this as `TOKEN_GUARD(coloncolon, Tok.prev().Kind != 
> tok::identifier)`
> 
> If it's that short i like having the guard logic inline to avoid the 
> indirection for the reader
Good point! 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130511

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

[PATCH] D129799: [clang-tidy] Add CLANG_TIDY_CONFUSABLE_CHARS_GEN cmake setting to avoid building when cross compiling

2022-07-28 Thread Sam McCall via Phabricator via cfe-commits

sammccall accepted this revision.
sammccall added inline comments.



Comment at: clang-tools-extra/clang-tidy/misc/CMakeLists.txt:13
+elseif(LLVM_USE_HOST_TOOLS)
   build_native_tool(clang-tidy-confusable-chars-gen make_confusable_table)
   set(make_confusable_table_target "${make_confusable_table}")

Should make_confusable_table and make_confusable_table_target also be renamed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129799

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

[PATCH] D130614: [C++20] [Modules] Merge same concept decls in global module fragment

2022-07-28 Thread Chuanqi Xu via Phabricator via cfe-commits

ChuanqiXu marked 2 inline comments as done.
ChuanqiXu added a comment.

BTW, I've sent a NFC test to show the current behavior for functions, classes 
and variables are basically fine: 
https://github.com/llvm/llvm-project/commit/fe1887da36c63f64903de112f2a8e88f973318fa


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

https://reviews.llvm.org/D130614

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

[PATCH] D129798: [clang-tidy] Rename the make-confusable-table executable

2022-07-28 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: clang-tools-extra/clang-tidy/misc/CMakeLists.txt:10
 else()
-  set(make_confusable_table $)
-  set(make_confusable_table_target make-confusable-table)
+  set(make_confusable_table $)
+  set(make_confusable_table_target clang-tidy-confusable-chars-gen)

(Should the variable names also be updated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129798

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

[clang-tools-extra] 18b4a8b - [clang-tidy] Rename the make-confusable-table executable

2022-07-28 Thread Martin Storsjö via cfe-commits


Author: Martin Storsjö
Date: 2022-07-28T12:00:20+03:00
New Revision: 18b4a8bcf3553174f770f09528c9bd01c8cebfe7

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

LOG: [clang-tidy] Rename the make-confusable-table executable

Rename it to clang-tidy-confusable-chars-gen, to make its role
clearer in a wider context.

In cross builds, the caller might want to provide this tool
externally (to avoid needing to rebuild it in the cross build).
In such a case, having the tool properly namespaced makes its role
clearer.

This matches how the clang-pseudo-gen tool was renamed in
a43fef05d4fae32f02365c7b8fef2aa631d23628 / D126725.

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/clang-tidy/misc/ConfusableTable/CMakeLists.txt

llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/misc/ConfusableTable/BUILD.gn

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
index 04172db29ea5e..ee8fe0b37fce9 100644
--- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -4,11 +4,11 @@ set(LLVM_LINK_COMPONENTS
   )
 
 if(LLVM_USE_HOST_TOOLS)
-  build_native_tool(make-confusable-table make_confusable_table)
+  build_native_tool(clang-tidy-confusable-chars-gen make_confusable_table)
   set(make_confusable_table_target "${make_confusable_table}")
 else()
-  set(make_confusable_table $)
-  set(make_confusable_table_target make-confusable-table)
+  set(make_confusable_table $)
+  set(make_confusable_table_target clang-tidy-confusable-chars-gen)
 endif()
 
 add_subdirectory(ConfusableTable)

diff  --git a/clang-tools-extra/clang-tidy/misc/ConfusableTable/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/misc/ConfusableTable/CMakeLists.txt
index a35f206fbf783..f0ad2dbc0c578 100644
--- a/clang-tools-extra/clang-tidy/misc/ConfusableTable/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/misc/ConfusableTable/CMakeLists.txt
@@ -1,6 +1,6 @@
 set(LLVM_LINK_COMPONENTS Support)
 list(REMOVE_ITEM LLVM_COMMON_DEPENDS clang-tablegen-targets)
 
-add_llvm_executable(make-confusable-table
+add_llvm_executable(clang-tidy-confusable-chars-gen
   BuildConfusableTable.cpp
   )

diff  --git 
a/llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/misc/ConfusableTable/BUILD.gn
 
b/llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/misc/ConfusableTable/BUILD.gn
index 10521474ce57e..25025c499f04a 100644
--- 
a/llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/misc/ConfusableTable/BUILD.gn
+++ 
b/llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/misc/ConfusableTable/BUILD.gn
@@ -1,4 +1,4 @@
-executable("make-confusable-table") {
+executable("clang-tidy-confusable-chars-gen") {
   deps = [ "//llvm/lib/Support" ]
   sources = [ "BuildConfusableTable.cpp" ]
 }



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

[clang-tools-extra] dc95d0c - [clang-tidy] Add CLANG_TIDY_CONFUSABLE_CHARS_GEN cmake cache variable to avoid building when cross compiling

2022-07-28 Thread Martin Storsjö via cfe-commits


Author: Martin Storsjö
Date: 2022-07-28T12:00:21+03:00
New Revision: dc95d0c525636aed53a3b38258efa2dff4c83edf

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

LOG: [clang-tidy] Add CLANG_TIDY_CONFUSABLE_CHARS_GEN cmake cache variable to 
avoid building when cross compiling

This is similar to the LLVM_TABLEGEN, CLANG_TABLEGEN and
CLANG_PSEUDO_GEN cmake cache variables.

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/misc/CMakeLists.txt

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
index ee8fe0b37fce9..de76b4b00c360 100644
--- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -3,7 +3,13 @@ set(LLVM_LINK_COMPONENTS
   Support
   )
 
-if(LLVM_USE_HOST_TOOLS)
+set(CLANG_TIDY_CONFUSABLE_CHARS_GEN "clang-tidy-confusable-chars-gen" CACHE
+  STRING "Host clang-tidy-confusable-chars-gen executable. Saves building if 
cross-compiling.")
+
+if(NOT CLANG_TIDY_CONFUSABLE_CHARS_GEN STREQUAL 
"clang-tidy-confusable-chars-gen")
+  set(make_confusable_table ${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
+  set(make_confusable_table_target ${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
+elseif(LLVM_USE_HOST_TOOLS)
   build_native_tool(clang-tidy-confusable-chars-gen make_confusable_table)
   set(make_confusable_table_target "${make_confusable_table}")
 else()



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

[PATCH] D129798: [clang-tidy] Rename the make-confusable-table executable

2022-07-28 Thread Martin Storsjö 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 rG18b4a8bcf355: [clang-tidy] Rename the make-confusable-table 
executable (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129798

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/ConfusableTable/CMakeLists.txt
  
llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/misc/ConfusableTable/BUILD.gn


Index: 
llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/misc/ConfusableTable/BUILD.gn
===
--- 
llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/misc/ConfusableTable/BUILD.gn
+++ 
llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/misc/ConfusableTable/BUILD.gn
@@ -1,4 +1,4 @@
-executable("make-confusable-table") {
+executable("clang-tidy-confusable-chars-gen") {
   deps = [ "//llvm/lib/Support" ]
   sources = [ "BuildConfusableTable.cpp" ]
 }
Index: clang-tools-extra/clang-tidy/misc/ConfusableTable/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/misc/ConfusableTable/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/misc/ConfusableTable/CMakeLists.txt
@@ -1,6 +1,6 @@
 set(LLVM_LINK_COMPONENTS Support)
 list(REMOVE_ITEM LLVM_COMMON_DEPENDS clang-tablegen-targets)
 
-add_llvm_executable(make-confusable-table
+add_llvm_executable(clang-tidy-confusable-chars-gen
   BuildConfusableTable.cpp
   )
Index: clang-tools-extra/clang-tidy/misc/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -4,11 +4,11 @@
   )
 
 if(LLVM_USE_HOST_TOOLS)
-  build_native_tool(make-confusable-table make_confusable_table)
+  build_native_tool(clang-tidy-confusable-chars-gen make_confusable_table)
   set(make_confusable_table_target "${make_confusable_table}")
 else()
-  set(make_confusable_table $)
-  set(make_confusable_table_target make-confusable-table)
+  set(make_confusable_table $)
+  set(make_confusable_table_target clang-tidy-confusable-chars-gen)
 endif()
 
 add_subdirectory(ConfusableTable)


Index: llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/misc/ConfusableTable/BUILD.gn
===
--- llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/misc/ConfusableTable/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/misc/ConfusableTable/BUILD.gn
@@ -1,4 +1,4 @@
-executable("make-confusable-table") {
+executable("clang-tidy-confusable-chars-gen") {
   deps = [ "//llvm/lib/Support" ]
   sources = [ "BuildConfusableTable.cpp" ]
 }
Index: clang-tools-extra/clang-tidy/misc/ConfusableTable/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/misc/ConfusableTable/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/misc/ConfusableTable/CMakeLists.txt
@@ -1,6 +1,6 @@
 set(LLVM_LINK_COMPONENTS Support)
 list(REMOVE_ITEM LLVM_COMMON_DEPENDS clang-tablegen-targets)
 
-add_llvm_executable(make-confusable-table
+add_llvm_executable(clang-tidy-confusable-chars-gen
   BuildConfusableTable.cpp
   )
Index: clang-tools-extra/clang-tidy/misc/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -4,11 +4,11 @@
   )
 
 if(LLVM_USE_HOST_TOOLS)
-  build_native_tool(make-confusable-table make_confusable_table)
+  build_native_tool(clang-tidy-confusable-chars-gen make_confusable_table)
   set(make_confusable_table_target "${make_confusable_table}")
 else()
-  set(make_confusable_table $)
-  set(make_confusable_table_target make-confusable-table)
+  set(make_confusable_table $)
+  set(make_confusable_table_target clang-tidy-confusable-chars-gen)
 endif()
 
 add_subdirectory(ConfusableTable)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 6f6c40a - [pseudo] Eliminate the false `::` nested-name-specifier ambiguity

2022-07-28 Thread Haojian Wu via cfe-commits


Author: Haojian Wu
Date: 2022-07-28T11:01:15+02:00
New Revision: 6f6c40a875c84443f255f3a6b4efc0bc0f2fb67a

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

LOG: [pseudo] Eliminate the false `::` nested-name-specifier ambiguity

The solution is to favor the longest possible nest-name-specifier, and
drop other alternatives by using the guard, per per C++ 
[basic.lookup.qual.general].

Motivated cases:

```
Foo::Foo() {};
// the constructor can be parsed as:
//  - Foo ::Foo(); // where the first Foo is return-type, and ::Foo is the 
function declarator
//  + Foo::Foo(); // where Foo::Foo is the function declarator
```

```
void test() {

// a very slow parsing case when there are many qualifers!
X::Y::Z;
// The statement can be parsed as:
//  - X ::Y::Z; // ::Y::Z is the declarator
//  - X::Y ::Z; // ::Z is the declarator
//  + X::Y::Z;  // a declaration without declarator (X::Y::Z is 
decl-specifier-seq)
//  + X::Y::Z;  // a qualifed-id expression
}
```

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

Added: 
clang-tools-extra/pseudo/test/cxx/nested-name-specifier.cpp

Modified: 
clang-tools-extra/pseudo/include/clang-pseudo/Token.h
clang-tools-extra/pseudo/lib/cxx/CXX.cpp
clang-tools-extra/pseudo/lib/cxx/cxx.bnf

Removed: 




diff  --git a/clang-tools-extra/pseudo/include/clang-pseudo/Token.h 
b/clang-tools-extra/pseudo/include/clang-pseudo/Token.h
index e4a8659f739cf..22b72c71cbbab 100644
--- a/clang-tools-extra/pseudo/include/clang-pseudo/Token.h
+++ b/clang-tools-extra/pseudo/include/clang-pseudo/Token.h
@@ -90,6 +90,11 @@ struct Token {
 while (T->Kind == tok::comment);
 return *T;
   }
+  /// Returns the previous token in the stream. this may not be a sentinel.
+  const Token &prev() const {
+assert(Kind != tok::eof);
+return *(this - 1);
+  }
   /// Returns the bracket paired with this one, if any.
   const Token *pair() const { return Pair == 0 ? nullptr : this + Pair; }
 

diff  --git a/clang-tools-extra/pseudo/lib/cxx/CXX.cpp 
b/clang-tools-extra/pseudo/lib/cxx/CXX.cpp
index a0a252c50b985..668c4d1bfcb6a 100644
--- a/clang-tools-extra/pseudo/lib/cxx/CXX.cpp
+++ b/clang-tools-extra/pseudo/lib/cxx/CXX.cpp
@@ -312,6 +312,14 @@ llvm::DenseMap buildGuards() {

IF__CONSTEXPR__L_PAREN__init_statement__condition__R_PAREN__statement,
guardNextTokenNotElse},
 
+  // Implement C++ [basic.lookup.qual.general]:
+  //   If a name, template-id, or decltype-specifier is followed by a
+  //   ::, it shall designate a namespace, class, enumeration, or
+  //   dependent type, and the :: is never interpreted as a complete
+  //   nested-name-specifier.
+  {rule::nested_name_specifier::COLONCOLON,
+   TOKEN_GUARD(coloncolon, Tok.prev().Kind != tok::identifier)},
+
   // The grammar distinguishes (only) user-defined vs plain string 
literals,
   // where the clang lexer distinguishes (only) encoding types.
   {rule::user_defined_string_literal_chunk::STRING_LITERAL,

diff  --git a/clang-tools-extra/pseudo/lib/cxx/cxx.bnf 
b/clang-tools-extra/pseudo/lib/cxx/cxx.bnf
index 8dfab8b9788d9..e1667fe8ea732 100644
--- a/clang-tools-extra/pseudo/lib/cxx/cxx.bnf
+++ b/clang-tools-extra/pseudo/lib/cxx/cxx.bnf
@@ -68,7 +68,7 @@ unqualified-id := ~ type-name
 unqualified-id := ~ decltype-specifier
 unqualified-id := template-id
 qualified-id := nested-name-specifier TEMPLATE_opt unqualified-id
-nested-name-specifier := ::
+nested-name-specifier := :: [guard]
 nested-name-specifier := type-name ::
 nested-name-specifier := namespace-name ::
 nested-name-specifier := decltype-specifier ::

diff  --git a/clang-tools-extra/pseudo/test/cxx/nested-name-specifier.cpp 
b/clang-tools-extra/pseudo/test/cxx/nested-name-specifier.cpp
new file mode 100644
index 0..41d0fa13ff6dd
--- /dev/null
+++ b/clang-tools-extra/pseudo/test/cxx/nested-name-specifier.cpp
@@ -0,0 +1,28 @@
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest | FileCheck %s
+
+// Verify that we don't form a complete `::` nested-name-specifier if there is
+// an identifier preceding it.
+Foo::Foo() {} // No  "Foo ::Foo()" false parse
+// CHECK:  ├─declaration-seq~function-definition := function-declarator 
function-body
+// CHECK-NEXT: │ ├─function-declarator~noptr-declarator := noptr-declarator 
parameters-and-qualifiers
+
+int ::x;
+// CHECK:  declaration~simple-declaration := decl-specifier-seq 
init-declarator-list ;
+// CHECK-NEXT: ├─decl-specifier-seq~INT
+
+void test() {
+  X::Y::Z; // No false qualified-declarator parses "X ::Y::Z" and "X::Y ::Z".
+// CHECK:  statement-seq~statement := 
+// CHECK:  statement~expression-statement := expression ;
+// CHECK:  statement~simple-declaration := decl-specifier-seq ;
+// CHECK-NOT:

[PATCH] D129799: [clang-tidy] Add CLANG_TIDY_CONFUSABLE_CHARS_GEN cmake setting to avoid building when cross compiling

2022-07-28 Thread Martin Storsjö via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdc95d0c52563: [clang-tidy] Add 
CLANG_TIDY_CONFUSABLE_CHARS_GEN cmake cache variable to avoid… (authored by 
mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129799

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt


Index: clang-tools-extra/clang-tidy/misc/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -3,7 +3,13 @@
   Support
   )
 
-if(LLVM_USE_HOST_TOOLS)
+set(CLANG_TIDY_CONFUSABLE_CHARS_GEN "clang-tidy-confusable-chars-gen" CACHE
+  STRING "Host clang-tidy-confusable-chars-gen executable. Saves building if 
cross-compiling.")
+
+if(NOT CLANG_TIDY_CONFUSABLE_CHARS_GEN STREQUAL 
"clang-tidy-confusable-chars-gen")
+  set(make_confusable_table ${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
+  set(make_confusable_table_target ${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
+elseif(LLVM_USE_HOST_TOOLS)
   build_native_tool(clang-tidy-confusable-chars-gen make_confusable_table)
   set(make_confusable_table_target "${make_confusable_table}")
 else()


Index: clang-tools-extra/clang-tidy/misc/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -3,7 +3,13 @@
   Support
   )
 
-if(LLVM_USE_HOST_TOOLS)
+set(CLANG_TIDY_CONFUSABLE_CHARS_GEN "clang-tidy-confusable-chars-gen" CACHE
+  STRING "Host clang-tidy-confusable-chars-gen executable. Saves building if cross-compiling.")
+
+if(NOT CLANG_TIDY_CONFUSABLE_CHARS_GEN STREQUAL "clang-tidy-confusable-chars-gen")
+  set(make_confusable_table ${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
+  set(make_confusable_table_target ${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
+elseif(LLVM_USE_HOST_TOOLS)
   build_native_tool(clang-tidy-confusable-chars-gen make_confusable_table)
   set(make_confusable_table_target "${make_confusable_table}")
 else()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130511: [pseudo] Eliminate the false `::` nested-name-specifier ambiguity

2022-07-28 Thread Haojian Wu 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 rG6f6c40a875c8: [pseudo] Eliminate the false `::` 
nested-name-specifier ambiguity (authored by hokein).

Changed prior to commit:
  https://reviews.llvm.org/D130511?vs=448265&id=448271#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130511

Files:
  clang-tools-extra/pseudo/include/clang-pseudo/Token.h
  clang-tools-extra/pseudo/lib/cxx/CXX.cpp
  clang-tools-extra/pseudo/lib/cxx/cxx.bnf
  clang-tools-extra/pseudo/test/cxx/nested-name-specifier.cpp


Index: clang-tools-extra/pseudo/test/cxx/nested-name-specifier.cpp
===
--- /dev/null
+++ clang-tools-extra/pseudo/test/cxx/nested-name-specifier.cpp
@@ -0,0 +1,28 @@
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest | FileCheck %s
+
+// Verify that we don't form a complete `::` nested-name-specifier if there is
+// an identifier preceding it.
+Foo::Foo() {} // No  "Foo ::Foo()" false parse
+// CHECK:  ├─declaration-seq~function-definition := function-declarator 
function-body
+// CHECK-NEXT: │ ├─function-declarator~noptr-declarator := noptr-declarator 
parameters-and-qualifiers
+
+int ::x;
+// CHECK:  declaration~simple-declaration := decl-specifier-seq 
init-declarator-list ;
+// CHECK-NEXT: ├─decl-specifier-seq~INT
+
+void test() {
+  X::Y::Z; // No false qualified-declarator parses "X ::Y::Z" and "X::Y ::Z".
+// CHECK:  statement-seq~statement := 
+// CHECK:  statement~expression-statement := expression ;
+// CHECK:  statement~simple-declaration := decl-specifier-seq ;
+// CHECK-NOT: simple-declaration := decl-specifier-seq init-declarator-list ;
+
+  // FIXME: eliminate the false `a ::c` declaration parse.
+  a::c;
+// CHECK: statement := 
+// CHECK-NEXT: ├─statement~expression-statement := expression ;
+// CHECK-NEXT: │ ├─expression~relational-expression :=
+// CHECK:  └─statement~simple-declaration := 
+// CHECK-NEXT:   ├─simple-declaration := decl-specifier-seq ;
+// CHECK:└─simple-declaration := decl-specifier-seq 
init-declarator-list ;
+}
Index: clang-tools-extra/pseudo/lib/cxx/cxx.bnf
===
--- clang-tools-extra/pseudo/lib/cxx/cxx.bnf
+++ clang-tools-extra/pseudo/lib/cxx/cxx.bnf
@@ -68,7 +68,7 @@
 unqualified-id := ~ decltype-specifier
 unqualified-id := template-id
 qualified-id := nested-name-specifier TEMPLATE_opt unqualified-id
-nested-name-specifier := ::
+nested-name-specifier := :: [guard]
 nested-name-specifier := type-name ::
 nested-name-specifier := namespace-name ::
 nested-name-specifier := decltype-specifier ::
Index: clang-tools-extra/pseudo/lib/cxx/CXX.cpp
===
--- clang-tools-extra/pseudo/lib/cxx/CXX.cpp
+++ clang-tools-extra/pseudo/lib/cxx/CXX.cpp
@@ -312,6 +312,14 @@

IF__CONSTEXPR__L_PAREN__init_statement__condition__R_PAREN__statement,
guardNextTokenNotElse},
 
+  // Implement C++ [basic.lookup.qual.general]:
+  //   If a name, template-id, or decltype-specifier is followed by a
+  //   ::, it shall designate a namespace, class, enumeration, or
+  //   dependent type, and the :: is never interpreted as a complete
+  //   nested-name-specifier.
+  {rule::nested_name_specifier::COLONCOLON,
+   TOKEN_GUARD(coloncolon, Tok.prev().Kind != tok::identifier)},
+
   // The grammar distinguishes (only) user-defined vs plain string 
literals,
   // where the clang lexer distinguishes (only) encoding types.
   {rule::user_defined_string_literal_chunk::STRING_LITERAL,
Index: clang-tools-extra/pseudo/include/clang-pseudo/Token.h
===
--- clang-tools-extra/pseudo/include/clang-pseudo/Token.h
+++ clang-tools-extra/pseudo/include/clang-pseudo/Token.h
@@ -90,6 +90,11 @@
 while (T->Kind == tok::comment);
 return *T;
   }
+  /// Returns the previous token in the stream. this may not be a sentinel.
+  const Token &prev() const {
+assert(Kind != tok::eof);
+return *(this - 1);
+  }
   /// Returns the bracket paired with this one, if any.
   const Token *pair() const { return Pair == 0 ? nullptr : this + Pair; }
 


Index: clang-tools-extra/pseudo/test/cxx/nested-name-specifier.cpp
===
--- /dev/null
+++ clang-tools-extra/pseudo/test/cxx/nested-name-specifier.cpp
@@ -0,0 +1,28 @@
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest | FileCheck %s
+
+// Verify that we don't form a complete `::` nested-name-specifier if there is
+// an identifier preceding it.
+Foo::Foo() {} // No  "Foo ::Foo()" false parse
+// CHECK:  ââdeclaration-seq~function-definition := function-declarator function-

[PATCH] D130690: [clangd][NFCI] Store TUPath inside ParsedAST

2022-07-28 Thread Kadir Cetinkaya via Phabricator via cfe-commits

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

Lots of features built on top of ASTs require getting back to the path
of the TU and they used lossy conversion from file ids using sourcemanager.
This patch preserves the file path passed by the caller inside ParsedAST for
later use.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130690

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp

Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -408,12 +408,7 @@
 
   Expected apply(const Selection &Sel) override {
 const SourceManager &SM = Sel.AST->getSourceManager();
-auto MainFileName =
-getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
-if (!MainFileName)
-  return error("Couldn't get absolute path for main file.");
-
-auto CCFile = getSourceFile(*MainFileName, Sel);
+auto CCFile = getSourceFile(Sel.AST->tuPath(), Sel);
 
 if (!CCFile)
   return error("Couldn't find a suitable implementation file.");
Index: clang-tools-extra/clangd/XRefs.h
===
--- clang-tools-extra/clangd/XRefs.h
+++ clang-tools-extra/clangd/XRefs.h
@@ -20,6 +20,7 @@
 #include "support/Path.h"
 #include "clang/AST/ASTTypeTraits.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 
@@ -59,10 +60,11 @@
 // AST-based resolution does not work, such as comments, strings, and PP
 // disabled regions.
 // (This is for internal use by locateSymbolAt, and is exposed for testing).
-std::vector
-locateSymbolTextually(const SpelledWord &Word, ParsedAST &AST,
-  const SymbolIndex *Index, const std::string &MainFilePath,
-  ASTNodeKind NodeKind);
+std::vector locateSymbolTextually(const SpelledWord &Word,
+ ParsedAST &AST,
+ const SymbolIndex *Index,
+ llvm::StringRef MainFilePath,
+ ASTNodeKind NodeKind);
 
 // Try to find a proximate occurrence of `Word` as an identifier, which can be
 // used to resolve it.
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -484,12 +484,7 @@
 std::vector locateSymbolForType(const ParsedAST &AST,
const QualType &Type) {
   const auto &SM = AST.getSourceManager();
-  auto MainFilePath =
-  getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
-  if (!MainFilePath) {
-elog("Failed to get a path for the main file, so no symbol.");
-return {};
-  }
+  auto MainFilePath = AST.tuPath();
 
   // FIXME: this sends unique_ptr to unique_ptr.
   // Likely it would be better to send it to Foo (heuristically) or to both.
@@ -505,7 +500,7 @@
   for (const NamedDecl *D : Decls) {
 D = getPreferredDecl(D);
 
-auto Loc = makeLocation(ASTContext, nameLocation(*D, SM), *MainFilePath);
+auto Loc = makeLocation(ASTContext, nameLocation(*D, SM), MainFilePath);
 if (!Loc)
   continue;
 
@@ -515,7 +510,7 @@
 Results.back().ID = getSymbolID(D);
 if (const NamedDecl *Def = getDefinition(D))
   Results.back().Definition =
-  makeLocation(ASTContext, nameLocation(*Def, SM), *MainFilePath);
+  makeLocation(ASTContext, nameLocation(*Def, SM), MainFilePath);
   }
 
   return Results;
@@ -546,10 +541,11 @@
 
 } // namespace
 
-std::vector
-locateSymbolTextually(const SpelledWord &Word, ParsedAST &AST,
-  const SymbolIndex *Index, const std::string &MainFilePath,
-  ASTNodeKind NodeKind) {
+std::vector locateSymbolTextually(const SpelledWord &Word,
+ ParsedAST &AST,
+ const SymbolIndex *Index,
+ llvm::StringRef MainFilePath,
+ ASTNodeKind NodeKind) {
   // Don't use heuristics if this is a real identifier, or not an
   // identifier.
   // Exception: dependent names, because those may have useful textua

[PATCH] D130041: [clangd] Add decl/def support to SymbolDetails

2022-07-28 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.cpp:1528
   }
-
+  auto MainFilePath =
+  getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM);

dgoldman wrote:
> dgoldman wrote:
> > sammccall wrote:
> > > kadircet wrote:
> > > > let's just pass the TUPath from ClangdServer into the request. (we 
> > > > should do the same for `locateSymbolAt`, but on a separate change)
> > > driveby: @kadircet any reason not to store the TUPath on ParsedAST? We 
> > > already pass it to ParsedAST::build()
> > I think this would simplify things quite a bit - otherwise I also need to 
> > update the DumpSymbol tweak to store/compute the TUPath (or just provide an 
> > empty one). LMK what I should do.
> Also, not sure how this would affect the Windows failure, which currently 
> fails since the decl/definition locations reference 
> "file:///C:/clangd-test/simple.cpp" instead of "test:///simple.cpp"... could 
> add a regex or limit the test to non-Windows as a work around though. 
> "textDocument/publishDiagnostics" also seems to reference that absolute path 
> as well instead of the test:// URI
sent out D130690 for preserving TUPath in ParsedAST.

as for matching output in lit tests, clangd always outputs file uris. so you 
should rather match with a regex (unless testing something platform-specific).
you can use `# CHECK-NEXT:  "uri": "file://{{.*}}/clangd-test/simple.cpp"` 
instead to match output.



Comment at: clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp:29
+
+  const char *DeclMarker = nullptr;
+  const char *DefMarker = nullptr;

dgoldman wrote:
> kadircet wrote:
> > since you're always calling these `decl` or `def`, you can instead check 
> > whether the annotation has any range named `def`/`decl` and expect those 
> > accordingly rather than mentioning them one extra time inside the 
> > testcases. i.e:
> > 
> > case:
> > 
> > ```
> > void $def[[foo]]() {}
> >   int bar() {
> > fo^o();
> >   }
> > ```
> > 
> > check:
> > ```
> > Annotations Test(Case);
> > SymbolDetails Expected;
> > auto Def = Test.ranges("def");
> > auto Decl = Test.ranges("decl");
> > if (!Def.empty()) {
> >   Expected.decl = Expected.def = makelocation ...
> > } else if (!Decl.empty()) {
> >   Expected.decl = makelocation ...
> > }
> > ```
> Thanks, although that won't work for the `// Multiple symbols returned - 
> using overloaded function name` test case. Should I move that one out to to 
> be handled separately?
ah sorry i missed those, nvm this one. no need for splitting those out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130041

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

[PATCH] D127293: [clang-tidy] Ignore other members in a union if any member of it is initialized in cppcoreguidelines-pro-type-member-init

2022-07-28 Thread gehry via Phabricator via cfe-commits

Sockke added a comment.

Friendly ping.


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

https://reviews.llvm.org/D127293

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

[PATCH] D128697: [clang-tidy] Add new check `bugprone-unhandled-exception-at-sto`

2022-07-28 Thread gehry via Phabricator via cfe-commits

Sockke added a comment.

Friendly ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128697

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

[PATCH] D130678: [clang-tools-extra] Refactor to reduce code duplication

2022-07-28 Thread Adrian Vogelsgesang via Phabricator via cfe-commits

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

LGTM

(note that I am rather inexperienced with LLVM, though, so take my approval 
with a grain of salt)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130678

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

[PATCH] D130690: [clangd][NFCI] Store TUPath inside ParsedAST

2022-07-28 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

I want to understand the trade-offs a bit better.

- what are the actual differences between the file paths passed to TU and the 
canonical path in source manager?
- what issues does having these differences result in?

Having some tests that illustrate the difference in a real-world setting would 
be helpful too.

We would still have to go through source manager to produce results for 
anything but the main file.
This means that after this change we effectively add complexity the path 
handling by adding an extra code path for the main file.
Moreover, it's the most common path we return, so we could make any problems 
with the paths from the source manager appear less frequently and, hence, 
harder to notice and diagnose.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130690

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

[PATCH] D129798: [clang-tidy] Rename the make-confusable-table executable

2022-07-28 Thread Martin Storsjö via Phabricator via cfe-commits

mstorsjo added inline comments.



Comment at: clang-tools-extra/clang-tidy/misc/CMakeLists.txt:10
 else()
-  set(make_confusable_table $)
-  set(make_confusable_table_target make-confusable-table)
+  set(make_confusable_table $)
+  set(make_confusable_table_target clang-tidy-confusable-chars-gen)

sammccall wrote:
> (Should the variable names also be updated?
I guess I could do that for consistency too. (Sorry for not noticing your 
comment before pushing!)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129798

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

[PATCH] D127293: [clang-tidy] Ignore other members in a union if any member of it is initialized in cppcoreguidelines-pro-type-member-init

2022-07-28 Thread Nathan James via Phabricator via cfe-commits

njames93 accepted this revision.
njames93 added a comment.

LGTM, You'd need to rebase before landing as the release notes have been reset 
for the 15 release branching.


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

https://reviews.llvm.org/D127293

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

[PATCH] D122768: [Clang][C++20] Support capturing structured bindings in lambdas

2022-07-28 Thread Corentin Jabot via Phabricator via cfe-commits

cor3ntin added a comment.

@aaron.ballman @shafik I'm perfectly happy to add isInitCapture in `ValueDecl` 
if that allows us to make progress on this. Let me know what you think!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122768

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

[PATCH] D130273: [clang][Driver] Handle SPARC -mcpu=native etc.

2022-07-28 Thread Rainer Orth via Phabricator via cfe-commits

ro updated this revision to Diff 448290.
ro marked 3 inline comments as done.
ro edited the summary of this revision.
ro added a comment.

- Incorporate review comments.
- Add testcases.
- Reject `-march`, matching GCC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130273

Files:
  clang/lib/Driver/ToolChains/Arch/Sparc.cpp
  clang/lib/Driver/ToolChains/Arch/Sparc.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/sparc-march.c
  clang/test/Driver/sparc-mcpu.c
  clang/test/Driver/sparc-mtune.c

Index: clang/test/Driver/sparc-mtune.c
===
--- /dev/null
+++ clang/test/Driver/sparc-mtune.c
@@ -0,0 +1,21 @@
+// RUN: %clang -target sparcv9 -### -c %s 2>&1 | FileCheck -check-prefix=SPARCV9 %s
+// SPARCV9: "-cc1"{{.*}} "-triple" "sparcv9"
+
+// RUN: %clang -target sparc64 -### -c %s 2>&1 | FileCheck -check-prefix=SPARC64 %s
+// SPARC64: "-cc1"{{.*}} "-triple" "sparc64"
+
+// RUN: %clang -target sparcv9 -mtune=v9 -### -c %s 2>&1 | FileCheck -check-prefix=SPARCV9_V9 %s
+// SPARCV9_V9: "-cc1"{{.*}} "-triple" "sparcv9"{{.*}} "-tune-cpu" "v9"
+
+// RUN: %clang -target sparcv9 -mtune=ultrasparc -### -c %s 2>&1 | FileCheck -check-prefix=SPARCV9_US %s
+// SPARCV9_US: "-cc1"{{.*}} "-triple" "sparcv9"{{.*}} "-tune-cpu" "ultrasparc"
+
+// RUN: %clang -target sparc -### -c %s 2>&1 | FileCheck -check-prefix=SPARC %s
+// SPARC: "-cc1"{{.*}} "-triple" "sparc"
+
+// RUN: %clang -target sparc -mtune=v9 -### -c %s 2>&1 | FileCheck -check-prefix=SPARC_V9 %s
+// SPARC_V9: "-cc1"{{.*}} "-triple" "sparc"{{.*}} "-tune-cpu" "v9"
+
+// RUN: %clang -target sparc -mtune=ultrasparc -### -c %s 2>&1 | FileCheck -check-prefix=SPARC_US %s
+// SPARC_US: "-cc1"{{.*}} "-triple" "sparc"{{.*}} "-tune-cpu" "ultrasparc"
+
Index: clang/test/Driver/sparc-mcpu.c
===
--- /dev/null
+++ clang/test/Driver/sparc-mcpu.c
@@ -0,0 +1,21 @@
+// RUN: %clang -target sparcv9 -### -c %s 2>&1 | FileCheck -check-prefix=SPARCV9 %s
+// SPARCV9: "-cc1"{{.*}} "-triple" "sparcv9"
+
+// RUN: %clang -target sparc64 -### -c %s 2>&1 | FileCheck -check-prefix=SPARC64 %s
+// SPARC64: "-cc1"{{.*}} "-triple" "sparc64"
+
+// RUN: %clang -target sparcv9 -mcpu=v9 -### -c %s 2>&1 | FileCheck -check-prefix=SPARCV9_V9 %s
+// SPARCV9_V9: "-cc1"{{.*}} "-triple" "sparcv9"{{.*}} "-target-cpu" "v9"
+
+// RUN: %clang -target sparcv9 -mcpu=ultrasparc -### -c %s 2>&1 | FileCheck -check-prefix=SPARCV9_US %s
+// SPARCV9_US: "-cc1"{{.*}} "-triple" "sparcv9"{{.*}} "-target-cpu" "ultrasparc"
+
+// RUN: %clang -target sparc -### -c %s 2>&1 | FileCheck -check-prefix=SPARC %s
+// SPARC: "-cc1"{{.*}} "-triple" "sparc"
+
+// RUN: %clang -target sparc -mcpu=v9 -### -c %s 2>&1 | FileCheck -check-prefix=SPARC_V9 %s
+// SPARC_V9: "-cc1"{{.*}} "-triple" "sparc"{{.*}} "-target-cpu" "v9"
+
+// RUN: %clang -target sparc -mcpu=ultrasparc -### -c %s 2>&1 | FileCheck -check-prefix=SPARC_US %s
+// SPARC_US: "-cc1"{{.*}} "-triple" "sparc"{{.*}} "-target-cpu" "ultrasparc"
+
Index: clang/test/Driver/sparc-march.c
===
--- /dev/null
+++ clang/test/Driver/sparc-march.c
@@ -0,0 +1,4 @@
+// RUN: %clang -target sparcv9 -march=v9 -### -c %s 2>&1 | FileCheck %s
+// RUN: %clang -target sparc64 -march=v9 -### -c %s 2>&1 | FileCheck %s
+// RUN: %clang -target sparc -march=v9 -### -c %s 2>&1 | FileCheck %s
+// CHECK: error: unsupported option '-march=' for target
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -12,6 +12,7 @@
 #include "Arch/M68k.h"
 #include "Arch/Mips.h"
 #include "Arch/PPC.h"
+#include "Arch/Sparc.h"
 #include "Arch/SystemZ.h"
 #include "Arch/VE.h"
 #include "Arch/X86.h"
@@ -431,14 +432,14 @@
 
   case llvm::Triple::bpfel:
   case llvm::Triple::bpfeb:
+if (const Arg *A = Args.getLastArg(options::OPT_mcpu_EQ))
+  return A->getValue();
+return "";
+
   case llvm::Triple::sparc:
   case llvm::Triple::sparcel:
   case llvm::Triple::sparcv9:
-if (const Arg *A = Args.getLastArg(options::OPT_mcpu_EQ))
-  return A->getValue();
-if (T.getArch() == llvm::Triple::sparc && T.isOSSolaris())
-  return "v9";
-return "";
+return sparc::getSparcTargetCPU(D, Args, T);
 
   case llvm::Triple::x86:
   case llvm::Triple::x86_64:
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2205,6 +2205,18 @@
 CmdArgs.push_back("-mfloat-abi");
 CmdArgs.push_back("hard");
   }
+
+  if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_mtune_EQ)) {
+Str

[PATCH] D130701: [clang-tidy] Rename a local cmake variables to match the new tool name. NFC.

2022-07-28 Thread Martin Storsjö via Phabricator via cfe-commits

mstorsjo created this revision.
mstorsjo added a reviewer: sammccall.
Herald added subscribers: carlosgalvezp, xazax.hun, mgorny.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: clang-tools-extra.

This shouldn't have any externally visible effect.

This matches the new name from 18b4a8bcf3553174f770f09528c9bd01c8cebfe7 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130701

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt


Index: clang-tools-extra/clang-tidy/misc/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -7,14 +7,14 @@
   STRING "Host clang-tidy-confusable-chars-gen executable. Saves building if 
cross-compiling.")
 
 if(NOT CLANG_TIDY_CONFUSABLE_CHARS_GEN STREQUAL 
"clang-tidy-confusable-chars-gen")
-  set(make_confusable_table ${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
-  set(make_confusable_table_target ${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
+  set(clang_tidy_confusable_chars_gen ${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
+  set(clang_tidy_confusable_chars_gen_target 
${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
 elseif(LLVM_USE_HOST_TOOLS)
-  build_native_tool(clang-tidy-confusable-chars-gen make_confusable_table)
-  set(make_confusable_table_target "${make_confusable_table}")
+  build_native_tool(clang-tidy-confusable-chars-gen 
clang_tidy_confusable_chars_gen)
+  set(clang_tidy_confusable_chars_gen_target 
"${clang_tidy_confusable_chars_gen}")
 else()
-  set(make_confusable_table $)
-  set(make_confusable_table_target clang-tidy-confusable-chars-gen)
+  set(clang_tidy_confusable_chars_gen 
$)
+  set(clang_tidy_confusable_chars_gen_target clang-tidy-confusable-chars-gen)
 endif()
 
 add_subdirectory(ConfusableTable)
@@ -22,8 +22,8 @@
 
 add_custom_command(
 OUTPUT Confusables.inc
-COMMAND ${make_confusable_table} 
${CMAKE_CURRENT_SOURCE_DIR}/ConfusableTable/confusables.txt 
${CMAKE_CURRENT_BINARY_DIR}/Confusables.inc
-DEPENDS ${make_confusable_table_target} ConfusableTable/confusables.txt)
+COMMAND ${clang_tidy_confusable_chars_gen} 
${CMAKE_CURRENT_SOURCE_DIR}/ConfusableTable/confusables.txt 
${CMAKE_CURRENT_BINARY_DIR}/Confusables.inc
+DEPENDS ${clang_tidy_confusable_chars_gen_target} 
ConfusableTable/confusables.txt)
 
 add_custom_target(genconfusable DEPENDS Confusables.inc)
 


Index: clang-tools-extra/clang-tidy/misc/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -7,14 +7,14 @@
   STRING "Host clang-tidy-confusable-chars-gen executable. Saves building if cross-compiling.")
 
 if(NOT CLANG_TIDY_CONFUSABLE_CHARS_GEN STREQUAL "clang-tidy-confusable-chars-gen")
-  set(make_confusable_table ${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
-  set(make_confusable_table_target ${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
+  set(clang_tidy_confusable_chars_gen ${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
+  set(clang_tidy_confusable_chars_gen_target ${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
 elseif(LLVM_USE_HOST_TOOLS)
-  build_native_tool(clang-tidy-confusable-chars-gen make_confusable_table)
-  set(make_confusable_table_target "${make_confusable_table}")
+  build_native_tool(clang-tidy-confusable-chars-gen clang_tidy_confusable_chars_gen)
+  set(clang_tidy_confusable_chars_gen_target "${clang_tidy_confusable_chars_gen}")
 else()
-  set(make_confusable_table $)
-  set(make_confusable_table_target clang-tidy-confusable-chars-gen)
+  set(clang_tidy_confusable_chars_gen $)
+  set(clang_tidy_confusable_chars_gen_target clang-tidy-confusable-chars-gen)
 endif()
 
 add_subdirectory(ConfusableTable)
@@ -22,8 +22,8 @@
 
 add_custom_command(
 OUTPUT Confusables.inc
-COMMAND ${make_confusable_table} ${CMAKE_CURRENT_SOURCE_DIR}/ConfusableTable/confusables.txt ${CMAKE_CURRENT_BINARY_DIR}/Confusables.inc
-DEPENDS ${make_confusable_table_target} ConfusableTable/confusables.txt)
+COMMAND ${clang_tidy_confusable_chars_gen} ${CMAKE_CURRENT_SOURCE_DIR}/ConfusableTable/confusables.txt ${CMAKE_CURRENT_BINARY_DIR}/Confusables.inc
+DEPENDS ${clang_tidy_confusable_chars_gen_target} ConfusableTable/confusables.txt)
 
 add_custom_target(genconfusable DEPENDS Confusables.inc)
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129798: [clang-tidy] Rename the make-confusable-table executable

2022-07-28 Thread Nico Weber via Phabricator via cfe-commits

thakis added inline comments.



Comment at: 
llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/misc/ConfusableTable/BUILD.gn:1
-executable("make-confusable-table") {
+executable("clang-tidy-confusable-chars-gen") {
   deps = [ "//llvm/lib/Support" ]

Thanks for attempting to update the GN build, but if you don't test it it's 
less confusing to not touch it at all. I fixed it in 
dd428a571c69621d5b6eb2e0e3ce5497c304fb2c and I was confused for a bit since 
this file here was already updated.

(I suppose the GN build wouldn't have broken if you hadn't changed this file 
and I wouldn't have noticed and not renamed the executable there for a while…)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129798

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

[PATCH] D130055: Clang extensions yolo, woot & kaboom

2022-07-28 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added subscribers: aaronpuchert, delesley.
aaron.ballman added a comment.

In D130055#3683279 , @beanz wrote:

> In D130055#3683173 , @aaron.ballman 
> wrote:
>
>> Are there circumstances where we cannot "simply" infer this from the 
>> constructor itself?
>
> I think this gets tricky in cases where objects may have valid 
> "uninitialized" state. Like a tagged union that hasn't been set might set its 
> tag value to "uninitialized" and zero its storage, which would make it look 
> like all the members are initialized to the compiler, they just happen to be 
> initialized to known sentinel values.

Ah, sentinel values in general all have that same general property, good point!

>> (After instantiation) we know the class hierarchy, we know the data members, 
>> and we know the ctor init list/in-class initializers, so it seems like we 
>> should be able to recursively mark a constructor as yolo for the user rather 
>> than making them write it themselves. It seems like inferring this would 
>> reduce false positives and false negatives (in the case the user marks the 
>> constructor incorrectly or the code gets out of sync with the marking), and 
>> we might only need this attribute in very rare circumstances (or not at all, 
>> as a user-facing attribute). Is that an approach you're considering?
>
> I hadn't thought at all about automated propagation.

My experiences with thread safety analysis annotations was that inference (or 
even a tool to automatically slap the right annotations explicitly on to 
declarations) was a huge usability win for users. This particular kind of 
diagnostic tends to be somewhat "viral" in that starting to use the annotations 
only helps if everything in some subset of your project is annotated. You can 
start from the leaf nodes in your types to get some benefit, but the most 
benefit tends to come from composition of these leaf types into more complex 
types. Reducing the number of annotations the user has to write makes this 
transition significantly easier if possible to accomplish with good fidelity.

> One of the other use cases I was thinking about was `std::optional`, where 
> you could specify that initialization to `std::nullopt` is `yolo`, and then 
> `get` becomes `kaboom`.
>
> With these annotations we can conservatively diagnose cases where an 
> optional's value is accessed when uninitialized.
>
>> Fully? Partially? I'm trying to decide whether this is only useful for 
>> functions named `init()` and `clear()` and the like, or whether this is 
>> useful for functions like `setWhatever()` where you can have a partially 
>> constructed object that is not fully initialized by any one member function.
>
> I had also thought about this too. As implemented in this patch `woot` 
> implies fully initialized, but there could be some interesting complex 
> initializations. I could imagine builder patterns where given code like 
> `Builder::Create().X().Y().Z()`, you might want `Y` to only be callable after 
> `X` and `Z` might be the required final initializer.

I was thinking about our own AST nodes where, for example, you deserialize an 
AST by creating an empty node and then filling out its structure piecemeal. I 
think partial construction is a pretty important concept to consider early in 
the design given its prevalence as a reasonable implementation strategy.

>> Same question here about fully vs partially initialized. e.g., where you 
>> have a partially constructed object and you can only call `getWhatever()` 
>> after `setWhatever()` has been called.
>
> I'm wondering if there could be a generalization of the attribute like:
>
>   class TaggedValue {
> enum Kind {
>   Uninitialized = 0,
>   Integer,
>   Float
> };
> Kind VK = Uninitialized;
>   
> union {
>   int I;
>   float F;
> };
>   public:
> 
> [[clang::yolo]]
> TaggedValue() = default;
> 
> TaggedValue(TaggedValue&) = default;
>   
> void hasValue() { return VK == Uninitialized; } // always safe
>   
> [[clang::woot("FloatSet"]] // Marks as safe for functions with matching 
> kaboom arguments
> void set(float V) {
>   VK= Float;
>   F = V;
> }
>   
> [[clang::woot("IntSet")]] // Marks as safe for functions with matching 
> kaboom arguments
> void set(int V) {
>   VK= Integer;
>   I = V;
> }
>   
>   [[clang::woot]] // Marks as safe for all kaboom functions (because I'm 
> sad)
>  void zero() {
>VK= Integer;
>I = 0;
>  }
>   
> [[clang::kaboom("FloatSet"]]
> operator float() {
>   return F;
> }
>   
> [[clang::kaboom("IntSet")]]
> operator int() {
>   return I;
> }
>   };
>
> This does get into some more complex questions of whether `woot` would change 
> or append status, and I can see arguments for both so we might need 
> `appending_woot` too...

T

[PATCH] D130055: Clang extensions yolo, woot & kaboom

2022-07-28 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D130055#3683822 , @jkorous wrote:

> In D130055#3683173 , @aaron.ballman 
> wrote:
>
>> Are there circumstances where we cannot "simply" infer this from the 
>> constructor itself? (After instantiation) we know the class hierarchy, we 
>> know the data members, and we know the ctor init list/in-class initializers, 
>> so it seems like we should be able to recursively mark a constructor as yolo 
>> for the user rather than making them write it themselves.
>
> The init list can be in a different translation unit (TU) which means we 
> can't compute all properties just from the code in current TU. (Unless we 
> decide to pessimistically consider such unanalyzable constructors as 
> non-initializing.)

I was imagining that this analysis is most powerful when done as a cross-TU 
static analysis pass. However, having markings for when there's less context is 
definitely a benefit, so that's a great point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130055

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

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

2022-07-28 Thread Hans Wennborg via Phabricator via cfe-commits

hans added a comment.

Here's an example where I think this regressed a Clang diagnostic. Consider:

  template  struct Template { Template(int x) {} };
  
  struct S1 {
struct Foo;
typedef Template Typedef;
  };
  
  struct S2 {
struct Foo;
typedef Template Typedef;
  };
  
  typedef S1::Typedef Bar;
  Bar f;

before this change, Clang would say:

  /tmp/a.cc:14:5: error: no matching constructor for initialization of 'Bar' 
(aka 'Template')
  Bar f;
  ^

however, after this change it says:

  /tmp/a.cc:14:5: error: no matching constructor for initialization of 'Bar' 
(aka 'Template')
  Bar f;
  ^

The problem is that just based on `Template` it's not clear whether it's 
`S1::Foo` or `S2::Foo` that's referred to.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112374

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

[PATCH] D130224: [Clang][Attribute] Introduce maybe_undef attribute for function arguments which accepts undef values

2022-07-28 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

FWIW, precommit CI has some relevant failures for a change:

   TEST 'Clang :: CodeGen/attr-maybeundef-template.cpp' 
FAILED 
  Script:
  --
  : 'RUN: at line 1';   
c:\ws\w4\llvm-project\premerge-checks\build\bin\clang.exe -cc1 
-internal-isystem 
c:\ws\w4\llvm-project\premerge-checks\build\lib\clang\16.0.0\include 
-nostdsysteminc -no-opaque-pointers -emit-llvm 
C:\ws\w4\llvm-project\premerge-checks\clang\test\CodeGen\attr-maybeundef-template.cpp
 -o - | c:\ws\w4\llvm-project\premerge-checks\build\bin\filecheck.exe 
C:\ws\w4\llvm-project\premerge-checks\clang\test\CodeGen\attr-maybeundef-template.cpp
  --
  Exit Code: 1
  
  Command Output (stdout):
  --
  $ ":" "RUN: at line 1"
  $ "c:\ws\w4\llvm-project\premerge-checks\build\bin\clang.exe" "-cc1" 
"-internal-isystem" 
"c:\ws\w4\llvm-project\premerge-checks\build\lib\clang\16.0.0\include" 
"-nostdsysteminc" "-no-opaque-pointers" "-emit-llvm" 
"C:\ws\w4\llvm-project\premerge-checks\clang\test\CodeGen\attr-maybeundef-template.cpp"
 "-o" "-"
  $ "c:\ws\w4\llvm-project\premerge-checks\build\bin\filecheck.exe" 
"C:\ws\w4\llvm-project\premerge-checks\clang\test\CodeGen\attr-maybeundef-template.cpp"
  # command stderr:
  
C:\ws\w4\llvm-project\premerge-checks\clang\test\CodeGen\attr-maybeundef-template.cpp:3:11:
 error: CHECK: expected string not found in input
  // CHECK: define weak_odr void @_Z5test4IfEvT_(float noundef [[TMP1:%.*]])
^
  :1:1: note: scanning from here
  ; ModuleID = 
'C:\ws\w4\llvm-project\premerge-checks\clang\test\CodeGen\attr-maybeundef-template.cpp'
  ^
  :11:14: note: possible intended match here
  define weak_odr dso_local void @"??$test4@M@@YAXM@Z"(float noundef %arg) #0 
comdat {
   ^
  
  Input file: 
  Check file: 
C:\ws\w4\llvm-project\premerge-checks\clang\test\CodeGen\attr-maybeundef-template.cpp
  
  -dump-input=help explains the following input dump.
  
  Input was:
  <<
 1: ; ModuleID = 
'C:\ws\w4\llvm-project\premerge-checks\clang\test\CodeGen\attr-maybeundef-template.cpp'
 
  check:3'0 
X
 error: no match found
 2: source_filename = 
"C:\\ws\\w4\\llvm-project\\premerge-checks\\clang\\test\\CodeGen\\attr-maybeundef-template.cpp"
 
  check:3'0 
~~
 3: target datalayout = 
"e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" 
  check:3'0 
~
 4: target triple = "x86_64-pc-windows-msvc" 
  check:3'0 ~
 5:  
  check:3'0 ~
 6: $"??$test4@M@@YAXM@Z" = comdat any 
  check:3'0 ~~~
 7:  
  check:3'0 ~
 8: $"??$test4@H@@YAXH@Z" = comdat any 
  check:3'0 ~~~
 9:  
  check:3'0 ~
10: ; Function Attrs: mustprogress noinline nounwind optnone 
  check:3'0 ~
11: define weak_odr dso_local void @"??$test4@M@@YAXM@Z"(float 
noundef %arg) #0 comdat { 
  check:3'0 
~
  check:3'1  ?  
  possible intended match
12: entry: 
  check:3'0 ~~~
13:  %arg.addr = alloca float, align 4 
  check:3'0 ~~~
14:  store float %arg, float* %arg.addr, align 4 
  check:3'0 ~
15:  ret void 
  check:3'0 ~~
16: } 
  check:3'0 ~~
 .
 .
 .
  >>
  
  error: command failed with exit status: 1
  
  --
  
  
  FAIL: Clang :: CodeGen/attr-maybeundef.c (4048 of 15750)
   TEST 'Clang :: CodeGen/attr-maybeundef.c' FAILED 

  Script:
  --
  : 'RUN: at line 1';   
c:\ws\w4\llvm-project\premerge-checks\build\bin\clang.exe -cc1 
-internal-isystem 
c:\ws\w4\llvm-project\premerge-checks\build\lib\clang\16.0.0\include 
-nostdsysteminc -no-opaque-pointers -emit-llvm 
C:\ws\w4\llvm-project\premerge-checks\clang\test\CodeGen\attr-maybeundef.c -o - 
| c:\ws\w4\llvm-project\premerge-checks\build\bin\filecheck.exe 
C:\ws\w4\llvm-project\premerge-checks\clang\test\CodeGen\attr-maybeundef.c
  --
  Exit Code: 1
  
  Command Output (stdout):
  --
  $ ":" "RUN: at line 1"
  $ "c:\ws\w4\llvm-project\premerge-checks\build\bin\clang.exe" "-cc1" 
"-internal-isystem" 
"c:\ws\w4\llvm-project\premerge-checks\build\lib\clang\16.0.0\

[PATCH] D130510: Missing tautological compare warnings due to unary operators

2022-07-28 Thread 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.

LGTM! I'll make some editorial corrections for grammar to the release note when 
I land.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130510

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

[clang] 0cc3c18 - Missing tautological compare warnings due to unary operators

2022-07-28 Thread Aaron Ballman via cfe-commits


Author: Muhammad Usman Shahid
Date: 2022-07-28T07:45:28-04:00
New Revision: 0cc3c184c784d5f0d55de8ad0a9876acd149

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

LOG: Missing tautological compare warnings due to unary operators

The patch mainly focuses on the lack of warnings for
-Wtautological-compare. It works fine for positive numbers but doesn't
for negative numbers. This is because the warning explicitly checks for
an IntegerLiteral AST node, but -1 is represented by a UnaryOperator
with an IntegerLiteral sub-Expr.

For the below code we have warnings:

if (0 == (5 | x)) {}

but not for

if (0 == (-5 | x)) {}

This patch changes the analysis to not look at the AST node directly to
see if it is an IntegerLiteral, but instead attempts to evaluate the
expression to see if it is an integer constant expression. This handles
unary negation signs, but also handles all the other possible operators
as well.

Fixes #42918
Differential Revision: https://reviews.llvm.org/D130510

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Analysis/CFG.cpp
clang/test/Sema/warn-bitwise-compare.c
clang/test/SemaCXX/warn-unreachable.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 4f5e71e0ba80..5d5e709ba4a1 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -49,6 +49,10 @@ Major New Features
 
 Bug Fixes
 -
+- ``-Wtautological-compare`` missed warnings for tautological comparisons
+  involving a negative integer literal. This fixes
+  `Issue 42918 `_.
+
 
 Improvements to Clang's diagnostics
 ^^^

diff  --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 84178ff488a5..30617e48fde4 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -964,43 +964,44 @@ class CFGBuilder {
 const Expr *LHSExpr = B->getLHS()->IgnoreParens();
 const Expr *RHSExpr = B->getRHS()->IgnoreParens();
 
-const IntegerLiteral *IntLiteral = dyn_cast(LHSExpr);
-const Expr *BoolExpr = RHSExpr;
+const Expr *BoolExpr = nullptr; // To store the expression.
+Expr::EvalResult IntExprResult; // If integer literal then will save value.
 
-if (!IntLiteral) {
-  IntLiteral = dyn_cast(RHSExpr);
+if (LHSExpr->EvaluateAsInt(IntExprResult, *Context))
+  BoolExpr = RHSExpr;
+else if (RHSExpr->EvaluateAsInt(IntExprResult, *Context))
   BoolExpr = LHSExpr;
-}
-
-if (!IntLiteral)
+else
   return TryResult();
 
+llvm::APInt L1 = IntExprResult.Val.getInt();
+
 const BinaryOperator *BitOp = dyn_cast(BoolExpr);
-if (BitOp && (BitOp->getOpcode() == BO_And ||
-  BitOp->getOpcode() == BO_Or)) {
-  const Expr *LHSExpr2 = BitOp->getLHS()->IgnoreParens();
-  const Expr *RHSExpr2 = BitOp->getRHS()->IgnoreParens();
+if (BitOp &&
+(BitOp->getOpcode() == BO_And || BitOp->getOpcode() == BO_Or)) {
 
-  const IntegerLiteral *IntLiteral2 = dyn_cast(LHSExpr2);
+  // If integer literal in expression identified then will save value.
+  Expr::EvalResult IntExprResult2;
 
-  if (!IntLiteral2)
-IntLiteral2 = dyn_cast(RHSExpr2);
+  if (BitOp->getLHS()->EvaluateAsInt(IntExprResult2, *Context))
+; // LHS is a constant expression.
+  else if (BitOp->getRHS()->EvaluateAsInt(IntExprResult2, *Context))
+; // RHS is a constant expression.
+  else
+return TryResult(); // Neither is a constant expression, bail out.
 
-  if (!IntLiteral2)
-return TryResult();
+  llvm::APInt L2 = IntExprResult2.Val.getInt();
 
-  llvm::APInt L1 = IntLiteral->getValue();
-  llvm::APInt L2 = IntLiteral2->getValue();
   if ((BitOp->getOpcode() == BO_And && (L2 & L1) != L1) ||
-  (BitOp->getOpcode() == BO_Or  && (L2 | L1) != L1)) {
+  (BitOp->getOpcode() == BO_Or && (L2 | L1) != L1)) {
 if (BuildOpts.Observer)
   BuildOpts.Observer->compareBitwiseEquality(B,
  B->getOpcode() != BO_EQ);
 TryResult(B->getOpcode() != BO_EQ);
   }
 } else if (BoolExpr->isKnownToHaveBooleanValue()) {
-  llvm::APInt IntValue = IntLiteral->getValue();
-  if ((IntValue == 1) || (IntValue == 0)) {
+  llvm::APInt IntValue = IntExprResult.Val.getInt(); // Getting the value.
+  if ((L1 == 1) || (L1 == 0)) {
 return TryResult();
   }
   return TryResult(B->getOpcode() != BO_EQ);

diff  --git a/clang/test/Sema/warn-bitwise-compare.c 
b/clang/test/Sema/warn-bitwise-compare.c
index 08a8b084fe79..b96881fe296e 100644
--- a/clang/test/Sema/warn-bitwise-compare.c
+++ b/clang/test/Sema/

[PATCH] D130510: Missing tautological compare warnings due to unary operators

2022-07-28 Thread Aaron Ballman 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 rG0cc3c184c784: Missing tautological compare warnings due to 
unary operators (authored by Codesbyusman, committed by aaron.ballman).

Changed prior to commit:
  https://reviews.llvm.org/D130510?vs=448141&id=448302#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130510

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Analysis/CFG.cpp
  clang/test/Sema/warn-bitwise-compare.c
  clang/test/SemaCXX/warn-unreachable.cpp

Index: clang/test/SemaCXX/warn-unreachable.cpp
===
--- clang/test/SemaCXX/warn-unreachable.cpp
+++ clang/test/SemaCXX/warn-unreachable.cpp
@@ -396,15 +396,16 @@
   if (y == -1 && y != -1)  // expected-note {{silence}}
 calledFun();// expected-warning {{will never be executed}}
 
-  // TODO: Extend warning to the following code:
-  if (x < -1)
-calledFun();
-  if (x == -1)
-calledFun();
+  if (x == -1)   // expected-note {{silence}}
+calledFun(); // expected-warning {{will never be executed}}
 
-  if (x != -1)
+  if (x != -1)   // expected-note {{silence}}
 calledFun();
   else
+calledFun(); // expected-warning {{will never be executed}}
+
+  // TODO: Extend warning to the following code:
+  if (x < -1)
 calledFun();
   if (-1 > x)
 calledFun();
Index: clang/test/Sema/warn-bitwise-compare.c
===
--- clang/test/Sema/warn-bitwise-compare.c
+++ clang/test/Sema/warn-bitwise-compare.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wall -Wno-unused %s
 
 #define mydefine 2
+#define mydefine2 -2
 
 enum {
   ZERO,
@@ -11,29 +12,67 @@
 void f(int x) {
   if ((8 & x) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
   if ((x & 8) == 4) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+  if ((-8 & x) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+  if ((x & -8) == 4) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+
   if ((x & 8) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
   if ((2 & x) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+  if ((x & -8) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+  if ((-2 & x) != 3) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+
   if ((x | 4) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
   if ((x | 3) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
   if ((5 | x) != 3) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+
+  if ((x | -4) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+  if ((x | -3) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+  if ((-5 | x) != 3) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+
+
   if ((x & 0x15) == 0x13) {} // expected-warning {{bitwise comparison always evaluates to false}}
+  if ((x & 0xFFEB) == 0x13) {} // expected-warning {{bitwise comparison always evaluates to false}}
+
   if ((0x23 | x) == 0x155){} // expected-warning {{bitwise comparison always evaluates to false}}
+  if ((0xFFDD | x) == 0x155){} // expected-warning {{bitwise comparison always evaluates to false}}
 
   if (!!((8 & x) == 3)) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+  if (!!((-8 & x) == 3)) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+
   int y = ((8 & x) == 3) ? 1 : 2;  // expected-warning {{bitwise comparison always evaluates to false}}
+  y = ((-8 & x) == 3) ? 1 : 2;  // expected-warning {{bitwise comparison always evaluates to false}}
+  y = ((3 | x) != 5) ? 1 : 2;  // expected-warning {{bitwise comparison always evaluates to true}}
+  y = ((-3 | x) != 5) ? 1 : 2;  // expected-warning {{bitwise comparison always evaluates to true}}
 
   if ((x & 8) == 8) {}
   if ((x & 8) != 8) {}
   if ((x | 4) == 4) {}
   if ((x | 4) != 4) {}
 
+  if ((-2 & x) != 4) {}
+  if ((x & -8) == -8) {}
+  if ((x & -8) != -8) {}
+  if ((x | -4) == -4) {}
+  if ((x | -4) != -4) {}
+
+
   if ((x & 9) == 8) {}
   if ((x & 9) != 8) {}
   if ((x | 4) == 5) {}
   if ((x | 4) != 5) {}
 
+  if ((x & -9) == -10) {}
+  if ((x & -9) != -10) {}
+  if ((x | -4) == -3) {}
+  if ((x | -4) != -3) {}
+
+  if ((x^0) == 0) {}
+
   if ((x & mydefine) == 8) {}
   if ((x | mydefine) == 4) {}
+
+  if ((x & mydefine2) == 8) {}
+  if ((x | mydefine2) == 4) {}
+
 }
 
 void g(int x) {
Index: clang/lib/Analysis/CFG.cpp
===
--- clang/lib/Analysis/CFG.cpp
+++ clan

[PATCH] D130510: Missing tautological compare warnings due to unary operators

2022-07-28 Thread Muhammad Usman Shahid via Phabricator via cfe-commits

Codesbyusman added a comment.

In D130510#3684740 , @aaron.ballman 
wrote:

> LGTM! I'll make some editorial corrections for grammar to the release note 
> when I land.

Thank you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130510

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

[PATCH] D129446: [clang][driver] Find Apple default SDK path

2022-07-28 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits

dexonsmith added a subscriber: arphaman.
dexonsmith added a comment.

I’m not at Apple anymore, but this is a long-standing platform decision that is 
intentional and seems unlikely to change. Note that `/usr/bin/clang` isn’t 
really clang, but a tool called `xcrun` which adds environment variables to 
help find a default SDK before executing `/path/to/toolchain/usr/bin/clang`.

When the driver is called directly it expects the build system to provide the 
appropriate SDK. There are situations where it could cause a problem for the 
driver to hunt around tor arbitrary SDKs.

If you still want to pursue this, @arphaman might be able to help find someone 
in a position to consider the impact / policy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129446

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

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

2022-07-28 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov added a comment.

In D112374#3684722 , @hans wrote:

> The problem is that just based on `Template` it's not clear whether it's 
> `S1::Foo` or `S2::Foo` that's referred to.

The logic in the diagnostic here ended up doing a silent single step desugar 
through a typedef in the 'aka'.

I think the danger of doing that as you exemplify is that, if you don't know 
what context the printed type belongs to, then it's hard to make sense of the 
meaning of what was written.

If we have to step through a sugar node which carries context, like type 
aliases / using types and such, perhaps we should go straight for the canonical 
type instead.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112374

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

[clang] 72ea1a7 - [ORC] Fix weak hidden symbols failure on PPC with runtimedyld

2022-07-28 Thread Sunho Kim via cfe-commits


Author: Sunho Kim
Date: 2022-07-28T21:12:25+09:00
New Revision: 72ea1a721e005f29c6fea4a684807a68abd93c39

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

LOG: [ORC] Fix weak hidden symbols failure on PPC with runtimedyld

Fix "JIT session error: Symbols not found: [ DW.ref.__gxx_personality_v0 ] 
error" which happens when trying to use exceptions on ppc linux. To do this, it 
expands AutoClaimSymbols option in RTDyldObjectLinkingLayer to also claim weak 
symbols before they are tried to be resovled. In ppc linux, DW.ref symbols is 
emitted as weak hidden symbols in the later stage of MC pipeline. This means 
when using IRLayer (i.e. LLJIT), IRLayer will not claim responsibility for such 
symbols and RuntimeDyld will skip defining this symbol even though it couldn't 
resolve corresponding external symbol.

Reviewed By: sgraenitz

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

Added: 


Modified: 
clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp

Removed: 




diff  --git 
a/clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp 
b/clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
index 75928d912dd4..a1dbcfa3410f 100644
--- a/clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
+++ b/clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
@@ -98,11 +98,6 @@ extern "C" int throw_exception() {
   // FIXME: Re-enable the excluded target triples.
   const clang::CompilerInstance *CI = Interp->getCompilerInstance();
   const llvm::Triple &Triple = CI->getASTContext().getTargetInfo().getTriple();
-  // FIXME: PPC fails due to `Symbols not found: [DW.ref.__gxx_personality_v0]`
-  // The current understanding is that the JIT should emit this symbol if it 
was
-  // not (eg. the way passing clang -fPIC does it).
-  if (Triple.isPPC())
-return;
 
   // FIXME: ARM fails due to `Not implemented relocation type!`
   if (Triple.isARM())

diff  --git a/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp 
b/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
index 1926ef1ecc72..17ad0a2170f9 100644
--- a/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
@@ -746,6 +746,11 @@ LLJIT::createObjectLinkingLayer(LLJITBuilderState &S, 
ExecutionSession &ES) {
 Layer->setAutoClaimResponsibilityForObjectSymbols(true);
   }
 
+  if (S.JTMB->getTargetTriple().isOSBinFormatELF() &&
+  (S.JTMB->getTargetTriple().getArch() == Triple::ArchType::ppc64 ||
+   S.JTMB->getTargetTriple().getArch() == Triple::ArchType::ppc64le))
+Layer->setAutoClaimResponsibilityForObjectSymbols(true);
+
   // FIXME: Explicit conversion to std::unique_ptr added to 
silence
   //errors from some GCC / libstdc++ bots. Remove this conversion (i.e.
   //just return ObjLinkingLayer) once those bots are upgraded.

diff  --git a/llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp 
b/llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp
index 27044f66a55d..5d1c9afc560a 100644
--- a/llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp
@@ -108,6 +108,7 @@ void RTDyldObjectLinkingLayer::emit(
   // filter these later.
   auto InternalSymbols = std::make_shared>();
   {
+SymbolFlagsMap ExtraSymbolsToClaim;
 for (auto &Sym : (*Obj)->symbols()) {
 
   // Skip file symbols.
@@ -128,6 +129,33 @@ void RTDyldObjectLinkingLayer::emit(
 return;
   }
 
+  // Try to claim responsibility of weak symbols
+  // if AutoClaimObjectSymbols flag is set.
+  if (AutoClaimObjectSymbols &&
+  (*SymFlagsOrErr & object::BasicSymbolRef::SF_Weak)) {
+auto SymName = Sym.getName();
+if (!SymName) {
+  ES.reportError(SymName.takeError());
+  R->failMaterialization();
+  return;
+}
+
+// Already included in responsibility set, skip it
+SymbolStringPtr SymbolName = ES.intern(*SymName);
+if (R->getSymbols().count(SymbolName))
+  continue;
+
+auto SymFlags = JITSymbolFlags::fromObjectSymbol(Sym);
+if (!SymFlags) {
+  ES.reportError(SymFlags.takeError());
+  R->failMaterialization();
+  return;
+}
+
+ExtraSymbolsToClaim[SymbolName] = *SymFlags;
+continue;
+  }
+
   // Don't include symbols that aren't global.
   if (!(*SymFlagsOrErr & object::BasicSymbolRef::SF_Global)) {
 if (auto SymName = Sym.getName())
@@ -139,6 +167,13 @@ void RTDyldObjectLinkingLayer::emit(
 }
   }
 }
+
+if (!ExtraSymbolsToClaim.empty()) {
+  if (au

[PATCH] D129175: [ORC] Fix weak hidden symbols failure on PPC with runtimedyld

2022-07-28 Thread Sunho Kim 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 rG72ea1a721e00: [ORC] Fix weak hidden symbols failure on PPC 
with runtimedyld (authored by sunho).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129175

Files:
  clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
  llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
  llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp


Index: llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp
===
--- llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp
+++ llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp
@@ -108,6 +108,7 @@
   // filter these later.
   auto InternalSymbols = std::make_shared>();
   {
+SymbolFlagsMap ExtraSymbolsToClaim;
 for (auto &Sym : (*Obj)->symbols()) {
 
   // Skip file symbols.
@@ -128,6 +129,33 @@
 return;
   }
 
+  // Try to claim responsibility of weak symbols
+  // if AutoClaimObjectSymbols flag is set.
+  if (AutoClaimObjectSymbols &&
+  (*SymFlagsOrErr & object::BasicSymbolRef::SF_Weak)) {
+auto SymName = Sym.getName();
+if (!SymName) {
+  ES.reportError(SymName.takeError());
+  R->failMaterialization();
+  return;
+}
+
+// Already included in responsibility set, skip it
+SymbolStringPtr SymbolName = ES.intern(*SymName);
+if (R->getSymbols().count(SymbolName))
+  continue;
+
+auto SymFlags = JITSymbolFlags::fromObjectSymbol(Sym);
+if (!SymFlags) {
+  ES.reportError(SymFlags.takeError());
+  R->failMaterialization();
+  return;
+}
+
+ExtraSymbolsToClaim[SymbolName] = *SymFlags;
+continue;
+  }
+
   // Don't include symbols that aren't global.
   if (!(*SymFlagsOrErr & object::BasicSymbolRef::SF_Global)) {
 if (auto SymName = Sym.getName())
@@ -139,6 +167,13 @@
 }
   }
 }
+
+if (!ExtraSymbolsToClaim.empty()) {
+  if (auto Err = R->defineMaterializing(ExtraSymbolsToClaim)) {
+ES.reportError(std::move(Err));
+R->failMaterialization();
+  }
+}
   }
 
   auto MemMgr = GetMemoryManager();
Index: llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
===
--- llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
+++ llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
@@ -746,6 +746,11 @@
 Layer->setAutoClaimResponsibilityForObjectSymbols(true);
   }
 
+  if (S.JTMB->getTargetTriple().isOSBinFormatELF() &&
+  (S.JTMB->getTargetTriple().getArch() == Triple::ArchType::ppc64 ||
+   S.JTMB->getTargetTriple().getArch() == Triple::ArchType::ppc64le))
+Layer->setAutoClaimResponsibilityForObjectSymbols(true);
+
   // FIXME: Explicit conversion to std::unique_ptr added to 
silence
   //errors from some GCC / libstdc++ bots. Remove this conversion (i.e.
   //just return ObjLinkingLayer) once those bots are upgraded.
Index: clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
===
--- clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
+++ clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
@@ -98,11 +98,6 @@
   // FIXME: Re-enable the excluded target triples.
   const clang::CompilerInstance *CI = Interp->getCompilerInstance();
   const llvm::Triple &Triple = CI->getASTContext().getTargetInfo().getTriple();
-  // FIXME: PPC fails due to `Symbols not found: [DW.ref.__gxx_personality_v0]`
-  // The current understanding is that the JIT should emit this symbol if it 
was
-  // not (eg. the way passing clang -fPIC does it).
-  if (Triple.isPPC())
-return;
 
   // FIXME: ARM fails due to `Not implemented relocation type!`
   if (Triple.isARM())


Index: llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp
===
--- llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp
+++ llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp
@@ -108,6 +108,7 @@
   // filter these later.
   auto InternalSymbols = std::make_shared>();
   {
+SymbolFlagsMap ExtraSymbolsToClaim;
 for (auto &Sym : (*Obj)->symbols()) {
 
   // Skip file symbols.
@@ -128,6 +129,33 @@
 return;
   }
 
+  // Try to claim responsibility of weak symbols
+  // if AutoClaimObjectSymbols flag is set.
+  if (AutoClaimObjectSymbols &&
+  (*SymFlagsOrErr & object::BasicSymbolRef::SF_Weak)) {
+auto SymName = Sym.getName();
+if (!SymName) {
+  ES.reportError(SymName.takeError());
+  R->failMateriali

[PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

2022-07-28 Thread Sebastian Neubauer via Phabricator via cfe-commits

sebastian-ne added inline comments.



Comment at: clang/CMakeLists.txt:100
   set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin)
-  set(LLVM_LIBRARY_OUTPUT_INTDIR 
${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib${LLVM_LIBDIR_SUFFIX})
+  set(LLVM_LIBRARY_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib)
   if(WIN32 OR CYGWIN)

Ericson2314 wrote:
> sebastian-ne wrote:
> > Just to check if your intention aligns with my understanding, removing the 
> > suffix here is fine because the destination is in the binary dir and not 
> > the final install destination?
> Yes exactly.
> 
> I really should write up the "rules" that I've (a) discovered from reading 
> (b) invented somewhere. Any idea where?
I think the commit message is a good place.



Comment at: llvm/CMakeLists.txt:59-60
+if (NOT DEFINED CMAKE_INSTALL_LIBDIR AND DEFINED LLVM_LIBDIR_SUFFIX)
+  message(WARNING "\"LLVM_LIBDIR_SUFFIX\" is deprecated. "
+  "Please set \"CMAKE_INSTALL_LIBDIR\" directly instead.")
+  set(CMAKE_INSTALL_LIBDIR "lib${LLVM_LIBDIR_SUFFIX}")

There’s also `message(DEPRECATION …)` :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130586

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

[clang] 3cc3be8 - [clang-repl] Add host exception support check utility flag.

2022-07-28 Thread Sunho Kim via cfe-commits


Author: Sunho Kim
Date: 2022-07-28T21:14:58+09:00
New Revision: 3cc3be8fa471c69ac7cf9c071554a14020226158

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

LOG: [clang-repl] Add host exception support check utility flag.

Add host exception support check utility flag. This is needed to not run tests 
that require exception support in few buildbots that lacks related symbols for 
some reason.

Reviewed By: lhames

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

Added: 
clang/test/Interpreter/simple-exception.cpp

Modified: 
clang/test/lit.cfg.py
clang/tools/clang-repl/ClangRepl.cpp

Removed: 




diff  --git a/clang/test/Interpreter/simple-exception.cpp 
b/clang/test/Interpreter/simple-exception.cpp
new file mode 100644
index 0..c56fbbb417f08
--- /dev/null
+++ b/clang/test/Interpreter/simple-exception.cpp
@@ -0,0 +1,14 @@
+// clang-format off
+// REQUIRES: host-supports-jit, host-supports-exception 
+// UNSUPPORTED: system-aix
+// XFAIL: arm, arm64-apple, windows-msvc, windows-gnu
+// RUN: cat %s | clang-repl | FileCheck %s
+extern "C" int printf(const char *, ...);
+
+int f() { throw "Simple exception"; return 0; }
+int checkException() { try { printf("Running f()\n"); f(); } catch (const char 
*e) { printf("%s\n", e); } return 0; }
+auto r1 = checkException();
+// CHECK: Running f()
+// CHECK-NEXT: Simple exception
+
+%quit
\ No newline at end of file

diff  --git a/clang/test/lit.cfg.py b/clang/test/lit.cfg.py
index d95ea5d2da198..01da76de86a15 100644
--- a/clang/test/lit.cfg.py
+++ b/clang/test/lit.cfg.py
@@ -70,7 +70,7 @@
 if config.clang_examples:
 config.available_features.add('examples')
 
-def have_host_jit_support():
+def have_host_jit_feature_support(feature_name):
 clang_repl_exe = lit.util.which('clang-repl', config.clang_tools_dir)
 
 if not clang_repl_exe:
@@ -79,7 +79,7 @@ def have_host_jit_support():
 
 try:
 clang_repl_cmd = subprocess.Popen(
-[clang_repl_exe, '--host-supports-jit'], stdout=subprocess.PIPE)
+[clang_repl_exe, '--host-supports-' + feature_name], 
stdout=subprocess.PIPE)
 except OSError:
 print('could not exec clang-repl')
 return False
@@ -89,9 +89,12 @@ def have_host_jit_support():
 
 return 'true' in clang_repl_out
 
-if have_host_jit_support():
+if have_host_jit_feature_support('jit'):
 config.available_features.add('host-supports-jit')
 
+if have_host_jit_feature_support('exception'):
+config.available_features.add('host-supports-exception')
+
 if config.clang_staticanalyzer:
 config.available_features.add('staticanalyzer')
 tools.append('clang-check')

diff  --git a/clang/tools/clang-repl/ClangRepl.cpp 
b/clang/tools/clang-repl/ClangRepl.cpp
index 4f673bdcb7cc5..490e2feed50e1 100644
--- a/clang/tools/clang-repl/ClangRepl.cpp
+++ b/clang/tools/clang-repl/ClangRepl.cpp
@@ -28,6 +28,8 @@ static llvm::cl::list
   llvm::cl::CommaSeparated);
 static llvm::cl::opt OptHostSupportsJit("host-supports-jit",
   llvm::cl::Hidden);
+static llvm::cl::opt OptHostSupportsException("host-supports-exception",
+llvm::cl::Hidden);
 static llvm::cl::list OptInputs(llvm::cl::Positional,
  llvm::cl::desc("[code to run]"));
 
@@ -65,6 +67,42 @@ static int checkDiagErrors(const clang::CompilerInstance 
*CI) {
   return Errs ? EXIT_FAILURE : EXIT_SUCCESS;
 }
 
+// Check if the host environment supports c++ exception handling
+// by querying the existence of symbol __cxa_throw.
+static bool checkExceptionSupport() {
+  auto J = llvm::orc::LLJITBuilder().create();
+  if (!J) {
+llvm::consumeError(J.takeError());
+return false;
+  }
+
+  std::vector Dummy;
+  auto CI = clang::IncrementalCompilerBuilder::create(Dummy);
+  if (!CI) {
+llvm::consumeError(CI.takeError());
+return false;
+  }
+
+  auto Interp = clang::Interpreter::create(std::move(*CI));
+  if (!Interp) {
+llvm::consumeError(Interp.takeError());
+return false;
+  }
+
+  if (auto Err = (*Interp)->ParseAndExecute("")) {
+llvm::consumeError(std::move(Err));
+return false;
+  }
+
+  auto Sym = (*Interp)->getSymbolAddress("__cxa_throw");
+  if (!Sym) {
+llvm::consumeError(Sym.takeError());
+return false;
+  }
+
+  return true;
+}
+
 llvm::ExitOnError ExitOnErr;
 int main(int argc, const char **argv) {
   ExitOnErr.setBanner("clang-repl: ");
@@ -87,6 +125,14 @@ int main(int argc, const char **argv) {
 return 0;
   }
 
+  if (OptHostSupportsException) {
+if (checkExceptionSupport())
+  llvm::outs() << "true\n";
+else
+  llvm::outs() << "false\n";
+return 0;
+  }
+
   // FIXME: Investigate if we

[PATCH] D129242: [clang-repl] Add host exception support check utility flag.

2022-07-28 Thread Sunho Kim via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3cc3be8fa471: [clang-repl] Add host exception support check 
utility flag. (authored by sunho).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129242

Files:
  clang/test/Interpreter/simple-exception.cpp
  clang/test/lit.cfg.py
  clang/tools/clang-repl/ClangRepl.cpp

Index: clang/tools/clang-repl/ClangRepl.cpp
===
--- clang/tools/clang-repl/ClangRepl.cpp
+++ clang/tools/clang-repl/ClangRepl.cpp
@@ -28,6 +28,8 @@
   llvm::cl::CommaSeparated);
 static llvm::cl::opt OptHostSupportsJit("host-supports-jit",
   llvm::cl::Hidden);
+static llvm::cl::opt OptHostSupportsException("host-supports-exception",
+llvm::cl::Hidden);
 static llvm::cl::list OptInputs(llvm::cl::Positional,
  llvm::cl::desc("[code to run]"));
 
@@ -65,6 +67,42 @@
   return Errs ? EXIT_FAILURE : EXIT_SUCCESS;
 }
 
+// Check if the host environment supports c++ exception handling
+// by querying the existence of symbol __cxa_throw.
+static bool checkExceptionSupport() {
+  auto J = llvm::orc::LLJITBuilder().create();
+  if (!J) {
+llvm::consumeError(J.takeError());
+return false;
+  }
+
+  std::vector Dummy;
+  auto CI = clang::IncrementalCompilerBuilder::create(Dummy);
+  if (!CI) {
+llvm::consumeError(CI.takeError());
+return false;
+  }
+
+  auto Interp = clang::Interpreter::create(std::move(*CI));
+  if (!Interp) {
+llvm::consumeError(Interp.takeError());
+return false;
+  }
+
+  if (auto Err = (*Interp)->ParseAndExecute("")) {
+llvm::consumeError(std::move(Err));
+return false;
+  }
+
+  auto Sym = (*Interp)->getSymbolAddress("__cxa_throw");
+  if (!Sym) {
+llvm::consumeError(Sym.takeError());
+return false;
+  }
+
+  return true;
+}
+
 llvm::ExitOnError ExitOnErr;
 int main(int argc, const char **argv) {
   ExitOnErr.setBanner("clang-repl: ");
@@ -87,6 +125,14 @@
 return 0;
   }
 
+  if (OptHostSupportsException) {
+if (checkExceptionSupport())
+  llvm::outs() << "true\n";
+else
+  llvm::outs() << "false\n";
+return 0;
+  }
+
   // FIXME: Investigate if we could use runToolOnCodeWithArgs from tooling. It
   // can replace the boilerplate code for creation of the compiler instance.
   auto CI = ExitOnErr(clang::IncrementalCompilerBuilder::create(ClangArgv));
Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -70,7 +70,7 @@
 if config.clang_examples:
 config.available_features.add('examples')
 
-def have_host_jit_support():
+def have_host_jit_feature_support(feature_name):
 clang_repl_exe = lit.util.which('clang-repl', config.clang_tools_dir)
 
 if not clang_repl_exe:
@@ -79,7 +79,7 @@
 
 try:
 clang_repl_cmd = subprocess.Popen(
-[clang_repl_exe, '--host-supports-jit'], stdout=subprocess.PIPE)
+[clang_repl_exe, '--host-supports-' + feature_name], stdout=subprocess.PIPE)
 except OSError:
 print('could not exec clang-repl')
 return False
@@ -89,9 +89,12 @@
 
 return 'true' in clang_repl_out
 
-if have_host_jit_support():
+if have_host_jit_feature_support('jit'):
 config.available_features.add('host-supports-jit')
 
+if have_host_jit_feature_support('exception'):
+config.available_features.add('host-supports-exception')
+
 if config.clang_staticanalyzer:
 config.available_features.add('staticanalyzer')
 tools.append('clang-check')
Index: clang/test/Interpreter/simple-exception.cpp
===
--- /dev/null
+++ clang/test/Interpreter/simple-exception.cpp
@@ -0,0 +1,14 @@
+// clang-format off
+// REQUIRES: host-supports-jit, host-supports-exception 
+// UNSUPPORTED: system-aix
+// XFAIL: arm, arm64-apple, windows-msvc, windows-gnu
+// RUN: cat %s | clang-repl | FileCheck %s
+extern "C" int printf(const char *, ...);
+
+int f() { throw "Simple exception"; return 0; }
+int checkException() { try { printf("Running f()\n"); f(); } catch (const char *e) { printf("%s\n", e); } return 0; }
+auto r1 = checkException();
+// CHECK: Running f()
+// CHECK-NEXT: Simple exception
+
+%quit
\ No newline at end of file
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

2022-07-28 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov added a comment.

In D112374#3684722 , @hans wrote:

> Here's an example where I think this regressed a Clang diagnostic. Consider:

Consider this simple extension of this example, to show how this general 
problem already existed: https://godbolt.org/z/n6nGhejTc

  template  struct Template { Template(int x) {} };
  
  struct S1 {
struct Baz {
  struct Foo;
};
typedef Template Typedef;
  };
  
  struct S2 {
struct Baz {
  struct Foo;
};
typedef Template Typedef;
  };
  
  typedef S1::Typedef Bar;
  Bar f;

Prints: `error: no matching constructor for initialization of 'Bar' (aka 
'Template')`

You still don't know which `Foo` this refers to, because you don't know which 
`Baz` it is either.

This patch fixed the inconsistency where we printed the bare `Foo` with the 
synthetic nested name, but printed `Baz::Foo` as written.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112374

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

[PATCH] D130545: [cmake] Slight fix ups to make robust to the full range of GNUInstallDirs

2022-07-28 Thread Sebastian Neubauer via Phabricator via cfe-commits

sebastian-ne added inline comments.



Comment at: openmp/runtime/src/CMakeLists.txt:383
 \"${alias}${LIBOMP_LIBRARY_SUFFIX}\" WORKING_DIRECTORY
-\"\$ENV{DESTDIR}\${CMAKE_INSTALL_PREFIX}/${OPENMP_INSTALL_LIBDIR}\")")
+\"\$ENV{DESTDIR}\${outdir}\")")
 endforeach()

The backslash before `\${outdir}` is wrong here, it must be 
`\"\$ENV{DESTDIR}${outdir}\")")`.

Also, before this patch, there were a lot of backslashes before e.g. 
`\${CMAKE_INSTALL_PREFIX}`, but I hope that shouldn’t matter, the value should 
be the same at configure and install time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130545

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

[PATCH] D120185: [ASTMatchers] Output currently processing match and nodes on crash

2022-07-28 Thread Michał Górny via Phabricator via cfe-commits

mgorny added a comment.

Thanks. The usual way of doing is to move the necessary bits to `llvm-config.h`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120185

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

[PATCH] D130705: [clang][ASTImporter] Improve import of functions with auto return type.

2022-07-28 Thread Balázs Kéri via Phabricator via cfe-commits

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

ASTImporter used to crash in some cases when a function is imported with
`auto` return type and the return type has references into the function.
The handling of such cases is improved and crash should not occur any more
but it is not fully verified, there are very many different types of
cases to care for.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130705

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
@@ -6318,64 +6318,195 @@
   EXPECT_EQ(ToEPI.ExceptionSpec.SourceDecl, ToCtor);
 }
 
-struct ImportAutoFunctions : ASTImporterOptionSpecificTestBase {};
+struct ImportAutoFunctions : ASTImporterOptionSpecificTestBase {
+  void testImport(llvm::StringRef Code, clang::TestLanguage Lang = Lang_CXX14) {
+Decl *FromTU = getTuDecl(Code, Lang, "input0.cc");
+FunctionDecl *From = FirstDeclMatcher().match(
+FromTU, functionDecl(hasName("foo")));
+
+FunctionDecl *To = Import(From, Lang);
+EXPECT_TRUE(To);
+// We check here only that the type is auto type.
+// These tests are to verify that no crash happens.
+// The crash possibility is the presence of a reference to a declaration
+// in the function's body from the return type, if the function has auto
+// return type.
+EXPECT_TRUE(isa(To->getReturnType()));
+  }
+};
+
+TEST_P(ImportAutoFunctions, ReturnWithAutoUnresolvedArg) {
+  testImport(
+  R"(
+  template
+  auto foo() {
+return 22;
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithTemplateTemplateArg) {
+  // FIXME: Is it possible to have the template arg inside the function?
+  testImport(
+  R"(
+  template struct Tmpl {};
+  template class> struct TmplTmpl {};
+  auto foo() {
+return TmplTmpl();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithDeclarationTemplateArg) {
+  // FIXME: Is it possible to have the template arg inside the function?
+  testImport(
+  R"(
+  template struct Tmpl {};
+  int A[10];
+  auto foo() {
+return Tmpl();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithNullPtrTemplateArg) {
+  testImport(
+  R"(
+  template struct Tmpl {};
+  auto foo() {
+constexpr int* A = nullptr;
+return Tmpl();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithIntegralTemplateArg) {
+  testImport(
+  R"(
+  template struct Tmpl {};
+  auto foo() {
+using Int = int;
+constexpr Int A = 7;
+return Tmpl();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithDecltypeTypeDeclaredInside) {
+  testImport(
+  R"(
+  template struct Tmpl {};
+  auto foo() {
+struct X {};
+X x;
+return Tmpl();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithUsingTypeDeclaredInside) {
+  testImport(
+  R"(
+  template struct Tmpl {};
+  namespace A { struct X {}; }
+  auto foo() {
+using A::X;
+return Tmpl();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithArrayTypeDeclaredInside) {
+  testImport(
+  R"(
+  template struct Tmpl {};
+  auto foo() {
+struct X {};
+return Tmpl();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithArraySizeExprDeclaredInside) {
+  testImport(
+  R"(
+  template struct Tmpl {};
+  auto foo() {
+constexpr int S = 10;
+return Tmpl();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithPackArgDeclaredInside) {
+  testImport(
+  R"(
+  template struct Tmpl {};
+  auto foo() {
+using X = bool;
+return Tmpl();
+  }
+  )");
+}
 
 TEST_P(ImportAutoFunctions, ReturnWithTemplateWithIntegerArgDeclaredInside) {
-  Decl *FromTU = getTuDecl(
+  testImport(
   R"(
   template struct Tmpl {};
   auto foo() {
 constexpr int X = 1;
 return Tmpl();
   }
-  )",
-  Lang_CXX14, "input0.cc");
-  FunctionDecl *From = FirstDeclMatcher().match(
-  FromTU, functionDecl(hasName("foo")));
+  )");
+}
 
-  FunctionDecl *To = Import(From, Lang_CXX14);
-  EXPECT_TRUE(To);
-  EXPECT_TRUE(isa(To->getReturnType()));
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithPtrToStructDeclaredInside) {
+  testImport(
+  R"(
+  template struct Tmpl {

[PATCH] D130545: [cmake] Slight fix ups to make robust to the full range of GNUInstallDirs

2022-07-28 Thread Sebastian Neubauer via Phabricator via cfe-commits

sebastian-ne added a comment.

I pushed a potential fix (removing the backslash) in 
50716ba2b337afe46ac256cc91673dc27356a776 
.
I don’t know which buildbots to look at though, it looks like the OpenMP ones 
all succeed.

PS: I’ll be away for a few weeks, please revert my fix if it causes problems.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130545

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

[PATCH] D129016: [PowerPC] implemented @llvm.ppc.kill.canary to corrupt stack guard

2022-07-28 Thread Lei Huang via Phabricator via cfe-commits

lei added inline comments.



Comment at: llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll:4
+; RUN:   --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=AIX
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux \
+; RUN:   --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=LINUX

missing LE run test line?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129016

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

[clang] 3b09e53 - [ARM] Remove duplicate fp16 intrinsics

2022-07-28 Thread David Green via cfe-commits


Author: David Green
Date: 2022-07-28T14:26:17+01:00
New Revision: 3b09e532ee396bb07820ecadb29e1ed88f6e6c25

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

LOG: [ARM] Remove duplicate fp16 intrinsics

These vdup and vmov float16 intrinsics are being defined in both the
general section and then again in fp16 under a !aarch64 flag. The
vdup_lane intrinsics were being defined in both aarch64 and !aarch64
sections, so have been commoned.  They are defined as macros, so do not
give duplicate warnings, but removing the duplicates shouldn't alter the
available intrinsics.

Added: 


Modified: 
clang/include/clang/Basic/arm_neon.td
clang/test/CodeGen/aarch64-v8.2a-neon-intrinsics.c

Removed: 




diff  --git a/clang/include/clang/Basic/arm_neon.td 
b/clang/include/clang/Basic/arm_neon.td
index 2e9798129fdfb..93f9961931370 100644
--- a/clang/include/clang/Basic/arm_neon.td
+++ b/clang/include/clang/Basic/arm_neon.td
@@ -530,7 +530,7 @@ def VMOV_N   : WOpInst<"vmov_n", ".1",
 }
 let InstName = "" in
 def VDUP_LANE: WOpInst<"vdup_lane", ".qI",
-   "UcUsUicsiPcPsfQUcQUsQUiQcQsQiQPcQPsQflUlQlQUl",
+   "UcUsUicsiPcPshfQUcQUsQUiQcQsQiQPcQPsQhQflUlQlQUl",
OP_DUP_LN>;
 
 

@@ -980,7 +980,7 @@ def COPYQ_LANEQ : IOpInst<"vcopy_laneq", "..I.I",
 
 

 // Set all lanes to same value
-def VDUP_LANE1: WOpInst<"vdup_lane", ".qI", "hdQhQdPlQPl", OP_DUP_LN>;
+def VDUP_LANE1: WOpInst<"vdup_lane", ".qI", "dQdPlQPl", OP_DUP_LN>;
 def VDUP_LANE2: WOpInst<"vdup_laneq", ".QI",
   "csilUcUsUiUlPcPshfdQcQsQiQlQPcQPsQUcQUsQUiQUlQhQfQdPlQPl",
 OP_DUP_LN> {
@@ -1644,7 +1644,8 @@ def SCALAR_VDUP_LANE : IInst<"vdup_lane", "1.I", 
"ScSsSiSlSfSdSUcSUsSUiSUlSPcSPs
 def SCALAR_VDUP_LANEQ : IInst<"vdup_laneq", "1QI", 
"ScSsSiSlSfSdSUcSUsSUiSUlSPcSPs"> {
   let isLaneQ = 1;
 }
-}
+
+} // ArchGuard = "defined(__aarch64__)"
 
 // ARMv8.2-A FP16 vector intrinsics for A32/A64.
 let ArchGuard = "defined(__ARM_FEATURE_FP16_VECTOR_ARITHMETIC)" in {
@@ -1763,15 +1764,6 @@ let ArchGuard = 
"defined(__ARM_FEATURE_FP16_VECTOR_ARITHMETIC)" in {
   def VUZPH: WInst<"vuzp", "2..", "hQh">;
   def VTRNH: WInst<"vtrn", "2..", "hQh">;
 
-
-  let ArchGuard = "!defined(__aarch64__)" in {
-// Set all lanes to same value.
-// Already implemented prior to ARMv8.2-A.
-def VMOV_NH  : WOpInst<"vmov_n", ".1", "hQh", OP_DUP>;
-def VDUP_NH  : WOpInst<"vdup_n", ".1", "hQh", OP_DUP>;
-def VDUP_LANE1H : WOpInst<"vdup_lane", ".qI", "hQh", OP_DUP_LN>;
-  }
-
   // Vector Extract
   def VEXTH  : WInst<"vext", "...I", "hQh">;
 

diff  --git a/clang/test/CodeGen/aarch64-v8.2a-neon-intrinsics.c 
b/clang/test/CodeGen/aarch64-v8.2a-neon-intrinsics.c
index 08e7fecd1330f..3dc3a49a9bfd5 100644
--- a/clang/test/CodeGen/aarch64-v8.2a-neon-intrinsics.c
+++ b/clang/test/CodeGen/aarch64-v8.2a-neon-intrinsics.c
@@ -1754,15 +1754,15 @@ float16x8_t test_vmulq_n_f16(float16x8_t a, float16_t 
b) {
 // CHECK-LABEL: define {{[^@]+}}@test_vmulh_lane_f16
 // CHECK-SAME: (half noundef [[A:%.*]], <4 x half> noundef [[B:%.*]]) 
#[[ATTR0]] {
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[__REINT_851:%.*]] = alloca <4 x half>, align 8
-// CHECK-NEXT:[[__REINT1_851:%.*]] = alloca i16, align 2
+// CHECK-NEXT:[[__REINT_847:%.*]] = alloca <4 x half>, align 8
+// CHECK-NEXT:[[__REINT1_847:%.*]] = alloca i16, align 2
 // CHECK-NEXT:[[CONV:%.*]] = fpext half [[A]] to float
-// CHECK-NEXT:store <4 x half> [[B]], <4 x half>* [[__REINT_851]], align 8
-// CHECK-NEXT:[[TMP0:%.*]] = bitcast <4 x half>* [[__REINT_851]] to <4 x 
i16>*
+// CHECK-NEXT:store <4 x half> [[B]], <4 x half>* [[__REINT_847]], align 8
+// CHECK-NEXT:[[TMP0:%.*]] = bitcast <4 x half>* [[__REINT_847]] to <4 x 
i16>*
 // CHECK-NEXT:[[TMP1:%.*]] = load <4 x i16>, <4 x i16>* [[TMP0]], align 8
 // CHECK-NEXT:[[VGET_LANE:%.*]] = extractelement <4 x i16> [[TMP1]], i32 3
-// CHECK-NEXT:store i16 [[VGET_LANE]], i16* [[__REINT1_851]], align 2
-// CHECK-NEXT:[[TMP2:%.*]] = bitcast i16* [[__REINT1_851]] to half*
+// CHECK-NEXT:store i16 [[VGET_LANE]], i16* [[__REINT1_847]], align 2
+// CHECK-NEXT:[[TMP2:%.*]] = bitcast i16* [[__REINT1_847]] to half*
 // CHECK-NEXT:[[TMP3:%.*]] = load half, half* [[TMP2]], align 2
 // CHECK-NEXT:[[CONV2:%.*]] = fpext half [[TMP3]] to float
 // CHECK-NEXT:[[MUL:%.*]] = fmul float [[CONV]], [[CONV2]]
@@ -1776,15 +1776,15 @@ float16_t test_vmulh_lane_f16(float16_t a, float16x4_t 
b) {
 // CHECK-LABEL: define {{[^@]+}}@test_vmulh_laneq_f16
 // CHEC

[PATCH] D130600: [clang][dataflow] Handle return statements

2022-07-28 Thread Sam Estep via Phabricator via cfe-commits

samestep added inline comments.



Comment at: 
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:114
+  ///
+  ///  `return` must not be assigned a storage location.
+  void setReturnStorageLocation(StorageLocation &Loc) {

sgatev wrote:
> li.zhe.hua wrote:
> > samestep wrote:
> > > li.zhe.hua wrote:
> > > > Fix this as well? A reader shouldn't need to root around in private 
> > > > implementation details to understand the requirements for calling a 
> > > > function.
> > > Could you clarify what you mean? I was simply copying the signature and 
> > > docstring from `setThisPointeeStorageLocation`.
> > You've marked `return` in backticks. There is no parameter named `return` 
> > and it is unclear what `return` refers to. My best guess is that this is a 
> > typo of `ReturnLoc`, which is a private data member. So this is a public 
> > interface with a requirement that a private data member has some property. 
> > This should instead reframe the requirement as properties from the external 
> > reader's perspective.
> That was my guess initially too, but my next best guess is that `return` in 
> backticks stands for the keyword/AST node. In any case, let's make it less 
> ambiguous and let's not add requirements based on implementation details. How 
> about: `The return value must not be assigned a storage location.`?
Ah sorry, I understand now; you simply meant that I should make the same change 
here that @gribozavr2 suggested I make to the other docstrings? I'll go ahead 
and do that (once I am able to re-export this patch again).



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:338-339
+if (Loc == nullptr) {
+  // The outermost context does not set a storage location for `return`, so
+  // in that case we just ignore `return` statements.
+  return;

sgatev wrote:
> samestep wrote:
> > sgatev wrote:
> > > Let's make this a FIXME to set a storage location for the outermost 
> > > context too.
> > @sgatev I could add a `FIXME` for that, or I could just do it in this same 
> > patch; do you have a preference between those two options?
> Same patch works!
Cool, I have an update to this patch which does that, so I'll export it here as 
soon as I am able to do so.



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:3908
+  Var = true;
+  return;
+}

li.zhe.hua wrote:
> samestep wrote:
> > li.zhe.hua wrote:
> > > Why is this change to the test necessary?
> > This is mentioned in the patch description (updated earlier today); it's 
> > not necessary, but I added it to get a bit of extra coverage for some cases 
> > in the `VisitReturnStmt` method this patch adds.
> Please add the coverage as a separate test. Separate behaviors should be 
> tested as separate tests. go/unit-testing-practices#behavior-testing
Makes sense, I'll do that. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130600

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

[PATCH] D129973: [clang] Pass FoundDecl to DeclRefExpr creator for operator overloads

2022-07-28 Thread Ivan Donchevskii via Phabricator via cfe-commits

yvvan added a comment.

With this change we don't pass "LocInfo" directly and it seems to break the 
locations when calling "getCXXOperatorNameRange" for this DeclRefExpr later on. 
Please fix it. You can introduce another "Create" static method for DeclRefExpr 
that accepts LocInfo and passes it to the DeclarationNameInfo constructor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129973

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

[PATCH] D130460: [pseudo] Add recovery for declarations

2022-07-28 Thread Haojian Wu via Phabricator via cfe-commits

hokein added inline comments.

Comment at: clang-tools-extra/pseudo/lib/cxx/CXX.cpp:267

+Token::Index recoverNextDeclaration(const RecoveryParams &P) {
+  if (P.Begin == P.Tokens.tokens().size())

I think this excludes the parameter-declaration (from the grammar spec, it is a 
different nonterminal).

Comment at: clang-tools-extra/pseudo/lib/cxx/CXX.cpp:267

+Token::Index recoverNextDeclaration(const RecoveryParams &P) {
+  if (P.Begin == P.Tokens.tokens().size())

hokein wrote:
> I think this excludes the parameter-declaration (from the grammar spec, it is 
> a different nonterminal).
The logic of implementations looks very solid. I think it would be good to have 
some unittests for it. 

Comment at: clang-tools-extra/pseudo/lib/cxx/CXX.cpp:282
+  //  - if the braces are missing, we don't know where to recover
+  // So We only try to detect the others.
+  bool AcceptBraces = false;

if I read the code correctly, the following case should work like

```
void foo() {}

LLVM_DISCARD int bar() {} // this is an opaque `declaration` node?

void foo2() {} // it won't work if we change void -> Foo.
```

Comment at: clang-tools-extra/pseudo/lib/cxx/CXX.cpp:291
+  // they appear at the beginning of a line.
+  auto LikelyStartsDeclaration = [&](tok::TokenKind K) -> bool {
+switch (K) {

the list looks reasonable -- I think we can lookup the `Follow(declaration)` 
set to find more.

Comment at: clang-tools-extra/pseudo/lib/cxx/CXX.cpp:301
+case tok::kw_union:
+// Types & other decl-specifiers
+case tok::kw_void:

maybe consider `tok::identifier` (if the identifier is a type specifier, it is 
a good indicator of a declaration starting) as well? And yeah, we're less 
certain about it, but since we use the indentation information, I guess it 
might probably work well?

Comment at: clang-tools-extra/pseudo/lib/cxx/CXX.cpp:352
+}
+
+// Semicolons terminate statements.

I think we miss to check the return index >= Cursor here.

Comment at: clang-tools-extra/pseudo/lib/cxx/cxx.bnf:313
-declaration-seq := declaration
-declaration-seq := declaration-seq declaration
 declaration := block-declaration

Just want to spell out, this causes a performance hit :(
(a pure change of this has a ~6% slowdown, 7.17 MB/s -> 6.7MB/s on 
SemaCodeComplete.cpp).

left-cursive grammar is more friendly to LR-like parsers, because the parser 
can do the reduction earlier (after a parsed a `declaration`, we can perform a 
`declaration-seq` reduce); while right-cursive grammar, the reduction starts 
from the last declaration (this means our GSS is more deeper).

> change decl-sequence to right-recursive to simplify this
I remembered we discussed this bit before, but I don't really remember the 
details now :(

Comment at: clang-tools-extra/pseudo/test/cxx/recovery-declarations.cpp:6
+
+int x[0];
+// CHECK:  simple-declaration := decl-specifier-seq init-declarator-list ;

nit: add some tests to cover the initializer, e.g. 

```
auto lambda = []  () xxx {};
```

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130460

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

[PATCH] D126880: [clang-tidy] Add cppcoreguidelines-avoid-const-or-ref-data-members check

2022-07-28 Thread Carlos Galvez via Phabricator via cfe-commits

carlosgalvezp updated this revision to Diff 448323.
carlosgalvezp added a comment.

Rebase onto latest main.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126880

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-const-or-ref-data-members.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp
@@ -0,0 +1,157 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-const-or-ref-data-members %t
+namespace std {
+template 
+struct unique_ptr {};
+
+template 
+struct shared_ptr {};
+} // namespace std
+
+namespace gsl {
+template 
+struct not_null {};
+} // namespace gsl
+
+struct Ok {
+  int i;
+  int *p;
+  const int *pc;
+  std::unique_ptr up;
+  std::shared_ptr sp;
+  gsl::not_null n;
+};
+
+struct ConstMember {
+  const int c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' is const qualified [cppcoreguidelines-avoid-const-or-ref-data-members]
+};
+
+struct LvalueRefMember {
+  int &lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' is a reference
+};
+
+struct ConstRefMember {
+  const int &cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' is a reference
+};
+
+struct RvalueRefMember {
+  int &&rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' is a reference
+};
+
+struct ConstAndRefMembers {
+  int const c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' is const qualified
+  int &lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' is a reference
+  const int &cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' is a reference
+  int &&rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' is a reference
+};
+
+struct Foo {};
+
+struct Ok2 {
+  Foo i;
+  Foo *p;
+  const Foo *pc;
+  std::unique_ptr up;
+  std::shared_ptr sp;
+  gsl::not_null n;
+};
+
+struct ConstMember2 {
+  const Foo c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' is const qualified
+};
+
+struct LvalueRefMember2 {
+  Foo &lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' is a reference
+};
+
+struct ConstRefMember2 {
+  const Foo &cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' is a reference
+};
+
+struct RvalueRefMember2 {
+  Foo &&rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' is a reference
+};
+
+struct ConstAndRefMembers2 {
+  const Foo c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' is const qualified
+  Foo &lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' is a reference
+  const Foo &cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' is a reference
+  Foo &&rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' is a reference
+};
+
+using ConstType = const int;
+using RefType = int &;
+using ConstRefType = const int &;
+using RefRefType = int &&;
+
+struct WithAlias {
+  ConstType c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' is const qualified
+  RefType lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: member 'lr' is a reference
+  ConstRefType cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: member 'cr' is a reference
+  RefRefType rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'rr' is a reference
+};
+
+template 
+using Array = int[N];
+
+struct ConstArrayMember {
+  const Array<1> c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: member 'c' is const qualified
+};
+
+struct LvalueRefArrayMember {
+  Array<2> &lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'lr' is a reference
+};
+
+struct ConstLvalueRefArrayMember {
+  Array<3> const &cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: member 'cr' is a reference
+};
+
+struct RvalueRefArrayMember {
+  Array<4> &&rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'rr' is a reference
+};
+
+template 
+struct TemplatedOk {
+  T t;
+};
+
+template 
+struct TemplatedConst {
+  T t;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: member 't' is const qualified
+};
+
+template 
+struct TemplatedRef {
+  T t;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: member 't' is a reference
+};
+
+TemplatedOk t1{};
+TemplatedConst t2{123};
+TemplatedRef t3{123};
+TemplatedRef t4{123};
+TemplatedRef t5

[PATCH] D126880: [clang-tidy] Add cppcoreguidelines-avoid-const-or-ref-data-members check

2022-07-28 Thread Carlos Galvez via Phabricator via cfe-commits

carlosgalvezp added a comment.

@njames93 Would you mind reviewing? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126880

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

[PATCH] D130566: [Driver] Default to DWARF 4 on Linux/sparc64

2022-07-28 Thread Rainer Orth via Phabricator via cfe-commits

ro added a comment.

In D130566#3682923 , @dblaikie wrote:

>> This is all extremely weird, but until the error shows up again, I'll put 
>> this patch on hold (not yet abandoning since there seems to be no way to 
>> unabandon if necessary).
>
> You can abandon and then "reclaim" the revision to reopen it and continue 
> work.

Ah, I missed that: the reclaim action is only offered for abandoned patches, I 
assume.  Thanks.

In D130566#3683174 , @MaskRay wrote:

> I accepted this because this seems a major showstopper and we want to resolve 
> it for the upcoming major release
> But I just recall that clang 14.0.0 has defaulted to DWARF v5 for most ELF 
> operating systems on all architectures, including Sparc.
> I think we need more justification to downgrade the DWARV version.

Fully agreed.  However, clang 14.0.0 was in a way worse shape on Debian/sparc64 
(wouldn't even build), so we wouldn't have noticed.  Solaris/sparcv9 wouldn't 
notice either since it uses the native linker, not GNU ld.

>> So far, I had only seen it with the GNU ld 2.38.50 bundled with Debian 
>> 11/sparc64, but couldn't reproduce on Ubuntu 20.04/x86_64 (neither bundled 
>> GNU ld 2.34, nor self-compiled 2.38.90)
>
> Thanks for the additional note. Seems worth investigating whether it is an 
> issue which should be addressed on GNU ld side.
> binutils 2.39 will be released on 2022-08-06 
> (https://sourceware.org/pipermail/binutils/2022-July/121656.html) and such a 
> regression style issue should be worked on quickly.

Indeed.  However, I won't be able to spend much time on this: Linux/sparc64 is 
only tangential to my work (as a vehicle to test SPARC patches where I want to 
make sure they don't break that target).  I've got enough to do testing 
binutils 2.39 on Solaris ;-)

That said, I've now filed binutils PR binutils PR ld/29424 
.  The binutils 
maintainers can now decide what the want to do about this, if anything.

> The binutils code does suggest it doesn't handle the DWARF v5 features, but I 
> am curious why older releases don't have the problem.

They might if one tried.  However, the bundled GNU ld is already 2.38.50.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130566

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

[clang] bd08f41 - [clang-repl] Disable exception unittest on AIX.

2022-07-28 Thread Sunho Kim via cfe-commits


Author: Sunho Kim
Date: 2022-07-28T22:48:51+09:00
New Revision: bd08f413c089da5a56438cc8902f60df91a08a66

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

LOG: [clang-repl] Disable exception unittest on AIX.

AIX platform was not supported but it was not explicitly checked in exception 
test as it was excluded by isPPC() check.

Added: 


Modified: 
clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp

Removed: 




diff  --git 
a/clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp 
b/clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
index a1dbcfa3410f8..7a5f07e25bc6f 100644
--- a/clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
+++ b/clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
@@ -99,6 +99,10 @@ extern "C" int throw_exception() {
   const clang::CompilerInstance *CI = Interp->getCompilerInstance();
   const llvm::Triple &Triple = CI->getASTContext().getTargetInfo().getTriple();
 
+  // AIX is unsupported.
+  if (Triple.isOSAIX())
+return;
+
   // FIXME: ARM fails due to `Not implemented relocation type!`
   if (Triple.isARM())
 return;



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

[clang] 6e56d0d - Start support for HLSL `RWBuffer`

2022-07-28 Thread Chris Bieneman via cfe-commits


Author: Chris Bieneman
Date: 2022-07-28T08:49:50-05:00
New Revision: 6e56d0dbe3c89d3cd5730a57b9049b74e0bf605f

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

LOG: Start support for HLSL `RWBuffer`

Most of the change here is fleshing out the HLSLExternalSemaSource with
builder implementations to build the builtin types. Eventually, I may
move some of this code into tablegen or a more managable declarative
file but I want to get the AST generation logic ready first.

This code adds two new types into the HLSL AST, `hlsl::Resource` and
`hlsl::RWBuffer`. The `Resource` type is just a wrapper around a handle
identifier, and is largely unused in source. It will morph a bit over
time as I work on getting the source compatability correct, but for now
it is a reasonable stand-in. The `RWBuffer` type is not ready for use.
I'm posting this change for review because it adds a lot of
infrastructure code and is testable.

There is one change to clang code outside the HLSL-specific logic here,
which addresses a behavior change introduced a long time ago in
967d438439ac. That change resulted in unintentionally breaking
situations where an incomplete template declaration was provided from
an AST source, and needed to be completed later by the external AST.
That situation doesn't happen in the normal AST importer flow, but can
happen when an AST source provides incomplete declarations of
templates. The solution is to annotate template specializations of
incomplete types with the HasExternalLexicalSource bit from the base
template.

Depends on D128012.

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

Added: 
clang/test/AST/HLSL/RWBuffer-AST.hlsl
clang/test/AST/HLSL/ResourceStruct.hlsl
clang/test/SemaHLSL/BuiltIns/RWBuffers.hlsl

Modified: 
clang/include/clang/Sema/HLSLExternalSemaSource.h
clang/lib/AST/DeclTemplate.cpp
clang/lib/Sema/HLSLExternalSemaSource.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/HLSLExternalSemaSource.h 
b/clang/include/clang/Sema/HLSLExternalSemaSource.h
index 439fc3d10f333..a5aa21da12934 100644
--- a/clang/include/clang/Sema/HLSLExternalSemaSource.h
+++ b/clang/include/clang/Sema/HLSLExternalSemaSource.h
@@ -12,6 +12,8 @@
 #ifndef CLANG_SEMA_HLSLEXTERNALSEMASOURCE_H
 #define CLANG_SEMA_HLSLEXTERNALSEMASOURCE_H
 
+#include "llvm/ADT/DenseMap.h"
+
 #include "clang/Sema/ExternalSemaSource.h"
 
 namespace clang {
@@ -21,8 +23,16 @@ class Sema;
 class HLSLExternalSemaSource : public ExternalSemaSource {
   Sema *SemaPtr = nullptr;
   NamespaceDecl *HLSLNamespace;
+  CXXRecordDecl *ResourceDecl;
+
+  using CompletionFunction = std::function;
+  llvm::DenseMap Completions;
 
   void defineHLSLVectorAlias();
+  void defineTrivialHLSLTypes();
+  void forwardDeclareHLSLTypes();
+
+  void completeBufferType(CXXRecordDecl *Record);
 
 public:
   ~HLSLExternalSemaSource() override;
@@ -34,6 +44,9 @@ class HLSLExternalSemaSource : public ExternalSemaSource {
 
   /// Inform the semantic consumer that Sema is no longer available.
   void ForgetSema() override { SemaPtr = nullptr; }
+
+  /// Complete an incomplete HLSL builtin type
+  void CompleteType(TagDecl *Tag) override;
 };
 
 } // namespace clang

diff  --git a/clang/lib/AST/DeclTemplate.cpp b/clang/lib/AST/DeclTemplate.cpp
index e7e5f355809b0..e3dd7804c99ba 100644
--- a/clang/lib/AST/DeclTemplate.cpp
+++ b/clang/lib/AST/DeclTemplate.cpp
@@ -930,6 +930,14 @@ ClassTemplateSpecializationDecl::Create(ASTContext 
&Context, TagKind TK,
   SpecializedTemplate, Args, PrevDecl);
   Result->setMayHaveOutOfDateDef(false);
 
+  // If the template decl is incomplete, copy the external lexical storage from
+  // the base template. This allows instantiations of incomplete types to
+  // complete using the external AST if the template's declaration came from an
+  // external AST.
+  if (!SpecializedTemplate->getTemplatedDecl()->isCompleteDefinition())
+Result->setHasExternalLexicalStorage(
+  SpecializedTemplate->getTemplatedDecl()->hasExternalLexicalStorage());
+
   Context.getTypeDeclType(Result, PrevDecl);
   return Result;
 }

diff  --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp 
b/clang/lib/Sema/HLSLExternalSemaSource.cpp
index 56c2dd40bd9a8..d4ea0344adc4a 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -15,8 +15,161 @@
 #include "clang/Basic/AttrKinds.h"
 #include "clang/Sema/Sema.h"
 
+#include 
+
 using namespace clang;
 
+namespace {
+
+struct TemplateParameterListBuilder;
+
+struct BuiltinTypeDeclBuilder {
+  CXXRecordDecl *Record = nullptr;
+  ClassTemplateDecl *Template = nullptr;
+  NamespaceDecl *HLSLNamespace = nullptr;
+
+  BuiltinTypeDeclBuilder(CXXRecordDecl *R) : Record(R) {
+Record->startDefinition();
+

[PATCH] D128569: Start support for HLSL `RWBuffer`

2022-07-28 Thread Chris Bieneman 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 rG6e56d0dbe3c8: Start support for HLSL `RWBuffer` (authored by 
beanz).

Changed prior to commit:
  https://reviews.llvm.org/D128569?vs=442690&id=448326#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128569

Files:
  clang/include/clang/Sema/HLSLExternalSemaSource.h
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/Sema/HLSLExternalSemaSource.cpp
  clang/test/AST/HLSL/RWBuffer-AST.hlsl
  clang/test/AST/HLSL/ResourceStruct.hlsl
  clang/test/SemaHLSL/BuiltIns/RWBuffers.hlsl

Index: clang/test/SemaHLSL/BuiltIns/RWBuffers.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/BuiltIns/RWBuffers.hlsl
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -fsyntax-only -verify %s
+
+Resource ResourceDescriptorHeap[5];
+typedef vector float3;
+
+RWBuffer Buffer;
+
+[numthreads(1,1,1)]
+void main() {
+  (void)Buffer.h; // expected-error {{'h' is a private member of 'hlsl::RWBuffer'}}
+  // expected-note@* {{implicitly declared private here}}
+}
Index: clang/test/AST/HLSL/ResourceStruct.hlsl
===
--- /dev/null
+++ clang/test/AST/HLSL/ResourceStruct.hlsl
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -fsyntax-only -ast-dump %s | FileCheck %s 
+
+// CHECK: NamespaceDecl {{.*}} implicit hlsl
+// CHECK: CXXRecordDecl 0x{{[0-9A-Fa-f]+}} <>  implicit  class Resource definition
+// CHECK-NEXT: DefinitionData
+// CHECK-NEXT: DefaultConstructor exists trivial needs_implicit
+// CHECK-NEXT: CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
+// CHECK-NEXT: MoveConstructor exists simple trivial needs_implicit
+// CHECK-NEXT: CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
+// CHECK-NEXT: MoveAssignment exists simple trivial needs_implicit
+// CHECK-NEXT: Destructor simple irrelevant trivial needs_implicit
+// CHECK-NEXT: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
+// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <> 
+// implicit h 'void *'
Index: clang/test/AST/HLSL/RWBuffer-AST.hlsl
===
--- /dev/null
+++ clang/test/AST/HLSL/RWBuffer-AST.hlsl
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -fsyntax-only -ast-dump -DEMPTY %s | FileCheck -check-prefix=EMPTY %s 
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -fsyntax-only -ast-dump %s | FileCheck %s 
+
+
+// This test tests two different AST generations. The "EMPTY" test mode verifies
+// the AST generated by forward declaration of the HLSL types which happens on
+// initializing the HLSL external AST with an AST Context.
+
+// The non-empty mode has a use that requires the RWBuffer type be complete,
+// which results in the AST being populated by the external AST source. That
+// case covers the full implementation of the template declaration and the
+// instantiated specialization.
+
+// EMPTY: ClassTemplateDecl 0x{{[0-9A-Fa-f]+}} <>  implicit RWBuffer
+// EMPTY-NEXT: TemplateTypeParmDecl 0x{{[0-9A-Fa-f]+}} <>  class depth 0 index 0 element_type
+// EMPTY-NEXT: TemplateArgument type 'float'
+// EMPTY-NEXT: BuiltinType 0x{{[0-9A-Fa-f]+}} 'float'
+// EMPTY-NEXT: CXXRecordDecl 0x{{[0-9A-Fa-f]+}} <>  implicit  class RWBuffer
+// EMPTY-NEXT: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
+
+// There should be no more occurrances of RWBuffer
+// EMPTY-NOT: RWBuffer
+
+#ifndef EMPTY
+
+RWBuffer Buffer;
+
+#endif
+
+// CHECK: CXXRecordDecl 0x{{[0-9A-Fa-f]+}} <>  implicit  class Resource definition
+// CHECK: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
+// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <>  implicit h 'void *'
+
+// CHECK: ClassTemplateDecl 0x{{[0-9A-Fa-f]+}} <>  implicit RWBuffer
+// CHECK-NEXT: TemplateTypeParmDecl 0x{{[0-9A-Fa-f]+}} <>  class depth 0 index 0 element_type
+// CHECK-NEXT: TemplateArgument type 'float'
+// CHECK-NEXT: BuiltinType 0x{{[0-9A-Fa-f]+}} 'float'
+// CHECK-NEXT: CXXRecordDecl 0x{{[0-9A-Fa-f]+}} <>  implicit class RWBuffer definition
+
+// CHECK: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
+// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <>  implicit h 'void *'
+// CHECK: ClassTemplateSpecializationDecl 0x{{[0-9A-Fa-f]+}} <>  class RWBuffer definition
+
+// CHECK: TemplateArgument type 'float'
+// CHECK-NEXT: BuiltinType 0x{{[0-9A-Fa-f]+}} 'float'
+// CHECK-NEXT: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
+// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <>   implicit h 'void *'
Index: clang/lib/Sema/HLSLExternalSemaSource.cpp
===
--- clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ clang/li

[PATCH] D130523: [pseudo] Perform unconstrained recovery prior to completion.

2022-07-28 Thread Haojian Wu via Phabricator via cfe-commits

hokein accepted this revision.
hokein added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRTable.h:109
+// tok::unknown is a sentinel value used in recovery: can follow anything.
+if (tok::unknown)
+  return true;

this `if` doesn't make sense, and is not needed, I think. 



Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:621
 // Consume the token.
 glrShift(Heads, Terminals[I], Params, Lang, NextHeads);
 

I think we can move the Line634 `Heads.resize(HeadsPartition)` before the 
`glrShift()` as we only do shift on the nearly-created heads, we might gain 
some performance back.



Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:649
+HeadsPartition = NextHeads.size();
 Reduce(NextHeads, Lookahead);
 // Prepare for the next token.

Can we have some comments on `GLRReduce::operator()` on how does parameter 
`Head` get modified  (new heads are appended to it)?  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130523

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

[PATCH] D130224: [Clang][Attribute] Introduce maybe_undef attribute for function arguments which accepts undef values

2022-07-28 Thread krishna chaitanya sankisa via Phabricator via cfe-commits

skc7 updated this revision to Diff 448330.
skc7 marked an inline comment as not done.
skc7 added a comment.

Fix tests failures.


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

https://reviews.llvm.org/D130224

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/attr-maybeundef-template.cpp
  clang/test/CodeGen/attr-maybeundef.c
  clang/test/CodeGenHIP/maybe_undef-attr-verify.hip
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/attr-maybeundef.c

Index: clang/test/Sema/attr-maybeundef.c
===
--- /dev/null
+++ clang/test/Sema/attr-maybeundef.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1  -fsyntax-only -verify %s
+
+// Decl annotations.
+void f(int __attribute__((maybe_undef)) *a);
+void (*fp)(int __attribute__((maybe_undef)) handle);
+__attribute__((maybe_undef)) int i(); // expected-warning {{'maybe_undef' attribute only applies to parameters}}
+int __attribute__((maybe_undef)) a; // expected-warning {{'maybe_undef' attribute only applies to parameters}}
+int (* __attribute__((maybe_undef)) fpt)(char *); // expected-warning {{'maybe_undef' attribute only applies to parameters}}
+void h(int *a __attribute__((maybe_undef("RandomString"; // expected-error {{'maybe_undef' attribute takes no arguments}}
+
+// Type annotations.
+int __attribute__((maybe_undef)) ta; // expected-warning {{'maybe_undef' attribute only applies to parameters}}
+
+// Typedefs.
+typedef int callback(char *) __attribute__((maybe_undef)); // expected-warning {{'maybe_undef' attribute only applies to parameters}}
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -83,6 +83,7 @@
 // CHECK-NEXT: Lockable (SubjectMatchRule_record)
 // CHECK-NEXT: MIGServerRoutine (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_block)
 // CHECK-NEXT: MSStruct (SubjectMatchRule_record)
+// CHECK-NEXT: MaybeUndef (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: MicroMips (SubjectMatchRule_function)
 // CHECK-NEXT: MinSize (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: MinVectorWidth (SubjectMatchRule_function)
Index: clang/test/CodeGenHIP/maybe_undef-attr-verify.hip
===
--- /dev/null
+++ clang/test/CodeGenHIP/maybe_undef-attr-verify.hip
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -no-opaque-pointers -triple amdgcn-amd-amdhsa -target-cpu gfx906 -x hip -fcuda-is-device -emit-llvm  %s \
+// RUN:   -o - | FileCheck %s
+
+// CHECK: define dso_local amdgpu_kernel void @_Z13shufflekernelv()
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP1:%.*]] = alloca i32, align 4, addrspace(5)
+// CHECK-NEXT:[[TMP2:%.*]] = alloca i32, align 4, addrspace(5)
+// CHECK-NEXT:[[TMP3:%.*]] = addrspacecast i32 addrspace(5)* [[TMP1:%.*]] to i32*
+// CHECK-NEXT:[[TMP4:%.*]] = addrspacecast i32 addrspace(5)* [[TMP2:%.*]] to i32*
+// CHECK-NEXT:[[TMP5:%.*]] = load i32, i32* [[TMP3:%.*]], align 4
+// CHECK-NEXT:[[TMP6:%.*]] = freeze i32 [[TMP5:%.*]]
+// CHECK-NEXT:%call = call noundef i32 @_Z11__shfl_synciii(i32 noundef [[TMP6:%.*]], i32 noundef 64, i32 noundef 0) #4
+// CHECK-NEXT:store i32 %call, i32* [[TMP4:%.*]], align 4
+// CHECK-NEXT:  ret void
+
+// CHECK: define linkonce_odr noundef i32 @_Z11__shfl_synciii(i32 noundef [[TMP1:%.*]], i32 noundef [[TMP2:%.*]], i32 noundef [[TMP3:%.*]])
+
+#define __global__ __attribute__((global))
+#define __device__ __attribute__((device))
+#define __maybe_undef __attribute__((maybe_undef))
+#define WARP_SIZE 64
+
+static constexpr int warpSize = __AMDGCN_WAVEFRONT_SIZE;
+
+__device__ static inline unsigned int __lane_id() {
+return  __builtin_amdgcn_mbcnt_hi(
+-1, __builtin_amdgcn_mbcnt_lo(-1, 0));
+}
+
+__device__
+inline
+int __shfl_sync(int __maybe_undef var, int src_lane, int width = warpSize) {
+int self = __lane_id();
+int index = src_lane + (self & ~(width-1));
+return __builtin_amdgcn_ds_bpermute(index<<2, var);
+}
+
+__global__ void
+shufflekernel()
+{
+int t;
+int res;
+res = __shfl_sync(t, WARP_SIZE, 0);
+}
Index: clang/test/CodeGen/attr-maybeundef.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-maybeundef.c
@@ -0,0 +1,109 @@
+// RUN: %clang_cc1 -no-opaque-pointers -emit-llvm %s -o - | FileCheck %s
+
+#define __maybe_undef __attribute__((maybe_undef))
+
+// CHECK:  define dso_local void @t1(i32 noundef [[TMP1:%.*]], i32 noundef [[TMP2:%.*]], i32 noundef [[TMP3:%.*]])
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   [[TMP4:%.*]] = alloca i32, align 4
+/

[PATCH] D130523: [pseudo] Perform unconstrained recovery prior to completion.

2022-07-28 Thread Haojian Wu via Phabricator via cfe-commits

hokein added inline comments.



Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:621
 // Consume the token.
 glrShift(Heads, Terminals[I], Params, Lang, NextHeads);
 

hokein wrote:
> I think we can move the Line634 `Heads.resize(HeadsPartition)` before the 
> `glrShift()` as we only do shift on the nearly-created heads, we might gain 
> some performance back.
oops, my previous comment is incorrect, here we want the second part of the 
partition; while on recovery, we want the first part of partition.

we can pass `llvm::ArrayRef(Heads).drop_front(HeadsPartition);` as the Heads to `glrShift`.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130523

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

[PATCH] D130690: [clangd][NFCI] Store TUPath inside ParsedAST

2022-07-28 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM and a few nits. Could you please add a summary of our offline discussion 
here.




Comment at: clang-tools-extra/clangd/ParsedAST.cpp:763
 
-ParsedAST::ParsedAST(llvm::StringRef Version,
+ParsedAST::ParsedAST(PathRef TUPath, llvm::StringRef Version,
  std::shared_ptr Preamble,

NIT: use `Path`.
 this allows the callers to avoid an extra allocation, e.g. if they already 
allocated a `std::string` and they can `std::move` into the constructor.



Comment at: clang-tools-extra/clangd/ParsedAST.h:112
 
+  /// Returns the path passed by the caller when building this AST.
+  PathRef tuPath() const { return TUPath; }

NIT: could you add a comment how this is used, e.g. `this path is used as a 
hint when canonicalize the path`.
Also useful to document that we expect this to be absolute/"canonical".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130690

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

[PATCH] D130016: [HLSL] Add __builtin_hlsl_create_handle

2022-07-28 Thread Chris Bieneman 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 rGfe13002bb37c: [HLSL] Add __builtin_hlsl_create_handle 
(authored by beanz).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130016

Files:
  clang/include/clang/Basic/Builtins.def
  clang/test/CodeGenHLSL/builtins/create_handle.hlsl
  llvm/include/llvm/IR/IntrinsicsDirectX.td


Index: llvm/include/llvm/IR/IntrinsicsDirectX.td
===
--- llvm/include/llvm/IR/IntrinsicsDirectX.td
+++ llvm/include/llvm/IR/IntrinsicsDirectX.td
@@ -17,4 +17,6 @@
 def int_dx_thread_id_in_group : Intrinsic<[llvm_i32_ty], [llvm_i32_ty], 
[IntrNoMem, IntrWillReturn]>;
 def int_dx_flattened_thread_id_in_group : Intrinsic<[llvm_i32_ty], [], 
[IntrNoMem, IntrWillReturn]>;
 
+def int_dx_create_handle : ClangBuiltin<"__builtin_hlsl_create_handle">,
+Intrinsic<[ llvm_ptr_ty ], [llvm_i8_ty], [IntrNoMem, IntrWillReturn]>;
 }
Index: clang/test/CodeGenHLSL/builtins/create_handle.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/builtins/create_handle.hlsl
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -emit-llvm 
-disable-llvm-passes -o - %s | FileCheck %s
+
+void fn() {
+  (void)__builtin_hlsl_create_handle(0);
+}
+
+// CHECK: call ptr @llvm.dx.create.handle(i8 0)
Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -1700,6 +1700,7 @@
 
 // HLSL
 LANGBUILTIN(__builtin_hlsl_wave_active_count_bits, "Uib", "nc", HLSL_LANG)
+LANGBUILTIN(__builtin_hlsl_create_handle, "v*Uc", "nc", HLSL_LANG)
 
 // Builtins for XRay
 BUILTIN(__xray_customevent, "vcC*z", "")


Index: llvm/include/llvm/IR/IntrinsicsDirectX.td
===
--- llvm/include/llvm/IR/IntrinsicsDirectX.td
+++ llvm/include/llvm/IR/IntrinsicsDirectX.td
@@ -17,4 +17,6 @@
 def int_dx_thread_id_in_group : Intrinsic<[llvm_i32_ty], [llvm_i32_ty], [IntrNoMem, IntrWillReturn]>;
 def int_dx_flattened_thread_id_in_group : Intrinsic<[llvm_i32_ty], [], [IntrNoMem, IntrWillReturn]>;
 
+def int_dx_create_handle : ClangBuiltin<"__builtin_hlsl_create_handle">,
+Intrinsic<[ llvm_ptr_ty ], [llvm_i8_ty], [IntrNoMem, IntrWillReturn]>;
 }
Index: clang/test/CodeGenHLSL/builtins/create_handle.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/builtins/create_handle.hlsl
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s
+
+void fn() {
+  (void)__builtin_hlsl_create_handle(0);
+}
+
+// CHECK: call ptr @llvm.dx.create.handle(i8 0)
Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -1700,6 +1700,7 @@
 
 // HLSL
 LANGBUILTIN(__builtin_hlsl_wave_active_count_bits, "Uib", "nc", HLSL_LANG)
+LANGBUILTIN(__builtin_hlsl_create_handle, "v*Uc", "nc", HLSL_LANG)
 
 // Builtins for XRay
 BUILTIN(__xray_customevent, "vcC*z", "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] fe13002 - [HLSL] Add __builtin_hlsl_create_handle

2022-07-28 Thread Chris Bieneman via cfe-commits


Author: Chris Bieneman
Date: 2022-07-28T09:16:11-05:00
New Revision: fe13002bb37caf7425dfdb56b8f891c3f33b54b6

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

LOG: [HLSL] Add __builtin_hlsl_create_handle

This is pretty straightforward, it just adds a builtin to return a
pointer to a resource handle. This maps to a dx intrinsic.

The shape of this builtin and the underlying intrinsic will likely
shift a bit as this implementation becomes more feature complete, but
this is a good basis to get started.

Depends on D128569.

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

Added: 
clang/test/CodeGenHLSL/builtins/create_handle.hlsl

Modified: 
clang/include/clang/Basic/Builtins.def
llvm/include/llvm/IR/IntrinsicsDirectX.td

Removed: 




diff  --git a/clang/include/clang/Basic/Builtins.def 
b/clang/include/clang/Basic/Builtins.def
index f19807dbbb0bb..c67f0dfe9ab04 100644
--- a/clang/include/clang/Basic/Builtins.def
+++ b/clang/include/clang/Basic/Builtins.def
@@ -1700,6 +1700,7 @@ LANGBUILTIN(__builtin_get_device_side_mangled_name, 
"cC*.", "ncT", CUDA_LANG)
 
 // HLSL
 LANGBUILTIN(__builtin_hlsl_wave_active_count_bits, "Uib", "nc", HLSL_LANG)
+LANGBUILTIN(__builtin_hlsl_create_handle, "v*Uc", "nc", HLSL_LANG)
 
 // Builtins for XRay
 BUILTIN(__xray_customevent, "vcC*z", "")

diff  --git a/clang/test/CodeGenHLSL/builtins/create_handle.hlsl 
b/clang/test/CodeGenHLSL/builtins/create_handle.hlsl
new file mode 100644
index 0..61226c2b54e72
--- /dev/null
+++ b/clang/test/CodeGenHLSL/builtins/create_handle.hlsl
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -emit-llvm 
-disable-llvm-passes -o - %s | FileCheck %s
+
+void fn() {
+  (void)__builtin_hlsl_create_handle(0);
+}
+
+// CHECK: call ptr @llvm.dx.create.handle(i8 0)

diff  --git a/llvm/include/llvm/IR/IntrinsicsDirectX.td 
b/llvm/include/llvm/IR/IntrinsicsDirectX.td
index 57c47a15bd70a..80e5b7919b9c6 100644
--- a/llvm/include/llvm/IR/IntrinsicsDirectX.td
+++ b/llvm/include/llvm/IR/IntrinsicsDirectX.td
@@ -17,4 +17,6 @@ def int_dx_group_id : Intrinsic<[llvm_i32_ty], [llvm_i32_ty], 
[IntrNoMem, IntrWi
 def int_dx_thread_id_in_group : Intrinsic<[llvm_i32_ty], [llvm_i32_ty], 
[IntrNoMem, IntrWillReturn]>;
 def int_dx_flattened_thread_id_in_group : Intrinsic<[llvm_i32_ty], [], 
[IntrNoMem, IntrWillReturn]>;
 
+def int_dx_create_handle : ClangBuiltin<"__builtin_hlsl_create_handle">,
+Intrinsic<[ llvm_ptr_ty ], [llvm_i8_ty], [IntrNoMem, IntrWillReturn]>;
 }



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

[PATCH] D130709: MSVC compatibility mode: fix error on unqualified templated base class initialization in case of partial specialization

2022-07-28 Thread Fred Tingaud via Phabricator via cfe-commits

frederic-tingaud-sonarsource created this revision.
frederic-tingaud-sonarsource added a reviewer: rnk.
Herald added a project: All.
frederic-tingaud-sonarsource requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

I introduced a patch to handle unqualified templated base class initialization 
in MSVC compatibility mode: 
https://reviews.llvm.org/rGc894e85fc64dd8d83b460de81080fff93c5ca334
We identified a problem with this patch in the case where the base class is 
partially specialized, which can lead to triggering an assertion in the case of 
a mix between types and values.
The minimal test case is:

  cpp
  template  class Vec {};
  
  template  class Index : public Vec {
Index() : Vec() {}
  };
  template class Index<0>;

The detailed problem is that I was using the InjectedClassNameSpecialization, 
to which the class template arguments were then applied in order. But in the 
process, we were losing all the partial specializations of the base class and 
creating an index mismatch between the expected and passed arguments.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130709

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaTemplate/ms-unqualified-base-class.cpp


Index: clang/test/SemaTemplate/ms-unqualified-base-class.cpp
===
--- clang/test/SemaTemplate/ms-unqualified-base-class.cpp
+++ clang/test/SemaTemplate/ms-unqualified-base-class.cpp
@@ -83,3 +83,37 @@
 
   return I;
 }
+
+template  class Vec {}; // expected-note {{template 
is declared here}}
+
+template  class Index : public Vec {
+  // after-error@+1 {{member initializer 'Vec' does not name a non-static data 
member or base class}}
+  Index() : Vec() {} // before-warning {{unqualified base initializer of class 
templates is a Microsoft extension}}
+};
+
+template class Index<0>;
+
+template  class Array : public Vec {
+  // after-error@+1 {{member initializer 'Vec' does not name a non-static data 
member or base class}}
+  Array() : Vec() {} // before-warning {{unqualified base initializer of class 
templates is a Microsoft extension}}
+};
+
+template class Array;
+
+template  class Wrong : public Vec {
+  Wrong() : NonExistent() {} // expected-error {{member initializer 
'NonExistent' does not name a non-static data member or base class}}
+};
+
+template class Wrong;
+
+template  class Wrong2 : public Vec {
+  Wrong2() : Vec() {} // expected-error {{too few template arguments for 
class template 'Vec'}}
+};
+
+template class Wrong2;
+
+template  class Wrong3 : public Vec {
+  Wrong3() : Base() {} // expected-error {{member initializer 'Base' does not 
name a non-static data member or base class}}
+};
+
+template class Wrong3;
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -4308,11 +4308,21 @@
   }
 
   if (getLangOpts().MSVCCompat && !getLangOpts().CPlusPlus20) {
-auto UnqualifiedBase = R.getAsSingle();
-if (UnqualifiedBase) {
-  Diag(IdLoc, diag::ext_unqualified_base_class)
-  << SourceRange(IdLoc, Init->getSourceRange().getEnd());
-  BaseType = UnqualifiedBase->getInjectedClassNameSpecialization();
+if (auto UnqualifiedBase = R.getAsSingle()) {
+  auto *TempSpec = cast(
+  UnqualifiedBase->getInjectedClassNameSpecialization());
+  TemplateName TN = TempSpec->getTemplateName();
+  for (auto const &Base : ClassDecl->bases()) {
+QualType BT = cast(Base.getType())->getNamedType();
+const auto *BaseTemplate = 
dyn_cast(BT);
+if (BaseTemplate && Context.hasSameTemplateName(
+BaseTemplate->getTemplateName(), TN)) {
+  Diag(IdLoc, diag::ext_unqualified_base_class)
+  << SourceRange(IdLoc, Init->getSourceRange().getEnd());
+  BaseType = BT;
+  break;
+}
+  }
 }
   }
 


Index: clang/test/SemaTemplate/ms-unqualified-base-class.cpp
===
--- clang/test/SemaTemplate/ms-unqualified-base-class.cpp
+++ clang/test/SemaTemplate/ms-unqualified-base-class.cpp
@@ -83,3 +83,37 @@
 
   return I;
 }
+
+template  class Vec {}; // expected-note {{template is declared here}}
+
+template  class Index : public Vec {
+  // after-error@+1 {{member initializer 'Vec' does not name a non-static data member or base class}}
+  Index() : Vec() {} // before-warning {{unqualified base initializer of class templates is a Microsoft extension}}
+};
+
+template class Index<0>;
+
+template  class Array : public Vec {
+  // after-error@+1 {{member initializer 'Vec' does not name a non-static data member or base class}}
+  Array() : Vec() {} // before-warning {{unqualified base initializer of class

[PATCH] D130710: [ASTWriter] Provide capability to output a PCM/PCH file that does not write out information about its output path

2022-07-28 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits

akyrtzi created this revision.
Herald added a project: All.
akyrtzi requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is useful to enable sharing of the same PCH file even when it's intended 
for a different output path.

The only information this option disables writing is for `ORIGINAL_PCH_DIR` 
record which is treated as optional and (when present) used as fallback for 
resolving input file paths relative to it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130710

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/GeneratePCH.cpp
  clang/test/PCH/pch-output-path-independent.c

Index: clang/test/PCH/pch-output-path-independent.c
===
--- /dev/null
+++ clang/test/PCH/pch-output-path-independent.c
@@ -0,0 +1,6 @@
+// RUN: rm -rf %t && mkdir -p %t/a %t/b
+
+// RUN: %clang_cc1 -triple x86_64-apple-macos11 -emit-pch %s -o %t/a/t1.pch -fpcm-output-path-independent
+// RUN: %clang_cc1 -triple x86_64-apple-macos11 -emit-pch %s -o %t/b/t2.pch -fpcm-output-path-independent
+
+// RUN: diff %t/a/t1.pch %t/b/t2.pch
Index: clang/lib/Serialization/GeneratePCH.cpp
===
--- clang/lib/Serialization/GeneratePCH.cpp
+++ clang/lib/Serialization/GeneratePCH.cpp
@@ -25,13 +25,14 @@
 StringRef OutputFile, StringRef isysroot, std::shared_ptr Buffer,
 ArrayRef> Extensions,
 bool AllowASTWithErrors, bool IncludeTimestamps,
-bool ShouldCacheASTInMemory)
+bool ShouldCacheASTInMemory, bool OutputPathIndependent)
 : PP(PP), OutputFile(OutputFile), isysroot(isysroot.str()),
   SemaPtr(nullptr), Buffer(std::move(Buffer)), Stream(this->Buffer->Data),
   Writer(Stream, this->Buffer->Data, ModuleCache, Extensions,
  IncludeTimestamps),
   AllowASTWithErrors(AllowASTWithErrors),
-  ShouldCacheASTInMemory(ShouldCacheASTInMemory) {
+  ShouldCacheASTInMemory(ShouldCacheASTInMemory),
+  OutputPathIndependent(OutputPathIndependent) {
   this->Buffer->IsComplete = false;
 }
 
@@ -70,7 +71,7 @@
   // For serialization we are lenient if the errors were
   // only warn-as-error kind.
   PP.getDiagnostics().hasUncompilableErrorOccurred(),
-  ShouldCacheASTInMemory);
+  ShouldCacheASTInMemory, OutputPathIndependent);
 
   Buffer->IsComplete = true;
 }
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -4483,7 +4483,8 @@
 ASTFileSignature ASTWriter::WriteAST(Sema &SemaRef, StringRef OutputFile,
  Module *WritingModule, StringRef isysroot,
  bool hasErrors,
- bool ShouldCacheASTInMemory) {
+ bool ShouldCacheASTInMemory,
+ bool OutputPathIndependent) {
   WritingAST = true;
 
   ASTHasCompilerErrors = hasErrors;
@@ -4499,8 +4500,9 @@
   Context = &SemaRef.Context;
   PP = &SemaRef.PP;
   this->WritingModule = WritingModule;
-  ASTFileSignature Signature =
-  WriteASTCore(SemaRef, isysroot, OutputFile, WritingModule);
+  ASTFileSignature Signature = WriteASTCore(
+  SemaRef, isysroot, OutputPathIndependent ? StringRef() : OutputFile,
+  WritingModule);
   Context = nullptr;
   PP = nullptr;
   this->WritingModule = nullptr;
Index: clang/lib/Frontend/FrontendActions.cpp
===
--- clang/lib/Frontend/FrontendActions.cpp
+++ clang/lib/Frontend/FrontendActions.cpp
@@ -140,7 +140,8 @@
   CI.getPreprocessor(), CI.getModuleCache(), OutputFile, Sysroot, Buffer,
   FrontendOpts.ModuleFileExtensions,
   CI.getPreprocessorOpts().AllowPCHWithCompilerErrors,
-  FrontendOpts.IncludeTimestamps, +CI.getLangOpts().CacheGeneratedPCH));
+  FrontendOpts.IncludeTimestamps, +CI.getLangOpts().CacheGeneratedPCH,
+  FrontendOpts.OutputPathIndependentPCM));
   Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(
   CI, std::string(InFile), OutputFile, std::move(OS), Buffer));
 
@@ -203,7 +204,9 @@
   /*IncludeTimestamps=*/
   +CI.getFrontendOpts().BuildingImplicitModule,
   /*ShouldCacheASTInMemory=*/
-  +CI.getFrontendOpts().BuildingImplicitModule));
+  +CI.getFrontendOpts().BuildingImplicitModule,
+  /*OutputPathIndependent=*/
+  +CI.getFrontendOpts().OutputPathIndependentPCM));
   Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGe

[PATCH] D130270: [clang][dataflow] Use a dedicated bool to encode which branch was taken

2022-07-28 Thread Stanislav Gatev via Phabricator via cfe-commits

sgatev accepted this revision.
sgatev added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:103
   if (Value *MergedVal = MergedEnv.createValue(Type))
 if (Model.merge(Type, *Val1, Env1, *Val2, Env2, *MergedVal, MergedEnv))
   return MergedVal;

We should probably pass `TookSecondBranch` here so that models can also use it 
in the implementation of `merge`. Could be in a follow up, but let's leave a 
FIXME.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130270

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

[PATCH] D130688: [Driver][Sparc] Default to -mcpu=v9 for SparcV8 on Linux

2022-07-28 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits

glaubitz added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/Sparc.cpp:139
+  if (Triple.getArch() == llvm::Triple::sparc &&
+  (Triple.isOSSolaris() || Distro(D.getVFS(), Triple).IsDebian()))
 return "v9";

Can we do it "IsLinux()" instead of "IsDebian()"?

I think Gentoo should profit from this change as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130688

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

[PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

2022-07-28 Thread John Ericson via Phabricator via cfe-commits

Ericson2314 added inline comments.



Comment at: clang/include/clang/Config/config.h.cmake:57
+/* Multilib basename for libdir. */
+#define CLANG_INSTALL_LIBDIR_BASENAME "${CLANG_INSTALL_LIBDIR_BASENAME}"
 

compnerd wrote:
> Does this not potentially break downstreams?
At the top of this file it says

```
/* This generated file is for internal use. Do not include it from headers. */
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130586

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

[PATCH] D130614: [C++20] [Modules] Merge same concept decls in global module fragment

2022-07-28 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

Thanks for the thorough explanation and testing.
LGTM.




Comment at: clang/lib/Sema/SemaDecl.cpp:1738
+// Return true if the redefinition is not allowed. Return false otherwise.
+bool Sema::CheckRedefinitionInModule(const NamedDecl *New,
+ const NamedDecl *Old) const {

NIT: maybe rename to `IsRedefinitionInModule`?
I would expect `Check...` to produce a diagnostic itself. Having `Is...` in the 
name also removes any confusion about whether the `true` as a return value 
indicates an error or not.




Comment at: clang/lib/Sema/SemaDecl.cpp:1747
+
+  Module *NewM = New->getOwningModule();
+  Module *OldM = Old->getOwningModule();

NIT: maybe accept the modules directly?



Comment at: clang/test/Modules/merge-concepts.cppm:22
+// Testing with module map modules. It is unclear about the relation ship 
between Clang modules and
+// C++20 Named Modules. Will them co_exist? Or will them be mutual from each 
other?
+// The test here is for primarily coverity.

NIT on wording


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

https://reviews.llvm.org/D130614

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

[PATCH] D126880: [clang-tidy] Add cppcoreguidelines-avoid-const-or-ref-data-members check

2022-07-28 Thread Nathan James via Phabricator via cfe-commits

njames93 added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp:103-104
+struct WithAlias {
+  ConstType c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' is const qualified
+  RefType lr;

I feel like this could potentially confuse users. Maybe we should walk the 
alias to show where the type was declared to be const/reference.
Or, not a fan of this choice, don't diagnose these unless the user opts into 
this behaviour.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-const-or-ref-data-members.cpp:136-157
+template 
+struct TemplatedOk {
+  T t;
+};
+
+template 
+struct TemplatedConst {

This whole block displeases me. Warning on the instantiation isn't a great idea 
and could confuse users.
Would again need some expansion notes to explain why the warning is being 
triggered, especially when if for 99% of the uses one of these structs has a T 
which isn't const or a reference.
Failing that just disabling the check in template instantiations would also fix 
the issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126880

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

[PATCH] D122768: [Clang][C++20] Support capturing structured bindings in lambdas

2022-07-28 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/include/clang/AST/Stmt.h:62-63
 class Token;
 class VarDecl;
+class ValueDecl;
 

We usually keep these alphabetical.



Comment at: clang/lib/AST/ExprCXX.cpp:1214-1216
+  return (C->capturesVariable() && isa(C->getCapturedVar()) &&
+  cast(C->getCapturedVar())->isInitCapture() &&
   (getCallOperator() == C->getCapturedVar()->getDeclContext()));

I think early returns help make this a bit more clear.



Comment at: clang/lib/AST/StmtPrinter.cpp:2167
 if (Node->isInitCapture(C)) {
-  VarDecl *D = C->getCapturedVar();
+  VarDecl *D = cast(C->getCapturedVar());
 





Comment at: clang/lib/Analysis/AnalysisDeclContext.cpp:173
+ValueDecl *VD = LC.getCapturedVar();
+if (isSelfDecl(dyn_cast(VD)))
   return dyn_cast(VD);

This looks dangerous -- `isSelfDecl()` uses the pointer variable in ways that 
would be Bad News for null pointers.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9060
 continue;
-  const VarDecl *VD = LC.getCapturedVar();
+  const VarDecl *VD = cast(LC.getCapturedVar());
   if (LC.getCaptureKind() != LCK_ByRef && !VD->getType()->isPointerType())

cor3ntin wrote:
> cor3ntin wrote:
> > I'm not sure we currently prohibit capturing a structured binding in 
> > openmp, i should fix that
> I disabled the feature in OpenMP mode, as this needs more investigation
Ping @jdoerfert -- do you happen to know what's up here?



Comment at: clang/lib/Sema/ScopeInfo.cpp:223-224
   // init-capture.
-  return !isNested() && isVariableCapture() && getVariable()->isInitCapture();
+  return !isNested() && isVariableCapture() && isa(getVariable()) &&
+ cast(getVariable())->isInitCapture();
 }

This also has a bad code smell for `isa<>` followed by `cast<>`, but I'm not 
certain if the rewrite is actually better or not.



Comment at: clang/lib/Sema/SemaExpr.cpp:16393
 
-VarDecl *Var = Cap.getVariable();
+VarDecl *Var = cast(Cap.getVariable());
 Expr *CopyExpr = nullptr;

Is `cast<>` safe here?



Comment at: clang/lib/Sema/SemaExpr.cpp:18324
+
+  assert((isa(Var) || isa(Var)) &&
+ "Only variables and structured bindings can be captured");





Comment at: clang/lib/Sema/SemaExpr.cpp:18527
+  BindingDecl *BD = nullptr;
+  if (VarDecl *V = dyn_cast_or_null(Var)) {
+if (V->getInit())

I'm assuming, given that the function didn't used to accept nullptr for `Var` 
and the `else if` is using `dyn_cast<>`.



Comment at: clang/lib/Sema/SemaExpr.cpp:18531
+  } else if ((BD = dyn_cast(Var))) {
+ME = dyn_cast_or_null(BD->getBinding());
+  }

Can this return nullptr?



Comment at: clang/lib/Sema/SemaExpr.cpp:18534-18535
+
+  // Capturing a bitfield by reference is not allowed
+  // except in OpenMP mode
+  if (ByRef && ME &&





Comment at: clang/lib/Sema/SemaExpr.cpp:18539
+   !S.isOpenMPCapturedDecl(Var))) {
+FieldDecl *FD = dyn_cast_or_null(ME->getMemberDecl());
+if (FD &&

Can this return nullptr?



Comment at: clang/lib/Sema/SemaExpr.cpp:18551
+  }
+  // FIXME: We should support capturing structured bindings in OpenMP
+  if (!Invalid && BD && S.LangOpts.OpenMP) {





Comment at: clang/lib/Sema/SemaExpr.cpp:18749
   DeclContext *VarDC = Var->getDeclContext();
-  if (Var->isInitCapture())
-VarDC = VarDC->getParent();
+  VarDecl *VD = dyn_cast(Var);
+  if (VD) {





Comment at: clang/lib/Sema/SemaExpr.cpp:18750-18756
+  if (VD) {
+if (VD && VD->isInitCapture())
+  VarDC = VarDC->getParent();
+  } else {
+VD = dyn_cast(
+cast(Var)->getDecomposedDecl());
+  }





Comment at: clang/lib/Sema/SemaInit.cpp:7851
+bool InitCapture =
+isa(VD) && cast(VD)->isInitCapture();
 Diag(Elem.Capture->getLocation(), 
diag::note_lambda_capture_initializer)

shafik wrote:
> cor3ntin wrote:
> > shafik wrote:
> > > I see we are doing this kind of check to see if we have a `VarDecl` and 
> > > then check if it is an init capture and I wish there was a way not to 
> > > repeat this but I don't see it.
> > I considered having a function In ValueDecl, what do you think?
> I thought about that but when I looked at `ValueDecl` it seemed pretty 
> lightweight.
> 
> @aaron.ballman wdyt is `ValueDecl` the right place or is this code repetition 
> ok? 
I think it makes sense to sink this into `ValueDecl` given how often it's come 
up. We really try to discourage people from doing `isa<>` followed by a 
`cast<>` as that's a bad cod

[PATCH] D130224: [Clang][Attribute] Introduce maybe_undef attribute for function arguments which accepts undef values

2022-07-28 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/test/CodeGen/attr-maybeundef-template.cpp:3
+
+// CHECK-LABEL: @_Z5test4IfEvT_(
+// CHECK-NEXT:  entry:

This mangling works for Itanium targets, but doesn't match MSVC targets, which 
is why the test is failing on Windows.



Comment at: clang/test/CodeGen/attr-maybeundef.c:48
+
+// CHECK: declare void @VariadicFunction(i32 noundef, ...)
+

It looks like dso_local is missing here on Windows and expected to be missing 
on Linux? Not certain what's up with that...


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

https://reviews.llvm.org/D130224

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

[PATCH] D122768: [Clang][C++20] Support capturing structured bindings in lambdas

2022-07-28 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added inline comments.



Comment at: clang/lib/AST/ExprCXX.cpp:1214-1216
+  return (C->capturesVariable() && isa(C->getCapturedVar()) &&
+  cast(C->getCapturedVar())->isInitCapture() &&
   (getCallOperator() == C->getCapturedVar()->getDeclContext()));

aaron.ballman wrote:
> I think early returns help make this a bit more clear.
I might suggest making that:

```
if (const auto *VD = dyn_castgetCapturedVar())
   return VD->...
return false;
```

But otherwise agree with the suggestion.



Comment at: clang/lib/Analysis/AnalysisDeclContext.cpp:173
+ValueDecl *VD = LC.getCapturedVar();
+if (isSelfDecl(dyn_cast(VD)))
   return dyn_cast(VD);

aaron.ballman wrote:
> This looks dangerous -- `isSelfDecl()` uses the pointer variable in ways that 
> would be Bad News for null pointers.
Yep, `isSelfDecl` seems to do:

`return isa(VD) && VD->getName() == "self";`

`isa` isn't nullptr safe, we have `isa_and_nonnull` for that (if we want to 
update `isSelfDecl`).



Comment at: clang/lib/Sema/SemaDecl.cpp:14594-14596
+  ValueDecl *VD = C.getCapturedVar();
+  if (VarDecl *Var = dyn_cast(VD)) {
+if (Var->isInitCapture())




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122768

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

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

2022-07-28 Thread Joseph Huber via Phabricator via cfe-commits

jhuber6 added a comment.

I still think we shouldn't bother making all the noise containing the original 
name. Just mangle it and treat it like every other declare target variable 
without introducing any extra complexity. These symbols never should've been 
emitted in the first place so I'm not concerned if someone cracks open a binary 
and sees some ugly names. CUDA and HIP just mangle the declaration directly as 
far as I'm aware.




Comment at: clang/lib/CodeGen/TargetInfo.cpp:9439
 const Decl *D, llvm::GlobalValue *GV, CodeGen::CodeGenModule &M) const {
+  // Make sure any variable with OpenMP declare target is visible to runtime
+  // except for those with hidden visibility

jhuber6 wrote:
> ssquare08 wrote:
> > jhuber6 wrote:
> > > Just spitballing, is it possible to do this when we make the global 
> > > instead?
> > This is something I was wondering as well.  In 
> > CodeGenModule::GetOrCreateLLVMGlobal, when it creates a new global 
> > variable, it always uses the llvm::GlobalValue::ExternalLinkage. Seems like 
> > this changes somewhere later to internal for static globals. Do you know 
> > where that would be?
> I'm not exactly sure, I remember deleting some code in D117806 that did 
> something like that, albeit incorrectly. But I'm not sure if you'd have the 
> necessary information to check whether or not there are updates attached to 
> it. We don't want to externalize things if we don't need to, otherwise we'd 
> get a lot of our device runtime variables with external visibility that now 
> can't be optimized out.
Were you able to find a place for this when we generate the variable? You 
should be able to do something similar to the patch above if it's a declare 
target static to force it to have external visibility, but as mentioned before 
I would prefer we only do this if necessary which might take some extra 
analysis.



Comment at: clang/test/OpenMP/declare_target_visibility_codegen.cpp:11-12
 // HOST: @z = global i32 0
-// HOST-NOT: @.omp_offloading.entry.c
-// HOST-NOT: @.omp_offloading.entry.x
+// HOST: @.omp_offloading.entry.c__static__{{[0-9a-z]+_[0-9a-z]+}}
+// HOST: @.omp_offloading.entry.x__static__{{[0-9a-z]+_[0-9a-z]+}}
 // HOST-NOT: @.omp_offloading.entry.y

ssquare08 wrote:
> ssquare08 wrote:
> > jhuber6 wrote:
> > > If there are no updates between the host and device we can keep these 
> > > static without emitting an offloading entry.
> > That 's a good point. I'll fix that.
> I thought about this more and I think the behavior for these  declare target 
> static globals should be the same as the other declare target. Checking for 
> update is not enough because users could also map these variables. For 
> update, it could be mapped with a pointer or the users could pass address of 
> these variables to an external function. Please let me know what you think of 
> these cases below:
> ```
> #pragma omp declare target
> static int x[10];
> #pragma omp end declare target
> 
> //case 1
> #pragma omp target update to(x)
> 
> //case 2
> int* y = &x[2];
> #pragma omp target update to(y[0])
> 
> //case 3
> #pragma omp target map(always to:x)
> {
>  x[0]= 111;
> }
> 
> //case 4
> #pragma omp target
> { 
>   foo(&x[3]);
> }
> ```
We should still be able to do this if there are either no updates at all in the 
module, or if the declare type is `nohost`. Doing anything more complicated 
would require some optimizations between the host and device we can't do yet. 
I'm making this point because making these statics external is a performance 
regression so we should only do it when needed. To that end we may even want a 
flag that entirely disables this feature.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129694

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

[PATCH] D129752: Thread safety analysis: Handle additional cast in scoped capability construction

2022-07-28 Thread Aaron Puchert via Phabricator via cfe-commits

aaronpuchert marked an inline comment as done.
aaronpuchert added a comment.

In D129752#3682977 , @aaron.ballman 
wrote:

> Do you think this warrants a release note (or did it close any open issues in 
> the tracker)?

It's probably too technical/minor to be worth mentioning. It also doesn't close 
any issues that I'm aware of, I mostly did it in preparation for D129755 
, where I'm reusing the new function.




Comment at: clang/lib/Analysis/ThreadSafety.cpp:2091-2097
+  if (auto *CE = dyn_cast(E))
+if (CE->getCastKind() == CK_NoOp)
+  E = CE->getSubExpr()->IgnoreParens();
+  if (auto *CE = dyn_cast(E))
+if (CE->getCastKind() == CK_ConstructorConversion ||
+CE->getCastKind() == CK_UserDefinedConversion)
+  E = CE->getSubExpr();

aaron.ballman wrote:
> I almost wonder if we should just turn this into a while loop rather than two 
> casts in a specific order. However, I can't think of a situation where we'd 
> need the loop, so I think this is fine. At the very least, it's incremental 
> progress!
They should always be in that order, or at least until we get to the 
`CXXConstructExpr` or `CXXMemberCallExpr` that we want (there could be another 
standard conversion sequence for the argument inside of that, but that's not 
our concern here). The reason is that this constructor or conversion function 
call is the `CK_ConstructorConversion` or `CK_UserDefinedConversion`, 
respectively, while the remaining standard conversion sequence (to which the 
`NoOp` belongs) [happens after/above 
that](https://eel.is/c++draft/over.best.ics#over.ics.user).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129752

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

[PATCH] D130636: [clangd] Upgrade vlog() to log() for preamble build stats

2022-07-28 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet accepted this revision.
kadircet added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/clangd/Preamble.cpp:555
   if (BuiltPreamble) {
-vlog("Built preamble of size {0} for file {1} version {2} in {3} seconds",
+log("Built preamble of size {0} for file {1} version {2} in {3} seconds",
  BuiltPreamble->getSize(), FileName, Inputs.Version,

what about also moving this log into TUScheduler.cpp::reportPreamble, in which 
we can also log whether this is a "first preamble" or not?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130636

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

[PATCH] D129752: Thread safety analysis: Handle additional cast in scoped capability construction

2022-07-28 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/lib/Analysis/ThreadSafety.cpp:2091-2097
+  if (auto *CE = dyn_cast(E))
+if (CE->getCastKind() == CK_NoOp)
+  E = CE->getSubExpr()->IgnoreParens();
+  if (auto *CE = dyn_cast(E))
+if (CE->getCastKind() == CK_ConstructorConversion ||
+CE->getCastKind() == CK_UserDefinedConversion)
+  E = CE->getSubExpr();

aaronpuchert wrote:
> aaron.ballman wrote:
> > I almost wonder if we should just turn this into a while loop rather than 
> > two casts in a specific order. However, I can't think of a situation where 
> > we'd need the loop, so I think this is fine. At the very least, it's 
> > incremental progress!
> They should always be in that order, or at least until we get to the 
> `CXXConstructExpr` or `CXXMemberCallExpr` that we want (there could be 
> another standard conversion sequence for the argument inside of that, but 
> that's not our concern here). The reason is that this constructor or 
> conversion function call is the `CK_ConstructorConversion` or 
> `CK_UserDefinedConversion`, respectively, while the remaining standard 
> conversion sequence (to which the `NoOp` belongs) [happens after/above 
> that](https://eel.is/c++draft/over.best.ics#over.ics.user).
Ah, thank you for confirming my intuition!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129752

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

1 2 >

1 - 100 of 199 matches

Mail list logo