[PATCH] D123498: [clang] Adding Platform/Architecture Specific Resource Header Installation Targets

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/lib/Headers/CMakeLists.txt:421
 
 install(
   FILES ${ppc_wrapper_files}

qiongsiwu1 wrote:
> craig.topper wrote:
> > qiongsiwu1 wrote:
> > > craig.topper wrote:
> > > > There appear to be two installs of ppc_wrapper_files with different 
> > > > components. Is that intentional?
> > > Ah yes this is indeed intentional. `cuda_wrapper_files` and 
> > > `openmp_wrapper_files` have two install targets as well. The first target 
> > > is part of the "catch-all" `clang-resource-headers`.  The second targets 
> > > are not installed by default (`EXCLUDE_FROM_ALL`). and a distribution 
> > > build can opt-in the relevant set of headers. 
> > > 
> > > That said, if there are better ways to implement the logic I am all ears. 
> > As far as I know, the ppc_wrapper_files are only usable on the powerpc 
> > target so I don't think they should be treated any different than 
> > ppc_files. ppc_files only has one install target right?
> > As far as I know, the ppc_wrapper_files are only usable on the powerpc 
> > target 
> Yes that is my understanding as well. 
> 
> > ppc_files only has one install target right?
> There is indeed one install target specifically setup to include only ppc 
> files. Meanwhile, the ppc files are also included in the `${files}` list 
> (which is installed as as a component of the catch-all 
> `clang-resource-headers`), so the ppc files are included in two different 
> install targets. 
> 
Thanks. I was trying to resolve some merge issues in my downstream and I really 
should have read your changes more closely.



Comment at: clang/lib/Headers/CMakeLists.txt:548
+install(
+  FILES ${windows_files}
+  DESTINATION ${header_install_dir}

Should this be windows_only_files. I don't see a windows_files defined anywhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123498

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

[PATCH] D125944: Template instantiation error recovery

[Purva Chaudhari via Phabricator via cfe-commits]

Purva-Chaudhari created this revision.
Purva-Chaudhari added a reviewer: v.g.vassilev.
Herald added a project: All.
Purva-Chaudhari requested review of this revision.

If error was encountered after template instantiation, the clang-repl 
interactive mode was aborted. The patch adds recovery support for template 
instantiation

Eg:

  purva@purva-HP-Laptop-15-bs0xx:~/llvm-project/build$ bin/clang-repl
  clang-repl> template T f() { return T(); }
  clang-repl> auto ptu2 = f(); err;
  In file included from <<< inputs >>>:1:
  input_line_1:1:25: error: C++ requires a type specifier for all declarations
  auto ptu2 = f(); err;
  ^
  clang-repl: /home/purva/llvm-project/clang/include/clang/Sema/Sema.h:9406: 
clang::Sema::GlobalEagerInstantiationScope::~GlobalEagerInstantiationScope(): 
Assertion `S.PendingInstantiations.empty() && "PendingInstantiations should be 
empty before it is discarded."' failed.
  Aborted
   (core dumped)


https://reviews.llvm.org/D125944

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/test/Interpreter/execute.cpp


Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -9456,6 +9456,19 @@
 SavedPendingLocalImplicitInstantiations;
   };
 
+  class PerformPendingInstantiationsRAII {
+  public:
+PerformPendingInstantiationsRAII(Sema ): S(S) {} ;
+
+~PerformPendingInstantiationsRAII() {
+  S.PerformPendingInstantiations();
+  assert(S.PendingInstantiations.empty() &&
+ "there shouldn't be any pending instantiations");
+}
+  private:
+Sema 
+  };
+
   /// A helper class for building up ExtParameterInfos.
   class ExtParameterInfoBuilder {
 SmallVector Infos;
Index: clang/lib/Interpreter/IncrementalParser.cpp
===
--- clang/lib/Interpreter/IncrementalParser.cpp
+++ clang/lib/Interpreter/IncrementalParser.cpp
@@ -177,6 +177,7 @@
   }
 
   DiagnosticsEngine  = getCI()->getDiagnostics();
+  Sema::PerformPendingInstantiationsRAII PerformPendingInstantiations(S);
   if (Diags.hasErrorOccurred()) {
 TranslationUnitDecl *MostRecentTU = C.getTranslationUnitDecl();

Index: clang/test/Interpreter/execute.cpp
===
--- clang/test/Interpreter/execute.cpp
+++ clang/test/Interpreter/execute.cpp
@@ -1,3 +1,5 @@
+// RUN: clang-repl "template T f() { return T(); }" "auto ptu2 = 
f(); err;" \
+// RUN: "auto ptu2 = f();" "int i = 0;"
 // RUN: clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);' \
 // RUN:'auto r1 = printf("i = %d\n", i);' | FileCheck 
--check-prefix=CHECK-DRIVER %s
 // REQUIRES: host-supports-jit


Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -9456,6 +9456,19 @@
 SavedPendingLocalImplicitInstantiations;
   };
 
+  class PerformPendingInstantiationsRAII {
+  public:
+PerformPendingInstantiationsRAII(Sema ): S(S) {} ;
+
+~PerformPendingInstantiationsRAII() {
+  S.PerformPendingInstantiations();
+  assert(S.PendingInstantiations.empty() &&
+ "there shouldn't be any pending instantiations");
+}
+  private:
+Sema 
+  };
+
   /// A helper class for building up ExtParameterInfos.
   class ExtParameterInfoBuilder {
 SmallVector Infos;
Index: clang/lib/Interpreter/IncrementalParser.cpp
===
--- clang/lib/Interpreter/IncrementalParser.cpp
+++ clang/lib/Interpreter/IncrementalParser.cpp
@@ -177,6 +177,7 @@
   }
 
   DiagnosticsEngine  = getCI()->getDiagnostics();
+  Sema::PerformPendingInstantiationsRAII PerformPendingInstantiations(S);
   if (Diags.hasErrorOccurred()) {
 TranslationUnitDecl *MostRecentTU = C.getTranslationUnitDecl();

Index: clang/test/Interpreter/execute.cpp
===
--- clang/test/Interpreter/execute.cpp
+++ clang/test/Interpreter/execute.cpp
@@ -1,3 +1,5 @@
+// RUN: clang-repl "template T f() { return T(); }" "auto ptu2 = f(); err;" \
+// RUN: "auto ptu2 = f();" "int i = 0;"
 // RUN: clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);' \
 // RUN:'auto r1 = printf("i = %d\n", i);' | FileCheck --check-prefix=CHECK-DRIVER %s
 // REQUIRES: host-supports-jit
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125875: [RISCV] Add vread_csr and vwrite_csr to riscv_vector.h

[Wang Pengcheng via Phabricator via cfe-commits]

pcwang-thead updated this revision to Diff 430567.
pcwang-thead added a comment.

Add double underscore to all variables.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125875

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/test/CodeGen/RISCV/rvv-vread-csr.c
  clang/test/CodeGen/RISCV/rvv-vwrite-csr.c

Index: clang/test/CodeGen/RISCV/rvv-vwrite-csr.c
===
--- /dev/null
+++ clang/test/CodeGen/RISCV/rvv-vwrite-csr.c
@@ -0,0 +1,41 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -triple riscv64 -target-feature +v -disable-O0-optnone -emit-llvm %s -o - \
+// RUN: | opt -S -O2 | FileCheck  %s
+
+#include 
+
+// CHECK-LABEL: @vwrite_csr_vstart(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:tail call void asm sideeffect "csrw\09vstart, ${0:z}", "rJ,~{memory}"(i64 [[VALUE:%.*]]) #[[ATTR1:[0-9]+]], !srcloc !4
+// CHECK-NEXT:ret void
+//
+void vwrite_csr_vstart(unsigned long value) {
+  vwrite_csr(RVV_VSTART, value);
+}
+
+// CHECK-LABEL: @vwrite_csr_vxsat(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:tail call void asm sideeffect "csrw\09vxsat, ${0:z}", "rJ,~{memory}"(i64 [[VALUE:%.*]]) #[[ATTR1]], !srcloc !5
+// CHECK-NEXT:ret void
+//
+void vwrite_csr_vxsat(unsigned long value) {
+  vwrite_csr(RVV_VXSAT, value);
+}
+
+// CHECK-LABEL: @vwrite_csr_vxrm(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:tail call void asm sideeffect "csrw\09vxrm, ${0:z}", "rJ,~{memory}"(i64 [[VALUE:%.*]]) #[[ATTR1]], !srcloc !6
+// CHECK-NEXT:ret void
+//
+void vwrite_csr_vxrm(unsigned long value) {
+  vwrite_csr(RVV_VXRM, value);
+}
+
+// CHECK-LABEL: @vwrite_csr_vcsr(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:tail call void asm sideeffect "csrw\09vcsr, ${0:z}", "rJ,~{memory}"(i64 [[VALUE:%.*]]) #[[ATTR1]], !srcloc !7
+// CHECK-NEXT:ret void
+//
+void vwrite_csr_vcsr(unsigned long value) {
+  vwrite_csr(RVV_VCSR, value);
+}
Index: clang/test/CodeGen/RISCV/rvv-vread-csr.c
===
--- /dev/null
+++ clang/test/CodeGen/RISCV/rvv-vread-csr.c
@@ -0,0 +1,41 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -triple riscv64 -target-feature +v -disable-O0-optnone -emit-llvm %s -o - \
+// RUN: | opt -S -O2 | FileCheck  %s
+
+#include 
+
+// CHECK-LABEL: @vread_csr_vstart(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = tail call i64 asm sideeffect "csrr\09$0, vstart", "=r,~{memory}"() #[[ATTR1:[0-9]+]], !srcloc !4
+// CHECK-NEXT:ret i64 [[TMP0]]
+//
+unsigned long vread_csr_vstart(void) {
+  return vread_csr(RVV_VSTART);
+}
+
+// CHECK-LABEL: @vread_csr_vxsat(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = tail call i64 asm sideeffect "csrr\09$0, vxsat", "=r,~{memory}"() #[[ATTR1]], !srcloc !5
+// CHECK-NEXT:ret i64 [[TMP0]]
+//
+unsigned long vread_csr_vxsat(void) {
+  return vread_csr(RVV_VXSAT);
+}
+
+// CHECK-LABEL: @vread_csr_vxrm(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = tail call i64 asm sideeffect "csrr\09$0, vxrm", "=r,~{memory}"() #[[ATTR1]], !srcloc !6
+// CHECK-NEXT:ret i64 [[TMP0]]
+//
+unsigned long vread_csr_vxrm(void) {
+  return vread_csr(RVV_VXRM);
+}
+
+// CHECK-LABEL: @vread_csr_vcsr(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = tail call i64 asm sideeffect "csrr\09$0, vcsr", "=r,~{memory}"() #[[ATTR1]], !srcloc !7
+// CHECK-NEXT:ret i64 [[TMP0]]
+//
+unsigned long vread_csr_vcsr(void) {
+  return vread_csr(RVV_VCSR);
+}
Index: clang/include/clang/Basic/riscv_vector.td
===
--- clang/include/clang/Basic/riscv_vector.td
+++ clang/include/clang/Basic/riscv_vector.td
@@ -1497,6 +1497,58 @@
   }
 }
 
+// Define vread_csr_csr described in RVV intrinsics doc.
+let HeaderCode =
+[{
+enum RVV_CSR {
+  RVV_VSTART = 0,
+  RVV_VXSAT,
+  RVV_VXRM,
+  RVV_VCSR,
+};
+
+__extension__ extern __inline
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
+unsigned long vread_csr(enum RVV_CSR __csr) {
+  unsigned long __rv = 0;
+  switch (__csr) {
+case RVV_VSTART:
+  __asm__ __volatile__ ("csrr\t%0, vstart" : "=r"(__rv) : : "memory");
+  break;
+case RVV_VXSAT:
+  __asm__ __volatile__ ("csrr\t%0, vxsat" : "=r"(__rv) : : "memory");
+  break;
+case RVV_VXRM:
+  __asm__ __volatile__ ("csrr\t%0, vxrm" : "=r"(__rv) : : "memory");
+  break;
+case RVV_VCSR:
+  __asm__ __volatile__ ("csrr\t%0, vcsr" : "=r"(__rv) : : "memory");
+  break;
+  }
+  return __rv;
+}
+
+__extension__ extern __inline
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
+void vwrite_csr(enum RVV_CSR __csr, unsigned long __value) {
+  switch (__csr) {
+case RVV_VSTART:
+  __asm__ __volatile__ ("csrw\tvstart, %z0" : : 

[PATCH] D125875: [RISCV] Add vread_csr and vwrite_csr to riscv_vector.h

[Wang Pengcheng via Phabricator via cfe-commits]

pcwang-thead added inline comments.



Comment at: clang/include/clang/Basic/riscv_vector.td:1512
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
+unsigned long vread_csr(enum RVV_CSR csr) {
+  unsigned long rv = 0;

craig.topper wrote:
> pcwang-thead wrote:
> > craig.topper wrote:
> > > X86 and arm_neon.h uses double underscore prefixes on all variables in 
> > > intrinsic headers to reduce the chance of a collision with a user having 
> > > a macro with the same name.
> > The function prototypes have already been defined in RVV intrinsics doc 
> > (https://github.com/riscv-non-isa/rvv-intrinsic-doc/blob/master/rvv-intrinsic-api.md#readwrite-urw-vector-csrs).
> I'm only refering to `csr`, `rv` and `value`. Those don't need to match the 
> intrinsics doc. They aren't visible to code using the header.
Oh I get it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125875

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

[PATCH] D125875: [RISCV] Add vread_csr and vwrite_csr to riscv_vector.h

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/include/clang/Basic/riscv_vector.td:1512
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
+unsigned long vread_csr(enum RVV_CSR csr) {
+  unsigned long rv = 0;

pcwang-thead wrote:
> craig.topper wrote:
> > X86 and arm_neon.h uses double underscore prefixes on all variables in 
> > intrinsic headers to reduce the chance of a collision with a user having a 
> > macro with the same name.
> The function prototypes have already been defined in RVV intrinsics doc 
> (https://github.com/riscv-non-isa/rvv-intrinsic-doc/blob/master/rvv-intrinsic-api.md#readwrite-urw-vector-csrs).
I'm only refering to `csr`, `rv` and `value`. Those don't need to match the 
intrinsics doc. They aren't visible to code using the header.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125875

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

[PATCH] D125875: [RISCV] Add vread_csr and vwrite_csr to riscv_vector.h

[Wang Pengcheng via Phabricator via cfe-commits]

pcwang-thead added inline comments.



Comment at: clang/include/clang/Basic/riscv_vector.td:1511
+__extension__ extern __inline
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
+unsigned long vread_csr(enum RVV_CSR csr) {

craig.topper wrote:
> Do we need __gnu_inline__ and __artificial__? The only intrinsics headers 
> that have them in clang are in ppc_wrappers. X86 uses 
> `__attribute__((__always_inline__, __nodebug__`
I just made it the same as GCC.



Comment at: clang/include/clang/Basic/riscv_vector.td:1512
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
+unsigned long vread_csr(enum RVV_CSR csr) {
+  unsigned long rv = 0;

craig.topper wrote:
> X86 and arm_neon.h uses double underscore prefixes on all variables in 
> intrinsic headers to reduce the chance of a collision with a user having a 
> macro with the same name.
The function prototypes have already been defined in RVV intrinsics doc 
(https://github.com/riscv-non-isa/rvv-intrinsic-doc/blob/master/rvv-intrinsic-api.md#readwrite-urw-vector-csrs).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125875

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

[PATCH] D125291: Introduce @llvm.threadlocal.address intrinsic to access TLS variable (1/3)

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D125291#3523818 , @efriedma wrote:

> I don't really understand how this is supposed to interact with D125292 
> ; even if you strip the readnone attribute 
> from the call instruction, we'll still treat a call to the intrinsic as 
> readnone.

Yeah, the point here is that we used operand bundle in `D125292`. The operand 
bundles would break the logic you said, see: 
https://github.com/llvm/llvm-project/blob/3b762b3ab8d205cd6a7d42c96d39d5f4f701f2ab/llvm/include/llvm/IR/InstrTypes.h#L2315-L2325.

> I think I'd prefer to lower the intrinsic as part of the codegen optimization 
> pipeline, not the optimization pipeline.  So maybe just in in SelectionDAG?  
> I mean, it doesn't matter much for your planned usage in coroutines, but it's 
> more similar to other intrinsics, and more like what we expect it to look 
> like in the future.

Good idea. Would do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125291

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

[PATCH] D125679: [Clang] Added options for integrated backend only used for SPIR-V for now

[Michal Paszkowski via Phabricator via cfe-commits]

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

The typos noticed by Ilia need to be fixed, but apart from this the patch LGTM! 
Thank you Anastasia!


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

https://reviews.llvm.org/D125679

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

[PATCH] D125875: [RISCV] Add vread_csr and vwrite_csr to riscv_vector.h

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/include/clang/Basic/riscv_vector.td:1511
+__extension__ extern __inline
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
+unsigned long vread_csr(enum RVV_CSR csr) {

Do we need __gnu_inline__ and __artificial__? The only intrinsics headers that 
have them in clang are in ppc_wrappers. X86 uses 
`__attribute__((__always_inline__, __nodebug__`



Comment at: clang/include/clang/Basic/riscv_vector.td:1512
+__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
+unsigned long vread_csr(enum RVV_CSR csr) {
+  unsigned long rv = 0;

X86 and arm_neon.h uses double underscore prefixes on all variables in 
intrinsic headers to reduce the chance of a collision with a user having a 
macro with the same name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125875

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

[PATCH] D123649: Allow flexible array initialization in C++.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4639
+  getDataLayout().getTypeAllocSize(Init->getType()));
+  assert(VarSize == CstSize && "Emitted constant has unexpected size");
+#endif

efriedma wrote:
> ahatanak wrote:
> > This assertion fails when the following code is compiled:
> > 
> > 
> > ```
> > typedef unsigned char uint8_t;
> > typedef uint8_t uint8_a16 __attribute__((aligned(16)));
> > 
> > void foo1() {
> >   static const uint8_a16 array1[] = { 1 };
> > }
> > ```
> `sizeof(uint8_a16[1])` is 16, but we currently emit a one-byte global.  So it 
> seems like there's an underlying bug exposed by the assertion.
> 
> gcc thinks this is nonsense, and just prints an error.
It seems to me that we should disallow arrays that have an element whose 
alignment is larger than its size, just as gcc does.

But I see arrays like that declared in a couple of clang's regression tests 
(e.g., `Sema/attr-aligned.c`). I'm not sure whether there was a reason for not 
rejecting it or it was just an oversight.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123649

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

[PATCH] D125291: Introduce @llvm.threadlocal.address intrinsic to access TLS variable (1/3)

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

I don't really understand how this is supposed to interact with D125292 
; even if you strip the readnone attribute 
from the call instruction, we'll still treat a call to the intrinsic as 
readnone.

I think I'd prefer to lower the intrinsic as part of the codegen optimization 
pipeline, not the optimization pipeline.  So maybe just in in SelectionDAG?  I 
mean, it doesn't matter much for your planned usage in coroutines, but it's 
more similar to other intrinsics, and more like what we expect it to look like 
in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125291

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

[PATCH] D125936: [Sema] Relax an assertion in BuildStmtExpr

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added reviewers: rjmccall, akyrtzi.
ahatanak added a project: clang.
Herald added a project: All.
ahatanak requested review of this revision.

The assertion doesn't hold if there are temporaries created in the AsmStmt 
passed to the method.

rdar://92845835


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125936

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaTemplate/instantiate-expr-1.cpp


Index: clang/test/SemaTemplate/instantiate-expr-1.cpp
===
--- clang/test/SemaTemplate/instantiate-expr-1.cpp
+++ clang/test/SemaTemplate/instantiate-expr-1.cpp
@@ -190,3 +190,19 @@
 test_asm_tied(1.f); // expected-note {{instantiation of}}
   }
 }
+
+namespace TestAsmCleanup {
+struct S {
+  operator int() const { return 1; }
+  ~S();
+};
+
+template 
+void foo() {
+  __asm__ __volatile__("%[i]"
+   :
+   : [i] "r"(-S()));
+}
+
+void test() { foo(); }
+} // namespace TestAsmCleanup
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -15745,7 +15745,9 @@
 
   if (hasAnyUnrecoverableErrorsInThisFunction())
 DiscardCleanupsInEvaluationContext();
-  assert(!Cleanup.exprNeedsCleanups() &&
+  // Cleanups may be needed if temporaries are created in an AsmStmt.
+  assert((!Cleanup.exprNeedsCleanups() ||
+  (!Compound->body_empty() && isa(Compound->body_front( &&
  "cleanups within StmtExpr not correctly bound!");
   PopExpressionEvaluationContext();
 


Index: clang/test/SemaTemplate/instantiate-expr-1.cpp
===
--- clang/test/SemaTemplate/instantiate-expr-1.cpp
+++ clang/test/SemaTemplate/instantiate-expr-1.cpp
@@ -190,3 +190,19 @@
 test_asm_tied(1.f); // expected-note {{instantiation of}}
   }
 }
+
+namespace TestAsmCleanup {
+struct S {
+  operator int() const { return 1; }
+  ~S();
+};
+
+template 
+void foo() {
+  __asm__ __volatile__("%[i]"
+   :
+   : [i] "r"(-S()));
+}
+
+void test() { foo(); }
+} // namespace TestAsmCleanup
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -15745,7 +15745,9 @@
 
   if (hasAnyUnrecoverableErrorsInThisFunction())
 DiscardCleanupsInEvaluationContext();
-  assert(!Cleanup.exprNeedsCleanups() &&
+  // Cleanups may be needed if temporaries are created in an AsmStmt.
+  assert((!Cleanup.exprNeedsCleanups() ||
+  (!Compound->body_empty() && isa(Compound->body_front( &&
  "cleanups within StmtExpr not correctly bound!");
   PopExpressionEvaluationContext();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124382: [Clang] Recognize target address space in superset calculation

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia requested changes to this revision.
Anastasia added a comment.
This revision now requires changes to proceed.

I feel that to progress further on this change, it would be good to get details 
about the use cases and the limitations first.

However, if there is sufficient evidence of the need to extend clang builtins 
with language address spaces I am not convinced the approach here is suitable 
as:

1. It doesn't allow uses of language builtins with language address spaces 
generically as mapping to the target address spaces is not portable. In general 
there can also be generic buitins that are normally mapped to native LLVM 
intrinsics (not target intrinsics!) shared among multiple targets that might 
also benefit from having this implemented in a target agnostic way. Such 
intrinsics could be shares for example between PTX and SPIR-V targets as there 
is quite some overlap in the functionality.
2. It has much wider impact on the language semantics then just allowing 
language address spaces being used in builtins i.e. it results in implicit 
conversions more broadly. This might not be desirable evolution and we might 
need to reach some consensus with more languages or targets using the address 
spaces in order to proceed with such change. In fact current title and 
description doesn't adequately reflect the impact of the change.

Has extending the builtin definition syntax been considered for this problem? 
That seems like a more natural and fairly localized change. For example the 
syntax in 
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/Builtins.def#L65
 could be changed to:

  // * -> pointer (optionally followed by an address space number for target 
address space
  //   or by 00 and a number for language address space as it is 
set in LangAS, if no
  //   address space is specified than any address space will be 
accepted)

Note that we will likely need to set LangAS entry values explicitly which would 
make maintaing the enum slightly more painful but it doesn't seem like a 
concern.

If the only use case we have right now is ability to specify generic address 
space in kernel-like langauges we could also just reserve a special `00` number 
in address space field of the buitin prototype description  for `generic 
address space` and leave full support as a future work.

If we understand the use case better we might be able to look at other 
alternatives too...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124382

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

[PATCH] D125349: [Sema] Fix crash for C11 atomic in gnu++ mode

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

In D125349#3509073 , @aaron.ballman 
wrote:

> It's interesting to note that `an_atomic_uint = an_atomic_uint + 
> an_enum_value` works correctly: https://godbolt.org/z/cvP9e6nh7. I was trying 
> to figure out whether the atomic qualifier is properly stripped for the 
> compound operator. When I run under a debugger and dump the AST for 
> `Args[0]`, I get: `DeclRefExpr 0x26efcf87f88 '_Atomic(unsigned int)' lvalue 
> Var 0x26efcf44cb8 'an_atomic_uint' '_Atomic(unsigned int)'` which seems like 
> it may be the root cause of the problem here (tough to say given that this is 
> a C extension in C++ though). The lvalue conversion that takes place for 
> `an_atomic_uint` should drop the atomic qualifier per C2x 6.3.2.1p2.

Thank you for the suggestion! I'm looking into this.


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

https://reviews.llvm.org/D125349

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

[PATCH] D125349: [Sema] Fix crash for C11 atomic in gnu++ mode

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

In D125349#3509546 , @ahatanak wrote:

> Is it not possible to handle this similarly to `volatile unsigned`? If I 
> replace `_Atomic unsigned` with `volatile unsigned`, I see 
> `LookupOverloadedBinOp` succeed without having to strip volatile because 
> `addAssignmentArithmeticOverloads` adds candidates with volatile types.

Possibly? I am just generally apprehensive about changing the C++ side of 
things as I can't imagine all the consequences.
One difference that I see is the existence of `volatile`-qualified versions of 
these operators being prescribed by C++ standard while the operators you 
suggest definitely aren't:

  // C++ [over.built]p18:
  //
  //   For every triple (L, VQ, R), where L is an arithmetic type,
  //   VQ is either volatile or empty, and R is a promoted
  //   arithmetic type, there exist candidate operator functions of
  //   the form

https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaOverload.cpp#L8926


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

https://reviews.llvm.org/D125349

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

[PATCH] D125893: [RISCV][NFC] Change interface of RVVIntrinsic::getSuffixStr

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/include/clang/Support/RISCVVIntrinsicUtils.h:346
+  getSuffixStr(BasicType Type, int Log2LMUL,
+   const llvm::ArrayRef 
);
 };

Drop the `const` and the reference


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125893

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

[PATCH] D125931: [clang][dataflow] Add support for correlated branches to optional model

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev created this revision.
sgatev added reviewers: ymandel, xazax.hun, gribozavr2.
Herald added subscribers: tschuett, steakhal, rnkovacs.
Herald added a project: All.
sgatev requested review of this revision.
Herald added a project: clang.

Add support for correlated branches to the std::optional dataflow model.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125931

Files:
  
clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1852,7 +1852,25 @@
   )",
  UnorderedElementsAre(Pair("check", "safe")));
 
-  // FIXME: Add tests that call `emplace` in conditional branches.
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+void target($ns::$optional opt, bool b) {
+  if (b) {
+opt.emplace(0);
+  }
+  if (b) {
+opt.value();
+/*[[check-1]]*/
+  } else {
+opt.value();
+/*[[check-2]]*/
+  }
+}
+  )",
+  UnorderedElementsAre(Pair("check-1", "safe"),
+   Pair("check-2", "unsafe: input.cc:12:9")));
 }
 
 TEST_P(UncheckedOptionalAccessTest, Reset) {
@@ -1883,7 +1901,26 @@
   )",
   UnorderedElementsAre(Pair("check", "unsafe: input.cc:7:9")));
 
-  // FIXME: Add tests that call `reset` in conditional branches.
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+void target(bool b) {
+  $ns::$optional opt = $ns::make_optional(0);
+  if (b) {
+opt.reset();
+  }
+  if (b) {
+opt.value();
+/*[[check-1]]*/
+  } else {
+opt.value();
+/*[[check-2]]*/
+  }
+}
+  )",
+  UnorderedElementsAre(Pair("check-1", "unsafe: input.cc:10:9"),
+   Pair("check-2", "safe")));
 }
 
 TEST_P(UncheckedOptionalAccessTest, ValueAssignment) {
@@ -2150,6 +2187,106 @@
   UnorderedElementsAre(Pair("check-1", "safe"), Pair("check-2", "safe")));
 }
 
+TEST_P(UncheckedOptionalAccessTest, CorrelatedBranches) {
+  ExpectLatticeChecksFor(R"code(
+#include "unchecked_optional_access_test.h"
+
+void target(bool b, $ns::$optional opt) {
+  if (b || opt.has_value()) {
+if (!b) {
+  opt.value();
+  /*[[check]]*/
+}
+  }
+}
+  )code",
+ UnorderedElementsAre(Pair("check", "safe")));
+
+  ExpectLatticeChecksFor(R"code(
+#include "unchecked_optional_access_test.h"
+
+void target(bool b, $ns::$optional opt) {
+  if (b && !opt.has_value()) return;
+  if (b) {
+opt.value();
+/*[[check]]*/
+  }
+}
+  )code",
+ UnorderedElementsAre(Pair("check", "safe")));
+
+  ExpectLatticeChecksFor(
+  R"code(
+#include "unchecked_optional_access_test.h"
+
+void target(bool b, $ns::$optional opt) {
+  if (opt.has_value()) b = true;
+  if (b) {
+opt.value();
+/*[[check]]*/
+  }
+}
+  )code",
+  UnorderedElementsAre(Pair("check", "unsafe: input.cc:7:9")));
+
+  ExpectLatticeChecksFor(R"code(
+#include "unchecked_optional_access_test.h"
+
+void target(bool b, $ns::$optional opt) {
+  if (b) return;
+  if (opt.has_value()) b = true;
+  if (b) {
+opt.value();
+/*[[check]]*/
+  }
+}
+  )code",
+ UnorderedElementsAre(Pair("check", "safe")));
+
+  ExpectLatticeChecksFor(R"(
+#include "unchecked_optional_access_test.h"
+
+void target(bool b, $ns::$optional opt) {
+  if (opt.has_value() == b) {
+if (b) {
+  opt.value();
+  /*[[check]]*/
+}
+  }
+}
+  )",
+ UnorderedElementsAre(Pair("check", "safe")));
+
+  ExpectLatticeChecksFor(R"(
+#include "unchecked_optional_access_test.h"
+
+void target(bool b, $ns::$optional opt) {
+  if (opt.has_value() != b) {
+if (!b) {
+  opt.value();
+  /*[[check]]*/
+}
+  }
+}
+  )",
+ UnorderedElementsAre(Pair("check", "safe")));
+
+  // FIXME: Add support for operator==.
+  // ExpectLatticeChecksFor(R"(
+  //   #include "unchecked_optional_access_test.h"
+  //
+  //   void target($ns::$optional opt1, $ns::$optional opt2) {
+  // if (opt1 == opt2) {
+  //   if (opt1.has_value()) {
+  // opt2.value();
+  // /*[[check]]*/
+  //   }
+  // }
+  //   }
+  // )",
+  // UnorderedElementsAre(Pair("check", 

[PATCH] D123538: [symbolizer] Parse DW_TAG_variable DIs to show line info for globals

[Mitch Phillips via Phabricator via cfe-commits]

hctim added inline comments.



Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp:769-783
+DataExtractor Data(Location.Expr, /*IsLittleEndian=*/true, 8);
+uint64_t DataOffset = 0;
+uint8_t Operation = Data.getU8();
+if (Operation == dwarf::DW_OP_addr) {
+  uint64_t Pointer = Data.getAddress();
+  VariableDieMap[Pointer] = Die;
+  return;

dblaikie wrote:
> hctim wrote:
> > dblaikie wrote:
> > > Are there any examples of global merge where this might not be correct?
> > > 
> > > Like if a global was described as "addrx, 5, add" (eg: 5 beyond this 
> > > address) then only looking at the first operation might mis-conclude that 
> > > the variable is at this address when it's at 5 bytes past this address - 
> > > and some other variable is at this address)
> > > 
> > > LLVM has a "global merge" optimization that might cause something like 
> > > this. Let's see if I can create an example.
> > > 
> > > Ah, yeah, here we go:
> > > ```
> > > static int x;
> > > static int y;
> > > int *f(bool b) { return b ?  :  }
> > > ```
> > > ```
> > > $ clang++-tot -O3 merge.cpp -c -g -target aarch64-linux-gnuabi -mllvm 
> > > -global-merge-ignore-single-use && llvm-dwarfdump-tot merge.o | grep 
> > > DW_AT_location
> > > DW_AT_location  (DW_OP_addrx 0x0)
> > > DW_AT_location  (DW_OP_addrx 0x0, DW_OP_plus_uconst 0x4)
> > > ```
> > > 
> > > (the `-global-merge-ignore-single-use` is to simplify the test case - 
> > > without that it could still be tickled with a more complicated example - 
> > > seems we only enable global merge on ARM 32 and 64 bit due to the higher 
> > > register pressure there, by the sounds of it)
> > > 
> > > I'm guessing this patch would overwrite the VariableDieMap entry for the 
> > > first global with the second one so queries for the first address would 
> > > result in the second DIE and queries for the second address wouldn't give 
> > > any result?
> > > 
> > > Hmm, also - how does this handle queries that aren't at exactly the 
> > > starting address of the variable? Presumably the `DW_AT_type` of the 
> > > `DW_TAG_global_variable` would have to be inspected to find the size of 
> > > the variable starting at the address, and any query in that range should 
> > > be successful?
> > I've added some handling for the `DW_OP_plus_uconst`. Unfortunately I 
> > didn't find any generic existing handling for the expression evaluation 
> > (which surprised me, maybe I just suck at looking), so I'm just making the 
> > assumption that global variable addresses aren't described using 
> > `DW_OP_plus` or anything else...
> > 
> > Fixed it up to now provide line info when you provide an address halfway 
> > through a symbol. Good thing there wasn't a big impact in D123534 about 
> > keeping the string sizes around :). Worst case (if the type info sucks) we 
> > just only accept the precise start addresses of the symbols.
> Probably worth maybe /only/ matching these expressions, then, rather than 
> currently it's matching any expression that starts with addr, or addr+plus? 
> Even if that's followed by any other garbage that would be ignored (& so the 
> expression would be misinterpreted)
> 
> There's probably no expression evaluation logic in LLVM's libDebugInfoDWARF - 
> there's no need to evaluate expressions when symbolizing, and dwarfdump just 
> wants to dump it, not compute the resulting value. (some similar code exists 
> in lldb that would do evaluation).
Sure, done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123538

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

[PATCH] D123538: [symbolizer] Parse DW_TAG_variable DIs to show line info for globals

[Mitch Phillips via Phabricator via cfe-commits]

hctim updated this revision to Diff 430509.
hctim marked an inline comment as done.
hctim added a comment.

Explicitly only match 'DW_OP_addr[x] [+ DW_OP_plus_uconst]'


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123538

Files:
  llvm/include/llvm/DebugInfo/DIContext.h
  llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h
  llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h
  llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
  llvm/include/llvm/DebugInfo/PDB/PDBContext.h
  llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
  llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
  llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
  llvm/lib/DebugInfo/PDB/PDBContext.cpp
  llvm/lib/DebugInfo/Symbolize/DIPrinter.cpp
  llvm/lib/DebugInfo/Symbolize/SymbolizableObjectFile.cpp
  llvm/test/DebugInfo/Symbolize/ELF/data-command-symtab.yaml
  llvm/test/tools/llvm-symbolizer/data-location.yaml
  llvm/test/tools/llvm-symbolizer/data.s

Index: llvm/test/tools/llvm-symbolizer/data.s
===
--- llvm/test/tools/llvm-symbolizer/data.s
+++ llvm/test/tools/llvm-symbolizer/data.s
@@ -7,9 +7,12 @@
 
 # CHECK:  d1
 # CHECK-NEXT: 0 8
+# CHECK-NEXT: ??:?
 # CHECK-EMPTY:
 # CHECK-NEXT: d2
 # CHECK-NEXT: 8 4
+# CHECK-NEXT: ??:?
+# CHECK-EMPTY:
 
 d1:
 .quad 0x1122334455667788
Index: llvm/test/tools/llvm-symbolizer/data-location.yaml
===
--- /dev/null
+++ llvm/test/tools/llvm-symbolizer/data-location.yaml
@@ -0,0 +1,450 @@
+## Show that when "DATA" is used with an address, it forces the found location
+## to be symbolized as data, including the source information.
+
+# RUN: yaml2obj %s -o %t.so
+
+# RUN: llvm-symbolizer 'DATA 0x304d0' 'DATA 0x304d1' 'DATA 0x304d3' \
+# RUN:   'DATA 0x304c0' 'DATA 0x304c8' 'DATA 0x304d4' 'DATA 0x304dc' \
+# RUN:   'DATA 0x304d8' --obj=%t.so | FileCheck %s
+
+# CHECK:  bss_global
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:1
+# CHECK-EMPTY:
+
+## Check that lookups in the middle of the symbol are also resolved correctly.
+# CHECK:  bss_global
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:1
+# CHECK-EMPTY:
+# CHECK:  bss_global
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:1
+# CHECK-EMPTY:
+
+## Now, the remainder of the symbols.
+# CHECK-NEXT: data_global
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:2
+# CHECK-EMPTY:
+# CHECK-NEXT: str
+# CHECK-NEXT: {{[0-9]+}} 8
+# CHECK-NEXT: /tmp/file.cpp:4
+# CHECK-EMPTY:
+# CHECK-NEXT: f()::function_global
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:8
+# CHECK-EMPTY:
+
+## Including the one that includes an addend.
+# CHECK-NEXT: alpha
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:12
+# CHECK-EMPTY:
+# CHECK-NEXT: beta
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:13
+# CHECK-EMPTY:
+
+## Ensure there's still a global that's offset-based.
+# RUN: llvm-dwarfdump --debug-info %t.so | FileCheck %s --check-prefix=OFFSET
+
+# OFFSET: DW_AT_location (DW_OP_addrx 0x4, DW_OP_plus_uconst 0x4)
+
+
+## File below was generated using:
+##
+##   $ clang++ -g -O3 /tmp/file.cpp -shared -fuse-ld=lld -nostdlib \
+## -target aarch64-linux-gnuabi -mllvm -global-merge-ignore-single-use \
+## -o /tmp/file.so
+##
+##  With /tmp/file.cpp as:
+##1: int bss_global;
+##2: int data_global = 2;
+##3:
+##4: const char* str =
+##5: "12345678";
+##6:
+##7: int* f() {
+##8:   static int function_global;
+##9:   return _global;
+##   10: }
+##   11:
+##   12: static int alpha;
+##   13: static int beta;
+##   14: int *f(bool b) { return beta ?  :  }
+##   15:
+##
+## ... then, one can get the offsets using `nm`, like:
+##   $ nm out.so | grep bss_global
+## 38fc B bss_global
+##
+## Note the use of the aarch64 target (with -nostdlib in order to allow linkage
+## without libraries for cross-compilation) as well as -O3 and
+## -global-merge-ignore-single-use. This is a specific combination that makes
+## the compiler emit the `alpha` global variable with a more complex
+## DW_AT_location than just a DW_OP_addr/DW_OP_addrx. In this instance, it
+## outputs a `DW_AT_location  (DW_OP_addrx 0x4, DW_OP_plus_uconst 0x4)`.
+##
+## Ideally, this would be tested by invoking clang directly on a C source file,
+## but unfortunately there's no way to do that for LLVM tests. The other option
+## is to compile IR to an objfile, but llvm-symbolizer doesn't understand that
+## two symbols can have the same address in different sections. In the code
+## above, for example, we'd have bss_global at .bss+0x0, and data_global at
+## .data+0x0, and so the symbolizer would only print one of them. Hence, we have
+## the ugly dso-to-yaml blob below.
+##
+## For now, constant strings don't have a debuginfo entry, and so 

[PATCH] D125840: [Analyzer] Removed extra space from NSErrorChecker debug message and updated relevant tests

[Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdd7233bc67e4: [Analyzer] Remove extra space from 
NSErrorChecker message. (authored by usama54321, committed by Artem Dergachev 
adergac...@apple.com).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125840

Files:
  clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
  clang/test/Analysis/CheckNSError.m
  clang/test/Analysis/incorrect-checker-names.mm


Index: clang/test/Analysis/incorrect-checker-names.mm
===
--- clang/test/Analysis/incorrect-checker-names.mm
+++ clang/test/Analysis/incorrect-checker-names.mm
@@ -107,7 +107,7 @@
 
 void foo(CFErrorRef* error) { // expected-warning{{Function accepting 
CFErrorRef* should have a non-void return value to indicate whether or not an 
error occurred [osx.coreFoundation.CFError]}}
   // FIXME: This shouldn't be tied to a modeling checker.
-  *error = 0; // expected-warning {{Potential null dereference.  According to 
coding standards documented in CoreFoundation/CFError.h the parameter may be 
null [osx.coreFoundation.CFError]}}
+  *error = 0; // expected-warning {{Potential null dereference. According to 
coding standards documented in CoreFoundation/CFError.h the parameter may be 
null [osx.coreFoundation.CFError]}}
 }
 
 @interface A
@@ -116,7 +116,7 @@
 
 @implementation A
 - (void)myMethodWhichMayFail:(NSError **)error {  // 
expected-warning {{Method accepting NSError** should have a non-void return 
value to indicate whether or not an error occurred [osx.cocoa.NSError]}}
-  *error = [NSError errorWithDomain:@"domain" code:1 userInfo:0]; // 
expected-warning {{Potential null dereference.  According to coding standards 
in 'Creating and Returning NSError Objects' the parameter may be null 
[osx.cocoa.NSError]}}
+  *error = [NSError errorWithDomain:@"domain" code:1 userInfo:0]; // 
expected-warning {{Potential null dereference. According to coding standards in 
'Creating and Returning NSError Objects' the parameter may be null 
[osx.cocoa.NSError]}}
 }
 @end
 
Index: clang/test/Analysis/CheckNSError.m
===
--- clang/test/Analysis/CheckNSError.m
+++ clang/test/Analysis/CheckNSError.m
@@ -27,7 +27,7 @@
 
 @implementation A
 - (void)myMethodWhichMayFail:(NSError **)error {   // expected-warning 
{{Method accepting NSError** should have a non-void return value to indicate 
whether or not an error occurred}}
-  *error = [NSError errorWithDomain:@"domain" code:1 userInfo:0]; // 
expected-warning {{Potential null dereference}}
+  *error = [NSError errorWithDomain:@"domain" code:1 userInfo:0]; // 
expected-warning {{Potential null dereference. According to coding standards in 
'Creating and Returning NSError Objects' the parameter may be null 
[osx.cocoa.NSError]}}
 }
 
 - (BOOL)myMethodWhichMayFail2:(NSError **)error {  // no-warning
@@ -50,7 +50,7 @@
 typedef struct __CFError* CFErrorRef;
 
 void foo(CFErrorRef* error) { // expected-warning {{Function accepting 
CFErrorRef* should have a non-void return value to indicate whether or not an 
error occurred}}
-  *error = 0;  // expected-warning {{Potential null dereference}}
+  *error = 0; // expected-warning {{Potential null 
dereference. According to coding standards documented in 
CoreFoundation/CFError.h the parameter may be null 
[osx.coreFoundation.CFError]}}
 }
 
 int f1(CFErrorRef* error) {
@@ -74,7 +74,7 @@
 }
 
 int __attribute__((nonnull(1))) f5(int *x, CFErrorRef *error) {
-  *error = 0; // expected-warning {{Potential null dereference}}
+  *error = 0; // expected-warning {{Potential null dereference. According to 
coding standards documented in CoreFoundation/CFError.h the parameter may be 
null [osx.coreFoundation.CFError]}}
   return 0;
 }
 
Index: clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
@@ -266,7 +266,7 @@
   SmallString<128> Buf;
   llvm::raw_svector_ostream os(Buf);
 
-  os << "Potential null dereference.  According to coding standards ";
+  os << "Potential null dereference. According to coding standards ";
   os << (isNSError
  ? "in 'Creating and Returning NSError Objects' the parameter"
  : "documented in CoreFoundation/CFError.h the parameter");


Index: clang/test/Analysis/incorrect-checker-names.mm
===
--- clang/test/Analysis/incorrect-checker-names.mm
+++ clang/test/Analysis/incorrect-checker-names.mm
@@ -107,7 +107,7 @@
 
 void foo(CFErrorRef* error) { // expected-warning{{Function accepting CFErrorRef* should have a non-void return value to indicate whether or not an error occurred 

[clang] dd7233b - [Analyzer] Remove extra space from NSErrorChecker message.

[Artem Dergachev via cfe-commits]

Author: Usama Hameed
Date: 2022-05-18T14:35:12-07:00
New Revision: dd7233bc67e45106c4625df7d20391798f31d4a7

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

LOG: [Analyzer] Remove extra space from NSErrorChecker message.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
clang/test/Analysis/CheckNSError.m
clang/test/Analysis/incorrect-checker-names.mm

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
index 79b4f40417264..33242b827bacd 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
@@ -266,7 +266,7 @@ void 
NSOrCFErrorDerefChecker::checkEvent(ImplicitNullDerefEvent event) const {
   SmallString<128> Buf;
   llvm::raw_svector_ostream os(Buf);
 
-  os << "Potential null dereference.  According to coding standards ";
+  os << "Potential null dereference. According to coding standards ";
   os << (isNSError
  ? "in 'Creating and Returning NSError Objects' the parameter"
  : "documented in CoreFoundation/CFError.h the parameter");

diff  --git a/clang/test/Analysis/CheckNSError.m 
b/clang/test/Analysis/CheckNSError.m
index 510f11c055795..47a6b432cbe0c 100644
--- a/clang/test/Analysis/CheckNSError.m
+++ b/clang/test/Analysis/CheckNSError.m
@@ -27,7 +27,7 @@ - (BOOL)myMethodWhichMayFail4:(NSError **)error 
__attribute__((nonnull));
 
 @implementation A
 - (void)myMethodWhichMayFail:(NSError **)error {   // expected-warning 
{{Method accepting NSError** should have a non-void return value to indicate 
whether or not an error occurred}}
-  *error = [NSError errorWithDomain:@"domain" code:1 userInfo:0]; // 
expected-warning {{Potential null dereference}}
+  *error = [NSError errorWithDomain:@"domain" code:1 userInfo:0]; // 
expected-warning {{Potential null dereference. According to coding standards in 
'Creating and Returning NSError Objects' the parameter may be null 
[osx.cocoa.NSError]}}
 }
 
 - (BOOL)myMethodWhichMayFail2:(NSError **)error {  // no-warning
@@ -50,7 +50,7 @@ - (BOOL)myMethodWhichMayFail4:(NSError **)error { 
// no-warning
 typedef struct __CFError* CFErrorRef;
 
 void foo(CFErrorRef* error) { // expected-warning {{Function accepting 
CFErrorRef* should have a non-void return value to indicate whether or not an 
error occurred}}
-  *error = 0;  // expected-warning {{Potential null dereference}}
+  *error = 0; // expected-warning {{Potential null 
dereference. According to coding standards documented in 
CoreFoundation/CFError.h the parameter may be null 
[osx.coreFoundation.CFError]}}
 }
 
 int f1(CFErrorRef* error) {
@@ -74,7 +74,7 @@ int __attribute__((nonnull)) f4(CFErrorRef *error) {
 }
 
 int __attribute__((nonnull(1))) f5(int *x, CFErrorRef *error) {
-  *error = 0; // expected-warning {{Potential null dereference}}
+  *error = 0; // expected-warning {{Potential null dereference. According to 
coding standards documented in CoreFoundation/CFError.h the parameter may be 
null [osx.coreFoundation.CFError]}}
   return 0;
 }
 

diff  --git a/clang/test/Analysis/incorrect-checker-names.mm 
b/clang/test/Analysis/incorrect-checker-names.mm
index bf7c6c071153a..12cab9be1df18 100644
--- a/clang/test/Analysis/incorrect-checker-names.mm
+++ b/clang/test/Analysis/incorrect-checker-names.mm
@@ -107,7 +107,7 @@ + (id)errorWithDomain:(NSString *)domain 
code:(NSInteger)code userInfo:(NSDictio
 
 void foo(CFErrorRef* error) { // expected-warning{{Function accepting 
CFErrorRef* should have a non-void return value to indicate whether or not an 
error occurred [osx.coreFoundation.CFError]}}
   // FIXME: This shouldn't be tied to a modeling checker.
-  *error = 0; // expected-warning {{Potential null dereference.  According to 
coding standards documented in CoreFoundation/CFError.h the parameter may be 
null [osx.coreFoundation.CFError]}}
+  *error = 0; // expected-warning {{Potential null dereference. According to 
coding standards documented in CoreFoundation/CFError.h the parameter may be 
null [osx.coreFoundation.CFError]}}
 }
 
 @interface A
@@ -116,7 +116,7 @@ - (void)myMethodWhichMayFail:(NSError **)error;
 
 @implementation A
 - (void)myMethodWhichMayFail:(NSError **)error {  // 
expected-warning {{Method accepting NSError** should have a non-void return 
value to indicate whether or not an error occurred [osx.cocoa.NSError]}}
-  *error = [NSError errorWithDomain:@"domain" code:1 userInfo:0]; // 
expected-warning {{Potential null dereference.  According to coding standards 
in 'Creating and Returning NSError Objects' the parameter may be null 

[PATCH] D125930: [clangd] WIP: Experimental support for "IWYU pragma: export"

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125930

Files:
  clang-tools-extra/clangd/Headers.cpp
  clang-tools-extra/clangd/Headers.h
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/clangd/unittests/HeadersTests.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -582,6 +582,10 @@
   TestTU TU;
   TU.Code = R"cpp(
 #include "foo.h"
+
+void foo() {
+  bar();
+}
 )cpp";
   TU.AdditionalFiles["foo.h"] = R"cpp(
 #ifndef FOO_H
@@ -600,8 +604,6 @@
   findReferencedLocations(AST), AST.getIncludeStructure(),
   AST.getCanonicalIncludes(), AST.getSourceManager());
   EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
-  // FIXME: This is not correct: foo.h is unused but is not diagnosed as such
-  // because we ignore headers with IWYU export pragmas for now.
   EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
 }
 
Index: clang-tools-extra/clangd/unittests/HeadersTests.cpp
===
--- clang-tools-extra/clangd/unittests/HeadersTests.cpp
+++ clang-tools-extra/clangd/unittests/HeadersTests.cpp
@@ -258,18 +258,6 @@
directive(tok::pp_include_next)));
 }
 
-TEST_F(HeadersTest, IWYUPragmaKeep) {
-  FS.Files[MainFile] = R"cpp(
-#include "bar.h" // IWYU pragma: keep
-#include "foo.h"
-)cpp";
-
-  EXPECT_THAT(
-  collectIncludes().MainFileIncludes,
-  UnorderedElementsAre(AllOf(written("\"foo.h\""), hasPragmaKeep(false)),
-   AllOf(written("\"bar.h\""), hasPragmaKeep(true;
-}
-
 TEST_F(HeadersTest, InsertInclude) {
   std::string Path = testPath("sub/bar.h");
   FS.Files[Path] = "";
@@ -418,31 +406,43 @@
   EXPECT_FALSE(Includes.isSelfContained(getID("pp_depend.h", Includes)));
 }
 
-TEST_F(HeadersTest, HasIWYUPragmas) {
+TEST_F(HeadersTest, IWYUPragmaKeep) {
   FS.Files[MainFile] = R"cpp(
-#include "export.h"
-#include "begin_exports.h"
-#include "none.h"
+#include "bar.h" // IWYU pragma: keep
+#include "foo.h"
 )cpp";
-  FS.Files["export.h"] = R"cpp(
-#pragma once
-#include "none.h" // IWYU pragma: export
+
+  EXPECT_THAT(
+  collectIncludes().MainFileIncludes,
+  UnorderedElementsAre(AllOf(written("\"foo.h\""), hasPragmaKeep(false)),
+   AllOf(written("\"bar.h\""), hasPragmaKeep(true;
+}
+
+TEST_F(HeadersTest, IWYUPragmaExport) {
+  FS.Files[MainFile] = R"cpp(
+// WHATEVER COMMENT
+#include "umbrella.h"
+
+void foo() {
+  bar();
+}
 )cpp";
-  FS.Files["begin_exports.h"] = R"cpp(
+  FS.Files["umbrella.h"] = R"cpp(
 #pragma once
-// IWYU pragma: begin_exports
-#include "none.h"
-// IWYU pragma: end_exports
+
+#include "lib.h" // IWYU pragma: export
 )cpp";
-  FS.Files["none.h"] = R"cpp(
+  FS.Files["lib.h"] = R"cpp(
 #pragma once
-// Not a pragma.
+
+void bar() {}
 )cpp";
 
   auto Includes = collectIncludes();
-  EXPECT_TRUE(Includes.hasIWYUExport(getID("export.h", Includes)));
-  EXPECT_TRUE(Includes.hasIWYUExport(getID("begin_exports.h", Includes)));
-  EXPECT_FALSE(Includes.hasIWYUExport(getID("none.h", Includes)));
+  auto LibExporters = Includes.exporters(getID("lib.h", Includes));
+  ASSERT_TRUE(LibExporters);
+  EXPECT_THAT(*LibExporters,
+  UnorderedElementsAre(getID("umbrella.h", Includes)));
 }
 
 } // namespace
Index: clang-tools-extra/clangd/IncludeCleaner.h
===
--- clang-tools-extra/clangd/IncludeCleaner.h
+++ clang-tools-extra/clangd/IncludeCleaner.h
@@ -67,15 +67,15 @@
 /// Retrieves IDs of all files containing SourceLocations from \p Locs.
 /// The output only includes things SourceManager sees as files (not macro IDs).
 /// This can include ,  etc that are not true files.
-/// \p HeaderResponsible returns the public header that should be included given
-/// symbols from a file with the given FileID (example: public headers should be
-/// preferred to non self-contained and private headers).
-/// \p UmbrellaHeader returns the public public header is responsible for
-/// providing symbols from a file with the given FileID (example: MyType.h
-/// should be included instead of MyType_impl.h).
+/// \p HeadersResponsible returns the public header that should be included
+/// given symbols from a file with the given FileID (example: public headers
+/// should be preferred to non self-contained and private 

[PATCH] D125667: [pseudo] A basic implementation of compiling cxx grammar at build time.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/pseudo/CMakeLists.txt:1
+set(CLANG_PSEUDO_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
+

sammccall wrote:
> I think these variables shared across CMakeLists.txt files generally add more 
> confusion than value, it doesn't seem to be needed here - can we use relative 
> paths instead?
removed



Comment at: clang-tools-extra/pseudo/include/CMakeLists.txt:1
+# We put an empty cmake file here so that cmake can create an include directory
+# in the build directory, the include directory is the home for generated 
source

sammccall wrote:
> it seems to me the build rules for files that end up in the include/ 
> directory should go here?
sounds a good idea, move the cxx_gen.cmake to this file.



Comment at: clang-tools-extra/pseudo/lib/CMakeLists.txt:3
 
-add_clang_library(clangPseudo
+# Needed by LLVM's CMake checks because this file defines multiple targets.
+set(LLVM_OPTIONAL_SOURCES

sammccall wrote:
> We do have a layering relationship here, and a requirement to keep the 
> "grammar" dependencies small - should we move it into a subdirectory?
split it to a grammar subdirectory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125667

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

[PATCH] D125667: [pseudo] A basic implementation of compiling cxx grammar at build time.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 430506.
hokein added a comment.

format fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125667

Files:
  clang-tools-extra/pseudo/CMakeLists.txt
  clang-tools-extra/pseudo/gen/CMakeLists.txt
  clang-tools-extra/pseudo/gen/Main.cpp
  clang-tools-extra/pseudo/include/CMakeLists.txt
  clang-tools-extra/pseudo/include/clang-pseudo/cxx/CXX.h
  clang-tools-extra/pseudo/lib/CMakeLists.txt
  clang-tools-extra/pseudo/lib/Grammar.cpp
  clang-tools-extra/pseudo/lib/GrammarBNF.cpp
  clang-tools-extra/pseudo/lib/LRGraph.cpp
  clang-tools-extra/pseudo/lib/LRTable.cpp
  clang-tools-extra/pseudo/lib/LRTableBuild.cpp
  clang-tools-extra/pseudo/lib/cxx/CMakeLists.txt
  clang-tools-extra/pseudo/lib/cxx/CXX.cpp
  clang-tools-extra/pseudo/lib/grammar/CMakeLists.txt
  clang-tools-extra/pseudo/lib/grammar/Grammar.cpp
  clang-tools-extra/pseudo/lib/grammar/GrammarBNF.cpp
  clang-tools-extra/pseudo/lib/grammar/LRGraph.cpp
  clang-tools-extra/pseudo/lib/grammar/LRTable.cpp
  clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp

Index: clang-tools-extra/pseudo/lib/LRTableBuild.cpp
===
--- /dev/null
+++ clang-tools-extra/pseudo/lib/LRTableBuild.cpp
@@ -1,146 +0,0 @@
-//===--- LRTableBuild.cpp - Build a LRTable from LRGraph -*- C++-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "clang-pseudo/Grammar.h"
-#include "clang-pseudo/LRGraph.h"
-#include "clang-pseudo/LRTable.h"
-#include "clang/Basic/TokenKinds.h"
-#include 
-
-namespace llvm {
-template <> struct DenseMapInfo {
-  using Entry = clang::pseudo::LRTable::Entry;
-  static inline Entry getEmptyKey() {
-static Entry E{static_cast(-1), 0,
-   clang::pseudo::LRTable::Action::sentinel()};
-return E;
-  }
-  static inline Entry getTombstoneKey() {
-static Entry E{static_cast(-2), 0,
-   clang::pseudo::LRTable::Action::sentinel()};
-return E;
-  }
-  static unsigned getHashValue(const Entry ) {
-return llvm::hash_combine(I.State, I.Symbol, I.Act.opaque());
-  }
-  static bool isEqual(const Entry , const Entry ) {
-return LHS.State == RHS.State && LHS.Symbol == RHS.Symbol &&
-   LHS.Act == RHS.Act;
-  }
-};
-} // namespace llvm
-
-namespace clang {
-namespace pseudo {
-
-class LRTable::Builder {
-public:
-  Builder(llvm::ArrayRef> StartStates)
-  : StartStates(StartStates) {}
-
-  bool insert(Entry E) { return Entries.insert(std::move(E)).second; }
-  LRTable build(const GrammarTable ) && {
-// E.g. given the following parsing table with 3 states and 3 terminals:
-//
-//ab c
-// +---++---+-+
-// |state0 || s0,r0 | |
-// |state1 | acc|   | |
-// |state2 ||  r1   | |
-// +---++---+-+
-//
-// The final LRTable:
-//  - TerminalOffset: [a] = 0, [b] = 1, [c] = 4, [d] = 4 (d is a sentinel)
-//  -  States: [ 1,0,  0,  2]
-//Actions: [ acc, s0, r0, r1]
-//   ~~~ corresponding range for terminal a
-//~~ corresponding range for terminal b
-// First step, we sort all entries by (Symbol, State, Action).
-std::vector Sorted(Entries.begin(), Entries.end());
-llvm::sort(Sorted, [](const Entry , const Entry ) {
-  return std::forward_as_tuple(L.Symbol, L.State, L.Act.opaque()) <
- std::forward_as_tuple(R.Symbol, R.State, R.Act.opaque());
-});
-
-LRTable Table;
-Table.Actions.reserve(Sorted.size());
-Table.States.reserve(Sorted.size());
-// We are good to finalize the States and Actions.
-for (const auto  : Sorted) {
-  Table.Actions.push_back(E.Act);
-  Table.States.push_back(E.State);
-}
-// Initialize the terminal and nonterminal offset, all ranges are empty by
-// default.
-Table.TerminalOffset = std::vector(GT.Terminals.size() + 1, 0);
-Table.NontermOffset = std::vector(GT.Nonterminals.size() + 1, 0);
-size_t SortedIndex = 0;
-for (SymbolID NonterminalID = 0; NonterminalID < Table.NontermOffset.size();
- ++NonterminalID) {
-  Table.NontermOffset[NonterminalID] = SortedIndex;
-  while (SortedIndex < Sorted.size() &&
- Sorted[SortedIndex].Symbol == NonterminalID)
-++SortedIndex;
-}
-for (size_t Terminal = 0; Terminal < Table.TerminalOffset.size();
- ++Terminal) {
-  Table.TerminalOffset[Terminal] = SortedIndex;
-  while (SortedIndex < Sorted.size() &&
- Sorted[SortedIndex].Symbol ==
- tokenSymbol(static_cast(Terminal)))
- 

[PATCH] D125667: [pseudo] A basic implementation of compiling cxx grammar at build time.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 430504.
hokein marked 8 inline comments as done.
hokein added a comment.
Herald added a subscriber: mgrang.

Address comments:

- split to a grammar subdirectory
- remove cxx_gen.cmake, move it to include/CMakeLists.txt
- rename cxx -> CXX


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125667

Files:
  clang-tools-extra/pseudo/CMakeLists.txt
  clang-tools-extra/pseudo/gen/CMakeLists.txt
  clang-tools-extra/pseudo/gen/Main.cpp
  clang-tools-extra/pseudo/include/CMakeLists.txt
  clang-tools-extra/pseudo/include/clang-pseudo/cxx/CXX.h
  clang-tools-extra/pseudo/lib/CMakeLists.txt
  clang-tools-extra/pseudo/lib/Grammar.cpp
  clang-tools-extra/pseudo/lib/GrammarBNF.cpp
  clang-tools-extra/pseudo/lib/LRGraph.cpp
  clang-tools-extra/pseudo/lib/LRTable.cpp
  clang-tools-extra/pseudo/lib/LRTableBuild.cpp
  clang-tools-extra/pseudo/lib/cxx/CMakeLists.txt
  clang-tools-extra/pseudo/lib/cxx/CXX.cpp
  clang-tools-extra/pseudo/lib/grammar/CMakeLists.txt
  clang-tools-extra/pseudo/lib/grammar/Grammar.cpp
  clang-tools-extra/pseudo/lib/grammar/GrammarBNF.cpp
  clang-tools-extra/pseudo/lib/grammar/LRGraph.cpp
  clang-tools-extra/pseudo/lib/grammar/LRTable.cpp
  clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp

Index: clang-tools-extra/pseudo/lib/LRTableBuild.cpp
===
--- /dev/null
+++ clang-tools-extra/pseudo/lib/LRTableBuild.cpp
@@ -1,146 +0,0 @@
-//===--- LRTableBuild.cpp - Build a LRTable from LRGraph -*- C++-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "clang-pseudo/Grammar.h"
-#include "clang-pseudo/LRGraph.h"
-#include "clang-pseudo/LRTable.h"
-#include "clang/Basic/TokenKinds.h"
-#include 
-
-namespace llvm {
-template <> struct DenseMapInfo {
-  using Entry = clang::pseudo::LRTable::Entry;
-  static inline Entry getEmptyKey() {
-static Entry E{static_cast(-1), 0,
-   clang::pseudo::LRTable::Action::sentinel()};
-return E;
-  }
-  static inline Entry getTombstoneKey() {
-static Entry E{static_cast(-2), 0,
-   clang::pseudo::LRTable::Action::sentinel()};
-return E;
-  }
-  static unsigned getHashValue(const Entry ) {
-return llvm::hash_combine(I.State, I.Symbol, I.Act.opaque());
-  }
-  static bool isEqual(const Entry , const Entry ) {
-return LHS.State == RHS.State && LHS.Symbol == RHS.Symbol &&
-   LHS.Act == RHS.Act;
-  }
-};
-} // namespace llvm
-
-namespace clang {
-namespace pseudo {
-
-class LRTable::Builder {
-public:
-  Builder(llvm::ArrayRef> StartStates)
-  : StartStates(StartStates) {}
-
-  bool insert(Entry E) { return Entries.insert(std::move(E)).second; }
-  LRTable build(const GrammarTable ) && {
-// E.g. given the following parsing table with 3 states and 3 terminals:
-//
-//ab c
-// +---++---+-+
-// |state0 || s0,r0 | |
-// |state1 | acc|   | |
-// |state2 ||  r1   | |
-// +---++---+-+
-//
-// The final LRTable:
-//  - TerminalOffset: [a] = 0, [b] = 1, [c] = 4, [d] = 4 (d is a sentinel)
-//  -  States: [ 1,0,  0,  2]
-//Actions: [ acc, s0, r0, r1]
-//   ~~~ corresponding range for terminal a
-//~~ corresponding range for terminal b
-// First step, we sort all entries by (Symbol, State, Action).
-std::vector Sorted(Entries.begin(), Entries.end());
-llvm::sort(Sorted, [](const Entry , const Entry ) {
-  return std::forward_as_tuple(L.Symbol, L.State, L.Act.opaque()) <
- std::forward_as_tuple(R.Symbol, R.State, R.Act.opaque());
-});
-
-LRTable Table;
-Table.Actions.reserve(Sorted.size());
-Table.States.reserve(Sorted.size());
-// We are good to finalize the States and Actions.
-for (const auto  : Sorted) {
-  Table.Actions.push_back(E.Act);
-  Table.States.push_back(E.State);
-}
-// Initialize the terminal and nonterminal offset, all ranges are empty by
-// default.
-Table.TerminalOffset = std::vector(GT.Terminals.size() + 1, 0);
-Table.NontermOffset = std::vector(GT.Nonterminals.size() + 1, 0);
-size_t SortedIndex = 0;
-for (SymbolID NonterminalID = 0; NonterminalID < Table.NontermOffset.size();
- ++NonterminalID) {
-  Table.NontermOffset[NonterminalID] = SortedIndex;
-  while (SortedIndex < Sorted.size() &&
- Sorted[SortedIndex].Symbol == NonterminalID)
-++SortedIndex;
-}
-for (size_t Terminal = 0; Terminal < Table.TerminalOffset.size();
- ++Terminal) {
- 

[PATCH] D124806: [clang-tidy] add support for Demorgan conversions to readability-simplify-bool-expr

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added a comment.

In D124806#3523228 , @njames93 wrote:

> In D124806#3523109 , 
> @LegalizeAdulthood wrote:
>
>> LGTM!  Thanks for taking my idea and implementing it much better than what I 
>> had `:)`.
>
> Cheers.
> Can I ask why you feel that this should be behind an option?

When I've brought it up in manual code reviews before, there were some 
objections.
That's why I thought it was worth adding an option to disable it.  Many people 
aren't
even familiar with DeMorgan's Theorem/Law and might be afraid that the 
simplification
changes semantics, although the underlying github issue shows that this is not 
the case.

> Also do you think there is merit in handling the case `!(A && B)` -> `!A || 
> !B` obviously that could be behind an option.

The reason I proposed the simplification `!(!A && B)` -> `A || !B` is because 
the former employs
"double negatives" and provides a stumbling block for readability.  All 
readability "checks"
are somewhat influenced by opinion and style to a certain extent, but I think 
the avoid
"double negative" idea is widely accepted.

In the case of `!(A && B)` -> `!A || !B`, no double negative is involved and 
one could argue
that it went from one negative to two, which is why I didn't include that in my 
original
proposal.  The "goodness" of it just doesn't seem universal and personally if 
the check
grew to enable that case, I'd want an option to disable that behavior `:)`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124806

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

[PATCH] D125770: [clang-tidy] modernize-deprecated-headers should ignore system headers

[Richard via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125770

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

[PATCH] D123534: [dwarf] Emit a DIGlobalVariable for constant strings.

[Mitch Phillips via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7aa1fa0a0a07: Reland [dwarf] Emit a DIGlobalVariable 
for constant strings. (authored by hctim).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123534

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/debug-info-variables.c
  clang/test/VFS/external-names.c
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
  llvm/test/Assembler/invalid-diglobalvariable-missing-name.ll
  llvm/test/DebugInfo/COFF/global-no-strings.ll

Index: llvm/test/DebugInfo/COFF/global-no-strings.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/COFF/global-no-strings.ll
@@ -0,0 +1,59 @@
+; RUN: llc < %s | FileCheck %s
+; RUN: llc < %s -filetype=obj | llvm-readobj - --codeview | FileCheck %s
+
+; This test ensures that no debuginfo is emitted for the constant "123456789"
+; string. These global variables have debug expressions because DWARF has the
+; ability to tie them to a file name and line number, but this isn't possible
+; in CodeView, so we make sure it's omitted to save file size.
+;
+; The various CodeView outputs should have a description of "my_string", but not
+; the string contents itself.
+
+; C++ source to regenerate:
+; $ cat a.cpp
+; char* my_string =
+;   "12345679";
+;
+; $ clang-cl a.cpp /GS- /Z7 /GR- /clang:-S /clang:-emit-llvm
+
+; CHECK-NOT: S_LDATA
+; CHECK: S_GDATA
+; CHECK-NOT: S_LDATA
+; CHECK-NOT: S_GDATA
+
+; ModuleID = '/tmp/a.c'
+source_filename = "/tmp/a.c"
+target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc19.20.0"
+
+$"??_C@_08HCBFHPJA@12345679?$AA@" = comdat any
+
+@"??_C@_08HCBFHPJA@12345679?$AA@" = linkonce_odr dso_local unnamed_addr constant [9 x i8] c"12345679\00", comdat, align 1, !dbg !0
+@my_string = dso_local global ptr @"??_C@_08HCBFHPJA@12345679?$AA@", align 8, !dbg !7
+
+!llvm.dbg.cu = !{!9}
+!llvm.linker.options = !{!13, !14}
+!llvm.module.flags = !{!15, !16, !17, !18, !19}
+!llvm.ident = !{!20}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(scope: null, file: !2, line: 2, type: !3, isLocal: true, isDefinition: true)
+!2 = !DIFile(filename: "/tmp/a.c", directory: "", checksumkind: CSK_MD5, checksum: "b972961d64de3c90497767110ab58eb6")
+!3 = !DICompositeType(tag: DW_TAG_array_type, baseType: !4, size: 72, elements: !5)
+!4 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+!5 = !{!6}
+!6 = !DISubrange(count: 9)
+!7 = !DIGlobalVariableExpression(var: !8, expr: !DIExpression())
+!8 = distinct !DIGlobalVariable(name: "my_string", scope: !9, file: !2, line: 1, type: !12, isLocal: false, isDefinition: true)
+!9 = distinct !DICompileUnit(language: DW_LANG_C99, file: !10, producer: "clang version 15.0.0 (https://github.com/llvm/llvm-project.git 7c1c0849f8a1a6f1bf5f5b554484bbf8b0debd0a)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, globals: !11, splitDebugInlining: false, nameTableKind: None)
+!10 = !DIFile(filename: "/tmp/a.c", directory: "/usr/local/google/home/mitchp/llvm-build/opt", checksumkind: CSK_MD5, checksum: "b972961d64de3c90497767110ab58eb6")
+!11 = !{!0, !7}
+!12 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !4, size: 64)
+!13 = !{!"/DEFAULTLIB:libcmt.lib"}
+!14 = !{!"/DEFAULTLIB:oldnames.lib"}
+!15 = !{i32 2, !"CodeView", i32 1}
+!16 = !{i32 2, !"Debug Info Version", i32 3}
+!17 = !{i32 1, !"wchar_size", i32 2}
+!18 = !{i32 7, !"PIC Level", i32 2}
+!19 = !{i32 7, !"uwtable", i32 2}
+!20 = !{!"clang version 15.0.0 (https://github.com/llvm/llvm-project.git 7c1c0849f8a1a6f1bf5f5b554484bbf8b0debd0a)"}
Index: llvm/test/Assembler/invalid-diglobalvariable-missing-name.ll
===
--- llvm/test/Assembler/invalid-diglobalvariable-missing-name.ll
+++ /dev/null
@@ -1,4 +0,0 @@
-; RUN: not llvm-as < %s -disable-output 2>&1 | FileCheck %s
-
-; CHECK: :[[@LINE+1]]:24: error: missing required field 'name'
-!0 = !DIGlobalVariable()
Index: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
@@ -165,7 +165,9 @@
   } else {
 DeclContext = GV->getScope();
 // Add name and type.
-addString(*VariableDIE, dwarf::DW_AT_name, GV->getDisplayName());
+StringRef DisplayName = GV->getDisplayName();
+if (!DisplayName.empty())
+  addString(*VariableDIE, dwarf::DW_AT_name, GV->getDisplayName());
 if (GTy)
   

[clang] 7aa1fa0 - Reland "[dwarf] Emit a DIGlobalVariable for constant strings."

[Mitch Phillips via cfe-commits]

Author: Mitch Phillips
Date: 2022-05-18T13:56:45-07:00
New Revision: 7aa1fa0a0a07f7949d2d77c099aab43cf9b75a91

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

LOG: Reland "[dwarf] Emit a DIGlobalVariable for constant strings."

An upcoming patch will extend llvm-symbolizer to provide the source line
information for global variables. The goal is to move AddressSanitizer
off of internal debug info for symbolization onto the DWARF standard
(and doing a clean-up in the process). Currently, ASan reports the line
information for constant strings if a memory safety bug happens around
them. We want to keep this behaviour, so we need to emit debuginfo for
these variables as well.

Reviewed By: dblaikie, rnk, aprantl

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

Added: 
clang/test/CodeGen/debug-info-variables.c
llvm/test/DebugInfo/COFF/global-no-strings.ll

Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CGDebugInfo.h
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/VFS/external-names.c
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

Removed: 
llvm/test/Assembler/invalid-diglobalvariable-missing-name.ll



diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 3d73bfb8ce79..753427029441 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -5132,7 +5132,7 @@ std::string CGDebugInfo::GetName(const Decl *D, bool 
Qualified) const {
 return Name;
   codegenoptions::DebugTemplateNamesKind TemplateNamesKind =
   CGM.getCodeGenOpts().getDebugSimpleTemplateNames();
-  
+
   if (!CGM.getCodeGenOpts().hasReducedDebugInfo())
 TemplateNamesKind = codegenoptions::DebugTemplateNamesKind::Full;
 
@@ -5459,6 +5459,21 @@ void CGDebugInfo::EmitGlobalAlias(const 
llvm::GlobalValue *GV,
   ImportedDeclCache[GD.getCanonicalDecl().getDecl()].reset(ImportDI);
 }
 
+void CGDebugInfo::AddStringLiteralDebugInfo(llvm::GlobalVariable *GV,
+const StringLiteral *S) {
+  SourceLocation Loc = S->getStrTokenLoc(0);
+  PresumedLoc PLoc = CGM.getContext().getSourceManager().getPresumedLoc(Loc);
+  if (!PLoc.isValid())
+return;
+
+  llvm::DIFile *File = getOrCreateFile(Loc);
+  llvm::DIGlobalVariableExpression *Debug =
+  DBuilder.createGlobalVariableExpression(
+  nullptr, StringRef(), StringRef(), getOrCreateFile(Loc),
+  getLineNumber(Loc), getOrCreateType(S->getType(), File), true);
+  GV->addDebugInfo(Debug);
+}
+
 llvm::DIScope *CGDebugInfo::getCurrentContextDescriptor(const Decl *D) {
   if (!LexicalBlockStack.empty())
 return LexicalBlockStack.back();

diff  --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h
index 8984f3eb1d7a..38e3fa5b2fa9 100644
--- a/clang/lib/CodeGen/CGDebugInfo.h
+++ b/clang/lib/CodeGen/CGDebugInfo.h
@@ -533,6 +533,14 @@ class CGDebugInfo {
   /// Emit an @import declaration.
   void EmitImportDecl(const ImportDecl );
 
+  /// DebugInfo isn't attached to string literals by default. While certain
+  /// aspects of debuginfo aren't useful for string literals (like a name), 
it's
+  /// nice to be able to symbolize the line and column information. This is
+  /// especially useful for sanitizers, as it allows symbolization of
+  /// heap-buffer-overflows on constant strings.
+  void AddStringLiteralDebugInfo(llvm::GlobalVariable *GV,
+ const StringLiteral *S);
+
   /// Emit C++ namespace alias.
   llvm::DIImportedEntity *EmitNamespaceAlias(const NamespaceAliasDecl );
 

diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index f8bf210dc0e2..703cf4edf5f5 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -5670,6 +5670,11 @@ CodeGenModule::GetAddrOfConstantStringFromLiteral(const 
StringLiteral *S,
   }
 
   auto GV = GenerateStringLiteral(C, LT, *this, GlobalVariableName, Alignment);
+
+  CGDebugInfo *DI = getModuleDebugInfo();
+  if (DI && getCodeGenOpts().hasReducedDebugInfo())
+DI->AddStringLiteralDebugInfo(GV, S);
+
   if (Entry)
 *Entry = GV;
 

diff  --git a/clang/test/CodeGen/debug-info-variables.c 
b/clang/test/CodeGen/debug-info-variables.c
new file mode 100644
index ..8ec60ff7c1d9
--- /dev/null
+++ b/clang/test/CodeGen/debug-info-variables.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -debug-info-kind=standalone -S -emit-llvm -o - | 
FileCheck %s
+
+// CHECK: DIGlobalVariable(name: "global",{{.*}} line: [[@LINE+1]]
+int global = 42;
+
+// CHECK: DIGlobalVariable({{.*}}line: [[@LINE+4]],{{.*}} type: 
[[TYPEID:![0-9]+]]
+// CHECK: [[TYPEID]] = 

[PATCH] D123534: [dwarf] Emit a DIGlobalVariable for constant strings.

[Mitch Phillips via Phabricator via cfe-commits]

hctim updated this revision to Diff 430494.
hctim added a comment.

Fix the fuchsia windows bot by making the DILocalVariables in the test 
order-independent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123534

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/debug-info-variables.c
  clang/test/VFS/external-names.c
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
  llvm/test/Assembler/invalid-diglobalvariable-missing-name.ll
  llvm/test/DebugInfo/COFF/global-no-strings.ll

Index: llvm/test/DebugInfo/COFF/global-no-strings.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/COFF/global-no-strings.ll
@@ -0,0 +1,59 @@
+; RUN: llc < %s | FileCheck %s
+; RUN: llc < %s -filetype=obj | llvm-readobj - --codeview | FileCheck %s
+
+; This test ensures that no debuginfo is emitted for the constant "123456789"
+; string. These global variables have debug expressions because DWARF has the
+; ability to tie them to a file name and line number, but this isn't possible
+; in CodeView, so we make sure it's omitted to save file size.
+;
+; The various CodeView outputs should have a description of "my_string", but not
+; the string contents itself.
+
+; C++ source to regenerate:
+; $ cat a.cpp
+; char* my_string =
+;   "12345679";
+;
+; $ clang-cl a.cpp /GS- /Z7 /GR- /clang:-S /clang:-emit-llvm
+
+; CHECK-NOT: S_LDATA
+; CHECK: S_GDATA
+; CHECK-NOT: S_LDATA
+; CHECK-NOT: S_GDATA
+
+; ModuleID = '/tmp/a.c'
+source_filename = "/tmp/a.c"
+target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc19.20.0"
+
+$"??_C@_08HCBFHPJA@12345679?$AA@" = comdat any
+
+@"??_C@_08HCBFHPJA@12345679?$AA@" = linkonce_odr dso_local unnamed_addr constant [9 x i8] c"12345679\00", comdat, align 1, !dbg !0
+@my_string = dso_local global ptr @"??_C@_08HCBFHPJA@12345679?$AA@", align 8, !dbg !7
+
+!llvm.dbg.cu = !{!9}
+!llvm.linker.options = !{!13, !14}
+!llvm.module.flags = !{!15, !16, !17, !18, !19}
+!llvm.ident = !{!20}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(scope: null, file: !2, line: 2, type: !3, isLocal: true, isDefinition: true)
+!2 = !DIFile(filename: "/tmp/a.c", directory: "", checksumkind: CSK_MD5, checksum: "b972961d64de3c90497767110ab58eb6")
+!3 = !DICompositeType(tag: DW_TAG_array_type, baseType: !4, size: 72, elements: !5)
+!4 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+!5 = !{!6}
+!6 = !DISubrange(count: 9)
+!7 = !DIGlobalVariableExpression(var: !8, expr: !DIExpression())
+!8 = distinct !DIGlobalVariable(name: "my_string", scope: !9, file: !2, line: 1, type: !12, isLocal: false, isDefinition: true)
+!9 = distinct !DICompileUnit(language: DW_LANG_C99, file: !10, producer: "clang version 15.0.0 (https://github.com/llvm/llvm-project.git 7c1c0849f8a1a6f1bf5f5b554484bbf8b0debd0a)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, globals: !11, splitDebugInlining: false, nameTableKind: None)
+!10 = !DIFile(filename: "/tmp/a.c", directory: "/usr/local/google/home/mitchp/llvm-build/opt", checksumkind: CSK_MD5, checksum: "b972961d64de3c90497767110ab58eb6")
+!11 = !{!0, !7}
+!12 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !4, size: 64)
+!13 = !{!"/DEFAULTLIB:libcmt.lib"}
+!14 = !{!"/DEFAULTLIB:oldnames.lib"}
+!15 = !{i32 2, !"CodeView", i32 1}
+!16 = !{i32 2, !"Debug Info Version", i32 3}
+!17 = !{i32 1, !"wchar_size", i32 2}
+!18 = !{i32 7, !"PIC Level", i32 2}
+!19 = !{i32 7, !"uwtable", i32 2}
+!20 = !{!"clang version 15.0.0 (https://github.com/llvm/llvm-project.git 7c1c0849f8a1a6f1bf5f5b554484bbf8b0debd0a)"}
Index: llvm/test/Assembler/invalid-diglobalvariable-missing-name.ll
===
--- llvm/test/Assembler/invalid-diglobalvariable-missing-name.ll
+++ /dev/null
@@ -1,4 +0,0 @@
-; RUN: not llvm-as < %s -disable-output 2>&1 | FileCheck %s
-
-; CHECK: :[[@LINE+1]]:24: error: missing required field 'name'
-!0 = !DIGlobalVariable()
Index: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
@@ -165,7 +165,9 @@
   } else {
 DeclContext = GV->getScope();
 // Add name and type.
-addString(*VariableDIE, dwarf::DW_AT_name, GV->getDisplayName());
+StringRef DisplayName = GV->getDisplayName();
+if (!DisplayName.empty())
+  addString(*VariableDIE, dwarf::DW_AT_name, GV->getDisplayName());
 if (GTy)
   addType(*VariableDIE, GTy);
 
Index: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

[PATCH] D125094: [ARM][Thumb] Command-line option to ensure AAPCS compliant Frame Records

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: llvm/lib/Target/ARM/ARMFrameLowering.cpp:1844
+for (const auto  : MBB)
+  if (MI.getOpcode() == ARM::tSTRspi || MI.getOpcode() == ARM::tSTRi)
+for (const auto  : MI.operands())

How do you end up with tSTRi with a frameindex operand?  SelectionDAG won't 
generate that, I think.



Comment at: llvm/lib/Target/ARM/ARMFrameLowering.cpp:2110
+  if ((requiresAAPCSFrameRecord(MF) ||
+   MF.getTarget().Options.DisableFramePointerElim(MF)) &&
+  !LRSpilled) {

Should requiresAAPCSFrameRecord() be orthogonal to DisableFramePointerElim()?  
I mean, we have a complete set of flags controlling frame pointer elimination; 
the only part that's "new" here is that the frame pointer is stored in r11, 
instead of r7.



Comment at: llvm/lib/Target/ARM/ARMFrameLowering.cpp:2317
+if ((BigFrameOffsets || canSpillOnFrameIndexAccess(MF, *this)) &&
+!ExtraCSSpill) {
   // If any non-reserved CS register isn't spilled, just spill one or two

Rearrange this to check ExtraCSSpill first, so we don't run 
canSpillOnFrameIndexAccess() if it isn't necessary?



Comment at: llvm/test/CodeGen/Thumb/frame-access.ll:73
+; CHECK-AAPCS: mov r0, r11
+; CHECK-AAPCS: str r1, [r0, #8]
+; CHECK-AAPCS: mov r0, r11

Can we use sp-relative accesses here?  If we're not doing dynamic allocations, 
it should be a fixed offset.



Comment at: llvm/test/CodeGen/Thumb/frame-access.ll:96
+; CHECK-ATPCS:   add r7, sp, #8
+; CHECK-AAPCS:   add r11, sp, #0
 ; Align stack

I don't think "add r11, sp, #0" corresponds to a legal Thumb1 instruction?  
(Does -verify-machine-instrs catch this?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125094

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

[PATCH] D125773: [Driver] Do not auto-enable header modules with -std=c++20

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Header modules are part of the C++20 standard (where they are called "header 
units"), and module maps are an intended way for Clang to provide this 
functionality in C++20 mode. I don't think turning this off by default in C++20 
is the right forward-looking plan; rather, I think we should be thinking about 
moving towards header modules simply always being something that Clang can do, 
with no flag to control that.

That said: this new flag is not actually turning off header modules -- we'll 
still treat headers listed in module maps as having modular semantics with this 
flag specified (especially if `-fmodules-local-submodule-visibility` is 
enabled, but some modules semantics such as checking for undeclared 
dependencies are applied regardless). Rather, this flag will cause us to parse 
header modules on the fly, as part of the same parsing action, rather than 
building and importing a separate AST for them. And I think that is a 
reasonable thing to have a flag for. Maybe `-fheader-modules=parse` (to parse 
them on the fly) versus `-fheader-modules=import` (to import them from a `pcm` 
file)? This would also leave the door open for being able to specify whether 
modular semantics are enabled for header modules or not, as a different value 
for the same flag.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125773

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

[PATCH] D125925: Add an option to fill container for ref

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 created this revision.
usaxena95 added a reviewer: kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
usaxena95 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

This allows index implementations to fill container details when required 
specially when computing containerID is expensive.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125925

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/index/Index.h


Index: clang-tools-extra/clangd/index/Index.h
===
--- clang-tools-extra/clangd/index/Index.h
+++ clang-tools-extra/clangd/index/Index.h
@@ -72,6 +72,8 @@
   /// choose to return less than this, e.g. it tries to avoid returning stale
   /// results.
   llvm::Optional Limit;
+  /// If set, populates the SymbolID for the container of the reference.
+  bool WantContainer;
 };
 
 struct RelationsRequest {
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -2112,6 +2112,7 @@
   // FIXME: Consider also using AST information when feasible.
   RefsRequest Request;
   Request.IDs.insert(*ID);
+  Request.WantContainer = true;
   // We could restrict more specifically to calls by introducing a new RefKind,
   // but non-call references (such as address-of-function) can still be
   // interesting as they can indicate indirect calls.


Index: clang-tools-extra/clangd/index/Index.h
===
--- clang-tools-extra/clangd/index/Index.h
+++ clang-tools-extra/clangd/index/Index.h
@@ -72,6 +72,8 @@
   /// choose to return less than this, e.g. it tries to avoid returning stale
   /// results.
   llvm::Optional Limit;
+  /// If set, populates the SymbolID for the container of the reference.
+  bool WantContainer;
 };
 
 struct RelationsRequest {
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -2112,6 +2112,7 @@
   // FIXME: Consider also using AST information when feasible.
   RefsRequest Request;
   Request.IDs.insert(*ID);
+  Request.WantContainer = true;
   // We could restrict more specifically to calls by introducing a new RefKind,
   // but non-call references (such as address-of-function) can still be
   // interesting as they can indicate indirect calls.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125788: [flang][driver] Rename `flang-new` as `flang`

[Damian Rouson via Phabricator via cfe-commits]

rouson added a comment.

I think this is an exciting step.  I hope it gets approved.  Although it's 
technically true that this could appear to be a regression for current flang 
script users, the ultimate compilation currently happens by invoking an 
external compiler so most current flang script users can eliminate the 
regression by simply calling the external compiler.  The exception would be if 
the current flang script user specifically wants flang's parsing capabilities 
for checking code correctness (do we know if anyone is using the flang script 
that way?) or for testing the parser (which I presume can still happen by other 
means).  It might be nice to simply rename the current script so those who want 
it can simply change the name they use to invoke it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125788

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

[PATCH] D125919: Drop qualifiers from return types in C (DR423)

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

We definitely don't want ObjC to differ here; thanks for the CC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125919

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

[PATCH] D125788: [flang][driver] Rename `flang-new` as `flang`

[Shraiysh via Phabricator via cfe-commits]

shraiysh added a comment.

> With this change the flang driver will not generate an executable unless it 
> is built with an option.

Okay, thank you for the clarification. Is this a cmake option and? Are we 
documenting this somewhere except this patch summary, like on the flang 
documentation or something? Apologies if this a repetition to what you meant by 
clarifying that it is available via a flag.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125788

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

[PATCH] D125311: [pseudo] Share the underly payload when stripping comments for a token stream

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D125311#3503452 , @sammccall wrote:

> This looks good, but if that's the idiom we should add it to cook() also.

cook is more tricky, it takes a TokenStream, and returns a new one with an 
allocator payload:

- the input TokenStream doesn't have a payload, this is OK
- the input TokenStream has a payload, but what if the type of payload is not 
an allocator?

Possible solution: 1) always assert the input TokenStream doesn't have a 
payload or the payload is allocator type 2) merge the lex and cook function, it 
looks like we always use the cook one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125311

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

[PATCH] D125788: [flang][driver] Rename `flang-new` as `flang`

[Kiran Chandramohan via Phabricator via cfe-commits]

kiranchandramohan added a comment.

In D125788#3523259 , @shraiysh wrote:

>> clarify that execution is still restricted to developers via a flag.
>
> Just confirming what this means: After this patch, would `flang sample.f90` 
> generate an executable? If not, how to generate an executable using 
> `flang-new`? If yes, does this mean that if I just replace classic-flang with 
> new llvm `flang` (along with library and include paths), I should be able to 
> find and report bugs (expected) or run the application (best case)?

Sorry, the point I was trying to say here is that till now the `flang` script 
will parse, unparse and call an external compiler to do the codegen/generate 
executable. With this change the `flang` driver will not generate an executable 
unless it is built with an option. This default behaviour might be seen as a 
regression from the point of view of a `flang` script user.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125788

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

[PATCH] D125919: Drop qualifiers from return types in C (DR423)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: rjmccall, dexonsmith.
aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.

Adding some Apple reviewers because this touches ObjC behavior (I would be very 
surprised if ObjC wanted different rules here than C though).




Comment at: clang/test/Sema/warn-missing-prototypes.c:61
 
+// FIXME: because qualifiers are ignored in the return type when forming the
+// type from the declarator, we get the position incorrect for the fix-it hint.

erichkeane wrote:
> Hrmph, this is unfortunate... 
It is, but it didn't seem like a sufficiently common situation to warrant 
trying to fix it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125919

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

[PATCH] D125874: [clang-tidy] Fix readability-simplify-boolean-expr when Ifs have an init statement or condition variable

[Nathan James via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG35660247dd9c: [clang-tidy] Fix 
readability-simplify-boolean-expr when Ifs have an init… (authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125874

Files:
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-cxx17.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
@@ -478,6 +478,14 @@
   // CHECK-FIXES-NEXT: {{^}}  };{{$}}
 }
 
+bool condition_variable_return_stmt(int i) {
+  // Unchanged: condition variable.
+  if (bool Res = i == 0)
+return true;
+  else
+return false;
+}
+
 void simple_conditional_assignment_statements(int i) {
   bool b;
   if (i > 10)
@@ -594,6 +602,13 @@
 h = true;
   } else
 h = false;
+
+  // Unchanged: condition variable.
+  bool k;
+  if (bool Res = j > 10)
+k = true;
+  else
+k = false;
 }
 
 // Unchanged: chained return statements, but ChainedConditionalReturn not set.
Index: clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-cxx17.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-cxx17.cpp
@@ -0,0 +1,19 @@
+// RUN: clang-tidy %s -checks='-*,readability-simplify-boolean-expr' -- -std=c++17 | count 0
+struct RAII {};
+bool foo(bool Cond) {
+  bool Result;
+
+  if (RAII Object; Cond)
+Result = true;
+  else
+Result = false;
+
+  if (bool X = Cond; X)
+Result = true;
+  else
+Result = false;
+
+  if (bool X = Cond; X)
+return true;
+  return false;
+}
Index: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -351,6 +351,9 @@
   }
 
   bool VisitIfStmt(IfStmt *If) {
+// Skip any if's that have a condition var or an init statement.
+if (If->hasInitStorage() || If->hasVarStorage())
+  return true;
 /*
  * if (true) ThenStmt(); -> ThenStmt();
  * if (false) ThenStmt(); -> ;
@@ -461,14 +464,17 @@
  * if (Cond) return false; return true; -> return !Cond;
  */
 auto *If = cast(*First);
-ExprAndBool ThenReturnBool =
-checkSingleStatement(If->getThen(), parseReturnLiteralBool);
-if (ThenReturnBool && ThenReturnBool.Bool != TrailingReturnBool.Bool) {
-  if (Check->ChainedConditionalReturn ||
-  (!PrevIf && If->getElse() == nullptr)) {
-Check->replaceCompoundReturnWithCondition(
-Context, cast(*Second), TrailingReturnBool.Bool, If,
-ThenReturnBool.Item);
+if (!If->hasInitStorage() && !If->hasVarStorage()) {
+  ExprAndBool ThenReturnBool =
+  checkSingleStatement(If->getThen(), parseReturnLiteralBool);
+  if (ThenReturnBool &&
+  ThenReturnBool.Bool != TrailingReturnBool.Bool) {
+if (Check->ChainedConditionalReturn ||
+(!PrevIf && If->getElse() == nullptr)) {
+  Check->replaceCompoundReturnWithCondition(
+  Context, cast(*Second), TrailingReturnBool.Bool,
+  If, ThenReturnBool.Item);
+}
   }
 }
   } else if (isa(*First)) {
@@ -481,7 +487,8 @@
 : isa(*First) ? cast(*First)->getSubStmt()
 : cast(*First)->getSubStmt();
 auto *SubIf = dyn_cast(SubStmt);
-if (SubIf && !SubIf->getElse()) {
+if (SubIf && !SubIf->getElse() && !SubIf->hasInitStorage() &&
+!SubIf->hasVarStorage()) {
   ExprAndBool ThenReturnBool =
   checkSingleStatement(SubIf->getThen(), parseReturnLiteralBool);
   if (ThenReturnBool &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 3566024 - [clang-tidy] Fix readability-simplify-boolean-expr when Ifs have an init statement or condition variable

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2022-05-18T20:47:37+01:00
New Revision: 35660247dd9ce850dccd60cc55428a510dbdd1c8

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

LOG: [clang-tidy] Fix readability-simplify-boolean-expr when Ifs have an init 
statement or condition variable

Fixes https://github.com/llvm/llvm-project/issues/3.

Reviewed By: LegalizeAdulthood

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

Added: 

clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-cxx17.cpp

Modified: 
clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp

clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
index 1db440120aa3a..323cf1f2d5ecb 100644
--- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -351,6 +351,9 @@ class SimplifyBooleanExprCheck::Visitor : public 
RecursiveASTVisitor {
   }
 
   bool VisitIfStmt(IfStmt *If) {
+// Skip any if's that have a condition var or an init statement.
+if (If->hasInitStorage() || If->hasVarStorage())
+  return true;
 /*
  * if (true) ThenStmt(); -> ThenStmt();
  * if (false) ThenStmt(); -> ;
@@ -461,14 +464,17 @@ class SimplifyBooleanExprCheck::Visitor : public 
RecursiveASTVisitor {
  * if (Cond) return false; return true; -> return !Cond;
  */
 auto *If = cast(*First);
-ExprAndBool ThenReturnBool =
-checkSingleStatement(If->getThen(), parseReturnLiteralBool);
-if (ThenReturnBool && ThenReturnBool.Bool != TrailingReturnBool.Bool) {
-  if (Check->ChainedConditionalReturn ||
-  (!PrevIf && If->getElse() == nullptr)) {
-Check->replaceCompoundReturnWithCondition(
-Context, cast(*Second), TrailingReturnBool.Bool, 
If,
-ThenReturnBool.Item);
+if (!If->hasInitStorage() && !If->hasVarStorage()) {
+  ExprAndBool ThenReturnBool =
+  checkSingleStatement(If->getThen(), parseReturnLiteralBool);
+  if (ThenReturnBool &&
+  ThenReturnBool.Bool != TrailingReturnBool.Bool) {
+if (Check->ChainedConditionalReturn ||
+(!PrevIf && If->getElse() == nullptr)) {
+  Check->replaceCompoundReturnWithCondition(
+  Context, cast(*Second), TrailingReturnBool.Bool,
+  If, ThenReturnBool.Item);
+}
   }
 }
   } else if (isa(*First)) {
@@ -481,7 +487,8 @@ class SimplifyBooleanExprCheck::Visitor : public 
RecursiveASTVisitor {
 : isa(*First) ? cast(*First)->getSubStmt()
 : cast(*First)->getSubStmt();
 auto *SubIf = dyn_cast(SubStmt);
-if (SubIf && !SubIf->getElse()) {
+if (SubIf && !SubIf->getElse() && !SubIf->hasInitStorage() &&
+!SubIf->hasVarStorage()) {
   ExprAndBool ThenReturnBool =
   checkSingleStatement(SubIf->getThen(), parseReturnLiteralBool);
   if (ThenReturnBool &&

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-cxx17.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-cxx17.cpp
new file mode 100644
index 0..310afe04672ae
--- /dev/null
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-cxx17.cpp
@@ -0,0 +1,19 @@
+// RUN: clang-tidy %s -checks='-*,readability-simplify-boolean-expr' -- 
-std=c++17 | count 0
+struct RAII {};
+bool foo(bool Cond) {
+  bool Result;
+
+  if (RAII Object; Cond)
+Result = true;
+  else
+Result = false;
+
+  if (bool X = Cond; X)
+Result = true;
+  else
+Result = false;
+
+  if (bool X = Cond; X)
+return true;
+  return false;
+}

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
index acf0cfe301279..c14438aa93801 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
@@ -478,6 +478,14 @@ void lambda_conditional_return_statements() {
   // CHECK-FIXES-NEXT: {{^}}  };{{$}}
 }
 
+bool condition_variable_return_stmt(int i) {
+  // Unchanged: condition variable.
+  if (bool Res = i == 0)
+return true;
+  else
+return false;
+}
+
 void simple_conditional_assignment_statements(int i) {
   bool b;
   if (i > 10)
@@ -594,6 +602,13 

[PATCH] D125920: [analyzer][NFC] Remove the unused LocAsInteger::getPersistentLoc()

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

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125920

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h


Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -432,13 +432,6 @@
 return D->first.castAs();
   }
 
-  Loc getPersistentLoc() const {
-const std::pair *D =
-  static_cast *>(Data);
-const SVal& V = D->first;
-return V.castAs();
-  }
-
   unsigned getNumBits() const {
 const std::pair *D =
   static_cast *>(Data);


Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -432,13 +432,6 @@
 return D->first.castAs();
   }
 
-  Loc getPersistentLoc() const {
-const std::pair *D =
-  static_cast *>(Data);
-const SVal& V = D->first;
-return V.castAs();
-  }
-
   unsigned getNumBits() const {
 const std::pair *D =
   static_cast *>(Data);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125862: [clang][driver] Add gcc-toolset/devtoolset 12 to prefixes

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In addition, if every new GCC release requires addition to 
/opt/rh/gcc-toolset-$major (if I understand correctly), we probably want to 
switch to `llvm::vfs::directory_iterator` iteration and using the largest 
version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125862

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

[PATCH] D125885: [clang-tidy] bugprone-argument-comment: Ignore calls to user-defined literals

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

These tests aren't that great.
I'd advise adding a test that just checks using the userDefinedLiteral in 
`bugprone-argument-comment-literals.cpp`, That check file will add comments to 
parameters that don't have them.
That way you aren't testing against distinguishing between the actual parameter 
comment and the literal comment.
This simple edit without your fix results in this diff in the output, with your 
fix that diff wont exist.

  [build]  int operator""_op(unsigned long long Value);
  [build]  
  [build]  void test() {
  [build] -  1_op;
  [build] +  /*Value=*/1_op;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125885

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

[PATCH] D125862: [clang][driver] Add gcc-toolset/devtoolset 12 to prefixes

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

It may be time to add unittests to `clang/unittests/Driver/ToolChainTest.cpp` 
for /opt/rh detection. You may use `TEST(ToolChainTest, VFSGCCInstallation)` as 
an example creating a pseudo file hierarchy in memory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125862

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

[clang] d8166e1 - [Driver] Refactor /opt/rh detection

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2022-05-18T12:40:27-07:00
New Revision: d8166e1900c05429abc726d379bc33281c4c98e4

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

LOG: [Driver] Refactor /opt/rh detection

Check /opt/rh first to avoid `/opt/rh/*` newfstatat/etc calls on other
distributions.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Gnu.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index 003a97e2c3ebb..b73634bad631a 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2147,9 +2147,10 @@ void 
Generic_GCC::GCCInstallationDetector::AddDefaultGCCPrefixes(
 return;
   }
 
-  // Non-Solaris is much simpler - most systems just go with "/usr".
-  if (SysRoot.empty() && TargetTriple.getOS() == llvm::Triple::Linux) {
-// Yet, still look for RHEL/CentOS devtoolsets and gcc-toolsets.
+  // For Linux, if --sysroot is not specified, look for RHEL/CentOS devtoolsets
+  // and gcc-toolsets.
+  if (SysRoot.empty() && TargetTriple.getOS() == llvm::Triple::Linux &&
+  D.getVFS().exists("/opt/rh")) {
 Prefixes.push_back("/opt/rh/gcc-toolset-11/root/usr");
 Prefixes.push_back("/opt/rh/gcc-toolset-10/root/usr");
 Prefixes.push_back("/opt/rh/devtoolset-11/root/usr");
@@ -2162,6 +2163,8 @@ void 
Generic_GCC::GCCInstallationDetector::AddDefaultGCCPrefixes(
 Prefixes.push_back("/opt/rh/devtoolset-3/root/usr");
 Prefixes.push_back("/opt/rh/devtoolset-2/root/usr");
   }
+
+  // Fall back to /usr which is used by most non-Solaris systems.
   Prefixes.push_back(SysRoot.str() + "/usr");
 }
 



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

[PATCH] D125919: Drop qualifiers from return types in C (DR423)

[Erich Keane via Phabricator via cfe-commits]

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



Comment at: clang/test/Sema/warn-missing-prototypes.c:61
 
+// FIXME: because qualifiers are ignored in the return type when forming the
+// type from the declarator, we get the position incorrect for the fix-it hint.

Hrmph, this is unfortunate... 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125919

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

[PATCH] D125919: Drop qualifiers from return types in C (DR423)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: erichkeane, tahonermann, jyknight, efriedma, 
clang-language-wg.
Herald added a project: All.
aaron.ballman requested review of this revision.
Herald added a project: clang.

WG14 DR423 (https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2148.htm#dr_423), 
resolved during the C11 time frame, changed the way qualifiers are handled on 
function return types and in cast expressions after it was noted that these 
types are now directly observable via generic selection expressions. In C, the 
function declarator is adjusted to ignore all qualifiers (including `_Atomic` 
qualifiers).

Clang already handles the cast expression case correctly (by performing the 
lvalue conversion, which drops the qualifiers as well), but with these changes 
it will now also handle function declarations appropriately.

Fixes #39595


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125919

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/block-call.c
  clang/test/Sema/c89.c
  clang/test/Sema/function.c
  clang/test/Sema/warn-missing-prototypes.c
  clang/test/Sema/wg14-dr423.c
  clang/test/SemaObjC/block-omitted-return-type.m

Index: clang/test/SemaObjC/block-omitted-return-type.m
===
--- clang/test/SemaObjC/block-omitted-return-type.m
+++ clang/test/SemaObjC/block-omitted-return-type.m
@@ -23,8 +23,8 @@
   void (^simpleBlock4)(void) = ^ const { //expected-warning {{'const' qualifier on omitted return type '' has no effect}}
 return;
   };
-  void (^simpleBlock5)(void) = ^ const void { //expected-error {{incompatible block pointer types initializing 'void (^)(void)' with an expression of type 'const void (^)(void)'}}
-return; // expected-warning@-1 {{function cannot return qualified void type 'const void'}}
+  void (^simpleBlock5)(void) = ^ const void { // OK after DR 423.
+return;
   };
   void (^simpleBlock6)(void) = ^ const (void) { //expected-warning {{'const' qualifier on omitted return type '' has no effect}}
 return;
Index: clang/test/Sema/wg14-dr423.c
===
--- /dev/null
+++ clang/test/Sema/wg14-dr423.c
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -ast-dump %s | FileCheck %s
+// expected-no-diagnostics
+
+void GH39595(void) {
+  // Ensure that qualifiers on function return types are dropped as part of the
+  // declaration.
+  extern const int const_int(void);
+  // CHECK: FunctionDecl {{.*}} parent {{.*}}  col:20 referenced const_int 'int (void)' extern
+  extern _Atomic int atomic(void);
+  // CHECK: FunctionDecl {{.*}} parent {{.*}}  col:22 referenced atomic 'int (void)' extern
+
+  (void)_Generic(const_int(), int : 1);
+  (void)_Generic(atomic(), int : 1);
+
+  // Make sure they're dropped from function pointers as well.
+  _Atomic int (*fp)(void);
+  (void)_Generic(fp(), int : 1);
+}
+
+void casting(void) {
+  // Ensure that qualifiers on cast operations are also dropped.
+  (void)_Generic((const int)12, int : 1);
+
+  struct S { int i; } s;
+  (void)_Generic((const struct S)s, struct S : 1);
+
+  int i;
+  __typeof__((const int)i) j;
+  j = 100; // If we didn't strip the qualifiers during the cast, this would err.
+}
Index: clang/test/Sema/warn-missing-prototypes.c
===
--- clang/test/Sema/warn-missing-prototypes.c
+++ clang/test/Sema/warn-missing-prototypes.c
@@ -58,9 +58,14 @@
 
 struct MyStruct {};
 
+// FIXME: because qualifiers are ignored in the return type when forming the
+// type from the declarator, we get the position incorrect for the fix-it hint.
+// It suggests 'const static struct' instead of 'static const struct'. However,
+// thanks to the awful rules of parsing in C, the effect is the same and the
+// code is valid, if a bit funny looking.
 const struct MyStruct get_struct() { // expected-warning{{no previous prototype for function 'get_struct'}}
   // expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:1-[[@LINE-2]]:1}:"static "
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:7-[[@LINE-2]]:7}:"static "
   struct MyStruct ret;
   return ret;
 }
@@ -70,7 +75,7 @@
 // Two spaces between cost and struct
 const  struct MyStruct get_struct_2() {  // expected-warning{{no previous prototype for function 'get_struct_2'}}
   // expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:1-[[@LINE-2]]:1}:"static "
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:8-[[@LINE-2]]:8}:"static "
   struct MyStruct ret;
   return ret;
 }
Index: clang/test/Sema/function.c

[PATCH] D125892: [analyzer] Implement assumeInclusiveRange in terms of assumeInclusiveRangeDual

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

steakhal added a comment.

Okay, it took me a while to get what's going on.
Do you have anything in mind to express it by slightly less indirection, 
references, templates and well, virtual functions xD


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125892

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

[PATCH] D125693: [DebugInfo][WIP] Support types, imports and static locals declared in a lexical block (3/5)

[Andrew Savonichev via Phabricator via cfe-commits]

asavonic added a comment.

Thanks a lot for the patch! It would be great to get this issue finally fixed. 
I assume that this is the main patch, other patches in the stack seem like just 
preparation/adjustments needed for this one to work.

> This an alternative implementation for D113741 
>  which relies on localDecls field of 
> DISubprogram or DILexicalBlock for getting local declarations (types, imports 
> or static variable) for a particular local scope.

Can you include a short description of the issue? PR19238 has a good and short 
example, we can inline it here.

> Another thing that should be noted that with this patch we will no longer 
> emit local declarations if its parent scope doesn't have a LexicalScope 
> calculated for it

If a local declaration (a type or a static variable) is optimized away, then 
with this patch we have no debug information for it, right? For types we have 
`-debug-info-kind=unused-types` as a workaround (as shown in 
lexical-block-retained-types.ll), so the issue is only with static variables 
that have no uses. Given that they were broken before this patch and there is 
no way to access them from a debugger (cannot set a breakpoint if a block is 
optimized away), I think it is fine.




Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1382
+  assert((!GV->getScope() || !isa(GV->getScope())) &&
+ "Unexpected function-local declaration!");
   if (Processed.insert(GV).second)

So here we discard LLVM IR metadata that have local declarations tied to a CU, 
right? This will break compatibility with old LLVM IR. Can we do some upgrade 
to convert this "old style" metadata the way we expect it now? Does it make 
sense to support both variants?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125693

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

[PATCH] D125874: [clang-tidy] Fix readability-simplify-boolean-expr when Ifs have an init statement or condition variable

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 430473.
njames93 added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125874

Files:
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-cxx17.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
@@ -478,6 +478,14 @@
   // CHECK-FIXES-NEXT: {{^}}  };{{$}}
 }
 
+bool condition_variable_return_stmt(int i) {
+  // Unchanged: condition variable.
+  if (bool Res = i == 0)
+return true;
+  else
+return false;
+}
+
 void simple_conditional_assignment_statements(int i) {
   bool b;
   if (i > 10)
@@ -594,6 +602,13 @@
 h = true;
   } else
 h = false;
+
+  // Unchanged: condition variable.
+  bool k;
+  if (bool Res = j > 10)
+k = true;
+  else
+k = false;
 }
 
 // Unchanged: chained return statements, but ChainedConditionalReturn not set.
Index: clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-cxx17.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-cxx17.cpp
@@ -0,0 +1,19 @@
+// RUN: clang-tidy %s -checks='-*,readability-simplify-boolean-expr' -- -std=c++17 | count 0
+struct RAII {};
+bool foo(bool Cond) {
+  bool Result;
+
+  if (RAII Object; Cond)
+Result = true;
+  else
+Result = false;
+
+  if (bool X = Cond; X)
+Result = true;
+  else
+Result = false;
+
+  if (bool X = Cond; X)
+return true;
+  return false;
+}
Index: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -351,6 +351,9 @@
   }
 
   bool VisitIfStmt(IfStmt *If) {
+// Skip any if's that have a condition var or an init statement.
+if (If->hasInitStorage() || If->hasVarStorage())
+  return true;
 /*
  * if (true) ThenStmt(); -> ThenStmt();
  * if (false) ThenStmt(); -> ;
@@ -461,14 +464,17 @@
  * if (Cond) return false; return true; -> return !Cond;
  */
 auto *If = cast(*First);
-ExprAndBool ThenReturnBool =
-checkSingleStatement(If->getThen(), parseReturnLiteralBool);
-if (ThenReturnBool && ThenReturnBool.Bool != TrailingReturnBool.Bool) {
-  if (Check->ChainedConditionalReturn ||
-  (!PrevIf && If->getElse() == nullptr)) {
-Check->replaceCompoundReturnWithCondition(
-Context, cast(*Second), TrailingReturnBool.Bool, If,
-ThenReturnBool.Item);
+if (!If->hasInitStorage() && !If->hasVarStorage()) {
+  ExprAndBool ThenReturnBool =
+  checkSingleStatement(If->getThen(), parseReturnLiteralBool);
+  if (ThenReturnBool &&
+  ThenReturnBool.Bool != TrailingReturnBool.Bool) {
+if (Check->ChainedConditionalReturn ||
+(!PrevIf && If->getElse() == nullptr)) {
+  Check->replaceCompoundReturnWithCondition(
+  Context, cast(*Second), TrailingReturnBool.Bool,
+  If, ThenReturnBool.Item);
+}
   }
 }
   } else if (isa(*First)) {
@@ -481,7 +487,8 @@
 : isa(*First) ? cast(*First)->getSubStmt()
 : cast(*First)->getSubStmt();
 auto *SubIf = dyn_cast(SubStmt);
-if (SubIf && !SubIf->getElse()) {
+if (SubIf && !SubIf->getElse() && !SubIf->hasInitStorage() &&
+!SubIf->hasVarStorage()) {
   ExprAndBool ThenReturnBool =
   checkSingleStatement(SubIf->getThen(), parseReturnLiteralBool);
   if (ThenReturnBool &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125788: [flang][driver] Rename `flang-new` as `flang`

[Shraiysh via Phabricator via cfe-commits]

shraiysh added a comment.

> clarify that execution is still restricted to developers via a flag.

Just confirming what this means: After this patch, would `flang sample.f90` 
generate an executable? If not, how to generate an executable using 
`flang-new`? If yes, does this mean that if I just replace classic-flang with 
new llvm `flang` (along with library and include paths), I should be able to 
find and report bugs (expected) or run the application (best case)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125788

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

[PATCH] D125912: [Clang][[OpenMP5.1] Initial parser/sema for default(private) clause

[Alexey Bataev via Phabricator via cfe-commits]

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

LG with a nit




Comment at: clang/lib/Parse/ParseOpenMP.cpp:2925
 EndLoc);
-
 // Exit scope.

Restore


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125912

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

[PATCH] D124806: [clang-tidy] add support for Demorgan conversions to readability-simplify-bool-expr

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 430472.
njames93 added a comment.

Remove unnecessary parens in documentation as check will remove the parens when 
possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124806

Files:
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-demorgan.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-demorgan.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-demorgan.cpp
@@ -0,0 +1,87 @@
+// RUN: %check_clang_tidy %s readability-simplify-boolean-expr %t
+
+void eat(bool);
+
+void foo(bool A1, bool A2, bool A3, bool A4) {
+  bool X;
+  X = !(!A1 || A2);
+  X = !(A1 || !A2);
+  X = !(!A1 || !A2);
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = A1 && !A2;
+  // CHECK-FIXES-NEXT: X = !A1 && A2;
+  // CHECK-FIXES-NEXT: X = A1 && A2;
+
+  X = !(!A1 && A2);
+  X = !(A1 && !A2);
+  X = !(!A1 && !A2);
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = A1 || !A2;
+  // CHECK-FIXES-NEXT: X = !A1 || A2;
+  // CHECK-FIXES-NEXT: X = A1 || A2;
+
+  X = !(!A1 && !A2 && !A3);
+  X = !(!A1 && (!A2 && !A3));
+  X = !(!A1 && (A2 && A3));
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = A1 || A2 || A3;
+  // CHECK-FIXES-NEXT: X = A1 || A2 || A3;
+  // CHECK-FIXES-NEXT: X = A1 || !A2 || !A3;
+
+  X = !(A1 && A2 == A3);
+  X = !(!A1 && A2 > A3);
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = !A1 || A2 != A3;
+  // CHECK-FIXES-NEXT: X = A1 || A2 <= A3;
+
+  // Ensure the check doesn't try to combine fixes for the inner and outer demorgan simplification.
+  X = !(!A1 && !(!A2 && !A3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = A1 || (!A2 && !A3);
+
+  // Testing to see how it handles parens
+  X = !(A1 && !A2 && !A3);
+  X = !(A1 && !A2 || !A3);
+  X = !(!A1 || A2 && !A3);
+  X = !((A1 || !A2) && !A3);
+  X = !((A1 || !A2) || !A3);
+  // CHECK-MESSAGES: :[[@LINE-5]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-5]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-5]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-5]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-5]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = !A1 || A2 || A3;
+  // CHECK-FIXES-NEXT: X = (!A1 || A2) && A3;
+  // CHECK-FIXES-NEXT: X = A1 && (!A2 || A3);
+  // CHECK-FIXES-NEXT: X = (!A1 && A2) || A3;
+  // CHECK-FIXES-NEXT: X = !A1 && A2 && A3;
+  X = !((A1 || A2) && (!A3 || A4));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = (!A1 && !A2) || (A3 && !A4);
+
+  eat(!(!A1 && !A2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: eat(A1 || A2);
+
+  bool Init = !(!A1 || !A2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: bool Init = A1 && A2;
+
+  X = A1 && !(!A2 || !A3);
+  X = A1 || !(!A2 || !A3);
+  X = A1 && !(!A2 && !A3);
+  // CHECK-MESSAGES: :[[@LINE-3]]:13: warning: boolean 

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

[Shraiysh via Phabricator via cfe-commits]

shraiysh added inline comments.



Comment at: clang/lib/Testing/CMakeLists.txt:29
   llvm_gtest
+  clangBasic
+  clangFrontend

abidmalikwaterloo wrote:
> shraiysh wrote:
> > unrelated change?
> When I rebase, these changes were highlighted in the main branch which was 
> missing in the patch as it was too old.
Hmm, these are in llvm-project/main right now but this means that the patch has 
not been rebased properly. These changes should not be a part of this patch :/ 
Can you try rebasing again? Otherwise this will cause issues while/after 
landing this patch.



Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:514
+oilist(`schedule` `(`
+  custom(
+   $dist_schedule_var,

abidmalikwaterloo wrote:
> shraiysh wrote:
> > dist_schedule is a dummy clause, where kind is only allowed to be `static` 
> > according to the standard. I don't think that requires a custom function, 
> > we can instead have something like the following -
> > ```
> > arguments = UnitAttr:$static_dist_schedule, 
> > Optional:$schedule_chunk
> > 
> > assemblyFormat = `dist_schedule` `(` (`static` $static_dist_schedule^)? 
> > (`:` $schedule_chunk^)? `)`
> > ```
> > Would that work? Let me know if there are any suggestions.
> My only concern is; will this be a dummy clause with the static scheduler 
> forever? I am pretty sure dist_schedule will have a dynamic or a  user 
> defined scheduling strategy as well to improve the performance of a given 
> application 
If and when it changes in the standard, at that time we can change the 
parsing/printing accordingly. Till then such a function seems unnecessary and a 
possible source of errors because it accepts invalid OpenMP code.



Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:528
+
+  let regions = (region AnyRegion:$region);
+}

abidmalikwaterloo wrote:
> shraiysh wrote:
> > I think we need a verifier for this too. There are a couple semantic checks 
> > which we can do in verifier.
> Can you say more about the semantic checks you have in mind?
The following restriction from the standard can be added to the 
verifier/Operation definition - 
> The region corresponding to the distribute construct must be strictly nested 
> inside a teams region.

The other restrictions - I am okay with not adding them because I don't know 
how they would be added. Needless to say if we figure out how to add them, then 
we should do it.



Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:265
+ Variadic:$step,
+Variadic:$private_var,
+ Variadic:$firstprivate_var,

abidmalikwaterloo wrote:
> abidmalikwaterloo wrote:
> > kiranchandramohan wrote:
> > > kiranchandramohan wrote:
> > > > Nit: Alignment
> > > You can remove private, firstprivate and lastprivate for now.
> > Any special reason for this?
> Any reason for removing them? 
They are being removed because we do not have a clean way of handling these 
clauses in MLIR for any generic frontend.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105584

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

[PATCH] D124806: [clang-tidy] add support for Demorgan conversions to readability-simplify-bool-expr

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D124806#3523109 , 
@LegalizeAdulthood wrote:

> LGTM!  Thanks for taking my idea and implementing it much better than what I 
> had `:)`.

Cheers.
Can I ask why you feel that this should be behind an option?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124806

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

[PATCH] D125840: [Analyzer] Removed extra space from NSErrorChecker debug message and updated relevant tests

[Usama Hameed via Phabricator via cfe-commits]

usama54321 updated this revision to Diff 430469.
usama54321 added a comment.

Fixed clang-format issue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125840

Files:
  clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
  clang/test/Analysis/CheckNSError.m
  clang/test/Analysis/incorrect-checker-names.mm


Index: clang/test/Analysis/incorrect-checker-names.mm
===
--- clang/test/Analysis/incorrect-checker-names.mm
+++ clang/test/Analysis/incorrect-checker-names.mm
@@ -107,7 +107,7 @@
 
 void foo(CFErrorRef* error) { // expected-warning{{Function accepting 
CFErrorRef* should have a non-void return value to indicate whether or not an 
error occurred [osx.coreFoundation.CFError]}}
   // FIXME: This shouldn't be tied to a modeling checker.
-  *error = 0; // expected-warning {{Potential null dereference.  According to 
coding standards documented in CoreFoundation/CFError.h the parameter may be 
null [osx.coreFoundation.CFError]}}
+  *error = 0; // expected-warning {{Potential null dereference. According to 
coding standards documented in CoreFoundation/CFError.h the parameter may be 
null [osx.coreFoundation.CFError]}}
 }
 
 @interface A
@@ -116,7 +116,7 @@
 
 @implementation A
 - (void)myMethodWhichMayFail:(NSError **)error {  // 
expected-warning {{Method accepting NSError** should have a non-void return 
value to indicate whether or not an error occurred [osx.cocoa.NSError]}}
-  *error = [NSError errorWithDomain:@"domain" code:1 userInfo:0]; // 
expected-warning {{Potential null dereference.  According to coding standards 
in 'Creating and Returning NSError Objects' the parameter may be null 
[osx.cocoa.NSError]}}
+  *error = [NSError errorWithDomain:@"domain" code:1 userInfo:0]; // 
expected-warning {{Potential null dereference. According to coding standards in 
'Creating and Returning NSError Objects' the parameter may be null 
[osx.cocoa.NSError]}}
 }
 @end
 
Index: clang/test/Analysis/CheckNSError.m
===
--- clang/test/Analysis/CheckNSError.m
+++ clang/test/Analysis/CheckNSError.m
@@ -27,7 +27,7 @@
 
 @implementation A
 - (void)myMethodWhichMayFail:(NSError **)error {   // expected-warning 
{{Method accepting NSError** should have a non-void return value to indicate 
whether or not an error occurred}}
-  *error = [NSError errorWithDomain:@"domain" code:1 userInfo:0]; // 
expected-warning {{Potential null dereference}}
+  *error = [NSError errorWithDomain:@"domain" code:1 userInfo:0]; // 
expected-warning {{Potential null dereference. According to coding standards in 
'Creating and Returning NSError Objects' the parameter may be null 
[osx.cocoa.NSError]}}
 }
 
 - (BOOL)myMethodWhichMayFail2:(NSError **)error {  // no-warning
@@ -50,7 +50,7 @@
 typedef struct __CFError* CFErrorRef;
 
 void foo(CFErrorRef* error) { // expected-warning {{Function accepting 
CFErrorRef* should have a non-void return value to indicate whether or not an 
error occurred}}
-  *error = 0;  // expected-warning {{Potential null dereference}}
+  *error = 0; // expected-warning {{Potential null 
dereference. According to coding standards documented in 
CoreFoundation/CFError.h the parameter may be null 
[osx.coreFoundation.CFError]}}
 }
 
 int f1(CFErrorRef* error) {
@@ -74,7 +74,7 @@
 }
 
 int __attribute__((nonnull(1))) f5(int *x, CFErrorRef *error) {
-  *error = 0; // expected-warning {{Potential null dereference}}
+  *error = 0; // expected-warning {{Potential null dereference. According to 
coding standards documented in CoreFoundation/CFError.h the parameter may be 
null [osx.coreFoundation.CFError]}}
   return 0;
 }
 
Index: clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
@@ -266,7 +266,7 @@
   SmallString<128> Buf;
   llvm::raw_svector_ostream os(Buf);
 
-  os << "Potential null dereference.  According to coding standards ";
+  os << "Potential null dereference. According to coding standards ";
   os << (isNSError
  ? "in 'Creating and Returning NSError Objects' the parameter"
  : "documented in CoreFoundation/CFError.h the parameter");


Index: clang/test/Analysis/incorrect-checker-names.mm
===
--- clang/test/Analysis/incorrect-checker-names.mm
+++ clang/test/Analysis/incorrect-checker-names.mm
@@ -107,7 +107,7 @@
 
 void foo(CFErrorRef* error) { // expected-warning{{Function accepting CFErrorRef* should have a non-void return value to indicate whether or not an error occurred [osx.coreFoundation.CFError]}}
   // FIXME: This shouldn't be tied to a modeling checker.
-  *error = 0; // expected-warning {{Potential null 

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

[Abid via Phabricator via cfe-commits]

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



Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:488-490
+`private_var`, `firstprivate_var`, and `lastprivate_var` arguments are 
+variadic list of operands that specify the data sharing attributes of the 
+list of values 

shraiysh wrote:
> If they are not added, please remove them from documentation too. It would be 
> misleading otherwise.
Ok



Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:528
+
+  let regions = (region AnyRegion:$region);
+}

shraiysh wrote:
> I think we need a verifier for this too. There are a couple semantic checks 
> which we can do in verifier.
Can you say more about the semantic checks you have in mind?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105584

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

[PATCH] D125195: [asan][ARMCXXABI] Added missing asan poison array cookie hooks.

[Roy Sundahl via Phabricator via cfe-commits]

rsundahl updated this revision to Diff 430461.
rsundahl added a comment.

Fix (and cleanup) for failure of CodeGen arm.c check in ci/cd pipeline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125195

Files:
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/arm.cpp
  compiler-rt/lib/asan/asan_poisoning.cpp
  compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp
  compiler-rt/test/asan/TestCases/Posix/new_array_cookie_uaf_test.cpp
  compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp

Index: compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp
===
--- compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp
+++ compiler-rt/test/asan/TestCases/Posix/new_array_cookie_with_new_from_class.cpp
@@ -1,16 +1,22 @@
-// Test that we do not poison the array cookie if the operator new is defined
-// inside the class.
-// RUN: %clangxx_asan  %s -o %t && %run %t
+// For arrays allocated by classes that define their own custom "new", ASAN
+// should NOT poison the array cookie unless the compilation unit is compiled
+// with "-fsanitize-address-poison-custom-array-cookie".
+// Here we test that:
+//   1) w/o ASan, the array cookie location IS writable
+//   2) w/ASan, the array cookie location IS writeable by default
+//   3) w/ASan, the flag "-fno-sanitize-address-poison-custom-array-cookie"
+//  is a NOP (because this is the default) and finally,
+//   4) w/ASan AND "-fsanitize-address-poison-custom-array-cookie", the
+//  array cookie location is NOT writable (ASAN successfully stopped it)
 //
-// XFAIL: arm
+// RUN: %clangxx   %s -o %t && %run %t 2>&1 | FileCheck %s
+// RUN: %clangxx_asan  %s -o %t && %run %t 2>&1 | FileCheck %s
+// RUN: %clangxx_asan -fno-sanitize-address-poison-custom-array-cookie %s -o %t && %run %t 2>&1 | FileCheck %s
+// RUN: %clangxx_asan -fsanitize-address-poison-custom-array-cookie%s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=COOKIE
 
-// UNSUPPORTED: ios
-
-#include 
-#include 
-#include 
-#include 
 #include 
+#include 
+
 struct Foo {
   void *operator new(size_t s) { return Allocate(s); }
   void *operator new[] (size_t s) { return Allocate(s); }
@@ -22,7 +28,6 @@
   }
 };
 
-Foo::~Foo() {}
 void *Foo::allocated;
 
 Foo *getFoo(size_t n) {
@@ -33,8 +38,12 @@
   Foo *foo = getFoo(10);
   fprintf(stderr, "foo  : %p\n", foo);
   fprintf(stderr, "alloc: %p\n", Foo::allocated);
-  assert(reinterpret_cast(foo) ==
- reinterpret_cast(Foo::allocated) + sizeof(void*));
-  *reinterpret_cast(Foo::allocated) = 42;
+  reinterpret_cast(foo)[-1] = 42;
+// COOKIE: AddressSanitizer: heap-buffer-overflow
+// COOKIE: in main {{.*}}new_array_cookie_with_new_from_class.cpp:[[@LINE-2]]
+// COOKIE: is located {{7 bytes inside of 18|15 bytes inside of 26}}-byte region
+  printf("Unsurprisingly, we were able to write to the array cookie\n");
+// CHECK: Unsurprisingly, we were able to write to the array cookie
+
   return 0;
 }
Index: compiler-rt/test/asan/TestCases/Posix/new_array_cookie_uaf_test.cpp
===
--- compiler-rt/test/asan/TestCases/Posix/new_array_cookie_uaf_test.cpp
+++ compiler-rt/test/asan/TestCases/Posix/new_array_cookie_uaf_test.cpp
@@ -3,12 +3,10 @@
 // RUN: %env_asan_opts=poison_array_cookie=1 not %run %t 2>&1  | FileCheck %s --check-prefix=COOKIE
 // RUN: %env_asan_opts=poison_array_cookie=0 not %run %t 2>&1  | FileCheck %s --check-prefix=NO_COOKIE
 
-// UNSUPPORTED: ios
-
-#include 
-#include 
 #include 
-int dtor_counter;
+#include 
+
+int dtor_counter=0;
 struct C {
   int x;
   ~C() {
@@ -19,12 +17,11 @@
 
 __attribute__((noinline)) void Delete(C *c) { delete[] c; }
 __attribute__((no_sanitize_address)) void Write42ToCookie(C *c) {
-  long *p = reinterpret_cast(c);
-  p[-1] = 42;
+  reinterpret_cast(c)[-1] = 42;
 }
 
 int main(int argc, char **argv) {
-  C *buffer = new C[argc];
+  C *buffer = new C[1];
   delete [] buffer;
   Write42ToCookie(buffer);
   delete [] buffer;
Index: compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp
===
--- compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp
+++ compiler-rt/test/asan/TestCases/Posix/new_array_cookie_test.cpp
@@ -1,13 +1,12 @@
 // REQUIRES: asan-64-bits
 // RUN: %clangxx_asan -O3 %s -o %t
-// RUN:not %run %t 2>&1  | FileCheck %s
+// RUN:  not %run %t 2>&1  | FileCheck %s
 // RUN: %env_asan_opts=poison_array_cookie=1 not %run %t 2>&1  | FileCheck %s
 // RUN: %env_asan_opts=poison_array_cookie=0 not %run %t 2>&1  | FileCheck %s 

[PATCH] D124650: [clang-tidy] Simplify boolean expressions by DeMorgan's theorem

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood abandoned this revision.
LegalizeAdulthood added a comment.

Discarding in favor of Nathan James's improved implementation.


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

https://reviews.llvm.org/D124650

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

[PATCH] D124806: [clang-tidy] add support for Demorgan conversions to readability-simplify-bool-expr

[Richard via Phabricator via cfe-commits]

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

LGTM!  Thanks for taking my idea and implementing it much better than what I 
had `:)`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124806

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

[clang] e64722f - [CMake][Fuchsia] Build runtimes as universal libraries on OS X

[Petr Hosek via cfe-commits]

Author: Petr Hosek
Date: 2022-05-18T18:14:42Z
New Revision: e64722f686bd8a6d0c2c099ce6fb90e84f51341d

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

LOG: [CMake][Fuchsia] Build runtimes as universal libraries on OS X

We want to build libunwind, libc++abi and libc++ as universal libraries
supporting both x86_64 and arm64 architectures.

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

Added: 


Modified: 
clang/cmake/caches/Fuchsia-stage2.cmake
clang/cmake/caches/Fuchsia.cmake

Removed: 




diff  --git a/clang/cmake/caches/Fuchsia-stage2.cmake 
b/clang/cmake/caches/Fuchsia-stage2.cmake
index 5d9257951e71d..6a00556e977f4 100644
--- a/clang/cmake/caches/Fuchsia-stage2.cmake
+++ b/clang/cmake/caches/Fuchsia-stage2.cmake
@@ -61,10 +61,11 @@ if(APPLE)
   set(LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "")
   set(LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
   set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
-  set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
+  set(LIBCXX_ABI_VERSION 2 CACHE STRING "")
   set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
   set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
-  set(LIBCXX_ABI_VERSION 2 CACHE STRING "")
+  set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
+  set(RUNTIMES_CMAKE_ARGS 
"-DCMAKE_OSX_DEPLOYMENT_TARGET=10.13;-DCMAKE_OSX_ARCHITECTURES=arm64|x86_64" 
CACHE STRING "")
 endif()
 
 if(WIN32)

diff  --git a/clang/cmake/caches/Fuchsia.cmake 
b/clang/cmake/caches/Fuchsia.cmake
index f7b51d5861335..7b534e61f9520 100644
--- a/clang/cmake/caches/Fuchsia.cmake
+++ b/clang/cmake/caches/Fuchsia.cmake
@@ -69,7 +69,9 @@ else()
   set(LIBCXX_ABI_VERSION 2 CACHE STRING "")
   set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
   set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
+  set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
   set(LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi;libunwind" CACHE 
STRING "")
+  set(RUNTIMES_CMAKE_ARGS 
"-DCMAKE_OSX_DEPLOYMENT_TARGET=10.13;-DCMAKE_OSX_ARCHITECTURES=arm64|x86_64" 
CACHE STRING "")
 endif()
 
 if(BOOTSTRAP_CMAKE_SYSTEM_NAME)



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

[PATCH] D125908: [CMake][Fuchsia] Build runtimes as universal libraries on OS X

[Petr Hosek via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe64722f686bd: [CMake][Fuchsia] Build runtimes as universal 
libraries on OS X (authored by phosek).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125908

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake
  clang/cmake/caches/Fuchsia.cmake


Index: clang/cmake/caches/Fuchsia.cmake
===
--- clang/cmake/caches/Fuchsia.cmake
+++ clang/cmake/caches/Fuchsia.cmake
@@ -69,7 +69,9 @@
   set(LIBCXX_ABI_VERSION 2 CACHE STRING "")
   set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
   set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
+  set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
   set(LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi;libunwind" CACHE 
STRING "")
+  set(RUNTIMES_CMAKE_ARGS 
"-DCMAKE_OSX_DEPLOYMENT_TARGET=10.13;-DCMAKE_OSX_ARCHITECTURES=arm64|x86_64" 
CACHE STRING "")
 endif()
 
 if(BOOTSTRAP_CMAKE_SYSTEM_NAME)
Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -61,10 +61,11 @@
   set(LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "")
   set(LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
   set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
-  set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
+  set(LIBCXX_ABI_VERSION 2 CACHE STRING "")
   set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
   set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
-  set(LIBCXX_ABI_VERSION 2 CACHE STRING "")
+  set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
+  set(RUNTIMES_CMAKE_ARGS 
"-DCMAKE_OSX_DEPLOYMENT_TARGET=10.13;-DCMAKE_OSX_ARCHITECTURES=arm64|x86_64" 
CACHE STRING "")
 endif()
 
 if(WIN32)


Index: clang/cmake/caches/Fuchsia.cmake
===
--- clang/cmake/caches/Fuchsia.cmake
+++ clang/cmake/caches/Fuchsia.cmake
@@ -69,7 +69,9 @@
   set(LIBCXX_ABI_VERSION 2 CACHE STRING "")
   set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
   set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
+  set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
   set(LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING "")
+  set(RUNTIMES_CMAKE_ARGS "-DCMAKE_OSX_DEPLOYMENT_TARGET=10.13;-DCMAKE_OSX_ARCHITECTURES=arm64|x86_64" CACHE STRING "")
 endif()
 
 if(BOOTSTRAP_CMAKE_SYSTEM_NAME)
Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -61,10 +61,11 @@
   set(LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "")
   set(LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
   set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
-  set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
+  set(LIBCXX_ABI_VERSION 2 CACHE STRING "")
   set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
   set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
-  set(LIBCXX_ABI_VERSION 2 CACHE STRING "")
+  set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
+  set(RUNTIMES_CMAKE_ARGS "-DCMAKE_OSX_DEPLOYMENT_TARGET=10.13;-DCMAKE_OSX_ARCHITECTURES=arm64|x86_64" CACHE STRING "")
 endif()
 
 if(WIN32)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125912: [Clang][[OpenMP5.1] Initial parser/sema for default(private) clause

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 created this revision.
jyu2 added reviewers: ABataev, mikerice, jdoerfert.
jyu2 added projects: OpenMP, clang.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
jyu2 requested review of this revision.
Herald added subscribers: llvm-commits, sstefan1.
Herald added a project: LLVM.

This implements the default(private) clause as defined in OMP5.1


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125912

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/default_firstprivate_ast_print.cpp
  clang/test/OpenMP/default_private_ast_print.cpp
  clang/test/OpenMP/distribute_parallel_for_default_messages.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_default_messages.cpp
  clang/test/OpenMP/parallel_default_messages.cpp
  clang/test/OpenMP/parallel_for_default_messages.cpp
  clang/test/OpenMP/parallel_for_simd_default_messages.cpp
  clang/test/OpenMP/parallel_master_codegen.cpp
  clang/test/OpenMP/parallel_master_default_messages.cpp
  clang/test/OpenMP/parallel_sections_default_messages.cpp
  clang/test/OpenMP/target_parallel_default_messages.cpp
  clang/test/OpenMP/target_parallel_for_default_messages.cpp
  clang/test/OpenMP/target_parallel_for_simd_default_messages.cpp
  clang/test/OpenMP/target_teams_default_messages.cpp
  clang/test/OpenMP/target_teams_distribute_default_messages.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_default_messages.cpp
  
clang/test/OpenMP/target_teams_distribute_parallel_for_simd_default_messages.cpp
  clang/test/OpenMP/task_default_messages.cpp
  clang/test/OpenMP/teams_default_messages.cpp
  clang/test/OpenMP/teams_distribute_default_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_default_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_simd_default_messages.cpp
  clang/test/OpenMP/teams_distribute_simd_default_messages.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def

Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -1032,6 +1032,7 @@
 
 __OMP_DEFAULT_KIND(none)
 __OMP_DEFAULT_KIND(shared)
+__OMP_DEFAULT_KIND(private)
 __OMP_DEFAULT_KIND(firstprivate)
 __OMP_DEFAULT_KIND(unknown)
 
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -4122,12 +4122,19 @@
 })";
   EXPECT_TRUE(notMatchesWithOpenMP51(Source4, Matcher));
 
-  const std::string Source5 = R"(
+  StringRef Source5 = R"(
+void x(int x) {
+#pragma omp parallel default(private)
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP51(Source5, Matcher));
+
+  const std::string Source6 = R"(
 void x(int x) {
 #pragma omp parallel num_threads(x)
 ;
 })";
-  EXPECT_TRUE(notMatchesWithOpenMP(Source5, Matcher));
+  EXPECT_TRUE(notMatchesWithOpenMP(Source6, Matcher));
 }
 
 TEST_P(ASTMatchersTest, OMPDefaultClause_IsSharedKind) {
@@ -4168,12 +4175,19 @@
 })";
   EXPECT_TRUE(notMatchesWithOpenMP51(Source4, Matcher));
 
-  const std::string Source5 = R"(
+  StringRef Source5 = R"(
+void x(int x) {
+#pragma omp parallel default(private)
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP51(Source5, Matcher));
+
+  const std::string Source6 = R"(
 void x(int x) {
 #pragma omp parallel num_threads(x)
 ;
 })";
-  EXPECT_TRUE(notMatchesWithOpenMP(Source5, Matcher));
+  EXPECT_TRUE(notMatchesWithOpenMP(Source6, Matcher));
 }
 
 TEST(OMPDefaultClause, isFirstPrivateKind) {
@@ -4216,10 +4230,70 @@
 
   const std::string Source5 = R"(
 void x(int x) {
+#pragma omp parallel default(private)
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP51(Source5, Matcher));
+
+  const std::string Source6 = R"(
+void x(int x) {
 #pragma omp parallel num_threads(x)
 ;
 })";
-  EXPECT_TRUE(notMatchesWithOpenMP(Source5, Matcher));
+  EXPECT_TRUE(notMatchesWithOpenMP(Source6, Matcher));
+}
+
+TEST(OMPDefaultClause, istPrivateKind) {
+  auto Matcher =
+  ompExecutableDirective(hasAnyClause(ompDefaultClause(isPrivateKind(;
+
+  const std::string Source0 = R"(
+void x() {
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source0, Matcher));
+
+  const std::string Source1 = R"(
+void x() {
+#pragma omp parallel
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source1, Matcher));
+
+  const std::string Source2 = R"(
+void x() {
+#pragma omp parallel default(shared)
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source2, Matcher));
+
+  const std::string Source3 = R"(
+void x() {
+#pragma omp parallel default(none)
+;
+})";
+  EXPECT_TRUE(notMatchesWithOpenMP(Source3, Matcher));
+
+  

[PATCH] D125909: [AMDGPU] emit macro __GFX9__ etc

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/lib/Basic/Targets/AMDGPU.cpp:414
+  assert(CanonName.startswith("gfx") && "Invalid amdgcn canonical name");
+  Builder.defineMacro(Twine("__") + Twine(CanonName.drop_back(2).upper()) +
+  Twine("__"));

tra wrote:
> Without an example of the input we're dealing with here it was not obvious 
> that the idea here is to convert `gfx` -> `__GFX__`. I'd change the 
> comment a bit : `e.g  gfx901 ->__GFX9__, gfx1033 -> __GFX10__, etc.`
will do when committing


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

https://reviews.llvm.org/D125909

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

[PATCH] D125840: [Analyzer] Removed extra space from NSErrorChecker debug message and updated relevant tests

[Usama Hameed via Phabricator via cfe-commits]

usama54321 updated this revision to Diff 430450.
usama54321 added a comment.

Updated tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125840

Files:
  clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
  clang/test/Analysis/CheckNSError.m
  clang/test/Analysis/incorrect-checker-names.mm


Index: clang/test/Analysis/incorrect-checker-names.mm
===
--- clang/test/Analysis/incorrect-checker-names.mm
+++ clang/test/Analysis/incorrect-checker-names.mm
@@ -107,7 +107,7 @@
 
 void foo(CFErrorRef* error) { // expected-warning{{Function accepting 
CFErrorRef* should have a non-void return value to indicate whether or not an 
error occurred [osx.coreFoundation.CFError]}}
   // FIXME: This shouldn't be tied to a modeling checker.
-  *error = 0; // expected-warning {{Potential null dereference.  According to 
coding standards documented in CoreFoundation/CFError.h the parameter may be 
null [osx.coreFoundation.CFError]}}
+  *error = 0; // expected-warning {{Potential null dereference. According to 
coding standards documented in CoreFoundation/CFError.h the parameter may be 
null [osx.coreFoundation.CFError]}}
 }
 
 @interface A
@@ -116,7 +116,7 @@
 
 @implementation A
 - (void)myMethodWhichMayFail:(NSError **)error {  // 
expected-warning {{Method accepting NSError** should have a non-void return 
value to indicate whether or not an error occurred [osx.cocoa.NSError]}}
-  *error = [NSError errorWithDomain:@"domain" code:1 userInfo:0]; // 
expected-warning {{Potential null dereference.  According to coding standards 
in 'Creating and Returning NSError Objects' the parameter may be null 
[osx.cocoa.NSError]}}
+  *error = [NSError errorWithDomain:@"domain" code:1 userInfo:0]; // 
expected-warning {{Potential null dereference. According to coding standards in 
'Creating and Returning NSError Objects' the parameter may be null 
[osx.cocoa.NSError]}}
 }
 @end
 
Index: clang/test/Analysis/CheckNSError.m
===
--- clang/test/Analysis/CheckNSError.m
+++ clang/test/Analysis/CheckNSError.m
@@ -27,7 +27,7 @@
 
 @implementation A
 - (void)myMethodWhichMayFail:(NSError **)error {   // expected-warning 
{{Method accepting NSError** should have a non-void return value to indicate 
whether or not an error occurred}}
-  *error = [NSError errorWithDomain:@"domain" code:1 userInfo:0]; // 
expected-warning {{Potential null dereference}}
+  *error = [NSError errorWithDomain:@"domain" code:1 userInfo:0]; // 
expected-warning {{Potential null dereference. According to coding standards in 
'Creating and Returning NSError Objects' the parameter may be null 
[osx.cocoa.NSError]}}
 }
 
 - (BOOL)myMethodWhichMayFail2:(NSError **)error {  // no-warning
@@ -50,7 +50,7 @@
 typedef struct __CFError* CFErrorRef;
 
 void foo(CFErrorRef* error) { // expected-warning {{Function accepting 
CFErrorRef* should have a non-void return value to indicate whether or not an 
error occurred}}
-  *error = 0;  // expected-warning {{Potential null dereference}}
+  *error = 0;  // expected-warning {{Potential null dereference. According to 
coding standards documented in CoreFoundation/CFError.h the parameter may be 
null [osx.coreFoundation.CFError]}}
 }
 
 int f1(CFErrorRef* error) {
@@ -74,7 +74,7 @@
 }
 
 int __attribute__((nonnull(1))) f5(int *x, CFErrorRef *error) {
-  *error = 0; // expected-warning {{Potential null dereference}}
+  *error = 0; // expected-warning {{Potential null dereference. According to 
coding standards documented in CoreFoundation/CFError.h the parameter may be 
null [osx.coreFoundation.CFError]}}
   return 0;
 }
 
Index: clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
@@ -266,7 +266,7 @@
   SmallString<128> Buf;
   llvm::raw_svector_ostream os(Buf);
 
-  os << "Potential null dereference.  According to coding standards ";
+  os << "Potential null dereference. According to coding standards ";
   os << (isNSError
  ? "in 'Creating and Returning NSError Objects' the parameter"
  : "documented in CoreFoundation/CFError.h the parameter");


Index: clang/test/Analysis/incorrect-checker-names.mm
===
--- clang/test/Analysis/incorrect-checker-names.mm
+++ clang/test/Analysis/incorrect-checker-names.mm
@@ -107,7 +107,7 @@
 
 void foo(CFErrorRef* error) { // expected-warning{{Function accepting CFErrorRef* should have a non-void return value to indicate whether or not an error occurred [osx.coreFoundation.CFError]}}
   // FIXME: This shouldn't be tied to a modeling checker.
-  *error = 0; // expected-warning {{Potential null dereference.  According to 

[PATCH] D125909: [AMDGPU] emit macro __GFX9__ etc

[Artem Belevich via Phabricator via cfe-commits]

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



Comment at: clang/lib/Basic/Targets/AMDGPU.cpp:414
+  assert(CanonName.startswith("gfx") && "Invalid amdgcn canonical name");
+  Builder.defineMacro(Twine("__") + Twine(CanonName.drop_back(2).upper()) +
+  Twine("__"));

Without an example of the input we're dealing with here it was not obvious that 
the idea here is to convert `gfx` -> `__GFX__`. I'd change the comment 
a bit : `e.g  gfx901 ->__GFX9__, gfx1033 -> __GFX10__, etc.`


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

https://reviews.llvm.org/D125909

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

[PATCH] D125829: [clang] Fix __has_builtin

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:2555-2557
 if (FeatureList.empty())
   return;
 assert(!FeatureList.contains(' ') && "Space in feature list");

tra wrote:
> Should we move that into the `evaluateRequiredTargetFeatures` ? 
> 
> `Empty feature list -> true` sounds like something that belongs to the 
> evaluation logic. So does the assertion on the no-spaces assumption we make 
> about the input.
> 
will do when committing


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

https://reviews.llvm.org/D125829

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

[PATCH] D124382: [Clang] Recognize target address space in superset calculation

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D124382#3496417 , @jchlanda wrote:

> In D124382#3480600 , @Anastasia 
> wrote:
>
>> 
>
>
>
>> And I think we could add this feature in a very light way for example by 
>> reserving the numbers from the clang `LangAS` enum to be used with the 
>> language address spaces in the prototypes of builtins.
>
> I'm not sure I understand how that would look, could you please elaborate?

We could reserve the IDs from LangAS enums to be used in 
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/Builtins.def#L65,
 but we could also extend the syntax more naturally i.e. numbers could be used 
for target address spaces and we could add some letter based syntax for 
language address spaces.

>> Just to understand further - do you need the builtins to have a specific 
>> address space prototype? And do you need this to improve error handling in 
>> libclc code base?
>
> With the wrappers suggested we could declare all the pointers to be in 
> generic AS and that would get us around the target vs language AS problem. I 
> don't think that would improve the situation, as from llvm perspective/use 
> case all those builtins would be incorrect and there would be no way for 
> users to tell that there is a specific AS requirement on them, nor would the 
> compiler be able to warn/error. Then the only thing making it work would be 
> those wrappers, embedded deeply in the source of libclc, which at the moment 
> is not even shipped with upstream llvm.
> The builtins in question are not exclusively OpenCL/SYCL related, say 
> `TARGET_BUILTIN(__nvvm_cp_async_ca_shared_global_16, "vv*3vC*1", "", 
> AND(SM_80,PTX70))` would need to take both pointer in a generic address space 
> here. It feels like explicitly providing AS in the prototype is needed.

My understanding was that Clang low level builtins are not desirable for uses 
in the application code directly. They are more targeted at tooling developers 
and low level libraries uses.  Which is why this problem has been worked around 
by declaring the wrapper overload with correct address spaces in the tooling 
projects.

My understanding was that you need those builtins in the libclc code?

>> this for example won’t work for builtins that are shared between targets.
>
> While I agree in principle, I'm not sure if there are any target agnostic and 
> AS specific builtins, sounds like a dangerous thing to introduce. And in any 
> case, it's the target that provides the AS map.

In OpenCL we actually have quite some target agnostic builtins.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124382

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

[PATCH] D125829: [clang] Fix __has_builtin

[Artem Belevich via Phabricator via cfe-commits]

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

LGTM with a possible nit.




Comment at: clang/lib/Basic/BuiltinTargetFeatures.h:32
+/// pairs.
+class TargetFeatures {
+  struct FeatureListStatus {

yaxunl wrote:
> tra wrote:
> > I don't think it needs to be part of the public API. Feature check function 
> > operates on `llvm::StringMap` and does not need to know about this 
> > class, and the implementation of this class can be moved into 
> > `Builtins.cpp` where it's actually used.
> It is not a public API since it is under lib/Basic.
> 
> The reason it is made a header file because we have a unit test 
> clang/unittests/CodeGen/CheckTargetFeaturesTest.cpp for class TargetFeatures. 
> This is because TargetFeatures is relatively complicated.
Got it. I've missed the test. 



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:2555-2557
 if (FeatureList.empty())
   return;
 assert(!FeatureList.contains(' ') && "Space in feature list");

Should we move that into the `evaluateRequiredTargetFeatures` ? 

`Empty feature list -> true` sounds like something that belongs to the 
evaluation logic. So does the assertion on the no-spaces assumption we make 
about the input.



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

https://reviews.llvm.org/D125829

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

[libunwind] bedf657 - [runtimes] Default LIB*_HERMETIC_STATIC_LIBRARY to ON on Windows

[Martin Storsjö via cfe-commits]

Author: Martin Storsjö
Date: 2022-05-18T20:31:51+03:00
New Revision: bedf657d0f4c54ffe9ef4303382657a74296b544

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

LOG: [runtimes] Default LIB*_HERMETIC_STATIC_LIBRARY to ON on Windows

(In the case of libunwind, the cmake option is called
LIBUNWIND_HIDE_SYMBOLS, but it has the same effect as
LIBCXX_HERMETIC_STATIC_LIBRARY and
LIBCXXABI_HERMETIC_STATIC_LIBRARY.)

Previously, the same issue was dealt with by setting a project wide
define (_LIBUNWIND_HIDE_SYMBOLS,
_LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS and
_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS) if only building a static
library.  If building both static and shared at the same time, this
wasn't set, and the static library would contain dllexport directives.

The LIB*_HERMETIC_STATIC_LIBRARY and LIBUNWIND_HIDE_SYMBOLS cmake
options only apply the defines to the static library in the build,
even if building both static and shared at the same time.

(This could only be done use after the object libraries were
enabled, as a shared libcxx needs libcxxabi object files built
with dllexports included.)

This allows removing inelegant code for deciding how to build the
libcxxabi static library and a TODO comment that suggested that
users should need to start setting an option, which they shouldn't
need to. Finally, this gets rid of two XFAILs in tests.

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

Added: 


Modified: 
libcxx/CMakeLists.txt
libcxx/src/CMakeLists.txt
libcxx/test/libcxx/vendor/clang-cl/static-lib-exports.sh.cpp
libcxx/test/libcxx/vendor/mingw/static-lib-exports.sh.cpp
libcxxabi/CMakeLists.txt
libcxxabi/src/CMakeLists.txt
libunwind/CMakeLists.txt

Removed: 




diff  --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index 2b2c42cf8ff6e..ec4c2e408ca90 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -315,8 +315,12 @@ endif()
 option(LIBCXX_CONFIGURE_IDE "Configure libcxx for use within an IDE"
   ${LIBCXX_CONFIGURE_IDE_DEFAULT})
 
+set(LIBCXX_HERMETIC_STATIC_LIBRARY_DEFAULT OFF)
+if (WIN32)
+  set(LIBCXX_HERMETIC_STATIC_LIBRARY_DEFAULT ON)
+endif()
 option(LIBCXX_HERMETIC_STATIC_LIBRARY
-  "Do not export any symbols from the static library." OFF)
+  "Do not export any symbols from the static library." 
${LIBCXX_HERMETIC_STATIC_LIBRARY_DEFAULT})
 
 
#===
 # Check option configurations

diff  --git a/libcxx/src/CMakeLists.txt b/libcxx/src/CMakeLists.txt
index 962f6f54e28f2..42f41719eec23 100644
--- a/libcxx/src/CMakeLists.txt
+++ b/libcxx/src/CMakeLists.txt
@@ -300,7 +300,9 @@ if (LIBCXX_ENABLE_STATIC)
   append_flags_if_supported(CXX_STATIC_LIBRARY_FLAGS 
-fvisibility-global-new-delete-hidden)
 endif()
 target_compile_options(cxx_static PRIVATE ${CXX_STATIC_LIBRARY_FLAGS})
-target_compile_definitions(cxx_static PRIVATE 
_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS)
+# _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS can be defined in __config_site
+# too. Define it in the same way here, to avoid redefinition conflicts.
+target_compile_definitions(cxx_static PRIVATE 
_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS=)
   endif()
 
   list(APPEND LIBCXX_BUILD_TARGETS "cxx_static")

diff  --git a/libcxx/test/libcxx/vendor/clang-cl/static-lib-exports.sh.cpp 
b/libcxx/test/libcxx/vendor/clang-cl/static-lib-exports.sh.cpp
index 35f2221bf1c25..fb66c4a28fa08 100644
--- a/libcxx/test/libcxx/vendor/clang-cl/static-lib-exports.sh.cpp
+++ b/libcxx/test/libcxx/vendor/clang-cl/static-lib-exports.sh.cpp
@@ -12,8 +12,3 @@
 // directives in clang-cl builds.
 
 // RUN: llvm-readobj --coff-directives "%{lib}/libc++.lib" | not grep -i 
"export:" > /dev/null
-
-// It's a known issue, that when building a shared library at the same time
-// as the static library, the generated static library does contain dllexport
-// directives.
-// XFAIL: windows-dll

diff  --git a/libcxx/test/libcxx/vendor/mingw/static-lib-exports.sh.cpp 
b/libcxx/test/libcxx/vendor/mingw/static-lib-exports.sh.cpp
index 7af240ebe2854..7dfedb761f0c7 100644
--- a/libcxx/test/libcxx/vendor/mingw/static-lib-exports.sh.cpp
+++ b/libcxx/test/libcxx/vendor/mingw/static-lib-exports.sh.cpp
@@ -12,8 +12,3 @@
 // directives in MinGW builds.
 
 // RUN: llvm-readobj --coff-directives "%{lib}/libc++.a" | not grep -i 
"export:" > /dev/null
-
-// It's a known issue, that when building a shared library at the same time
-// as the static library, the generated static library does contain dllexport
-// directives.
-// XFAIL: windows-dll

diff  --git a/libcxxabi/CMakeLists.txt b/libcxxabi/CMakeLists.txt
index aa5ab9a0086b2..791ff6c75df72 100644
--- a/libcxxabi/CMakeLists.txt
+++ b/libcxxabi/CMakeLists.txt
@@ -136,8 

[PATCH] D125911: [pseudo] (trivial) bracket-matching

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

The Bracket struct & translation between token/bracket streams is not really 
justified by the implementation in this patch.
Next patch will run nontrivial algorithms on the bracket strings.

Happy to scope this patch down (avoid that struct) or up (include the full 
algorithm) if you prefer, but I think this keeps the diffs clearest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125911

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

[PATCH] D122256: [clang][ABI] New C++20 module mangling scheme

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

The differential has the same intention/subject as D118352 
. In such a case reusing the preexisting 
differential would have been better. I have left more comments on 
https://reviews.llvm.org/D125825#3522911

Note: a revert was committed (https://reviews.llvm.org/D118352#3368921) in 11 
hours after the problem was reported.
It was fairly legitimate. The user pushing the revert helps maintain build bots 
green. (Some groups may be more aggressive, e.g. they may leave a comment and 
perform a revert in 10 minutes.
Technically that is also fine, but sometimes can be done better.)
In this particular case, I believe the problem was just a Mach-O platform 
differential that Mach-O IR output doesn't use `dso_local `. This is very 
common when adding new tests to `test/clang/CodeGen*` and contributors often 
neglect it.
If I had seen the failure in time, I would have tried fixing the test as it is 
quite straightforward, instead of reverting it.

(Thanks for adding #clang-language-wg 
 since this was a patch that 
some folks would like to subscribe to.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122256

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

[PATCH] D125911: [pseudo] (trivial) bracket-matching

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added a subscriber: mgorny.
Herald added a project: All.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, alextsao1999.
Herald added a project: clang-tools-extra.

Error-tolerant bracket matching enables our error-tolerant parsing strategies.
The implementation here is *not* yet error tolerant: this patch sets up the APIs
and plumbing, and describes the planned approach.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125911

Files:
  clang-tools-extra/pseudo/benchmarks/Benchmark.cpp
  clang-tools-extra/pseudo/include/clang-pseudo/Bracket.h
  clang-tools-extra/pseudo/include/clang-pseudo/Token.h
  clang-tools-extra/pseudo/lib/Bracket.cpp
  clang-tools-extra/pseudo/lib/CMakeLists.txt
  clang-tools-extra/pseudo/tool/ClangPseudo.cpp
  clang-tools-extra/pseudo/unittests/BracketTest.cpp
  clang-tools-extra/pseudo/unittests/CMakeLists.txt

Index: clang-tools-extra/pseudo/unittests/CMakeLists.txt
===
--- clang-tools-extra/pseudo/unittests/CMakeLists.txt
+++ clang-tools-extra/pseudo/unittests/CMakeLists.txt
@@ -1,9 +1,11 @@
 set(LLVM_LINK_COMPONENTS
   Support
+  TestingSupport
   )
 
 add_custom_target(ClangPseudoUnitTests)
 add_unittest(ClangPseudoUnitTests ClangPseudoTests
+  BracketTest.cpp
   DirectiveTreeTest.cpp
   ForestTest.cpp
   GLRTest.cpp
Index: clang-tools-extra/pseudo/unittests/BracketTest.cpp
===
--- /dev/null
+++ clang-tools-extra/pseudo/unittests/BracketTest.cpp
@@ -0,0 +1,109 @@
+//===--- BracketTest.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
+//
+//===--===//
+
+#include "clang-pseudo/Bracket.h"
+#include "clang-pseudo/Token.h"
+#include "clang/Basic/LangOptions.h"
+#include "llvm/Testing/Support/Annotations.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace pseudo {
+
+// Return a version of Code with each paired bracket marked with ^.
+std::string decorate(llvm::StringRef Code, const TokenStream ) {
+  std::string Result;
+  const char *Pos = Code.data();
+  for (const Token  : Stream.tokens()) {
+if (Tok.Pair == 0)
+  continue;
+const char *NewPos = Tok.text().begin();
+assert(NewPos >= Code.begin() && NewPos < Code.end());
+Result.append(Pos, NewPos - Pos);
+Result.push_back('^');
+Pos = NewPos;
+  }
+  Result.append(Pos, Code.end() - Pos);
+  return Result;
+}
+
+// Checks that the brackets matched in Stream are those annotated in MarkedCode.
+void verifyMatchedSet(llvm::StringRef Code, llvm::StringRef MarkedCode,
+  const TokenStream ) {
+  EXPECT_EQ(MarkedCode, decorate(Code, Stream));
+}
+
+// Checks that paired brackets within the stream nest properly.
+void verifyNesting(const TokenStream ) {
+  std::vector Stack;
+  for (const auto  : Stream.tokens()) {
+if (Tok.Pair > 0)
+  Stack.push_back();
+else if (Tok.Pair < 0) {
+  ASSERT_FALSE(Stack.empty()) << Tok;
+  ASSERT_EQ(Stack.back(), Tok.pair())
+  << *Stack.back() << " != " << *Tok.pair() << " = pair of " << Tok;
+  Stack.pop_back();
+}
+  }
+  ASSERT_THAT(Stack, testing::IsEmpty());
+}
+
+// Checks that ( pairs with a ) on its right, etc.
+void verifyMatchKind(const TokenStream ) {
+  for (const auto  : Stream.tokens()) {
+if (Tok.Pair == 0)
+  continue;
+auto Want = [&]() -> std::pair {
+  switch (Tok.Kind) {
+  case tok::l_paren:
+return {true, tok::r_paren};
+  case tok::r_paren:
+return {false, tok::l_paren};
+  case tok::l_brace:
+return {true, tok::r_brace};
+  case tok::r_brace:
+return {false, tok::l_brace};
+  case tok::l_square:
+return {true, tok::r_square};
+  case tok::r_square:
+return {false, tok::l_square};
+  default:
+ADD_FAILURE() << "Paired non-bracket " << Tok;
+return {false, tok::eof};
+  }
+}();
+EXPECT_EQ(Tok.Pair > 0, Want.first) << Tok;
+EXPECT_EQ(Tok.pair()->Kind, Want.second) << Tok;
+  }
+}
+
+// Verifies an expected bracket pairing like: ^( [ ^)
+// The input is annotated code, with the brackets expected to be matched marked.
+void verifyBrackets(llvm::StringRef MarkedCode) {
+  SCOPED_TRACE(MarkedCode);
+  llvm::Annotations A(MarkedCode);
+  std::string Code = A.code().str();
+  LangOptions LangOpts;
+  auto Stream = lex(Code, LangOpts);
+  pairBrackets(Stream);
+
+  verifyMatchedSet(Code, MarkedCode, Stream);
+  verifyNesting(Stream);
+  verifyMatchKind(Stream);
+}
+
+TEST(Bracket, SimplePair) {
+  verifyBrackets("^{ ^[ 

[PATCH] D125909: [AMDGPU] emit macro __GFX9__ etc

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: b-sumner, tra, arsenm.
Herald added subscribers: kosarev, kerbowa, t-tye, tpr, dstuttard, jvesely, 
kzhuravl.
Herald added a project: All.
yaxunl requested review of this revision.
Herald added a subscriber: wdng.

Emit predefined macros for GPU family. e.g. 
for GPU gfx9xx emit `__GFX9__`, etc.


https://reviews.llvm.org/D125909

Files:
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/test/Driver/amdgpu-macros.cl

Index: clang/test/Driver/amdgpu-macros.cl
===
--- clang/test/Driver/amdgpu-macros.cl
+++ clang/test/Driver/amdgpu-macros.cl
@@ -67,63 +67,63 @@
 // AMDGCN-based processors.
 //
 
-// RUN: %clang -E -dM -target amdgcn -mcpu=gfx600 %s 2>&1 | FileCheck --check-prefixes=ARCH-GCN,FAST_FMAF %s -DWAVEFRONT_SIZE=64 -DCPU=gfx600
-// RUN: %clang -E -dM -target amdgcn -mcpu=tahiti %s 2>&1 | FileCheck --check-prefixes=ARCH-GCN,FAST_FMAF %s -DWAVEFRONT_SIZE=64 -DCPU=gfx600
-// RUN: %clang -E -dM -target amdgcn -mcpu=gfx601 %s 2>&1 | FileCheck --check-prefixes=ARCH-GCN,SLOW_FMAF %s -DWAVEFRONT_SIZE=64 -DCPU=gfx601
-// RUN: %clang -E -dM -target amdgcn -mcpu=pitcairn %s 2>&1 | FileCheck --check-prefix=ARCH-GCN %s -DWAVEFRONT_SIZE=64 -DCPU=gfx601
-// RUN: %clang -E -dM -target amdgcn -mcpu=verde %s 2>&1 | FileCheck --check-prefix=ARCH-GCN %s -DWAVEFRONT_SIZE=64 -DCPU=gfx601
-// RUN: %clang -E -dM -target amdgcn -mcpu=gfx602 %s 2>&1 | FileCheck --check-prefix=ARCH-GCN %s -DWAVEFRONT_SIZE=64 -DCPU=gfx602
-// RUN: %clang -E -dM -target amdgcn -mcpu=hainan %s 2>&1 | FileCheck --check-prefix=ARCH-GCN %s -DWAVEFRONT_SIZE=64 -DCPU=gfx602
-// RUN: %clang -E -dM -target amdgcn -mcpu=oland %s 2>&1 | FileCheck --check-prefix=ARCH-GCN %s -DWAVEFRONT_SIZE=64 -DCPU=gfx602
-// RUN: %clang -E -dM -target amdgcn -mcpu=gfx700 %s 2>&1 | FileCheck --check-prefix=ARCH-GCN %s -DWAVEFRONT_SIZE=64 -DCPU=gfx700
-// RUN: %clang -E -dM -target amdgcn -mcpu=kaveri %s 2>&1 | FileCheck --check-prefix=ARCH-GCN %s -DWAVEFRONT_SIZE=64 -DCPU=gfx700
-// RUN: %clang -E -dM -target amdgcn -mcpu=gfx701 %s 2>&1 | FileCheck --check-prefixes=ARCH-GCN,FAST_FMAF %s -DWAVEFRONT_SIZE=64 -DCPU=gfx701
-// RUN: %clang -E -dM -target amdgcn -mcpu=hawaii %s 2>&1 | FileCheck --check-prefixes=ARCH-GCN,FAST_FMAF %s -DWAVEFRONT_SIZE=64 -DCPU=gfx701
-// RUN: %clang -E -dM -target amdgcn -mcpu=gfx702 %s 2>&1 | FileCheck --check-prefixes=ARCH-GCN,FAST_FMAF %s -DWAVEFRONT_SIZE=64 -DCPU=gfx702
-// RUN: %clang -E -dM -target amdgcn -mcpu=gfx703 %s 2>&1 | FileCheck --check-prefix=ARCH-GCN %s -DWAVEFRONT_SIZE=64 -DCPU=gfx703
-// RUN: %clang -E -dM -target amdgcn -mcpu=kabini %s 2>&1 | FileCheck --check-prefix=ARCH-GCN %s -DWAVEFRONT_SIZE=64 -DCPU=gfx703
-// RUN: %clang -E -dM -target amdgcn -mcpu=mullins %s 2>&1 | FileCheck --check-prefix=ARCH-GCN %s -DWAVEFRONT_SIZE=64 -DCPU=gfx703
-// RUN: %clang -E -dM -target amdgcn -mcpu=gfx704 %s 2>&1 | FileCheck --check-prefix=ARCH-GCN %s -DWAVEFRONT_SIZE=64 -DCPU=gfx704
-// RUN: %clang -E -dM -target amdgcn -mcpu=bonaire %s 2>&1 | FileCheck --check-prefix=ARCH-GCN %s -DWAVEFRONT_SIZE=64 -DCPU=gfx704
-// RUN: %clang -E -dM -target amdgcn -mcpu=gfx705 %s 2>&1 | FileCheck --check-prefix=ARCH-GCN %s -DWAVEFRONT_SIZE=64 -DCPU=gfx705
-// RUN: %clang -E -dM -target amdgcn -mcpu=gfx801 %s 2>&1 | FileCheck --check-prefixes=ARCH-GCN,FAST_FMAF %s -DWAVEFRONT_SIZE=64 -DCPU=gfx801
-// RUN: %clang -E -dM -target amdgcn -mcpu=carrizo %s 2>&1 | FileCheck --check-prefixes=ARCH-GCN,FAST_FMAF %s -DWAVEFRONT_SIZE=64 -DCPU=gfx801
-// RUN: %clang -E -dM -target amdgcn -mcpu=gfx802 %s 2>&1 | FileCheck --check-prefix=ARCH-GCN %s -DWAVEFRONT_SIZE=64 -DCPU=gfx802
-// RUN: %clang -E -dM -target amdgcn -mcpu=iceland %s 2>&1 | FileCheck --check-prefix=ARCH-GCN %s -DWAVEFRONT_SIZE=64 -DCPU=gfx802
-// RUN: %clang -E -dM -target amdgcn -mcpu=tonga %s 2>&1 | FileCheck --check-prefix=ARCH-GCN %s -DWAVEFRONT_SIZE=64 -DCPU=gfx802
-// RUN: %clang -E -dM -target amdgcn -mcpu=gfx803 %s 2>&1 | FileCheck --check-prefix=ARCH-GCN %s -DWAVEFRONT_SIZE=64 -DCPU=gfx803
-// RUN: %clang -E -dM -target amdgcn -mcpu=fiji %s 2>&1 | FileCheck --check-prefix=ARCH-GCN %s -DWAVEFRONT_SIZE=64 -DCPU=gfx803
-// RUN: %clang -E -dM -target amdgcn -mcpu=polaris10 %s 2>&1 | FileCheck --check-prefix=ARCH-GCN %s -DWAVEFRONT_SIZE=64 -DCPU=gfx803
-// RUN: %clang -E -dM -target amdgcn -mcpu=polaris11 %s 2>&1 | FileCheck --check-prefix=ARCH-GCN %s -DWAVEFRONT_SIZE=64 -DCPU=gfx803
-// RUN: %clang -E -dM -target amdgcn -mcpu=gfx805 %s 2>&1 | FileCheck --check-prefix=ARCH-GCN %s -DWAVEFRONT_SIZE=64 -DCPU=gfx805
-// RUN: %clang -E -dM -target amdgcn -mcpu=tongapro %s 2>&1 | FileCheck --check-prefix=ARCH-GCN %s -DWAVEFRONT_SIZE=64 -DCPU=gfx805
-// RUN: %clang -E -dM -target amdgcn -mcpu=gfx810 %s 2>&1 | FileCheck --check-prefix=ARCH-GCN %s -DWAVEFRONT_SIZE=64 -DCPU=gfx810
-// RUN: %clang -E -dM -target amdgcn -mcpu=stoney %s 2>&1 | FileCheck --check-prefix=ARCH-GCN %s -DWAVEFRONT_SIZE=64 

[PATCH] D125908: [CMake][Fuchsia] Build runtimes as universal libraries on OS X

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
phosek added a reviewer: abrachet.
Herald added subscribers: pengfei, kristof.beyls, mgorny.
Herald added a project: All.
phosek requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We want to build libunwind, libc++abi and libc++ as universal libraries
supporting both x86_64 and arm64 architectures.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125908

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake
  clang/cmake/caches/Fuchsia.cmake


Index: clang/cmake/caches/Fuchsia.cmake
===
--- clang/cmake/caches/Fuchsia.cmake
+++ clang/cmake/caches/Fuchsia.cmake
@@ -69,7 +69,9 @@
   set(LIBCXX_ABI_VERSION 2 CACHE STRING "")
   set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
   set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
+  set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
   set(LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi;libunwind" CACHE 
STRING "")
+  set(RUNTIMES_CMAKE_ARGS 
"-DCMAKE_OSX_DEPLOYMENT_TARGET=10.13;-DCMAKE_OSX_ARCHITECTURES=arm64|x86_64" 
CACHE STRING "")
 endif()
 
 if(BOOTSTRAP_CMAKE_SYSTEM_NAME)
Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -61,10 +61,11 @@
   set(LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "")
   set(LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
   set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
-  set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
+  set(LIBCXX_ABI_VERSION 2 CACHE STRING "")
   set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
   set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
-  set(LIBCXX_ABI_VERSION 2 CACHE STRING "")
+  set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
+  set(RUNTIMES_CMAKE_ARGS 
"-DCMAKE_OSX_DEPLOYMENT_TARGET=10.13;-DCMAKE_OSX_ARCHITECTURES=arm64|x86_64" 
CACHE STRING "")
 endif()
 
 if(WIN32)


Index: clang/cmake/caches/Fuchsia.cmake
===
--- clang/cmake/caches/Fuchsia.cmake
+++ clang/cmake/caches/Fuchsia.cmake
@@ -69,7 +69,9 @@
   set(LIBCXX_ABI_VERSION 2 CACHE STRING "")
   set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
   set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
+  set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
   set(LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING "")
+  set(RUNTIMES_CMAKE_ARGS "-DCMAKE_OSX_DEPLOYMENT_TARGET=10.13;-DCMAKE_OSX_ARCHITECTURES=arm64|x86_64" CACHE STRING "")
 endif()
 
 if(BOOTSTRAP_CMAKE_SYSTEM_NAME)
Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -61,10 +61,11 @@
   set(LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL "")
   set(LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
   set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
-  set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
+  set(LIBCXX_ABI_VERSION 2 CACHE STRING "")
   set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
   set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
-  set(LIBCXX_ABI_VERSION 2 CACHE STRING "")
+  set(LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
+  set(RUNTIMES_CMAKE_ARGS "-DCMAKE_OSX_DEPLOYMENT_TARGET=10.13;-DCMAKE_OSX_ARCHITECTURES=arm64|x86_64" CACHE STRING "")
 endif()
 
 if(WIN32)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115232: [clangd] Indexing of standard library

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

I managed to get a stack trace from a bot 
 (by leaving the 
broken commit up for longer this time).

In D115232#3522598 , @thakis wrote:

> In D115232#3522571 , @sammccall 
> wrote:
>
>> In D115232#3522514 , @thakis wrote:
>>
>>> In D115232#3520461 , @sammccall 
>>> wrote:
>>>
 Hmm, the test keeps crashing on the GN bot: 
 http://45.33.8.238/win/58316/step_9.txt
 Unfortunately the stacktrace is not symbolized, and I'm not seeing this 
 elsewhere (e.g. premerge bot).

 @thakis, any idea why unittests no longer manage to symbolize stack traces 
 on crash on the windows bot? I believe this used to work...
>>>
>>> I do not know. Maybe related to the "run many unit tests in a single 
>>> process" lit change from a month ago?
>>
>> I suspected that, and verified locally that:
>>
>> - llvm-symbolizer on PATH still works
>> - LLVM_SYMBOLIZER_PATH env variable didn't work, but I fixed it in 
>> 1236b66a98197109ed40141329d6056dfbe25967 
>>  along 
>> with this reland, still no dice.
>
> (My bot neither has llvm-symbolizer on path, nor sets LLVM_SYMBOLIZER_PATH 
> fwiw.)

Is this something you could add? I'd much rather revert quickly when after 
seeing a problem than wait for the slower official bots to catch it, but the 
stacktrace is pretty key.

>>> Anyways, looks like this relanded and broke tests yet again. Maybe find a 
>>> win box before relanding the next time?
>>
>> I found one, but the test doesn't crash (nor on the premerge bots).
>>
>> I'll revert again, but I have no idea how to proceed. Only this bot and 
>> `llvm-avr-linux` show the failure, and neither of them have a working 
>> symbolizer.
>
> I wouldn't be super surprised if this is related to windows and delayed 
> template parsing.

It seems possible. The other other failure I'd seen was the `avr-linux` bot 
(not windows).
However it definitely passes in some configurations: both the premerge tests 
 and my local 
build.

> I haven't found any bots on llvm's official waterfall that run 
> ClangdTests.exe – maybe that's why there aren't more bots finding this?

clang-x64-windows-msvc does, but it's slow.

> I'd recommend building and running the test on a win box and see if it repros 
> locally.

Again, I have done this, and cannot reproduce (on 19.29.30038.1).

Now I have some idea where the crash is I can try some blind fixes, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115232

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

[PATCH] D125904: [Cuda] Use fallback method to mangle externalized decls if no CUID given

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 created this revision.
jhuber6 added reviewers: yaxunl, tra.
Herald added subscribers: mattd, carlosgalvezp.
Herald added a project: All.
jhuber6 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

CUDA requires that static variables be visible to the host when
offloading. However, The standard semantics of a stiatc variable dictate
that it should not be visible outside of the current file. In order to
access it from the host we need to perform "externalization" on the
static variable on the device. This requires generating a semi-unique
name that can be affixed to the variable as to not cause linker errors.

This is currently done using the CUID functionality, an MD5 hash value
set up by the clang driver. This allows us to achieve is mostly unique
ID that is unique even between multiple compilations of the same file.
However, this is not always availible. Instead, this patch uses the
unique ID from the file to generate a unique symbol name. This will
create a unique name that is consistent between the host and device side
compilations without requiring the CUID to be entered by the driver. The
one downside to this is that we are no longer stable under multiple
compilations of the same file. However, this is a very niche use-case
and is not supported by Nvidia's CUDA compiler so it likely to be good
enough.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125904

Files:
  clang/lib/CodeGen/CGCUDANV.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenCUDA/device-fun-linkage.cu
  clang/test/CodeGenCUDA/static-device-var-rdc.cu

Index: clang/test/CodeGenCUDA/static-device-var-rdc.cu
===
--- clang/test/CodeGenCUDA/static-device-var-rdc.cu
+++ clang/test/CodeGenCUDA/static-device-var-rdc.cu
@@ -2,12 +2,12 @@
 // REQUIRES: amdgpu-registered-target
 
 // RUN: %clang_cc1 -no-opaque-pointers -triple amdgcn-amd-amdhsa -fcuda-is-device \
-// RUN:   -std=c++11 -fgpu-rdc -emit-llvm -o - -x hip %s | FileCheck \
-// RUN:   -check-prefixes=DEV,INT-DEV %s
+// RUN:   -std=c++11 -fgpu-rdc -emit-llvm -o %t.nocuid.dev -x hip %s
+// RUN: cat %t.nocuid.dev | FileCheck -check-prefixes=DEV,INT-DEV %s
 
 // RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-gnu-linux \
-// RUN:   -std=c++11 -fgpu-rdc -emit-llvm -o - -x hip %s | FileCheck \
-// RUN:   -check-prefixes=HOST,INT-HOST %s
+// RUN:   -std=c++11 -fgpu-rdc -emit-llvm -o %t.nocuid.host -x hip %s
+// RUN: cat %t.nocuid.host | FileCheck -check-prefixes=HOST,INT-HOST %s
 
 // RUN: %clang_cc1 -no-opaque-pointers -triple amdgcn-amd-amdhsa -fcuda-is-device -cuid=abc \
 // RUN:   -std=c++11 -fgpu-rdc -emit-llvm -o - -x hip %s > %t.dev
@@ -21,6 +21,7 @@
 // variable names.
 
 // RUN: cat %t.dev %t.host | FileCheck -check-prefix=POSTFIX %s
+// RUN: cat %t.nocuid.dev %t.nocuid.host | FileCheck -check-prefix=POSTFIX-ID %s
 
 // Negative tests.
 
@@ -56,8 +57,8 @@
 // HOST-DAG: @_ZL1y = internal global i32 undef
 
 // Test normal static device variables
-// INT-DEV-DAG: @_ZL1x = addrspace(1) externally_initialized global i32 0
-// INT-HOST-DAG: @[[DEVNAMEX:[0-9]+]] = {{.*}}c"_ZL1x\00"
+// INT-DEV-DAG: @_ZL1x[[FILEID:.*]] = addrspace(1) externally_initialized global i32 0
+// INT-HOST-DAG: @[[DEVNAMEX:[0-9]+]] = {{.*}}c"_ZL1x[[FILEID:.*]]\00"
 
 // Test externalized static device variables
 // EXT-DEV-DAG: @_ZL1x.static.[[HASH:.*]] = addrspace(1) externally_initialized global i32 0
@@ -66,6 +67,8 @@
 
 // POSTFIX: @_ZL1x.static.[[HASH:.*]] = addrspace(1) externally_initialized global i32 0
 // POSTFIX: @[[DEVNAMEX:[0-9]+]] = {{.*}}c"_ZL1x.static.[[HASH]]\00"
+// POSTFIX-ID: @_ZL1x.static.[[FILEID:.*]] = addrspace(1) externally_initialized global i32 0
+// POSTFIX-ID: @[[DEVNAMEX:[0-9]+]] = {{.*}}c"_ZL1x.static.[[FILEID]]\00"
 
 static __device__ int x;
 
@@ -75,8 +78,8 @@
 static __device__ int x2;
 
 // Test normal static device variables
-// INT-DEV-DAG: @_ZL1y = addrspace(4) externally_initialized global i32 0
-// INT-HOST-DAG: @[[DEVNAMEY:[0-9]+]] = {{.*}}c"_ZL1y\00"
+// INT-DEV-DAG: @_ZL1y[[FILEID:.*]] = addrspace(4) externally_initialized global i32 0
+// INT-HOST-DAG: @[[DEVNAMEY:[0-9]+]] = {{.*}}c"_ZL1y[[FILEID:.*]]\00"
 
 // Test externalized static device variables
 // EXT-DEV-DAG: @_ZL1y.static.[[HASH]] = addrspace(4) externally_initialized global i32 0
Index: clang/test/CodeGenCUDA/device-fun-linkage.cu
===
--- clang/test/CodeGenCUDA/device-fun-linkage.cu
+++ clang/test/CodeGenCUDA/device-fun-linkage.cu
@@ -23,10 +23,10 @@
 // Ensure that unused static device function is eliminated
 static __device__ void static_func() {}
 // NORDC-NEG-NOT: define{{.*}} void @_ZL13static_funcv()
-// RDC-NEG-NOT:   define{{.*}} void @_ZL13static_funcv()
+// RDC-NEG-NOT:   define{{.*}} void @_ZL13static_funcv[[FILEID:.*]]()
 
 // Ensure that kernel function has external or 

[PATCH] D125829: [clang] Fix __has_builtin

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 430420.
yaxunl marked 2 inline comments as done.
yaxunl added a comment.

revised by Artem's comments


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

https://reviews.llvm.org/D125829

Files:
  clang/include/clang/Basic/Builtins.h
  clang/lib/Basic/BuiltinTargetFeatures.h
  clang/lib/Basic/Builtins.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/Preprocessor/feature_tests.c
  clang/test/Preprocessor/hash_builtin.cpp
  clang/unittests/CodeGen/CheckTargetFeaturesTest.cpp

Index: clang/unittests/CodeGen/CheckTargetFeaturesTest.cpp
===
--- clang/unittests/CodeGen/CheckTargetFeaturesTest.cpp
+++ clang/unittests/CodeGen/CheckTargetFeaturesTest.cpp
@@ -1,4 +1,4 @@
-#include "../lib/CodeGen/CodeGenFunction.h"
+#include "../lib/Basic/BuiltinTargetFeatures.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
@@ -11,7 +11,7 @@
 StringMap SM;
 for (StringRef F : Features)
   SM.insert(std::make_pair(F, true));
-clang::CodeGen::TargetFeatures TF(SM);
+clang::Builtin::TargetFeatures TF(SM);
 return TF.hasRequiredFeatures(BuiltinFeatures);
   };
   // Make sure the basic function ',' and '|' works correctly
Index: clang/test/Preprocessor/hash_builtin.cpp
===
--- /dev/null
+++ clang/test/Preprocessor/hash_builtin.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple amdgcn -target-cpu gfx906 -E %s -o - | FileCheck %s
+
+// CHECK: has_s_memtime_inst
+#if __has_builtin(__builtin_amdgcn_s_memtime)
+  int has_s_memtime_inst;
+#endif
+
+// CHECK-NOT: has_gfx10_inst
+#if __has_builtin(__builtin_amdgcn_mov_dpp8)
+  int has_gfx10_inst;
+#endif
Index: clang/test/Preprocessor/feature_tests.c
===
--- clang/test/Preprocessor/feature_tests.c
+++ clang/test/Preprocessor/feature_tests.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 %s -triple=i686-apple-darwin9 -verify -DVERIFY
-// RUN: %clang_cc1 %s -E -triple=i686-apple-darwin9
+// RUN: %clang_cc1 %s -triple=i686-apple-darwin9 -target-cpu pentium4 -verify -DVERIFY
+// RUN: %clang_cc1 %s -E -triple=i686-apple-darwin9 -target-cpu pentium4
 #ifndef __has_feature
 #error Should have __has_feature
 #endif
Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -1639,8 +1639,16 @@
 // denotes date of behavior change to support calling arbitrary
 // usual allocation and deallocation functions. Required by libc++
 return 201802;
-  default:
-return true;
+  default: {
+StringRef FeatureList(getBuiltinInfo().getRequiredFeatures(II->getBuiltinID()));
+// Return true if the builtin doesn't have any required features.
+if (FeatureList.empty())
+  return true;
+assert(!FeatureList.contains(' ') && "Space in feature list");
+
+return Builtin::evaluateRequiredTargetFeatures(
+FeatureList, getTargetInfo().getTargetOpts().FeatureMap);
+  }
   }
   return true;
 } else if (II->getTokenID() != tok::identifier ||
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -4840,76 +4840,6 @@
   llvm::Value *FormResolverCondition(const MultiVersionResolverOption );
 };
 
-/// TargetFeatures - This class is used to check whether the builtin function
-/// has the required tagert specific features. It is able to support the
-/// combination of ','(and), '|'(or), and '()'. By default, the priority of
-/// ',' is higher than that of '|' .
-/// E.g:
-/// A,B|C means the builtin function requires both A and B, or C.
-/// If we want the builtin function requires both A and B, or both A and C,
-/// there are two ways: A,B|A,C or A,(B|C).
-/// The FeaturesList should not contain spaces, and brackets must appear in
-/// pairs.
-class TargetFeatures {
-  struct FeatureListStatus {
-bool HasFeatures;
-StringRef CurFeaturesList;
-  };
-
-  const llvm::StringMap 
-
-  FeatureListStatus getAndFeatures(StringRef FeatureList) {
-int InParentheses = 0;
-bool HasFeatures = true;
-size_t SubexpressionStart = 0;
-for (size_t i = 0, e = FeatureList.size(); i < e; ++i) {
-  char CurrentToken = FeatureList[i];
-  switch (CurrentToken) {
-  default:
-break;
-  case '(':
-if (InParentheses == 0)
-  SubexpressionStart = i + 1;
-++InParentheses;
-break;
-  case ')':
---InParentheses;
-assert(InParentheses >= 0 && "Parentheses are not in pair");
-

[PATCH] D125829: [clang] Fix __has_builtin

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/Builtins.h:263
+  ///false if it is disabled.
+  bool isRequiredTargetFeaturesEnabled(
+  unsigned BuiltinID, const llvm::StringMap ) const;

tra wrote:
> I think we should boil it down to just 
> `evaluateRequiredTargetFeatures(StringRef RequiredFeatures, ... 
> TargetFeatureMap)`.
> 
will do



Comment at: clang/lib/Basic/BuiltinTargetFeatures.h:32
+/// pairs.
+class TargetFeatures {
+  struct FeatureListStatus {

tra wrote:
> I don't think it needs to be part of the public API. Feature check function 
> operates on `llvm::StringMap` and does not need to know about this 
> class, and the implementation of this class can be moved into `Builtins.cpp` 
> where it's actually used.
It is not a public API since it is under lib/Basic.

The reason it is made a header file because we have a unit test 
clang/unittests/CodeGen/CheckTargetFeaturesTest.cpp for class TargetFeatures. 
This is because TargetFeatures is relatively complicated.


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

https://reviews.llvm.org/D125829

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

[PATCH] D125259: [C11] Diagnose unreachable generic selection associations

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D125259#3522198 , @aaron.ballman 
wrote:

> In D125259#3520118 , @aaron.ballman 
> wrote:
>
>> In D125259#3519947 , 
>> @aaron.ballman wrote:
>>
>>> Oh wow, good catch! I'll correct this.
>>
>> Oof, the plot thickens... the diagnostic is correct for some types, but is 
>> incorrect for others: https://godbolt.org/z/fahzx53W6. I also discovered a 
>> parsing issue in C++ where we get confused by elaborated type specifiers: 
>> https://godbolt.org/z/1ejxqd9ss.
>
> https://reviews.llvm.org/D125882 addresses the diagnostic behavior for the 
> unreachable warning.

I landed that fix in 47b8424a533d5c02fd8b3517047cf93e533f00d0 
, thank 
you for pointing it out @aeubanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125259

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

[PATCH] D125882: Correct the diagnostic behavior for unreachable _Generic associations in C++

[Aaron Ballman via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG47b8424a533d: Correct the diagnostic behavior for 
unreachable _Generic associations in C++ (authored by aaron.ballman).

Changed prior to commit:
  https://reviews.llvm.org/D125882?vs=430354=430417#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125882

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/generic-selection.c
  clang/test/SemaCXX/generic-selection.cpp


Index: clang/test/SemaCXX/generic-selection.cpp
===
--- clang/test/SemaCXX/generic-selection.cpp
+++ clang/test/SemaCXX/generic-selection.cpp
@@ -44,3 +44,28 @@
 static_assert(TypeMask::result == 7, "fail");
 static_assert(TypeMask::result == 12, "fail");
 static_assert(TypeMask::result == 9, "fail");
+
+
+struct Test {
+  int i;
+};
+
+void unreachable_associations(const int i, const Test t) {
+  // FIXME: it's not clear to me whether we intended to deviate from the C
+  // semantics in terms of how qualifiers are handled, so this documents the
+  // existing behavior but perhaps not the desired behavior.
+  static_assert(
+_Generic(i,
+  const int : 1,// expected-warning {{due to lvalue conversion of the 
controlling expression, association of type 'const int' will never be selected 
because it is qualified}}
+  volatile int : 2, // expected-warning {{due to lvalue conversion of the 
controlling expression, association of type 'volatile int' will never be 
selected because it is qualified}}
+  int[12] : 3,  // expected-warning {{due to lvalue conversion of the 
controlling expression, association of type 'int[12]' will never be selected 
because it is of array type}}
+  int : 4,
+  default : 5
+) == 4, "we had better pick int, not const int!");
+  static_assert(
+_Generic(t,
+  Test : 1,
+  const Test : 2,  // Ok in C++, warned in C
+  default : 3
+) == 2, "we had better pick const Test, not Test!"); // C++-specific result
+}
Index: clang/test/Sema/generic-selection.c
===
--- clang/test/Sema/generic-selection.c
+++ clang/test/Sema/generic-selection.c
@@ -58,7 +58,11 @@
 ), int : 0);
 }
 
-void unreachable_associations(const int i) {
+struct Test {
+  int i;
+};
+
+void unreachable_associations(const int i, const struct Test t) {
   _Static_assert( // ext-warning {{'_Static_assert' is a C11 extension}}
 _Generic(i, // ext-warning {{'_Generic' is a C11 extension}}
   const int : 1,// expected-warning {{due to lvalue conversion of the 
controlling expression, association of type 'const int' will never be selected 
because it is qualified}}
@@ -67,4 +71,10 @@
   int : 4,
   default : 5
 ) == 4, "we had better pick int!");
+  _Static_assert( // ext-warning {{'_Static_assert' is a C11 extension}}
+_Generic(t, // ext-warning {{'_Generic' is a C11 extension}}
+  struct Test : 1,
+  const struct Test : 2, // expected-warning {{due to lvalue conversion of 
the controlling expression, association of type 'const struct Test' will never 
be selected because it is qualified}}
+  default : 3
+) == 1, "we had better pick struct Test, not const struct Test!"); // 
C-specific result
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -1692,11 +1692,20 @@
   // reached. We will warn about this so users are less surprised by
   // the unreachable association. However, we don't have to handle
   // function types; that's not an object type, so it's handled above.
+  //
+  // The logic is somewhat different for C++ because C++ has different
+  // lvalue to rvalue conversion rules than C. [conv.lvalue]p1 says,
+  // If T is a non-class type, the type of the prvalue is the cv-
+  // unqualified version of T. Otherwise, the type of the prvalue is T.
+  // The result of these rules is that all qualified types in an
+  // association in C are unreachable, and in C++, only qualified non-
+  // class types are unreachable.
   unsigned Reason = 0;
   QualType QT = Types[i]->getType();
   if (QT->isArrayType())
 Reason = 1;
-  else if (QT.hasQualifiers())
+  else if (QT.hasQualifiers() &&
+   (!LangOpts.CPlusPlus || !QT->isRecordType()))
 Reason = 2;
 
   if (Reason)


Index: clang/test/SemaCXX/generic-selection.cpp
===
--- clang/test/SemaCXX/generic-selection.cpp
+++ clang/test/SemaCXX/generic-selection.cpp
@@ -44,3 +44,28 @@
 

[clang] 47b8424 - Correct the diagnostic behavior for unreachable _Generic associations in C++

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2022-05-18T12:45:38-04:00
New Revision: 47b8424a533d5c02fd8b3517047cf93e533f00d0

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

LOG: Correct the diagnostic behavior for unreachable _Generic associations in 
C++

New diagnostics were added for unreachable generic selection expression
associations in ca75ac5f04f269def97e6844c2f5c9596b29c84c, but it did
not account for a difference in behavior between C and C++ regarding
lvalue to rvalue conversions. So we would issue diagnostics about a
selection being unreachable and then reach it. This corrects the
diagnostic behavior in that case.

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

Added: 


Modified: 
clang/lib/Sema/SemaExpr.cpp
clang/test/Sema/generic-selection.c
clang/test/SemaCXX/generic-selection.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 84ffa98c2cd61..9d9cb99085f33 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -1692,11 +1692,20 @@ Sema::CreateGenericSelectionExpr(SourceLocation KeyLoc,
   // reached. We will warn about this so users are less surprised by
   // the unreachable association. However, we don't have to handle
   // function types; that's not an object type, so it's handled above.
+  //
+  // The logic is somewhat 
diff erent for C++ because C++ has 
diff erent
+  // lvalue to rvalue conversion rules than C. [conv.lvalue]p1 says,
+  // If T is a non-class type, the type of the prvalue is the cv-
+  // unqualified version of T. Otherwise, the type of the prvalue is T.
+  // The result of these rules is that all qualified types in an
+  // association in C are unreachable, and in C++, only qualified non-
+  // class types are unreachable.
   unsigned Reason = 0;
   QualType QT = Types[i]->getType();
   if (QT->isArrayType())
 Reason = 1;
-  else if (QT.hasQualifiers())
+  else if (QT.hasQualifiers() &&
+   (!LangOpts.CPlusPlus || !QT->isRecordType()))
 Reason = 2;
 
   if (Reason)

diff  --git a/clang/test/Sema/generic-selection.c 
b/clang/test/Sema/generic-selection.c
index f6299aae25046..0bd537c79d316 100644
--- a/clang/test/Sema/generic-selection.c
+++ b/clang/test/Sema/generic-selection.c
@@ -58,7 +58,11 @@ void GH50227(void) {
 ), int : 0);
 }
 
-void unreachable_associations(const int i) {
+struct Test {
+  int i;
+};
+
+void unreachable_associations(const int i, const struct Test t) {
   _Static_assert( // ext-warning {{'_Static_assert' is a C11 extension}}
 _Generic(i, // ext-warning {{'_Generic' is a C11 extension}}
   const int : 1,// expected-warning {{due to lvalue conversion of the 
controlling expression, association of type 'const int' will never be selected 
because it is qualified}}
@@ -67,4 +71,10 @@ void unreachable_associations(const int i) {
   int : 4,
   default : 5
 ) == 4, "we had better pick int!");
+  _Static_assert( // ext-warning {{'_Static_assert' is a C11 extension}}
+_Generic(t, // ext-warning {{'_Generic' is a C11 extension}}
+  struct Test : 1,
+  const struct Test : 2, // expected-warning {{due to lvalue conversion of 
the controlling expression, association of type 'const struct Test' will never 
be selected because it is qualified}}
+  default : 3
+) == 1, "we had better pick struct Test, not const struct Test!"); // 
C-specific result
 }

diff  --git a/clang/test/SemaCXX/generic-selection.cpp 
b/clang/test/SemaCXX/generic-selection.cpp
index 8bf4a784f9bff..79e0776cfa395 100644
--- a/clang/test/SemaCXX/generic-selection.cpp
+++ b/clang/test/SemaCXX/generic-selection.cpp
@@ -44,3 +44,28 @@ template  struct TypeMask {
 static_assert(TypeMask::result == 7, "fail");
 static_assert(TypeMask::result == 12, "fail");
 static_assert(TypeMask::result == 9, "fail");
+
+
+struct Test {
+  int i;
+};
+
+void unreachable_associations(const int i, const Test t) {
+  // FIXME: it's not clear to me whether we intended to deviate from the C
+  // semantics in terms of how qualifiers are handled, so this documents the
+  // existing behavior but perhaps not the desired behavior.
+  static_assert(
+_Generic(i,
+  const int : 1,// expected-warning {{due to lvalue conversion of the 
controlling expression, association of type 'const int' will never be selected 
because it is qualified}}
+  volatile int : 2, // expected-warning {{due to lvalue conversion of the 
controlling expression, association of type 'volatile int' will never be 
selected because it is qualified}}
+  int[12] : 3,  // expected-warning {{due to lvalue 

[PATCH] D125882: Correct the diagnostic behavior for unreachable _Generic associations in C++

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:1699
+  // If T is a non-class type, the type of the prvalue is the cv-
+  // unqualified version of T. Otherwise, the type of the prvalue is T.
   unsigned Reason = 0;

erichkeane wrote:
> This seems like it is a sentence short (or perhaps I'm a few synapses 
> short?). 
> 
> The fallout from these rules is that "Non-class-types" never have qualifiers, 
> thus cannot match a qualified type, but a class-type can, because it keeps 
> its qualifiers?
> 
>  
> The fallout from these rules is that "Non-class-types" never have qualifiers, 
> thus cannot match a qualified type, but a class-type can, because it keeps 
> its qualifiers?

Correct for C++ -- the basic idea is that in C, all qualifiers are stripped, 
and in C++, only non-class type types have their qualifiers stripped. I'll 
update the comment when landing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125882

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

[clang-tools-extra] 4739176 - [clang-tidy] Fix readability-simplify-boolean-expr crash with implicit cast in return.

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2022-05-18T17:38:44+01:00
New Revision: 4739176fd3047dfa13eae307c56c6dba7b605019

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

LOG: [clang-tidy] Fix readability-simplify-boolean-expr crash with implicit 
cast in return.

Fixes https://github.com/llvm/llvm-project/issues/7

Reviewed By: LegalizeAdulthood

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
index 2300b4260c600..1db440120aa3a 100644
--- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -236,41 +236,6 @@ static std::string replacementExpression(const ASTContext 
,
   return asBool(getText(Context, *E), NeedsStaticCast);
 }
 
-static const Expr *stmtReturnsBool(const ReturnStmt *Ret, bool Negated) {
-  if (const auto *Bool = dyn_cast(Ret->getRetValue())) {
-if (Bool->getValue() == !Negated)
-  return Bool;
-  }
-  if (const auto *Unary = dyn_cast(Ret->getRetValue())) {
-if (Unary->getOpcode() == UO_LNot) {
-  if (const auto *Bool =
-  dyn_cast(Unary->getSubExpr())) {
-if (Bool->getValue() == Negated)
-  return Bool;
-  }
-}
-  }
-
-  return nullptr;
-}
-
-static const Expr *stmtReturnsBool(const IfStmt *IfRet, bool Negated) {
-  if (IfRet->getElse() != nullptr)
-return nullptr;
-
-  if (const auto *Ret = dyn_cast(IfRet->getThen()))
-return stmtReturnsBool(Ret, Negated);
-
-  if (const auto *Compound = dyn_cast(IfRet->getThen())) {
-if (Compound->size() == 1) {
-  if (const auto *CompoundRet = 
dyn_cast(Compound->body_back()))
-return stmtReturnsBool(CompoundRet, Negated);
-}
-  }
-
-  return nullptr;
-}
-
 static bool containsDiscardedTokens(const ASTContext ,
 CharSourceRange CharRange) {
   std::string ReplacementText =
@@ -502,8 +467,8 @@ class SimplifyBooleanExprCheck::Visitor : public 
RecursiveASTVisitor {
   if (Check->ChainedConditionalReturn ||
   (!PrevIf && If->getElse() == nullptr)) {
 Check->replaceCompoundReturnWithCondition(
-Context, cast(*Second), TrailingReturnBool.Bool,
-If);
+Context, cast(*Second), TrailingReturnBool.Bool, 
If,
+ThenReturnBool.Item);
   }
 }
   } else if (isa(*First)) {
@@ -523,7 +488,7 @@ class SimplifyBooleanExprCheck::Visitor : public 
RecursiveASTVisitor {
   ThenReturnBool.Bool != TrailingReturnBool.Bool) {
 Check->replaceCompoundReturnWithCondition(
 Context, cast(*Second), TrailingReturnBool.Bool,
-SubIf);
+SubIf, ThenReturnBool.Item);
   }
 }
   }
@@ -689,11 +654,11 @@ void SimplifyBooleanExprCheck::replaceWithReturnCondition(
 
 void SimplifyBooleanExprCheck::replaceCompoundReturnWithCondition(
 const ASTContext , const ReturnStmt *Ret, bool Negated,
-const IfStmt *If) {
-  const auto *Lit = stmtReturnsBool(If, Negated);
+const IfStmt *If, const Expr *ThenReturn) {
   const std::string Replacement =
   "return " + replacementExpression(Context, Negated, If->getCond());
-  issueDiag(Context, Lit->getBeginLoc(), SimplifyConditionalReturnDiagnostic,
+  issueDiag(Context, ThenReturn->getBeginLoc(),
+SimplifyConditionalReturnDiagnostic,
 SourceRange(If->getBeginLoc(), Ret->getEndLoc()), Replacement);
 }
 

diff  --git 
a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h 
b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
index 9a362d7bcc3f8..2724c9a4eddd7 100644
--- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
@@ -55,7 +55,8 @@ class SimplifyBooleanExprCheck : public ClangTidyCheck {
 
   void replaceCompoundReturnWithCondition(const ASTContext ,
   const ReturnStmt *Ret, bool Negated,
-  const IfStmt *If);
+  const IfStmt *If,
+  const Expr *ThenReturn);
 
   void issueDiag(const ASTContext , SourceLocation Loc,
  StringRef Description, SourceRange ReplacementRange,

diff  

[PATCH] D125877: [clang-tidy] Fix readability-simplify-boolean-expr crash with implicit cast in return.

[Nathan James via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4739176fd304: [clang-tidy] Fix 
readability-simplify-boolean-expr crash with implicit cast in… (authored by 
njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125877

Files:
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
  clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp
@@ -2,6 +2,7 @@
 #include "ClangTidyTest.h"
 #include "readability/BracesAroundStatementsCheck.h"
 #include "readability/NamespaceCommentCheck.h"
+#include "readability/SimplifyBooleanExprCheck.h"
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -10,6 +11,7 @@
 
 using readability::BracesAroundStatementsCheck;
 using readability::NamespaceCommentCheck;
+using readability::SimplifyBooleanExprCheck;
 using namespace ast_matchers;
 
 // Copied from ASTMatchersTests
@@ -533,6 +535,16 @@
 runCheckOnCode(Input));
 }
 
+TEST(SimplifyBooleanExprCheckTest, CodeWithError) {
+  // Fixes PR7
+  // Need to downgrade Wreturn-type from error as runCheckOnCode will fatal_exit
+  // if any errors occur.
+  EXPECT_EQ("void foo(bool b){ return b; }",
+runCheckOnCode(
+"void foo(bool b){ if (b) return true; return false; }",
+nullptr, "input.cc", {"-Wno-error=return-type"}));
+}
+
 } // namespace test
 } // namespace tidy
 } // namespace clang
Index: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
===
--- clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
+++ clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
@@ -55,7 +55,8 @@
 
   void replaceCompoundReturnWithCondition(const ASTContext ,
   const ReturnStmt *Ret, bool Negated,
-  const IfStmt *If);
+  const IfStmt *If,
+  const Expr *ThenReturn);
 
   void issueDiag(const ASTContext , SourceLocation Loc,
  StringRef Description, SourceRange ReplacementRange,
Index: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -236,41 +236,6 @@
   return asBool(getText(Context, *E), NeedsStaticCast);
 }
 
-static const Expr *stmtReturnsBool(const ReturnStmt *Ret, bool Negated) {
-  if (const auto *Bool = dyn_cast(Ret->getRetValue())) {
-if (Bool->getValue() == !Negated)
-  return Bool;
-  }
-  if (const auto *Unary = dyn_cast(Ret->getRetValue())) {
-if (Unary->getOpcode() == UO_LNot) {
-  if (const auto *Bool =
-  dyn_cast(Unary->getSubExpr())) {
-if (Bool->getValue() == Negated)
-  return Bool;
-  }
-}
-  }
-
-  return nullptr;
-}
-
-static const Expr *stmtReturnsBool(const IfStmt *IfRet, bool Negated) {
-  if (IfRet->getElse() != nullptr)
-return nullptr;
-
-  if (const auto *Ret = dyn_cast(IfRet->getThen()))
-return stmtReturnsBool(Ret, Negated);
-
-  if (const auto *Compound = dyn_cast(IfRet->getThen())) {
-if (Compound->size() == 1) {
-  if (const auto *CompoundRet = dyn_cast(Compound->body_back()))
-return stmtReturnsBool(CompoundRet, Negated);
-}
-  }
-
-  return nullptr;
-}
-
 static bool containsDiscardedTokens(const ASTContext ,
 CharSourceRange CharRange) {
   std::string ReplacementText =
@@ -502,8 +467,8 @@
   if (Check->ChainedConditionalReturn ||
   (!PrevIf && If->getElse() == nullptr)) {
 Check->replaceCompoundReturnWithCondition(
-Context, cast(*Second), TrailingReturnBool.Bool,
-If);
+Context, cast(*Second), TrailingReturnBool.Bool, If,
+ThenReturnBool.Item);
   }
 }
   } else if (isa(*First)) {
@@ -523,7 +488,7 @@
   ThenReturnBool.Bool != TrailingReturnBool.Bool) {
 Check->replaceCompoundReturnWithCondition(
 Context, cast(*Second), TrailingReturnBool.Bool,
-SubIf);
+SubIf, ThenReturnBool.Item);
   }
 }
   }
@@ -689,11 +654,11 @@
 
 void SimplifyBooleanExprCheck::replaceCompoundReturnWithCondition(
 const ASTContext , const 

[PATCH] D125770: [clang-tidy] modernize-deprecated-headers should ignore system headers

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

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

Thanks for the review @LegalizeAdulthood!
Can I consider this change accepted?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125770

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

[PATCH] D123352: [analyzer] Add FixItHint to `nullability.NullReturnedFromNonnull` and `nullability.NullableReturnedFromNonnull`

[Moshe via Phabricator via cfe-commits]

MosheBerman updated this revision to Diff 430408.
MosheBerman marked an inline comment as done and 2 inline comments as not done.
MosheBerman added a comment.

Addressed feedback.

- Removed comment about NS-classes
- Changed some of the LIT test invocations
- Added StringRef where appropriate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123352

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/nullability-fixits.mm

Index: clang/test/Analysis/nullability-fixits.mm
===
--- /dev/null
+++ clang/test/Analysis/nullability-fixits.mm
@@ -0,0 +1,185 @@
+// This test runs the nullability checkers with fixits enabled to verify that the
+// fixits are produced as expected. Note that we disable `-Wnonnull` since it
+// pollutes the output and doesn't provide fixits.
+
+// RUN: %clang_analyze_cc1 \
+// RUN: -Wno-nonnull \
+// RUN: -analyzer-checker=core,nullability \
+// RUN: -fdiagnostics-parseable-fixits %s 2>%t
+// RUN: cat %t | FileCheck %s -check-prefix=CHECK-FIXIT-DISABLED-IMPLICIT
+
+// RUN: %clang_analyze_cc1 \
+// RUN: -Wno-nonnull \
+// RUN: -analyzer-checker=core,nullability \
+// RUN: -analyzer-config nullability.NullReturnedFromNonnull:ShowFixIts=false \
+// RUN: -analyzer-config nullability.NullableReturnedFromNonnull:ShowFixIts=false \
+// RUN: -fdiagnostics-parseable-fixits %s 2>%t
+// RUN: cat %t | FileCheck %s -check-prefix=CHECK-FIXIT-DISABLED-EXPLICIT
+
+// RUN: %clang_analyze_cc1 \
+// RUN: -Wno-nonnull \
+// RUN: -analyzer-checker=core,nullability \
+// RUN: -analyzer-config nullability.NullReturnedFromNonnull:ShowFixIts=true \
+// RUN: -analyzer-config nullability.NullableReturnedFromNonnull:ShowFixIts=true \
+// RUN: -fdiagnostics-parseable-fixits %s 2>%t
+// RUN: cat %t | FileCheck %s -check-prefix=CHECK-FIXIT-ENABLED
+
+// RUN: cp %s %t
+// RUN: %clang_analyze_cc1 \
+// RUN: -Wno-nonnull \
+// RUN: -analyzer-checker=core,nullability \
+// RUN: -analyzer-config nullability.NullReturnedFromNonnull:ShowFixIts=true \
+// RUN: -analyzer-config nullability.NullableReturnedFromNonnull:ShowFixIts=true \
+// RUN: -analyzer-config apply-fixits=true %s
+
+// Use grep to skip lines with comments to avoid matching on checks or RUN lines:
+// RUN: cat %s | grep -v -E "\/\/|^\s*\$" | FileCheck %t -check-prefix=CHECK-FIXIT-VERIFY-OUT
+// RUN: cp %t %s
+
+// CHECK-FIXIT-ENABLED: fix-it:"{{.*[/\\]*}}clang/test/Analysis/nullability-fixits.mm"
+// CHECK-FIXIT-DISABLED-EXPLICIT-NOT: fix-it:"{{.*[/\\]*}}clang/test/Analysis/nullability-fixits.mm"
+// CHECK-FIXIT-DISABLED-IMPLICIT-NOT: fix-it:"{{.*[/\\]*}}clang/test/Analysis/nullability-fixits.mm"
+
+#include "Inputs/system-header-simulator-for-nullability.h"
+
+@interface TestObject : NSObject
+@end
+
+@interface ClassWithInitializers : NSObject
+@end
+
+@implementation ClassWithInitializers
+
+// CHECK-FIXIT-VERIFY-OUT: - (TestObject *_Nullable)returnsNil
+- (TestObject *_Nonnull)returnsNil {
+  return nil;
+}
+
+// CHECK-FIXIT-VERIFY-OUT: - (TestObject *_Nullable)returnsNilProtocol
+- (TestObject *_Nonnull)returnsNilProtocol {
+  return nil;
+}
+
+// CHECK-FIXIT-VERIFY-OUT: - (nullable TestObject *)returnsNilProtocolSyntaxSugar
+- (nonnull TestObject *)returnsNilProtocolSyntaxSugar {
+  return nil;
+}
+
+// CHECK-FIXIT-VERIFY-OUT: - (id)returnsNilId
+- (id)returnsNilId {
+  return nil;
+}
+
+// CHECK-FIXIT-VERIFY-OUT: - (instancetype)returnsNilInstancetype
+- (instancetype)returnsNilInstancetype {
+  return nil;
+}
+
+// CHECK-FIXIT-VERIFY-OUT: - (nullable TestObject *)returnsNilSyntaxSugar
+- (nonnull TestObject *)returnsNilSyntaxSugar {
+  return nil;
+}
+
+// CHECK-FIXIT-VERIFY-OUT: - (TestObject *_Nonnull)returnsNilSuppressed
+- (TestObject *_Nonnull)returnsNilSuppressed {
+  return (TestObject *_Nonnull)nil;
+}
+
+// Implicitly unspecified are not auto-fixed.
+// CHECK-FIXIT-VERIFY-OUT: - (TestObject *)returnsNilUnannotated
+- (TestObject *)returnsNilUnannotated {
+  return nil;
+}
+
+// Implicitly unspecified are not auto-fixed.
+// CHECK-FIXIT-VERIFY-OUT: - (TestObject *_Null_unspecified)returnsNilUnannotatedExplicitlyUnspecified
+- (TestObject *_Null_unspecified)returnsNilUnannotatedExplicitlyUnspecified {
+  return nil;
+}
+
+NS_ASSUME_NONNULL_BEGIN
+// CHECK-FIXIT-VERIFY-OUT: - (nullable TestObject *)returnsNilAssumeInClass
+- (TestObject *)returnsNilAssumeInClass {
+  return nil;
+}
+
+// CHECK-FIXIT-VERIFY-OUT: - (TestObject *_Nullable)returnsNilAssumeInClass
+- (TestObject *_Nonnull)returnsNilAssumeInClassAnnotated {
+  return nil;
+}
+
+// CHECK-FIXIT-VERIFY-OUT: - (nullable TestObject *)returnsNilAssumeInClassAnnotatedSyntaxSugar
+- (nonnull TestObject *)returnsNilAssumeInClassAnnotatedSyntaxSugar {
+  return nil;
+}
+
+// CHECK-FIXIT-VERIFY-OUT: - (TestObject 

[PATCH] D105584: [MLIR][OpenMP] Distribute Construct Operation

[Abid via Phabricator via cfe-commits]

abidmalikwaterloo added inline comments.



Comment at: clang/lib/Testing/CMakeLists.txt:29
   llvm_gtest
+  clangBasic
+  clangFrontend

shraiysh wrote:
> unrelated change?
When I rebase, these changes were highlighted in the main branch which was 
missing in the patch as it was too old.



Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:514
+oilist(`schedule` `(`
+  custom(
+   $dist_schedule_var,

shraiysh wrote:
> dist_schedule is a dummy clause, where kind is only allowed to be `static` 
> according to the standard. I don't think that requires a custom function, we 
> can instead have something like the following -
> ```
> arguments = UnitAttr:$static_dist_schedule, 
> Optional:$schedule_chunk
> 
> assemblyFormat = `dist_schedule` `(` (`static` $static_dist_schedule^)? (`:` 
> $schedule_chunk^)? `)`
> ```
> Would that work? Let me know if there are any suggestions.
My only concern is; will this be a dummy clause with the static scheduler 
forever? I am pretty sure dist_schedule will have a dynamic or a  user defined 
scheduling strategy as well to improve the performance of a given application 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105584

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

[PATCH] D125882: Correct the diagnostic behavior for unreachable _Generic associations in C++

[Tom Honermann via Phabricator via cfe-commits]

tahonermann accepted this revision.
tahonermann added a comment.

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125882

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

[libunwind] c218fd3 - [libunwind][AArch64] Add support for DWARF expression for RA_SIGN_STATE.

[Daniel Kiss via cfe-commits]

Author: Daniel Kiss
Date: 2022-05-18T17:56:16+02:00
New Revision: c218fd3d7d3764eb123c8429bbcd33bacfe2e633

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

LOG: [libunwind][AArch64] Add support for DWARF expression for RA_SIGN_STATE.

Program may set the RA_SIGN_STATE pseudo register by expressions.
Libunwind expected only the DW_CFA_AARCH64_negate_ra_state could change the 
value
of the register which leads to runtime errors on PAC enabled systems.
In the recent version of the aadwarf64[1] a limitation is added[2] to forbid 
the mixing the
DW_CFA_AARCH64_negate_ra_state with other DWARF Register Rule Instructions.

[1] https://github.com/ARM-software/abi-aa/releases/tag/2022Q1
[2] https://github.com/ARM-software/abi-aa/pull/129

Reviewed By: #libunwind, MaskRay

Differential Revision: https://reviews.llvm.org/D123692
Reland: test moved because it depends on exceptions.

Added: 
libcxxabi/test/native/AArch64/ra_sign_state.pass.cpp

Modified: 
libunwind/src/DwarfInstructions.hpp

Removed: 




diff  --git a/libcxxabi/test/native/AArch64/ra_sign_state.pass.cpp 
b/libcxxabi/test/native/AArch64/ra_sign_state.pass.cpp
new file mode 100644
index ..1e09c936d664
--- /dev/null
+++ b/libcxxabi/test/native/AArch64/ra_sign_state.pass.cpp
@@ -0,0 +1,63 @@
+// -*- C++ -*-
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+// REQUIRES: linux && target={{aarch64-.+}}
+
+// This test ensures the .cfi_negate_ra_state the RA_SIGN_STATE pseudo register
+// could be set directly set by a DWARF expression and the unwinder handles it
+// correctly. The two directives can't be mixed in one CIE/FDE sqeuence.
+
+#include 
+
+__attribute__((noinline, target("branch-protection=pac-ret+leaf")))
+void bar() {
+  // ".cfi_negate_ra_state" is emitted by the compiler.
+  throw 1;
+}
+
+__attribute__((noinline, target("branch-protection=none")))
+void foo() {
+  // Here a DWARF expression sets RA_SIGN_STATE.
+  // The LR is signed manually and stored on the stack.
+  asm volatile(
+  ".cfi_escape 0x16,"// DW_CFA_val_expression
+"34,"// REG_34(RA_SIGN_STATE)
+ "1,"// expression_length(1)
+"0x31\n" // DW_OP_lit1
+  "add sp, sp, 16\n" // Restore SP's value before the stack frame is
+ // created.
+  "paciasp\n"// Sign the LR.
+  "str lr, [sp, -0x8]\n" // Overwrite LR on the stack.
+  "sub sp, sp, 16\n" // Restore SP's value.
+  );
+  bar();
+  _Exit(-1);
+}
+
+__attribute__((noinline, target("branch-protection=pac-ret")))
+void bazz() {
+  // ".cfi_negate_ra_state" is emitted by the compiler.
+  try {
+foo();
+  } catch (int i) {
+if (i == 1)
+  throw i;
+throw 2;
+  }
+}
+
+int main() {
+  try {
+bazz();
+  } catch (int i) {
+if (i == 1)
+  _Exit(0);
+  }
+  return -1;
+}

diff  --git a/libunwind/src/DwarfInstructions.hpp 
b/libunwind/src/DwarfInstructions.hpp
index ab83b0c87acd..865a48952604 100644
--- a/libunwind/src/DwarfInstructions.hpp
+++ b/libunwind/src/DwarfInstructions.hpp
@@ -72,6 +72,10 @@ class DwarfInstructions {
 assert(0 && "getCFA(): unknown location");
 __builtin_unreachable();
   }
+#if defined(_LIBUNWIND_TARGET_AARCH64)
+  static bool getRA_SIGN_STATE(A , R registers, pint_t cfa,
+   PrologInfo );
+#endif
 };
 
 template 
@@ -166,6 +170,21 @@ v128 DwarfInstructions::getSavedVectorRegister(
   }
   _LIBUNWIND_ABORT("unsupported restore location for vector register");
 }
+#if defined(_LIBUNWIND_TARGET_AARCH64)
+template 
+bool DwarfInstructions::getRA_SIGN_STATE(A , R registers,
+   pint_t cfa, PrologInfo ) 
{
+  pint_t raSignState;
+  auto regloc = prolog.savedRegisters[UNW_AARCH64_RA_SIGN_STATE];
+  if (regloc.location == CFI_Parser::kRegisterUnused)
+raSignState = regloc.value;
+  else
+raSignState = getSavedRegister(addressSpace, registers, cfa, regloc);
+
+  // Only bit[0] is meaningful.
+  return raSignState & 0x01;
+}
+#endif
 
 template 
 int DwarfInstructions::stepWithDwarf(A , pint_t pc,
@@ -235,7 +254,7 @@ int DwarfInstructions::stepWithDwarf(A , 
pint_t pc,
   // restored. autia1716 is used instead of autia as autia1716 assembles
   // to a NOP on pre-v8.3a architectures.
   if ((R::getArch() == REGISTERS_ARM64) &&
-  prolog.savedRegisters[UNW_AARCH64_RA_SIGN_STATE].value &&
+   

[PATCH] D116088: [compiler-rt] Implement ARM atomic operations for architectures without SMP support

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

On a target that doesn't support SMP, you don't need memory barriers, sure.  (I 
think we'd want a CMake flag to explicitly assert that you're only going to run 
the code on chips without SMP.)

That doesn't really solve your issue, though.  To implement atomic 
compare-and-swap or rmw operations, you need to ensure your code doesn't get 
interrupted in the middle.  If your system supports multithreading, and a 
thread can be preempted at any time, you need one of the following:

1. A natively atomic operation, like strex.
2. A way to temporarily turn off interrupts, so the thread can't be preempted 
for a short time.
3. Kernel-assisted operations, like the Linux kernel provides.
4. A separate lock.

None of these options work on armv5 without some sort of kernel assistance.  
(Well, technically, you can implement a spinlock with swp, but that's very 
inefficient.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116088

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

[PATCH] D125770: [clang-tidy] modernize-deprecated-headers should ignore system headers

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp:37
 
-#include  // FIXME: We should have no warning into system 
headers.
-// CHECK-MESSAGES-WARN-INTO-HEADERS: mysystemlib.h:1:10: warning: inclusion of 
deprecated C++ header 'assert.h'; consider using 'cassert' instead 
[modernize-deprecated-headers]
+#include  // no-warning: Don't warn into system headers.
 

steakhal wrote:
> LegalizeAdulthood wrote:
> > steakhal wrote:
> > > LegalizeAdulthood wrote:
> > > > Where is this file?  I can't find it in my tree.
> > > > 
> > > > ```
> > > > D:\legalize\llvm\llvm-project\clang-tools-extra
> > > > > dir/s/b mysystemlib.h
> > > > File Not Found
> > > > ```
> > > > 
> > > > Does this patch depend on some other unsubmitted patch?
> > > D125209 should have included this file at 
> > > `clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-deprecated-headers/mysystemlib.h`.
> > OK, so it depends on D125209  why not just include these small changes 
> > in that?
> > 
> > Supposedly there is some way in phabricator to make one review dependent on 
> > another, but instead of being an expert in phabricator this change seems 
> > small enough to just fold into the other review, but perhaps opinions 
> > differ on that.
> I set the 'parent revision', so the dependencies are present under the 
> "Stack" tab. Sorry if I confused you.
> I still think it's a logically separate issue, which should be addressed in a 
> separate commit. That change is already quite confusing. I already had to 
> ever it once, thus I want to keep the cognitive complexity as low as possible 
> there. Same applies here.
> 
> Do you insist on merging these two?
Nope, no insistence on my part.  Just asking the question, which you've 
answered.

I never even noticed the "stack" tab in phabricator before.

Honestly, phabricator is not my favorite code reviewing tool, but it's what 
clang/llvm has chosen, so I live with it `:)`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125770

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

[PATCH] D115232: [clangd] Indexing of standard library

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D115232#3522571 , @sammccall wrote:

> In D115232#3522514 , @thakis wrote:
>
>> In D115232#3520461 , @sammccall 
>> wrote:
>>
>>> Hmm, the test keeps crashing on the GN bot: 
>>> http://45.33.8.238/win/58316/step_9.txt
>>> Unfortunately the stacktrace is not symbolized, and I'm not seeing this 
>>> elsewhere (e.g. premerge bot).
>>>
>>> @thakis, any idea why unittests no longer manage to symbolize stack traces 
>>> on crash on the windows bot? I believe this used to work...
>>
>> I do not know. Maybe related to the "run many unit tests in a single 
>> process" lit change from a month ago?
>
> I suspected that, and verified locally that:
>
> - llvm-symbolizer on PATH still works
> - LLVM_SYMBOLIZER_PATH env variable didn't work, but I fixed it in 
> 1236b66a98197109ed40141329d6056dfbe25967 
>  along 
> with this reland, still no dice.

(My bot neither has llvm-symbolizer on path, nor sets LLVM_SYMBOLIZER_PATH 
fwiw.)

>> Anyways, looks like this relanded and broke tests yet again. Maybe find a 
>> win box before relanding the next time?
>
> I found one, but the test doesn't crash (nor on the premerge bots).
>
> I'll revert again, but I have no idea how to proceed. Only this bot and 
> `llvm-avr-linux` show the failure, and neither of them have a working 
> symbolizer.

I wouldn't be super surprised if this is related to windows and delayed 
template parsing. I haven't found any bots on llvm's official waterfall that 
run ClangdTests.exe – maybe that's why there aren't more bots finding this?

I'd recommend building and running the test on a win box and see if it repros 
locally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115232

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

[clang-tools-extra] 77533ea - Revert "Reland(2) "[clangd] Indexing of standard library""

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-05-18T17:38:45+02:00
New Revision: 77533ea443aca6e9978d7c8a6822420f8345f6af

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

LOG: Revert "Reland(2) "[clangd] Indexing of standard library""

This reverts commit ca875539f788c8063e243ce9ceb877a0d2ad9115.

Added: 


Modified: 
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/TUScheduler.h
clang-tools-extra/clangd/index/FileIndex.cpp
clang-tools-extra/clangd/index/FileIndex.h
clang-tools-extra/clangd/index/SymbolOrigin.cpp
clang-tools-extra/clangd/index/SymbolOrigin.h
clang-tools-extra/clangd/unittests/CMakeLists.txt
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Removed: 
clang-tools-extra/clangd/index/StdLib.cpp
clang-tools-extra/clangd/index/StdLib.h
clang-tools-extra/clangd/unittests/StdLibTests.cpp



diff  --git a/clang-tools-extra/clangd/CMakeLists.txt 
b/clang-tools-extra/clangd/CMakeLists.txt
index 7cfbd6f95750e..9c37cfe7b7001 100644
--- a/clang-tools-extra/clangd/CMakeLists.txt
+++ b/clang-tools-extra/clangd/CMakeLists.txt
@@ -119,7 +119,6 @@ add_clang_library(clangDaemon
   index/Ref.cpp
   index/Relation.cpp
   index/Serialization.cpp
-  index/StdLib.cpp
   index/Symbol.cpp
   index/SymbolCollector.cpp
   index/SymbolID.cpp

diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index 69a0f63972aae..80d7d5c5ece19 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -26,7 +26,6 @@
 #include "index/CanonicalIncludes.h"
 #include "index/FileIndex.h"
 #include "index/Merge.h"
-#include "index/StdLib.h"
 #include "refactor/Rename.h"
 #include "refactor/Tweak.h"
 #include "support/Cancellation.h"
@@ -60,39 +59,16 @@ namespace {
 // Update the FileIndex with new ASTs and plumb the diagnostics responses.
 struct UpdateIndexCallbacks : public ParsingCallbacks {
   UpdateIndexCallbacks(FileIndex *FIndex,
-   ClangdServer::Callbacks *ServerCallbacks,
-   const ThreadsafeFS , AsyncTaskRunner *Tasks)
-  : FIndex(FIndex), ServerCallbacks(ServerCallbacks), TFS(TFS),
-Tasks(Tasks) {}
+   ClangdServer::Callbacks *ServerCallbacks)
+  : FIndex(FIndex), ServerCallbacks(ServerCallbacks) {}
 
-  void onPreambleAST(PathRef Path, llvm::StringRef Version,
- const CompilerInvocation , ASTContext ,
+  void onPreambleAST(PathRef Path, llvm::StringRef Version, ASTContext ,
  Preprocessor ,
  const CanonicalIncludes ) override {
-// If this preamble uses a standard library we haven't seen yet, index it.
-if (FIndex)
-  if (auto Loc = Stdlib.add(*CI.getLangOpts(), PP.getHeaderSearchInfo()))
-indexStdlib(CI, std::move(*Loc));
-
 if (FIndex)
   FIndex->updatePreamble(Path, Version, Ctx, PP, CanonIncludes);
   }
 
-  void indexStdlib(const CompilerInvocation , StdLibLocation Loc) {
-auto Task = [this, LO(*CI.getLangOpts()), Loc(std::move(Loc)),
- CI(std::make_unique(CI))]() mutable {
-  IndexFileIn IF;
-  IF.Symbols = indexStandardLibrary(std::move(CI), Loc, TFS);
-  if (Stdlib.isBest(LO))
-FIndex->updatePreamble(std::move(IF));
-};
-if (Tasks)
-  // This doesn't have a semaphore to enforce -j, but it's rare.
-  Tasks->runAsync("IndexStdlib", std::move(Task));
-else
-  Task();
-  }
-
   void onMainAST(PathRef Path, ParsedAST , PublishFn Publish) override {
 if (FIndex)
   FIndex->updateMain(Path, AST);
@@ -127,9 +103,6 @@ struct UpdateIndexCallbacks : public ParsingCallbacks {
 private:
   FileIndex *FIndex;
   ClangdServer::Callbacks *ServerCallbacks;
-  const ThreadsafeFS 
-  StdLibSet Stdlib;
-  AsyncTaskRunner *Tasks;
 };
 
 class DraftStoreFS : public ThreadsafeFS {
@@ -181,15 +154,12 @@ ClangdServer::ClangdServer(const 
GlobalCompilationDatabase ,
   Transient(Opts.ImplicitCancellation ? TUScheduler::InvalidateOnUpdate
   : TUScheduler::NoInvalidation),
   DirtyFS(std::make_unique(TFS, DraftMgr)) {
-  if (Opts.AsyncThreadsCount != 0)
-IndexTasks.emplace();
   // Pass a callback into `WorkScheduler` to extract symbols from a newly
   // parsed file and rebuild the file index synchronously each time an AST
   // is parsed.
-  WorkScheduler.emplace(CDB, 

[PATCH] D115232: [clangd] Indexing of standard library

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D115232#3522514 , @thakis wrote:

> In D115232#3520461 , @sammccall 
> wrote:
>
>> Hmm, the test keeps crashing on the GN bot: 
>> http://45.33.8.238/win/58316/step_9.txt
>> Unfortunately the stacktrace is not symbolized, and I'm not seeing this 
>> elsewhere (e.g. premerge bot).
>>
>> @thakis, any idea why unittests no longer manage to symbolize stack traces 
>> on crash on the windows bot? I believe this used to work...
>
> I do not know. Maybe related to the "run many unit tests in a single process" 
> lit change from a month ago?

I suspected that, and verified locally that:

- llvm-symbolizer on PATH still works
- LLVM_SYMBOLIZER_PATH env variable didn't work, but I fixed it in 
1236b66a98197109ed40141329d6056dfbe25967 
 along 
with this reland, still no dice.

> Anyways, looks like this relanded and broke tests yet again. Maybe find a win 
> box before relanding the next time?

I found one, but the test doesn't crash (nor on the premerge bots).

I'll revert again, but I have no idea how to proceed. Only this bot and 
`llvm-avr-linux` show the failure, and neither of them have a working 
symbolizer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115232

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

[PATCH] D125770: [clang-tidy] modernize-deprecated-headers should ignore system headers

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

steakhal added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp:37
 
-#include  // FIXME: We should have no warning into system 
headers.
-// CHECK-MESSAGES-WARN-INTO-HEADERS: mysystemlib.h:1:10: warning: inclusion of 
deprecated C++ header 'assert.h'; consider using 'cassert' instead 
[modernize-deprecated-headers]
+#include  // no-warning: Don't warn into system headers.
 

LegalizeAdulthood wrote:
> steakhal wrote:
> > LegalizeAdulthood wrote:
> > > Where is this file?  I can't find it in my tree.
> > > 
> > > ```
> > > D:\legalize\llvm\llvm-project\clang-tools-extra
> > > > dir/s/b mysystemlib.h
> > > File Not Found
> > > ```
> > > 
> > > Does this patch depend on some other unsubmitted patch?
> > D125209 should have included this file at 
> > `clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-deprecated-headers/mysystemlib.h`.
> OK, so it depends on D125209  why not just include these small changes in 
> that?
> 
> Supposedly there is some way in phabricator to make one review dependent on 
> another, but instead of being an expert in phabricator this change seems 
> small enough to just fold into the other review, but perhaps opinions differ 
> on that.
I set the 'parent revision', so the dependencies are present under the "Stack" 
tab. Sorry if I confused you.
I still think it's a logically separate issue, which should be addressed in a 
separate commit. That change is already quite confusing. I already had to ever 
it once, thus I want to keep the cognitive complexity as low as possible there. 
Same applies here.

Do you insist on merging these two?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125770

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

[PATCH] D125604: [FileCheck] Catch missspelled directives.

[Ivan Kosarev via Phabricator via cfe-commits]

kosarev updated this revision to Diff 430392.
kosarev added a comment.
Herald added a project: Flang.

Added Flang fixes and rebased.

Thanks Simon for the quick turnaround!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125604

Files:
  clang/test/CodeGen/cmse-clear-return.c
  clang/test/CodeGenCXX/attr-mustprogress.cpp
  clang/test/CodeGenCXX/eh-aggregate-copy-destroy.cpp
  clang/test/CodeGenCXX/inheriting-constructor.cpp
  clang/test/CodeGenObjC/non-runtime-protocol.m
  clang/test/OpenMP/master_taskloop_private_codegen.cpp
  clang/test/OpenMP/master_taskloop_simd_private_codegen.cpp
  clang/test/OpenMP/parallel_master_taskloop_private_codegen.cpp
  clang/test/OpenMP/parallel_master_taskloop_simd_private_codegen.cpp
  clang/test/OpenMP/task_private_codegen.cpp
  clang/test/OpenMP/taskgroup_task_reduction_codegen.cpp
  clang/test/OpenMP/taskloop_private_codegen.cpp
  clang/test/OpenMP/taskloop_simd_private_codegen.cpp
  flang/test/Fir/convert-to-llvm.fir
  flang/test/Lower/Intrinsics/not.f90
  llvm/include/llvm/FileCheck/FileCheck.h
  llvm/lib/FileCheck/FileCheck.cpp
  llvm/test/Analysis/MemorySSA/phi-translation.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_4.ll
  llvm/test/CodeGen/AMDGPU/divergence-driven-bfe-isel.ll
  llvm/test/CodeGen/AMDGPU/hoist-cond.ll
  llvm/test/CodeGen/AMDGPU/llvm.amdgcn.ps.live.ll
  llvm/test/CodeGen/AMDGPU/mode-register.mir
  llvm/test/CodeGen/AMDGPU/phi-vgpr-input-moveimm.mir
  llvm/test/CodeGen/AMDGPU/smrd.ll
  llvm/test/CodeGen/ARM/cmpxchg-O0-be.ll
  llvm/test/CodeGen/AVR/atomics/fence.ll
  llvm/test/CodeGen/BPF/CORE/offset-reloc-middle-chain.ll
  llvm/test/CodeGen/MIR/AMDGPU/extra-imm-operand.mir
  llvm/test/CodeGen/MIR/AMDGPU/extra-reg-operand.mir
  llvm/test/CodeGen/WebAssembly/libcalls.ll
  llvm/test/DebugInfo/NVPTX/debug-info.ll
  llvm/test/FileCheck/missspelled-directive.txt
  llvm/test/MC/AMDGPU/data.s
  llvm/test/MC/AsmParser/directive_file-g.s
  llvm/test/MC/PowerPC/ppc64-reloc-directive-pcrel.s
  llvm/test/MC/WebAssembly/unnamed-data.ll
  llvm/test/Transforms/Inline/inline-strictfp.ll
  llvm/test/Transforms/LoopVectorize/X86/gather-vs-interleave.ll
  llvm/test/Transforms/MergeFunc/alias.ll
  llvm/test/Transforms/PGOProfile/PR41279.ll
  llvm/test/Transforms/PGOProfile/memop_clone.ll
  llvm/test/Transforms/PGOProfile/memop_size_from_strlen.ll
  llvm/test/tools/llvm-dwp/X86/tu_units_v5.s
  llvm/test/tools/llvm-dwp/X86/type_dedup_v5.test
  llvm/test/tools/llvm-objdump/MachO/disassemble-all.test
  llvm/test/tools/llvm-readobj/COFF/unwind-arm64-windows.test
  mlir/test/Conversion/MemRefToSPIRV/alloc.mlir
  mlir/test/Dialect/Affine/loop-coalescing.mlir
  mlir/test/Dialect/Linalg/fuse-with-reshape-by-collapsing.mlir
  mlir/test/Dialect/Linalg/tile-and-fuse-no-fuse.mlir
  mlir/test/Dialect/MemRef/canonicalize.mlir
  mlir/test/Dialect/SPIRV/IR/memory-ops.mlir
  mlir/test/Dialect/Vector/vector-transfer-full-partial-split.mlir
  mlir/test/IR/dynamic.mlir
  mlir/test/mlir-tblgen/op-decl-and-defs.td
  polly/test/ScopDetect/dot-scops-npm.ll

Index: polly/test/ScopDetect/dot-scops-npm.ll
===
--- polly/test/ScopDetect/dot-scops-npm.ll
+++ polly/test/ScopDetect/dot-scops-npm.ll
@@ -30,35 +30,35 @@
 ; CHECK-NEXT: Node0x[[OUTER_EXIT]] -> Node0x[[RETURN_ID:.*]];
 ; CHECK-NEXT: Node0x[[RETURN_ID]] [shape=record,label="{return:
 ; CHECK-NEXT: colorscheme = "paired12"
-; CHECK_NEXT: subgraph cluster_0x[[:.*]] {
-; CHECK_NEXT: label = "";
-; CHECK_NEXT: style = solid;
-; CHECK_NEXT: color = 1
-; CHECK_NEXT: subgraph cluster_0x[[:.*]] {
-; CHECK_NEXT: label = "";
-; CHECK_NEXT: style = filled;
-; CHECK_NEXT: color = 3subgraph cluster_0x7152c40 {
-; CHECK_NEXT: label = "";
-; CHECK_NEXT: style = solid;
-; CHECK_NEXT: color = 5
-; CHECK_NEXT: subgraph cluster_0x[[:.*]] {
-; CHECK_NEXT: label = "";
-; CHECK_NEXT: style = solid;
-; CHECK_NEXT: color = 7
-; CHECK_NEXT: Node0x[[INNER_FOR_ID]];
-; CHECK_NEXT: Node0x[[BABY1_ID]];
-; CHECK_NEXT: Node0x[[INNER_INC_ID]];
-; CHECK_NEXT: }
-; CHECK_NEXT: Node0x[[OUTER_FOR_ID]];
-; CHECK_NEXT: Node0x[[INNER_EXIT_ID]];
-; CHECK_NEXT: Node0x[[OUTER_INC_ID]];
-; CHECK_NEXT: }
-; CHECK_NEXT: Node0x[[OUTER_EXIT]];
-; CHECK_NEXT: }
-; CHECK_NEXT: Node0x[[EntryID]];
-; CHECK_NEXT: Node0x[[RETURN_ID]];
-; CHECK_NEXT: }
-; CHECK_NEXT: }
+; CHECK-NEXT: subgraph cluster_0x{{.*}} {
+; CHECK-NEXT: label = "";
+; CHECK-NEXT: style = solid;
+; CHECK-NEXT: color = 1
+; CHECK-NEXT: subgraph cluster_0x{{.*}} {
+; CHECK-NEXT: label = "";
+; CHECK-NEXT: style = filled;
+; CHECK-NEXT: color = 3subgraph cluster_0x{{.*}} {
+; CHECK-NEXT: label = "";
+; CHECK-NEXT: style = solid;
+; CHECK-NEXT: color = 5
+; CHECK-NEXT: subgraph cluster_0x{{.*}} {
+; CHECK-NEXT: label = "";
+; CHECK-NEXT: style = solid;
+; CHECK-NEXT: color = 7
+; CHECK-NEXT: Node0x[[INNER_FOR_ID]];
+; CHECK-NEXT: Node0x[[BABY1_ID]];

[PATCH] D125893: [RISCV][NFC] Change interface of RVVIntrinsic::getSuffixStr

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng updated this revision to Diff 430391.
kito-cheng added a comment.

Changes:

- clang-format has applied on unexpected part, remove that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125893

Files:
  clang/include/clang/Support/RISCVVIntrinsicUtils.h
  clang/lib/Support/RISCVVIntrinsicUtils.cpp


Index: clang/lib/Support/RISCVVIntrinsicUtils.cpp
===
--- clang/lib/Support/RISCVVIntrinsicUtils.cpp
+++ clang/lib/Support/RISCVVIntrinsicUtils.cpp
@@ -928,7 +928,7 @@
 
 std::string RVVIntrinsic::getSuffixStr(
 BasicType Type, int Log2LMUL,
-const llvm::SmallVector ) {
+const llvm::ArrayRef ) {
   SmallVector SuffixStrs;
   for (auto PD : PrototypeDescriptors) {
 auto T = RVVType::computeType(Type, Log2LMUL, PD);
Index: clang/include/clang/Support/RISCVVIntrinsicUtils.h
===
--- clang/include/clang/Support/RISCVVIntrinsicUtils.h
+++ clang/include/clang/Support/RISCVVIntrinsicUtils.h
@@ -341,9 +341,9 @@
   // Return the type string for a BUILTIN() macro in Builtins.def.
   std::string getBuiltinTypeStr() const;
 
-  static std::string getSuffixStr(
-  BasicType Type, int Log2LMUL,
-  const llvm::SmallVector );
+  static std::string
+  getSuffixStr(BasicType Type, int Log2LMUL,
+   const llvm::ArrayRef 
);
 };
 
 } // end namespace RISCV


Index: clang/lib/Support/RISCVVIntrinsicUtils.cpp
===
--- clang/lib/Support/RISCVVIntrinsicUtils.cpp
+++ clang/lib/Support/RISCVVIntrinsicUtils.cpp
@@ -928,7 +928,7 @@
 
 std::string RVVIntrinsic::getSuffixStr(
 BasicType Type, int Log2LMUL,
-const llvm::SmallVector ) {
+const llvm::ArrayRef ) {
   SmallVector SuffixStrs;
   for (auto PD : PrototypeDescriptors) {
 auto T = RVVType::computeType(Type, Log2LMUL, PD);
Index: clang/include/clang/Support/RISCVVIntrinsicUtils.h
===
--- clang/include/clang/Support/RISCVVIntrinsicUtils.h
+++ clang/include/clang/Support/RISCVVIntrinsicUtils.h
@@ -341,9 +341,9 @@
   // Return the type string for a BUILTIN() macro in Builtins.def.
   std::string getBuiltinTypeStr() const;
 
-  static std::string getSuffixStr(
-  BasicType Type, int Log2LMUL,
-  const llvm::SmallVector );
+  static std::string
+  getSuffixStr(BasicType Type, int Log2LMUL,
+   const llvm::ArrayRef );
 };
 
 } // end namespace RISCV
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125885: [clang-tidy] bugprone-argument-comment: Ignore calls to user-defined literals

[Richard via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125885

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