[PATCH] D120876: [Driver] Split up huge arm-cortex-cpus.c test.

2022-03-03 Thread Dave Green via Phabricator via cfe-commits
dmgreen accepted this revision.
dmgreen added a comment.
This revision is now accepted and ready to land.

Same as D120875 , if anyone wants to split 
this up more naturally that would be good, but until then this LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120876

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


[PATCH] D120395: [X86] Prohibit arithmetic operations on type `__bfloat16`

2022-03-03 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei added a comment.

> if we define the `__bfloat16` type as the built-in `__bf16` type, then the 
> front end can apply whatever rules it has for that type, including adding 
> whatever ABI handling is needed for BF16 values.

I don't agree. Unlike `__fp16`, `__bf16` is simple an ARM specific type. I 
don't see why we need to define with it. 
https://clang.llvm.org/docs/LanguageExtensions.html#half-precision-floating-point
 . Besides, the doc says it "is only available when supported in hardware". 
Since we only have vector instructions, I don't think we support the scalar 
type here.

> If that ends up being the same as the rules for unsigned short, that's no 
> problem. The front end can implement it that way.

Doesn't it mean front end still generate `i16` in IR for it?

> The point is, by telling the front end that this is a BF16 value, we allow 
> the front end to control the semantics for it.

If you are saying we support a separate `__bfloat16` type in front end and 
generate `i16` in IR just for doing diagnose, I'm fine with it. The problem is 
we can't reuse the `__bf16` 's representation `BFloat16Ty`. Instead, we have to 
follow the way we are using for fp16, e.g., `HalfTy` for `__fp16` (no ABI) and 
`Float16Ty` for `_Float16` (has ABI). But it doesn't worth the effort to me.

> Also, you say we can't assume what registers will be used (in the eventual 
> ABI?) but we are assuming exactly that. If the ABI is ever defined 
> differently than what clang and gcc are currently doing, they will both be 
> wrong.

No new IR types, no ABI issues. The declaration of double underscore type are 
free without ABI. `__fp16` is a good example: https://godbolt.org/z/6qKqGc6Gj

> I don't see why we would treat BF16 values as unsigned short and i16 
> throughout the compiler just to make the backend implementation easier when 
> we already have types available for BF16.

So, it is not a problem of easy or difficult. It's a wrong direction we can't 
go with it (introducing new IR type without clear ABI declarations). We made 
such mistake with the `half` type. https://godbolt.org/z/K55s5zqPG We shouldn't 
make it again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120395

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


[PATCH] D120875: [Driver] Split up huge aarch64-cpus.c test.

2022-03-03 Thread Dave Green via Phabricator via cfe-commits
dmgreen accepted this revision.
dmgreen added a comment.
This revision is now accepted and ready to land.

OK Cool. Lets not punish Florian for improving things. If anyone want to go and 
split this file sensibly, that sounds good. In the meantime this LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120875

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


[PATCH] D120489: [analyzer] Done some changes to detect Uninitialized read by the char array manipulation functions

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

I would recommend committing changes in the morning, to give some time for the 
bots to chew through your commit. This way you could react to breakages and 
revert if needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120489

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


[PATCH] D120489: [analyzer] Done some changes to detect Uninitialized read by the char array manipulation functions

2022-03-03 Thread Douglas Yung via Phabricator via cfe-commits
dyung added a comment.

Thanks for fixing the problem with the original commit, but in the future, if 
you make follow-up commits, can you change the commit message to reflect what 
the commit contains? Having three different commits with the exact same commit 
message can be very confusing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120489

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


[PATCH] D120902: [clang-format] Fix assertion failure/crash with `AllowShortFunctionsOnASingleLine: Inline/InlineOnly`.

2022-03-03 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:313
   break;
+  if ((*J)->Level == TheLine->Level)
+return false;

owenpan wrote:
> To be safe?
Good idea. Will do.
Any idea how to provoke this (if at all possible)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120902

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


[PATCH] D120967: [NFC] Divide tests into smaller files

2022-03-03 Thread Brandon Wu via Phabricator via cfe-commits
4vtomat added a comment.

In D120967#3359188 , @kito-cheng 
wrote:

> LGTM, but do you mind give more comment on SUMMARY to describe what you did?

You are right lol. I've added some~


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120967

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


[PATCH] D120967: [NFC] Divide tests into smaller files

2022-03-03 Thread Kito Cheng via Phabricator via cfe-commits
kito-cheng accepted this revision.
kito-cheng added a comment.
This revision is now accepted and ready to land.

Do you mind give more comment on SUMMARY to describe what you did?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120967

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


[clang] 9c300c1 - [analyzer] Done some changes to detect Uninitialized read by the char array manipulation functions

2022-03-03 Thread via cfe-commits

Author: phyBrackets
Date: 2022-03-04T12:17:58+05:30
New Revision: 9c300c18a4eaf79eb7044744bbdb705764579220

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

LOG: [analyzer] Done some changes to detect Uninitialized read by the char 
array manipulation functions

Few weeks back I was experimenting with reading the uninitialized values from 
src , which is actually a bug but the CSA seems to give up at that point . I 
was curious about that and I pinged @steakhal on the discord and according to 
him this seems to be a genuine issue and needs to be fix. So I goes with fixing 
this bug and thanks to @steakhal who help me creating this patch. This feature 
seems to break some tests but this was the genuine problem and the broken tests 
also needs to fix in certain manner. I add a test but yeah we need more 
tests,I'll try to add more tests.Thanks

Reviewed By: steakhal, NoQ

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

Added: 


Modified: 
clang/docs/analyzer/checkers.rst

Removed: 




diff  --git a/clang/docs/analyzer/checkers.rst 
b/clang/docs/analyzer/checkers.rst
index bc42b80d166c7..a9ebe063c6c8b 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -2650,7 +2650,6 @@ Limitations:
   
- Due to limitations of the memory modeling in the analyzer, one can likely
  observe a lot of false-positive reports like this:
-
   .. code-block:: c
   
 void false_positive() {



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


[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

2022-03-03 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu marked an inline comment as done.
ChuanqiXu added a comment.

In D120874#3358870 , @tschuett wrote:

> Stupid question: this works with Windows as well? And the `BTW` sounds odd. 
> gcc also decided to use a dash as the separator.

It should work with Windows since Windows should allow '-' in filename. The 
test shows this, too.




Comment at: clang/lib/Lex/HeaderSearch.cpp:199
+  // The separator of C++20 modules partitions (':') is not good for file
+  // systems, here we choose '-' by default since it is not a valid
+  // character of C++ indentifiers. So we could avoid conflicts. BTW, '-' 
is

tschuett wrote:
> Or: here clang and gcc choose ...
Thanks. It is helpful for non-native speakers : )


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

https://reviews.llvm.org/D120874

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


[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

2022-03-03 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 412928.
ChuanqiXu added a comment.

Address comments.


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

https://reviews.llvm.org/D120874

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Modules/search-partitions.cpp


Index: clang/test/Modules/search-partitions.cpp
===
--- /dev/null
+++ clang/test/Modules/search-partitions.cpp
@@ -0,0 +1,44 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/partition1.cpp \
+// RUN:  -o %t/A-Part1.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/partition2.cpp \
+// RUN:  -o %t/A-Part2.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/partition3.cpp \
+// RUN:  -o %t/A-Part3.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/moduleA.cpp \
+// RUN:  -fprebuilt-module-path=%t
+
+// expected-no-diagnostics
+
+//--- partition1.cpp
+export module A:Part1;
+
+int part1();
+
+//--- partition2.cpp
+
+export module A:Part2;
+
+int part2();
+
+//--- partition3.cpp
+
+export module A:Part3;
+
+int part3();
+
+//--- moduleA.cpp
+
+export module A;
+
+import :Part1;
+export import :Part2;
+import :Part3;
+
+int foo();
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -194,10 +194,19 @@
   for (const std::string  : HSOpts->PrebuiltModulePaths) {
 SmallString<256> Result(Dir);
 llvm::sys::fs::make_absolute(Result);
-llvm::sys::path::append(Result, ModuleName + ".pcm");
+if (ModuleName.contains(':'))
+  // The separator of C++20 modules partitions (':') is not good for file
+  // systems, here clang and gcc choose '-' by default since it is not a
+  // valid character of C++ indentifiers. So we could avoid conflicts.
+  llvm::sys::path::append(Result, ModuleName.split(':').first + "-" +
+  ModuleName.split(':').second +
+  ".pcm");
+else
+  llvm::sys::path::append(Result, ModuleName + ".pcm");
 if (getFileMgr().getFile(Result.str()))
   return std::string(Result);
   }
+
   return {};
 }
 


Index: clang/test/Modules/search-partitions.cpp
===
--- /dev/null
+++ clang/test/Modules/search-partitions.cpp
@@ -0,0 +1,44 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/partition1.cpp \
+// RUN:  -o %t/A-Part1.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/partition2.cpp \
+// RUN:  -o %t/A-Part2.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/partition3.cpp \
+// RUN:  -o %t/A-Part3.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/moduleA.cpp \
+// RUN:  -fprebuilt-module-path=%t
+
+// expected-no-diagnostics
+
+//--- partition1.cpp
+export module A:Part1;
+
+int part1();
+
+//--- partition2.cpp
+
+export module A:Part2;
+
+int part2();
+
+//--- partition3.cpp
+
+export module A:Part3;
+
+int part3();
+
+//--- moduleA.cpp
+
+export module A;
+
+import :Part1;
+export import :Part2;
+import :Part3;
+
+int foo();
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -194,10 +194,19 @@
   for (const std::string  : HSOpts->PrebuiltModulePaths) {
 SmallString<256> Result(Dir);
 llvm::sys::fs::make_absolute(Result);
-llvm::sys::path::append(Result, ModuleName + ".pcm");
+if (ModuleName.contains(':'))
+  // The separator of C++20 modules partitions (':') is not good for file
+  // systems, here clang and gcc choose '-' by default since it is not a
+  // valid character of C++ indentifiers. So we could avoid conflicts.
+  llvm::sys::path::append(Result, ModuleName.split(':').first + "-" +
+  ModuleName.split(':').second +
+  ".pcm");
+else
+  llvm::sys::path::append(Result, ModuleName + ".pcm");
 if (getFileMgr().getFile(Result.str()))
   return std::string(Result);
   }
+
   return {};
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D120967: [NFC] Divide tests into smaller files

2022-03-03 Thread Brandon Wu via Phabricator via cfe-commits
4vtomat created this revision.
Herald added subscribers: luke957, frasercrmck, luismarques, apazos, 
sameer.abuasal, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, 
MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, niosHD, sabuasal, 
simoncook, johnrusso, rbar, asb.
Herald added a project: All.
4vtomat requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, MaskRay.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120967

Files:
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vloxseg.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vloxseg_mask.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vluxseg.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vluxseg_mask.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vloxseg.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vloxseg_mask.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vluxseg.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vluxseg_mask.c

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


[PATCH] D120272: [CUDA] Add driver support for compiling CUDA with the new driver

2022-03-03 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 412908.
jhuber6 added a comment.

Correctly handle offloading architecture options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120272

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/cuda-openmp-driver.cu

Index: clang/test/Driver/cuda-openmp-driver.cu
===
--- /dev/null
+++ clang/test/Driver/cuda-openmp-driver.cu
@@ -0,0 +1,16 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu -nocudalib -ccc-print-bindings -fgpu-rdc \
+// RUN:-foffload-new-driver --offload-arch=sm_35 --offload-arch=sm_70 %s 2>&1 \
+// RUN: | FileCheck -check-prefix CHECK %s
+
+// CHECK: "nvptx64-nvidia-cuda" - "clang", inputs: ["[[INPUT:.+]]"], output: "[[PTX_SM_35:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: ["[[PTX_SM_35]]"], output: "[[CUBIN_SM_35:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "NVPTX::Linker", inputs: ["[[CUBIN_SM_35]]", "[[PTX_SM_35]]"], output: "[[FATBIN_SM_35:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "clang", inputs: ["[[INPUT]]"], output: "[[PTX_SM_70:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: ["[[PTX_SM_70:.+]]"], output: "[[CUBIN_SM_70:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "NVPTX::Linker", inputs: ["[[CUBIN_SM_70]]", "[[PTX_SM_70:.+]]"], output: "[[FATBIN_SM_70:.+]]"
+// CHECK: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[INPUT]]", "[[FATBIN_SM_35]]", "[[FATBIN_SM_70]]"], output: "[[HOST_OBJ:.+]]"
+// CHECK: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: ["[[HOST_OBJ]]"], output: "a.out"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4391,6 +4391,7 @@
   // one input.
   bool IsCuda = JA.isOffloading(Action::OFK_Cuda);
   bool IsCudaDevice = JA.isDeviceOffloading(Action::OFK_Cuda);
+  bool IsCudaHost = JA.isHostOffloading(Action::OFK_Cuda);
   bool IsHIP = JA.isOffloading(Action::OFK_HIP);
   bool IsHIPDevice = JA.isDeviceOffloading(Action::OFK_HIP);
   bool IsOpenMPDevice = JA.isDeviceOffloading(Action::OFK_OpenMP);
@@ -4414,6 +4415,7 @@
 
   InputInfoList ModuleHeaderInputs;
   InputInfoList OpenMPHostInputs;
+  InputInfoList CudaHostInputs;
   const InputInfo *CudaDeviceInput = nullptr;
   const InputInfo *OpenMPDeviceInput = nullptr;
   for (const InputInfo  : Inputs) {
@@ -4428,6 +4430,8 @@
 << types::getTypeName(Expected);
   }
   ModuleHeaderInputs.push_back(I);
+} else if (IsCudaHost && Args.hasArg(options::OPT_fopenmp_new_driver)) {
+  CudaHostInputs.push_back(I);
 } else if ((IsCuda || IsHIP) && !CudaDeviceInput) {
   CudaDeviceInput = 
 } else if (IsOpenMPDevice && !OpenMPDeviceInput) {
@@ -6957,6 +6961,7 @@
 auto OpenMPTCs = C.getOffloadToolChains();
 for (auto TI = OpenMPTCs.first, TE = OpenMPTCs.second; TI != TE;
  ++TI, ++InputFile) {
+  assert(InputFile->isFilename() && "Offloading requires a filename");
   const ToolChain *TC = TI->second;
   const ArgList  = C.getArgsForToolChain(TC, "", Action::OFK_OpenMP);
   StringRef File =
@@ -6969,6 +6974,21 @@
   TC->getTripleString() + "." +
   TCArgs.getLastArgValue(options::OPT_march_EQ) + "." + InputName));
 }
+  } else if (IsCudaHost && !CudaHostInputs.empty()) {
+const ToolChain *TC = C.getSingleOffloadToolChain();
+for (const auto  : CudaHostInputs) {
+  assert(InputFile.isFilename() && "Offloading requires a filename");
+  StringRef File =
+  C.getArgs().MakeArgString(TC->getInputFilename(InputFile));
+  StringRef InputName = Clang::getBaseInputStem(Args, Inputs);
+  // The CUDA toolchain should have a bound arch appended to the filename.
+  StringRef Arch = File.split(".").first.rsplit('-').second;
+  CmdArgs.push_back(Args.MakeArgString(
+  "-fembed-offload-object=" + File + "," +
+  Action::GetOffloadKindName(Action::OFK_Cuda) + "." +
+  TC->getTripleString() + "." +
+  Arch + "." + InputName));
+}
   }
 
   if (Triple.isAMDGPU()) {
@@ -8217,17 +8237,21 @@
   const Driver  = getToolChain().getDriver();
   const llvm::Triple TheTriple = getToolChain().getTriple();
   auto OpenMPTCRange = C.getOffloadToolChains();
+  auto CudaTCRange = C.getOffloadToolChains();
   ArgStringList CmdArgs;
 
   // Pass the CUDA path to the linker wrapper tool.
-  for (auto  : llvm::make_range(OpenMPTCRange.first, OpenMPTCRange.second)) {
-const ToolChain *TC = I.second;
-if (TC->getTriple().isNVPTX()) {
-  CudaInstallationDetector 

[PATCH] D120949: [clang][AST matchers] adding attributedStmt AST matcher

2022-03-03 Thread Alister Johnson via Phabricator via cfe-commits
ajohnson-uoregon updated this revision to Diff 412901.
ajohnson-uoregon added a comment.

making clang-format happy


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120949

Files:
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp

Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -142,6 +142,7 @@
   REGISTER_MATCHER(atomicExpr);
   REGISTER_MATCHER(atomicType);
   REGISTER_MATCHER(attr);
+  REGISTER_MATCHER(attributedStmt);
   REGISTER_MATCHER(autoType);
   REGISTER_MATCHER(autoreleasePoolStmt)
   REGISTER_MATCHER(binaryConditionalOperator);
@@ -355,6 +356,7 @@
   REGISTER_MATCHER(hasSpecializedTemplate);
   REGISTER_MATCHER(hasStaticStorageDuration);
   REGISTER_MATCHER(hasStructuredBlock);
+  REGISTER_MATCHER(hasSubStmt);
   REGISTER_MATCHER(hasSyntacticForm);
   REGISTER_MATCHER(hasTargetDecl);
   REGISTER_MATCHER(hasTemplateArgument);
@@ -395,6 +397,7 @@
   REGISTER_MATCHER(isArrow);
   REGISTER_MATCHER(isAssignmentOperator);
   REGISTER_MATCHER(isAtPosition);
+  REGISTER_MATCHER(isAttr);
   REGISTER_MATCHER(isBaseInitializer);
   REGISTER_MATCHER(isBitField);
   REGISTER_MATCHER(isCatchAll);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -981,6 +981,8 @@
 predefinedExpr;
 const internal::VariadicDynCastAllOfMatcher
 designatedInitExpr;
+const internal::VariadicDynCastAllOfMatcher
+attributedStmt;
 const internal::VariadicOperatorMatcherFunc<
 2, std::numeric_limits::max()>
 eachOf = {internal::DynTypedMatcher::VO_EachOf};
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2699,6 +2699,62 @@
   return Node.size() == N;
 }
 
+/// Matches the attribute(s) attached to a Stmt
+///
+/// Example: Matches [[likely]] and [[unlikely]]
+/// \code
+///   constexpr double pow(double x, long long n) noexcept {
+/// if (n > 0) [[likely]]
+///  return x * pow(x, n - 1);
+/// else [[unlikely]]
+///  return 1;
+///   }
+/// \endcode
+extern const internal::VariadicDynCastAllOfMatcher
+attributedStmt;
+
+/// Matches the specified attribute.
+///
+/// Example:
+/// \code
+///   attributedStmt(isAttr(attr::Likely))
+/// \endcode
+/// would only match [[likely]] here:
+/// \code
+///   constexpr double pow(double x, long long n) noexcept {
+/// if (n > 0) [[likely]]
+///  return x * pow(x, n - 1);
+/// else [[unlikely]]
+///  return 1;
+///   }
+/// \endcode
+AST_MATCHER_P(AttributedStmt, isAttr, attr::Kind, AttrKind) {
+  for (const auto *Attr : Node.getAttrs()) {
+if (Attr->getKind() == AttrKind) {
+  return true;
+}
+  }
+  return false;
+}
+
+/// Matches the statement an attribute is attached to.
+///
+/// Example:
+/// \code
+///   attributedStmt(hasSubStmt(returnStmt()))
+/// \endcode
+/// would match return 1; here:
+/// \code
+///   else [[unlikely]]
+/// return 1;
+/// \endcode
+AST_MATCHER_P(AttributedStmt, hasSubStmt, internal::Matcher,
+  InnerMatcher) {
+  const Stmt *const Statement = Node.getSubStmt();
+  return (Statement != nullptr &&
+  InnerMatcher.matches(*Statement, Finder, Builder));
+}
+
 /// Matches \c QualTypes in the clang AST.
 extern const internal::VariadicAllOfMatcher qualType;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

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



Comment at: clang/lib/Lex/HeaderSearch.cpp:199
+  // The separator of C++20 modules partitions (':') is not good for file
+  // systems, here we choose '-' by default since it is not a valid
+  // character of C++ indentifiers. So we could avoid conflicts. BTW, '-' 
is

Or: here clang and gcc choose ...


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

https://reviews.llvm.org/D120874

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


[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

2022-03-03 Thread Thorsten via Phabricator via cfe-commits
tschuett added a comment.

Stupid question: this works with Windows as well? And the `BTW` sounds odd. gcc 
also decided to use a dash as the separator.


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

https://reviews.llvm.org/D120874

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


[PATCH] D118352: [clang][ABI] New c++20 modules mangling scheme

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

If I don't misread the codes, it looks like `mangleModuleInitializer` is not 
called.

---

Now we could add test for partitions.


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

https://reviews.llvm.org/D118352

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


[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

2022-03-03 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments.



Comment at: clang/test/Modules/search-partitions.cppm:20
+//--- partition1.cpp
+export module A : Part1;
+

iains wrote:
> ChuanqiXu wrote:
> > @MyDeveloperDay hi, I remember the support for partitions in clang-format 
> > is done in https://reviews.llvm.org/D114151. I think I did something wrong. 
> > I format my code with the following command in the root directory of llvm 
> > project:
> > ```
> > git diff -U0 --no-color --relative HEAD^ | 
> > clang/tools/clang-format/clang-format-diff.py -p1 -i
> > ```
> > 
> > Would it use an out-dated clang-format? If yes, how should I use the proper 
> > one. Or if is it possible that the `.clang-format` files is not updated 
> > correctly?
> FWIW, I have been using `git clang-format  ` with llvm-14 clang-format in 
> my PATH and it seems to work OK there - but not for llvm <= 13 (on macOS 
> 10.15).
> 
It works now. Thanks!


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

https://reviews.llvm.org/D120874

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


[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

2022-03-03 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 412899.
ChuanqiXu added a comment.

Format tests.


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

https://reviews.llvm.org/D120874

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Modules/search-partitions.cpp


Index: clang/test/Modules/search-partitions.cpp
===
--- /dev/null
+++ clang/test/Modules/search-partitions.cpp
@@ -0,0 +1,44 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/partition1.cpp \
+// RUN:  -o %t/A-Part1.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/partition2.cpp \
+// RUN:  -o %t/A-Part2.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/partition3.cpp \
+// RUN:  -o %t/A-Part3.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/moduleA.cpp \
+// RUN:  -fprebuilt-module-path=%t
+
+// expected-no-diagnostics
+
+//--- partition1.cpp
+export module A:Part1;
+
+int part1();
+
+//--- partition2.cpp
+
+export module A:Part2;
+
+int part2();
+
+//--- partition3.cpp
+
+export module A:Part3;
+
+int part3();
+
+//--- moduleA.cpp
+
+export module A;
+
+import :Part1;
+export import :Part2;
+import :Part3;
+
+int foo();
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -194,10 +194,20 @@
   for (const std::string  : HSOpts->PrebuiltModulePaths) {
 SmallString<256> Result(Dir);
 llvm::sys::fs::make_absolute(Result);
-llvm::sys::path::append(Result, ModuleName + ".pcm");
+if (ModuleName.contains(':'))
+  // The separator of C++20 modules partitions (':') is not good for file
+  // systems, here we choose '-' by default since it is not a valid
+  // character of C++ indentifiers. So we could avoid conflicts. BTW, '-' 
is
+  // the choice of GCC too.
+  llvm::sys::path::append(Result, ModuleName.split(':').first + "-" +
+  ModuleName.split(':').second +
+  ".pcm");
+else
+  llvm::sys::path::append(Result, ModuleName + ".pcm");
 if (getFileMgr().getFile(Result.str()))
   return std::string(Result);
   }
+
   return {};
 }
 


Index: clang/test/Modules/search-partitions.cpp
===
--- /dev/null
+++ clang/test/Modules/search-partitions.cpp
@@ -0,0 +1,44 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/partition1.cpp \
+// RUN:  -o %t/A-Part1.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/partition2.cpp \
+// RUN:  -o %t/A-Part2.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/partition3.cpp \
+// RUN:  -o %t/A-Part3.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/moduleA.cpp \
+// RUN:  -fprebuilt-module-path=%t
+
+// expected-no-diagnostics
+
+//--- partition1.cpp
+export module A:Part1;
+
+int part1();
+
+//--- partition2.cpp
+
+export module A:Part2;
+
+int part2();
+
+//--- partition3.cpp
+
+export module A:Part3;
+
+int part3();
+
+//--- moduleA.cpp
+
+export module A;
+
+import :Part1;
+export import :Part2;
+import :Part3;
+
+int foo();
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -194,10 +194,20 @@
   for (const std::string  : HSOpts->PrebuiltModulePaths) {
 SmallString<256> Result(Dir);
 llvm::sys::fs::make_absolute(Result);
-llvm::sys::path::append(Result, ModuleName + ".pcm");
+if (ModuleName.contains(':'))
+  // The separator of C++20 modules partitions (':') is not good for file
+  // systems, here we choose '-' by default since it is not a valid
+  // character of C++ indentifiers. So we could avoid conflicts. BTW, '-' is
+  // the choice of GCC too.
+  llvm::sys::path::append(Result, ModuleName.split(':').first + "-" +
+  ModuleName.split(':').second +
+  ".pcm");
+else
+  llvm::sys::path::append(Result, ModuleName + ".pcm");
 if (getFileMgr().getFile(Result.str()))
   return std::string(Result);
   }
+
   return {};
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D120954: adding a check that the number of arguments given in the matcher is the same as the number of arguments in the matched code

2022-03-03 Thread Alister Johnson via Phabricator via cfe-commits
ajohnson-uoregon abandoned this revision.
ajohnson-uoregon added a comment.

created by accident


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120954

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


[PATCH] D120959: [clang][AST matchers] new hasExpectedReturnType submatcher for ReturnStmts

2022-03-03 Thread Alister Johnson via Phabricator via cfe-commits
ajohnson-uoregon created this revision.
ajohnson-uoregon added reviewers: klimek, jdoerfert.
Herald added a project: All.
ajohnson-uoregon requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

new AST matcher that matches the expected return type of a ReturnStmt by 
looking at the enclosing function's stated return type; matches against the 
function's type


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120959

Files:
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp


Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -314,6 +314,7 @@
   REGISTER_MATCHER(hasEitherOperand);
   REGISTER_MATCHER(hasElementType);
   REGISTER_MATCHER(hasElse);
+  REGISTER_MATCHER(hasExpectedReturnType);
   REGISTER_MATCHER(hasExplicitSpecifier);
   REGISTER_MATCHER(hasExternalFormalLinkage);
   REGISTER_MATCHER(hasFalseExpression);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -3965,6 +3965,44 @@
   return false;
 }
 
+/// Matches the type of a return statement as declared by the enclosing
+/// function.
+///
+/// Example: returnStmt(hasExpectedReturnType(asString("int *"))) will match
+/// return 0; in
+/// \code
+///   int* foo() {
+/// return 0;
+///   }
+/// \endcode
+/// despite the return value being an IntegerLiteral.
+
+AST_MATCHER_P(ReturnStmt, hasExpectedReturnType, internal::Matcher,
+InnerMatcher) {
+  const auto  = Finder->getASTContext().getParents(Node);
+
+  llvm::SmallVector Stack(Parents.begin(), Parents.end());
+  const FunctionDecl* FuncDeclNode;
+  while (!Stack.empty()) {
+const auto  = Stack.back();
+Stack.pop_back();
+FuncDeclNode = CurNode.get();
+if (FuncDeclNode != nullptr) {
+  break;
+}
+else {
+  for (const auto  : 
Finder->getASTContext().getParents(CurNode))
+Stack.push_back(Parent);
+}
+  }
+  if (FuncDeclNode == nullptr) {
+return false;
+  }
+  else {
+return InnerMatcher.matches(FuncDeclNode->getReturnType(), Finder, 
Builder);
+  }
+}
+
 /// Matches if the type location of a node matches the inner matcher.
 ///
 /// Examples:


Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -314,6 +314,7 @@
   REGISTER_MATCHER(hasEitherOperand);
   REGISTER_MATCHER(hasElementType);
   REGISTER_MATCHER(hasElse);
+  REGISTER_MATCHER(hasExpectedReturnType);
   REGISTER_MATCHER(hasExplicitSpecifier);
   REGISTER_MATCHER(hasExternalFormalLinkage);
   REGISTER_MATCHER(hasFalseExpression);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -3965,6 +3965,44 @@
   return false;
 }
 
+/// Matches the type of a return statement as declared by the enclosing
+/// function.
+///
+/// Example: returnStmt(hasExpectedReturnType(asString("int *"))) will match
+/// return 0; in
+/// \code
+///   int* foo() {
+/// return 0;
+///   }
+/// \endcode
+/// despite the return value being an IntegerLiteral.
+
+AST_MATCHER_P(ReturnStmt, hasExpectedReturnType, internal::Matcher,
+InnerMatcher) {
+  const auto  = Finder->getASTContext().getParents(Node);
+
+  llvm::SmallVector Stack(Parents.begin(), Parents.end());
+  const FunctionDecl* FuncDeclNode;
+  while (!Stack.empty()) {
+const auto  = Stack.back();
+Stack.pop_back();
+FuncDeclNode = CurNode.get();
+if (FuncDeclNode != nullptr) {
+  break;
+}
+else {
+  for (const auto  : Finder->getASTContext().getParents(CurNode))
+Stack.push_back(Parent);
+}
+  }
+  if (FuncDeclNode == nullptr) {
+return false;
+  }
+  else {
+return InnerMatcher.matches(FuncDeclNode->getReturnType(), Finder, Builder);
+  }
+}
+
 /// Matches if the type location of a node matches the inner matcher.
 ///
 /// Examples:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D120956: [clang][AST matchers] new AST matcher argumentsGivenCountIs(n) that checks the actual number of arguments given in a function call

2022-03-03 Thread Alister Johnson via Phabricator via cfe-commits
ajohnson-uoregon created this revision.
ajohnson-uoregon added reviewers: klimek, jdoerfert.
Herald added a project: All.
ajohnson-uoregon requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

new AST matcher that checks the actual number of arguments given to a function 
call, *excluding* defaults not present; works like argumentCountIs(n) when 
isTraversalIgnoringImplicitNodes() is true


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120956

Files:
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp


Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -135,6 +135,7 @@
   REGISTER_MATCHER(anyOf);
   REGISTER_MATCHER(anything);
   REGISTER_MATCHER(argumentCountIs);
+  REGISTER_MATCHER(argumentsGivenCountIs);
   REGISTER_MATCHER(arraySubscriptExpr);
   REGISTER_MATCHER(arrayType);
   REGISTER_MATCHER(asString);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -4440,6 +4440,29 @@
   return NumArgs == N;
 }
 
+/// Checks that a call expression or a constructor call expression has
+/// a specific number of arguments (EXCLUDING absent default arguments).
+///
+/// Example matches f(0, 0) but not f(0) (matcher = 
callExpr(argumentsGivenCountIs(2)))
+/// \code
+///   void f(int x, int y = 0);
+///   f(0, 0);
+///   f(0);
+/// \endcode
+AST_POLYMORPHIC_MATCHER_P(argumentsGivenCountIs,
+  AST_POLYMORPHIC_SUPPORTED_TYPES(
+  CallExpr, CXXConstructExpr,
+  CXXUnresolvedConstructExpr, ObjCMessageExpr),
+  unsigned, N) {
+  unsigned NumArgs = Node.getNumArgs();
+  while (NumArgs) {
+if (!isa(Node.getArg(NumArgs - 1)))
+  break;
+--NumArgs;
+  }
+  return NumArgs == N;
+}
+
 /// Matches the n'th argument of a call expression or a constructor
 /// call expression.
 ///


Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -135,6 +135,7 @@
   REGISTER_MATCHER(anyOf);
   REGISTER_MATCHER(anything);
   REGISTER_MATCHER(argumentCountIs);
+  REGISTER_MATCHER(argumentsGivenCountIs);
   REGISTER_MATCHER(arraySubscriptExpr);
   REGISTER_MATCHER(arrayType);
   REGISTER_MATCHER(asString);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -4440,6 +4440,29 @@
   return NumArgs == N;
 }
 
+/// Checks that a call expression or a constructor call expression has
+/// a specific number of arguments (EXCLUDING absent default arguments).
+///
+/// Example matches f(0, 0) but not f(0) (matcher = callExpr(argumentsGivenCountIs(2)))
+/// \code
+///   void f(int x, int y = 0);
+///   f(0, 0);
+///   f(0);
+/// \endcode
+AST_POLYMORPHIC_MATCHER_P(argumentsGivenCountIs,
+  AST_POLYMORPHIC_SUPPORTED_TYPES(
+  CallExpr, CXXConstructExpr,
+  CXXUnresolvedConstructExpr, ObjCMessageExpr),
+  unsigned, N) {
+  unsigned NumArgs = Node.getNumArgs();
+  while (NumArgs) {
+if (!isa(Node.getArg(NumArgs - 1)))
+  break;
+--NumArgs;
+  }
+  return NumArgs == N;
+}
+
 /// Matches the n'th argument of a call expression or a constructor
 /// call expression.
 ///
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D120954: adding a check that the number of arguments given in the matcher is the same as the number of arguments in the matched code

2022-03-03 Thread Alister Johnson via Phabricator via cfe-commits
ajohnson-uoregon created this revision.
Herald added a project: All.
ajohnson-uoregon requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120954

Files:
  clang-tools-extra/clang-rewrite/ConstructMatchers.cpp


Index: clang-tools-extra/clang-rewrite/ConstructMatchers.cpp
===
--- clang-tools-extra/clang-rewrite/ConstructMatchers.cpp
+++ clang-tools-extra/clang-rewrite/ConstructMatchers.cpp
@@ -523,7 +523,7 @@
 }
   }
 }
-// child_matchers.push_back(constructMatcher("parameterCountIs", 
VariantValue(argnum), level+5));
+child_matchers.push_back(constructMatcher("argumentsGivenCountIs", 
VariantValue(argnum), level+5));
   }
   if (child_matchers.size() < 1) {
 // guarantee child_matchers.size() >= 1 (also required to not make an


Index: clang-tools-extra/clang-rewrite/ConstructMatchers.cpp
===
--- clang-tools-extra/clang-rewrite/ConstructMatchers.cpp
+++ clang-tools-extra/clang-rewrite/ConstructMatchers.cpp
@@ -523,7 +523,7 @@
 }
   }
 }
-// child_matchers.push_back(constructMatcher("parameterCountIs", VariantValue(argnum), level+5));
+child_matchers.push_back(constructMatcher("argumentsGivenCountIs", VariantValue(argnum), level+5));
   }
   if (child_matchers.size() < 1) {
 // guarantee child_matchers.size() >= 1 (also required to not make an
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D120397: [C++20] [Modules] Make the linkage consistent for template and its specialization

2022-03-03 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments.



Comment at: clang/test/Modules/inconsist-export-template.cpp:19-23
+// FIXME: We should reject following specialization,
+// since it tries to export a name which is already introduced.
+export template <>
+void f1() {
+

dblaikie wrote:
> urnathan wrote:
> > ChuanqiXu wrote:
> > > iains wrote:
> > > > iains wrote:
> > > > > ChuanqiXu wrote:
> > > > > > urnathan wrote:
> > > > > > > I don't think we should be testing for ill-formed code here.  We 
> > > > > > > want to verify that explicit instantiations, explicit 
> > > > > > > specializations and implicit instantiations all get the expected 
> > > > > > > linkage -- both external linkage on exported entities, module 
> > > > > > > linkage on non-exported module-purview entities.
> > > > > > I think it could add an `expected-error` once we complete the check 
> > > > > > in compiler so I added the FIXME here.
> > > > > would it be possible to find a suitable place in the source for the 
> > > > > FIXME?
> > > > > I would be concerned that it could get forgotten in the test-case.
> > > > or maybe just file a PR for accepts invalid code?
> > > I would like to send an issue after the patch to remind me not forgetting 
> > > it. The issue is needed after the patch since it would crash too. I 
> > > prefer to remain the FIXME here so that it would look like a pre-commit 
> > > test case, which should be good.
> > The way to not forget about this is to file a bug and assign it to 
> > yourself.  Not add a known-wrong testcase.
> FWIW, I do sometimes put in known-bad test cases to document existing issues 
> and also to make it easier to find existing test coverage/a good home for the 
> test case (often I/we end up adding a new test file, because it's not obvious 
> where related existing test coverage is).
> 
> If this is the right place for the future test case, and it documents a 
> limitation with the existing behavior, I wouldn't be totally against 
> including it here, with a FIXME and reference to a filed bug, ideally (though 
> I think I've done this without a bug filed too).
Agree with @dblaikie here, I filed an issue 
(https://github.com/llvm/llvm-project/issues/54189) assigned to myself and a 
reference it. @urnathan , a key reason why I don't move the known-wrong test 
case is that this case would crash now. And I think it is good to test it 
wouldn't crash. Crash shouldn't be good all the time.


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

https://reviews.llvm.org/D120397

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


[PATCH] D120397: [C++20] [Modules] Make the linkage consistent for template and its specialization

2022-03-03 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 412886.
ChuanqiXu added a comment.

Address comments:

- File an issue and add a reference to it.


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

https://reviews.llvm.org/D120397

Files:
  clang/lib/AST/Decl.cpp
  clang/test/CodeGenCXX/inconsistent-export-template.cpp
  clang/test/Modules/inconsist-export-template.cpp


Index: clang/test/Modules/inconsist-export-template.cpp
===
--- /dev/null
+++ clang/test/Modules/inconsist-export-template.cpp
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -std=c++20 %s -fsyntax-only -verify
+// expected-no-diagnostics
+export module m;
+export template 
+void f() {
+}
+
+template <>
+void f() {
+}
+
+template 
+void f1() {
+}
+
+// FIXME: We should reject following specialization,
+// since it tries to export a name which is already introduced.
+// See https://llvm.org/PR54189 for details.
+export template <>
+void f1() {
+}
+
+export template 
+class C {
+};
+
+template <>
+class C {
+public:
+  void M(){};
+};
+
+void Use(C ) { p.M(); }
Index: clang/test/CodeGenCXX/inconsistent-export-template.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/inconsistent-export-template.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -std=c++20 %s -S -emit-llvm -triple %itanium_abi_triple 
-disable-llvm-passes -o - | FileCheck %s
+
+export module m;
+export template 
+void f() {
+}
+
+// CHECK: void @_Z1fIiEvv
+template <>
+void f() {
+}
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -391,11 +391,18 @@
   bool considerVisibility =
 shouldConsiderTemplateVisibility(fn, specInfo);
 
-  // Merge information from the template parameters.
   FunctionTemplateDecl *temp = specInfo->getTemplate();
-  LinkageInfo tempLV =
-getLVForTemplateParameterList(temp->getTemplateParameters(), computation);
-  LV.mergeMaybeWithVisibility(tempLV, considerVisibility);
+
+  // Merge information from the template declaration.
+  LinkageInfo tempLV = getLVForDecl(temp, computation);
+  // The linkage of the specialization should be consistent with the
+  // template declaration.
+  LV.setLinkage(tempLV.getLinkage());
+
+  // Merge information from the template parameters.
+  LinkageInfo paramsLV =
+  getLVForTemplateParameterList(temp->getTemplateParameters(), 
computation);
+  LV.mergeMaybeWithVisibility(paramsLV, considerVisibility);
 
   // Merge information from the template arguments.
   const TemplateArgumentList  = *specInfo->TemplateArguments;


Index: clang/test/Modules/inconsist-export-template.cpp
===
--- /dev/null
+++ clang/test/Modules/inconsist-export-template.cpp
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -std=c++20 %s -fsyntax-only -verify
+// expected-no-diagnostics
+export module m;
+export template 
+void f() {
+}
+
+template <>
+void f() {
+}
+
+template 
+void f1() {
+}
+
+// FIXME: We should reject following specialization,
+// since it tries to export a name which is already introduced.
+// See https://llvm.org/PR54189 for details.
+export template <>
+void f1() {
+}
+
+export template 
+class C {
+};
+
+template <>
+class C {
+public:
+  void M(){};
+};
+
+void Use(C ) { p.M(); }
Index: clang/test/CodeGenCXX/inconsistent-export-template.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/inconsistent-export-template.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -std=c++20 %s -S -emit-llvm -triple %itanium_abi_triple -disable-llvm-passes -o - | FileCheck %s
+
+export module m;
+export template 
+void f() {
+}
+
+// CHECK: void @_Z1fIiEvv
+template <>
+void f() {
+}
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -391,11 +391,18 @@
   bool considerVisibility =
 shouldConsiderTemplateVisibility(fn, specInfo);
 
-  // Merge information from the template parameters.
   FunctionTemplateDecl *temp = specInfo->getTemplate();
-  LinkageInfo tempLV =
-getLVForTemplateParameterList(temp->getTemplateParameters(), computation);
-  LV.mergeMaybeWithVisibility(tempLV, considerVisibility);
+
+  // Merge information from the template declaration.
+  LinkageInfo tempLV = getLVForDecl(temp, computation);
+  // The linkage of the specialization should be consistent with the
+  // template declaration.
+  LV.setLinkage(tempLV.getLinkage());
+
+  // Merge information from the template parameters.
+  LinkageInfo paramsLV =
+  getLVForTemplateParameterList(temp->getTemplateParameters(), computation);
+  LV.mergeMaybeWithVisibility(paramsLV, considerVisibility);
 
   // Merge information from the template arguments.
   const TemplateArgumentList  = *specInfo->TemplateArguments;

[PATCH] D120952: [clang][AST matchers] adding submatchers under cudaKernelCallExpr to match kernel launch config

2022-03-03 Thread Alister Johnson via Phabricator via cfe-commits
ajohnson-uoregon created this revision.
ajohnson-uoregon added reviewers: klimek, jdoerfert.
Herald added a subscriber: yaxunl.
Herald added a project: All.
ajohnson-uoregon requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

adding more AST matchers for all possible launch params to a CUDA kernel, e.g. 
cudaGridDim and cudaSharedMemPerBlock


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120952

Files:
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp

Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -177,7 +177,11 @@
   REGISTER_MATCHER(continueStmt);
   REGISTER_MATCHER(coreturnStmt);
   REGISTER_MATCHER(coyieldExpr);
+  REGISTER_MATCHER(cudaBlockDim);
+  REGISTER_MATCHER(cudaGridDim);
   REGISTER_MATCHER(cudaKernelCallExpr);
+  REGISTER_MATCHER(cudaSharedMemPerBlock);
+  REGISTER_MATCHER(cudaStream);
   REGISTER_MATCHER(cxxBaseSpecifier);
   REGISTER_MATCHER(cxxBindTemporaryExpr);
   REGISTER_MATCHER(cxxBoolLiteral);
@@ -320,6 +324,7 @@
   REGISTER_MATCHER(hasInit);
   REGISTER_MATCHER(hasInitializer);
   REGISTER_MATCHER(hasInitStatement);
+  REGISTER_MATCHER(hasKernelConfig);
   REGISTER_MATCHER(hasKeywordSelector);
   REGISTER_MATCHER(hasLHS);
   REGISTER_MATCHER(hasLocalQualifiers);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -7848,6 +7848,80 @@
 extern const internal::VariadicDynCastAllOfMatcher
 cudaKernelCallExpr;
 
+/// Matches the config in <<<>>> on CUDA kernel calls.
+///
+/// Example: will match <<>> in
+/// \code
+///   kernel<<>>();
+/// \endcode
+AST_MATCHER_P(CUDAKernelCallExpr, hasKernelConfig, internal::Matcher,
+  InnerMatcher) {
+  if (const CallExpr *Config = Node.getConfig()) {
+return InnerMatcher.matches(*Config, Finder, Builder);
+  }
+  return false;
+}
+
+/// Matches the first argument (grid dim) in <<<>>> on CUDA kernel calls.
+///
+/// Example: will match i in
+/// \code
+///   kernel<<>>();
+/// \endcode
+AST_MATCHER_P(CUDAKernelCallExpr, cudaGridDim, internal::Matcher,
+  InnerMatcher) {
+  const CallExpr *Config = Node.getConfig();
+  if (Config && Config->getNumArgs() > 0) {
+return InnerMatcher.matches(*(Config->getArg(0)), Finder, Builder);
+  }
+  return false;
+}
+
+/// Matches the second argument (block dim) in <<<>>> on CUDA kernel calls.
+///
+/// Example: will match j in
+/// \code
+///   kernel<<>>();
+/// \endcode
+AST_MATCHER_P(CUDAKernelCallExpr, cudaBlockDim, internal::Matcher,
+  InnerMatcher) {
+  const CallExpr *Config = Node.getConfig();
+  if (Config && Config->getNumArgs() > 1) {
+return InnerMatcher.matches(*(Config->getArg(1)), Finder, Builder);
+  }
+  return false;
+}
+
+/// Matches the third argument (shared mem size) in <<<>>> on CUDA kernel calls.
+///
+/// Example: will match mem in
+/// \code
+///   kernel<<>>();
+/// \endcode
+AST_MATCHER_P(CUDAKernelCallExpr, cudaSharedMemPerBlock, internal::Matcher,
+  InnerMatcher) {
+  const CallExpr *Config = Node.getConfig();
+  if (Config && Config->getNumArgs() > 2) {
+return InnerMatcher.matches(*(Config->getArg(2)), Finder, Builder);
+  }
+  return false;
+}
+
+/// Matches the fourth argument (CUDA stream) in <<<>>> on CUDA kernel calls.
+///
+/// Example: will match 0 in
+/// \code
+///   kernel<<>>();
+/// \endcode
+AST_MATCHER_P(CUDAKernelCallExpr, cudaStream, internal::Matcher,
+  InnerMatcher) {
+  const CallExpr *Config = Node.getConfig();
+  if (Config && Config->getNumArgs() > 3) {
+return InnerMatcher.matches(*(Config->getArg(3)), Finder, Builder);
+  }
+  return false;
+}
+
 /// Matches expressions that resolve to a null pointer constant, such as
 /// GNU's __null, C++11's nullptr, or C's NULL macro.
 ///
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D120949: [clang][AST matchers] adding attributedStmt AST matcher

2022-03-03 Thread Alister Johnson via Phabricator via cfe-commits
ajohnson-uoregon added a comment.

Okay this is actually the right commits now, sorry about the spam, Aaron.  
Didn't realize I'd put unrelated things in the first commit.

The names for isAttr and hasSubStmt are up for debate, I just picked things 
that were close enough and weren't already in use.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120949

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


[PATCH] D120949: [clang][AST matchers] adding attributedStmt AST matcher

2022-03-03 Thread Alister Johnson via Phabricator via cfe-commits
ajohnson-uoregon updated this revision to Diff 412883.
ajohnson-uoregon added a comment.

getting the commit range right, finally


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120949

Files:
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp

Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -142,6 +142,7 @@
   REGISTER_MATCHER(atomicExpr);
   REGISTER_MATCHER(atomicType);
   REGISTER_MATCHER(attr);
+  REGISTER_MATCHER(attributedStmt);
   REGISTER_MATCHER(autoType);
   REGISTER_MATCHER(autoreleasePoolStmt)
   REGISTER_MATCHER(binaryConditionalOperator);
@@ -355,6 +356,7 @@
   REGISTER_MATCHER(hasSpecializedTemplate);
   REGISTER_MATCHER(hasStaticStorageDuration);
   REGISTER_MATCHER(hasStructuredBlock);
+  REGISTER_MATCHER(hasSubStmt);
   REGISTER_MATCHER(hasSyntacticForm);
   REGISTER_MATCHER(hasTargetDecl);
   REGISTER_MATCHER(hasTemplateArgument);
@@ -395,6 +397,7 @@
   REGISTER_MATCHER(isArrow);
   REGISTER_MATCHER(isAssignmentOperator);
   REGISTER_MATCHER(isAtPosition);
+  REGISTER_MATCHER(isAttr);
   REGISTER_MATCHER(isBaseInitializer);
   REGISTER_MATCHER(isBitField);
   REGISTER_MATCHER(isCatchAll);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -981,6 +981,8 @@
 predefinedExpr;
 const internal::VariadicDynCastAllOfMatcher
 designatedInitExpr;
+const internal::VariadicDynCastAllOfMatcher
+attributedStmt;
 const internal::VariadicOperatorMatcherFunc<
 2, std::numeric_limits::max()>
 eachOf = {internal::DynTypedMatcher::VO_EachOf};
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2699,6 +2699,60 @@
   return Node.size() == N;
 }
 
+/// Matches the attribute(s) attached to a Stmt
+///
+/// Example: Matches [[likely]] and [[unlikely]]
+/// \code
+///   constexpr double pow(double x, long long n) noexcept {
+/// if (n > 0) [[likely]]
+///  return x * pow(x, n - 1);
+/// else [[unlikely]]
+///  return 1;
+///   }
+/// \endcode
+extern const internal::VariadicDynCastAllOfMatcher
+attributedStmt;
+
+/// Matches the specified attribute.
+///
+/// Example:
+/// \code
+///   attributedStmt(isAttr(attr::Likely))
+/// \endcode
+/// would only match [[likely]] here:
+/// \code
+///   constexpr double pow(double x, long long n) noexcept {
+/// if (n > 0) [[likely]]
+///  return x * pow(x, n - 1);
+/// else [[unlikely]]
+///  return 1;
+///   }
+/// \endcode
+AST_MATCHER_P(AttributedStmt, isAttr, attr::Kind, AttrKind) {
+  for (const auto *Attr : Node.getAttrs()) {
+if (Attr->getKind() == AttrKind) {
+  return true;
+}
+  }
+  return false;
+}
+
+/// Matches the statement an attribute is attached to.
+///
+/// Example:
+/// \code
+///   attributedStmt(hasSubStmt(returnStmt()))
+/// \endcode
+/// would match return 1; here:
+/// \code
+///   else [[unlikely]]
+/// return 1;
+/// \endcode
+AST_MATCHER_P(AttributedStmt, hasSubStmt, internal::Matcher, InnerMatcher) {
+  const Stmt *const Statement = Node.getSubStmt();
+  return (Statement != nullptr && InnerMatcher.matches(*Statement, Finder, Builder));
+}
+
 /// Matches \c QualTypes in the clang AST.
 extern const internal::VariadicAllOfMatcher qualType;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D120949: [clang][AST matchers] adding attributedStmt AST matcher

2022-03-03 Thread Alister Johnson via Phabricator via cfe-commits
ajohnson-uoregon updated this revision to Diff 412882.
ajohnson-uoregon added a comment.

had to split a commit because past me messed up


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120949

Files:
  clang-tools-extra/clang-rewrite/ClangRewrite.cpp
  clang-tools-extra/clang-rewrite/MatcherGenCallback.h
  clang-tools-extra/clang-rewrite/NewCodeCallback.h
  clang-tools-extra/clang-rewrite/RewriteCallback.h
  clang-tools-extra/clang-rewrite/tests/new.cpp
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp

Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -142,6 +142,7 @@
   REGISTER_MATCHER(atomicExpr);
   REGISTER_MATCHER(atomicType);
   REGISTER_MATCHER(attr);
+  REGISTER_MATCHER(attributedStmt);
   REGISTER_MATCHER(autoType);
   REGISTER_MATCHER(autoreleasePoolStmt)
   REGISTER_MATCHER(binaryConditionalOperator);
@@ -355,6 +356,7 @@
   REGISTER_MATCHER(hasSpecializedTemplate);
   REGISTER_MATCHER(hasStaticStorageDuration);
   REGISTER_MATCHER(hasStructuredBlock);
+  REGISTER_MATCHER(hasSubStmt);
   REGISTER_MATCHER(hasSyntacticForm);
   REGISTER_MATCHER(hasTargetDecl);
   REGISTER_MATCHER(hasTemplateArgument);
@@ -395,6 +397,7 @@
   REGISTER_MATCHER(isArrow);
   REGISTER_MATCHER(isAssignmentOperator);
   REGISTER_MATCHER(isAtPosition);
+  REGISTER_MATCHER(isAttr);
   REGISTER_MATCHER(isBaseInitializer);
   REGISTER_MATCHER(isBitField);
   REGISTER_MATCHER(isCatchAll);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -981,6 +981,8 @@
 predefinedExpr;
 const internal::VariadicDynCastAllOfMatcher
 designatedInitExpr;
+const internal::VariadicDynCastAllOfMatcher
+attributedStmt;
 const internal::VariadicOperatorMatcherFunc<
 2, std::numeric_limits::max()>
 eachOf = {internal::DynTypedMatcher::VO_EachOf};
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2699,6 +2699,60 @@
   return Node.size() == N;
 }
 
+/// Matches the attribute(s) attached to a Stmt
+///
+/// Example: Matches [[likely]] and [[unlikely]]
+/// \code
+///   constexpr double pow(double x, long long n) noexcept {
+/// if (n > 0) [[likely]]
+///  return x * pow(x, n - 1);
+/// else [[unlikely]]
+///  return 1;
+///   }
+/// \endcode
+extern const internal::VariadicDynCastAllOfMatcher
+attributedStmt;
+
+/// Matches the specified attribute.
+///
+/// Example:
+/// \code
+///   attributedStmt(isAttr(attr::Likely))
+/// \endcode
+/// would only match [[likely]] here:
+/// \code
+///   constexpr double pow(double x, long long n) noexcept {
+/// if (n > 0) [[likely]]
+///  return x * pow(x, n - 1);
+/// else [[unlikely]]
+///  return 1;
+///   }
+/// \endcode
+AST_MATCHER_P(AttributedStmt, isAttr, attr::Kind, AttrKind) {
+  for (const auto *Attr : Node.getAttrs()) {
+if (Attr->getKind() == AttrKind) {
+  return true;
+}
+  }
+  return false;
+}
+
+/// Matches the statement an attribute is attached to.
+///
+/// Example:
+/// \code
+///   attributedStmt(hasSubStmt(returnStmt()))
+/// \endcode
+/// would match return 1; here:
+/// \code
+///   else [[unlikely]]
+/// return 1;
+/// \endcode
+AST_MATCHER_P(AttributedStmt, hasSubStmt, internal::Matcher, InnerMatcher) {
+  const Stmt *const Statement = Node.getSubStmt();
+  return (Statement != nullptr && InnerMatcher.matches(*Statement, Finder, Builder));
+}
+
 /// Matches \c QualTypes in the clang AST.
 extern const internal::VariadicAllOfMatcher qualType;
 
Index: clang-tools-extra/clang-rewrite/tests/new.cpp
===
--- clang-tools-extra/clang-rewrite/tests/new.cpp
+++ clang-tools-extra/clang-rewrite/tests/new.cpp
@@ -6,14 +6,26 @@
   for (int i = 0; i < 3; i++) {
 x = i;
   }
+  [[likely]]
+  {
+printf("not a matcher\n");
+  }
+  [[clang::matcher_block]]
   {
 return x;
   }
+}
 
+constexpr double pow(double x, long long n) noexcept {
+if (n > 0) [[likely]]
+return x * pow(x, n - 1);
+else [[unlikely]]
+return 1;
 }
 
 // [[clang::matcher("cuda_kernel")]]
 // auto kern() {
+//   [[clang::matcher_block]]
 //   {
 // kernel<<>>(arg1, arg2, ...);
 //   }
@@ -21,25 +33,37 @@
 //
 // [[clang::replace("cuda_kernel")]]
 // auto hip() {
-//   {
-// hip_launch(kernkel, numblocks, numthreads, 0, 0, arg1, arg2, ...);
+//   if (kernel == 

[PATCH] D120949: [clang][AST matchers] adding attributedStmt AST matcher

2022-03-03 Thread Alister Johnson via Phabricator via cfe-commits
ajohnson-uoregon updated this revision to Diff 412879.
ajohnson-uoregon added a comment.
Herald added a reviewer: aaron.ballman.

maybe this will work


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120949

Files:
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/Basic/Attr.td
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/lib/Sema/SemaStmtAttr.cpp

Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -233,6 +233,11 @@
   return ::new (S.Context) UnlikelyAttr(S.Context, A);
 }
 
+static Attr *handleMatcherBlock(Sema , Stmt *St, const ParsedAttr ,
+SourceRange Range) {
+  return ::new (S.Context) MatcherBlockAttr(S.Context, A);
+}
+
 #define WANT_STMT_MERGE_LOGIC
 #include "clang/Sema/AttrParsedAttrImpl.inc"
 #undef WANT_STMT_MERGE_LOGIC
@@ -424,6 +429,8 @@
 return handleLikely(S, St, A, Range);
   case ParsedAttr::AT_Unlikely:
 return handleUnlikely(S, St, A, Range);
+  case ParsedAttr::AT_MatcherBlock:
+return handleMatcherBlock(S, St, A, Range);
   default:
 // N.B., ClangAttrEmitter.cpp emits a diagnostic helper that ensures a
 // declaration attribute is not written on a statement, but this code is
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -142,6 +142,7 @@
   REGISTER_MATCHER(atomicExpr);
   REGISTER_MATCHER(atomicType);
   REGISTER_MATCHER(attr);
+  REGISTER_MATCHER(attributedStmt);
   REGISTER_MATCHER(autoType);
   REGISTER_MATCHER(autoreleasePoolStmt)
   REGISTER_MATCHER(binaryConditionalOperator);
@@ -355,6 +356,7 @@
   REGISTER_MATCHER(hasSpecializedTemplate);
   REGISTER_MATCHER(hasStaticStorageDuration);
   REGISTER_MATCHER(hasStructuredBlock);
+  REGISTER_MATCHER(hasSubStmt);
   REGISTER_MATCHER(hasSyntacticForm);
   REGISTER_MATCHER(hasTargetDecl);
   REGISTER_MATCHER(hasTemplateArgument);
@@ -395,6 +397,7 @@
   REGISTER_MATCHER(isArrow);
   REGISTER_MATCHER(isAssignmentOperator);
   REGISTER_MATCHER(isAtPosition);
+  REGISTER_MATCHER(isAttr);
   REGISTER_MATCHER(isBaseInitializer);
   REGISTER_MATCHER(isBitField);
   REGISTER_MATCHER(isCatchAll);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -981,6 +981,8 @@
 predefinedExpr;
 const internal::VariadicDynCastAllOfMatcher
 designatedInitExpr;
+const internal::VariadicDynCastAllOfMatcher
+attributedStmt;
 const internal::VariadicOperatorMatcherFunc<
 2, std::numeric_limits::max()>
 eachOf = {internal::DynTypedMatcher::VO_EachOf};
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -3959,3 +3959,9 @@
   let Args = [StringArgument<"MatcherName">];
   let Documentation = [Undocumented];
 }
+
+def MatcherBlock : StmtAttr {
+  let Spellings = [Clang<"matcher_block">];
+  let Subjects = SubjectList<[CompoundStmt], ErrorDiag, "compound stmts">;
+  let Documentation = [Undocumented];
+}
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2699,6 +2699,60 @@
   return Node.size() == N;
 }
 
+/// Matches the attribute(s) attached to a Stmt
+///
+/// Example: Matches [[likely]] and [[unlikely]]
+/// \code
+///   constexpr double pow(double x, long long n) noexcept {
+/// if (n > 0) [[likely]]
+///  return x * pow(x, n - 1);
+/// else [[unlikely]]
+///  return 1;
+///   }
+/// \endcode
+extern const internal::VariadicDynCastAllOfMatcher
+attributedStmt;
+
+/// Matches the specified attribute.
+///
+/// Example:
+/// \code
+///   attributedStmt(isAttr(attr::Likely))
+/// \endcode
+/// would only match [[likely]] here:
+/// \code
+///   constexpr double pow(double x, long long n) noexcept {
+/// if (n > 0) [[likely]]
+///  return x * pow(x, n - 1);
+/// else [[unlikely]]
+///  return 1;
+///   }
+/// \endcode
+AST_MATCHER_P(AttributedStmt, isAttr, attr::Kind, AttrKind) {
+  for (const auto *Attr : Node.getAttrs()) {
+if (Attr->getKind() == AttrKind) {
+  return true;
+}
+  }
+  return false;
+}
+
+/// Matches the statement an attribute is attached to.
+///
+/// Example:
+/// \code
+///   attributedStmt(hasSubStmt(returnStmt()))
+/// \endcode
+/// would match return 1; here:
+/// \code
+///   else 

[PATCH] D120949: [clang][AST matchers] adding attributedStmt AST matcher

2022-03-03 Thread Alister Johnson via Phabricator via cfe-commits
ajohnson-uoregon updated this revision to Diff 412878.
ajohnson-uoregon added a comment.

arcanist why


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120949

Files:
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp


Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -356,6 +356,7 @@
   REGISTER_MATCHER(hasSpecializedTemplate);
   REGISTER_MATCHER(hasStaticStorageDuration);
   REGISTER_MATCHER(hasStructuredBlock);
+  REGISTER_MATCHER(hasSubStmt);
   REGISTER_MATCHER(hasSyntacticForm);
   REGISTER_MATCHER(hasTargetDecl);
   REGISTER_MATCHER(hasTemplateArgument);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2737,6 +2737,22 @@
   return false;
 }
 
+/// Matches the statement an attribute is attached to.
+///
+/// Example:
+/// \code
+///   attributedStmt(hasSubStmt(returnStmt()))
+/// \endcode
+/// would match return 1; here:
+/// \code
+///   else [[unlikely]]
+/// return 1;
+/// \endcode
+AST_MATCHER_P(AttributedStmt, hasSubStmt, internal::Matcher, 
InnerMatcher) {
+  const Stmt *const Statement = Node.getSubStmt();
+  return (Statement != nullptr && InnerMatcher.matches(*Statement, Finder, 
Builder));
+}
+
 /// Matches \c QualTypes in the clang AST.
 extern const internal::VariadicAllOfMatcher qualType;
 


Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -356,6 +356,7 @@
   REGISTER_MATCHER(hasSpecializedTemplate);
   REGISTER_MATCHER(hasStaticStorageDuration);
   REGISTER_MATCHER(hasStructuredBlock);
+  REGISTER_MATCHER(hasSubStmt);
   REGISTER_MATCHER(hasSyntacticForm);
   REGISTER_MATCHER(hasTargetDecl);
   REGISTER_MATCHER(hasTemplateArgument);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2737,6 +2737,22 @@
   return false;
 }
 
+/// Matches the statement an attribute is attached to.
+///
+/// Example:
+/// \code
+///   attributedStmt(hasSubStmt(returnStmt()))
+/// \endcode
+/// would match return 1; here:
+/// \code
+///   else [[unlikely]]
+/// return 1;
+/// \endcode
+AST_MATCHER_P(AttributedStmt, hasSubStmt, internal::Matcher, InnerMatcher) {
+  const Stmt *const Statement = Node.getSubStmt();
+  return (Statement != nullptr && InnerMatcher.matches(*Statement, Finder, Builder));
+}
+
 /// Matches \c QualTypes in the clang AST.
 extern const internal::VariadicAllOfMatcher qualType;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D120949: [clang][AST matchers] adding attributedStmt AST matcher

2022-03-03 Thread Alister Johnson via Phabricator via cfe-commits
ajohnson-uoregon updated this revision to Diff 412877.
ajohnson-uoregon added a comment.

updated resolution rules to try to get right commits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120949

Files:
  clang-tools-extra/clang-rewrite/ClangRewrite.cpp
  clang-tools-extra/clang-rewrite/MatcherGenCallback.h
  clang-tools-extra/clang-rewrite/NewCodeCallback.h
  clang-tools-extra/clang-rewrite/RewriteCallback.h
  clang-tools-extra/clang-rewrite/tests/new.cpp
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp

Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -356,6 +356,7 @@
   REGISTER_MATCHER(hasSpecializedTemplate);
   REGISTER_MATCHER(hasStaticStorageDuration);
   REGISTER_MATCHER(hasStructuredBlock);
+  REGISTER_MATCHER(hasSubStmt);
   REGISTER_MATCHER(hasSyntacticForm);
   REGISTER_MATCHER(hasTargetDecl);
   REGISTER_MATCHER(hasTemplateArgument);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2737,6 +2737,22 @@
   return false;
 }
 
+/// Matches the statement an attribute is attached to.
+///
+/// Example:
+/// \code
+///   attributedStmt(hasSubStmt(returnStmt()))
+/// \endcode
+/// would match return 1; here:
+/// \code
+///   else [[unlikely]]
+/// return 1;
+/// \endcode
+AST_MATCHER_P(AttributedStmt, hasSubStmt, internal::Matcher, InnerMatcher) {
+  const Stmt *const Statement = Node.getSubStmt();
+  return (Statement != nullptr && InnerMatcher.matches(*Statement, Finder, Builder));
+}
+
 /// Matches \c QualTypes in the clang AST.
 extern const internal::VariadicAllOfMatcher qualType;
 
Index: clang-tools-extra/clang-rewrite/tests/new.cpp
===
--- clang-tools-extra/clang-rewrite/tests/new.cpp
+++ clang-tools-extra/clang-rewrite/tests/new.cpp
@@ -6,14 +6,26 @@
   for (int i = 0; i < 3; i++) {
 x = i;
   }
+  [[likely]]
+  {
+printf("not a matcher\n");
+  }
+  [[clang::matcher_block]]
   {
 return x;
   }
+}
 
+constexpr double pow(double x, long long n) noexcept {
+if (n > 0) [[likely]]
+return x * pow(x, n - 1);
+else [[unlikely]]
+return 1;
 }
 
 // [[clang::matcher("cuda_kernel")]]
 // auto kern() {
+//   [[clang::matcher_block]]
 //   {
 // kernel<<>>(arg1, arg2, ...);
 //   }
@@ -21,25 +33,37 @@
 //
 // [[clang::replace("cuda_kernel")]]
 // auto hip() {
-//   {
-// hip_launch(kernkel, numblocks, numthreads, 0, 0, arg1, arg2, ...);
+//   if (kernel == "gaussian") {
+// [[clang::matcher_block]]
+// {
+//   hip_launch(kernel, numblocks, numthreads, 0, 0, arg1, arg2, ...);
+// }
 //   }
 // }
 
 
 [[clang::replace("returns")]]
 auto return42() {
-  return 42;
+  [[clang::matcher_block]]
+  {
+return 42;
+  }
 }
 
 [[clang::insert_before("returns", "thencode")]]
 auto foobar() {
-  printf("returning\n");;
+  [[clang::matcher_block]]
+  {
+printf("returning\n");
+  }
 }
 
 [[clang::insert_after("thencode")]]
 auto helloworld() {
-  printf("hello world\n");
+  [[clang::matcher_block]]
+  {
+printf("hello world\n");
+  }
 }
 
 int main() {
Index: clang-tools-extra/clang-rewrite/RewriteCallback.h
===
--- clang-tools-extra/clang-rewrite/RewriteCallback.h
+++ clang-tools-extra/clang-rewrite/RewriteCallback.h
@@ -137,11 +137,11 @@
 unsigned int end_line = end.getSpellingLineNumber();
 unsigned int end_col = end.getSpellingColumnNumber();
 
-if (verbose) {
+// if (verbose) {
   printf("FOUND match for %s at %d:%d - %d:%d\n",
  matcher->getName().c_str(), begin_line, begin_col, end_line,
  end_col);
-}
+// }
 if (rw.isRewritable(match->getBeginLoc()) &&
 rw.isRewritable(match->getEndLoc())) {
 
Index: clang-tools-extra/clang-rewrite/NewCodeCallback.h
===
--- clang-tools-extra/clang-rewrite/NewCodeCallback.h
+++ clang-tools-extra/clang-rewrite/NewCodeCallback.h
@@ -57,13 +57,39 @@
 // .bind("replace");
 
 DeclarationMatcher insert_before_match =
-  functionDecl(hasAttr(attr::InsertCodeBefore)).bind("insert_before_match");
+  functionDecl(allOf(
+hasAttr(attr::InsertCodeBefore),
+hasBody(compoundStmt(
+  hasAnySubstatement(attributedStmt(allOf(
+isAttr(attr::MatcherBlock),
+hasSubStmt(compoundStmt(anything()).bind("body"))
+  )))
+))
+  )).bind("insert_before_match");
 
 DeclarationMatcher insert_after_match =
-  

[PATCH] D120949: [clang][AST matchers] adding attributedStmt AST matcher

2022-03-03 Thread Alister Johnson via Phabricator via cfe-commits
ajohnson-uoregon updated this revision to Diff 412875.
ajohnson-uoregon added a comment.

Still trying to get right commits, apologies XD


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120949

Files:
  clang-tools-extra/clang-rewrite/ClangRewrite.cpp
  clang-tools-extra/clang-rewrite/MatcherGenCallback.h
  clang-tools-extra/clang-rewrite/NewCodeCallback.h
  clang-tools-extra/clang-rewrite/RewriteCallback.h
  clang-tools-extra/clang-rewrite/tests/new.cpp
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp

Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -356,6 +356,7 @@
   REGISTER_MATCHER(hasSpecializedTemplate);
   REGISTER_MATCHER(hasStaticStorageDuration);
   REGISTER_MATCHER(hasStructuredBlock);
+  REGISTER_MATCHER(hasSubStmt);
   REGISTER_MATCHER(hasSyntacticForm);
   REGISTER_MATCHER(hasTargetDecl);
   REGISTER_MATCHER(hasTemplateArgument);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2737,6 +2737,22 @@
   return false;
 }
 
+/// Matches the statement an attribute is attached to.
+///
+/// Example:
+/// \code
+///   attributedStmt(hasSubStmt(returnStmt()))
+/// \endcode
+/// would match return 1; here:
+/// \code
+///   else [[unlikely]]
+/// return 1;
+/// \endcode
+AST_MATCHER_P(AttributedStmt, hasSubStmt, internal::Matcher, InnerMatcher) {
+  const Stmt *const Statement = Node.getSubStmt();
+  return (Statement != nullptr && InnerMatcher.matches(*Statement, Finder, Builder));
+}
+
 /// Matches \c QualTypes in the clang AST.
 extern const internal::VariadicAllOfMatcher qualType;
 
Index: clang-tools-extra/clang-rewrite/tests/new.cpp
===
--- clang-tools-extra/clang-rewrite/tests/new.cpp
+++ clang-tools-extra/clang-rewrite/tests/new.cpp
@@ -6,14 +6,26 @@
   for (int i = 0; i < 3; i++) {
 x = i;
   }
+  [[likely]]
+  {
+printf("not a matcher\n");
+  }
+  [[clang::matcher_block]]
   {
 return x;
   }
+}
 
+constexpr double pow(double x, long long n) noexcept {
+if (n > 0) [[likely]]
+return x * pow(x, n - 1);
+else [[unlikely]]
+return 1;
 }
 
 // [[clang::matcher("cuda_kernel")]]
 // auto kern() {
+//   [[clang::matcher_block]]
 //   {
 // kernel<<>>(arg1, arg2, ...);
 //   }
@@ -21,25 +33,37 @@
 //
 // [[clang::replace("cuda_kernel")]]
 // auto hip() {
-//   {
-// hip_launch(kernkel, numblocks, numthreads, 0, 0, arg1, arg2, ...);
+//   if (kernel == "gaussian") {
+// [[clang::matcher_block]]
+// {
+//   hip_launch(kernel, numblocks, numthreads, 0, 0, arg1, arg2, ...);
+// }
 //   }
 // }
 
 
 [[clang::replace("returns")]]
 auto return42() {
-  return 42;
+  [[clang::matcher_block]]
+  {
+return 42;
+  }
 }
 
 [[clang::insert_before("returns", "thencode")]]
 auto foobar() {
-  printf("returning\n");;
+  [[clang::matcher_block]]
+  {
+printf("returning\n");
+  }
 }
 
 [[clang::insert_after("thencode")]]
 auto helloworld() {
-  printf("hello world\n");
+  [[clang::matcher_block]]
+  {
+printf("hello world\n");
+  }
 }
 
 int main() {
Index: clang-tools-extra/clang-rewrite/RewriteCallback.h
===
--- clang-tools-extra/clang-rewrite/RewriteCallback.h
+++ clang-tools-extra/clang-rewrite/RewriteCallback.h
@@ -137,11 +137,11 @@
 unsigned int end_line = end.getSpellingLineNumber();
 unsigned int end_col = end.getSpellingColumnNumber();
 
-if (verbose) {
+// if (verbose) {
   printf("FOUND match for %s at %d:%d - %d:%d\n",
  matcher->getName().c_str(), begin_line, begin_col, end_line,
  end_col);
-}
+// }
 if (rw.isRewritable(match->getBeginLoc()) &&
 rw.isRewritable(match->getEndLoc())) {
 
Index: clang-tools-extra/clang-rewrite/NewCodeCallback.h
===
--- clang-tools-extra/clang-rewrite/NewCodeCallback.h
+++ clang-tools-extra/clang-rewrite/NewCodeCallback.h
@@ -57,13 +57,39 @@
 // .bind("replace");
 
 DeclarationMatcher insert_before_match =
-  functionDecl(hasAttr(attr::InsertCodeBefore)).bind("insert_before_match");
+  functionDecl(allOf(
+hasAttr(attr::InsertCodeBefore),
+hasBody(compoundStmt(
+  hasAnySubstatement(attributedStmt(allOf(
+isAttr(attr::MatcherBlock),
+hasSubStmt(compoundStmt(anything()).bind("body"))
+  )))
+))
+  )).bind("insert_before_match");
 
 DeclarationMatcher insert_after_match =
-  

[PATCH] D120949: [clang][AST matchers] adding attributedStmt AST matcher

2022-03-03 Thread Alister Johnson via Phabricator via cfe-commits
ajohnson-uoregon updated this revision to Diff 412873.
ajohnson-uoregon added a comment.

Still doesn't have right files/commits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120949

Files:
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp


Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -356,6 +356,7 @@
   REGISTER_MATCHER(hasSpecializedTemplate);
   REGISTER_MATCHER(hasStaticStorageDuration);
   REGISTER_MATCHER(hasStructuredBlock);
+  REGISTER_MATCHER(hasSubStmt);
   REGISTER_MATCHER(hasSyntacticForm);
   REGISTER_MATCHER(hasTargetDecl);
   REGISTER_MATCHER(hasTemplateArgument);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2737,6 +2737,22 @@
   return false;
 }
 
+/// Matches the statement an attribute is attached to.
+///
+/// Example:
+/// \code
+///   attributedStmt(hasSubStmt(returnStmt()))
+/// \endcode
+/// would match return 1; here:
+/// \code
+///   else [[unlikely]]
+/// return 1;
+/// \endcode
+AST_MATCHER_P(AttributedStmt, hasSubStmt, internal::Matcher, 
InnerMatcher) {
+  const Stmt *const Statement = Node.getSubStmt();
+  return (Statement != nullptr && InnerMatcher.matches(*Statement, Finder, 
Builder));
+}
+
 /// Matches \c QualTypes in the clang AST.
 extern const internal::VariadicAllOfMatcher qualType;
 


Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -356,6 +356,7 @@
   REGISTER_MATCHER(hasSpecializedTemplate);
   REGISTER_MATCHER(hasStaticStorageDuration);
   REGISTER_MATCHER(hasStructuredBlock);
+  REGISTER_MATCHER(hasSubStmt);
   REGISTER_MATCHER(hasSyntacticForm);
   REGISTER_MATCHER(hasTargetDecl);
   REGISTER_MATCHER(hasTemplateArgument);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2737,6 +2737,22 @@
   return false;
 }
 
+/// Matches the statement an attribute is attached to.
+///
+/// Example:
+/// \code
+///   attributedStmt(hasSubStmt(returnStmt()))
+/// \endcode
+/// would match return 1; here:
+/// \code
+///   else [[unlikely]]
+/// return 1;
+/// \endcode
+AST_MATCHER_P(AttributedStmt, hasSubStmt, internal::Matcher, InnerMatcher) {
+  const Stmt *const Statement = Node.getSubStmt();
+  return (Statement != nullptr && InnerMatcher.matches(*Statement, Finder, Builder));
+}
+
 /// Matches \c QualTypes in the clang AST.
 extern const internal::VariadicAllOfMatcher qualType;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D120902: [clang-format] Fix assertion failure/crash with `AllowShortFunctionsOnASingleLine: Inline/InlineOnly`.

2022-03-03 Thread Owen Pan via Phabricator via cfe-commits
owenpan added inline comments.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:313
   break;
+  if ((*J)->Level == TheLine->Level)
+return false;

To be safe?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120902

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


[PATCH] D120949: [clang][AST matchers] adding hasSubStmt matcher under attributedStmt so we can actually match the stmt inside the attribute is attached to

2022-03-03 Thread Alister Johnson via Phabricator via cfe-commits
ajohnson-uoregon updated this revision to Diff 412871.
ajohnson-uoregon added a comment.
Herald added a reviewer: aaron.ballman.

Fixing to actually have the right commits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120949

Files:
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/Basic/Attr.td
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/lib/Sema/SemaStmtAttr.cpp

Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -233,6 +233,11 @@
   return ::new (S.Context) UnlikelyAttr(S.Context, A);
 }
 
+static Attr *handleMatcherBlock(Sema , Stmt *St, const ParsedAttr ,
+SourceRange Range) {
+  return ::new (S.Context) MatcherBlockAttr(S.Context, A);
+}
+
 #define WANT_STMT_MERGE_LOGIC
 #include "clang/Sema/AttrParsedAttrImpl.inc"
 #undef WANT_STMT_MERGE_LOGIC
@@ -424,6 +429,8 @@
 return handleLikely(S, St, A, Range);
   case ParsedAttr::AT_Unlikely:
 return handleUnlikely(S, St, A, Range);
+  case ParsedAttr::AT_MatcherBlock:
+return handleMatcherBlock(S, St, A, Range);
   default:
 // N.B., ClangAttrEmitter.cpp emits a diagnostic helper that ensures a
 // declaration attribute is not written on a statement, but this code is
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -142,6 +142,7 @@
   REGISTER_MATCHER(atomicExpr);
   REGISTER_MATCHER(atomicType);
   REGISTER_MATCHER(attr);
+  REGISTER_MATCHER(attributedStmt);
   REGISTER_MATCHER(autoType);
   REGISTER_MATCHER(autoreleasePoolStmt)
   REGISTER_MATCHER(binaryConditionalOperator);
@@ -355,6 +356,7 @@
   REGISTER_MATCHER(hasSpecializedTemplate);
   REGISTER_MATCHER(hasStaticStorageDuration);
   REGISTER_MATCHER(hasStructuredBlock);
+  REGISTER_MATCHER(hasSubStmt);
   REGISTER_MATCHER(hasSyntacticForm);
   REGISTER_MATCHER(hasTargetDecl);
   REGISTER_MATCHER(hasTemplateArgument);
@@ -395,6 +397,7 @@
   REGISTER_MATCHER(isArrow);
   REGISTER_MATCHER(isAssignmentOperator);
   REGISTER_MATCHER(isAtPosition);
+  REGISTER_MATCHER(isAttr);
   REGISTER_MATCHER(isBaseInitializer);
   REGISTER_MATCHER(isBitField);
   REGISTER_MATCHER(isCatchAll);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -981,6 +981,8 @@
 predefinedExpr;
 const internal::VariadicDynCastAllOfMatcher
 designatedInitExpr;
+const internal::VariadicDynCastAllOfMatcher
+attributedStmt;
 const internal::VariadicOperatorMatcherFunc<
 2, std::numeric_limits::max()>
 eachOf = {internal::DynTypedMatcher::VO_EachOf};
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -3959,3 +3959,9 @@
   let Args = [StringArgument<"MatcherName">];
   let Documentation = [Undocumented];
 }
+
+def MatcherBlock : StmtAttr {
+  let Spellings = [Clang<"matcher_block">];
+  let Subjects = SubjectList<[CompoundStmt], ErrorDiag, "compound stmts">;
+  let Documentation = [Undocumented];
+}
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2699,6 +2699,60 @@
   return Node.size() == N;
 }
 
+/// Matches the attribute(s) attached to a Stmt
+///
+/// Example: Matches [[likely]] and [[unlikely]]
+/// \code
+///   constexpr double pow(double x, long long n) noexcept {
+/// if (n > 0) [[likely]]
+///  return x * pow(x, n - 1);
+/// else [[unlikely]]
+///  return 1;
+///   }
+/// \endcode
+extern const internal::VariadicDynCastAllOfMatcher
+attributedStmt;
+
+/// Matches the specified attribute.
+///
+/// Example:
+/// \code
+///   attributedStmt(isAttr(attr::Likely))
+/// \endcode
+/// would only match [[likely]] here:
+/// \code
+///   constexpr double pow(double x, long long n) noexcept {
+/// if (n > 0) [[likely]]
+///  return x * pow(x, n - 1);
+/// else [[unlikely]]
+///  return 1;
+///   }
+/// \endcode
+AST_MATCHER_P(AttributedStmt, isAttr, attr::Kind, AttrKind) {
+  for (const auto *Attr : Node.getAttrs()) {
+if (Attr->getKind() == AttrKind) {
+  return true;
+}
+  }
+  return false;
+}
+
+/// Matches the statement an attribute is attached to.
+///
+/// Example:
+/// \code
+///   attributedStmt(hasSubStmt(returnStmt()))
+/// \endcode
+/// would match return 1; here:
+/// 

[PATCH] D120949: [clang][AST matchers] adding hasSubStmt matcher under attributedStmt so we can actually match the stmt inside the attribute is attached to

2022-03-03 Thread Alister Johnson via Phabricator via cfe-commits
ajohnson-uoregon created this revision.
Herald added a project: All.
ajohnson-uoregon requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.

fixing to use new [[clang::matcher_block]] attr and clean up some old code; 
currently inserts/replaces code at the wrong place because it thinks the 
compoundStmt brackets are the start and end of the match when really it's one 
char after/before...

...the brackets


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120949

Files:
  clang-tools-extra/clang-rewrite/ClangRewrite.cpp
  clang-tools-extra/clang-rewrite/MatcherGenCallback.h
  clang-tools-extra/clang-rewrite/NewCodeCallback.h
  clang-tools-extra/clang-rewrite/RewriteCallback.h
  clang-tools-extra/clang-rewrite/tests/new.cpp
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp

Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -356,6 +356,7 @@
   REGISTER_MATCHER(hasSpecializedTemplate);
   REGISTER_MATCHER(hasStaticStorageDuration);
   REGISTER_MATCHER(hasStructuredBlock);
+  REGISTER_MATCHER(hasSubStmt);
   REGISTER_MATCHER(hasSyntacticForm);
   REGISTER_MATCHER(hasTargetDecl);
   REGISTER_MATCHER(hasTemplateArgument);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2737,6 +2737,22 @@
   return false;
 }
 
+/// Matches the statement an attribute is attached to.
+///
+/// Example:
+/// \code
+///   attributedStmt(hasSubStmt(returnStmt()))
+/// \endcode
+/// would match return 1; here:
+/// \code
+///   else [[unlikely]]
+/// return 1;
+/// \endcode
+AST_MATCHER_P(AttributedStmt, hasSubStmt, internal::Matcher, InnerMatcher) {
+  const Stmt *const Statement = Node.getSubStmt();
+  return (Statement != nullptr && InnerMatcher.matches(*Statement, Finder, Builder));
+}
+
 /// Matches \c QualTypes in the clang AST.
 extern const internal::VariadicAllOfMatcher qualType;
 
Index: clang-tools-extra/clang-rewrite/tests/new.cpp
===
--- clang-tools-extra/clang-rewrite/tests/new.cpp
+++ clang-tools-extra/clang-rewrite/tests/new.cpp
@@ -6,14 +6,26 @@
   for (int i = 0; i < 3; i++) {
 x = i;
   }
+  [[likely]]
+  {
+printf("not a matcher\n");
+  }
+  [[clang::matcher_block]]
   {
 return x;
   }
+}
 
+constexpr double pow(double x, long long n) noexcept {
+if (n > 0) [[likely]]
+return x * pow(x, n - 1);
+else [[unlikely]]
+return 1;
 }
 
 // [[clang::matcher("cuda_kernel")]]
 // auto kern() {
+//   [[clang::matcher_block]]
 //   {
 // kernel<<>>(arg1, arg2, ...);
 //   }
@@ -21,25 +33,37 @@
 //
 // [[clang::replace("cuda_kernel")]]
 // auto hip() {
-//   {
-// hip_launch(kernkel, numblocks, numthreads, 0, 0, arg1, arg2, ...);
+//   if (kernel == "gaussian") {
+// [[clang::matcher_block]]
+// {
+//   hip_launch(kernel, numblocks, numthreads, 0, 0, arg1, arg2, ...);
+// }
 //   }
 // }
 
 
 [[clang::replace("returns")]]
 auto return42() {
-  return 42;
+  [[clang::matcher_block]]
+  {
+return 42;
+  }
 }
 
 [[clang::insert_before("returns", "thencode")]]
 auto foobar() {
-  printf("returning\n");;
+  [[clang::matcher_block]]
+  {
+printf("returning\n");
+  }
 }
 
 [[clang::insert_after("thencode")]]
 auto helloworld() {
-  printf("hello world\n");
+  [[clang::matcher_block]]
+  {
+printf("hello world\n");
+  }
 }
 
 int main() {
Index: clang-tools-extra/clang-rewrite/RewriteCallback.h
===
--- clang-tools-extra/clang-rewrite/RewriteCallback.h
+++ clang-tools-extra/clang-rewrite/RewriteCallback.h
@@ -137,11 +137,11 @@
 unsigned int end_line = end.getSpellingLineNumber();
 unsigned int end_col = end.getSpellingColumnNumber();
 
-if (verbose) {
+// if (verbose) {
   printf("FOUND match for %s at %d:%d - %d:%d\n",
  matcher->getName().c_str(), begin_line, begin_col, end_line,
  end_col);
-}
+// }
 if (rw.isRewritable(match->getBeginLoc()) &&
 rw.isRewritable(match->getEndLoc())) {
 
Index: clang-tools-extra/clang-rewrite/NewCodeCallback.h
===
--- clang-tools-extra/clang-rewrite/NewCodeCallback.h
+++ clang-tools-extra/clang-rewrite/NewCodeCallback.h
@@ -57,13 +57,39 @@
 // .bind("replace");
 
 DeclarationMatcher insert_before_match =
-  functionDecl(hasAttr(attr::InsertCodeBefore)).bind("insert_before_match");
+  functionDecl(allOf(
+hasAttr(attr::InsertCodeBefore),
+hasBody(compoundStmt(
+  

[PATCH] D120395: [X86] Prohibit arithmetic operations on type `__bfloat16`

2022-03-03 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added a comment.

In D120395#3358533 , @craig.topper 
wrote:

> In D120395#3358453 , 
> @andrew.w.kaylor wrote:
>
>> In D120395#3356355 , @pengfei 
>> wrote:
>>
>>> Good question! This is actually the scope of ABI. Unfortunately, we don't 
>>> have the BF16 ABI at the present. We can't assume what are the physical 
>>> registers the arguments been passed and returned before we have such a 
>>> hardware. For example, ARM has soft FP ABI that supports FP arithmetic 
>>> operations and passes and returns arguments by integer registers. When we 
>>> enabling some ISA set whose type doesn't have ABI representation, e.g., 
>>> F16C, we borrowed such conception. And as a trade off, we used integer 
>>> rather than introducing a new IR type, since we don't need to support the 
>>> arithmetic operations.
>>
>> I don't see the point of the ARM soft-float comparison, given that X86 
>> doesn't have the strict distinction between integer and floating point 
>> registers that ARM has, at least not for the XMM/YMM/ZMM registers. Consider 
>> the following code:
>>
>>   __m128bh foo(__m128 x) {
>> return _mm_cvtneps_pbh(x);
>>   }
>>   __m128 bar(__m128bh x) {
>> return _mm_cvtpbh_ps(x);
>>   }
>>
>> Currently, both clang and gcc will use XMM0 for the argument and return 
>> value in both functions. Is XMM0 an integer register or a floating point 
>> register? There is no such distinction. It's true that the x86_64 psABI does 
>> talk about the general purpose registers as integer registers, and both 
>> clang and gcc will use one of these registers for `__bfloat16` values, but 
>> that's an implementation detail (and a dubious one, considering that nearly 
>> anything useful that you can do with a `__bfloat16` will require moving it 
>> into an SSE register).
>>
>> Also, you say we can't assume what registers will be used (in the eventual 
>> ABI?) but we are assuming exactly that. If the ABI is ever defined 
>> differently than what clang and gcc are currently doing, they will both be 
>> wrong.
>>
>> But all of this only applies to the backend code generation. It has very 
>> little to do with the intrinsic definition in the header file or the IR 
>> generated by the front end. If we continue to define `__bfloat16` as an 
>> `unsigned short` in the header file, the front end will treat it as an 
>> `unsigned short` and it will use its rules for `unsigned short` to generate 
>> IR. If the ABI is ever defined to treat BF16 differently than `unsigned 
>> short`, the front end won't be able to do anything about that because we've 
>> told the front end that the value is an unsigned short.
>>
>> On the other hand, if we define the `__bfloat16` type as the built-in 
>> `__bf16` type, then the front end can apply whatever rules it has for that 
>> type, including adding whatever ABI handling is needed for BF16 values. If 
>> that ends up being the same as the rules for `unsigned short`, that's no 
>> problem. The front end can implement it that way. If it ends up being 
>> something different, the front end can apply rules for whatever the 
>> alternative is. The point is, by telling the front end that this is a BF16 
>> value, we allow the front end to control the semantics for it. This would 
>> then result in the front end generating IR using `bfloat` as the type for 
>> BF16 values. Again, this is a correct and accurate description of the value. 
>> It allows the optimizer to reason about it correctly in any way it needs to.
>>
>> I don't see why we would treat BF16 values as `unsigned short` and `i16` 
>> throughout the compiler just to make the backend implementation easier when 
>> we already have types available for BF16.
>
> __m256bh should not have been a new type. It should have been an alias of 
> __m256i. We don't have load/store intrinsics for __m256bh so if you can even 
> get the __m256bh type in and out of memory using load/store intrinsics, it is 
> only because we allow lax vector conversion by default. 
> -fno-lax-vector-conversions will probably break any code trying to load/store 
> it using a load/store intrinsic. If __m256bh was made a struct as at one 
> point proposed, this would have been broken.
>
> If we want __m256bh to be a unique type using __bf16, we must define load, 
> store, and cast intrinsics for it. We would probably want insert/extract 
> element intrinsics as well.

-fno-lax-vector-conversions does indeed break the load/store intrinsics 
https://godbolt.org/z/b5WPj84Pa


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120395

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


[PATCH] D120395: [X86] Prohibit arithmetic operations on type `__bfloat16`

2022-03-03 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added a comment.

In D120395#3358453 , @andrew.w.kaylor 
wrote:

> In D120395#3356355 , @pengfei wrote:
>
>> Good question! This is actually the scope of ABI. Unfortunately, we don't 
>> have the BF16 ABI at the present. We can't assume what are the physical 
>> registers the arguments been passed and returned before we have such a 
>> hardware. For example, ARM has soft FP ABI that supports FP arithmetic 
>> operations and passes and returns arguments by integer registers. When we 
>> enabling some ISA set whose type doesn't have ABI representation, e.g., 
>> F16C, we borrowed such conception. And as a trade off, we used integer 
>> rather than introducing a new IR type, since we don't need to support the 
>> arithmetic operations.
>
> I don't see the point of the ARM soft-float comparison, given that X86 
> doesn't have the strict distinction between integer and floating point 
> registers that ARM has, at least not for the XMM/YMM/ZMM registers. Consider 
> the following code:
>
>   __m128bh foo(__m128 x) {
> return _mm_cvtneps_pbh(x);
>   }
>   __m128 bar(__m128bh x) {
> return _mm_cvtpbh_ps(x);
>   }
>
> Currently, both clang and gcc will use XMM0 for the argument and return value 
> in both functions. Is XMM0 an integer register or a floating point register? 
> There is no such distinction. It's true that the x86_64 psABI does talk about 
> the general purpose registers as integer registers, and both clang and gcc 
> will use one of these registers for `__bfloat16` values, but that's an 
> implementation detail (and a dubious one, considering that nearly anything 
> useful that you can do with a `__bfloat16` will require moving it into an SSE 
> register).
>
> Also, you say we can't assume what registers will be used (in the eventual 
> ABI?) but we are assuming exactly that. If the ABI is ever defined 
> differently than what clang and gcc are currently doing, they will both be 
> wrong.
>
> But all of this only applies to the backend code generation. It has very 
> little to do with the intrinsic definition in the header file or the IR 
> generated by the front end. If we continue to define `__bfloat16` as an 
> `unsigned short` in the header file, the front end will treat it as an 
> `unsigned short` and it will use its rules for `unsigned short` to generate 
> IR. If the ABI is ever defined to treat BF16 differently than `unsigned 
> short`, the front end won't be able to do anything about that because we've 
> told the front end that the value is an unsigned short.
>
> On the other hand, if we define the `__bfloat16` type as the built-in 
> `__bf16` type, then the front end can apply whatever rules it has for that 
> type, including adding whatever ABI handling is needed for BF16 values. If 
> that ends up being the same as the rules for `unsigned short`, that's no 
> problem. The front end can implement it that way. If it ends up being 
> something different, the front end can apply rules for whatever the 
> alternative is. The point is, by telling the front end that this is a BF16 
> value, we allow the front end to control the semantics for it. This would 
> then result in the front end generating IR using `bfloat` as the type for 
> BF16 values. Again, this is a correct and accurate description of the value. 
> It allows the optimizer to reason about it correctly in any way it needs to.
>
> I don't see why we would treat BF16 values as `unsigned short` and `i16` 
> throughout the compiler just to make the backend implementation easier when 
> we already have types available for BF16.

__m256bh should not have been a new type. It should have been an alias of 
__m256i. We don't have load/store intrinsics for __m256bh so if you can even 
get the __m256bh type in and out of memory using load/store intrinsics, it is 
only because we allow lax vector conversion by default. 
-fno-lax-vector-conversions will probably break any code trying to load/store 
it using a load/store intrinsic. If __m256bh was made a struct as at one point 
proposed, this would have been broken.

If we want __m256bh to be a unique type using __bf16, we must define load, 
store, and cast intrinsics for it. We would probably want insert/extract 
element intrinsics as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120395

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


[PATCH] D120947: [tooling] Explain how to create a compilation database on Windows [NFC]

2022-03-03 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood created this revision.
LegalizeAdulthood added reviewers: njames93, aaron.ballman, ymandel.
LegalizeAdulthood added a project: clang-tools-extra.
Herald added a project: All.
LegalizeAdulthood requested review of this revision.
Herald added a project: clang.

Document how to create a `compile_commands.json` file on Windows


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120947

Files:
  clang/docs/HowToSetupToolingForLLVM.rst


Index: clang/docs/HowToSetupToolingForLLVM.rst
===
--- clang/docs/HowToSetupToolingForLLVM.rst
+++ clang/docs/HowToSetupToolingForLLVM.rst
@@ -53,6 +53,55 @@
 
   $ make check-all
 
+Setup Clang Tooling Using CMake on Windows
+==
+
+For Windows developers, the Visual Studio project generators in CMake do
+not support `CMAKE_EXPORT_COMPILE_COMMANDS
+`_.
+However, the Ninja generator does support this variable and can be used
+on Windows to generate a suitable ``compile_commands.json`` that invokes
+the MSVC compiler.
+
+First, you will need to install `Ninja`_.  Once
+installed, the Ninja executable will need to be in your search path for CMake
+to locate it.
+
+Next, assuming you already have Visual Studio installed on your machine, you
+need to have the appropriate environment variables configured so that CMake
+will locate the MSVC compiler for the Ninja generator.  The `documentation
+`_
+describes the necessary environment variable settings, but the simplest thing
+is to use a `developer command-prompt window
+`_
+or call a `developer command file
+`_
+to set the environment variables appropriately.
+
+Now you can run CMake with the Ninja generator to export a compilation
+database:
+
+.. code-block:: console
+
+  C:\> mkdir build-ninja
+  C:\> cd build-ninja
+  C:\build-ninja> cmake -G Ninja -DCMAKE_EXPORT_COMPILE_COMMANDS=ON 
path/to/llvm/sources
+
+It is best to keep your Visual Studio IDE build folder separate from the
+Ninja build folder.  This prevents the two build systems from negatively
+interacting with each other.
+
+Once the ``compile_commands.json`` file has been created by Ninja, you can
+use that compilation database with Clang Tooling.  One caveat is that because
+there are indirect settings obtained through the environment variables,
+you may need to run any Clang Tooling executables through a command prompt
+window created for use with Visual Studio as described above.  An
+alternative, e.g. for using the Visual Studio debugger on a Clang Tooling
+executable, is to ensure that the environment variables are also visible
+to the debugger settings.  This can be done locally in Visual Studio's
+debugger configuration locally or globally by launching the Visual Studio
+IDE from a suitable command-prompt window.
+
 Using Clang Tools
 =
 
@@ -143,7 +192,7 @@
 Using Ninja Build System
 ===
 
-Optionally you can use the `Ninja `_
+Optionally you can use the `Ninja`_
 build system instead of make. It is aimed at making your builds faster.
 Currently this step will require building Ninja from sources.
 
@@ -197,3 +246,5 @@
   $ ninja check-all
 
 Other target names can be used in the same way as with make.
+
+.. _Ninja: https://ninja-build.org/


Index: clang/docs/HowToSetupToolingForLLVM.rst
===
--- clang/docs/HowToSetupToolingForLLVM.rst
+++ clang/docs/HowToSetupToolingForLLVM.rst
@@ -53,6 +53,55 @@
 
   $ make check-all
 
+Setup Clang Tooling Using CMake on Windows
+==
+
+For Windows developers, the Visual Studio project generators in CMake do
+not support `CMAKE_EXPORT_COMPILE_COMMANDS
+`_.
+However, the Ninja generator does support this variable and can be used
+on Windows to generate a suitable ``compile_commands.json`` that invokes
+the MSVC compiler.
+
+First, you will need to install `Ninja`_.  Once
+installed, the Ninja executable will need to be in your search path for CMake
+to locate it.
+
+Next, assuming you already have Visual Studio installed on your machine, you
+need to have the appropriate environment variables configured so that CMake
+will locate the MSVC compiler for the Ninja generator.  The `documentation
+`_

[PATCH] D120931: [clang-format] fix namepsace format when the name is macro expansion

2022-03-03 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu added inline comments.



Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:86
+"int j;\n"
+"}// namespace A::M",
+fixNamespaceEndComments("#define M(x) x##x\n"

MyDeveloperDay wrote:
> same here its not A::M is it? at best its A::x but it depends on the macro ? 
> or have I miss understood?
Yes, you are right. Fixed to include the macro literally. 
Ideally, it should expand the macro to get the name or ignore the macro. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120931

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


[PATCH] D120931: [clang-format] fix namepsace format when the name is macro expansion

2022-03-03 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu updated this revision to Diff 412856.
zequanwu marked 2 inline comments as done.
zequanwu added a comment.

Address comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120931

Files:
  clang/lib/Format/NamespaceEndCommentsFixer.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -68,6 +68,57 @@
 "int i;\n"
 "int j;\n"
 "}"));
+  EXPECT_EQ("#define M(x) x##x\n"
+"namespace M(x) {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace M(x)",
+fixNamespaceEndComments("#define M(x) x##x\n"
+"namespace M(x) {\n"
+"int i;\n"
+"int j;\n"
+"}"));
+  EXPECT_EQ("#define M(x) x##x\n"
+"namespace A::M(x) {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A::M(x)",
+fixNamespaceEndComments("#define M(x) x##x\n"
+"namespace A::M(x) {\n"
+"int i;\n"
+"int j;\n"
+"}"));
+  EXPECT_EQ("#define M(x) x##x\n"
+"namespace M(x)::A {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace M(x)::A",
+fixNamespaceEndComments("#define M(x) x##x\n"
+"namespace M(x)::A {\n"
+"int i;\n"
+"int j;\n"
+"}"));
+  EXPECT_EQ("#define M(x) x##x\n"
+"namespace A::inline M(x)::B {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A::inline M(x)::B",
+fixNamespaceEndComments("#define M(x) x##x\n"
+"namespace A::inline M(x)::B {\n"
+"int i;\n"
+"int j;\n"
+"}"));
+  EXPECT_EQ("#define M(x) x##x\n"
+"namespace [[deprecated(\"foo\")]] A::inline M(x)::A {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A::inline M(x)::A",
+fixNamespaceEndComments(
+"#define M(x) x##x\n"
+"namespace [[deprecated(\"foo\")]] A::inline M(x)::A {\n"
+"int i;\n"
+"int j;\n"
+"}"));
   EXPECT_EQ("inline namespace A {\n"
 "int i;\n"
 "int j;\n"
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3738,6 +3738,36 @@
"void f() { f(); }\n"
"}",
LLVMWithNoNamespaceFix);
+  verifyFormat("#define M(x) x##x\n"
+   "namespace M(x) {\n"
+   "class A {};\n"
+   "void f() { f(); }\n"
+   "}",
+   LLVMWithNoNamespaceFix);
+  verifyFormat("#define M(x) x##x\n"
+   "namespace N::inline M(x) {\n"
+   "class A {};\n"
+   "void f() { f(); }\n"
+   "}",
+   LLVMWithNoNamespaceFix);
+  verifyFormat("#define M(x) x##x\n"
+   "namespace M(x)::inline N {\n"
+   "class A {};\n"
+   "void f() { f(); }\n"
+   "}",
+   LLVMWithNoNamespaceFix);
+  verifyFormat("#define M(x) x##x\n"
+   "namespace N::M(x) {\n"
+   "class A {};\n"
+   "void f() { f(); }\n"
+   "}",
+   LLVMWithNoNamespaceFix);
+  verifyFormat("#define M(x) x##x\n"
+   "namespace M::N(x) {\n"
+   "class A {};\n"
+   "void f() { f(); }\n"
+   "}",
+   LLVMWithNoNamespaceFix);
   verifyFormat("namespace N::inline D {\n"
"class A {};\n"
"void f() { f(); }\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2597,10 +2597,12 @@
 parseParens();
   } else {
 while (FormatTok->isOneOf(tok::identifier, tok::coloncolon, tok::kw_inline,
-  tok::l_square, tok::period) 

[PATCH] D118311: [Clang][ModuleMap] Add conditional parsing via requires block declaration

2022-03-03 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno requested changes to this revision.
bruno added a comment.
This revision now requires changes to proceed.
Herald added a project: All.

> It would be nice to have some mechanism to notify developers that includes 
> are still performed regardless of requires

I'd like to see a module remark for whenever a match fails 
`-Rmodule-requires-fail` or something like that. I've been bitten before by a 
regular submodule `requires` not triggering, just to find out some specific 
subfeature was missing in the compiler invocation - it can take a while to spot 
that. Doesn't need to be a blocker for getting this in though.




Comment at: clang/docs/Modules.rst:657
+}
+  }
+

Nice doc and examples!



Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:723
+  "invalid requires declaration outside of a module declaration, did you mean"
+  " to add '{' to open a requires block?">;
 def err_mmap_expected_rbrace : Error<"expected '}'">;

I'm not really too worried about a fixit here, but it would be nice if the "did 
you mean..." part came out of a note instead.



Comment at: clang/lib/Lex/ModuleMap.cpp:2355
+parseModuleMembers();
+  }
+}

Too many curly braces?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118311

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


[PATCH] D120395: [X86] Prohibit arithmetic operations on type `__bfloat16`

2022-03-03 Thread Andy Kaylor via Phabricator via cfe-commits
andrew.w.kaylor added a comment.

In D120395#3356355 , @pengfei wrote:

> Good question! This is actually the scope of ABI. Unfortunately, we don't 
> have the BF16 ABI at the present. We can't assume what are the physical 
> registers the arguments been passed and returned before we have such a 
> hardware. For example, ARM has soft FP ABI that supports FP arithmetic 
> operations and passes and returns arguments by integer registers. When we 
> enabling some ISA set whose type doesn't have ABI representation, e.g., F16C, 
> we borrowed such conception. And as a trade off, we used integer rather than 
> introducing a new IR type, since we don't need to support the arithmetic 
> operations.

I don't see the point of the ARM soft-float comparison, given that X86 doesn't 
have the strict distinction between integer and floating point registers that 
ARM has, at least not for the XMM/YMM/ZMM registers. Consider the following 
code:

  __m128bh foo(__m128 x) {
return _mm_cvtneps_pbh(x);
  }
  __m128 bar(__m128bh x) {
return _mm_cvtpbh_ps(x);
  }

Currently, both clang and gcc will use XMM0 for the argument and return value 
in both functions. Is XMM0 an integer register or a floating point register? 
There is no such distinction. It's true that the x86_64 psABI does talk about 
the general purpose registers as integer registers, and both clang and gcc will 
use one of these registers for `__bfloat16` values, but that's an 
implementation detail (and a dubious one, considering that nearly anything 
useful that you can do with a `__bfloat16` will require moving it into an SSE 
register).

Also, you say we can't assume what registers will be used (in the eventual 
ABI?) but we are assuming exactly that. If the ABI is ever defined differently 
than what clang and gcc are currently doing, they will both be wrong.

But all of this only applies to the backend code generation. It has very little 
to do with the intrinsic definition in the header file or the IR generated by 
the front end. If we continue to define `__bfloat16` as an `unsigned short` in 
the header file, the front end will treat it as an `unsigned short` and it will 
use its rules for `unsigned short` to generate IR. If the ABI is ever defined 
to treat BF16 differently than `unsigned short`, the front end won't be able to 
do anything about that because we've told the front end that the value is an 
unsigned short.

On the other hand, if we define the `__bfloat16` type as the built-in `__bf16` 
type, then the front end can apply whatever rules it has for that type, 
including adding whatever ABI handling is needed for BF16 values. If that ends 
up being the same as the rules for `unsigned short`, that's no problem. The 
front end can implement it that way. If it ends up being something different, 
the front end can apply rules for whatever the alternative is. The point is, by 
telling the front end that this is a BF16 value, we allow the front end to 
control the semantics for it. This would then result in the front end 
generating IR using `bfloat` as the type for BF16 values. Again, this is a 
correct and accurate description of the value. It allows the optimizer to 
reason about it correctly in any way it needs to.

I don't see why we would treat BF16 values as `unsigned short` and `i16` 
throughout the compiler just to make the backend implementation easier when we 
already have types available for BF16.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120395

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


[clang] b4c1cbf - [hmaptool] Fix string decoding for Python 3

2022-03-03 Thread Shoaib Meenai via cfe-commits

Author: Shoaib Meenai
Date: 2022-03-03T15:18:21-08:00
New Revision: b4c1cbff79d0631c35ca2efa97bd2a47929945b8

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

LOG: [hmaptool] Fix string decoding for Python 3

Our "strings" were actually bytes, which made verbose dumping fail.
Decode them so they actually become strings.

Reviewed By: bruno

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

Added: 


Modified: 
clang/utils/hmaptool/hmaptool

Removed: 




diff  --git a/clang/utils/hmaptool/hmaptool b/clang/utils/hmaptool/hmaptool
index 73897c8bb8e9f..d7754632b162b 100755
--- a/clang/utils/hmaptool/hmaptool
+++ b/clang/utils/hmaptool/hmaptool
@@ -100,7 +100,7 @@ class HeaderMap(object):
 raise SystemExit("error: %s: invalid string index" % (
 idx,))
 end_idx = self.strtable.index(0, idx)
-return self.strtable[idx:end_idx]
+return self.strtable[idx:end_idx].decode()
 
 @property
 def mappings(self):



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


[PATCH] D118005: [hmaptool] Fix string decoding for Python 3

2022-03-03 Thread Shoaib Meenai via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb4c1cbff79d0: [hmaptool] Fix string decoding for Python 3 
(authored by smeenai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118005

Files:
  clang/utils/hmaptool/hmaptool


Index: clang/utils/hmaptool/hmaptool
===
--- clang/utils/hmaptool/hmaptool
+++ clang/utils/hmaptool/hmaptool
@@ -100,7 +100,7 @@
 raise SystemExit("error: %s: invalid string index" % (
 idx,))
 end_idx = self.strtable.index(0, idx)
-return self.strtable[idx:end_idx]
+return self.strtable[idx:end_idx].decode()
 
 @property
 def mappings(self):


Index: clang/utils/hmaptool/hmaptool
===
--- clang/utils/hmaptool/hmaptool
+++ clang/utils/hmaptool/hmaptool
@@ -100,7 +100,7 @@
 raise SystemExit("error: %s: invalid string index" % (
 idx,))
 end_idx = self.strtable.index(0, idx)
-return self.strtable[idx:end_idx]
+return self.strtable[idx:end_idx].decode()
 
 @property
 def mappings(self):
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D118004: [hmaptool] Fix dumping

2022-03-03 Thread Shoaib Meenai via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0c1d330431f5: [hmaptool] Fix dumping (authored by smeenai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118004

Files:
  clang/utils/hmaptool/hmaptool


Index: clang/utils/hmaptool/hmaptool
===
--- clang/utils/hmaptool/hmaptool
+++ clang/utils/hmaptool/hmaptool
@@ -44,7 +44,7 @@
 path,))
 
 (version, reserved, strtable_offset, num_entries,
- num_buckets) = struct.unpack(header_fmt, data)
+ num_buckets, _) = struct.unpack(header_fmt, data)
 
 if version != 1:
 raise SystemExit("error: %s: unknown headermap version: %r" % (


Index: clang/utils/hmaptool/hmaptool
===
--- clang/utils/hmaptool/hmaptool
+++ clang/utils/hmaptool/hmaptool
@@ -44,7 +44,7 @@
 path,))
 
 (version, reserved, strtable_offset, num_entries,
- num_buckets) = struct.unpack(header_fmt, data)
+ num_buckets, _) = struct.unpack(header_fmt, data)
 
 if version != 1:
 raise SystemExit("error: %s: unknown headermap version: %r" % (
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 0c1d330 - [hmaptool] Fix dumping

2022-03-03 Thread Shoaib Meenai via cfe-commits

Author: Shoaib Meenai
Date: 2022-03-03T15:18:09-08:00
New Revision: 0c1d330431f59011fd861e291d5048858a973c5a

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

LOG: [hmaptool] Fix dumping

It was complaining about too many values to unpack, since our struct
unpack format string specified six members, but we only had five
variables to unpack to. The sixth value is the max value length, but
it's not used in dumping, so we can ignore it.

Reviewed By: bruno

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

Added: 


Modified: 
clang/utils/hmaptool/hmaptool

Removed: 




diff  --git a/clang/utils/hmaptool/hmaptool b/clang/utils/hmaptool/hmaptool
index 7949002020489..73897c8bb8e9f 100755
--- a/clang/utils/hmaptool/hmaptool
+++ b/clang/utils/hmaptool/hmaptool
@@ -44,7 +44,7 @@ class HeaderMap(object):
 path,))
 
 (version, reserved, strtable_offset, num_entries,
- num_buckets) = struct.unpack(header_fmt, data)
+ num_buckets, _) = struct.unpack(header_fmt, data)
 
 if version != 1:
 raise SystemExit("error: %s: unknown headermap version: %r" % (



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


[PATCH] D118005: [hmaptool] Fix string decoding for Python 3

2022-03-03 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno accepted this revision.
bruno added a comment.
This revision is now accepted and ready to land.
Herald added a project: All.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118005

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


[PATCH] D118004: [hmaptool] Fix dumping

2022-03-03 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno accepted this revision.
bruno added a comment.
This revision is now accepted and ready to land.
Herald added a project: All.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118004

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


[PATCH] D120931: [clang-format] fix namepsace format when the name is macro expansion

2022-03-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:76
+"int j;\n"
+"}// namespace",
+fixNamespaceEndComments("#define M(x) x##x\n"

but its not an anonymous namespace?

would

'''
}// namespace M(x)
'''

be clearer?



Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:86
+"int j;\n"
+"}// namespace A::M",
+fixNamespaceEndComments("#define M(x) x##x\n"

same here its not A::M is it? at best its A::x but it depends on the macro ? or 
have I miss understood?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120931

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


[clang] 19c1b08 - [CMake] Update cache file for Win to ARM cross tooolchain. NFC.

2022-03-03 Thread Vladimir Vereschaka via cfe-commits

Author: Vladimir Vereschaka
Date: 2022-03-03T14:32:16-08:00
New Revision: 19c1b084a7da9087fcdc16071b461ea33b6a68b4

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

LOG: [CMake] Update cache file for Win to ARM cross tooolchain. NFC.

Removed passing CMAKE_AR from the library configurations.

Added: 


Modified: 
clang/cmake/caches/CrossWinToARMLinux.cmake

Removed: 




diff  --git a/clang/cmake/caches/CrossWinToARMLinux.cmake 
b/clang/cmake/caches/CrossWinToARMLinux.cmake
index 5165992b7b923..5e62d01b297c2 100644
--- a/clang/cmake/caches/CrossWinToARMLinux.cmake
+++ b/clang/cmake/caches/CrossWinToARMLinux.cmake
@@ -42,9 +42,6 @@ if (NOT DEFINED DEFAULT_SYSROOT)
   message(WARNING "DEFAULT_SYSROOT must be specified for the cross toolchain 
build.")
 endif()
 
-if (DEFINED LLVM_AR)
-  set(CMAKE_AR "${LLVM_AR}" CACHE STRING "")
-endif()
 if (NOT DEFINED LLVM_TARGETS_TO_BUILD)
   set(LLVM_TARGETS_TO_BUILD "ARM" CACHE STRING "")
 endif()
@@ -111,7 +108,6 @@ set(LLVM_BUILTIN_TARGETS
"${TARGET_TRIPLE}" CACHE STRING "")
 
 set(BUILTINS_${TARGET_TRIPLE}_CMAKE_SYSTEM_NAME 
"Linux" CACHE STRING "")
 set(BUILTINS_${TARGET_TRIPLE}_CMAKE_SYSROOT 
"${DEFAULT_SYSROOT}"  CACHE STRING "")
-set(BUILTINS_${TARGET_TRIPLE}_CMAKE_AR  
"${CMAKE_AR}"  CACHE STRING "")
 set(BUILTINS_${TARGET_TRIPLE}_CMAKE_INSTALL_RPATH   
"${RUNTIMES_INSTALL_RPATH}"  CACHE STRING "")
 set(BUILTINS_${TARGET_TRIPLE}_CMAKE_BUILD_WITH_INSTALL_RPATHON  
CACHE BOOL "")
 
@@ -122,7 +118,6 @@ set(RUNTIMES_${TARGET_TRIPLE}_LLVM_ENABLE_RUNTIMES  
"${LLVM_
 
 set(RUNTIMES_${TARGET_TRIPLE}_CMAKE_SYSTEM_NAME 
"Linux" CACHE STRING "")
 set(RUNTIMES_${TARGET_TRIPLE}_CMAKE_SYSROOT 
"${DEFAULT_SYSROOT}"  CACHE STRING "")
-set(RUNTIMES_${TARGET_TRIPLE}_CMAKE_AR  
"${CMAKE_AR}"  CACHE STRING "")
 set(RUNTIMES_${TARGET_TRIPLE}_CMAKE_INSTALL_RPATH   
"${RUNTIMES_INSTALL_RPATH}"  CACHE STRING "")
 set(RUNTIMES_${TARGET_TRIPLE}_CMAKE_BUILD_WITH_INSTALL_RPATHON  
CACHE BOOL "")
 



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


[PATCH] D118350: [Clang][Sema][AIX][PowerPC] Emit byval alignment warning only when struct is passed to a function

2022-03-03 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:5246
+
+  if (const auto *ICE = dyn_cast(Arg->IgnoreParens())) {
+if (const auto *DR = dyn_cast(ICE->getSubExpr())) {

sfertile wrote:
> Nit: To avoid the deep nesting, can we instead have a series of casts, and if 
> the resulting pointer is null we return early?
Reworked it to use a few early returns. Does that work better? 



Comment at: clang/test/Sema/aix-attr-align.c:10
+  int a[8] __attribute__((aligned(8)));  // no-warning
+  int b[8] __attribute__((aligned(16))); // expected-warning {{alignment of 16 
bytes for a struct member is not binary compatible with IBM XL C/C++ for AIX 
16.1.0 or older}}
 };

sfertile wrote:
> As far as I am aware, the layout of a 16-byte aligned member is exactly the 
> same between the all the compilers on AIX, and the only incompatibility is 
> when passing them byval. If that's true then warning on the declaration here 
> is much too verbose, as most uses will be fine and warning on the callsite 
> (and later on the function definition) calls attention to it only when there 
> is indeed the possibility of an incompatability.
The way this test case was written it made it seem as if it would warn on any 
such declaration. I added `struct R` here where it has a member aligned 16 but 
it's not passed byval anywhere and no warning is expected. I also added a 
comment to make it clearer. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118350

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


[PATCH] D118350: [Clang][Sema][AIX][PowerPC] Emit byval alignment warning only when struct is passed to a function

2022-03-03 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA updated this revision to Diff 412840.
ZarkoCA added a comment.

- Remove formatting changes to unrelated code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118350

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Analysis/padding_c.c
  clang/test/Analysis/padding_cpp.cpp
  clang/test/CXX/drs/dr6xx.cpp
  clang/test/Sema/aix-attr-align.c
  clang/test/SemaTemplate/instantiate-attr.cpp

Index: clang/test/SemaTemplate/instantiate-attr.cpp
===
--- clang/test/SemaTemplate/instantiate-attr.cpp
+++ clang/test/SemaTemplate/instantiate-attr.cpp
@@ -1,7 +1,4 @@
-// FIXME -Wno-aix-compat added temporarily while the diagnostic is being
-// refined.
-
-// RUN: %clang_cc1 -fsyntax-only -verify -Wno-aix-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify %s
 // expected-no-diagnostics
 template 
 struct A {
Index: clang/test/Sema/aix-attr-align.c
===
--- clang/test/Sema/aix-attr-align.c
+++ clang/test/Sema/aix-attr-align.c
@@ -5,18 +5,37 @@
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -verify=off -Wno-aix-compat -fsyntax-only %s
 // RUN: %clang_cc1 -triple powerpc64le-unknown-linux -verify=off -fsyntax-only %s
 
-struct S {
-  int a[8] __attribute__((aligned(8))); // no-warning
+// We do not warn on any declaration with a member aligned 16. Only when the struct is passed byval.
+struct R {
+  int b[8] __attribute__((aligned(16))); // no-warning
 };
 
-struct T {
-  int a[4] __attribute__((aligned(16))); // expected-warning {{requesting an alignment of 16 bytes or greater for struct members is not binary compatible with IBM XL C/C++ for AIX 16.1.0 and older}}
+struct S {
+  int a[8] __attribute__((aligned(8)));  // no-warning
+  int b[8] __attribute__((aligned(16))); // expected-warning {{alignment of 16 bytes for a struct member is not binary compatible with IBM XL C/C++ for AIX 16.1.0 or older}}
 };
 
-struct U {
-  int a[2] __attribute__((aligned(32))); // expected-warning {{requesting an alignment of 16 bytes or greater for struct members is not binary compatible with IBM XL C/C++ for AIX 16.1.0 and older}}
+struct T {
+  int a[8] __attribute__((aligned(8))); // no-warning
+  int b[8] __attribute__((aligned(4))); // no-warning
 };
 
 int a[8] __attribute__((aligned(8)));  // no-warning
 int b[4] __attribute__((aligned(16))); // no-warning
-int c[2] __attribute__((aligned(32))); // no-warning
+
+void baz(int a, int b, int *c, int d, int *e, int f, struct S);
+void jaz(int a, int b, int *c, int d, int *e, int f, struct T);
+void vararg_baz(int a,...);
+static void static_baz(int a, int b, int *c, int d, int *e, int f, struct S sp2) {
+  a = *sp2.b + *c + *e;
+}
+
+void foo(int p1, int p2, int p3, int p4, int p5, int p6, int p7, int p8,
+ struct S s, struct T t) {
+
+  baz(p1, p2, s.b, p3, b, p5, s);// expected-note {{passing byval argument 's' with potentially incompatible alignment here}}
+  jaz(p1, p2, a, p3, s.a, p5, t);// no-note
+  jaz(p1, p2, s.b, p3, b, p5, t);// no-note
+  vararg_baz(p1, p2, s.b, p3, b, p5, s); // no-note
+  static_baz(p1, p2, s.b, p3, b, p5, s); // no-note
+}
Index: clang/test/CXX/drs/dr6xx.cpp
===
--- clang/test/CXX/drs/dr6xx.cpp
+++ clang/test/CXX/drs/dr6xx.cpp
@@ -1,10 +1,8 @@
-// FIXME -Wno-aix-compat added temporarily while the diagnostic is being
-// refined.
-// RUN: %clang_cc1 -std=c++98 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking -Wno-aix-compat
-// RUN: %clang_cc1 -std=c++11 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking -Wno-aix-compat
-// RUN: %clang_cc1 -std=c++14 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking -Wno-aix-compat
-// RUN: %clang_cc1 -std=c++17 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking -Wno-aix-compat
-// RUN: %clang_cc1 -std=c++20 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking -Wno-aix-compat
+// RUN: %clang_cc1 -std=c++98 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking
+// RUN: %clang_cc1 -std=c++11 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking
+// RUN: %clang_cc1 -std=c++14 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking
+// RUN: %clang_cc1 -std=c++17 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking
+// RUN: %clang_cc1 -std=c++20 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking
 
 namespace std {
   struct type_info {};
Index: clang/test/Analysis/padding_cpp.cpp

[PATCH] D118052: [X86] Fix CodeGen Module Flag for -mibt-seal

2022-03-03 Thread Joao Moreira via Phabricator via cfe-commits
joaomoreira planned changes to this revision.
joaomoreira added inline comments.
Herald added a project: All.



Comment at: clang/test/CodeGen/X86/x86-cf-protection.c:4
 // RUN: %clang -target i386-unknown-unknown -x c -E -dM -o - 
-fcf-protection=full %s   | FileCheck %s --check-prefix=FULL
+// RUN: %clang -target i386-unknown-unknown -o - -emit-llvm -S 
-fcf-protection=branch -mibt-seal -flto %s | FileCheck %s --check-prefix=IBTSEAL
 

pengfei wrote:
> Is `-flto` is required?
Yes, we can only suppress ENDBR if we are sure the given function is not 
address taken in all other translation units.



Comment at: clang/test/CodeGen/X86/x86-cf-protection.c:9-10
 // FULL: #define __CET__ 3
+// IBTSEAL: "cf-protection-branch", i32 1
+// IBTSEAL: "ibt-seal", i32 1
 void foo() {}

pengfei wrote:
> Can we add another RUN without `-mibt-seal` amd check no such flags?
Sure, I'll work on this try to track the possible bug mentioned by 
@aaron.ballman, then I'll update the diff.


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

https://reviews.llvm.org/D118052

___
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-03-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Essentially looks good to me now, thanks!




Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:789
+  else
+OS << " ";
+  D->getSourceRange().print(OS,

njames93 wrote:
> aaron.ballman wrote:
> > Should this be `" : "` instead?
> Good catch
It'd probably be good to add some test coverage for these weird edge cases 
(unnamed structs, constructors or other special member functions without a 
name).



Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:796
+MV.ActiveASTContext->getSourceManager());
+} else if (const auto *T = Item.second.get()) {
+  OS << T->getTypeClassName() << "Type : ";

njames93 wrote:
> aaron.ballman wrote:
> > njames93 wrote:
> > > aaron.ballman wrote:
> > > > Do we also need a match for `TypeLoc` matchers, or does the `else` 
> > > > cover that sufficiently well?
> > > > 
> > > > (Actually, should we handle all of the various matchers at: 
> > > > https://github.com/llvm/llvm-project/blob/main/clang/include/clang/ASTMatchers/ASTMatchers.h#L141
> > > >  rather than leaving it to an `else`? Then the `else` can become an 
> > > > unreachable so we know to update this interface?)
> > > The else should be sufficient for most general cases, the only reason 
> > > some are special cased is to improve the output, but I don't want there 
> > > to be a burden to update this interface if new nodes are added.
> > I should verify: does this map hold arbitrary AST nodes, or does the map 
> > only hold the top-level classes in the AST matching hierarchy?
> > 
> > If it's arbitrary AST nodes, then yeah, I definitely agree the `else` here 
> > is fine. If it's top-level classes instead, we don't add those all that 
> > often and so it doesn't seem like a major burden to keep those in sync.
> It can hold any kind of node that can be stored in a DynTypedNode, so 
> essentially it's any arbitrary AST node.
Thanks for clarifying that; the `else` look *beautiful* to me! :-D


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] D120185: [ASTMatchers] Output currently processing match and nodes on crash

2022-03-03 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments.



Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:796
+MV.ActiveASTContext->getSourceManager());
+} else if (const auto *T = Item.second.get()) {
+  OS << T->getTypeClassName() << "Type : ";

aaron.ballman wrote:
> njames93 wrote:
> > aaron.ballman wrote:
> > > Do we also need a match for `TypeLoc` matchers, or does the `else` cover 
> > > that sufficiently well?
> > > 
> > > (Actually, should we handle all of the various matchers at: 
> > > https://github.com/llvm/llvm-project/blob/main/clang/include/clang/ASTMatchers/ASTMatchers.h#L141
> > >  rather than leaving it to an `else`? Then the `else` can become an 
> > > unreachable so we know to update this interface?)
> > The else should be sufficient for most general cases, the only reason some 
> > are special cased is to improve the output, but I don't want there to be a 
> > burden to update this interface if new nodes are added.
> I should verify: does this map hold arbitrary AST nodes, or does the map only 
> hold the top-level classes in the AST matching hierarchy?
> 
> If it's arbitrary AST nodes, then yeah, I definitely agree the `else` here is 
> fine. If it's top-level classes instead, we don't add those all that often 
> and so it doesn't seem like a major burden to keep those in sync.
It can hold any kind of node that can be stored in a DynTypedNode, so 
essentially it's any arbitrary AST node.


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] D114425: [clang] Add __builtin_bswap128

2022-03-03 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:11071-11072
 
-QualType ElementType = DecodeTypeFromStr(Str, Context, Error,
- RequiresICE, false);
+QualType ElementType =
+DecodeTypeFromStr(Str, Context, Error, RequiresICE, false, true);
 assert(!RequiresICE && "Can't require vector ICE");

aaron.ballman wrote:
> Why is this change needed? We don't seem to make a vector of __int128 as part 
> of this patch, so I thought we wouldn't need the extra `AllowInt128` 
> parameter to the function.
It's also a little hard to imagine wanting to support a target that has vectors 
of `int128` but doesn't have `int128`. (If the target supports a vector of 
`int128`, why not use that to implement `int128`?) And there are ways to 
extract the element type from a vector type, so exposing such a vector type in 
a builtin would mean we expose non-vector `int128` too.



Comment at: clang/test/CodeGen/builtin-bswap128.c:1
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
+

This test needs a target to be specified; it will fail if run on a target that 
doesn't support `int128`.



Comment at: clang/test/CodeGen/builtin-bswap128.c:7-8
+// CHECK-NEXT:[[R2:%.*]] = alloca i128, align 16
+// CHECK-NEXT:store i128 1329227995784915872903807060280344576, i128* 
[[R1:%.*]], align 16
+// CHECK-NEXT:store i128 2658455991569831745807614120560689152, i128* 
[[R2:%.*]], align 16
+void test() {

You don't seem to have any test coverage for code generation of the `bswap` 
intrinsic; this is only testing the constant-evaluation path. I'd like to also 
see a test that covers a non-constant argument and ensures we call the 
appropriate LLVM intrinsic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114425

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


[PATCH] D120272: [CUDA] Add driver support for compiling CUDA with the new driver

2022-03-03 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:4099-4102
+  for (auto  : Args.getAllArgValues(options::OPT_offload_arch_EQ))
+Archs.insert(getCanonicalArchString(C, Args, Arg, Kind));
+  for (auto  : Args.getAllArgValues(options::OPT_no_offload_arch_EQ))
+Archs.erase(getCanonicalArchString(C, Args, Arg, Kind));

yaxunl wrote:
> The final set depends on the order of -offload-arch and -no-offload-arch 
> options, e.g. `--offload-arch=gfx906 --no-offload-arch=gfx906` and 
> `--no-offload-arch=gfx906 --offload-arch=gfx906` is different. Also there is 
> `--no-offload-arch=all` which removes all precedent --offload-arch= options.
I see, so we need to iterate the arguments in-order and insert or erase them as 
they are entered, I'll fix it.



Comment at: clang/lib/Driver/Driver.cpp:4107
+  Archs.insert(CudaArchToString(CudaArch::SM_35));
+else if (Kind == Action::OFK_Cuda)
+  Archs.insert(CudaArchToString(CudaArch::GFX803));

yaxunl wrote:
> should this be HIP?
Whoops, haven't tested it with HIP yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120272

___
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-03-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:796
+MV.ActiveASTContext->getSourceManager());
+} else if (const auto *T = Item.second.get()) {
+  OS << T->getTypeClassName() << "Type : ";

njames93 wrote:
> aaron.ballman wrote:
> > Do we also need a match for `TypeLoc` matchers, or does the `else` cover 
> > that sufficiently well?
> > 
> > (Actually, should we handle all of the various matchers at: 
> > https://github.com/llvm/llvm-project/blob/main/clang/include/clang/ASTMatchers/ASTMatchers.h#L141
> >  rather than leaving it to an `else`? Then the `else` can become an 
> > unreachable so we know to update this interface?)
> The else should be sufficient for most general cases, the only reason some 
> are special cased is to improve the output, but I don't want there to be a 
> burden to update this interface if new nodes are added.
I should verify: does this map hold arbitrary AST nodes, or does the map only 
hold the top-level classes in the AST matching hierarchy?

If it's arbitrary AST nodes, then yeah, I definitely agree the `else` here is 
fine. If it's top-level classes instead, we don't add those all that often and 
so it doesn't seem like a major burden to keep those in sync.


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] D120272: [CUDA] Add driver support for compiling CUDA with the new driver

2022-03-03 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:4099-4102
+  for (auto  : Args.getAllArgValues(options::OPT_offload_arch_EQ))
+Archs.insert(getCanonicalArchString(C, Args, Arg, Kind));
+  for (auto  : Args.getAllArgValues(options::OPT_no_offload_arch_EQ))
+Archs.erase(getCanonicalArchString(C, Args, Arg, Kind));

The final set depends on the order of -offload-arch and -no-offload-arch 
options, e.g. `--offload-arch=gfx906 --no-offload-arch=gfx906` and 
`--no-offload-arch=gfx906 --offload-arch=gfx906` is different. Also there is 
`--no-offload-arch=all` which removes all precedent --offload-arch= options.



Comment at: clang/lib/Driver/Driver.cpp:4107
+  Archs.insert(CudaArchToString(CudaArch::SM_35));
+else if (Kind == Action::OFK_Cuda)
+  Archs.insert(CudaArchToString(CudaArch::GFX803));

should this be HIP?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120272

___
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-03-03 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments.



Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:789
+  else
+OS << " ";
+  D->getSourceRange().print(OS,

aaron.ballman wrote:
> Should this be `" : "` instead?
Good catch



Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:796
+MV.ActiveASTContext->getSourceManager());
+} else if (const auto *T = Item.second.get()) {
+  OS << T->getTypeClassName() << "Type : ";

aaron.ballman wrote:
> Do we also need a match for `TypeLoc` matchers, or does the `else` cover that 
> sufficiently well?
> 
> (Actually, should we handle all of the various matchers at: 
> https://github.com/llvm/llvm-project/blob/main/clang/include/clang/ASTMatchers/ASTMatchers.h#L141
>  rather than leaving it to an `else`? Then the `else` can become an 
> unreachable so we know to update this interface?)
The else should be sufficient for most general cases, the only reason some are 
special cased is to improve the output, but I don't want there to be a burden 
to update this interface if new nodes are added.


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] D114425: [clang] Add __builtin_bswap128

2022-03-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:11071-11072
 
-QualType ElementType = DecodeTypeFromStr(Str, Context, Error,
- RequiresICE, false);
+QualType ElementType =
+DecodeTypeFromStr(Str, Context, Error, RequiresICE, false, true);
 assert(!RequiresICE && "Can't require vector ICE");

Why is this change needed? We don't seem to make a vector of __int128 as part 
of this patch, so I thought we wouldn't need the extra `AllowInt128` parameter 
to the function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114425

___
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-03-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:789
+  else
+OS << " ";
+  D->getSourceRange().print(OS,

Should this be `" : "` instead?



Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:796
+MV.ActiveASTContext->getSourceManager());
+} else if (const auto *T = Item.second.get()) {
+  OS << T->getTypeClassName() << "Type : ";

Do we also need a match for `TypeLoc` matchers, or does the `else` cover that 
sufficiently well?

(Actually, should we handle all of the various matchers at: 
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/ASTMatchers/ASTMatchers.h#L141
 rather than leaving it to an `else`? Then the `else` can become an unreachable 
so we know to update this interface?)


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] D118350: [Clang][Sema][AIX][PowerPC] Emit byval alignment warning only when struct is passed to a function

2022-03-03 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA updated this revision to Diff 412826.
ZarkoCA added a comment.

- Try to remove deeply nested ifs by using early returns
- Fix note so it refers to struct
- Add a struct that isn't called byval in test case to make it clearer that

warning isn't emitted on every declaration


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118350

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Analysis/padding_c.c
  clang/test/Analysis/padding_cpp.cpp
  clang/test/CXX/drs/dr6xx.cpp
  clang/test/Sema/aix-attr-align.c
  clang/test/SemaTemplate/instantiate-attr.cpp

Index: clang/test/SemaTemplate/instantiate-attr.cpp
===
--- clang/test/SemaTemplate/instantiate-attr.cpp
+++ clang/test/SemaTemplate/instantiate-attr.cpp
@@ -1,7 +1,4 @@
-// FIXME -Wno-aix-compat added temporarily while the diagnostic is being
-// refined.
-
-// RUN: %clang_cc1 -fsyntax-only -verify -Wno-aix-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify %s
 // expected-no-diagnostics
 template 
 struct A {
Index: clang/test/Sema/aix-attr-align.c
===
--- clang/test/Sema/aix-attr-align.c
+++ clang/test/Sema/aix-attr-align.c
@@ -5,18 +5,37 @@
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -verify=off -Wno-aix-compat -fsyntax-only %s
 // RUN: %clang_cc1 -triple powerpc64le-unknown-linux -verify=off -fsyntax-only %s
 
-struct S {
-  int a[8] __attribute__((aligned(8))); // no-warning
+// We do not warn on any declaration with a member aligned 16. Only when the struct is passed byval.
+struct R {
+  int b[8] __attribute__((aligned(16))); // no-warning
 };
 
-struct T {
-  int a[4] __attribute__((aligned(16))); // expected-warning {{requesting an alignment of 16 bytes or greater for struct members is not binary compatible with IBM XL C/C++ for AIX 16.1.0 and older}}
+struct S {
+  int a[8] __attribute__((aligned(8)));  // no-warning
+  int b[8] __attribute__((aligned(16))); // expected-warning {{alignment of 16 bytes for a struct member is not binary compatible with IBM XL C/C++ for AIX 16.1.0 or older}}
 };
 
-struct U {
-  int a[2] __attribute__((aligned(32))); // expected-warning {{requesting an alignment of 16 bytes or greater for struct members is not binary compatible with IBM XL C/C++ for AIX 16.1.0 and older}}
+struct T {
+  int a[8] __attribute__((aligned(8))); // no-warning
+  int b[8] __attribute__((aligned(4))); // no-warning
 };
 
 int a[8] __attribute__((aligned(8)));  // no-warning
 int b[4] __attribute__((aligned(16))); // no-warning
-int c[2] __attribute__((aligned(32))); // no-warning
+
+void baz(int a, int b, int *c, int d, int *e, int f, struct S);
+void jaz(int a, int b, int *c, int d, int *e, int f, struct T);
+void vararg_baz(int a,...);
+static void static_baz(int a, int b, int *c, int d, int *e, int f, struct S sp2) {
+  a = *sp2.b + *c + *e;
+}
+
+void foo(int p1, int p2, int p3, int p4, int p5, int p6, int p7, int p8,
+ struct S s, struct T t) {
+
+  baz(p1, p2, s.b, p3, b, p5, s);// expected-note {{passing byval argument 's' with potentially incompatible alignment here}}
+  jaz(p1, p2, a, p3, s.a, p5, t);// no-note
+  jaz(p1, p2, s.b, p3, b, p5, t);// no-note
+  vararg_baz(p1, p2, s.b, p3, b, p5, s); // no-note
+  static_baz(p1, p2, s.b, p3, b, p5, s); // no-note
+}
Index: clang/test/CXX/drs/dr6xx.cpp
===
--- clang/test/CXX/drs/dr6xx.cpp
+++ clang/test/CXX/drs/dr6xx.cpp
@@ -1,10 +1,8 @@
-// FIXME -Wno-aix-compat added temporarily while the diagnostic is being
-// refined.
-// RUN: %clang_cc1 -std=c++98 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking -Wno-aix-compat
-// RUN: %clang_cc1 -std=c++11 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking -Wno-aix-compat
-// RUN: %clang_cc1 -std=c++14 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking -Wno-aix-compat
-// RUN: %clang_cc1 -std=c++17 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking -Wno-aix-compat
-// RUN: %clang_cc1 -std=c++20 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking -Wno-aix-compat
+// RUN: %clang_cc1 -std=c++98 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking
+// RUN: %clang_cc1 -std=c++11 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking
+// RUN: %clang_cc1 -std=c++14 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking
+// RUN: %clang_cc1 -std=c++17 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking
+// RUN: %clang_cc1 -std=c++20 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors 

[PATCH] D120360: [libTooling] Generalize string explanation as templated metadata

2022-03-03 Thread Eric Li via Phabricator via cfe-commits
li.zhe.hua added inline comments.



Comment at: clang/include/clang/Tooling/Transformer/Transformer.h:103-108
+  template >>::value,
+int> = 0>

li.zhe.hua wrote:
> asoffer wrote:
> > Given that we're simply passing this off to the NoMetadataImpl which 
> > accepts a std::function, I don't see any reason to accept a generic 
> > parameter via sfinae. We could just accept the std::function and move it. 
> > Then the implicit conversion from whatever is passed in to std::function 
> > will do the heavy sfinae lifting.
> So, I ran into this during [[ https://reviews.llvm.org/D119745 | D119745 ]], 
> where one of the Windows CI builds doesn't have the fix for [[ 
> http://cplusplus.github.io/LWG/lwg-defects.html#2132 | DR 2132 ]] backported, 
> so overloading on different `std::function` doesn't work. (To be clear, this 
> using `std::function` and doing the whole overloading thing works fine in 
> clang locally.)
> 
> Thoughts?
Nevermind! I just tried and it works. I suspect this is because the 
non-template constructor and the template constructor are not ambiguous, as the 
non-template is tried first, and if it doesn't match, it goes to the template.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120360

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


[PATCH] D120360: [libTooling] Generalize string explanation as templated metadata

2022-03-03 Thread Eric Li via Phabricator via cfe-commits
li.zhe.hua updated this revision to Diff 412823.
li.zhe.hua marked 2 inline comments as done.
li.zhe.hua added a comment.

Update comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120360

Files:
  clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp
  clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
  clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
  clang/include/clang/Tooling/Transformer/RewriteRule.h
  clang/include/clang/Tooling/Transformer/Transformer.h
  clang/lib/Tooling/Transformer/RewriteRule.cpp
  clang/lib/Tooling/Transformer/Transformer.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -31,9 +31,11 @@
 using ::clang::transformer::member;
 using ::clang::transformer::name;
 using ::clang::transformer::node;
+using ::clang::transformer::noEdits;
 using ::clang::transformer::remove;
 using ::clang::transformer::rewriteDescendants;
 using ::clang::transformer::RewriteRule;
+using ::clang::transformer::RewriteRuleWith;
 using ::clang::transformer::statement;
 using ::testing::ElementsAre;
 using ::testing::IsEmpty;
@@ -129,7 +131,7 @@
 Changes.insert(Changes.end(), std::make_move_iterator(C->begin()),
std::make_move_iterator(C->end()));
   } else {
-// FIXME: stash this error rather then printing.
+// FIXME: stash this error rather than printing.
 llvm::errs() << "Error generating changes: "
  << llvm::toString(C.takeError()) << "\n";
 ++ErrorCount;
@@ -137,27 +139,58 @@
 };
   }
 
-  template 
-  void testRule(R Rule, StringRef Input, StringRef Expected) {
+  auto consumerWithStringMetadata() {
+return [this](Expected> C) {
+  if (C) {
+Changes.insert(Changes.end(),
+   std::make_move_iterator(C->Changes.begin()),
+   std::make_move_iterator(C->Changes.end()));
+StringMetadata.push_back(std::move(C->Metadata));
+  } else {
+// FIXME: stash this error rather than printing.
+llvm::errs() << "Error generating changes: "
+ << llvm::toString(C.takeError()) << "\n";
+++ErrorCount;
+  }
+};
+  }
+
+  void testRule(RewriteRule Rule, StringRef Input, StringRef Expected) {
 Transformers.push_back(
 std::make_unique(std::move(Rule), consumer()));
 Transformers.back()->registerMatchers();
 compareSnippets(Expected, rewrite(Input));
   }
 
-  template  void testRuleFailure(R Rule, StringRef Input) {
+  void testRule(RewriteRuleWith Rule, StringRef Input,
+StringRef Expected) {
+Transformers.push_back(std::make_unique(
+std::move(Rule), consumerWithStringMetadata()));
+Transformers.back()->registerMatchers();
+compareSnippets(Expected, rewrite(Input));
+  }
+
+  void testRuleFailure(RewriteRule Rule, StringRef Input) {
 Transformers.push_back(
 std::make_unique(std::move(Rule), consumer()));
 Transformers.back()->registerMatchers();
 ASSERT_FALSE(rewrite(Input)) << "Expected failure to rewrite code";
   }
 
+  void testRuleFailure(RewriteRuleWith Rule, StringRef Input) {
+Transformers.push_back(std::make_unique(
+std::move(Rule), consumerWithStringMetadata()));
+Transformers.back()->registerMatchers();
+ASSERT_FALSE(rewrite(Input)) << "Expected failure to rewrite code";
+  }
+
   // Transformers are referenced by MatchFinder.
   std::vector> Transformers;
   clang::ast_matchers::MatchFinder MatchFinder;
   // Records whether any errors occurred in individual changes.
   int ErrorCount = 0;
   AtomicChanges Changes;
+  std::vector StringMetadata;
 
 private:
   FileContentMappings FileContents = {{"header.h", ""}};
@@ -169,7 +202,7 @@
 };
 
 // Given string s, change strlen($s.c_str()) to REPLACED.
-static RewriteRule ruleStrlenSize() {
+static RewriteRuleWith ruleStrlenSize() {
   StringRef StringExpr = "strexpr";
   auto StringType = namedDecl(hasAnyName("::basic_string", "::string"));
   auto R = makeRule(
@@ -886,12 +919,12 @@
 
 TEST_F(TransformerTest, OrderedRuleUnrelated) {
   StringRef Flag = "flag";
-  RewriteRule FlagRule = makeRule(
+  RewriteRuleWith FlagRule = makeRule(
   cxxMemberCallExpr(on(expr(hasType(cxxRecordDecl(
 hasName("proto::ProtoCommandLineFlag"
.bind(Flag)),
 unless(callee(cxxMethodDecl(hasName("GetProto"),
-  

[PATCH] D120272: [CUDA] Add driver support for compiling CUDA with the new driver

2022-03-03 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 412821.
jhuber6 added a comment.

Adding test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120272

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/cuda-openmp-driver.cu

Index: clang/test/Driver/cuda-openmp-driver.cu
===
--- /dev/null
+++ clang/test/Driver/cuda-openmp-driver.cu
@@ -0,0 +1,16 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu -nocudalib -ccc-print-bindings -fgpu-rdc \
+// RUN:-foffload-new-driver --offload-arch=sm_35 --offload-arch=sm_70 %s 2>&1 \
+// RUN: | FileCheck -check-prefix CHECK %s
+
+// CHECK: "nvptx64-nvidia-cuda" - "clang", inputs: ["[[INPUT:.+]]"], output: "[[PTX_SM_35:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: ["[[PTX_SM_35]]"], output: "[[CUBIN_SM_35:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "NVPTX::Linker", inputs: ["[[CUBIN_SM_35]]", "[[PTX_SM_35]]"], output: "[[FATBIN_SM_35:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "clang", inputs: ["[[INPUT]]"], output: "[[PTX_SM_70:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: ["[[PTX_SM_70:.+]]"], output: "[[CUBIN_SM_70:.+]]"
+// CHECK: "nvptx64-nvidia-cuda" - "NVPTX::Linker", inputs: ["[[CUBIN_SM_70]]", "[[PTX_SM_70:.+]]"], output: "[[FATBIN_SM_70:.+]]"
+// CHECK: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[INPUT]]", "[[FATBIN_SM_35]]", "[[FATBIN_SM_70]]"], output: "[[HOST_OBJ:.+]]"
+// CHECK: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: ["[[HOST_OBJ]]"], output: "a.out"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4391,6 +4391,7 @@
   // one input.
   bool IsCuda = JA.isOffloading(Action::OFK_Cuda);
   bool IsCudaDevice = JA.isDeviceOffloading(Action::OFK_Cuda);
+  bool IsCudaHost = JA.isHostOffloading(Action::OFK_Cuda);
   bool IsHIP = JA.isOffloading(Action::OFK_HIP);
   bool IsHIPDevice = JA.isDeviceOffloading(Action::OFK_HIP);
   bool IsOpenMPDevice = JA.isDeviceOffloading(Action::OFK_OpenMP);
@@ -4414,6 +4415,7 @@
 
   InputInfoList ModuleHeaderInputs;
   InputInfoList OpenMPHostInputs;
+  InputInfoList CudaHostInputs;
   const InputInfo *CudaDeviceInput = nullptr;
   const InputInfo *OpenMPDeviceInput = nullptr;
   for (const InputInfo  : Inputs) {
@@ -4428,6 +4430,8 @@
 << types::getTypeName(Expected);
   }
   ModuleHeaderInputs.push_back(I);
+} else if (IsCudaHost && Args.hasArg(options::OPT_fopenmp_new_driver)) {
+  CudaHostInputs.push_back(I);
 } else if ((IsCuda || IsHIP) && !CudaDeviceInput) {
   CudaDeviceInput = 
 } else if (IsOpenMPDevice && !OpenMPDeviceInput) {
@@ -6957,6 +6961,7 @@
 auto OpenMPTCs = C.getOffloadToolChains();
 for (auto TI = OpenMPTCs.first, TE = OpenMPTCs.second; TI != TE;
  ++TI, ++InputFile) {
+  assert(InputFile->isFilename() && "Offloading requires a filename");
   const ToolChain *TC = TI->second;
   const ArgList  = C.getArgsForToolChain(TC, "", Action::OFK_OpenMP);
   StringRef File =
@@ -6969,6 +6974,21 @@
   TC->getTripleString() + "." +
   TCArgs.getLastArgValue(options::OPT_march_EQ) + "." + InputName));
 }
+  } else if (IsCudaHost && !CudaHostInputs.empty()) {
+const ToolChain *TC = C.getSingleOffloadToolChain();
+for (const auto  : CudaHostInputs) {
+  assert(InputFile.isFilename() && "Offloading requires a filename");
+  StringRef File =
+  C.getArgs().MakeArgString(TC->getInputFilename(InputFile));
+  StringRef InputName = Clang::getBaseInputStem(Args, Inputs);
+  // The CUDA toolchain should have a bound arch appended to the filename.
+  StringRef Arch = File.split(".").first.rsplit('-').second;
+  CmdArgs.push_back(Args.MakeArgString(
+  "-fembed-offload-object=" + File + "," +
+  Action::GetOffloadKindName(Action::OFK_Cuda) + "." +
+  TC->getTripleString() + "." +
+  Arch + "." + InputName));
+}
   }
 
   if (Triple.isAMDGPU()) {
@@ -8217,17 +8237,21 @@
   const Driver  = getToolChain().getDriver();
   const llvm::Triple TheTriple = getToolChain().getTriple();
   auto OpenMPTCRange = C.getOffloadToolChains();
+  auto CudaTCRange = C.getOffloadToolChains();
   ArgStringList CmdArgs;
 
   // Pass the CUDA path to the linker wrapper tool.
-  for (auto  : llvm::make_range(OpenMPTCRange.first, OpenMPTCRange.second)) {
-const ToolChain *TC = I.second;
-if (TC->getTriple().isNVPTX()) {
-  CudaInstallationDetector CudaInstallation(D, TheTriple, Args);
-  if 

[clang] d74a3a5 - Fixed sphinx build due to indentation

2022-03-03 Thread Aaron Ballman via cfe-commits

Author: Aaron Ballman
Date: 2022-03-03T16:03:05-05:00
New Revision: d74a3a514cf64731ecd21e1453aa78af79a565f2

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

LOG: Fixed sphinx build due to indentation

Added: 


Modified: 
clang/docs/analyzer/checkers.rst

Removed: 




diff  --git a/clang/docs/analyzer/checkers.rst 
b/clang/docs/analyzer/checkers.rst
index a9ebe063c6c8b..bc42b80d166c7 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -2650,6 +2650,7 @@ Limitations:
   
- Due to limitations of the memory modeling in the analyzer, one can likely
  observe a lot of false-positive reports like this:
+
   .. code-block:: c
   
 void false_positive() {



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


[PATCH] D120272: [CUDA] Add driver support for compiling CUDA with the new driver

2022-03-03 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 412816.
jhuber6 added a comment.

Adding comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120272

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp

Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4391,6 +4391,7 @@
   // one input.
   bool IsCuda = JA.isOffloading(Action::OFK_Cuda);
   bool IsCudaDevice = JA.isDeviceOffloading(Action::OFK_Cuda);
+  bool IsCudaHost = JA.isHostOffloading(Action::OFK_Cuda);
   bool IsHIP = JA.isOffloading(Action::OFK_HIP);
   bool IsHIPDevice = JA.isDeviceOffloading(Action::OFK_HIP);
   bool IsOpenMPDevice = JA.isDeviceOffloading(Action::OFK_OpenMP);
@@ -4414,6 +4415,7 @@
 
   InputInfoList ModuleHeaderInputs;
   InputInfoList OpenMPHostInputs;
+  InputInfoList CudaHostInputs;
   const InputInfo *CudaDeviceInput = nullptr;
   const InputInfo *OpenMPDeviceInput = nullptr;
   for (const InputInfo  : Inputs) {
@@ -4428,6 +4430,8 @@
 << types::getTypeName(Expected);
   }
   ModuleHeaderInputs.push_back(I);
+} else if (IsCudaHost && Args.hasArg(options::OPT_fopenmp_new_driver)) {
+  CudaHostInputs.push_back(I);
 } else if ((IsCuda || IsHIP) && !CudaDeviceInput) {
   CudaDeviceInput = 
 } else if (IsOpenMPDevice && !OpenMPDeviceInput) {
@@ -6957,6 +6961,7 @@
 auto OpenMPTCs = C.getOffloadToolChains();
 for (auto TI = OpenMPTCs.first, TE = OpenMPTCs.second; TI != TE;
  ++TI, ++InputFile) {
+  assert(InputFile->isFilename() && "Offloading requires a filename");
   const ToolChain *TC = TI->second;
   const ArgList  = C.getArgsForToolChain(TC, "", Action::OFK_OpenMP);
   StringRef File =
@@ -6969,6 +6974,21 @@
   TC->getTripleString() + "." +
   TCArgs.getLastArgValue(options::OPT_march_EQ) + "." + InputName));
 }
+  } else if (IsCudaHost && !CudaHostInputs.empty()) {
+const ToolChain *TC = C.getSingleOffloadToolChain();
+for (const auto  : CudaHostInputs) {
+  assert(InputFile.isFilename() && "Offloading requires a filename");
+  StringRef File =
+  C.getArgs().MakeArgString(TC->getInputFilename(InputFile));
+  StringRef InputName = Clang::getBaseInputStem(Args, Inputs);
+  // The CUDA toolchain should have a bound arch appended to the filename.
+  StringRef Arch = File.split(".").first.rsplit('-').second;
+  CmdArgs.push_back(Args.MakeArgString(
+  "-fembed-offload-object=" + File + "," +
+  Action::GetOffloadKindName(Action::OFK_Cuda) + "." +
+  TC->getTripleString() + "." +
+  Arch + "." + InputName));
+}
   }
 
   if (Triple.isAMDGPU()) {
@@ -8217,17 +8237,21 @@
   const Driver  = getToolChain().getDriver();
   const llvm::Triple TheTriple = getToolChain().getTriple();
   auto OpenMPTCRange = C.getOffloadToolChains();
+  auto CudaTCRange = C.getOffloadToolChains();
   ArgStringList CmdArgs;
 
   // Pass the CUDA path to the linker wrapper tool.
-  for (auto  : llvm::make_range(OpenMPTCRange.first, OpenMPTCRange.second)) {
-const ToolChain *TC = I.second;
-if (TC->getTriple().isNVPTX()) {
-  CudaInstallationDetector CudaInstallation(D, TheTriple, Args);
-  if (CudaInstallation.isValid())
-CmdArgs.push_back(Args.MakeArgString(
-"--cuda-path=" + CudaInstallation.getInstallPath()));
-  break;
+  for (Action::OffloadKind Kind : {Action::OFK_Cuda, Action::OFK_OpenMP}) {
+auto TCRange = C.getOffloadToolChains(Kind);
+for (auto  : llvm::make_range(TCRange.first, TCRange.second)) {
+  const ToolChain *TC = I.second;
+  if (TC->getTriple().isNVPTX()) {
+CudaInstallationDetector CudaInstallation(D, TheTriple, Args);
+if (CudaInstallation.isValid())
+  CmdArgs.push_back(Args.MakeArgString(
+  "--cuda-path=" + CudaInstallation.getInstallPath()));
+break;
+  }
 }
   }
 
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4045,6 +4045,72 @@
   Args.ClaimAllArgs(options::OPT_cuda_compile_host_device);
 }
 
+/// Returns the canonical name for the offloading architecture when using HIP or
+/// CUDA.
+static StringRef getCanonicalArchString(Compilation ,
+llvm::opt::DerivedArgList ,
+StringRef ArchStr,
+Action::OffloadKind Kind) {
+  if (Kind == Action::OFK_Cuda) {
+CudaArch Arch = StringToCudaArch(ArchStr);
+if (Arch == CudaArch::UNKNOWN || !IsNVIDIAGpuArch(Arch)) {
+  

[PATCH] D120936: [Sema][Windows] Don't special-case void* in __unaligned conversions.

2022-03-03 Thread Eli Friedman via Phabricator via cfe-commits
efriedma created this revision.
efriedma added reviewers: rnk, mstorsjo, andreybokhanko, majnemer, 
aaron.ballman.
Herald added a project: All.
efriedma requested review of this revision.
Herald added a project: clang.

As far as I can tell, MSVC allows the relevant conversions for all pointer 
types.  Found compiling a Windows SDK header.

I've verified that the updated errors in MicrosoftExtensions.cpp match the ones 
that MSVC actually emits, except for the one with a FIXME.  (Not sure why this 
wasn't done for the patch that added the tests.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120936

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCXX/MicrosoftExtensions.cpp


Index: clang/test/SemaCXX/MicrosoftExtensions.cpp
===
--- clang/test/SemaCXX/MicrosoftExtensions.cpp
+++ clang/test/SemaCXX/MicrosoftExtensions.cpp
@@ -85,18 +85,18 @@
   foo_unaligned(p2);
 
   __unaligned B_unaligned *p3 = 0;
-  int p4 = foo_unaligned(p3);
+  int p4 = foo_unaligned(p3); // expected-error {{cannot initialize a variable 
of type 'int' with an rvalue of type 'void *'}}
 
-  B_unaligned *p5 = p3; // expected-error {{cannot initialize a variable of 
type 'B_unaligned *' with an lvalue of type '__unaligned B_unaligned *'}}
+  B_unaligned *p5 = p3;
 
   __unaligned B_unaligned *p6 = p3;
 
   p1_aligned_type4 = p2_aligned_type4;
-  p2_aligned_type4 = p1_aligned_type4; // expected-error {{assigning to 'int 
aligned_type4::*' from incompatible type '__unaligned int aligned_type4::*'}}
+  p2_aligned_type4 = p1_aligned_type4;
   p3_aligned_type4 = p1_aligned_type4;
 
   __unaligned int a[10];
-  int *b = a; // expected-error {{cannot initialize a variable of type 'int *' 
with an lvalue of type '__unaligned int[10]'}}
+  int *b = a;
 }
 
 // Test from PR27367
@@ -115,13 +115,15 @@
 // We should accept type conversion of __unaligned to non-__unaligned 
references
 typedef struct in_addr {
 public:
-  in_addr(in_addr ) {} // expected-note {{candidate constructor not viable: 
no known conversion from '__unaligned IN_ADDR *' (aka '__unaligned in_addr *') 
to 'in_addr &' for 1st argument; dereference the argument with *}}
-  in_addr(in_addr *a) {} // expected-note {{candidate constructor not viable: 
1st argument ('__unaligned IN_ADDR *' (aka '__unaligned in_addr *')) would lose 
__unaligned qualifier}}
+  in_addr(in_addr ) {} // expected-note {{candidate constructor not viable: 
expects an lvalue for 1st argument}}
+  in_addr(in_addr *a) {} // expected-note {{candidate constructor not viable: 
no known conversion from 'IN_ADDR' (aka 'in_addr') to 'in_addr *' for 1st 
argument}}
 } IN_ADDR;
 
 void f(IN_ADDR __unaligned *a) {
   IN_ADDR local_addr = *a;
-  IN_ADDR local_addr2 = a; // expected-error {{no viable conversion from 
'__unaligned IN_ADDR *' (aka '__unaligned in_addr *') to 'IN_ADDR' (aka 
'in_addr')}}
+  // FIXME: MSVC accepts the following; not sure why clang tries to
+  // copy-construct an in_addr.
+  IN_ADDR local_addr2 = a; // expected-error {{no viable constructor copying 
variable of type 'IN_ADDR' (aka 'in_addr')}}
 }
 
 template void h1(T (__stdcall M::* const )()) { }
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -3195,9 +3195,8 @@
   Qualifiers FromQuals = FromType.getQualifiers();
   Qualifiers ToQuals = ToType.getQualifiers();
 
-  // Ignore __unaligned qualifier if this type is void.
-  if (ToType.getUnqualifiedType()->isVoidType())
-FromQuals.removeUnaligned();
+  // Ignore __unaligned qualifier.
+  FromQuals.removeUnaligned();
 
   // Objective-C ARC:
   //   Check Objective-C lifetime conversions.


Index: clang/test/SemaCXX/MicrosoftExtensions.cpp
===
--- clang/test/SemaCXX/MicrosoftExtensions.cpp
+++ clang/test/SemaCXX/MicrosoftExtensions.cpp
@@ -85,18 +85,18 @@
   foo_unaligned(p2);
 
   __unaligned B_unaligned *p3 = 0;
-  int p4 = foo_unaligned(p3);
+  int p4 = foo_unaligned(p3); // expected-error {{cannot initialize a variable of type 'int' with an rvalue of type 'void *'}}
 
-  B_unaligned *p5 = p3; // expected-error {{cannot initialize a variable of type 'B_unaligned *' with an lvalue of type '__unaligned B_unaligned *'}}
+  B_unaligned *p5 = p3;
 
   __unaligned B_unaligned *p6 = p3;
 
   p1_aligned_type4 = p2_aligned_type4;
-  p2_aligned_type4 = p1_aligned_type4; // expected-error {{assigning to 'int aligned_type4::*' from incompatible type '__unaligned int aligned_type4::*'}}
+  p2_aligned_type4 = p1_aligned_type4;
   p3_aligned_type4 = p1_aligned_type4;
 
   __unaligned int a[10];
-  int *b = a; // expected-error {{cannot initialize a variable of type 'int *' with an lvalue of type '__unaligned int[10]'}}
+  int *b = a;
 }
 
 // Test from PR27367
@@ -115,13 +115,15 @@
 // We should accept type 

[PATCH] D120272: [CUDA] Add driver support for compiling CUDA with the new driver

2022-03-03 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 412812.
jhuber6 added a comment.
Herald added a subscriber: dang.
Herald added a project: All.

Updating, embed fatbinaries now and small changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120272

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp

Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4391,6 +4391,7 @@
   // one input.
   bool IsCuda = JA.isOffloading(Action::OFK_Cuda);
   bool IsCudaDevice = JA.isDeviceOffloading(Action::OFK_Cuda);
+  bool IsCudaHost = JA.isHostOffloading(Action::OFK_Cuda);
   bool IsHIP = JA.isOffloading(Action::OFK_HIP);
   bool IsHIPDevice = JA.isDeviceOffloading(Action::OFK_HIP);
   bool IsOpenMPDevice = JA.isDeviceOffloading(Action::OFK_OpenMP);
@@ -4414,6 +4415,7 @@
 
   InputInfoList ModuleHeaderInputs;
   InputInfoList OpenMPHostInputs;
+  InputInfoList CudaHostInputs;
   const InputInfo *CudaDeviceInput = nullptr;
   const InputInfo *OpenMPDeviceInput = nullptr;
   for (const InputInfo  : Inputs) {
@@ -4428,6 +4430,8 @@
 << types::getTypeName(Expected);
   }
   ModuleHeaderInputs.push_back(I);
+} else if (IsCudaHost && Args.hasArg(options::OPT_fopenmp_new_driver)) {
+  CudaHostInputs.push_back(I);
 } else if ((IsCuda || IsHIP) && !CudaDeviceInput) {
   CudaDeviceInput = 
 } else if (IsOpenMPDevice && !OpenMPDeviceInput) {
@@ -6957,6 +6961,7 @@
 auto OpenMPTCs = C.getOffloadToolChains();
 for (auto TI = OpenMPTCs.first, TE = OpenMPTCs.second; TI != TE;
  ++TI, ++InputFile) {
+  assert(InputFile->isFilename() && "Offloading requires a filename");
   const ToolChain *TC = TI->second;
   const ArgList  = C.getArgsForToolChain(TC, "", Action::OFK_OpenMP);
   StringRef File =
@@ -6969,6 +6974,21 @@
   TC->getTripleString() + "." +
   TCArgs.getLastArgValue(options::OPT_march_EQ) + "." + InputName));
 }
+  } else if (IsCudaHost && !CudaHostInputs.empty()) {
+const ToolChain *TC = C.getSingleOffloadToolChain();
+for (const auto  : CudaHostInputs) {
+  assert(InputFile.isFilename() && "Offloading requires a filename");
+  StringRef File =
+  C.getArgs().MakeArgString(TC->getInputFilename(InputFile));
+  StringRef InputName = Clang::getBaseInputStem(Args, Inputs);
+  // The CUDA toolchain should have a bound arch appended to the filename.
+  StringRef Arch = File.split(".").first.rsplit('-').second;
+  CmdArgs.push_back(Args.MakeArgString(
+  "-fembed-offload-object=" + File + "," +
+  Action::GetOffloadKindName(Action::OFK_Cuda) + "." +
+  TC->getTripleString() + "." +
+  Arch + "." + InputName));
+}
   }
 
   if (Triple.isAMDGPU()) {
@@ -8217,17 +8237,21 @@
   const Driver  = getToolChain().getDriver();
   const llvm::Triple TheTriple = getToolChain().getTriple();
   auto OpenMPTCRange = C.getOffloadToolChains();
+  auto CudaTCRange = C.getOffloadToolChains();
   ArgStringList CmdArgs;
 
   // Pass the CUDA path to the linker wrapper tool.
-  for (auto  : llvm::make_range(OpenMPTCRange.first, OpenMPTCRange.second)) {
-const ToolChain *TC = I.second;
-if (TC->getTriple().isNVPTX()) {
-  CudaInstallationDetector CudaInstallation(D, TheTriple, Args);
-  if (CudaInstallation.isValid())
-CmdArgs.push_back(Args.MakeArgString(
-"--cuda-path=" + CudaInstallation.getInstallPath()));
-  break;
+  for (Action::OffloadKind Kind : {Action::OFK_Cuda, Action::OFK_OpenMP}) {
+auto TCRange = C.getOffloadToolChains(Kind);
+for (auto  : llvm::make_range(TCRange.first, TCRange.second)) {
+  const ToolChain *TC = I.second;
+  if (TC->getTriple().isNVPTX()) {
+CudaInstallationDetector CudaInstallation(D, TheTriple, Args);
+if (CudaInstallation.isValid())
+  CmdArgs.push_back(Args.MakeArgString(
+  "--cuda-path=" + CudaInstallation.getInstallPath()));
+break;
+  }
 }
   }
 
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4045,6 +4045,67 @@
   Args.ClaimAllArgs(options::OPT_cuda_compile_host_device);
 }
 
+static StringRef getCanonicalArchString(Compilation ,
+llvm::opt::DerivedArgList ,
+StringRef ArchStr,
+Action::OffloadKind Kind) {
+  if (Kind == Action::OFK_Cuda) {
+CudaArch Arch = StringToCudaArch(ArchStr);
+if (Arch == CudaArch::UNKNOWN || !IsNVIDIAGpuArch(Arch)) {
+  

[PATCH] D120934: [OpenMP][NFC] Refactor new driver to be more general

2022-03-03 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 created this revision.
jhuber6 added reviewers: jdoerfert, JonChesterfield.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
jhuber6 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

This path refactors the new driver to be less dependent on OpenMP. This
is done in preparation for the new driver to be able to handle other
offloading kinds and compile them together.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120934

Files:
  clang/include/clang/Driver/Compilation.h
  clang/lib/Driver/Driver.cpp

Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3830,11 +3830,6 @@
   // Builder to be used to build offloading actions.
   OffloadingActionBuilder OffloadBuilder(C, Args, Inputs);
 
-  // Offload kinds active for this compilation.
-  unsigned OffloadKinds = Action::OFK_None;
-  if (C.hasOffloadToolChain())
-OffloadKinds |= Action::OFK_OpenMP;
-
   // Construct the actions to perform.
   HeaderModulePrecompileJobAction *HeaderModuleAction = nullptr;
   ActionList LinkerInputs;
@@ -3935,7 +3930,7 @@
 if (!Args.hasArg(options::OPT_fopenmp_new_driver))
   OffloadBuilder.appendTopLevelActions(Actions, Current, InputArg);
 else if (Current)
-  Current->propagateHostOffloadInfo(OffloadKinds,
+  Current->propagateHostOffloadInfo(C.getActiveOffloadKinds(),
 /*BoundArch=*/nullptr);
   }
 
@@ -3956,9 +3951,9 @@
 if (ShouldEmitStaticLibrary(Args)) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
 } else if (Args.hasArg(options::OPT_fopenmp_new_driver) &&
-   OffloadKinds != Action::OFK_None) {
+   C.getActiveOffloadKinds() != Action::OFK_None) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
-  LA->propagateHostOffloadInfo(OffloadKinds,
+  LA->propagateHostOffloadInfo(C.getActiveOffloadKinds(),
/*BoundArch=*/nullptr);
 } else {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
@@ -4057,53 +4052,60 @@
   if (!isa(HostAction))
 return HostAction;
 
-  SmallVector ToolChains;
-  ActionList DeviceActions;
+  OffloadAction::DeviceDependences DDeps;
 
   types::ID InputType = Input.first;
   const Arg *InputArg = Input.second;
 
-  auto OpenMPTCRange = C.getOffloadToolChains();
-  for (auto TI = OpenMPTCRange.first, TE = OpenMPTCRange.second; TI != TE; ++TI)
-ToolChains.push_back(TI->second);
+  const Action::OffloadKind OffloadKinds[] = {Action::OFK_OpenMP};
 
-  for (unsigned I = 0; I < ToolChains.size(); ++I)
-DeviceActions.push_back(C.MakeAction(*InputArg, InputType));
+  for (Action::OffloadKind Kind : OffloadKinds) {
+SmallVector ToolChains;
+ActionList DeviceActions;
 
-  if (DeviceActions.empty())
-return HostAction;
+auto TCRange = C.getOffloadToolChains(Kind);
+for (auto TI = TCRange.first, TE = TCRange.second; TI != TE; ++TI)
+  ToolChains.push_back(TI->second);
 
-  auto PL = types::getCompilationPhases(*this, Args, InputType);
+if (ToolChains.empty())
+  continue;
 
-  for (phases::ID Phase : PL) {
-if (Phase == phases::Link) {
-  assert(Phase == PL.back() && "linking must be final compilation step.");
-  break;
-}
+for (unsigned I = 0; I < ToolChains.size(); ++I)
+  DeviceActions.push_back(C.MakeAction(*InputArg, InputType));
 
-auto TC = ToolChains.begin();
-for (Action * : DeviceActions) {
-  A = ConstructPhaseAction(C, Args, Phase, A, Action::OFK_OpenMP);
+if (DeviceActions.empty())
+  return HostAction;
 
-  if (isa(A)) {
-HostAction->setCannotBeCollapsedWithNextDependentAction();
-OffloadAction::HostDependence HDep(
-*HostAction, *C.getSingleOffloadToolChain(),
-/*BourdArch=*/nullptr, Action::OFK_OpenMP);
-OffloadAction::DeviceDependences DDep;
-DDep.add(*A, **TC, /*BoundArch=*/nullptr, Action::OFK_OpenMP);
-A = C.MakeAction(HDep, DDep);
+auto PL = types::getCompilationPhases(*this, Args, InputType);
+
+for (phases::ID Phase : PL) {
+  if (Phase == phases::Link) {
+assert(Phase == PL.back() && "linking must be final compilation step.");
+break;
   }
-  ++TC;
-}
-  }
 
-  OffloadAction::DeviceDependences DDeps;
+  auto TC = ToolChains.begin();
+  for (Action * : DeviceActions) {
+A = ConstructPhaseAction(C, Args, Phase, A, Kind);
+
+if (isa(A) && Kind == Action::OFK_OpenMP) {
+  HostAction->setCannotBeCollapsedWithNextDependentAction();
+  OffloadAction::HostDependence HDep(
+  *HostAction, *C.getSingleOffloadToolChain(),
+  /*BourdArch=*/nullptr, Action::OFK_OpenMP);
+  OffloadAction::DeviceDependences DDep;
+  DDep.add(*A, **TC, 

[PATCH] D120273: [OpenMP] Allow CUDA to be linked with OpenMP using the new driver

2022-03-03 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 412810.
jhuber6 added a comment.
Herald added a project: All.

Updating to use fatbinaries and fatbinary magic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120273

Files:
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp

Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -162,7 +162,10 @@
 };
 
 namespace llvm {
-/// Helper that allows DeviceFile to be used as a key in a DenseMap.
+/// Helper that allows DeviceFile to be used as a key in a DenseMap. For now we
+/// assume device files with matching architectures and triples but different
+/// offloading kinds should be handlded together, this may not be true in the
+/// future.
 template <> struct DenseMapInfo {
   static DeviceFile getEmptyKey() {
 return {DenseMapInfo::getEmptyKey(),
@@ -202,11 +205,15 @@
 }
 
 static StringRef getDeviceFileExtension(StringRef DeviceTriple,
-bool IsBitcode = false) {
+file_magic Magic) {
   Triple TheTriple(DeviceTriple);
-  if (TheTriple.isAMDGPU() || IsBitcode)
+  if (Magic == file_magic::bitcode)
 return "bc";
-  if (TheTriple.isNVPTX())
+  if (Magic == file_magic::cuda_fatbinary)
+return "fatbin";
+  if (Magic == file_magic::unknown)
+return "s";
+  if (TheTriple.isNVPTX() && Magic == file_magic::elf_relocatable)
 return "cubin";
   return "o";
 }
@@ -310,8 +317,8 @@
 
 if (Expected Contents = Sec.getContents()) {
   SmallString<128> TempFile;
-  StringRef DeviceExtension = getDeviceFileExtension(
-  DeviceTriple, identify_magic(*Contents) == file_magic::bitcode);
+  StringRef DeviceExtension =
+  getDeviceFileExtension(DeviceTriple, identify_magic(*Contents));
   if (Error Err = createOutputFile(Prefix + "-" + Kind + "-" +
DeviceTriple + "-" + Arch,
DeviceExtension, TempFile))
@@ -424,8 +431,8 @@
 
 StringRef Contents = CDS->getAsString();
 SmallString<128> TempFile;
-StringRef DeviceExtension = getDeviceFileExtension(
-DeviceTriple, identify_magic(Contents) == file_magic::bitcode);
+StringRef DeviceExtension =
+getDeviceFileExtension(DeviceTriple, identify_magic(Contents));
 if (Error Err = createOutputFile(Prefix + "-" + Kind + "-" + DeviceTriple +
  "-" + Arch,
  DeviceExtension, TempFile))
@@ -934,7 +941,22 @@
   return createFileError(File, EC);
 
 file_magic Type = identify_magic((*BufferOrErr)->getBuffer());
-if (Type != file_magic::bitcode) {
+switch (Type) {
+case file_magic::bitcode: {
+  Expected> InputFileOrErr =
+  llvm::lto::InputFile::create(**BufferOrErr);
+  if (!InputFileOrErr)
+return InputFileOrErr.takeError();
+
+  // Save the input file and the buffer associated with its memory.
+  BitcodeFiles.push_back(std::move(*InputFileOrErr));
+  SavedBuffers.push_back(std::move(*BufferOrErr));
+  continue;
+}
+case file_magic::elf_relocatable:
+case file_magic::elf_shared_object:
+case file_magic::macho_object:
+case file_magic::coff_object: {
   Expected> ObjFile =
   ObjectFile::createObjectFile(**BufferOrErr, Type);
   if (!ObjFile)
@@ -952,15 +974,10 @@
 else
   UsedInSharedLib.insert(Saver.save(*Name));
   }
-} else {
-  Expected> InputFileOrErr =
-  llvm::lto::InputFile::create(**BufferOrErr);
-  if (!InputFileOrErr)
-return InputFileOrErr.takeError();
-
-  // Save the input file and the buffer associated with its memory.
-  BitcodeFiles.push_back(std::move(*InputFileOrErr));
-  SavedBuffers.push_back(std::move(*BufferOrErr));
+  continue;
+}
+default:
+  continue;
 }
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D120931: [clang-format] fix namepsace format when the name is macro expansion

2022-03-03 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu created this revision.
zequanwu added reviewers: klimek, sammccall.
Herald added a project: All.
zequanwu requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Originally filed at crbug.com/1184570.
When the name of a namespace is a macro that takes arguments,

- It fixed the indentation.
- It tried to fix namespace end comment by making the name empty, because we

don't know the macro expansion. It seems like `FormatToken::MacroCtx` is always
None. Its assignment only happens in `MacroExpander` which is never used.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120931

Files:
  clang/lib/Format/NamespaceEndCommentsFixer.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -68,6 +68,27 @@
 "int i;\n"
 "int j;\n"
 "}"));
+  // FIXME: Expand macro for namepsace name.
+  EXPECT_EQ("#define M(x) x##x\n"
+"namespace M(x) {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace",
+fixNamespaceEndComments("#define M(x) x##x\n"
+"namespace M(x) {\n"
+"int i;\n"
+"int j;\n"
+"}"));
+  EXPECT_EQ("#define M(x) x##x\n"
+"namespace A::M(x) {\n"
+"int i;\n"
+"int j;\n"
+"}// namespace A::M",
+fixNamespaceEndComments("#define M(x) x##x\n"
+"namespace A::M(x) {\n"
+"int i;\n"
+"int j;\n"
+"}"));
   EXPECT_EQ("inline namespace A {\n"
 "int i;\n"
 "int j;\n"
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3738,6 +3738,36 @@
"void f() { f(); }\n"
"}",
LLVMWithNoNamespaceFix);
+  verifyFormat("#define M(x) x##x\n"
+   "namespace M(x) {\n"
+   "class A {};\n"
+   "void f() { f(); }\n"
+   "}",
+   LLVMWithNoNamespaceFix);
+  verifyFormat("#define M(x) x##x\n"
+   "namespace N::inline M(x) {\n"
+   "class A {};\n"
+   "void f() { f(); }\n"
+   "}",
+   LLVMWithNoNamespaceFix);
+  verifyFormat("#define M(x) x##x\n"
+   "namespace M(x)::inline N {\n"
+   "class A {};\n"
+   "void f() { f(); }\n"
+   "}",
+   LLVMWithNoNamespaceFix);
+  verifyFormat("#define M(x) x##x\n"
+   "namespace N::M(x) {\n"
+   "class A {};\n"
+   "void f() { f(); }\n"
+   "}",
+   LLVMWithNoNamespaceFix);
+  verifyFormat("#define M(x) x##x\n"
+   "namespace M::N(x) {\n"
+   "class A {};\n"
+   "void f() { f(); }\n"
+   "}",
+   LLVMWithNoNamespaceFix);
   verifyFormat("namespace N::inline D {\n"
"class A {};\n"
"void f() { f(); }\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2597,10 +2597,12 @@
 parseParens();
   } else {
 while (FormatTok->isOneOf(tok::identifier, tok::coloncolon, tok::kw_inline,
-  tok::l_square, tok::period) ||
+  tok::l_square, tok::period, tok::l_paren) ||
(Style.isCSharp() && FormatTok->is(tok::kw_union)))
   if (FormatTok->is(tok::l_square))
 parseSquare();
+  else if (FormatTok->is(tok::l_paren))
+parseParens();
   else
 nextToken();
   }
Index: clang/lib/Format/NamespaceEndCommentsFixer.cpp
===
--- clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -51,7 +51,9 @@
 }
 
 Tok = FirstNSTok;
-while (Tok && !Tok->is(tok::l_brace)) {
+while (Tok && !Tok->is(tok::l_brace) &&
+   (Tok->Tok.isAnyIdentifier() || Tok->is(tok::coloncolon) ||
+Tok->is(tok::kw_inline))) {
   name += Tok->TokenText;
   if (Tok->is(tok::kw_inline))
 name += " 

[PATCH] D120360: [libTooling] Generalize string explanation as templated metadata

2022-03-03 Thread Eric Li via Phabricator via cfe-commits
li.zhe.hua updated this revision to Diff 412799.
li.zhe.hua added a comment.

Try removing all the SFINAE


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120360

Files:
  clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp
  clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
  clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
  clang/include/clang/Tooling/Transformer/RewriteRule.h
  clang/include/clang/Tooling/Transformer/Transformer.h
  clang/lib/Tooling/Transformer/RewriteRule.cpp
  clang/lib/Tooling/Transformer/Transformer.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -31,9 +31,11 @@
 using ::clang::transformer::member;
 using ::clang::transformer::name;
 using ::clang::transformer::node;
+using ::clang::transformer::noEdits;
 using ::clang::transformer::remove;
 using ::clang::transformer::rewriteDescendants;
 using ::clang::transformer::RewriteRule;
+using ::clang::transformer::RewriteRuleWith;
 using ::clang::transformer::statement;
 using ::testing::ElementsAre;
 using ::testing::IsEmpty;
@@ -129,7 +131,7 @@
 Changes.insert(Changes.end(), std::make_move_iterator(C->begin()),
std::make_move_iterator(C->end()));
   } else {
-// FIXME: stash this error rather then printing.
+// FIXME: stash this error rather than printing.
 llvm::errs() << "Error generating changes: "
  << llvm::toString(C.takeError()) << "\n";
 ++ErrorCount;
@@ -137,27 +139,58 @@
 };
   }
 
-  template 
-  void testRule(R Rule, StringRef Input, StringRef Expected) {
+  auto consumerWithStringMetadata() {
+return [this](Expected> C) {
+  if (C) {
+Changes.insert(Changes.end(),
+   std::make_move_iterator(C->Changes.begin()),
+   std::make_move_iterator(C->Changes.end()));
+StringMetadata.push_back(std::move(C->Metadata));
+  } else {
+// FIXME: stash this error rather than printing.
+llvm::errs() << "Error generating changes: "
+ << llvm::toString(C.takeError()) << "\n";
+++ErrorCount;
+  }
+};
+  }
+
+  void testRule(RewriteRule Rule, StringRef Input, StringRef Expected) {
 Transformers.push_back(
 std::make_unique(std::move(Rule), consumer()));
 Transformers.back()->registerMatchers();
 compareSnippets(Expected, rewrite(Input));
   }
 
-  template  void testRuleFailure(R Rule, StringRef Input) {
+  void testRule(RewriteRuleWith Rule, StringRef Input,
+StringRef Expected) {
+Transformers.push_back(std::make_unique(
+std::move(Rule), consumerWithStringMetadata()));
+Transformers.back()->registerMatchers();
+compareSnippets(Expected, rewrite(Input));
+  }
+
+  void testRuleFailure(RewriteRule Rule, StringRef Input) {
 Transformers.push_back(
 std::make_unique(std::move(Rule), consumer()));
 Transformers.back()->registerMatchers();
 ASSERT_FALSE(rewrite(Input)) << "Expected failure to rewrite code";
   }
 
+  void testRuleFailure(RewriteRuleWith Rule, StringRef Input) {
+Transformers.push_back(std::make_unique(
+std::move(Rule), consumerWithStringMetadata()));
+Transformers.back()->registerMatchers();
+ASSERT_FALSE(rewrite(Input)) << "Expected failure to rewrite code";
+  }
+
   // Transformers are referenced by MatchFinder.
   std::vector> Transformers;
   clang::ast_matchers::MatchFinder MatchFinder;
   // Records whether any errors occurred in individual changes.
   int ErrorCount = 0;
   AtomicChanges Changes;
+  std::vector StringMetadata;
 
 private:
   FileContentMappings FileContents = {{"header.h", ""}};
@@ -169,7 +202,7 @@
 };
 
 // Given string s, change strlen($s.c_str()) to REPLACED.
-static RewriteRule ruleStrlenSize() {
+static RewriteRuleWith ruleStrlenSize() {
   StringRef StringExpr = "strexpr";
   auto StringType = namedDecl(hasAnyName("::basic_string", "::string"));
   auto R = makeRule(
@@ -886,12 +919,12 @@
 
 TEST_F(TransformerTest, OrderedRuleUnrelated) {
   StringRef Flag = "flag";
-  RewriteRule FlagRule = makeRule(
+  RewriteRuleWith FlagRule = makeRule(
   cxxMemberCallExpr(on(expr(hasType(cxxRecordDecl(
 hasName("proto::ProtoCommandLineFlag"
.bind(Flag)),
 unless(callee(cxxMethodDecl(hasName("GetProto"),
-  changeTo(node(std::string(Flag)), cat("PROTO")));
+  

[clang] 94623fb - [NFC] move CheckInstantiatedFunctionTemplateConstraints to SemaConcepts.cpp

2022-03-03 Thread Erich Keane via cfe-commits

Author: Erich Keane
Date: 2022-03-03T11:47:20-08:00
New Revision: 94623fb1c94841a3abc818c4d99a7154a9ed7df2

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

LOG: [NFC] move CheckInstantiatedFunctionTemplateConstraints to SemaConcepts.cpp

This is a Sema function that now no longer depends on any of the
functionality in SemaTemplateInstantiateDecl.cpp (as the static function
was moved to Sema in a previous NFC).  Moving it to SemaConcept means
that it and CheckFunctionConstraints can be changed to share more.

Added: 


Modified: 
clang/lib/Sema/SemaConcept.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index ce99d4848ccaa..449f9eb33f47b 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -409,6 +409,52 @@ bool Sema::EnsureTemplateArgumentListConstraints(
   return false;
 }
 
+bool Sema::CheckInstantiatedFunctionTemplateConstraints(
+SourceLocation PointOfInstantiation, FunctionDecl *Decl,
+ArrayRef TemplateArgs,
+ConstraintSatisfaction ) {
+  // In most cases we're not going to have constraints, so check for that 
first.
+  FunctionTemplateDecl *Template = Decl->getPrimaryTemplate();
+  // Note - code synthesis context for the constraints check is created
+  // inside CheckConstraintsSatisfaction.
+  SmallVector TemplateAC;
+  Template->getAssociatedConstraints(TemplateAC);
+  if (TemplateAC.empty()) {
+Satisfaction.IsSatisfied = true;
+return false;
+  }
+
+  // Enter the scope of this instantiation. We don't use
+  // PushDeclContext because we don't have a scope.
+  Sema::ContextRAII savedContext(*this, Decl);
+  LocalInstantiationScope Scope(*this);
+
+  // If this is not an explicit specialization - we need to get the 
instantiated
+  // version of the template arguments and add them to scope for the
+  // substitution.
+  if (Decl->isTemplateInstantiation()) {
+InstantiatingTemplate Inst(*this, Decl->getPointOfInstantiation(),
+InstantiatingTemplate::ConstraintsCheck{}, Decl->getPrimaryTemplate(),
+TemplateArgs, SourceRange());
+if (Inst.isInvalid())
+  return true;
+MultiLevelTemplateArgumentList MLTAL(
+*Decl->getTemplateSpecializationArgs());
+if (addInstantiatedParametersToScope(
+Decl, Decl->getPrimaryTemplate()->getTemplatedDecl(), Scope, 
MLTAL))
+  return true;
+  }
+  Qualifiers ThisQuals;
+  CXXRecordDecl *Record = nullptr;
+  if (auto *Method = dyn_cast(Decl)) {
+ThisQuals = Method->getMethodQualifiers();
+Record = Method->getParent();
+  }
+  CXXThisScopeRAII ThisScope(*this, Record, ThisQuals, Record != nullptr);
+  return CheckConstraintSatisfaction(Template, TemplateAC, TemplateArgs,
+ PointOfInstantiation, Satisfaction);
+}
+
 static void diagnoseUnsatisfiedRequirement(Sema ,
concepts::ExprRequirement *Req,
bool First) {

diff  --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp 
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 49fe11d3fa6df..aaee2d49c921c 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4569,52 +4569,6 @@ void Sema::InstantiateExceptionSpec(SourceLocation 
PointOfInstantiation,
  TemplateArgs);
 }
 
-bool Sema::CheckInstantiatedFunctionTemplateConstraints(
-SourceLocation PointOfInstantiation, FunctionDecl *Decl,
-ArrayRef TemplateArgs,
-ConstraintSatisfaction ) {
-  // In most cases we're not going to have constraints, so check for that 
first.
-  FunctionTemplateDecl *Template = Decl->getPrimaryTemplate();
-  // Note - code synthesis context for the constraints check is created
-  // inside CheckConstraintsSatisfaction.
-  SmallVector TemplateAC;
-  Template->getAssociatedConstraints(TemplateAC);
-  if (TemplateAC.empty()) {
-Satisfaction.IsSatisfied = true;
-return false;
-  }
-
-  // Enter the scope of this instantiation. We don't use
-  // PushDeclContext because we don't have a scope.
-  Sema::ContextRAII savedContext(*this, Decl);
-  LocalInstantiationScope Scope(*this);
-
-  // If this is not an explicit specialization - we need to get the 
instantiated
-  // version of the template arguments and add them to scope for the
-  // substitution.
-  if (Decl->isTemplateInstantiation()) {
-InstantiatingTemplate Inst(*this, Decl->getPointOfInstantiation(),
-InstantiatingTemplate::ConstraintsCheck{}, Decl->getPrimaryTemplate(),
-TemplateArgs, SourceRange());
-if (Inst.isInvalid())
-  return true;
-MultiLevelTemplateArgumentList MLTAL(
-

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

2022-03-03 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 412793.
njames93 added a comment.

Update test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120185

Files:
  clang-tools-extra/docs/ReleaseNotes.rst
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
@@ -12,6 +12,7 @@
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Config/config.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gtest/gtest.h"
@@ -34,6 +35,55 @@
   EXPECT_TRUE(notMatches("class X {};", HasEmptyName));
 }, "");
 }
+
+#if ENABLE_BACKTRACES
+TEST(MatcherCrashDeathTest, CrashOnCallbackDump) {
+  llvm::EnablePrettyStackTrace();
+  MatchFinder Finder;
+
+  struct CrashCallback : public MatchFinder::MatchCallback {
+void run(const MatchFinder::MatchResult ) override {
+  LLVM_BUILTIN_TRAP;
+}
+llvm::Optional getCheckTraversalKind() const override {
+  return TK_IgnoreUnlessSpelledInSource;
+}
+StringRef getID() const override { return "CrashTester"; }
+  } Callback;
+
+  Finder.addMatcher(
+  forStmt(hasLoopInit(declStmt(hasSingleDecl(
+   varDecl(hasType(qualType().bind("QT")),
+   hasType(type().bind("T")),
+   hasInitializer(
+   integerLiteral().bind("IL")))
+   .bind("VD")))
+  .bind("DS")))
+  .bind("FS"),
+  );
+
+  auto Matcher = testing::AllOf(
+  testing::ContainsRegex(
+  "ASTMatcher: Processing 'CrashTester'\n--- Bound Nodes Begin ---"),
+  testing::ContainsRegex("FS - \\{ ForStmt :  \\}"),
+  testing::ContainsRegex("DS - \\{ DeclStmt :  \\}"),
+  testing::ContainsRegex("IL - \\{ IntegerLiteral :  \\}"),
+  testing::ContainsRegex("QT - \\{ QualType : int \\}"),
+  testing::ContainsRegex("T - \\{ BuiltinType : int \\}"),
+  testing::ContainsRegex(
+  "VD - \\{ VarDecl I :  \\}"),
+  testing::ContainsRegex("--- Bound Nodes End ---"));
+
+  ASSERT_DEATH(
+  tooling::runToolOnCode(newFrontendActionFactory()->create(), R"cpp(
+  void foo() {
+for (int I = 0; I < 5; ++I) {
+}
+  }
+  )cpp"),
+  Matcher);
+}
+#endif // ENABLE_BACKTRACES
 #endif
 
 TEST(ConstructVariadic, MismatchedTypes_Regression) {
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Timer.h"
 #include 
 #include 
@@ -760,11 +761,66 @@
 D);
   }
 
+  class TraceReporter : llvm::PrettyStackTraceEntry {
+  public:
+TraceReporter(const MatchASTVisitor ) : MV(MV) {}
+void print(raw_ostream ) const override {
+  if (!MV.CurMatched) {
+OS << "ASTMatcher: Not currently matching\n";
+return;
+  }
+  assert(MV.ActiveASTContext &&
+ "ActiveASTContext should be set if there is a matched callback");
+
+  OS << "ASTMatcher: Processing '" << MV.CurMatched->getID() << "'\n";
+  const BoundNodes::IDToNodeMap  = MV.CurBoundNodes->getMap();
+  if (Map.empty()) {
+OS << "No bound nodes\n";
+return;
+  }
+  OS << "--- Bound Nodes Begin ---\n";
+  for (const auto  : Map) {
+OS << "" << Item.first << " - { ";
+if (const auto *D = Item.second.get()) {
+  OS << D->getDeclKindName() << "Decl ";
+  if (const auto *ND = dyn_cast(D))
+OS << ND->getDeclName() << " : ";
+  else
+OS << " ";
+  D->getSourceRange().print(OS,
+MV.ActiveASTContext->getSourceManager());
+} else if (const auto *S = Item.second.get()) {
+  OS << S->getStmtClassName() << " : ";
+  S->getSourceRange().print(OS,
+MV.ActiveASTContext->getSourceManager());
+} else if (const auto *T = Item.second.get()) {
+  OS << T->getTypeClassName() << "Type : ";
+  QualType(T, 0).print(OS, MV.ActiveASTContext->getPrintingPolicy());
+} else if (const auto *QT = Item.second.get()) {
+  OS << "QualType : ";
+  QT->print(OS, 

[PATCH] D120884: [format] Use int8_t as the underlying type of all enums in FormatStyle

2022-03-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment.

In D120884#3356751 , @kwk wrote:

> In D120884#3356746 , 
> @HazardyKnusperkeks wrote:
>
>> Please use the clang-format tag.
>
> Yes, sorry that I forgot it again. And thank you for approving. Shall I wait 
> for the others to approve this or land the patch?

I myself give patches always some time. Others commit nearly as soon as 
possible.
I would say you can land it, but maybe wait a day, to see if someone might 
object.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120884

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


[PATCH] D120360: [libTooling] Generalize string explanation as templated metadata

2022-03-03 Thread Eric Li via Phabricator via cfe-commits
li.zhe.hua added inline comments.



Comment at: clang/include/clang/Tooling/Transformer/Transformer.h:103-108
+  template >>::value,
+int> = 0>

asoffer wrote:
> Given that we're simply passing this off to the NoMetadataImpl which accepts 
> a std::function, I don't see any reason to accept a generic parameter via 
> sfinae. We could just accept the std::function and move it. Then the implicit 
> conversion from whatever is passed in to std::function will do the heavy 
> sfinae lifting.
So, I ran into this during [[ https://reviews.llvm.org/D119745 | D119745 ]], 
where one of the Windows CI builds doesn't have the fix for [[ 
http://cplusplus.github.io/LWG/lwg-defects.html#2132 | DR 2132 ]] backported, 
so overloading on different `std::function` doesn't work. (To be clear, this 
using `std::function` and doing the whole overloading thing works fine in clang 
locally.)

Thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120360

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


[PATCH] D120489: [analyzer] Done some changes to detect Uninitialized read by the char array manipulation functions

2022-03-03 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added a comment.

Looks like it also broke lldb green dragon incremental as well: 
https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/41865/consoleFull#-27141295349ba4694-19c4-4d7e-bec5-911270d8a58c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120489

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


[PATCH] D120360: [libTooling] Generalize string explanation as templated metadata

2022-03-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel accepted this revision.
ymandel added inline comments.



Comment at: clang/include/clang/Tooling/Transformer/Transformer.h:116
+  explicit Transformer(transformer::RewriteRuleWith Rule,
+   ConsumerFn Consumer);
 

asoffer wrote:
> li.zhe.hua wrote:
> > ymandel wrote:
> > > why won't we have the same SFINAE here to ensure ConsumerFn is invocable?
> > I can't figure out how to implement the SFINAE without instantiating 
> > `Result` (which is invalid because `T Metadata` is illegal then). My 
> > current attempt is
> > 
> > ```
> >   template <
> >   typename T, typename ConsumerFn,
> >   std::enable_if_t<
> >   std::conjunction<
> >   std::negation>,
> >   llvm::is_invocable > llvm::Expected>>>::value,
> >   int> = 0>
> >   explicit Transformer(transformer::RewriteRuleWith Rule,
> >ConsumerFn Consumer);
> > ```
> > 
> > I suppose I could provide a specialization of `Result` that is valid? 
> > I guess I'll go with that, and just document why it exists?
> Accepting the std:function directly should simplify this and avoid the need 
> for the Result specialization.
Seems fine, but I'll admit to being out of my depth. Seemed nice to have, but 
not necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120360

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


[PATCH] D120360: [libTooling] Generalize string explanation as templated metadata

2022-03-03 Thread Andy Soffer via Phabricator via cfe-commits
asoffer added inline comments.



Comment at: clang/include/clang/Tooling/Transformer/Transformer.h:103-108
+  template >>::value,
+int> = 0>

Given that we're simply passing this off to the NoMetadataImpl which accepts a 
std::function, I don't see any reason to accept a generic parameter via sfinae. 
We could just accept the std::function and move it. Then the implicit 
conversion from whatever is passed in to std::function will do the heavy sfinae 
lifting.



Comment at: clang/include/clang/Tooling/Transformer/Transformer.h:116
+  explicit Transformer(transformer::RewriteRuleWith Rule,
+   ConsumerFn Consumer);
 

li.zhe.hua wrote:
> ymandel wrote:
> > why won't we have the same SFINAE here to ensure ConsumerFn is invocable?
> I can't figure out how to implement the SFINAE without instantiating 
> `Result` (which is invalid because `T Metadata` is illegal then). My 
> current attempt is
> 
> ```
>   template <
>   typename T, typename ConsumerFn,
>   std::enable_if_t<
>   std::conjunction<
>   std::negation>,
>   llvm::is_invocable llvm::Expected>>>::value,
>   int> = 0>
>   explicit Transformer(transformer::RewriteRuleWith Rule,
>ConsumerFn Consumer);
> ```
> 
> I suppose I could provide a specialization of `Result` that is valid? I 
> guess I'll go with that, and just document why it exists?
Accepting the std:function directly should simplify this and avoid the need for 
the Result specialization.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120360

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


[PATCH] D120489: [analyzer] Done some changes to detect Uninitialized read by the char array manipulation functions

2022-03-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment.

Can you rollback until a fix is found?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120489

___
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-03-03 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 412772.
njames93 added a comment.
Herald added a project: All.

Moved tests into ASTMatchersInternal, makes sense in here and removes the dirty 
clang-tidy plugin hack.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120185

Files:
  clang-tools-extra/docs/ReleaseNotes.rst
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
@@ -12,6 +12,7 @@
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Config/config.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gtest/gtest.h"
@@ -34,6 +35,55 @@
   EXPECT_TRUE(notMatches("class X {};", HasEmptyName));
 }, "");
 }
+
+#if ENABLE_BACKTRACES
+TEST(MatcherCrashDeathTest, CrashOnCallbackDump) {
+  llvm::EnablePrettyStackTrace();
+  MatchFinder Finder;
+
+  struct CrashCallback : public MatchFinder::MatchCallback {
+void run(const MatchFinder::MatchResult ) override {
+  LLVM_BUILTIN_TRAP;
+}
+llvm::Optional getCheckTraversalKind() const override {
+  return TK_IgnoreUnlessSpelledInSource;
+}
+StringRef getID() const override { return "CrashTester"; }
+  } Callback;
+
+  Finder.addMatcher(
+  forStmt(hasLoopInit(declStmt(hasSingleDecl(
+   varDecl(hasType(qualType().bind("QT")),
+   hasType(type().bind("T")),
+   hasInitializer(
+   integerLiteral().bind("IL")))
+   .bind("VD")))
+  .bind("DS")))
+  .bind("FS"),
+  );
+
+  auto Matcher = testing::AllOf(
+  testing::ContainsRegex("ASTMatcher: Processing 'CrashTester'"),
+  testing::ContainsRegex("--- Bound Nodes Begin ---"),
+  testing::ContainsRegex("FS - \\{ ForStmt :  \\}"),
+  testing::ContainsRegex("DS - \\{ DeclStmt :  \\}"),
+  testing::ContainsRegex("IL - \\{ IntegerLiteral :  \\}"),
+  testing::ContainsRegex("QT - \\{ QualType : int \\}"),
+  testing::ContainsRegex("T - \\{ BuiltinType : int \\}"),
+  testing::ContainsRegex(
+  "VD - \\{ VarDecl I :  \\}"),
+  testing::ContainsRegex("--- Bound Nodes End ---"));
+
+  ASSERT_DEATH(
+  tooling::runToolOnCode(newFrontendActionFactory()->create(), R"cpp(
+  void foo() {
+for (int I = 0; I < 5; ++I) {
+}
+  }
+  )cpp"),
+  Matcher);
+}
+#endif // ENABLE_BACKTRACES
 #endif
 
 TEST(ConstructVariadic, MismatchedTypes_Regression) {
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Timer.h"
 #include 
 #include 
@@ -760,11 +761,66 @@
 D);
   }
 
+  class TraceReporter : llvm::PrettyStackTraceEntry {
+  public:
+TraceReporter(const MatchASTVisitor ) : MV(MV) {}
+void print(raw_ostream ) const override {
+  if (!MV.CurMatched) {
+OS << "ASTMatcher: Not currently matching\n";
+return;
+  }
+  assert(MV.ActiveASTContext &&
+ "ActiveASTContext should be set if there is a matched callback");
+
+  OS << "ASTMatcher: Processing '" << MV.CurMatched->getID() << "'\n";
+  const BoundNodes::IDToNodeMap  = MV.CurBoundNodes->getMap();
+  if (Map.empty()) {
+OS << "No bound nodes\n";
+return;
+  }
+  OS << "--- Bound Nodes Begin ---\n";
+  for (const auto  : Map) {
+OS << "" << Item.first << " - { ";
+if (const auto *D = Item.second.get()) {
+  OS << D->getDeclKindName() << "Decl ";
+  if (const auto *ND = dyn_cast(D))
+OS << ND->getDeclName() << " : ";
+  else
+OS << " ";
+  D->getSourceRange().print(OS,
+MV.ActiveASTContext->getSourceManager());
+} else if (const auto *S = Item.second.get()) {
+  OS << S->getStmtClassName() << " : ";
+  S->getSourceRange().print(OS,
+MV.ActiveASTContext->getSourceManager());
+} else if (const auto *T = Item.second.get()) {
+  OS << T->getTypeClassName() << "Type : ";
+  QualType(T, 0).print(OS, MV.ActiveASTContext->getPrintingPolicy());
+   

[clang] 56eaf86 - [analyzer] Done some changes to detect Uninitialized read by the char array manipulation functions

2022-03-03 Thread via cfe-commits

Author: Shivam
Date: 2022-03-04T00:21:06+05:30
New Revision: 56eaf869be27585bff7320505dfad32b5b3b6189

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

LOG: [analyzer] Done some changes to detect Uninitialized read by the char 
array manipulation functions

Few weeks back I was experimenting with reading the uninitialized values from 
src , which is actually a bug but the CSA seems to give up at that point . I 
was curious about that and I pinged @steakhal on the discord and according to 
him this seems to be a genuine issue and needs to be fix. So I goes with fixing 
this bug and thanks to @steakhal who help me creating this patch. This feature 
seems to break some tests but this was the genuine problem and the broken tests 
also needs to fix in certain manner. I add a test but yeah we need more 
tests,I'll try to add more tests.Thanks

Reviewed By: steakhal, NoQ

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 54c9e887b8c8c..23c10431a5dfb 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -259,6 +259,8 @@ class CStringChecker : public Checker< eval::Call,
   void emitNotCStringBug(CheckerContext , ProgramStateRef State,
  const Stmt *S, StringRef WarningMsg) const;
   void emitAdditionOverflowBug(CheckerContext , ProgramStateRef State) const;
+  void emitUninitializedReadBug(CheckerContext , ProgramStateRef State,
+ const Expr *E) const;
   ProgramStateRef checkAdditionOverflow(CheckerContext ,
 ProgramStateRef state,
 NonLoc left,



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


[PATCH] D120489: [analyzer] Done some changes to detect Uninitialized read by the char array manipulation functions

2022-03-03 Thread Douglas Yung via Phabricator via cfe-commits
dyung added a comment.

And the PS4 build bots:

https://lab.llvm.org/buildbot/#/builders/139/builds/18001
https://lab.llvm.org/buildbot/#/builders/216/builds/801


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120489

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


[PATCH] D120900: [clang][dataflow] Add `MatchSwitch` utility library.

2022-03-03 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun accepted this revision.
xazax.hun added inline comments.
This revision is now accepted and ready to land.
Herald added a subscriber: rnkovacs.



Comment at: clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h:51
+template 
+using MatchSwitch = std::function;
+

ymandel wrote:
> xazax.hun wrote:
> > When we instantiate this with `TransferState` we have `ASTContext` both as 
> > an argument and as a member of `State`. Is this intentional?
> Yes, but...
> The `ASTContext` is needed for the match itself, but the `State` type is not 
> guaranteed to be `TransferState`, so won't necessarily hold the context.
> 
> But, we could reorganize a bit. Either: 
> a) pass MatchFinder::MatchResult, instead of BoundNodes. That would bundle 
> the nodes, context and source manager, which seems like a good idea.
> b) bake TransferState into MatchSwitch, and make the parameter genericy named 
> rather than "lattice".
> 
> I'm inclined towards the first option, since it seems "right" to give full 
> access to the `MatchResult`. WDYT?
Option a) sounds good to me, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120900

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


[PATCH] D120900: [clang][dataflow] Add `MatchSwitch` utility library.

2022-03-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 412763.
ymandel marked an inline comment as done.
ymandel added a comment.

moved to `MatchFinder::MatchResult`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120900

Files:
  clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
  clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
  clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
===
--- /dev/null
+++ clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
@@ -0,0 +1,204 @@
+//===- unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+//  This file defines a simplistic version of Constant Propagation as an example
+//  of a forward, monotonic dataflow analysis. The analysis tracks all
+//  variables in the scope, but lacks escape analysis.
+//
+//===--===//
+
+#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
+#include "TestingSupport.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MapLattice.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Testing/Support/Annotations.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace clang;
+using namespace dataflow;
+
+namespace {
+using ::testing::Pair;
+using ::testing::UnorderedElementsAre;
+
+class BooleanLattice {
+public:
+  BooleanLattice() : Value(false) {}
+  explicit BooleanLattice(bool B) : Value(B) {}
+
+  static BooleanLattice bottom() { return BooleanLattice(false); }
+
+  static BooleanLattice top() { return BooleanLattice(true); }
+
+  LatticeJoinEffect join(BooleanLattice Other) {
+auto Prev = Value;
+Value = Value || Other.Value;
+return Prev == Value ? LatticeJoinEffect::Unchanged
+ : LatticeJoinEffect::Changed;
+  }
+
+  friend bool operator==(BooleanLattice LHS, BooleanLattice RHS) {
+return LHS.Value == RHS.Value;
+  }
+
+  friend std::ostream <<(std::ostream , const BooleanLattice ) {
+Os << B.Value;
+return Os;
+  }
+
+  bool value() const { return Value; }
+
+private:
+  bool Value;
+};
+} // namespace
+
+MATCHER_P(Holds, m,
+  ((negation ? "doesn't hold" : "holds") +
+   llvm::StringRef(" a lattice element that ") +
+   ::testing::DescribeMatcher(m, negation))
+  .str()) {
+  return ExplainMatchResult(m, arg.Lattice, result_listener);
+}
+
+void TransferSetTrue(const DeclRefExpr *,
+ TransferState ) {
+  State.Lattice = BooleanLattice(true);
+}
+
+void TransferSetFalse(const Stmt *,
+  const ast_matchers::MatchFinder::MatchResult &,
+  TransferState ) {
+  State.Lattice = BooleanLattice(false);
+}
+
+class TestAnalysis : public DataflowAnalysis {
+  MatchSwitch> TransferSwitch;
+
+public:
+  explicit TestAnalysis(ASTContext )
+  : DataflowAnalysis(Context) {
+using namespace ast_matchers;
+TransferSwitch =
+MatchSwitchBuilder>()
+.CaseOf(declRefExpr(to(varDecl(hasName("X", TransferSetTrue)
+.CaseOf(callExpr(callee(functionDecl(hasName("Foo",
+TransferSetFalse)
+.Build();
+  }
+
+  static BooleanLattice initialElement() { return BooleanLattice::bottom(); }
+
+  void transfer(const Stmt *S, BooleanLattice , Environment ) {
+TransferState State(L, Env);
+TransferSwitch(*S, getASTContext(), State);
+  }
+};
+
+class MatchSwitchTest : public ::testing::Test {
+protected:
+  template 
+  void RunDataflow(llvm::StringRef Code, Matcher Expectations) {
+ASSERT_THAT_ERROR(
+test::checkDataflow(
+Code, "fun",
+[](ASTContext , Environment &) { return TestAnalysis(C); },
+[](
+llvm::ArrayRef>>
+Results,
+ASTContext &) { EXPECT_THAT(Results, Expectations); },
+  

[PATCH] D120489: [analyzer] Done some changes to detect Uninitialized read by the char array manipulation functions

2022-03-03 Thread Ron Lieberman via Phabricator via cfe-commits
ronlieb added a comment.

broke our hip buildbot as well.

https://lab.llvm.org/buildbot/#/builders/165/builds/17076


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120489

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


[PATCH] D120907: [docs] Add PowerPC release notes for LLVM 14

2022-03-03 Thread Lei Huang via Phabricator via cfe-commits
lei closed this revision.
lei added a comment.

Commited to release/14.x


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120907

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


[PATCH] D120489: [analyzer] Done some changes to detect Uninitialized read by the char array manipulation functions

2022-03-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment.

Looks like this broke the build. I'm getting:

  FAILED: 
tools/clang/lib/StaticAnalyzer/Checkers/CMakeFiles/obj.clangStaticAnalyzerCheckers.dir/CStringChecker.cpp.o
  /usr/bin/clang++-11 ... -std=c++14 -MD -MT 
tools/clang/lib/StaticAnalyzer/Checkers/CMakeFiles/obj.clangStaticAnalyzerCheckers.dir/CStringChecker.cpp.o
 -MF 
tools/clang/lib/StaticAnalyzer/Checkers/CMakeFiles/obj.clangStaticAnalyzerCheckers.dir/CStringChecker.cpp.o.d
 -o 
tools/clang/lib/StaticAnalyzer/Checkers/CMakeFiles/obj.clangStaticAnalyzerCheckers.dir/CStringChecker.cpp.o
 -c 
/usr/local/google/home/yitzhakm/remote-build/llvm-git/llvm-project/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  
/usr/local/google/home/yitzhakm/remote-build/llvm-git/llvm-project/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:376:7:
 error: use of undeclared identifier 'emitUninitializedReadBug'
emitUninitializedReadBug(C, StInBound, Buffer.Expression);
^
  
/usr/local/google/home/yitzhakm/remote-build/llvm-git/llvm-project/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:592:22:
 error: out-of-line definition of 'emitUninitializedReadBug' does not match any 
declaration in '(anonymous namespace)::CStringChecker'
  void CStringChecker::emitUninitializedReadBug(CheckerContext ,
   ^~~~
  2 errors generated.FAILED: 
tools/clang/lib/StaticAnalyzer/Checkers/CMakeFiles/obj.clangStaticAnalyzerCheckers.dir/CStringChecker.cpp.o
  /usr/bin/clang++-11 -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE 
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
-I/usr/local/google/home/yitzhakm/remote-build/llvm-git/build/tools/clang/lib/StaticAnalyzer/Checkers
 
-I/usr/local/google/home/yitzhakm/remote-build/llvm-git/llvm-project/clang/lib/StaticAnalyzer/Checkers
 
-I/usr/local/google/home/yitzhakm/remote-build/llvm-git/llvm-project/clang/include
 
-I/usr/local/google/home/yitzhakm/remote-build/llvm-git/build/tools/clang/include
 -I/usr/local/google/home/yitzhakm/remote-build/llvm-git/build/include 
-I/usr/local/google/home/yitzhakm/remote-build/llvm-git/llvm-project/llvm/include
 -gmlt -Wall -fPIC -fvisibility-inlines-hidden -Werror=date-time 
-Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
-Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
-Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough 
-Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
-Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion 
-Wmisleading-indentation -fdiagnostics-color -ffunction-sections 
-fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 
-DNDEBUG  -fno-exceptions -fno-rtti -UNDEBUG -std=c++14 -MD -MT 
tools/clang/lib/StaticAnalyzer/Checkers/CMakeFiles/obj.clangStaticAnalyzerCheckers.dir/CStringChecker.cpp.o
 -MF 
tools/clang/lib/StaticAnalyzer/Checkers/CMakeFiles/obj.clangStaticAnalyzerCheckers.dir/CStringChecker.cpp.o.d
 -o 
tools/clang/lib/StaticAnalyzer/Checkers/CMakeFiles/obj.clangStaticAnalyzerCheckers.dir/CStringChecker.cpp.o
 -c 
/usr/local/google/home/yitzhakm/remote-build/llvm-git/llvm-project/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  
/usr/local/google/home/yitzhakm/remote-build/llvm-git/llvm-project/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:376:7:
 error: use of undeclared identifier 'emitUninitializedReadBug'
emitUninitializedReadBug(C, StInBound, Buffer.Expression);
^
  
/usr/local/google/home/yitzhakm/remote-build/llvm-git/llvm-project/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:592:22:
 error: out-of-line definition of 'emitUninitializedReadBug' does not match any 
declaration in '(anonymous namespace)::CStringChecker'
  void CStringChecker::emitUninitializedReadBug(CheckerContext ,
   ^~~~
  2 errors generated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120489

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


[PATCH] D120360: [libTooling] Generalize string explanation as Any metadata

2022-03-03 Thread Eric Li via Phabricator via cfe-commits
li.zhe.hua updated this revision to Diff 412755.
li.zhe.hua added a comment.

Fix use-after-move in asserts


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120360

Files:
  clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp
  clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
  clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
  clang/include/clang/Tooling/Transformer/RewriteRule.h
  clang/include/clang/Tooling/Transformer/Transformer.h
  clang/lib/Tooling/Transformer/RewriteRule.cpp
  clang/lib/Tooling/Transformer/Transformer.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -31,9 +31,11 @@
 using ::clang::transformer::member;
 using ::clang::transformer::name;
 using ::clang::transformer::node;
+using ::clang::transformer::noEdits;
 using ::clang::transformer::remove;
 using ::clang::transformer::rewriteDescendants;
 using ::clang::transformer::RewriteRule;
+using ::clang::transformer::RewriteRuleWith;
 using ::clang::transformer::statement;
 using ::testing::ElementsAre;
 using ::testing::IsEmpty;
@@ -129,7 +131,7 @@
 Changes.insert(Changes.end(), std::make_move_iterator(C->begin()),
std::make_move_iterator(C->end()));
   } else {
-// FIXME: stash this error rather then printing.
+// FIXME: stash this error rather than printing.
 llvm::errs() << "Error generating changes: "
  << llvm::toString(C.takeError()) << "\n";
 ++ErrorCount;
@@ -137,27 +139,58 @@
 };
   }
 
-  template 
-  void testRule(R Rule, StringRef Input, StringRef Expected) {
+  auto consumerWithStringMetadata() {
+return [this](Expected> C) {
+  if (C) {
+Changes.insert(Changes.end(),
+   std::make_move_iterator(C->Changes.begin()),
+   std::make_move_iterator(C->Changes.end()));
+StringMetadata.push_back(std::move(C->Metadata));
+  } else {
+// FIXME: stash this error rather than printing.
+llvm::errs() << "Error generating changes: "
+ << llvm::toString(C.takeError()) << "\n";
+++ErrorCount;
+  }
+};
+  }
+
+  void testRule(RewriteRule Rule, StringRef Input, StringRef Expected) {
 Transformers.push_back(
 std::make_unique(std::move(Rule), consumer()));
 Transformers.back()->registerMatchers();
 compareSnippets(Expected, rewrite(Input));
   }
 
-  template  void testRuleFailure(R Rule, StringRef Input) {
+  void testRule(RewriteRuleWith Rule, StringRef Input,
+StringRef Expected) {
+Transformers.push_back(std::make_unique(
+std::move(Rule), consumerWithStringMetadata()));
+Transformers.back()->registerMatchers();
+compareSnippets(Expected, rewrite(Input));
+  }
+
+  void testRuleFailure(RewriteRule Rule, StringRef Input) {
 Transformers.push_back(
 std::make_unique(std::move(Rule), consumer()));
 Transformers.back()->registerMatchers();
 ASSERT_FALSE(rewrite(Input)) << "Expected failure to rewrite code";
   }
 
+  void testRuleFailure(RewriteRuleWith Rule, StringRef Input) {
+Transformers.push_back(std::make_unique(
+std::move(Rule), consumerWithStringMetadata()));
+Transformers.back()->registerMatchers();
+ASSERT_FALSE(rewrite(Input)) << "Expected failure to rewrite code";
+  }
+
   // Transformers are referenced by MatchFinder.
   std::vector> Transformers;
   clang::ast_matchers::MatchFinder MatchFinder;
   // Records whether any errors occurred in individual changes.
   int ErrorCount = 0;
   AtomicChanges Changes;
+  std::vector StringMetadata;
 
 private:
   FileContentMappings FileContents = {{"header.h", ""}};
@@ -169,7 +202,7 @@
 };
 
 // Given string s, change strlen($s.c_str()) to REPLACED.
-static RewriteRule ruleStrlenSize() {
+static RewriteRuleWith ruleStrlenSize() {
   StringRef StringExpr = "strexpr";
   auto StringType = namedDecl(hasAnyName("::basic_string", "::string"));
   auto R = makeRule(
@@ -886,12 +919,12 @@
 
 TEST_F(TransformerTest, OrderedRuleUnrelated) {
   StringRef Flag = "flag";
-  RewriteRule FlagRule = makeRule(
+  RewriteRuleWith FlagRule = makeRule(
   cxxMemberCallExpr(on(expr(hasType(cxxRecordDecl(
 hasName("proto::ProtoCommandLineFlag"
.bind(Flag)),
 unless(callee(cxxMethodDecl(hasName("GetProto"),
-  changeTo(node(std::string(Flag)), cat("PROTO")));

[PATCH] D120489: [analyzer] Done some changes to detect Uninitialized read by the char array manipulation functions

2022-03-03 Thread Shivam Rajput via Phabricator via cfe-commits
phyBrackets added a comment.

https://github.com/llvm/llvm-project/commit/bd1917c88a32c0930864d04f4e71155dcc3fa592
 , Hey @steakhal , I land it but why it is not showing the

void emitUninitializedReadBug(CheckerContext , ProgramStateRef State,
   const Expr *E) const;
  ``` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120489

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


[PATCH] D120907: [docs] Add PowerPC release notes for LLVM 14

2022-03-03 Thread Lei Huang via Phabricator via cfe-commits
lei added a comment.

In D120907#3357433 , @ldionne wrote:

> LGTM for libc++. I assume this is targeting `release/14.x` only.

yes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120907

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


[PATCH] D120911: [CUDA][HIP] Fix offloading kind for linking C++ programs

2022-03-03 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

We should probably also check what happens when we specify compilation language 
explicitly: E.g. `clang -x cuda a.cu -x c++ b.cc`,  `clang -x cuda a.cu  b.cc` 
and `clang  a.cu  -x cuda b.cc`.


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

https://reviews.llvm.org/D120911

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


[PATCH] D120397: [C++20] [Modules] Make the linkage consistent for template and its specialization

2022-03-03 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added inline comments.



Comment at: clang/test/Modules/inconsist-export-template.cpp:19-23
+// FIXME: We should reject following specialization,
+// since it tries to export a name which is already introduced.
+export template <>
+void f1() {
+

urnathan wrote:
> ChuanqiXu wrote:
> > iains wrote:
> > > iains wrote:
> > > > ChuanqiXu wrote:
> > > > > urnathan wrote:
> > > > > > I don't think we should be testing for ill-formed code here.  We 
> > > > > > want to verify that explicit instantiations, explicit 
> > > > > > specializations and implicit instantiations all get the expected 
> > > > > > linkage -- both external linkage on exported entities, module 
> > > > > > linkage on non-exported module-purview entities.
> > > > > I think it could add an `expected-error` once we complete the check 
> > > > > in compiler so I added the FIXME here.
> > > > would it be possible to find a suitable place in the source for the 
> > > > FIXME?
> > > > I would be concerned that it could get forgotten in the test-case.
> > > or maybe just file a PR for accepts invalid code?
> > I would like to send an issue after the patch to remind me not forgetting 
> > it. The issue is needed after the patch since it would crash too. I prefer 
> > to remain the FIXME here so that it would look like a pre-commit test case, 
> > which should be good.
> The way to not forget about this is to file a bug and assign it to yourself.  
> Not add a known-wrong testcase.
FWIW, I do sometimes put in known-bad test cases to document existing issues 
and also to make it easier to find existing test coverage/a good home for the 
test case (often I/we end up adding a new test file, because it's not obvious 
where related existing test coverage is).

If this is the right place for the future test case, and it documents a 
limitation with the existing behavior, I wouldn't be totally against including 
it here, with a FIXME and reference to a filed bug, ideally (though I think 
I've done this without a bug filed too).


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

https://reviews.llvm.org/D120397

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


[PATCH] D120900: [clang][dataflow] Add `MatchSwitch` utility library.

2022-03-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked an inline comment as done.
ymandel added a comment.

Thanks for the fast review!




Comment at: clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h:51
+template 
+using MatchSwitch = std::function;
+

xazax.hun wrote:
> When we instantiate this with `TransferState` we have `ASTContext` both as an 
> argument and as a member of `State`. Is this intentional?
Yes, but...
The `ASTContext` is needed for the match itself, but the `State` type is not 
guaranteed to be `TransferState`, so won't necessarily hold the context.

But, we could reorganize a bit. Either: 
a) pass MatchFinder::MatchResult, instead of BoundNodes. That would bundle the 
nodes, context and source manager, which seems like a good idea.
b) bake TransferState into MatchSwitch, and make the parameter genericy named 
rather than "lattice".

I'm inclined towards the first option, since it seems "right" to give full 
access to the `MatchResult`. WDYT?



Comment at: clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h:110
+size_t Index = 0;
+if (ID.consume_front("Tag") && !ID.getAsInteger(10, Index) &&
+Index < Actions.size()) {

xazax.hun wrote:
> This does not need to be addressed here but it looks like it could be a 
> useful feature to be able to tag nodes with integer identifiers.
Great point. I'm not sure that would work here, fwiw, because we prefix with 
"Tag" here to protect against interfering with any IDs in the matcher. I 
suppose we could "reserve" a portion of the ID space.

But, in general, integers would make more sense in other situations that deal 
w/ matchers. The problem is that, unless we encode them as strings, the 
implementation inside the matchers could be kind of messy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120900

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


[clang] bd1917c - [analyzer] Done some changes to detect Uninitialized read by the char array manipulation functions

2022-03-03 Thread via cfe-commits

Author: Shivam
Date: 2022-03-03T23:21:26+05:30
New Revision: bd1917c88a32c0930864d04f4e71155dcc3fa592

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

LOG: [analyzer] Done some changes to detect Uninitialized read by the char 
array manipulation functions

Few weeks back I was experimenting with reading the uninitialized values from 
src , which is actually a bug but the CSA seems to give up at that point . I 
was curious about that and I pinged @steakhal on the discord and according to 
him this seems to be a genuine issue and needs to be fix. So I goes with fixing 
this bug and thanks to @steakhal who help me creating this patch. This feature 
seems to break some tests but this was the genuine problem and the broken tests 
also needs to fix in certain manner. I add a test but yeah we need more 
tests,I'll try to add more tests.Thanks

Reviewed By: steakhal, NoQ

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

Added: 
clang/test/Analysis/bstring_UninitRead.c

Modified: 
clang/docs/analyzer/checkers.rst
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
clang/test/Analysis/bstring.c

Removed: 




diff  --git a/clang/docs/analyzer/checkers.rst 
b/clang/docs/analyzer/checkers.rst
index a42e3e6960777..a9ebe063c6c8b 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -2625,13 +2625,44 @@ alpha.unix.cstring.OutOfBounds (C)
 ""
 Check for out-of-bounds access in string functions; applies to:`` strncopy, 
strncat``.
 
-
 .. code-block:: c
 
  void test() {
int y = strlen((char *)); // warn
  }
 
+.. _alpha-unix-cstring-UninitializedRead:
+
+alpha.unix.cstring.UninitializedRead (C)
+
+Check for uninitialized reads from common memory copy/manipulation functions 
such as:
+ ``memcpy, mempcpy, memmove, memcmp, strcmp, strncmp, strcpy, strlen, strsep`` 
and many more.
+
+.. code-block:: c 
+
+ void test() {
+  char src[10];
+  char dst[5];
+  memcpy(dst,src,sizeof(dst)); // warn: Bytes string function accesses 
uninitialized/garbage values
+ }
+
+Limitations:
+  
+   - Due to limitations of the memory modeling in the analyzer, one can likely
+ observe a lot of false-positive reports like this:
+  .. code-block:: c
+  
+void false_positive() {
+  int src[] = {1, 2, 3, 4};
+  int dst[5] = {0};
+  memcpy(dst, src, 4 * sizeof(int)); // false-positive:
+  // The 'src' buffer was correctly initialized, yet we cannot conclude
+  // that since the analyzer could not see a direct initialization of 
the
+  // very last byte of the source buffer.
+}
+  
+ More details at the corresponding `GitHub issue 
`_.
+  
 .. _alpha-nondeterminism-PointerIteration:
 
 alpha.nondeterminism.PointerIteration (C++)

diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td 
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 6b72d64950106..9340a114ac456 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -475,7 +475,12 @@ def CStringNotNullTerm : Checker<"NotNullTerminated">,
   HelpText<"Check for arguments which are not null-terminating strings">,
   Dependencies<[CStringModeling]>,
   Documentation;
-
+ 
+def CStringUninitializedRead : Checker<"UninitializedRead">,
+  HelpText<"Checks if the string manipulation function would read 
uninitialized bytes">,
+  Dependencies<[CStringModeling]>,
+  Documentation;
+  
 } // end "alpha.unix.cstring"
 
 let ParentPackage = Unix in {

diff  --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 8483a1a47cea1..54c9e887b8c8c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -80,7 +80,7 @@ class CStringChecker : public Checker< eval::Call,
  check::RegionChanges
  > {
   mutable std::unique_ptr BT_Null, BT_Bounds, BT_Overlap,
-  BT_NotCString, BT_AdditionOverflow;
+  BT_NotCString, BT_AdditionOverflow, BT_UninitRead;
 
   mutable const char *CurrentFunctionDescription;
 
@@ -92,11 +92,13 @@ class CStringChecker : public Checker< eval::Call,
 DefaultBool CheckCStringOutOfBounds;
 DefaultBool CheckCStringBufferOverlap;
 DefaultBool CheckCStringNotNullTerm;
+DefaultBool CheckCStringUninitializedRead;
 
 CheckerNameRef CheckNameCStringNullArg;
 CheckerNameRef CheckNameCStringOutOfBounds;
 CheckerNameRef 

[PATCH] D120489: [analyzer] Done some changes to detect Uninitialized read by the char array manipulation functions

2022-03-03 Thread Shivam Rajput via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbd1917c88a32: [analyzer] Done some changes to detect 
Uninitialized read by the char array… (authored by Shivam 
75530356+phybrack...@users.noreply.github.com, committed by 
phyBrackets).

Changed prior to commit:
  https://reviews.llvm.org/D120489?vs=412464=412750#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120489

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/test/Analysis/bstring.c
  clang/test/Analysis/bstring_UninitRead.c

Index: clang/test/Analysis/bstring_UninitRead.c
===
--- /dev/null
+++ clang/test/Analysis/bstring_UninitRead.c
@@ -0,0 +1,59 @@
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN: -analyzer-checker=core,alpha.unix.cstring
+
+
+// This file is generally for the alpha.unix.cstring.UninitializedRead Checker, the reason for putting it into
+// the separate file because the checker is break the some existing test cases in bstring.c file , so we don't 
+// wanna mess up with some existing test case so it's better to create separate file for it, this file also include 
+// the broken test for the reference in future about the broken tests.
+
+
+typedef typeof(sizeof(int)) size_t;
+
+void clang_analyzer_eval(int);
+
+void *memcpy(void *restrict s1, const void *restrict s2, size_t n);
+
+void top(char *dst) {
+  char buf[10];
+  memcpy(dst, buf, 10); // expected-warning{{Bytes string function accesses uninitialized/garbage values}}
+  (void)buf;
+}
+
+//===--===
+// mempcpy()
+//===--===
+
+void *mempcpy(void *restrict s1, const void *restrict s2, size_t n);
+
+void mempcpy14() {
+  int src[] = {1, 2, 3, 4};
+  int dst[5] = {0};
+  int *p;
+
+  p = mempcpy(dst, src, 4 * sizeof(int)); // expected-warning{{Bytes string function accesses uninitialized/garbage values}}
+   // FIXME: This behaviour is actually surprising and needs to be fixed, 
+   // mempcpy seems to consider the very last byte of the src buffer uninitialized
+   // and returning undef unfortunately. It should have returned unknown or a conjured value instead.
+
+  clang_analyzer_eval(p == [4]); // no-warning (above is fatal)
+}
+
+struct st {
+  int i;
+  int j;
+};
+
+
+void mempcpy15() {
+  struct st s1 = {0};
+  struct st s2;
+  struct st *p1;
+  struct st *p2;
+
+  p1 = () + 1;
+  p2 = mempcpy(, , sizeof(struct st)); // expected-warning{{Bytes string function accesses uninitialized/garbage values}}
+  // FIXME: It seems same as mempcpy14() case.
+  
+  clang_analyzer_eval(p1 == p2); // no-warning (above is fatal)
+}
Index: clang/test/Analysis/bstring.c
===
--- clang/test/Analysis/bstring.c
+++ clang/test/Analysis/bstring.c
@@ -2,13 +2,15 @@
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=unix.cstring \
 // RUN:   -analyzer-checker=alpha.unix.cstring \
+// RUN:   -analyzer-disable-checker=alpha.unix.cstring.UninitializedRead \
 // RUN:   -analyzer-checker=debug.ExprInspection \
-// RUN:   -analyzer-config eagerly-assume=false
+// RUN:   -analyzer-config eagerly-assume=false  
 //
 // RUN: %clang_analyze_cc1 -verify %s -DUSE_BUILTINS \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=unix.cstring \
 // RUN:   -analyzer-checker=alpha.unix.cstring \
+// RUN:   -analyzer-disable-checker=alpha.unix.cstring.UninitializedRead \
 // RUN:   -analyzer-checker=debug.ExprInspection \
 // RUN:   -analyzer-config eagerly-assume=false
 //
@@ -16,6 +18,7 @@
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=unix.cstring \
 // RUN:   -analyzer-checker=alpha.unix.cstring \
+// RUN:   -analyzer-disable-checker=alpha.unix.cstring.UninitializedRead \
 // RUN:   -analyzer-checker=debug.ExprInspection \
 // RUN:   -analyzer-config eagerly-assume=false
 //
@@ -23,6 +26,7 @@
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=unix.cstring \
 // RUN:   -analyzer-checker=alpha.unix.cstring \
+// RUN:   -analyzer-disable-checker=alpha.unix.cstring.UninitializedRead \
 // RUN:   -analyzer-checker=debug.ExprInspection \
 // RUN:   -analyzer-config eagerly-assume=false
 
@@ -315,7 +319,7 @@
 
   p1 = () + 1;
   p2 = mempcpy(, , sizeof(struct st));
-
+  
   clang_analyzer_eval(p1 == p2); // expected-warning{{TRUE}}
 }
 
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -80,7 +80,7 @@
  check::RegionChanges
  

  1   2   >