[PATCH] D158433: [Clang] Do not change the type of captured vars when checking lambda constraints

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/Sema/SemaConcept.cpp:722
+const_cast(cast(FD)));
+// Captures are not checked from within the lambda.
+LSI->AfterParameterList = false;

shafik wrote:
> This comment does not really explain to me the effect of 
> `LSI->AfterParameterList = false;`
`AfterParameterList` affects in which context we look for captured variables. I 
will try to think of a better comment.



Comment at: clang/lib/Sema/SemaExpr.cpp:19754
+
+// When evaluating some attributes (like enable_if) we might refer to a
+// function parameter appertaining to the same declaration as that

shafik wrote:
> Why move this block of code for?
`isVariableAlreadyCapturedInScopeInfo` is kinda badly name because it will 
adjust `DeclRefType` by adding `const` as necessary.

if we capture a parameter, and then use it in a concept - which are not checked 
from within the scope of the lambda, we need to add const to it, which we can 
only do by reordering these paths. It's a bit subtle, and could do with some 
improvement as I'm not sure parameters will always be const correct in 
attributes currently. 
But I tried to do a minimal fix






Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158433

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

[PATCH] D158329: [X86] Support arch=x86-64{,-v2,-v3,-v4} for target_clones attribute

[Freddy, Ye via Phabricator via cfe-commits]

FreddyYe added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13319
+Value *Features = Builder.CreateAlignedLoad(
+Int32Ty, Builder.CreateGEP(ATy, CpuFeatures2, Idxs),
+CharUnits::fromQuantity(4));

MaskRay wrote:
> FreddyYe wrote:
> > Will function multi version also be fixed in this patch? 
> > https://gcc.godbolt.org/z/cafhs9qbG If so, need to add test in 
> > clang/test/CodeGen/attr-target-mv.c
> The `target` attribute has strange semantics for overloading:
> ```
> int __attribute__((target("arch=skylake"))) foo(void) {return 0;}
> int __attribute__((target("arch=x86-64"))) foo(void) {return 1;}
> ```
> is not allowed in C mode of GCC.
> 
> I think such use cases are not recommended in C++.
> 
> If we don't use overloading, `int __attribute__((target("arch=x86-64"))) 
> foo(void) {return 1;}` is supported by Clang today and this patch does not 
> intend to change anything about it.
I think behavior for `target` attribute is not only overloading but also 
function multiversioning for redefined functions. And seems like C model of gcc 
haven't supported is due to it will target C23 standard: 
https://clang.llvm.org/docs/AttributeReference.html#target 
Comparing to gcc, clang misses not only `target` attribute but also other 
cpuname/feature related features for "x86-64*". See 
https://gcc.godbolt.org/z/arhne35GG (Seems gcc defined x86-64* as not only cpu 
name but also feature name.) Anyway, this patch is targeting for 
`target_clones` attribute only. So no problem here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158329

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

[PATCH] D158329: [X86] Support arch=x86-64{,-v2,-v3,-v4} for target_clones attribute

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13319
+Value *Features = Builder.CreateAlignedLoad(
+Int32Ty, Builder.CreateGEP(ATy, CpuFeatures2, Idxs),
+CharUnits::fromQuantity(4));

FreddyYe wrote:
> MaskRay wrote:
> > FreddyYe wrote:
> > > Will function multi version also be fixed in this patch? 
> > > https://gcc.godbolt.org/z/cafhs9qbG If so, need to add test in 
> > > clang/test/CodeGen/attr-target-mv.c
> > The `target` attribute has strange semantics for overloading:
> > ```
> > int __attribute__((target("arch=skylake"))) foo(void) {return 0;}
> > int __attribute__((target("arch=x86-64"))) foo(void) {return 1;}
> > ```
> > is not allowed in C mode of GCC.
> > 
> > I think such use cases are not recommended in C++.
> > 
> > If we don't use overloading, `int __attribute__((target("arch=x86-64"))) 
> > foo(void) {return 1;}` is supported by Clang today and this patch does not 
> > intend to change anything about it.
> I think behavior for `target` attribute is not only overloading but also 
> function multiversioning for redefined functions. And seems like C model of 
> gcc haven't supported is due to it will target C23 standard: 
> https://clang.llvm.org/docs/AttributeReference.html#target 
> Comparing to gcc, clang misses not only `target` attribute but also other 
> cpuname/feature related features for "x86-64*". See 
> https://gcc.godbolt.org/z/arhne35GG (Seems gcc defined x86-64* as not only 
> cpu name but also feature name.) Anyway, this patch is targeting for 
> `target_clones` attribute only. So no problem here.
> So no problem here.

Right. Not supporting C and requiring duplicate definitions makes `target` not 
really useful. 
https://gcc.gnu.org/pipermail/gcc-patches/2015-October/430585.html Jeff Law 
said: "I wasn't aware that multi-versioning was only implemented for C++, that 
seems fairly lame.  I hope I didn't approve that :-)"




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158329

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

[PATCH] D157385: [clang][CFG] Cleanup functions

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

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



Comment at: clang/test/Analysis/scopes-cfg-output.cpp:1473-1474
+// CHECK-NEXT:3: F f __attribute__((cleanup(cleanup_F)));
+// CHECK-NEXT:4: [B1.3].~F() (Implicit destructor)
+// CHECK-NEXT:5: CleanupFunction (cleanup_F)
+// CHECK-NEXT:6: CFGScopeEnd(f)

aaronpuchert wrote:
> Interesting test! But it seems CodeGen has them swapped: compiling this 
> snippet with `clang -c -S -emit-llvm` I get
> ```lang=LLVM
> define dso_local void @_Z4testv() #0 personality ptr @__gxx_personality_v0 {
>   %1 = alloca %class.F, align 1
>   %2 = alloca ptr, align 8
>   %3 = alloca i32, align 4
>   invoke void @_Z9cleanup_FP1F(ptr noundef %1)
>   to label %4 unwind label %5
> 
> 4:; preds = %0
>   call void @_ZN1FD2Ev(ptr noundef nonnull align 1 dereferenceable(1) %1) #3
>   ret void
> 
> ; ...
> }
> ```
> So first cleanup, then destructor. This is with 17.0.0-rc2.
Interesting, I thought I checked this and used the correct order. Will 
re-check, thanks.



Comment at: clang/test/Analysis/scopes-cfg-output.cpp:1480
+public:
+  ~F() {}
+};

aaronpuchert wrote:
> As with the cleanup function, a definition shouldn't be necessary.
Is there a way to test whether the contents of the cleanup function are being 
checked as well? From these tests, I only know we consider them called, but not 
whether we (properly) analyze their bodies in the context as well. Or is that 
separate from this patch?


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

https://reviews.llvm.org/D157385

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

[PATCH] D117929: [XRay] Add support for RISCV

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

> Context not available

See 
https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface




Comment at: compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake:83
 set(ALL_XRAY_SUPPORTED_ARCH ${X86_64} ${ARM32} ${ARM64} ${MIPS32} ${MIPS64}
-   powerpc64le ${HEXAGON} ${LOONGARCH64})
+   powerpc64le ${HEXAGON} ${LOONGARCH64} #[[${RISCV32}]] 
${RISCV64})
 endif()

Just make RISCV32 available? Otherwise we wouldn't need `riscv32_SOURCES`



Comment at: compiler-rt/lib/xray/xray_riscv.cpp:51
+encodeRTypeInstruction(uint32_t Opcode, uint32_t Rs1, uint32_t Rs2,
+   uint32_t Rd) XRAY_NEVER_INSTRUMENT {
+  return (Rs2 << 20 | Rs1 << 15 | Rd << 7 | Opcode);

For new functions, consider dropping `XRAY_NEVER_INSTRUMENT`. The runtime 
library cannot be instrumented with `-fxray-instrument` and I an unsure why the 
original impl adds a lot of `XRAY_NEVER_INSTRUMENT`.



Comment at: compiler-rt/lib/xray/xray_riscv.cpp:52
+   uint32_t Rd) XRAY_NEVER_INSTRUMENT {
+  return (Rs2 << 20 | Rs1 << 15 | Rd << 7 | Opcode);
+}





Comment at: compiler-rt/lib/xray/xray_riscv.cpp:286
+  const XRaySledEntry &Sled) XRAY_NEVER_INSTRUMENT {
+  // FIXME: Implement for riscv?
+  return false;

Unimplemented features should use TODO instead of FIXME.



Comment at: compiler-rt/lib/xray/xray_riscv.cpp:162
+uint32_t HighestTracingHookAddr =
+(reinterpret_cast(TracingHook + 0x800) >> 44) & 0xf;
+// We typecast the Tracing Hook to a 32 bit value for RISCV32

ashwin98 wrote:
> jrtc27 wrote:
> > This is definitely wrong; you probably mean `(((TracingHook >> 32) + 0x800) 
> > >> 12) & 0xf`?
> True
`(x + 0x800) >> 12` is used many times. We need a helper like 
`lld/ELF/Arch/RISCV.cpp hi20`. Ditto for lo12.



Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:322
 
+void RISCVAsmPrinter::LowerPATCHABLE_FUNCTION_ENTER(const MachineInstr *MI) {
+  emitSled(MI, SledKind::FUNCTION_ENTER);

I have many comments in D140727 (LoongArch port). Many are probably useful here 
as well.


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

https://reviews.llvm.org/D117929

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

[PATCH] D153114: [clangd] [C++20] [Modules] Support C++20 modules for clangd

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D153114#4603579 , @sammccall wrote:

> In D153114#4602414 , @ChuanqiXu 
> wrote:
>
>>> Don't attempt any cross-file or cross-version coordination: i.e. don't try 
>>> to reuse BMIs between different files, don't try to reuse BMIs between 
>>> (preamble) reparses of the same file, don't try to persist the module 
>>> graph. Instead, when building a preamble, synchronously scan for the module 
>>> graph, build the required PCMs on the single preamble thread with filenames 
>>> private to that preamble, and then proceed to build the preamble.
>>
>> Do I understand right? If I understand correctly, I fully agree with the 
>> direction. We can go slowly, as long as we keep moving forward.
>>
>> Then I'd like to leave the patch as-is for referencing and create new 
>> patches following the suggestion.
>
> Yes, that's the suggestion, and that plan makes sense to me, thanks!
>
> I did some more thinking about this (having a concrete implementation helps a 
> lot!) and had a couple more thoughts.
> At some point we should write down a design somewhere, need to strike a 
> balance between doing it early enough to be useful but late enough that we've 
> understood!

Yeah, then let's make the page into some design ideas discussing page.

> Dep scanning - roles
> 
>
> IIUC we do this for two reasons:
>
> - to identify what module names we must have PCMs for in order to build a 
> given TU (either an open file, or a module we're building as PCM)
> - to build a database mapping module name => filename, which we compose with 
> the CDB to know how to build a PCM for a given module name
>
> I think it would be good to clearly separate these. The latter is simpler, 
> more performance-critical, async, and is probably not used at all if the 
> build system can tell us this mapping.
> The latter is more complex, and will always be needed synchronously for the 
> mainfile regardless of the build system.

Yes, agreed.

> Dep scanning - implementation
> -
>
> The dep scanner seems to work by getting the compile command and running the 
> preprocessor. This is fairly heavyweight, and I can't see anywhere it's going 
> into single-file mode - is it really reading all `#included` headers? This is 
> certainly not workable for reparses of the mainfile (when no headers have 
> changed).
>
> It seems unneccesary: the standard seems to go to some lengths to ensure that 
> we (almost) only need to lex the top-level file:
>
> - module and import decls can't be produced by macros (seems to be the effect 
> of the `pp-module` directive etc)
> - module and import decls can't be `#include`d (definition of `module-file` 
> and `[cpp.import]` rules)
>
> The wrinkle I see is that some PP usage is possible: the module name can be 
> produced by a macro, and imports can be `#ifdef`d. I think the former is very 
> unlikely (like `#include MACRO_NAME`) and we can not support it, and the 
> latter will just result in us overestimating the deps, which seems OK.
> You have more context here though, and maybe I'm misreading the restrictions 
> placed by the standard. Today clang doesn't seem to enforce most of these 
> sort of restrictions, which is probably worth fixing if they're real.
>
> (This doesn't apply to header modules: it's perfectly possible to include a 
> textual header which includes a modular header, and it's impossible to know 
> without actually preprocessing. This divergence is nasty, but I don't think 
> we should pessimize standard modules for it).

There are some problems due to the complexities of the standard...

The take away may be:

- module and import decls can't be produced by macros (seems to be the effect 
of the `pp-module` directive etc)
  - yes
- module and import decls can't be `#include`d (definition of `module-file` and 
`[cpp.import]` rules)
  - the module declarations can't be `#include`d.
  - but the import decls can be `#include`d partially. See the discussion of 
https://github.com/llvm/llvm-project/issues/59688 for detail. The explanation 
is:
- the wording(http://eel.is/c++draft/cpp.import#3) is "If a pp-import is 
produced by source file inclusion (including by the rewrite produced when a 
#include directive names an importable header) while processing the group of a 
module-file, the program is ill-formed."
- and the definition of `module-file` 
(http://eel.is/c++draft/cpp.pre#nt:module-file) is `pp-global-module-fragment 
pp-module group pp-private-module-fragment`.
- so the phrase `the group of a module-file` only refers to the `group` in 
the definition of `module-file` literally. We can't expand the grammar.
- the module name can be produced by a macro
  - yes
- imports can be #ifdefd.
  - yes. And this is a pretty important use-case for using modules in practice.

So possibly we have to look into the `#include`s when scanning.

> 

[PATCH] D155776: [NFC] Add checks for self-assignment.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 552243.
schittir added a comment.

Fix errors.


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

https://reviews.llvm.org/D155776

Files:
  clang/lib/AST/APValue.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/Interpreter/Value.cpp


Index: clang/lib/Interpreter/Value.cpp
===
--- clang/lib/Interpreter/Value.cpp
+++ clang/lib/Interpreter/Value.cpp
@@ -201,16 +201,17 @@
 }
 
 Value &Value::operator=(Value &&RHS) noexcept {
-  if (IsManuallyAlloc)
-ValueStorage::getFromPayload(getPtr())->Release();
+  if (this != &RHS) {
+if (IsManuallyAlloc)
+  ValueStorage::getFromPayload(getPtr())->Release();
 
-  Interp = std::exchange(RHS.Interp, nullptr);
-  OpaqueType = std::exchange(RHS.OpaqueType, nullptr);
-  ValueKind = std::exchange(RHS.ValueKind, K_Unspecified);
-  IsManuallyAlloc = std::exchange(RHS.IsManuallyAlloc, false);
-
-  Data = RHS.Data;
+Interp = std::exchange(RHS.Interp, nullptr);
+OpaqueType = std::exchange(RHS.OpaqueType, nullptr);
+ValueKind = std::exchange(RHS.ValueKind, K_Unspecified);
+IsManuallyAlloc = std::exchange(RHS.IsManuallyAlloc, false);
 
+Data = RHS.Data;
+  }
   return *this;
 }
 
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -832,8 +832,10 @@
 
   // Define copy assignment operator.
   ApplyDebugLocation &operator=(ApplyDebugLocation &&Other) {
-CGF = Other.CGF;
-Other.CGF = nullptr;
+if (this != &Other) {
+  CGF = Other.CGF;
+  Other.CGF = nullptr;
+}
 return *this;
   }
 
Index: clang/lib/AST/APValue.cpp
===
--- clang/lib/AST/APValue.cpp
+++ clang/lib/AST/APValue.cpp
@@ -390,11 +390,13 @@
 }
 
 APValue &APValue::operator=(APValue &&RHS) {
-  if (Kind != None && Kind != Indeterminate)
-DestroyDataAndMakeUninit();
-  Kind = RHS.Kind;
-  Data = RHS.Data;
-  RHS.Kind = None;
+  if (this != &RHS) {
+if (Kind != None && Kind != Indeterminate)
+  DestroyDataAndMakeUninit();
+Kind = RHS.Kind;
+Data = RHS.Data;
+RHS.Kind = None;
+  }
   return *this;
 }
 


Index: clang/lib/Interpreter/Value.cpp
===
--- clang/lib/Interpreter/Value.cpp
+++ clang/lib/Interpreter/Value.cpp
@@ -201,16 +201,17 @@
 }
 
 Value &Value::operator=(Value &&RHS) noexcept {
-  if (IsManuallyAlloc)
-ValueStorage::getFromPayload(getPtr())->Release();
+  if (this != &RHS) {
+if (IsManuallyAlloc)
+  ValueStorage::getFromPayload(getPtr())->Release();
 
-  Interp = std::exchange(RHS.Interp, nullptr);
-  OpaqueType = std::exchange(RHS.OpaqueType, nullptr);
-  ValueKind = std::exchange(RHS.ValueKind, K_Unspecified);
-  IsManuallyAlloc = std::exchange(RHS.IsManuallyAlloc, false);
-
-  Data = RHS.Data;
+Interp = std::exchange(RHS.Interp, nullptr);
+OpaqueType = std::exchange(RHS.OpaqueType, nullptr);
+ValueKind = std::exchange(RHS.ValueKind, K_Unspecified);
+IsManuallyAlloc = std::exchange(RHS.IsManuallyAlloc, false);
 
+Data = RHS.Data;
+  }
   return *this;
 }
 
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -832,8 +832,10 @@
 
   // Define copy assignment operator.
   ApplyDebugLocation &operator=(ApplyDebugLocation &&Other) {
-CGF = Other.CGF;
-Other.CGF = nullptr;
+if (this != &Other) {
+  CGF = Other.CGF;
+  Other.CGF = nullptr;
+}
 return *this;
   }
 
Index: clang/lib/AST/APValue.cpp
===
--- clang/lib/AST/APValue.cpp
+++ clang/lib/AST/APValue.cpp
@@ -390,11 +390,13 @@
 }
 
 APValue &APValue::operator=(APValue &&RHS) {
-  if (Kind != None && Kind != Indeterminate)
-DestroyDataAndMakeUninit();
-  Kind = RHS.Kind;
-  Data = RHS.Data;
-  RHS.Kind = None;
+  if (this != &RHS) {
+if (Kind != None && Kind != Indeterminate)
+  DestroyDataAndMakeUninit();
+Kind = RHS.Kind;
+Data = RHS.Data;
+RHS.Kind = None;
+  }
   return *this;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155776: [NFC] Add checks for self-assignment.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir added a comment.

@jplehr @michaelplatings @MitalAshok @tahonermann - sorry this one slipped 
through the cracks. I fixed the errors now. Thank you for your review.


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

https://reviews.llvm.org/D155776

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

[PATCH] D158066: [PowerPC] Fix use of FPSCR builtins in smmintrin.h

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf added a comment.

In D158066#4601785 , @nemanjai wrote:

> It should be perfectly fine to provide pre-defined macros for these to match 
> GCC on PowerPC. The reason we went with the macro solution is to avoid 
> polluting the builtins namespace for other targets.
>
> Also, please add some C++ tests for these PPC wrappers so that we aren't 
> surprised again when someone tries to use these in their C++ code.

It seems these macros do not always work for all PowerPC targets:

  // -target=ppc64le -mcpu=power9 do not work
  // -target=ppc64le-unknown-linux-gnu -mcpu=power9 work
  long calldarn(void) { return __darn(); }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158066

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

[PATCH] D158259: [clang][RISCV] Support operators for RVV sizeless vector types

[Jianjian Guan via Phabricator via cfe-commits]

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



Comment at: clang/test/CodeGen/riscv-rvv-vla-arith-ops.c:90
+//
+vfloat32m1_t add_f32(vfloat32m1_t a, vfloat32m1_t b) {
+  return a + b;

Jim wrote:
> Do we support operation for vfloat16 here?
Thanks to point it. I added fp16 vector cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158259

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

[PATCH] D158487: [PowerPC][altivec] Optimize codegen of vec_promote

[Kai Luo via Phabricator via cfe-commits]

lkail updated this revision to Diff 552251.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158487

Files:
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/PowerPC/builtins-ppc-vsx.c
  llvm/test/CodeGen/PowerPC/vec-promote.ll

Index: llvm/test/CodeGen/PowerPC/vec-promote.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/vec-promote.ll
@@ -0,0 +1,276 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 2
+; RUN: llc -mtriple=powerpc64-unknown-unknown -verify-machineinstrs -mcpu=pwr8 \
+; RUN:   < %s | FileCheck %s --check-prefix=CHECK-BE
+; RUN: llc -mtriple=powerpc64le-unknown-unknown -verify-machineinstrs -mcpu=pwr8 \
+; RUN:   < %s | FileCheck %s --check-prefix=CHECK-LE
+
+define noundef <2 x double> @vec_promote_double_zeroed(ptr nocapture noundef readonly %p) {
+; CHECK-BE-LABEL: vec_promote_double_zeroed:
+; CHECK-BE:   # %bb.0: # %entry
+; CHECK-BE-NEXT:lfd 0, 0(3)
+; CHECK-BE-NEXT:xxlxor 1, 1, 1
+; CHECK-BE-NEXT:xxmrghd 34, 0, 1
+; CHECK-BE-NEXT:blr
+;
+; CHECK-LE-LABEL: vec_promote_double_zeroed:
+; CHECK-LE:   # %bb.0: # %entry
+; CHECK-LE-NEXT:lfd 0, 0(3)
+; CHECK-LE-NEXT:xxlxor 1, 1, 1
+; CHECK-LE-NEXT:xxmrghd 34, 1, 0
+; CHECK-LE-NEXT:blr
+entry:
+  %0 = load double, ptr %p, align 8
+  %vecins.i = insertelement <2 x double> , double %0, i64 0
+  ret <2 x double> %vecins.i
+}
+
+define noundef <2 x double> @vec_promote_double(ptr nocapture noundef readonly %p) {
+; CHECK-BE-LABEL: vec_promote_double:
+; CHECK-BE:   # %bb.0: # %entry
+; CHECK-BE-NEXT:lxvdsx 34, 0, 3
+; CHECK-BE-NEXT:blr
+;
+; CHECK-LE-LABEL: vec_promote_double:
+; CHECK-LE:   # %bb.0: # %entry
+; CHECK-LE-NEXT:lxvdsx 34, 0, 3
+; CHECK-LE-NEXT:blr
+entry:
+  %0 = load double, ptr %p, align 8
+  %vecins.i = insertelement <2 x double> poison, double %0, i64 0
+  ret <2 x double> %vecins.i
+}
+
+define noundef <4 x float> @vec_promote_float_zeroed(ptr nocapture noundef readonly %p) {
+; CHECK-BE-LABEL: vec_promote_float_zeroed:
+; CHECK-BE:   # %bb.0: # %entry
+; CHECK-BE-NEXT:lfs 1, 0(3)
+; CHECK-BE-NEXT:xxlxor 0, 0, 0
+; CHECK-BE-NEXT:xxspltd 2, 0, 0
+; CHECK-BE-NEXT:xxmrghd 0, 1, 0
+; CHECK-BE-NEXT:xvcvdpsp 34, 2
+; CHECK-BE-NEXT:xvcvdpsp 35, 0
+; CHECK-BE-NEXT:vmrgew 2, 3, 2
+; CHECK-BE-NEXT:blr
+;
+; CHECK-LE-LABEL: vec_promote_float_zeroed:
+; CHECK-LE:   # %bb.0: # %entry
+; CHECK-LE-NEXT:lfs 1, 0(3)
+; CHECK-LE-NEXT:xxlxor 0, 0, 0
+; CHECK-LE-NEXT:xxspltd 2, 0, 0
+; CHECK-LE-NEXT:xxmrghd 0, 0, 1
+; CHECK-LE-NEXT:xvcvdpsp 34, 2
+; CHECK-LE-NEXT:xvcvdpsp 35, 0
+; CHECK-LE-NEXT:vmrgew 2, 2, 3
+; CHECK-LE-NEXT:blr
+entry:
+  %0 = load float, ptr %p, align 8
+  %vecins.i = insertelement <4 x float> , float %0, i64 0
+  ret <4 x float> %vecins.i
+}
+
+define noundef <4 x float> @vec_promote_float(ptr nocapture noundef readonly %p) {
+; CHECK-BE-LABEL: vec_promote_float:
+; CHECK-BE:   # %bb.0: # %entry
+; CHECK-BE-NEXT:lfiwzx 0, 0, 3
+; CHECK-BE-NEXT:xxspltw 34, 0, 1
+; CHECK-BE-NEXT:blr
+;
+; CHECK-LE-LABEL: vec_promote_float:
+; CHECK-LE:   # %bb.0: # %entry
+; CHECK-LE-NEXT:lfiwzx 0, 0, 3
+; CHECK-LE-NEXT:xxspltw 34, 0, 1
+; CHECK-LE-NEXT:blr
+entry:
+  %0 = load float, ptr %p, align 8
+  %vecins.i = insertelement <4 x float> poison, float %0, i64 0
+  ret <4 x float> %vecins.i
+}
+
+define noundef <2 x i64> @vec_promote_long_long_zeroed(ptr nocapture noundef readonly %p) {
+; CHECK-BE-LABEL: vec_promote_long_long_zeroed:
+; CHECK-BE:   # %bb.0: # %entry
+; CHECK-BE-NEXT:ld 3, 0(3)
+; CHECK-BE-NEXT:li 4, 0
+; CHECK-BE-NEXT:mtfprd 0, 4
+; CHECK-BE-NEXT:mtfprd 1, 3
+; CHECK-BE-NEXT:xxmrghd 34, 1, 0
+; CHECK-BE-NEXT:blr
+;
+; CHECK-LE-LABEL: vec_promote_long_long_zeroed:
+; CHECK-LE:   # %bb.0: # %entry
+; CHECK-LE-NEXT:ld 3, 0(3)
+; CHECK-LE-NEXT:li 4, 0
+; CHECK-LE-NEXT:mtfprd 0, 4
+; CHECK-LE-NEXT:mtfprd 1, 3
+; CHECK-LE-NEXT:xxmrghd 34, 0, 1
+; CHECK-LE-NEXT:blr
+entry:
+  %0 = load i64, ptr %p, align 8
+  %vecins.i = insertelement <2 x i64> , i64 %0, i64 0
+  ret <2 x i64> %vecins.i
+}
+
+define noundef <2 x i64> @vec_promote_long_long(ptr nocapture noundef readonly %p) {
+; CHECK-BE-LABEL: vec_promote_long_long:
+; CHECK-BE:   # %bb.0: # %entry
+; CHECK-BE-NEXT:lxvdsx 34, 0, 3
+; CHECK-BE-NEXT:blr
+;
+; CHECK-LE-LABEL: vec_promote_long_long:
+; CHECK-LE:   # %bb.0: # %entry
+; CHECK-LE-NEXT:lxvdsx 34, 0, 3
+; CHECK-LE-NEXT:blr
+entry:
+  %0 = load i64, ptr %p, align 8
+  %vecins.i = insertelement <2 x i64> poison, i64 %0, i64 0
+  ret <2 x i64> %vecins.i
+}
+
+define noundef <4 x i32> @vec_promote_int_zeroed(ptr nocapture noundef readonly %p) {
+; CHECK-BE-LABEL: vec_promote_int_zeroed

[PATCH] D157684: [clang][ASTImporter] Repeated friend templates are partially imported

[Ding Fei via Phabricator via cfe-commits]

danix800 added a comment.

ping~ @balazske @aaron.ballman


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157684

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

[PATCH] D157297: [clang] Fixes compile error that double colon operator cannot resolve macro with parentheses.

[Yonggang Luo via Phabricator via cfe-commits]

lygstate added a comment.



> Thanks @aaron.ballman for the detailed explanation! I have learnt it. 
> @lygstate maybe include Aaron's explanation in commit message?

The commit message is updated, is that ready for merge?


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

https://reviews.llvm.org/D157297

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

[PATCH] D157497: feat: Migrate isArch16Bit

[Evgeniy Makarev via Phabricator via cfe-commits]

Pivnoy added a comment.
Herald added a subscriber: sunshaoce.

@bulbazord 
@MaskRay


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157497

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

[PATCH] D157497: feat: Migrate isArch16Bit

[Nikita Popov via Phabricator via cfe-commits]

nikic requested changes to this revision.
nikic added a comment.
This revision now requires changes to proceed.
Herald added a subscriber: StephenFan.

This change looks strictly worse in isolation, the proposal on discourse did 
not reach consensus, and looking at the diagram in 
https://discourse.llvm.org/t/rfc-refactor-triple-related-classes/70410/11 there 
is zero chance that it is ever going to be accepted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157497

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

[PATCH] D158487: [PowerPC][altivec] Optimize codegen of vec_promote

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf added inline comments.



Comment at: clang/lib/Headers/altivec.h:14662
+  vector unsigned char __res =
+  __builtin_shufflevector(__zero, __zero, -1, -1, -1, -1, -1, -1, -1, -1,
+  -1, -1, -1, -1, -1, -1, -1, -1);

Could we just define it without initialization? This can also make undefined 
vector.



Comment at: llvm/test/CodeGen/PowerPC/vec-promote.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py 
UTC_ARGS: --version 2
+; RUN: llc -mtriple=powerpc64-unknown-unknown -verify-machineinstrs -mcpu=pwr8 
\

We don't need this file because (1) no backend codegen changed; (2) further 
changes to `altivec.h` will not change this file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158487

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

[PATCH] D151730: [RISCV] Support target attribute for function

[Piyou Chen via Phabricator via cfe-commits]

BeMg updated this revision to Diff 552265.
BeMg added a comment.

Also remove the `-` operator from target attribute


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151730

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/test/CodeGen/RISCV/riscv-func-attr-target.c

Index: clang/test/CodeGen/RISCV/riscv-func-attr-target.c
===
--- /dev/null
+++ clang/test/CodeGen/RISCV/riscv-func-attr-target.c
@@ -0,0 +1,19 @@
+// REQUIRES: riscv-registered-target
+// RUN: %clang_cc1 -triple riscv64 -target-feature +zifencei \
+// RUN:  -target-feature +m -target-feature +a   \
+// RUN:  -target-feature +f -target-feature +d   \
+// RUN:  -emit-llvm %s -o - | FileCheck %s   \
+// RUN:  --check-prefix=CHECK-IR
+
+// CHECK-IR: void @test1() #0
+__attribute__((target("arch=+v,+zbb,+zicond1p0"))) void test1() {}
+
+// CHECK-IR: void @test2() #1
+__attribute__((target("arch=rv64gc_zbb"))) void test2 () {}
+
+// CHECK-IR: void @test3() #2
+__attribute__((target("cpu=rocket-rv64;tune=generic-rv64;arch=+v"))) void test3 () {}
+
+// CHECK-IR: attributes #0 {{.*}}+experimental-zicond{{.*}}+v,+zbb{{.*}}+zve32f,+zve32x,+zve64d,+zve64f,+zve64x,+zvl128b,+zvl32b,+zvl64b{{.*}}
+// CHECK-IR: attributes #1 {{.*}}+c{{.*}}+zbb{{.*}}
+// CHECK-IR: attributes #2 {{.*}} "target-cpu"="rocket-rv64" {{.*}}+v{{.*}} "tune-cpu"="generic-rv64" {{.*}}
Index: clang/lib/Basic/Targets/RISCV.h
===
--- clang/lib/Basic/Targets/RISCV.h
+++ clang/lib/Basic/Targets/RISCV.h
@@ -114,6 +114,9 @@
   void fillValidCPUList(SmallVectorImpl &Values) const override;
   bool isValidTuneCPUName(StringRef Name) const override;
   void fillValidTuneCPUList(SmallVectorImpl &Values) const override;
+  bool supportsTargetAttributeTune() const override { return true; }
+  bool validateCpuSupports(StringRef FeatureStr) const override;
+  ParsedTargetAttr parseTargetAttr(StringRef Str) const override;
 };
 class LLVM_LIBRARY_VISIBILITY RISCV32TargetInfo : public RISCVTargetInfo {
 public:
Index: clang/lib/Basic/Targets/RISCV.cpp
===
--- clang/lib/Basic/Targets/RISCV.cpp
+++ clang/lib/Basic/Targets/RISCV.cpp
@@ -250,12 +250,17 @@
 
   // RISCVISAInfo makes implications for ISA features
   std::vector ImpliedFeatures = (*ParseResult)->toFeatureVector();
+  std::vector UpdatedFeatures;
+
   // Add non-ISA features like `relax` and `save-restore` back
   for (const std::string &Feature : FeaturesVec)
 if (!llvm::is_contained(ImpliedFeatures, Feature))
-  ImpliedFeatures.push_back(Feature);
+  UpdatedFeatures.push_back(Feature);
+
+  UpdatedFeatures.insert(UpdatedFeatures.end(), ImpliedFeatures.begin(),
+ ImpliedFeatures.end());
 
-  return TargetInfo::initFeatureMap(Features, Diags, CPU, ImpliedFeatures);
+  return TargetInfo::initFeatureMap(Features, Diags, CPU, UpdatedFeatures);
 }
 
 std::optional>
@@ -346,3 +351,74 @@
   bool Is64Bit = getTriple().isArch64Bit();
   llvm::RISCV::fillValidTuneCPUArchList(Values, Is64Bit);
 }
+
+// Parse RISC-V Target attributes, which are a comma separated list of:
+//  "arch=" - parsed to features as per -march=..
+//  "cpu=" - parsed to features as per -mcpu=.., with CPU set to 
+//  "tune=" - TuneCPU set to 
+ParsedTargetAttr RISCVTargetInfo::parseTargetAttr(StringRef Features) const {
+  ParsedTargetAttr Ret;
+  if (Features == "default")
+return Ret;
+  SmallVector AttrFeatures;
+  Features.split(AttrFeatures, ";");
+  bool FoundArch = false;
+
+  for (auto &Feature : AttrFeatures) {
+Feature = Feature.trim();
+StringRef AttrString = Feature.split("=").second.trim();
+
+if (Feature.startswith("arch=")) {
+  if (FoundArch)
+Ret.Duplicate = "arch=";
+  FoundArch = true;
+
+  if (AttrString.startswith("+")) {
+// EXTENSION like arch=+v,+zbb
+SmallVector Exts;
+AttrString.split(Exts, ",");
+for (auto Ext : Exts) {
+  if (Ext.empty())
+continue;
+
+  StringRef ExtName = Ext.substr(1);
+  std::string TargetFeature =
+  llvm::RISCVISAInfo::getTargetFeatureForExtension(ExtName);
+  if (!TargetFeature.empty())
+Ret.Features.push_back(Ext.front() + TargetFeature);
+  else
+Ret.Features.push_back(Ext.str());
+}
+  } else {
+// full-arch-string like arch=rv64gcv
+auto RII = llvm::RISCVISAInfo::parseArchString(
+AttrString, /* EnableExperimentalExtension */ true);
+if (!RII) {
+  consumeError(RII.takeError());
+} else {
+  std::vector FeatStrings = (*RII)->toFeatureVector();
+  for (auto FeatString : FeatStrings)
+Ret.Features.push_

[PATCH] D158484: [PowerPC][altivec] Correct modulo number of vec_promote on vector char

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf accepted this revision as: qiucf.
qiucf added a comment.
This revision is now accepted and ready to land.

Thanks for the fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158484

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

[PATCH] D158496: [WIP][clang][modules] module build daemon initial commit

[Connor Sughrue via Phabricator via cfe-commits]

cpsughrue created this revision.
cpsughrue added reviewers: jansvoboda11, Bigcheese.
Herald added a project: All.
cpsughrue requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Initial commit for module build daemon. The title will be updated soon to 
remove the [WIP] tag once I am done writing tests.

The goal of the commit: A source file that doesn't depend on any modules can 
connect to the daemon, get scanned, find that there are no modules, then resume 
building. If there are any modules in the dependency graph have the daemon emit 
a warning that dependencies were detected but that the daemon cannot yet build 
modules or update the cc1 command line.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158496

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/include/clang/Tooling/ModuleBuildDaemon/Protocol.h
  clang/include/clang/Tooling/ModuleBuildDaemon/SocketSupport.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/ModuleBuildDaemon/CMakeLists.txt
  clang/lib/Tooling/ModuleBuildDaemon/Protocol.cpp
  clang/lib/Tooling/ModuleBuildDaemon/SocketSupport.cpp
  clang/tools/driver/CMakeLists.txt
  clang/tools/driver/cc1_main.cpp
  clang/tools/driver/cc1modbuildd_main.cpp
  clang/tools/driver/driver.cpp

Index: clang/tools/driver/driver.cpp
===
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -213,6 +213,9 @@
 extern int cc1gen_reproducer_main(ArrayRef Argv,
   const char *Argv0, void *MainAddr,
   const llvm::ToolContext &);
+#if LLVM_ON_UNIX
+extern int cc1modbuildd_main(ArrayRef Argv);
+#endif
 
 static void insertTargetAndModeArgs(const ParsedClangName &NameParts,
 SmallVectorImpl &ArgVector,
@@ -369,9 +372,20 @@
   if (Tool == "-cc1gen-reproducer")
 return cc1gen_reproducer_main(ArrayRef(ArgV).slice(2), ArgV[0],
   GetExecutablePathVP, ToolContext);
-  // Reject unknown tools.
+#if LLVM_ON_UNIX
+  if (Tool == "-cc1modbuildd")
+return cc1modbuildd_main(ArrayRef(ArgV).slice(2));
+
+  // FIXME: Once cc1modbuildd becomes portable unify llvm::errs messages
   llvm::errs() << "error: unknown integrated tool '" << Tool << "'. "
-   << "Valid tools include '-cc1' and '-cc1as'.\n";
+   << "Valid tools include '-cc1', '-cc1as', '-cc1gen-reproducer', "
+   << "and '-cc1modbuildd'.\n";
+#else
+  // Reject unknown tools
+  llvm::errs()
+  << "error: unknown integrated tool '" << Tool << "'. "
+  << "Valid tools include '-cc1', '-cc1as', and '-cc1gen-reproducer'.\n";
+#endif
   return 1;
 }
 
Index: clang/tools/driver/cc1modbuildd_main.cpp
===
--- /dev/null
+++ clang/tools/driver/cc1modbuildd_main.cpp
@@ -0,0 +1,265 @@
+//===--- cc1modbuildd_main.cpp - Clang CC1 Module Build Daemon ===//
+//
+// 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/Tooling/ModuleBuildDaemon/Protocol.h"
+#include "clang/Tooling/ModuleBuildDaemon/SocketSupport.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Config/llvm-config.h"
+#include "llvm/Support/ThreadPool.h"
+#include "llvm/Support/Threading.h"
+#include "llvm/Support/YAMLParser.h"
+#include "llvm/Support/YAMLTraits.h"
+
+// TODO: Make portable
+#if LLVM_ON_UNIX
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace llvm;
+using namespace clang;
+
+namespace {
+class ModuleBuildDaemonServer {
+public:
+  SmallString<128> BasePath;
+  SmallString<128> SocketPath;
+  SmallString<128> PidPath;
+
+  ModuleBuildDaemonServer(SmallString<128> Path)
+  : BasePath(Path), SocketPath(Path) {
+llvm::sys::path::append(SocketPath, SOCKET_FILE_NAME);
+  }
+
+  ~ModuleBuildDaemonServer() { Shutdown(SIGTERM); }
+
+  int Fork();
+  int Launch();
+  int Listen();
+  static llvm::Error Service(int Client);
+
+  void Shutdown(int signal) {
+unlink(SocketPath.c_str());
+shutdown(ListenSocketFD, SHUT_RD);
+close(ListenSocketFD);
+exit(EXIT_SUCCESS);
+  }
+
+private:
+  pid_t Pid = -1;
+  int ListenSocketFD = -1;
+};
+
+// Required to handle SIGTERM by calling Shutdown
+ModuleBuildDaemonServer *DaemonPtr = nullptr;
+void HandleSignal(int signal) {
+  if (DaemonPtr != nullptr) {
+DaemonPtr->Shutdown(signal);
+  }
+}
+} // namespace
+
+// Forks and detaches process, creating module build daemon
+int Mod

[PATCH] D158487: [PowerPC][altivec] Optimize codegen of vec_promote

[Kai Luo via Phabricator via cfe-commits]

lkail added inline comments.



Comment at: clang/lib/Headers/altivec.h:14662
+  vector unsigned char __res =
+  __builtin_shufflevector(__zero, __zero, -1, -1, -1, -1, -1, -1, -1, -1,
+  -1, -1, -1, -1, -1, -1, -1, -1);

qiucf wrote:
> Could we just define it without initialization? This can also make undefined 
> vector.
Using `__builtin_shufflevector` generates poison values, which is stronger than 
`undef`, exposing more optimizations in my view. See 
https://llvm.org/docs/LangRef.html#id1781.



Comment at: llvm/test/CodeGen/PowerPC/vec-promote.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py 
UTC_ARGS: --version 2
+; RUN: llc -mtriple=powerpc64-unknown-unknown -verify-machineinstrs -mcpu=pwr8 
\

qiucf wrote:
> We don't need this file because (1) no backend codegen changed; (2) further 
> changes to `altivec.h` will not change this file.
I prefer keeping this file
1. Backend lacks vec_promote equivalent tests.
2. to show different codegen of FE exposes different optimization opportunities 
to backend which is the core value of this patch. Codegen quality should be 
finally reflected by assembly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158487

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

[PATCH] D158487: [PowerPC][altivec] Optimize codegen of vec_promote

[Kai Luo via Phabricator via cfe-commits]

lkail added inline comments.



Comment at: clang/lib/Headers/altivec.h:14662
+  vector unsigned char __res =
+  __builtin_shufflevector(__zero, __zero, -1, -1, -1, -1, -1, -1, -1, -1,
+  -1, -1, -1, -1, -1, -1, -1, -1);

lkail wrote:
> qiucf wrote:
> > Could we just define it without initialization? This can also make 
> > undefined vector.
> Using `__builtin_shufflevector` generates poison values, which is stronger 
> than `undef`, exposing more optimizations in my view. See 
> https://llvm.org/docs/LangRef.html#id1781.
Also using `__builtin_shufflevector` is explicit, which has a formal 
specification 
https://clang.llvm.org/docs/LanguageExtensions.html#langext-builtin-shufflevector.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158487

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

[PATCH] D158239: [clang][ExtractAPI] Add support for namespaces

[Daniel Grumberg via Phabricator via cfe-commits]

dang accepted this revision.
dang 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/D158239/new/

https://reviews.llvm.org/D158239

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

[PATCH] D158474: [clang][ExtractAPI] Fix bool spelling coming from the macro definition.

[Daniel Grumberg via Phabricator via cfe-commits]

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

Nice!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158474

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

[PATCH] D158426: [clangd] Bump timeouts for LSPServerTests

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/unittests/LSPClient.cpp:27
   std::unique_lock Lock(Mu);
-  if (!clangd::wait(Lock, CV, timeoutSeconds(10),
 [this] { return Value.has_value(); })) {

I think we're better of just increasing the number, unless there's some reason 
to think that different timeouts are appropriate for different tests.



Comment at: clang-tools-extra/clangd/unittests/LSPClient.cpp:218
+void LSPClient::sync() {
+  // Sync should already be implemented with a timeout, so don't timeout.
+  call("sync", nullptr).takeValue(0);

Unless this is actually broken, it doesn't seem worth adding complexity for, 
though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158426

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

[clang] 23459f1 - [Lex] Preambles should contain the global module fragment.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2023-08-22T11:55:51+02:00
New Revision: 23459f13fcd9c424a41debb2d11eb56843d4e679

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

LOG: [Lex] Preambles should contain the global module fragment.

For applications like clangd, the preamble remains an important optimization
when editing a module definition. The global module fragment is a good fit for
it as it by definition contains only preprocessor directives.
Before this patch, we would terminate the preamble immediately at the "module"
keyword.

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

Added: 


Modified: 
clang-tools-extra/clangd/unittests/ModulesTests.cpp
clang/lib/Lex/Lexer.cpp
clang/unittests/Lex/CMakeLists.txt
clang/unittests/Lex/LexerTest.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/ModulesTests.cpp 
b/clang-tools-extra/clangd/unittests/ModulesTests.cpp
index 5f2fc5d6b8c4bc..1a73f84ff63cea 100644
--- a/clang-tools-extra/clangd/unittests/ModulesTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ModulesTests.cpp
@@ -8,6 +8,7 @@
 
 #include "TestFS.h"
 #include "TestTU.h"
+#include "llvm/ADT/StringRef.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -109,6 +110,34 @@ TEST(Modules, UnknownFormat) {
   // Test that we do not crash.
   TU.build();
 }
+
+// Test that we can build and use a preamble for a module unit.
+TEST(Modules, ModulePreamble) {
+  TestTU TU = TestTU::withCode(R"cpp(
+module;
+#define PREAMBLE_MACRO 1
+export module foo;
+#define MODULE_MACRO 2
+module :private;
+#define PRIVATE_MACRO 3
+  )cpp");
+  TU.ExtraArgs.push_back("-std=c++20");
+  TU.ExtraArgs.push_back("--precompile");
+
+  auto AST = TU.build();
+  auto &SM = AST.getSourceManager();
+  auto GetMacroFile = [&](llvm::StringRef Name) -> FileID {
+if (auto *MI = AST.getPreprocessor().getMacroInfo(
+&AST.getASTContext().Idents.get(Name)))
+  return SM.getFileID(MI->getDefinitionLoc());
+return {};
+  };
+
+  EXPECT_EQ(GetMacroFile("PREAMBLE_MACRO"), SM.getPreambleFileID());
+  EXPECT_EQ(GetMacroFile("MODULE_MACRO"), SM.getMainFileID());
+  EXPECT_EQ(GetMacroFile("PRIVATE_MACRO"), SM.getMainFileID());
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang

diff  --git a/clang/lib/Lex/Lexer.cpp b/clang/lib/Lex/Lexer.cpp
index f3c0e80651e762..f2b881833f4bcb 100644
--- a/clang/lib/Lex/Lexer.cpp
+++ b/clang/lib/Lex/Lexer.cpp
@@ -705,6 +705,22 @@ PreambleBounds Lexer::ComputePreamble(StringRef Buffer,
   // directive or it was one that can't occur in the preamble at this
   // point. Roll back the current token to the location of the '#'.
   TheTok = HashTok;
+} else if (TheTok.isAtStartOfLine() &&
+   TheTok.getKind() == tok::raw_identifier &&
+   TheTok.getRawIdentifier() == "module" &&
+   LangOpts.CPlusPlusModules) {
+  // The initial global module fragment introducer "module;" is part of
+  // the preamble, which runs up to the module declaration "module foo;".
+  Token ModuleTok = TheTok;
+  do {
+TheLexer.LexFromRawLexer(TheTok);
+  } while (TheTok.getKind() == tok::comment);
+  if (TheTok.getKind() != tok::semi) {
+// Not global module fragment, roll back.
+TheTok = ModuleTok;
+break;
+  }
+  continue;
 }
 
 // We hit a token that we don't recognize as being in the

diff  --git a/clang/unittests/Lex/CMakeLists.txt 
b/clang/unittests/Lex/CMakeLists.txt
index aead76c0a57b10..f7fc0eed06bec5 100644
--- a/clang/unittests/Lex/CMakeLists.txt
+++ b/clang/unittests/Lex/CMakeLists.txt
@@ -25,5 +25,6 @@ clang_target_link_libraries(LexTests
 
 target_link_libraries(LexTests
   PRIVATE
+  LLVMTestingAnnotations
   LLVMTestingSupport
   )

diff  --git a/clang/unittests/Lex/LexerTest.cpp 
b/clang/unittests/Lex/LexerTest.cpp
index 44ae368372b193..8932265674a59a 100644
--- a/clang/unittests/Lex/LexerTest.cpp
+++ b/clang/unittests/Lex/LexerTest.cpp
@@ -26,9 +26,11 @@
 #include "clang/Lex/PreprocessorOptions.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Testing/Annotations/Annotations.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 #include 
 
 namespace {
@@ -660,4 +662,36 @@ TEST_F(LexerTest, RawAndNormalLexSameForLineComments) {
   }
   EXPECT_TRUE(ToksView.empty());
 }
+
+TEST(LexerPreambleTest, PreambleBounds) {
+  std::vector Cases = {
+  R"cc([[
+#include 
+]]int bar;
+  )cc",
+  R"cc([[
+#include 
+  ]])cc",
+  R"cc([[
+// leading comment
+#include 
+]]// trailing comment
+int bar;
+  )cc",
+  R"cc([[
+module;
+#include 
+  

[PATCH] D158439: [Lex] Preambles should contain the global module fragment.

[Sam McCall 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 rG23459f13fcd9: [Lex] Preambles should contain the global 
module fragment. (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158439

Files:
  clang-tools-extra/clangd/unittests/ModulesTests.cpp
  clang/lib/Lex/Lexer.cpp
  clang/unittests/Lex/CMakeLists.txt
  clang/unittests/Lex/LexerTest.cpp

Index: clang/unittests/Lex/LexerTest.cpp
===
--- clang/unittests/Lex/LexerTest.cpp
+++ clang/unittests/Lex/LexerTest.cpp
@@ -26,9 +26,11 @@
 #include "clang/Lex/PreprocessorOptions.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Testing/Annotations/Annotations.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 #include 
 
 namespace {
@@ -660,4 +662,36 @@
   }
   EXPECT_TRUE(ToksView.empty());
 }
+
+TEST(LexerPreambleTest, PreambleBounds) {
+  std::vector Cases = {
+  R"cc([[
+#include 
+]]int bar;
+  )cc",
+  R"cc([[
+#include 
+  ]])cc",
+  R"cc([[
+// leading comment
+#include 
+]]// trailing comment
+int bar;
+  )cc",
+  R"cc([[
+module;
+#include 
+]]module bar;
+int x;
+  )cc",
+  };
+  for (const auto& Case : Cases) {
+llvm::Annotations A(Case);
+clang::LangOptions LangOpts;
+LangOpts.CPlusPlusModules = true;
+auto Bounds = Lexer::ComputePreamble(A.code(), LangOpts);
+EXPECT_EQ(Bounds.Size, A.range().End) << Case;
+  }
+}
+
 } // anonymous namespace
Index: clang/unittests/Lex/CMakeLists.txt
===
--- clang/unittests/Lex/CMakeLists.txt
+++ clang/unittests/Lex/CMakeLists.txt
@@ -25,5 +25,6 @@
 
 target_link_libraries(LexTests
   PRIVATE
+  LLVMTestingAnnotations
   LLVMTestingSupport
   )
Index: clang/lib/Lex/Lexer.cpp
===
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -705,6 +705,22 @@
   // directive or it was one that can't occur in the preamble at this
   // point. Roll back the current token to the location of the '#'.
   TheTok = HashTok;
+} else if (TheTok.isAtStartOfLine() &&
+   TheTok.getKind() == tok::raw_identifier &&
+   TheTok.getRawIdentifier() == "module" &&
+   LangOpts.CPlusPlusModules) {
+  // The initial global module fragment introducer "module;" is part of
+  // the preamble, which runs up to the module declaration "module foo;".
+  Token ModuleTok = TheTok;
+  do {
+TheLexer.LexFromRawLexer(TheTok);
+  } while (TheTok.getKind() == tok::comment);
+  if (TheTok.getKind() != tok::semi) {
+// Not global module fragment, roll back.
+TheTok = ModuleTok;
+break;
+  }
+  continue;
 }
 
 // We hit a token that we don't recognize as being in the
Index: clang-tools-extra/clangd/unittests/ModulesTests.cpp
===
--- clang-tools-extra/clangd/unittests/ModulesTests.cpp
+++ clang-tools-extra/clangd/unittests/ModulesTests.cpp
@@ -8,6 +8,7 @@
 
 #include "TestFS.h"
 #include "TestTU.h"
+#include "llvm/ADT/StringRef.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -109,6 +110,34 @@
   // Test that we do not crash.
   TU.build();
 }
+
+// Test that we can build and use a preamble for a module unit.
+TEST(Modules, ModulePreamble) {
+  TestTU TU = TestTU::withCode(R"cpp(
+module;
+#define PREAMBLE_MACRO 1
+export module foo;
+#define MODULE_MACRO 2
+module :private;
+#define PRIVATE_MACRO 3
+  )cpp");
+  TU.ExtraArgs.push_back("-std=c++20");
+  TU.ExtraArgs.push_back("--precompile");
+
+  auto AST = TU.build();
+  auto &SM = AST.getSourceManager();
+  auto GetMacroFile = [&](llvm::StringRef Name) -> FileID {
+if (auto *MI = AST.getPreprocessor().getMacroInfo(
+&AST.getASTContext().Idents.get(Name)))
+  return SM.getFileID(MI->getDefinitionLoc());
+return {};
+  };
+
+  EXPECT_EQ(GetMacroFile("PREAMBLE_MACRO"), SM.getPreambleFileID());
+  EXPECT_EQ(GetMacroFile("MODULE_MACRO"), SM.getMainFileID());
+  EXPECT_EQ(GetMacroFile("PRIVATE_MACRO"), SM.getMainFileID());
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 31a8f84 - [clangd] Fix crash with sanitizers if blocking with no timeout

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2023-08-22T11:57:42+02:00
New Revision: 31a8f840f327694a6c74dac15702eeb85281c1e2

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

LOG: [clangd] Fix crash with sanitizers if blocking with no timeout

Added: 


Modified: 
clang-tools-extra/clangd/ClangdServer.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index d44d1e272b9b77..6610961b2202d1 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -1117,7 +1117,8 @@ ClangdServer::blockUntilIdleForTest(std::optional 
TimeoutSeconds) {
 #if defined(__has_feature) &&  
\
 (__has_feature(address_sanitizer) || __has_feature(hwaddress_sanitizer) || 
\
  __has_feature(memory_sanitizer) || __has_feature(thread_sanitizer))
-  (*TimeoutSeconds) *= 10;
+  if (TimeoutSeconds.has_value())
+(*TimeoutSeconds) *= 10;
 #endif
 
   // Nothing else can schedule work on TUScheduler, because it's not threadsafe



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

[PATCH] D158426: [clangd] Bump timeouts for LSPServerTests

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 552284.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Just bump the timeouts


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158426

Files:
  clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
  clang-tools-extra/clangd/unittests/LSPClient.cpp
  clang-tools-extra/clangd/unittests/LSPClient.h

Index: clang-tools-extra/clangd/unittests/LSPClient.h
===
--- clang-tools-extra/clangd/unittests/LSPClient.h
+++ clang-tools-extra/clangd/unittests/LSPClient.h
@@ -9,12 +9,14 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_LSPCLIENT_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_LSPCLIENT_H
 
+#include "llvm/ADT/StringRef.h"
+#include 
 #include 
 #include 
-#include 
-#include 
+#include 
 #include 
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -32,7 +34,8 @@
   class CallResult {
   public:
 ~CallResult();
-// Blocks up to 10 seconds for the result to be ready.
+// Blocks up to \p TimeoutSeconds seconds for the result to be ready. If \p
+// TimeoutSeconds is zero, blocks indefinitely.
 // Records a test failure if there was no reply.
 llvm::Expected take();
 // Like take(), but records a test failure if the result was an error.
Index: clang-tools-extra/clangd/unittests/LSPClient.cpp
===
--- clang-tools-extra/clangd/unittests/LSPClient.cpp
+++ clang-tools-extra/clangd/unittests/LSPClient.cpp
@@ -12,19 +12,32 @@
 #include "Transport.h"
 #include "support/Logger.h"
 #include "support/Threading.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include "gtest/gtest.h"
 #include 
+#include 
+#include 
+#include 
+#include 
+#include 
 #include 
 #include 
+#include 
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
 
 llvm::Expected clang::clangd::LSPClient::CallResult::take() {
   std::unique_lock Lock(Mu);
-  if (!clangd::wait(Lock, CV, timeoutSeconds(10),
+  if (!clangd::wait(Lock, CV, timeoutSeconds(60),
 [this] { return Value.has_value(); })) {
 ADD_FAILURE() << "No result from call after 10 seconds!";
 return llvm::json::Value(nullptr);
Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -8,19 +8,38 @@
 
 #include "Annotations.h"
 #include "ClangdLSPServer.h"
+#include "ClangdServer.h"
+#include "ConfigProvider.h"
+#include "Diagnostics.h"
+#include "FeatureModule.h"
+#include "LSPBinder.h"
 #include "LSPClient.h"
-#include "Protocol.h"
 #include "TestFS.h"
+#include "support/Function.h"
 #include "support/Logger.h"
 #include "support/TestTracer.h"
+#include "support/Threading.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/FunctionExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/JSON.h"
+#include "llvm/Support/raw_ostream.h"
 #include "llvm/Testing/Support/Error.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
 #include 
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -358,7 +377,7 @@
   Client.notify("increment", nullptr);
   Client.notify("increment", nullptr);
   Client.notify("increment", nullptr);
-  EXPECT_THAT_EXPECTED(Client.call("sync", nullptr).take(), Succeeded());
+  Client.sync();
   EXPECT_EQ(3, FeatureModules.get()->getSync());
   // Throw some work on the queue to make sure shutdown blocks on it.
   Client.notify("increment", nullptr);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Arkadiy Yudintsev via Phabricator via cfe-commits]

d0nc1h0t updated this revision to Diff 552281.
d0nc1h0t added a comment.

Added unittest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4223,6 +4223,19 @@
"void foo() {}\n"
"} // namespace ns\n",
Style);
+
+  FormatStyle LLVMWithCompactInnerNamespace = getLLVMStyle();
+  LLVMWithCompactInnerNamespace.CompactNamespaces = true;
+  LLVMWithCompactInnerNamespace.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyNoCrash("namespace ns1 { namespace ns2 { namespace ns3 {\n"
+"void func_in_ns() {\n"
+"  int res{0};\n"
+"// block for debug mode\n"
+"#ifndef NDEBUG\n"
+"#endif\n"
+"}\n"
+"}}}\n",
+LLVMWithCompactInnerNamespace);
 }
 
 TEST_F(FormatTest, NamespaceMacros) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -390,7 +390,7 @@
   for (auto *CompactedLine = I + J; CompactedLine <= ClosingLine;
++CompactedLine) {
 if (!(*CompactedLine)->InPPDirective)
-  (*CompactedLine)->Level -= OutdentBy;
+  (*CompactedLine)->Level -= std::min(OutdentBy, 
(*CompactedLine)->Level);
   }
 }
 return J - 1;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4223,6 +4223,19 @@
"void foo() {}\n"
"} // namespace ns\n",
Style);
+
+  FormatStyle LLVMWithCompactInnerNamespace = getLLVMStyle();
+  LLVMWithCompactInnerNamespace.CompactNamespaces = true;
+  LLVMWithCompactInnerNamespace.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyNoCrash("namespace ns1 { namespace ns2 { namespace ns3 {\n"
+"void func_in_ns() {\n"
+"  int res{0};\n"
+"// block for debug mode\n"
+"#ifndef NDEBUG\n"
+"#endif\n"
+"}\n"
+"}}}\n",
+LLVMWithCompactInnerNamespace);
 }
 
 TEST_F(FormatTest, NamespaceMacros) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -390,7 +390,7 @@
   for (auto *CompactedLine = I + J; CompactedLine <= ClosingLine;
++CompactedLine) {
 if (!(*CompactedLine)->InPPDirective)
-  (*CompactedLine)->Level -= OutdentBy;
+  (*CompactedLine)->Level -= std::min(OutdentBy, (*CompactedLine)->Level);
   }
 }
 return J - 1;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Arkadiy Yudintsev via Phabricator via cfe-commits]

d0nc1h0t added a comment.

In D158363#4604468 , 
@HazardyKnusperkeks wrote:

> Please upload the patch with the full context.

I'm creating a patch via 'git diff' from the root of the project. What does 
full context mean?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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

[PATCH] D158499: [analyzer] Compute FAM dynamic size

[Ding Fei via Phabricator via cfe-commits]

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

This commit computes size of FAM which is normally allocated dynamically.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158499

Files:
  clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp
  clang/test/Analysis/flexible-array-members.c

Index: clang/test/Analysis/flexible-array-members.c
===
--- clang/test/Analysis/flexible-array-members.c
+++ clang/test/Analysis/flexible-array-members.c
@@ -44,20 +44,52 @@
   clang_analyzer_dump(clang_analyzer_getExtent(&fam));
   clang_analyzer_dump(clang_analyzer_getExtent(fam.data));
   // expected-warning@-2 {{4 S64b}}
-  // expected-warning@-2 {{Unknown}}
+  // expected-warning@-2 {{0 U64b}}
 
   FAM *p = (FAM *)alloca(sizeof(FAM));
   clang_analyzer_dump(clang_analyzer_getExtent(p));
   clang_analyzer_dump(clang_analyzer_getExtent(p->data));
   // expected-warning@-2 {{4 U64b}}
-  // expected-warning@-2 {{Unknown}}
+  // expected-warning@-2 {{0 U64b}}
 
   FAM *q = (FAM *)malloc(sizeof(FAM));
   clang_analyzer_dump(clang_analyzer_getExtent(q));
   clang_analyzer_dump(clang_analyzer_getExtent(q->data));
   // expected-warning@-2 {{4 U64b}}
-  // expected-warning@-2 {{Unknown}}
+  // expected-warning@-2 {{0 U64b}}
   free(q);
+
+  q = (FAM *)malloc(sizeof(FAM) + sizeof(int) * 2);
+  clang_analyzer_dump(clang_analyzer_getExtent(q));
+  clang_analyzer_dump(clang_analyzer_getExtent(q->data));
+  // expected-warning@-2 {{12 U64b}}
+  // expected-warning@-2 {{8 U64b}}
+  free(q);
+
+  typedef struct __attribute__((packed)) {
+char c;
+int data[];
+  } PackedFAM;
+
+  PackedFAM *t = (PackedFAM *)malloc(sizeof(PackedFAM) + sizeof(int) * 2);
+  clang_analyzer_dump(clang_analyzer_getExtent(t));
+  clang_analyzer_dump(clang_analyzer_getExtent(t->data));
+  // expected-warning@-2 {{9 U64b}}
+  // expected-warning@-2 {{8 U64b}}
+  free(t);
+}
+
+void test_too_small_base(void) {
+  typedef struct FAM {
+long c;
+int data[0];
+  } FAM;
+  short s = 0;
+  FAM *p = (FAM *) &s;
+  clang_analyzer_dump(clang_analyzer_getExtent(p));
+  clang_analyzer_dump(clang_analyzer_getExtent(p->data));
+  // expected-warning@-2 {{2 S64b}}
+  // expected-warning@-2 {{Unknown}}
 }
 
 void test_zero_length_array_fam(void) {
Index: clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp
===
--- clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp
+++ clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp
@@ -25,6 +25,51 @@
 namespace clang {
 namespace ento {
 
+DefinedOrUnknownSVal getDynamicExtent(ProgramStateRef State,
+  const MemRegion *MR, SValBuilder &SVB);
+
+static bool isFlexibleArrayMember(const FieldDecl *FD) {
+  const auto *RD = FD->getParent();
+  if (!RD->hasFlexibleArrayMember())
+return false;
+
+  const FieldDecl *LastFD = nullptr;
+  for (const FieldDecl *It : RD->fields())
+LastFD = It;
+
+  return FD == LastFD;
+}
+
+static std::optional
+getFlexibleArrayExtent(ProgramStateRef State, const MemRegion *MR,
+   SValBuilder &SVB) {
+  const auto *FieldMR = dyn_cast(MR);
+  if (nullptr == FieldMR)
+return std::nullopt;
+
+  const FieldDecl *FD = FieldMR->getDecl();
+  if (!isFlexibleArrayMember(FD))
+return std::nullopt;
+
+  const RecordDecl *RD = FD->getParent();
+  const MemRegion *BaseMR = FieldMR->getBaseRegion();
+  auto BaseSize = getDynamicExtent(State, BaseMR, SVB);
+
+  auto &C = SVB.getContext();
+  uint64_t RecordSize = C.getTypeSizeInChars(C.getRecordType(RD)).getQuantity();
+  SVal RecordSizeVal = SVB.makeIntVal(RecordSize, C.getSizeType());
+
+  SVal BaseTooSmall =
+  SVB.evalBinOp(State, BO_LT, BaseSize, RecordSizeVal, C.BoolTy);
+  if (!BaseTooSmall.isUndef() &&
+  State->assume(*BaseTooSmall.getAs(), true))
+return std::nullopt;
+
+  SVal FlexibleArrayExtent =
+  SVB.evalBinOp(State, BO_Sub, BaseSize, RecordSizeVal, C.getSizeType());
+  return FlexibleArrayExtent.getAs();
+}
+
 DefinedOrUnknownSVal getDynamicExtent(ProgramStateRef State,
   const MemRegion *MR, SValBuilder &SVB) {
   MR = MR->StripCasts();
@@ -32,6 +77,9 @@
   if (const DefinedOrUnknownSVal *Size = State->get(MR))
 return *Size;
 
+  if (auto FlexibleArrayExtent = getFlexibleArrayExtent(State, MR, SVB))
+return *FlexibleArrayExtent;
+
   return MR->getMemRegionManager().getStaticSize(MR, SVB);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158499: [analyzer] Compute FAM dynamic size

[Ding Fei via Phabricator via cfe-commits]

danix800 updated this revision to Diff 552288.
danix800 added a comment.

Update testcase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158499

Files:
  clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp
  clang/test/Analysis/flexible-array-members.c

Index: clang/test/Analysis/flexible-array-members.c
===
--- clang/test/Analysis/flexible-array-members.c
+++ clang/test/Analysis/flexible-array-members.c
@@ -44,20 +44,52 @@
   clang_analyzer_dump(clang_analyzer_getExtent(&fam));
   clang_analyzer_dump(clang_analyzer_getExtent(fam.data));
   // expected-warning@-2 {{4 S64b}}
-  // expected-warning@-2 {{Unknown}}
+  // expected-warning@-2 {{0 U64b}}
 
   FAM *p = (FAM *)alloca(sizeof(FAM));
   clang_analyzer_dump(clang_analyzer_getExtent(p));
   clang_analyzer_dump(clang_analyzer_getExtent(p->data));
   // expected-warning@-2 {{4 U64b}}
-  // expected-warning@-2 {{Unknown}}
+  // expected-warning@-2 {{0 U64b}}
 
   FAM *q = (FAM *)malloc(sizeof(FAM));
   clang_analyzer_dump(clang_analyzer_getExtent(q));
   clang_analyzer_dump(clang_analyzer_getExtent(q->data));
   // expected-warning@-2 {{4 U64b}}
-  // expected-warning@-2 {{Unknown}}
+  // expected-warning@-2 {{0 U64b}}
   free(q);
+
+  q = (FAM *)malloc(sizeof(FAM) + sizeof(int) * 2);
+  clang_analyzer_dump(clang_analyzer_getExtent(q));
+  clang_analyzer_dump(clang_analyzer_getExtent(q->data));
+  // expected-warning@-2 {{12 U64b}}
+  // expected-warning@-2 {{8 U64b}}
+  free(q);
+
+  typedef struct __attribute__((packed)) {
+char c;
+int data[];
+  } PackedFAM;
+
+  PackedFAM *t = (PackedFAM *)malloc(sizeof(PackedFAM) + sizeof(int) * 2);
+  clang_analyzer_dump(clang_analyzer_getExtent(t));
+  clang_analyzer_dump(clang_analyzer_getExtent(t->data));
+  // expected-warning@-2 {{9 U64b}}
+  // expected-warning@-2 {{8 U64b}}
+  free(t);
+}
+
+void test_too_small_base(void) {
+  typedef struct FAM {
+long c;
+int data[];
+  } FAM;
+  short s = 0;
+  FAM *p = (FAM *) &s;
+  clang_analyzer_dump(clang_analyzer_getExtent(p));
+  clang_analyzer_dump(clang_analyzer_getExtent(p->data));
+  // expected-warning@-2 {{2 S64b}}
+  // expected-warning@-2 {{Unknown}}
 }
 
 void test_zero_length_array_fam(void) {
Index: clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp
===
--- clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp
+++ clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp
@@ -25,6 +25,51 @@
 namespace clang {
 namespace ento {
 
+DefinedOrUnknownSVal getDynamicExtent(ProgramStateRef State,
+  const MemRegion *MR, SValBuilder &SVB);
+
+static bool isFlexibleArrayMember(const FieldDecl *FD) {
+  const auto *RD = FD->getParent();
+  if (!RD->hasFlexibleArrayMember())
+return false;
+
+  const FieldDecl *LastFD = nullptr;
+  for (const FieldDecl *It : RD->fields())
+LastFD = It;
+
+  return FD == LastFD;
+}
+
+static std::optional
+getFlexibleArrayExtent(ProgramStateRef State, const MemRegion *MR,
+   SValBuilder &SVB) {
+  const auto *FieldMR = dyn_cast(MR);
+  if (nullptr == FieldMR)
+return std::nullopt;
+
+  const FieldDecl *FD = FieldMR->getDecl();
+  if (!isFlexibleArrayMember(FD))
+return std::nullopt;
+
+  const RecordDecl *RD = FD->getParent();
+  const MemRegion *BaseMR = FieldMR->getBaseRegion();
+  auto BaseSize = getDynamicExtent(State, BaseMR, SVB);
+
+  auto &C = SVB.getContext();
+  uint64_t RecordSize = C.getTypeSizeInChars(C.getRecordType(RD)).getQuantity();
+  SVal RecordSizeVal = SVB.makeIntVal(RecordSize, C.getSizeType());
+
+  SVal BaseTooSmall =
+  SVB.evalBinOp(State, BO_LT, BaseSize, RecordSizeVal, C.BoolTy);
+  if (!BaseTooSmall.isUndef() &&
+  State->assume(*BaseTooSmall.getAs(), true))
+return std::nullopt;
+
+  SVal FlexibleArrayExtent =
+  SVB.evalBinOp(State, BO_Sub, BaseSize, RecordSizeVal, C.getSizeType());
+  return FlexibleArrayExtent.getAs();
+}
+
 DefinedOrUnknownSVal getDynamicExtent(ProgramStateRef State,
   const MemRegion *MR, SValBuilder &SVB) {
   MR = MR->StripCasts();
@@ -32,6 +77,9 @@
   if (const DefinedOrUnknownSVal *Size = State->get(MR))
 return *Size;
 
+  if (auto FlexibleArrayExtent = getFlexibleArrayExtent(State, MR, SVB))
+return *FlexibleArrayExtent;
+
   return MR->getMemRegionManager().getStaticSize(MR, SVB);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] acc5db2 - [Thumb1] Do not allow Armv6-m XO and PI code

[Keith Walker via cfe-commits]

Author: Keith Walker
Date: 2023-08-22T11:08:11+01:00
New Revision: acc5db2bedd50a66f156b63be8469271f7b19322

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

LOG: [Thumb1] Do not allow Armv6-m XO and PI code

When generating armv6-m (Thumb1) Position Independent (PI) code
there are currently some code sequences that are not compatible
with eXecute-Only (XO) code.

For example, this simple code sequence when compiler for XO & PI:

extern int x;
int fn() { return x; }
is a problem as the address of x is currently loaded by:

  ldr r0, .L0
:
:
.L0:
  .long   x

which is not XO compiant as this involves reading the value at
.L0 which is in the code section. Generating correct code is
currently hindered by lack of suitable relocations.

Disallow the generation of armv6-m PI code together with XO code
until they can be made to work together.

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

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticCommonKinds.td
clang/lib/Driver/ToolChains/Arch/ARM.cpp
clang/test/Driver/arm-execute-only.c

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td 
b/clang/include/clang/Basic/DiagnosticCommonKinds.td
index 195a61127c2a67..cd72e254ea3b1a 100644
--- a/clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ b/clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -338,6 +338,8 @@ def err_target_unsupported_mcmse : Error<
   "-mcmse is not supported for %0">;
 def err_opt_not_valid_with_opt : Error<
   "option '%0' cannot be specified with '%1'">;
+def err_opt_not_valid_with_opt_on_target : Error<
+  "option '%0' cannot be specified with '%1' for the %2 sub-architecture">;
 def err_opt_not_valid_without_opt : Error<
   "option '%0' cannot be specified without '%1'">;
 def err_opt_not_valid_on_target : Error<

diff  --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp 
b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index dad1e9e5bd3304..bb66db5feae8c3 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -847,7 +847,13 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver 
&D,
 llvm::ARM::parseArch(Triple.getArchName()) != 
llvm::ARM::ArchKind::ARMV6T2 &&
 llvm::ARM::parseArch(Triple.getArchName()) != 
llvm::ARM::ArchKind::ARMV6M)
   D.Diag(diag::err_target_unsupported_execute_only) << 
Triple.getArchName();
-else if (Arg *B = Args.getLastArg(options::OPT_mno_movt))
+else if (llvm::ARM::parseArch(Triple.getArchName()) == 
llvm::ARM::ArchKind::ARMV6M) {
+  if (Arg *PIArg = Args.getLastArg(options::OPT_fropi, 
options::OPT_frwpi,
+   options::OPT_fpic, 
options::OPT_fpie,
+   options::OPT_fPIC, 
options::OPT_fPIE))
+D.Diag(diag::err_opt_not_valid_with_opt_on_target)
+<< A->getAsString(Args) << PIArg->getAsString(Args) << 
Triple.getArchName();
+} else if (Arg *B = Args.getLastArg(options::OPT_mno_movt))
   D.Diag(diag::err_opt_not_valid_with_opt)
   << A->getAsString(Args) << B->getAsString(Args);
 Features.push_back("+execute-only");

diff  --git a/clang/test/Driver/arm-execute-only.c 
b/clang/test/Driver/arm-execute-only.c
index 1b2fad7299484a..a9bf1656fd27e5 100644
--- a/clang/test/Driver/arm-execute-only.c
+++ b/clang/test/Driver/arm-execute-only.c
@@ -20,3 +20,27 @@
 // RUN: not %clang -### --target=arm-arm-none-eabi -march=armv8-m.main 
-mpure-code -mno-movt %s 2>&1 \
 // RUN:| FileCheck %s -check-prefix CHECK-PURE-CODE-NO-MOVT
 // CHECK-PURE-CODE-NO-MOVT: error: option '-mpure-code' cannot be specified 
with '-mno-movt'
+
+// RUN: not %clang -### --target=arm-arm-none-eabi -march=armv6-m 
-mexecute-only -fropi %s 2>&1 \
+// RUN:| FileCheck %s -check-prefix CHECK-NO-EXECUTE-ROPI
+// CHECK-NO-EXECUTE-ROPI: error: option '-mexecute-only' cannot be specified 
with '-fropi' for the thumbv6m sub-architecture
+
+// RUN: not %clang -### --target=arm-arm-none-eabi -march=armv6-m 
-mexecute-only -frwpi %s 2>&1 \
+// RUN:| FileCheck %s -check-prefix CHECK-NO-EXECUTE-RWPI
+// CHECK-NO-EXECUTE-RWPI: error: option '-mexecute-only' cannot be specified 
with '-frwpi' for the thumbv6m sub-architecture
+
+// RUN: not %clang -### --target=arm-arm-none-eabi -march=armv6-m 
-mexecute-only -fpic %s 2>&1 \
+// RUN:| FileCheck %s -check-prefix CHECK-NO-EXECUTE-PIC
+// CHECK-NO-EXECUTE-PIC: error: option '-mexecute-only' cannot be specified 
with '-fpic' for the thumbv6m sub-architecture
+
+// RUN: not %clang -### --target=arm-arm-none-eabi -march=armv6-m 
-mexecute-only -fpie %s 2>&1 \
+// RUN:| FileCheck %s -check-prefix CHECK-NO-EXECUTE-PIE
+// CHECK-NO-E

[PATCH] D157620: [Thumb1] Do not allow Armv6-m XO and PI code

[Keith Walker 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 rGacc5db2bedd5: [Thumb1] Do not allow Armv6-m XO and PI code 
(authored by keith.walker.arm).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157620

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/Driver/arm-execute-only.c


Index: clang/test/Driver/arm-execute-only.c
===
--- clang/test/Driver/arm-execute-only.c
+++ clang/test/Driver/arm-execute-only.c
@@ -20,3 +20,27 @@
 // RUN: not %clang -### --target=arm-arm-none-eabi -march=armv8-m.main 
-mpure-code -mno-movt %s 2>&1 \
 // RUN:| FileCheck %s -check-prefix CHECK-PURE-CODE-NO-MOVT
 // CHECK-PURE-CODE-NO-MOVT: error: option '-mpure-code' cannot be specified 
with '-mno-movt'
+
+// RUN: not %clang -### --target=arm-arm-none-eabi -march=armv6-m 
-mexecute-only -fropi %s 2>&1 \
+// RUN:| FileCheck %s -check-prefix CHECK-NO-EXECUTE-ROPI
+// CHECK-NO-EXECUTE-ROPI: error: option '-mexecute-only' cannot be specified 
with '-fropi' for the thumbv6m sub-architecture
+
+// RUN: not %clang -### --target=arm-arm-none-eabi -march=armv6-m 
-mexecute-only -frwpi %s 2>&1 \
+// RUN:| FileCheck %s -check-prefix CHECK-NO-EXECUTE-RWPI
+// CHECK-NO-EXECUTE-RWPI: error: option '-mexecute-only' cannot be specified 
with '-frwpi' for the thumbv6m sub-architecture
+
+// RUN: not %clang -### --target=arm-arm-none-eabi -march=armv6-m 
-mexecute-only -fpic %s 2>&1 \
+// RUN:| FileCheck %s -check-prefix CHECK-NO-EXECUTE-PIC
+// CHECK-NO-EXECUTE-PIC: error: option '-mexecute-only' cannot be specified 
with '-fpic' for the thumbv6m sub-architecture
+
+// RUN: not %clang -### --target=arm-arm-none-eabi -march=armv6-m 
-mexecute-only -fpie %s 2>&1 \
+// RUN:| FileCheck %s -check-prefix CHECK-NO-EXECUTE-PIE
+// CHECK-NO-EXECUTE-PIE: error: option '-mexecute-only' cannot be specified 
with '-fpie' for the thumbv6m sub-architecture
+
+// RUN: not %clang -### --target=arm-arm-none-eabi -march=armv6-m 
-mexecute-only -fPIC %s 2>&1 \
+// RUN:| FileCheck %s -check-prefix CHECK-NO-EXECUTE-PIC2
+// CHECK-NO-EXECUTE-PIC2: error: option '-mexecute-only' cannot be specified 
with '-fPIC' for the thumbv6m sub-architecture
+
+// RUN: not %clang -### --target=arm-arm-none-eabi -march=armv6-m 
-mexecute-only -fPIE %s 2>&1 \
+// RUN:| FileCheck %s -check-prefix CHECK-NO-EXECUTE-PIE2
+// CHECK-NO-EXECUTE-PIE2: error: option '-mexecute-only' cannot be specified 
with '-fPIE' for the thumbv6m sub-architecture
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -847,7 +847,13 @@
 llvm::ARM::parseArch(Triple.getArchName()) != 
llvm::ARM::ArchKind::ARMV6T2 &&
 llvm::ARM::parseArch(Triple.getArchName()) != 
llvm::ARM::ArchKind::ARMV6M)
   D.Diag(diag::err_target_unsupported_execute_only) << 
Triple.getArchName();
-else if (Arg *B = Args.getLastArg(options::OPT_mno_movt))
+else if (llvm::ARM::parseArch(Triple.getArchName()) == 
llvm::ARM::ArchKind::ARMV6M) {
+  if (Arg *PIArg = Args.getLastArg(options::OPT_fropi, 
options::OPT_frwpi,
+   options::OPT_fpic, 
options::OPT_fpie,
+   options::OPT_fPIC, 
options::OPT_fPIE))
+D.Diag(diag::err_opt_not_valid_with_opt_on_target)
+<< A->getAsString(Args) << PIArg->getAsString(Args) << 
Triple.getArchName();
+} else if (Arg *B = Args.getLastArg(options::OPT_mno_movt))
   D.Diag(diag::err_opt_not_valid_with_opt)
   << A->getAsString(Args) << B->getAsString(Args);
 Features.push_back("+execute-only");
Index: clang/include/clang/Basic/DiagnosticCommonKinds.td
===
--- clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -338,6 +338,8 @@
   "-mcmse is not supported for %0">;
 def err_opt_not_valid_with_opt : Error<
   "option '%0' cannot be specified with '%1'">;
+def err_opt_not_valid_with_opt_on_target : Error<
+  "option '%0' cannot be specified with '%1' for the %2 sub-architecture">;
 def err_opt_not_valid_without_opt : Error<
   "option '%0' cannot be specified without '%1'">;
 def err_opt_not_valid_on_target : Error<


Index: clang/test/Driver/arm-execute-only.c
===
--- clang/test/Driver/arm-execute-only.c
+++ clang/test/Driver/arm-execute-only.c
@@ -20,3 +20,27 @@
 // RUN: not %c

[PATCH] D158502: [clang][Interp] Actually consider ConstantExpr result

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

tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, shafik, cor3ntin.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

  Since we have visitAPValue now, we might as well use it here.

Not really possible to test this, so no test included.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158502

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp


Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -764,8 +764,11 @@
 
 template 
 bool ByteCodeExprGen::VisitConstantExpr(const ConstantExpr *E) {
-  // TODO: Check if the ConstantExpr already has a value set and if so,
-  //   use that instead of evaluating it again.
+  std::optional T = classify(E->getType());
+  if (T && E->hasAPValueResult() &&
+  this->visitAPValue(E->getAPValueResult(), *T, E))
+return true;
+
   return this->delegate(E->getSubExpr());
 }
 


Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -764,8 +764,11 @@
 
 template 
 bool ByteCodeExprGen::VisitConstantExpr(const ConstantExpr *E) {
-  // TODO: Check if the ConstantExpr already has a value set and if so,
-  //   use that instead of evaluating it again.
+  std::optional T = classify(E->getType());
+  if (T && E->hasAPValueResult() &&
+  this->visitAPValue(E->getAPValueResult(), *T, E))
+return true;
+
   return this->delegate(E->getSubExpr());
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158499: [analyzer] Compute FAM dynamic size

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

steakhal added a comment.

Thanks for submitting this.
A funny thing is that in my free time I was also working on this last week. 
I'll have a look at this more in depth during the week.
For the mean time here is my version, committed pretty much a couple days ago 
to my fork.
https://github.com/llvm/llvm-project/commit/986059a146e036ec84db64868a079d3c4a0e5e16

Your proposed change looks pretty similar to mine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158499

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

[PATCH] D158301: Add back overriding-t-options for -m-version-min diagnostic

[Hans Wennborg via Phabricator via cfe-commits]

hans added inline comments.



Comment at: clang/test/Driver/darwin-version.c:217
 // RUN:   FileCheck --check-prefix=CHECK-VERSION-TNO-OSV1 %s
-// CHECK-VERSION-TNO-OSV1: overriding '-mmacos-version-min=10.6' option with 
'-target x86_64-apple-macos10.11.2'
+// CHECK-VERSION-TNO-OSV1: overriding '-mmacos-version-min=10.6' option with 
'-target x86_64-apple-macos10.11.2' [-Woverriding-t-option]
 

MaskRay wrote:
> hans wrote:
> > dexonsmith wrote:
> > > MaskRay wrote:
> > > > dexonsmith wrote:
> > > > > Why would we want to use the old name here? An alias seems strictly 
> > > > > better to me. 
> > > > Making `overriding-t-option` an alias for `overriding-option` would 
> > > > make `-Wno-overriding-t-option` applies to future overriding option 
> > > > diagnostics, which is exactly what I want to avoid.
> > > > 
> > > I understand that you don't want `-t-` to apply to work on future 
> > > overriding option diagnostics, but I think the platform divergence you're 
> > > adding here is worse.
> > > 
> > > Having a few Darwin-specific options use `-Woverriding-t-option` (and 
> > > everything else use `-Woverriding-option`) as the canonical spelling is 
> > > hard to reason about for maintainers, and for users.
> > > 
> > > And might not users on other platforms have `-Woverriding-t-option` 
> > > hardcoded in  build settings? (So @dblaikie's argument that we shouldn't 
> > > arbitrarily make things hard for users would apply to all options?)
> > > 
> > > IMO, if we're not comfortable removing `-Woverriding-t-option` entirely, 
> > > then it should live on as an alias (easy to reason about), not as 
> > > canonical-in-special-cases (hard to reason about).
> > > IMO, if we're not comfortable removing -Woverriding-t-option entirely, 
> > > then it should live on as an alias (easy to reason about), not as 
> > > canonical-in-special-cases (hard to reason about).
> > 
> > +1 if we can't drop the old spelling, an alias seems like the best option.
> Making `overriding-t-option` an alias for `overriding-option`, as I 
> mentioned, will make `-Wno-overriding-t-option` affect new overriding-options 
> uses. This is exactly what I want to avoid.
> 
> I know there are some `-Wno-overriding-t-option` uses. Honestly, they are far 
> fewer than other diagnostics we are introducing or changing in Clang. I can 
> understand the argument "make -Werror users easier for this specific 
> diagnostic" (but `-Werror` will complain about other new diagnostics), but do 
> we really want to in the Darwin case? I think no. They can remove the version 
> from the target triple like 
> https://github.com/facebook/ocamlrep/blame/abc14b8aafcc6746ec37bf7bf0de24bfc58d63a0/prelude/apple/apple_target_sdk_version.bzl#L50
> 　or make the version part consistent with `-m.*os-version-min`.
> 
> This change may force these users to re-think how they should fix  their 
> build. I apology to these users, but I don't feel that adding an alias is 
> really necessary.
> Making overriding-t-option an alias for overriding-option, as I mentioned, 
> will make -Wno-overriding-t-option affect new overriding-options uses. This 
> is exactly what I want to avoid.

Is keeping them separate actually important, though? -Wno-overriding-option has 
the same issue in that case, that using the flag will also affect any new 
overriding-options uses, and I don't think that's a problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158301

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

[PATCH] D158499: [analyzer] Compute FAM dynamic size

[Ding Fei via Phabricator via cfe-commits]

danix800 added a comment.

In D158499#4606291 , @steakhal wrote:

> Thanks for submitting this.
> A funny thing is that in my free time I was also working on this last week. 
> I'll have a look at this more in depth during the week.
> For the mean time here is my version, committed pretty much a couple days ago 
> to my fork.
> https://github.com/steakhal/llvm-project/commit/986059a146e036ec84db64868a079d3c4a0e5e16
>
> EDIT: fix the link to point to my fork.
>
> Your proposed change looks pretty similar to mine.

Are you going to submit your changes? Which one do you think is more suitable?

Please let me know whether I should continue on this one or wait for yours. 
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158499

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

[PATCH] D148381: [WIP][Clang] Add counted_by attribute

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 552305.
void added a comment.

Fix a segfault.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148381

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test

Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -56,6 +56,7 @@
 // CHECK-NEXT: ConsumableAutoCast (SubjectMatchRule_record)
 // CHECK-NEXT: ConsumableSetOnRead (SubjectMatchRule_record)
 // CHECK-NEXT: Convergent (SubjectMatchRule_function)
+// CHECK-NEXT: CountedBy (SubjectMatchRule_field)
 // CHECK-NEXT: DLLExport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface)
 // CHECK-NEXT: DLLImport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface)
 // CHECK-NEXT: Destructor (SubjectMatchRule_function)
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -8380,6 +8380,21 @@
   D->addAttr(ZeroCallUsedRegsAttr::Create(S.Context, Kind, AL));
 }
 
+static void handleCountedByAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  // TODO: Probably needs more processing here. See Sema::AddAlignValueAttr.
+  if (!AL.isArgIdent(0)) {
+S.Diag(AL.getLoc(), diag::err_attribute_argument_type)
+<< AL << AANT_ArgumentIdentifier;
+return;
+  }
+
+  IdentifierLoc *IL = AL.getArgAsIdent(0);
+  CountedByAttr *CBA = ::new (S.Context) CountedByAttr(S.Context, AL,
+   IL->Ident);
+  CBA->setCountedByFieldLoc(IL->Loc);
+  D->addAttr(CBA);
+}
+
 static void handleFunctionReturnThunksAttr(Sema &S, Decl *D,
const ParsedAttr &AL) {
   StringRef KindStr;
@@ -9326,6 +9341,10 @@
 handleAvailableOnlyInDefaultEvalMethod(S, D, AL);
 break;
 
+  case ParsedAttr::AT_CountedBy:
+handleCountedByAttr(S, D, AL);
+break;
+
   // Microsoft attributes:
   case ParsedAttr::AT_LayoutVersion:
 handleLayoutVersion(S, D, AL);
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -17904,6 +17904,37 @@
  "Broken injected-class-name");
 }
 
+static const FieldDecl *FindFieldWithCountedByAttr(const RecordDecl *RD) {
+  for (const Decl *D : RD->decls()) {
+if (const auto *FD = dyn_cast(D))
+  if (FD->hasAttr())
+return FD;
+
+if (const auto *SubRD = dyn_cast(D))
+  if (const FieldDecl *FD = FindFieldWithCountedByAttr(SubRD))
+return FD;
+  }
+
+  return nullptr;
+}
+
+static const IdentifierInfo *
+CheckCountedByAttr(const RecordDecl *RD, const FieldDecl *FD,
+   SourceRange &Loc) {
+  const CountedByAttr *ECA = FD->getAttr();
+
+  auto It = llvm::find_if(
+  RD->fields(), [&](const FieldDecl *Field){
+return Field->getName() == ECA->getCountedByField()->getName();
+  });
+  if (It == RD->field_end()) {
+Loc = ECA->getCountedByFieldLoc();
+return ECA->getCountedByField();
+  }
+
+  return nullptr;
+}
+
 void Sema::ActOnTagFinishDefinition(Scope *S, Decl *TagD,
 SourceRange BraceRange) {
   AdjustDeclIfTemplate(TagD);
@@ -17961,6 +17992,19 @@
  [](const FieldDecl *FD) { return FD->isBitField(); }))
   Diag(BraceRange.getBegin(), diag::warn_pragma_align_not_xl_compatible);
   }
+
+  // Check the "counted_by" attribute to ensure that the count field exists in
+  // the struct.
+  if (const RecordDecl *RD = dyn_cast(Tag)) {
+if (const FieldDecl *FD = FindFieldWithCountedByAttr(RD)) {
+  SourceRange SR;
+  const IdentifierInfo *Unknown = CheckCountedByAttr(RD, FD, SR);
+
+  if (Unknown)
+Diag(SR.getBegin(), diag::warn_counted_by_placeholder)
+<< Unknown << SR;
+}
+  }
 }
 
 void Sema::ActOnObjCContainerFinishDefinition() {
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -3027,6 +3027,12 @@
   void EmitBounds

[PATCH] D155858: Add a concept AST node.

[Jens Massberg via Phabricator via cfe-commits]

massberg updated this revision to Diff 552307.
massberg added a comment.

Resolve some of the reviewer's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155858

Files:
  clang/include/clang/AST/ASTConcept.h
  clang/include/clang/AST/DeclTemplate.h
  clang/include/clang/AST/ExprConcepts.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/Serialization/ASTRecordReader.h
  clang/include/clang/Serialization/ASTRecordWriter.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/AST/ExprConcepts.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/unittests/Tooling/RecursiveASTVisitorTests/Concept.cpp

Index: clang/unittests/Tooling/RecursiveASTVisitorTests/Concept.cpp
===
--- clang/unittests/Tooling/RecursiveASTVisitorTests/Concept.cpp
+++ clang/unittests/Tooling/RecursiveASTVisitorTests/Concept.cpp
@@ -29,12 +29,22 @@
 ++ConceptRequirementsTraversed;
 return ExpectedLocationVisitor::TraverseConceptRequirement(R);
   }
+  bool TraverseConceptReference(const ConceptReference *CR) {
+++ConceptReferencesTraversed;
+return ExpectedLocationVisitor::TraverseConceptReference(CR);
+  }
+  bool VisitConceptReference(const ConceptReference *CR) {
+++ConceptReferencesVisited;
+return true;
+  }
 
   bool shouldVisitImplicitCode() { return ShouldVisitImplicitCode; }
 
   int ConceptSpecializationExprsVisited = 0;
   int TypeConstraintsTraversed = 0;
   int ConceptRequirementsTraversed = 0;
+  int ConceptReferencesTraversed = 0;
+  int ConceptReferencesVisited = 0;
   bool ShouldVisitImplicitCode = false;
 };
 
@@ -50,6 +60,8 @@
   EXPECT_EQ(1, Visitor.ConceptSpecializationExprsVisited);
   // Also check we traversed the TypeConstraint that produced the expr.
   EXPECT_EQ(1, Visitor.TypeConstraintsTraversed);
+  EXPECT_EQ(1, Visitor.ConceptReferencesTraversed);
+  EXPECT_EQ(1, Visitor.ConceptReferencesVisited);
 
   Visitor = {}; // Don't visit implicit code now.
   EXPECT_TRUE(Visitor.runOver("template  concept Fooable = true;\n"
@@ -59,6 +71,8 @@
   // generated immediately declared expression.
   EXPECT_EQ(0, Visitor.ConceptSpecializationExprsVisited);
   EXPECT_EQ(1, Visitor.TypeConstraintsTraversed);
+  EXPECT_EQ(1, Visitor.ConceptReferencesTraversed);
+  EXPECT_EQ(1, Visitor.ConceptReferencesVisited);
 
   Visitor = {};
   EXPECT_TRUE(Visitor.runOver("template  concept A = true;\n"
@@ -70,6 +84,8 @@
   "};",
   ConceptVisitor::Lang_CXX2a));
   EXPECT_EQ(3, Visitor.ConceptRequirementsTraversed);
+  EXPECT_EQ(1, Visitor.ConceptReferencesTraversed);
+  EXPECT_EQ(1, Visitor.ConceptReferencesVisited);
 }
 
 struct VisitDeclOnlyOnce : ExpectedLocationVisitor {
@@ -86,6 +102,10 @@
 ++AutoTypeLocVisited;
 return true;
   }
+  bool VisitConceptReference(const ConceptReference *) {
+++ConceptReferencesVisited;
+return true;
+  }
 
   bool TraverseVarDecl(VarDecl *V) {
 // The base traversal visits only the `TypeLoc`.
@@ -99,6 +119,7 @@
   int ConceptDeclsVisited = 0;
   int AutoTypeVisited = 0;
   int AutoTypeLocVisited = 0;
+  int ConceptReferencesVisited = 0;
 };
 
 TEST(RecursiveASTVisitor, ConceptDeclInAutoType) {
@@ -111,6 +132,7 @@
   EXPECT_EQ(1, Visitor.AutoTypeVisited);
   EXPECT_EQ(1, Visitor.AutoTypeLocVisited);
   EXPECT_EQ(1, Visitor.ConceptDeclsVisited);
+  EXPECT_EQ(1, Visitor.ConceptReferencesVisited);
 }
 
 } // end anonymous namespace
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -11,15 +11,16 @@
 ///
 //===--===//
 
-#include "clang/AST/ExprOpenMP.h"
-#include "clang/Serialization/ASTRecordWriter.h"
-#include "clang/Sema/DeclSpec.h"
+#include "clang/AST/ASTConcept.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
+#include "clang/AST/ExprOpenMP.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Lex/Token.h"
+#include "clang/Sema/DeclSpec.h"
+#include "clang/Serialization/ASTRecordWriter.h"
 #include "llvm/Bitstream/BitstreamWriter.h"
 using namespace clang;
 
@@ -437,13 +438,11 @@
 void ASTStmtWriter::VisitConceptSpecializationExpr(
 Con

[PATCH] D155858: Add a concept AST node.

[Jens Massberg via Phabricator via cfe-commits]

massberg marked 13 inline comments as done.
massberg added a comment.

I have resolved some of the comments, but I need another round to finish all of 
the, Thanks to all reviewers so far!




Comment at: clang/include/clang/AST/ExprConcepts.h:90
+  // NOTE(massberg): For the first minimal prototype we keep the
+  // following functions to prevent. Later these functions should
+  // be accessed getConceptReference().

sammccall wrote:
> incomplete comment: "to prevent" what?
> 
> Also FWIW I don't think *all* uses should be migrated, e.g. 
> `getNamedConcept()` is fundamental enough to be exposed here even if it's 
> implemented via ConceptReference.
I have updated the FIXME.



Comment at: clang/include/clang/AST/TypeLoc.h:2198
+  return getConceptLoc()->getTemplateArgsAsWritten()->getTemplateArgs()[i];
+// FIXME: Check why we still need this fallback in case that there is no
+// ConceptLoc.

hokein wrote:
> which kind of tests were broken if we remove this?
This is fixed now


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155858

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

[PATCH] D158505: [clang-format] Fix weird handling of AfterColon

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

HazardyKnusperkeks created this revision.
Herald added projects: All, clang, clang-format.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, owenpan, MyDeveloperDay.
HazardyKnusperkeks requested review of this revision.

This fixes https://github.com/llvm/llvm-project/issues/64895.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158505

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7950,6 +7950,11 @@
   verifyFormat("template \n"
"Constructor() : Initializer(FitsOnTheLine) {}",
getStyleWithColumns(Style, 50));
+  verifyFormat(
+  "Class::Class(int some, int arguments, int loonoog,\n"
+  " int more) noexcept :\n"
+  "Super{some, arguments}, Member{5}, Member2{2} {}",
+  Style);
   Style.PackConstructorInitializers = FormatStyle::PCIS_NextLine;
   verifyFormat(
   "SomeClass::Constructor() :\n"
@@ -7987,8 +7992,8 @@
   "aaa() {}",
   Style);
   verifyFormat("Constructor(aa ,\n"
-   "aa ) :\n"
-   "aa(aa) {}",
+   "aa ) : "
+   "aa(aa) {}",
Style);
 
   verifyFormat("Constructor() :\n"
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1451,6 +1451,8 @@
 CurrentState.NestedBlockIndent = CurrentState.Indent;
 if (Style.PackConstructorInitializers > FormatStyle::PCIS_BinPack)
   CurrentState.AvoidBinPacking = true;
+else
+  CurrentState.BreakBeforeParameter = false;
   }
   if (Current.is(TT_InheritanceColon)) {
 CurrentState.Indent =


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7950,6 +7950,11 @@
   verifyFormat("template \n"
"Constructor() : Initializer(FitsOnTheLine) {}",
getStyleWithColumns(Style, 50));
+  verifyFormat(
+  "Class::Class(int some, int arguments, int loonoog,\n"
+  " int more) noexcept :\n"
+  "Super{some, arguments}, Member{5}, Member2{2} {}",
+  Style);
   Style.PackConstructorInitializers = FormatStyle::PCIS_NextLine;
   verifyFormat(
   "SomeClass::Constructor() :\n"
@@ -7987,8 +7992,8 @@
   "aaa() {}",
   Style);
   verifyFormat("Constructor(aa ,\n"
-   "aa ) :\n"
-   "aa(aa) {}",
+   "aa ) : "
+   "aa(aa) {}",
Style);
 
   verifyFormat("Constructor() :\n"
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1451,6 +1451,8 @@
 CurrentState.NestedBlockIndent = CurrentState.Indent;
 if (Style.PackConstructorInitializers > FormatStyle::PCIS_BinPack)
   CurrentState.AvoidBinPacking = true;
+else
+  CurrentState.BreakBeforeParameter = false;
   }
   if (Current.is(TT_InheritanceColon)) {
 CurrentState.Indent =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158505: [clang-format] Fix weird handling of AfterColon

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

HazardyKnusperkeks added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:7995
   verifyFormat("Constructor(aa ,\n"
-   "aa ) :\n"
-   "aa(aa) {}",
+   "aa ) : "
+   "aa(aa) {}",

I know we usually don't change existing tests. But I sincerely think this a 
bug. From the documentation I don't see any reason why there should be a line 
break.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158505

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

[PATCH] D155858: Add a concept AST node.

[Jens Massberg via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/TreeTransform.h:6824-6836
+  unsigned size = TL.getTypePtr()->getTypeConstraintArguments().size();
+  TemplateArgumentLocInfo *TALI = new TemplateArgumentLocInfo[size];
+  TemplateSpecializationTypeLoc::initializeArgLocs(
+  SemaRef.Context, TL.getTypePtr()->getTypeConstraintArguments(), TALI,
+  SourceLocation());
+  TemplateArgumentLoc *TAL = new TemplateArgumentLoc[size];
+  for (unsigned i = 0; i < size; ++i)

This is very ugly as I have to create the TALI and TAL arrays temporarily to 
use the existing transformations.
Is there a better way to do that?
Moreover, is this the correct place for the transformation? This is necessary 
as the passed `AutoTypeLoc` doesn't have an attached `ConceptReference` even if 
it is constrained. Has the `ConceptReferenec`of the original `AutoTypeLoc` be 
added somewhere earlier?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155858

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

[PATCH] D156658: [clang][dataflow] When checking `ExprToLoc` convergence, only consider children of block terminator.

[Martin Böhme via Phabricator via cfe-commits]

mboehme added a comment.

Sorry for the late response -- I was on vacation.

In D156658#4554347 , @xazax.hun wrote:

> In D156658#4552965 , @mboehme wrote:
>
>> I've investigated this in more detail. Unfortunately, it turns out that it's 
>> not quite as simple as just implementing widening on `ExprToLoc`.
>>
>> One of the reasons for this is that we only apply widening at loop heads, 
>> but the expressions that are "blocking" convergence may be contained in a 
>> block that is not a loop head.
>
> I am probably missing something, but I why does it matter where are we doing 
> the widening?

We only do widening at loop heads, and this means that widening only affects 
locations and values that flow into the loop from the outside or from a 
previous loop iteration.

But convergence can also be blocked by locations and values that are only used 
within the loop body. If these change from loop iteration to loop iteration and 
we don't perform widening on them, we will conclude that the state of the loop 
body never converges.

>> Essentially, what we want is a "top" `PointerValue` that does not have an 
>> associated `StorageLocation`. However, we don't want to eliminate the 
>> `PointerValue` entirely; we still want to be able to attach properties to 
>> it, so that, for example, an analysis can record that the `PointerValue` is 
>> non-null, even though we don't know what its exact location is.
>
> Another way to interpret "top": it points to a "summary" `StorageLocation` 
> that can be any other `StorageLocation`, we just do not know which one. This 
> interpretation/formulation has some advantages:
>
> - We have a `StorageLocation` to use when we dereference these top pointers.
> - It is compatible with the alias sets representation.
> - It is compatible with some other representations where we have other 
> "summary" locations, like "UnkownStackLocation" or "UnkownHeapLocation".
>
> These summary memory locations are sort of the union of all the potential 
> memory locations they could represent. I think in general it might be useful 
> to embrace this idea, e.g., when we model arrays, we can have a single 
> element region representing all the knowledge we know to be true for all 
> elements of the array.

I like this!

I'll have to do some thinking about how we want to represent these unknown / 
"top" storage locations exactly. Is there going to be a singleton "top" storage 
location? Are we going to allow associating the "top" storage location with a 
value (probably not...)? And so on. But this seems like a good direction to 
investigate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156658

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

[PATCH] D158499: [analyzer] Compute FAM dynamic size

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

steakhal added a comment.

In D158499#4606337 , @danix800 wrote:

> In D158499#4606291 , @steakhal 
> wrote:
>
>> Thanks for submitting this.
>> A funny thing is that in my free time I was also working on this last week. 
>> I'll have a look at this more in depth during the week.
>> For the mean time here is my version, committed pretty much a couple days 
>> ago to my fork.
>> https://github.com/steakhal/llvm-project/commit/986059a146e036ec84db64868a079d3c4a0e5e16
>>
>> EDIT: fix the link to point to my fork.
>>
>> Your proposed change looks pretty similar to mine.
>
> Are you going to submit your changes? Which one do you think is more suitable?
>
> Please let me know whether I should continue on this one or wait for yours. 
> Thanks!

I think the difference between yours and my implementation is that I try to 
accept FAM like arrays even if they are nested inside other strict, if they are 
notionally the last field of the whole thing. I'm doing it because that is how 
the machines work, thus the model should model that and return the right extent 
even for those cases.
What I can tell, that I didn't check my implementation with the packed 
attribute. I assume it should work.
The decl has a tempting method checking if its a FAM, however that does not 
accept FAMs of size 1 or of any size.

So its hard to tell right away which approach is better, but I'll look into 
this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158499

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

[PATCH] D155776: [NFC] Add checks for self-assignment.

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D155776

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

[PATCH] D155858: Add a concept AST node.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang/lib/Sema/TreeTransform.h:6824-6836
+  unsigned size = TL.getTypePtr()->getTypeConstraintArguments().size();
+  TemplateArgumentLocInfo *TALI = new TemplateArgumentLocInfo[size];
+  TemplateSpecializationTypeLoc::initializeArgLocs(
+  SemaRef.Context, TL.getTypePtr()->getTypeConstraintArguments(), TALI,
+  SourceLocation());
+  TemplateArgumentLoc *TAL = new TemplateArgumentLoc[size];
+  for (unsigned i = 0; i < size; ++i)

massberg wrote:
> This is very ugly as I have to create the TALI and TAL arrays temporarily to 
> use the existing transformations.
> Is there a better way to do that?
> Moreover, is this the correct place for the transformation? This is necessary 
> as the passed `AutoTypeLoc` doesn't have an attached `ConceptReference` even 
> if it is constrained. Has the `ConceptReferenec`of the original `AutoTypeLoc` 
> be added somewhere earlier?
If I'm understanding the situation right...
the AutoTypeLoc in the template should have a ConceptReference, since the 
`auto` is constrained.
This seems like a bug to be fixed elsewhere, and then this `else` becomes dead.

Do you have a minimal example?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155858

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

[PATCH] D158254: Fixing the memory leak using split() instead of strtok

[Abhina Sree via Phabricator via cfe-commits]

abhina.sreeskantharajan added a comment.

In D158254#4605050 , @hchilama wrote:

> Hi @abhina.sreeskantharajan can I please know the next process on how to land 
> this revision to opensource?

Usually you need to have someone with commit access commit the changes for you 
for your first couple of patches and then you can request commit access 
yourself. This is outlined here 
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access
 I'd be happy to commit this patch for you if needed :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158254

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

[PATCH] D158426: [clangd] Bump timeouts for LSPServerTests

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/unittests/LSPClient.cpp:42
 [this] { return Value.has_value(); })) {
 ADD_FAILURE() << "No result from call after 10 seconds!";
 return llvm::json::Value(nullptr);

nit: 60 here too



Comment at: clang-tools-extra/clangd/unittests/LSPClient.h:37
 ~CallResult();
-// Blocks up to 10 seconds for the result to be ready.
+// Blocks up to \p TimeoutSeconds seconds for the result to be ready. If \p
+// TimeoutSeconds is zero, blocks indefinitely.

revert comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158426

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

[PATCH] D148381: [WIP][Clang] Add counted_by attribute

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 552318.
void added a comment.

Support when doing a _bdos on the full structure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148381

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test

Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -56,6 +56,7 @@
 // CHECK-NEXT: ConsumableAutoCast (SubjectMatchRule_record)
 // CHECK-NEXT: ConsumableSetOnRead (SubjectMatchRule_record)
 // CHECK-NEXT: Convergent (SubjectMatchRule_function)
+// CHECK-NEXT: CountedBy (SubjectMatchRule_field)
 // CHECK-NEXT: DLLExport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface)
 // CHECK-NEXT: DLLImport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface)
 // CHECK-NEXT: Destructor (SubjectMatchRule_function)
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -8380,6 +8380,21 @@
   D->addAttr(ZeroCallUsedRegsAttr::Create(S.Context, Kind, AL));
 }
 
+static void handleCountedByAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  // TODO: Probably needs more processing here. See Sema::AddAlignValueAttr.
+  if (!AL.isArgIdent(0)) {
+S.Diag(AL.getLoc(), diag::err_attribute_argument_type)
+<< AL << AANT_ArgumentIdentifier;
+return;
+  }
+
+  IdentifierLoc *IL = AL.getArgAsIdent(0);
+  CountedByAttr *CBA = ::new (S.Context) CountedByAttr(S.Context, AL,
+   IL->Ident);
+  CBA->setCountedByFieldLoc(IL->Loc);
+  D->addAttr(CBA);
+}
+
 static void handleFunctionReturnThunksAttr(Sema &S, Decl *D,
const ParsedAttr &AL) {
   StringRef KindStr;
@@ -9326,6 +9341,10 @@
 handleAvailableOnlyInDefaultEvalMethod(S, D, AL);
 break;
 
+  case ParsedAttr::AT_CountedBy:
+handleCountedByAttr(S, D, AL);
+break;
+
   // Microsoft attributes:
   case ParsedAttr::AT_LayoutVersion:
 handleLayoutVersion(S, D, AL);
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -17904,6 +17904,37 @@
  "Broken injected-class-name");
 }
 
+static const FieldDecl *FindFieldWithCountedByAttr(const RecordDecl *RD) {
+  for (const Decl *D : RD->decls()) {
+if (const auto *FD = dyn_cast(D))
+  if (FD->hasAttr())
+return FD;
+
+if (const auto *SubRD = dyn_cast(D))
+  if (const FieldDecl *FD = FindFieldWithCountedByAttr(SubRD))
+return FD;
+  }
+
+  return nullptr;
+}
+
+static const IdentifierInfo *
+CheckCountedByAttr(const RecordDecl *RD, const FieldDecl *FD,
+   SourceRange &Loc) {
+  const CountedByAttr *ECA = FD->getAttr();
+
+  auto It = llvm::find_if(
+  RD->fields(), [&](const FieldDecl *Field){
+return Field->getName() == ECA->getCountedByField()->getName();
+  });
+  if (It == RD->field_end()) {
+Loc = ECA->getCountedByFieldLoc();
+return ECA->getCountedByField();
+  }
+
+  return nullptr;
+}
+
 void Sema::ActOnTagFinishDefinition(Scope *S, Decl *TagD,
 SourceRange BraceRange) {
   AdjustDeclIfTemplate(TagD);
@@ -17961,6 +17992,19 @@
  [](const FieldDecl *FD) { return FD->isBitField(); }))
   Diag(BraceRange.getBegin(), diag::warn_pragma_align_not_xl_compatible);
   }
+
+  // Check the "counted_by" attribute to ensure that the count field exists in
+  // the struct.
+  if (const RecordDecl *RD = dyn_cast(Tag)) {
+if (const FieldDecl *FD = FindFieldWithCountedByAttr(RD)) {
+  SourceRange SR;
+  const IdentifierInfo *Unknown = CheckCountedByAttr(RD, FD, SR);
+
+  if (Unknown)
+Diag(SR.getBegin(), diag::warn_counted_by_placeholder)
+<< Unknown << SR;
+}
+  }
 }
 
 void Sema::ActOnObjCContainerFinishDefinition() {
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -302

[PATCH] D158488: [NFC] Initialize member pointers to nullptr.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/AST/Stmt.h:2606-2607
   enum { INIT, CONDVAR, COND, INC, BODY, END_EXPR };
-  Stmt* SubExprs[END_EXPR]; // SubExprs[INIT] is an expression or declstmt.
+  Stmt *SubExprs[END_EXPR] = {
+  nullptr}; // SubExprs[INIT] is an expression or declstmt.
   SourceLocation LParenLoc, RParenLoc;

I don't think this initialization is necessary. The constructor for `ForStmt` 
initializes all of the valid elements: 
https://github.com/llvm/llvm-project/blob/87c5a3e203ae3643bc13c9a13917b92a657f4358/clang/lib/AST/Stmt.cpp#L1024

The `EmptyShell` constructor does not initialize anything but that's because it 
is piecemeal initialized by the AST importer, but all of its fields are also 
initialized: 
https://github.com/llvm/llvm-project/blob/87c5a3e203ae3643bc13c9a13917b92a657f4358/clang/lib/Serialization/ASTReaderStmt.cpp#L296



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:934
   // Return value of the runtime offloading call.
-  Value *Return;
+  Value *Return = nullptr;
 

This is necessary to initialize because `emitTargetKernel()` has an early 
return which does not initialize the passed reference to this object.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158488

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

[PATCH] D158507: [Flang][Driver] Add support for fomit-frame-pointer

[victorkingi via Phabricator via cfe-commits]

victorkingi created this revision.
Herald added subscribers: abrachet, phosek, s.egerton, simoncook, asb, 
fedor.sergeev, dschuff.
Herald added a reviewer: sscalpone.
Herald added a reviewer: awarzynski.
Herald added projects: Flang, All.
victorkingi requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, wangpc, jdoerfert, 
MaskRay, aheejin.
Herald added projects: clang, LLVM.

Temporary fix for unknown argument error '-fomit-frame-pointer'
when running flang tests


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158507

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/frontend-forwarding.f90
  llvm/include/llvm/Frontend/Driver/FramePointer.h

Index: llvm/include/llvm/Frontend/Driver/FramePointer.h
===
--- /dev/null
+++ llvm/include/llvm/Frontend/Driver/FramePointer.h
@@ -0,0 +1,153 @@
+//===-- Clang.cpp - Clang+LLVM ToolChain Implementations *- 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/include/clang/Basic/CodeGenOptions.h"
+#include "../../../../../clang/include/clang/Driver/Options.h"
+#include "../../../../../clang/lib/Driver/ToolChains/CommonArgs.h"
+#include "llvm/Option/ArgList.h"
+#include "llvm/Support/CodeGen.h"
+#include "llvm/TargetParser/Triple.h"
+
+using namespace llvm::opt;
+
+static bool useFramePointerForTargetByDefault(const ArgList &Args,
+  const llvm::Triple &Triple) {
+  if (Args.hasArg(clang::driver::options::OPT_pg) &&
+  !Args.hasArg(clang::driver::options::OPT_mfentry))
+return true;
+
+  if (Triple.isAndroid()) {
+switch (Triple.getArch()) {
+case llvm::Triple::aarch64:
+case llvm::Triple::arm:
+case llvm::Triple::armeb:
+case llvm::Triple::thumb:
+case llvm::Triple::thumbeb:
+case llvm::Triple::riscv64:
+  return true;
+default:
+  break;
+}
+  }
+
+  switch (Triple.getArch()) {
+  case llvm::Triple::xcore:
+  case llvm::Triple::wasm32:
+  case llvm::Triple::wasm64:
+  case llvm::Triple::msp430:
+// XCore never wants frame pointers, regardless of OS.
+// WebAssembly never wants frame pointers.
+return false;
+  case llvm::Triple::ppc:
+  case llvm::Triple::ppcle:
+  case llvm::Triple::ppc64:
+  case llvm::Triple::ppc64le:
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64:
+  case llvm::Triple::sparc:
+  case llvm::Triple::sparcel:
+  case llvm::Triple::sparcv9:
+  case llvm::Triple::amdgcn:
+  case llvm::Triple::r600:
+  case llvm::Triple::csky:
+  case llvm::Triple::loongarch32:
+  case llvm::Triple::loongarch64:
+return !clang::driver::tools::areOptimizationsEnabled(Args);
+  default:
+break;
+  }
+
+  if (Triple.isOSFuchsia() || Triple.isOSNetBSD()) {
+return !clang::driver::tools::areOptimizationsEnabled(Args);
+  }
+
+  if (Triple.isOSLinux() || Triple.getOS() == llvm::Triple::CloudABI ||
+  Triple.isOSHurd()) {
+switch (Triple.getArch()) {
+// Don't use a frame pointer on linux if optimizing for certain targets.
+case llvm::Triple::arm:
+case llvm::Triple::armeb:
+case llvm::Triple::thumb:
+case llvm::Triple::thumbeb:
+case llvm::Triple::mips64:
+case llvm::Triple::mips64el:
+case llvm::Triple::mips:
+case llvm::Triple::mipsel:
+case llvm::Triple::systemz:
+case llvm::Triple::x86:
+case llvm::Triple::x86_64:
+  return !clang::driver::tools::areOptimizationsEnabled(Args);
+default:
+  return true;
+}
+  }
+
+  if (Triple.isOSWindows()) {
+switch (Triple.getArch()) {
+case llvm::Triple::x86:
+  return !clang::driver::tools::areOptimizationsEnabled(Args);
+case llvm::Triple::x86_64:
+  return Triple.isOSBinFormatMachO();
+case llvm::Triple::arm:
+case llvm::Triple::thumb:
+  // Windows on ARM builds with FPO disabled to aid fast stack walking
+  return true;
+default:
+  // All other supported Windows ISAs use xdata unwind information, so frame
+  // pointers are not generally useful.
+  return false;
+}
+  }
+
+  return true;
+}
+
+static bool mustUseNonLeafFramePointerForTarget(const llvm::Triple &Triple) {
+  switch (Triple.getArch()) {
+  default:
+return false;
+  case llvm::Triple::arm:
+  case llvm::Triple::thumb:
+// ARM Darwin targets require a frame pointer to be always present to aid
+// offline debugging via backtraces.
+return Triple.isOSDarwin();
+  }
+}
+
+static clang::CodeGenOptions::FramePointerKind
+

[PATCH] D158507: [Flang][Driver] Add support for fomit-frame-pointer

[Kiran Chandramohan via Phabricator via cfe-commits]

kiranchandramohan added a comment.

For our immediate purpose, I think changing the visibility in the Driver to 
include `flang` is sufficient. After that, we can spend time implementing this 
properly by refactoring the code to llvm.




Comment at: llvm/include/llvm/Frontend/Driver/FramePointer.h:11
+#include "../../../../../clang/include/clang/Driver/Options.h"
+#include "../../../../../clang/lib/Driver/ToolChains/CommonArgs.h"
+#include "llvm/Option/ArgList.h"

We should not include clang headers in llvm. We also should not refer to clang 
inside llvm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158507

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

[PATCH] D158507: [Flang][Driver] Add support for fomit-frame-pointer

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

> Temporary fix for unknown argument error '-fomit-frame-pointer' when running 
> flang tests

I don't follow - there's quite a lot going on here. More than the summary 
suggests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158507

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

[PATCH] D141177: [Clang] Don't tell people to place _Alignas on a struct in diagnostics

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:5337-5346
+  if (AL.isC11AlignasAttribute()) {
+// Don't use the message with placement with _Alignas.
+// This is because C doesnt let you use _Alignas on type declarations.
+Diag(AL.getLoc(), diag::warn_attribute_ignored) << AL;
+  } else {
+Diag(AL.getLoc(), AL.isRegularKeywordAttribute()
+  ? diag::err_declspec_keyword_has_no_effect

I think this might be a bit more clear:
```
unsigned DiagID;
if (AL.isC11AlignasAttribute()) {
  // Don't use the message with placement with _Alignas.
  // This is because C doesnt let you use _Alignas on type declarations.
  DiagID = diag::warn_attribute_ignored;
} else if (AL.isRegularKeywordAttribute())
  DiagID = diag::err_declspec_keyword_has_no_effect;
else
  DiagID = diag::warn_declspec_attribute_ignored;

Diag(AL.getLoc(), DiagID) << AL << GetDiagnosticTypeSpecifierID(DS);
```



Comment at: clang/test/Parser/c1x-alignas.c:12
 
+_Alignas(int) struct c6; // expected-warning {{'_Alignas' attribute ignored}}
+

I'd like to see a C23-specific test that does:
```
alignas(int) struct c7;
```
(the test doesn't have to be in this file, but it's to validate that we 
correctly handle `alignas` when it's a keyword that aliases to `_Alignas`.)


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

https://reviews.llvm.org/D141177

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

[PATCH] D158507: [Flang][Driver] Add support for fomit-frame-pointer 1/2

[victorkingi via Phabricator via cfe-commits]

victorkingi added a comment.

In D158507#4606498 , @awarzynski 
wrote:

>> Temporary fix for unknown argument error '-fomit-frame-pointer' when running 
>> flang tests
>
> I don't follow - there's quite a lot going on here. More than the summary 
> suggests.

Sorry about that, will be pushing a minimal version since what was needed for 
the fix was only an update to Options.td file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158507

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

[PATCH] D157708: [Sema] Suppress lookup diagnostics when checking reversed operators

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov abandoned this revision.
ilya-biryukov added a comment.

Thanks for clarifying! I misread the standard.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157708

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

[PATCH] D157572: [clang] Add `[[clang::library_extension]]` attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D157572#4604595 , @philnik wrote:

> In D157572#4604482 , @aaron.ballman 
> wrote:
>
>>> This allows standard libraries to mark symbols as extensions, so the 
>>> compiler can generate extension warnings when they are used.
>>
>> Huh, so this is basically the opposite of the `__extension__` macro (which 
>> is used to silence extension warnings)?
>
> I guess, kind-of. I never really understood the semantics of `__extension__` 
> though, so I'm not 100% certain.

It's used in system headers to say "I'm using an extension here, don't warn 
about it in -pedantic mode".

>> I don't think we need to introduce a new attribute to do this, we already 
>> have `diagnose_if`. e.g., https://godbolt.org/z/a5ae4T56o would that suffice?
>
> Part of the idea here is that the diagnostics should be part of 
> `-Wc++ab-extension`.

Hmmm, okay. And I'm assuming `-Wsystem-headers -pedantic` is too chatty because 
it's telling the user about all use of extensions, not extensions being 
introduced by the library itself? (e.g., https://godbolt.org/z/Gs3YGheMM is 
also not what you're after)

> I guess we could allow warning flags instead of just `"warning"` and 
> `"error"` in `diagnose_if` that specifies which warning group the diagnostic 
> should be part of. Something like `__attribute__((__diagnose_if__(__cplusplus 
> >= 201703L, "basic_string_view is a C++17 extension", 
> "-Wc++17-extensions")))`. I'm not sure how one could implement that, but I 
> guess there is some mechanism to translate "-Wwhatever" to a warning group, 
> since you can push and pop warnings.  That would open people up to add a 
> diagnostic to pretty much any warning group. I don't know if that's a good 
> idea. I don't really see a problem with that other than people writing weird 
> code, but people do that all the time anyways. Maybe I'm missing something 
> really problematic though.

That's actually a pretty interesting idea; `diagnose_if` could be given another 
parameter to specify a diagnostic group to associate the diagnostic with. This 
would let you do some really handy things like:

  void func(int i) __attribute__((diagnose_if(i < 0, "passing a negative value 
to 'func' is deprecated", "warning", "-Wdeprecated")));

But if we went this route, would we want to expose other diagnostic-related 
knobs like "show in system header" and "default to an error"? Also, the 
attribute currently can only be associated with a function; we can use this for 
classes by sticking it on a constructor but there's not much help for putting 
it on say a namespace or an enumeration. So we may need to extend the attribute 
in other ways. CC @cjdb as this seems of interest to you as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157572

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

[PATCH] D157879: [clang] Report missing designated initializers in C++

[Aaron Ballman via Phabricator via cfe-commits]

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

In D157879#4604288 , @phosek wrote:

> Note that there's an ongoing discussion on the GCC bug 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96868 whether 
> `-Wmissing-field-initializers` should apply to both C and C++ or just C.

Thank you for posting this! I'm with @jwakely on this: 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96868#c3 -- the warning is for 
people who want to be told they've not explicitly specified all of the 
initializers, it is not for telling you "this hasn't been initialized". I think 
the desire makes as much sense in C++ as it does in C, but this is more of a 
style warning than a correctness warning.

In D157879#4604471 , @ianloic wrote:

> In D157879#4604233 , @aeubanks 
> wrote:
>
>> ah I thought this was in `-Wall` but it's not
>>
>> the struct is
>>
>>   struct Foo {
>> const void* buffer;
>> uint32_t capacity;
>> uint32_t reserved;
>>   };
>>
>> where `reserved` isn't explicitly initialized. that seems like reasonable 
>> code, but I suppose we can just explicitly initialize `reserved` here
>
> FWIW, if this is the specific piece of code that led me to this conversation, 
> that declaration is in a header which at least in theory is supposed to 
> remain compatible with C. It's inside `extern "C" { ... }` even. We can't add 
> explicit initialize `reserved` and keep the definition C comaptible.

Ouch, yeah, that does make it a challenge. But why is 
`-Wmissing-field-initializers` enabled in the first place for the code? It 
seems like there are fields you *don't* want explicit initialization for, so 
perhaps the answer is to disable the diagnostic for those types?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157879

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

[PATCH] D158433: [Clang] Do not change the type of captured vars when checking lambda constraints

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 552329.
cor3ntin added a comment.

Improve comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158433

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/lambda-capture-type-deduction.cpp

Index: clang/test/SemaCXX/lambda-capture-type-deduction.cpp
===
--- clang/test/SemaCXX/lambda-capture-type-deduction.cpp
+++ clang/test/SemaCXX/lambda-capture-type-deduction.cpp
@@ -246,3 +246,17 @@
 static_assert(is_same);
   };
 }
+
+namespace GH61267 {
+template  concept C = true;
+
+template
+void f(int) {
+  int i;
+  [i](P) {}(0);
+  i = 4;
+}
+
+void test() { f(0);  }
+
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -19723,13 +19723,6 @@
 FunctionScopesIndex == MaxFunctionScopesIndex && VarDC == DC)
   return true;
 
-// When evaluating some attributes (like enable_if) we might refer to a
-// function parameter appertaining to the same declaration as that
-// attribute.
-if (const auto *Parm = dyn_cast(Var);
-Parm && Parm->getDeclContext() == DC)
-  return true;
-
 // Only block literals, captured statements, and lambda expressions can
 // capture; other scopes don't work.
 DeclContext *ParentDC =
@@ -19757,6 +19750,14 @@
   CSI->getCapture(Var).markUsed(BuildAndDiagnose);
   break;
 }
+
+// When evaluating some attributes (like enable_if) we might refer to a
+// function parameter appertaining to the same declaration as that
+// attribute.
+if (const auto *Parm = dyn_cast(Var);
+Parm && Parm->getDeclContext() == DC)
+  return true;
+
 // If we are instantiating a generic lambda call operator body,
 // we do not want to capture new variables.  What was captured
 // during either a lambdas transformation or initial parsing
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -15289,11 +15289,10 @@
   FD->setInvalidDecl();
 }
 
-static void RebuildLambdaScopeInfo(CXXMethodDecl *CallOperator,
-   Sema &S) {
-  CXXRecordDecl *const LambdaClass = CallOperator->getParent();
+LambdaScopeInfo *Sema::RebuildLambdaScopeInfo(CXXMethodDecl *CallOperator) {
+  CXXRecordDecl *LambdaClass = CallOperator->getParent();
 
-  LambdaScopeInfo *LSI = S.PushLambdaScope();
+  LambdaScopeInfo *LSI = PushLambdaScope();
   LSI->CallOperator = CallOperator;
   LSI->Lambda = LambdaClass;
   LSI->ReturnType = CallOperator->getReturnType();
@@ -15317,7 +15316,7 @@
 if (C.capturesVariable()) {
   ValueDecl *VD = C.getCapturedVar();
   if (VD->isInitCapture())
-S.CurrentInstantiationScope->InstantiatedLocal(VD, VD);
+CurrentInstantiationScope->InstantiatedLocal(VD, VD);
   const bool ByRef = C.getCaptureKind() == LCK_ByRef;
   LSI->addCapture(VD, /*IsBlock*/false, ByRef,
   /*RefersToEnclosingVariableOrCapture*/true, C.getLocation(),
@@ -15334,6 +15333,7 @@
 }
 ++I;
   }
+  return LSI;
 }
 
 Decl *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Decl *D,
@@ -15437,7 +15437,7 @@
 assert(inTemplateInstantiation() &&
"There should be an active template instantiation on the stack "
"when instantiating a generic lambda!");
-RebuildLambdaScopeInfo(cast(D), *this);
+RebuildLambdaScopeInfo(cast(D));
   } else {
 // Enter a new function scope
 PushFunctionScope();
Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -13,12 +13,14 @@
 #include "clang/Sema/SemaConcept.h"
 #include "TreeTransform.h"
 #include "clang/AST/ASTLambda.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/AST/ExprConcepts.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/OperatorPrecedence.h"
 #include "clang/Sema/EnterExpressionEvaluationContext.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Overload.h"
+#include "clang/Sema/ScopeInfo.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaDiagnostic.h"
 #include "clang/Sema/SemaInternal.h"
@@ -540,11 +542,6 @@
   auto AddSingleCapture = [&](const ValueDecl *CapturedPattern,
   unsigned Index) {
 ValueDecl *CapturedVar = LambdaClass->getCapture(Index)->getCapturedVar();
-if (cast(Function)->isConst()) {
-  QualType T = CapturedVar->getType();
-  T.addConst();
-  CapturedVar->setType(T);
-}
 if (CapturedVar->isInitCapture())
   Scope.InstantiatedL

[PATCH] D158507: [Flang][Driver] Add support for fomit-frame-pointer 1/2

[victorkingi via Phabricator via cfe-commits]

victorkingi updated this revision to Diff 552330.
victorkingi added a comment.

Minimalized patch to only have update to Options.td as well as
 tests accompanying the change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158507

Files:
  clang/include/clang/Driver/Options.td
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/fomit-frame-pointer.f90


Index: flang/test/Driver/fomit-frame-pointer.f90
===
--- /dev/null
+++ flang/test/Driver/fomit-frame-pointer.f90
@@ -0,0 +1,12 @@
+! Test no error emitted with 'fomit-frame-pointer' flag.
+
+! Test opt_record flags get generated for fc1
+! RUN: %flang -fomit-frame-pointer %s
+
+program forttest
+implicit none
+integer :: n
+
+n = 1
+
+end program forttest
Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -57,6 +57,7 @@
 ! HELP-NEXT: -fno-stack-arrays   Allocate array temporaries on the heap 
(default)
 ! HELP-NEXT: -fno-version-loops-for-stride
 ! HELP-NEXT: Do not create unit-strided loops (default)
+! HELP-NEXT: -fomit-frame-pointerOmit the frame pointer from functions 
that don't need it. Some stack unwinding cases, such as profilers and 
sanitizers, may prefer specifying -fno-omit-frame-pointer. On many targets, -O1 
and higher omit the frame pointer by default. -m[no-]omit-leaf-frame-pointer 
takes precedence for leaf functions
 ! HELP-NEXT: -fopenacc   Enable OpenACC
 ! HELP-NEXT: -fopenmp-target-debug   Enable debugging in the OpenMP offloading 
device RTL
 ! HELP-NEXT: -fopenmp-targets=
@@ -219,6 +220,7 @@
 ! HELP-FC1-NEXT: -load  Load the named plugin (dynamic shared 
object)
 ! HELP-FC1-NEXT: -menable-no-infsAllow optimization to assume there 
are no infinities.
 ! HELP-FC1-NEXT: -menable-no-nansAllow optimization to assume there 
are no NaNs.
+! HELP-FC1-NEXT: -mframe-pointer= Specify which frame pointers to 
retain.
 ! HELP-FC1-NEXT: -mllvm   Additional arguments to forward to 
LLVM's option processing
 ! HELP-FC1-NEXT: -mmlir   Additional arguments to forward to 
MLIR's option processing
 ! HELP-FC1-NEXT: -module-dirPut MODULE files in 
Index: flang/test/Driver/driver-help-hidden.f90
===
--- flang/test/Driver/driver-help-hidden.f90
+++ flang/test/Driver/driver-help-hidden.f90
@@ -65,6 +65,7 @@
 ! CHECK-NEXT: -fno-stack-arrays   Allocate array temporaries on the heap 
(default)
 ! CHECK-NEXT: -fno-version-loops-for-stride
 ! CHECK-NEXT: Do not create unit-strided loops 
(default)
+! CHECK-NEXT: -fomit-frame-pointerOmit the frame pointer from functions 
that don't need it. Some stack unwinding cases, such as profilers and 
sanitizers, may prefer specifying -fno-omit-frame-pointer. On many targets, -O1 
and higher omit the frame pointer by default. -m[no-]omit-leaf-frame-pointer 
takes precedence for leaf functions
 ! CHECK-NEXT: -fopenacc   Enable OpenACC
 ! CHECK-NEXT: -fopenmp-assume-no-nested-parallelism
 ! CHECK-NEXT: Assert no nested parallel regions in the 
GPU
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3061,7 +3061,8 @@
 def fno_objc_legacy_dispatch : Flag<["-"], "fno-objc-legacy-dispatch">, 
Group;
 def fno_objc_weak : Flag<["-"], "fno-objc-weak">, Group,
   Visibility<[ClangOption, CC1Option]>;
-def fno_omit_frame_pointer : Flag<["-"], "fno-omit-frame-pointer">, 
Group;
+def fno_omit_frame_pointer : Flag<["-"], "fno-omit-frame-pointer">, 
Group,
+  Visibility<[ClangOption, FlangOption]>;
 defm operator_names : BoolFOption<"operator-names",
   LangOpts<"CXXOperatorNames">, Default,
   NegFlag>;
 
 def fomit_frame_pointer : Flag<["-"], "fomit-frame-pointer">, Group,
+  Visibility<[ClangOption, FlangOption]>,
   HelpText<"Omit the frame pointer from functions that don't need it. "
   "Some stack unwinding cases, such as profilers and sanitizers, may prefer 
specifying -fno-omit-frame-pointer. "
   "On many targets, -O1 and higher omit the frame pointer by default. "
@@ -6615,10 +6617,6 @@
 def mdebug_pass : Separate<["-"], "mdebug-pass">,
   HelpText<"Enable additional debug output">,
   MarshallingInfoString>;
-def mframe_pointer_EQ : Joined<["-"], "mframe-pointer=">,
-  HelpText<"Specify which frame pointers to retain.">, 
Values<"all,non-leaf,none">,
-  NormalizedValuesScope<"CodeGenOptions::FramePointerKind">, 
NormalizedValues<["All", "NonLeaf", "None"]>,
-  MarshallingInfoEnum, "None">;
 def mabi_EQ_ieeelongdouble : Fla

[PATCH] D158507: [Flang][Driver] Add support for fomit-frame-pointer 1/2

[victorkingi via Phabricator via cfe-commits]

victorkingi added inline comments.



Comment at: clang/include/clang/Driver/Options.td:7236-7240
+def mframe_pointer_EQ : Joined<["-"], "mframe-pointer=">,
+  HelpText<"Specify which frame pointers to retain.">, 
Values<"all,non-leaf,none">,
+  NormalizedValuesScope<"CodeGenOptions::FramePointerKind">, 
NormalizedValues<["All", "NonLeaf", "None"]>,
+  MarshallingInfoEnum, "None">;
+

This will be needed in the second patch when the flag functionality is added


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158507

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

[PATCH] D157684: [clang][ASTImporter] Repeated friend templates are partially imported

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

balazske added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:4072
+
+  if (auto *TSI = FD1->getFriendType())
+return Importer.IsStructurallyEquivalent(

According to the coding rules `auto` should not be used here.



Comment at: clang/lib/AST/ASTImporter.cpp:4079
+  Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer),
+  /* StrictTypeSpelling = */ false, /* Complain = */ false);
+  return Ctx.IsEquivalent(FD1, FD2);

This is a comparison in the same AST context ("From" side). 
`Importer.getNonEquivalentDecls()` returns a cache that is used at compares 
between "From" and "To" context. This is not valid for this use, you can simply 
pass an empty map instead, or add a new member that is used (only) here. 
`getStructuralEquivalenceKind(Importer)` is not needed for this compare, it can 
be always "Default".



Comment at: clang/lib/AST/ASTImporter.cpp:4089
 
-  T TypeOrDecl = GetCanTypeOrDecl(FD);
-
-  for (const FriendDecl *FoundFriend : RD->friends()) {
+  for (auto *FoundFriend : RD->friends()) {
 if (FoundFriend == FD) {

`auto` is not good here too.



Comment at: clang/lib/AST/ASTImporter.cpp:4114
   SmallVector ImportedEquivalentFriends;
-
-  while (ImportedFriend) {
-bool Match = false;
-if (D->getFriendDecl() && ImportedFriend->getFriendDecl()) {
-  Match =
-  IsStructuralMatch(D->getFriendDecl(), 
ImportedFriend->getFriendDecl(),
-/*Complain=*/false);
-} else if (D->getFriendType() && ImportedFriend->getFriendType()) {
-  Match = Importer.IsStructurallyEquivalent(
-  D->getFriendType()->getType(),
-  ImportedFriend->getFriendType()->getType(), /*Complain=*/false);
-}
-if (Match)
+  for (auto *ImportedFriend : RD->friends())
+if (IsEquivalentFriend(Importer, D, ImportedFriend))

`auto` should not be used here, this loop could be replaced by some generic 
"algorithm" function call (`llvm::copy_if`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157684

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

[PATCH] D157331: [clang] Implement C23

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Headers/stdckdint.h:13
+
+#define __STDC_VERSION_STDCKDINT_H__ 202311L
+

I think this should only be defined when the function-like macros are being 
defined, so this should move down into the `#if` block below. That way, people 
can do:
```
#if __STDC_VERSION_STDCKDINT_H__  >= 202311L
```
to know they've got access to the macros. Which is weird because they could 
also do `#ifdef ckd_add`, but oh well, C is weird.



Comment at: clang/lib/Headers/stdckdint.h:1
+/*=== stdckdint.h - Standard header for checking integer
+ *-===

ZijunZhao wrote:
> aaron.ballman wrote:
> > hiraditya wrote:
> > > nit: format.
> > The formatting for this is still off (it line wraps here and on line 7-8 -- 
> > it should be shortened so that the closing comment fits within the 80 col 
> > limit).
> oh yes I set 80 col limit and they don't exceed the line. {F28780476} 
> I attach the screenshort and hope it help explain. I think I might 
> misunderstand something?
Oh! I was thrown off by the C-style commenting, but you're right, this is 
correct as-is (and matches how the other C headers look), sorry for the noise!



Comment at: clang/test/C/C2x/n2683.c:16
+
+bool flag_add = ckd_add(&result, a33, char_var);
+bool flag_sub = ckd_sub(&result, bool_var, day);

ZijunZhao wrote:
> aaron.ballman wrote:
> > It looks like the builtins are missing some checks that are required by the 
> > C standard.
> > 
> > 7.20p3: Both type2 and type3 shall be any integer type other than "plain" 
> > char, bool, a bit-precise integer type, or an enumerated type, and they 
> > need not be the same.  ...
> > 
> > So we should get a (warning?) diagnostic on all of these uses.
> > 
> > We should also add a test when the result type is not suitable for the 
> > given operand types. e.g.,
> > ```
> > void func(int one, int two) {
> >   short result;
> >   ckd_add(&result, one, two); // `short` may not be suitable to hold the 
> > result of adding two `int`s
> > 
> >   const int other_result = 0;
> >   ckd_add(&other_result, one, two); // `const int` is definitely not 
> > suitable because it's not a modifiable lvalue
> > }
> > ```
> > This is because of:
> > 
> > 7.20.1p4: It is recommended to produce a diagnostic message if type2 or 
> > type3 are not suitable integer types, or if *result is not a modifiable 
> > lvalue of a suitable integer type.
> yes, I am trying to add the tests about `short` type and `const` variable but 
> there is no warning about inappropriate type 😢 and for const one I get 
> ```result argument to overflow builtin must be a pointer to a non-const 
> integer ('const int *' invalid)``` error 😂 
I think the diagnostic with a `const` result is reasonable enough (unfortunate 
it talks about builtins but we can correct the wording later). The other type 
checking is something that will need to be added to: 
https://github.com/llvm/llvm-project/blob/8f6a1a07cb85980013c70d5af6d28f5fcf75e732/clang/lib/Sema/SemaChecking.cpp#L368

One thing you'll have to do is determine whether the builtin is being used via 
the standard interface or whether it's being used directly by the user. We 
don't want to change the behavior of the call through `__builtin_add_overflow` 
because that can break existing code. So you'll need to see if the call 
expression is a macro expansion from a macro named `ckd_add` (for example) and 
perform the extra checking only in that case.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157331

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

[clang] f740bcb - [AIX] supporting -X options for llvm-ranlib in AIX OS

[via cfe-commits]

Author: zhijian
Date: 2023-08-22T09:41:33-04:00
New Revision: f740bcb3707a17ed4ccd52157089011a586cc2a6

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

LOG: [AIX] supporting -X options for llvm-ranlib in AIX OS

Summary:

llvm-ar is symlinked as llvm-ranlib and will act as ranlib when invoked in that 
mode. llvm-ar since [[ 
https://github.ibm.com/compiler/llvm-project/commit/4f2cfbe5314b064625b2c87bde6ce5c8d04004c5
 | compiler/llvm-project@4f2cfbe ]] supports the -X options, but doesn't seem 
to accept them when running as llvm-ranlib.

In AIX OS , according to 
https://www.ibm.com/docs/en/aix/7.2?topic=r-ranlib-command

-X mode Specifies the type of object file ranlib should examine. The 
mode must be one of the following:

32
Processes only 32-bit object files
64
Processes only 64-bit object files
32_64, any
Processes both 32-bit and 64-bit object files

The default is to process 32-bit object files (ignore 64-bit objects). The mode 
can also be set with the OBJECT_MODE environment variable. For example, 
OBJECT_MODE=64 causes ranlib to process any 64-bit objects and ignore 32-bit 
objects. The -X flag overrides the OBJECT_MODE variable.

Reviewers: James Henderson, MaskRay, Stephen Peckham
Differential Revision: https://reviews.llvm.org/D142660

Added: 
llvm/test/tools/llvm-ranlib/AIX-X-option-non-AIX.test
llvm/test/tools/llvm-ranlib/aix-X-option.test

Modified: 
clang/lib/Driver/OffloadBundler.cpp
clang/test/lit.cfg.py
clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
llvm/include/llvm/Object/Archive.h
llvm/include/llvm/Object/ArchiveWriter.h
llvm/lib/ObjCopy/Archive.cpp
llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp
llvm/lib/Object/Archive.cpp
llvm/lib/Object/ArchiveWriter.cpp
llvm/lib/Object/COFFImportFile.cpp
llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
llvm/tools/llvm-ar/llvm-ar.cpp
llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp

Removed: 




diff  --git a/clang/lib/Driver/OffloadBundler.cpp 
b/clang/lib/Driver/OffloadBundler.cpp
index 0ddfb07fdad5ba..d11c41605bf39e 100644
--- a/clang/lib/Driver/OffloadBundler.cpp
+++ b/clang/lib/Driver/OffloadBundler.cpp
@@ -1265,8 +1265,9 @@ Error OffloadBundler::UnbundleArchive() {
 OutputArchivesMap.find(Target);
 if (CurArchiveMembers != OutputArchivesMap.end()) {
   if (Error WriteErr = writeArchive(FileName, 
CurArchiveMembers->getValue(),
-true, getDefaultArchiveKindForHost(),
-true, false, nullptr))
+SymtabWritingMode::NormalSymtab,
+getDefaultArchiveKindForHost(), true,
+false, nullptr))
 return WriteErr;
 } else if (!BundlerConfig.AllowMissingBundles) {
   std::string ErrMsg =
@@ -1280,9 +1281,9 @@ Error OffloadBundler::UnbundleArchive() {
  // the missing input file.
   std::vector EmptyArchive;
   EmptyArchive.clear();
-  if (Error WriteErr = writeArchive(FileName, EmptyArchive, true,
-getDefaultArchiveKindForHost(), true,
-false, nullptr))
+  if (Error WriteErr = writeArchive(
+  FileName, EmptyArchive, SymtabWritingMode::NormalSymtab,
+  getDefaultArchiveKindForHost(), true, false, nullptr))
 return WriteErr;
 }
   }

diff  --git a/clang/test/lit.cfg.py b/clang/test/lit.cfg.py
index 68e038475a5c75..815ea219aaa81e 100644
--- a/clang/test/lit.cfg.py
+++ b/clang/test/lit.cfg.py
@@ -380,16 +380,15 @@ def exclude_unsupported_files_for_aix(dirname):
 elif platform.system() == "AIX":
 config.environment["AIXTHREAD_STK"] = "4194304"
 
-# The llvm-nm tool supports an environment variable "OBJECT_MODE" on AIX OS, 
which
+# Some tools support an environment variable "OBJECT_MODE" on AIX OS, which
 # controls the kind of objects they will support. If there is no "OBJECT_MODE"
 # environment variable specified, the default behaviour is to support 32-bit
 # objects only. In order to not affect most test cases, which expect to support
 # 32-bit and 64-bit objects by default, set the environment variable
-# "OBJECT_MODE" to 'any' for llvm-nm on AIX OS.
+# "OBJECT_MODE" to "any" by default on AIX OS.
 
 if "system-aix" in config.available_features:
-config.substitutions.append(("llvm-nm", "env OBJECT_MODE=any llvm-nm"))
-config.substitutions.append(("llvm-ar", "env OBJECT_MODE=any llvm-ar"))
+   config.environment["OBJECT_MODE"] = "any"
 
 # It is not realistically possible to account for all options that could
 # possibly be present in system and user configuration files, s

[PATCH] D142660: [AIX] supporting -X options for llvm-ranlib in AIX OS

[Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf740bcb3707a: [AIX] supporting -X options for llvm-ranlib in 
AIX OS (authored by zhijian ).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142660

Files:
  clang/lib/Driver/OffloadBundler.cpp
  clang/test/lit.cfg.py
  clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
  llvm/include/llvm/Object/Archive.h
  llvm/include/llvm/Object/ArchiveWriter.h
  llvm/lib/ObjCopy/Archive.cpp
  llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp
  llvm/lib/Object/Archive.cpp
  llvm/lib/Object/ArchiveWriter.cpp
  llvm/lib/Object/COFFImportFile.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/test/tools/llvm-ranlib/AIX-X-option-non-AIX.test
  llvm/test/tools/llvm-ranlib/aix-X-option.test
  llvm/tools/llvm-ar/llvm-ar.cpp
  llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp

Index: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp
===
--- llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp
+++ llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp
@@ -594,18 +594,17 @@
 
   if (NewMembers.size() == 1)
 return writeArchive(OutputFile, NewMembers.begin()->second.getMembers(),
-/*WriteSymtab=*/true,
+SymtabWritingMode::NormalSymtab,
 /*Kind=*/object::Archive::K_DARWIN, C.Deterministic,
 /*Thin=*/false);
 
   SmallVector, 2> OutputBinaries;
   for (const std::pair &M : NewMembers) {
 Expected> OutputBufferOrErr =
-writeArchiveToBuffer(M.second.getMembers(),
- /*WriteSymtab=*/true,
- /*Kind=*/object::Archive::K_DARWIN,
- C.Deterministic,
- /*Thin=*/false);
+writeArchiveToBuffer(
+M.second.getMembers(), SymtabWritingMode::NormalSymtab,
+/*Kind=*/object::Archive::K_DARWIN, C.Deterministic,
+/*Thin=*/false);
 if (!OutputBufferOrErr)
   return OutputBufferOrErr.takeError();
 std::unique_ptr &OutputBuffer = OutputBufferOrErr.get();
Index: llvm/tools/llvm-ar/llvm-ar.cpp
===
--- llvm/tools/llvm-ar/llvm-ar.cpp
+++ llvm/tools/llvm-ar/llvm-ar.cpp
@@ -69,7 +69,9 @@
  << "  -v --version  - Display the version of this program\n"
  << "  -D- Use zero for timestamps and uids/gids "
 "(default)\n"
- << "  -U- Use actual timestamps and uids/gids\n";
+ << "  -U- Use actual timestamps and uids/gids\n"
+ << "  -X{32|64|32_64|any}   - Specify which archive symbol tables "
+"should be generated if they do not already exist (AIX OS only)\n";
 }
 
 static void printArHelp(StringRef ToolName) {
@@ -225,7 +227,8 @@
 static bool CompareFullPath = false;  ///< 'P' modifier
 static bool OnlyUpdate = false;   ///< 'u' modifier
 static bool Verbose = false;  ///< 'v' modifier
-static bool Symtab = true;///< 's' modifier
+static SymtabWritingMode Symtab =
+SymtabWritingMode::NormalSymtab;  ///< 's' modifier
 static bool Deterministic = true; ///< 'D' and 'U' modifiers
 static bool Thin = false; ///< 'T' modifier
 static bool AddLibrary = false;   ///< 'L' modifier
@@ -371,11 +374,11 @@
   CompareFullPath = true;
   break;
 case 's':
-  Symtab = true;
+  Symtab = SymtabWritingMode::NormalSymtab;
   MaybeJustCreateSymTab = true;
   break;
 case 'S':
-  Symtab = false;
+  Symtab = SymtabWritingMode::NoSymtab;
   break;
 case 'u':
   OnlyUpdate = true;
@@ -1074,9 +1077,31 @@
   // In summary, we only need to update the symbol table if we have none.
   // This is actually very common because of broken build systems that think
   // they have to run ranlib.
-  if (OldArchive->hasSymbolTable())
-return;
+  if (OldArchive->hasSymbolTable()) {
+if (OldArchive->kind() != object::Archive::K_AIXBIG)
+  return;
 
+// For archives in the Big Archive format, the bit mode option specifies
+// which symbol table to generate. The presence of a symbol table that does
+// not match the specified bit mode does not prevent creation of the symbol
+// table that has been requested.
+if (OldArchive->kind() == object::Archive::K_AIXBIG) {
+  BigArchive *BigArc = dyn_cast(OldArchive);
+  if (BigArc->has32BitGlobalSymtab() &&
+  Symtab == SymtabWritingMode::BigArchive32)
+return;
+
+  if (BigArc->has64BitGlobalSymtab() &&
+  Symtab == SymtabWritingMode::BigArchive64)
+return;
+
+  if (BigArc->has32BitGlobalSymtab() && BigArc->has64BitGloba

[PATCH] D158502: [clang][Interp] Actually consider ConstantExpr result

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:767-772
+  std::optional T = classify(E->getType());
+  if (T && E->hasAPValueResult() &&
+  this->visitAPValue(E->getAPValueResult(), *T, E))
+return true;
+
   return this->delegate(E->getSubExpr());

so if `visitAPValue` fails, we continue. Couldn't that lead to duplicated 
diagnostics? Shouldn't we simply return whatever `visitAPValue` returns 
unconditionally?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158502

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

[PATCH] D158513: [clang][dataflow] Add two repros for non-convergence involving pointers in loops.

[Martin Böhme via Phabricator via cfe-commits]

mboehme created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

These are broken out from https://reviews.llvm.org/D156658, which it now seems
obvious isn't the right way to solve the non-convergence.

Instead, my plan is to address the non-convergence through pointer value
widening, but the exact way this should be implemented is TBD. In the meantime,
I think there's value in getting these repros submitted to record the current
undesirable behavior.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158513

Files:
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2667,11 +2667,7 @@
 void target() {}
   )";
   ASSERT_THAT_ERROR(
-  checkDataflowWithNoopAnalysis(
-  Code,
-  [](const llvm::StringMap> 
&Results,
- ASTContext &ASTCtx) {},
-  {BuiltinOptions()}),
+  checkDataflowWithNoopAnalysis(Code),
   llvm::FailedWithMessage("Cannot analyze templated declarations"));
 }
 
@@ -2683,11 +2679,7 @@
 };
   )";
   ASSERT_THAT_ERROR(
-  checkDataflowWithNoopAnalysis(
-  Code,
-  [](const llvm::StringMap> 
&Results,
- ASTContext &ASTCtx) {},
-  {BuiltinOptions()}),
+  checkDataflowWithNoopAnalysis(Code),
   llvm::FailedWithMessage("Cannot analyze templated declarations"));
 }
 
@@ -3836,6 +3828,52 @@
   });
 }
 
+TEST(TransferTest, LoopDereferencingChangingPointerConverges) {
+  std::string Code = R"cc(
+bool some_condition();
+
+void target(int i1, int i2) {
+  int *p = &i1;
+  while (true) {
+(void)*p;
+if (some_condition())
+  p = &i1;
+else
+  p = &i2;
+  }
+}
+  )cc";
+  // FIXME: Implement pointer value widening to make analysis converge.
+  ASSERT_THAT_ERROR(
+  checkDataflowWithNoopAnalysis(Code),
+  llvm::FailedWithMessage("maximum number of iterations reached"));
+}
+
+TEST(TransferTest, LoopDereferencingChangingRecordPointerConverges) {
+  std::string Code = R"cc(
+struct Lookup {
+  int x;
+};
+
+bool some_condition();
+
+void target(Lookup l1, Lookup l2) {
+  Lookup *l = &l1;
+  while (true) {
+(void)l->x;
+if (some_condition())
+  l = &l1;
+else
+  l = &l2;
+  }
+}
+  )cc";
+  // FIXME: Implement pointer value widening to make analysis converge.
+  ASSERT_THAT_ERROR(
+  checkDataflowWithNoopAnalysis(Code),
+  llvm::FailedWithMessage("maximum number of iterations reached"));
+}
+
 TEST(TransferTest, DoesNotCrashOnUnionThisExpr) {
   std::string Code = R"(
 union Union {
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -402,8 +402,8 @@
 std::function<
 void(const llvm::StringMap> &,
  ASTContext &)>
-VerifyResults,
-DataflowAnalysisOptions Options,
+VerifyResults = [](const auto &, auto &) {},
+DataflowAnalysisOptions Options = {BuiltinOptions()},
 LangStandard::Kind Std = LangStandard::lang_cxx17,
 llvm::StringRef TargetFun = "target");
 


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2667,11 +2667,7 @@
 void target() {}
   )";
   ASSERT_THAT_ERROR(
-  checkDataflowWithNoopAnalysis(
-  Code,
-  [](const llvm::StringMap> &Results,
- ASTContext &ASTCtx) {},
-  {BuiltinOptions()}),
+  checkDataflowWithNoopAnalysis(Code),
   llvm::FailedWithMessage("Cannot analyze templated declarations"));
 }
 
@@ -2683,11 +2679,7 @@
 };
   )";
   ASSERT_THAT_ERROR(
-  checkDataflowWithNoopAnalysis(
-  Code,
-  [](const llvm::StringMap> &Results,
- ASTContext &ASTCtx) {},
-  {BuiltinOptions()}),
+  checkDataflowWithNoopAnalysis(Code),
   llvm::FailedWithMessage("Cannot analyze templated declarations"));
 }
 
@@ -3836,6 +3828,52 @@
   });
 }
 
+TEST(TransferTest, LoopDereferencingChangingPointerConverges) {
+  std::string Code = R"cc(
+bool some_condition();
+
+void target(int i1, int i2) {
+  int *p = &i1;
+  while (true) {
+(void)*p;
+if (some_condition())
+  p = 

[PATCH] D156658: [clang][dataflow] When checking `ExprToLoc` convergence, only consider children of block terminator.

[Martin Böhme via Phabricator via cfe-commits]

mboehme added a comment.

I've broken out the tests into https://reviews.llvm.org/D158513, as it seems 
clear we want to get these submitted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156658

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

[PATCH] D139730: [OpenMP][DeviceRTL][AMDGPU] Support code object version 5

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam updated this revision to Diff 552344.
saiislam marked 5 inline comments as done.
saiislam added a comment.

Used CreateConstInBoundsGEP1_32 for emitting GEP statements. Changed lambda 
function to simple fucntion body for defining the global variable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139730

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/TargetInfo.h
  clang/lib/CodeGen/Targets/AMDGPU.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGenCUDA/amdgpu-code-object-version-linking.cu
  clang/test/CodeGenCUDA/amdgpu-workgroup-size.cu
  clang/test/CodeGenOpenCL/opencl_types.cl
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
  openmp/libomptarget/DeviceRTL/CMakeLists.txt
  openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp
  openmp/libomptarget/plugins-nextgen/amdgpu/utils/UtilitiesRTL.h

Index: openmp/libomptarget/plugins-nextgen/amdgpu/utils/UtilitiesRTL.h
===
--- openmp/libomptarget/plugins-nextgen/amdgpu/utils/UtilitiesRTL.h
+++ openmp/libomptarget/plugins-nextgen/amdgpu/utils/UtilitiesRTL.h
@@ -25,6 +25,7 @@
 #include "llvm/Support/MemoryBufferRef.h"
 
 #include "llvm/Support/YAMLTraits.h"
+using namespace llvm::ELF;
 
 namespace llvm {
 namespace omp {
@@ -34,17 +35,25 @@
 
 // The implicit arguments of AMDGPU kernels.
 struct AMDGPUImplicitArgsTy {
-  uint64_t OffsetX;
-  uint64_t OffsetY;
-  uint64_t OffsetZ;
-  uint64_t HostcallPtr;
-  uint64_t Unused0;
-  uint64_t Unused1;
-  uint64_t Unused2;
+  uint32_t BlockCountX;
+  uint32_t BlockCountY;
+  uint32_t BlockCountZ;
+  uint16_t GroupSizeX;
+  uint16_t GroupSizeY;
+  uint16_t GroupSizeZ;
+  uint8_t Unused0[46]; // 46 byte offset.
+  uint16_t GridDims;
+  uint8_t Unused1[190]; // 190 byte offset.
 };
 
-static_assert(sizeof(AMDGPUImplicitArgsTy) == 56,
-  "Unexpected size of implicit arguments");
+enum IMPLICITARGS : uint32_t {
+  COV4_SIZE = 56,
+  COV5_SIZE = 256,
+};
+
+uint16_t getImplicitArgsSize(uint16_t Version) {
+  return Version < ELF::ELFABIVERSION_AMDGPU_HSA_V5 ? COV4_SIZE : COV5_SIZE;
+}
 
 /// Parse a TargetID to get processor arch and feature map.
 /// Returns processor subarch.
@@ -295,7 +304,8 @@
 /// Reads the AMDGPU specific metadata from the ELF file and propagates the
 /// KernelInfoMap
 Error readAMDGPUMetaDataFromImage(MemoryBufferRef MemBuffer,
-  StringMap &KernelInfoMap) {
+  StringMap &KernelInfoMap,
+  uint16_t &ELFABIVersion) {
   Error Err = Error::success(); // Used later as out-parameter
 
   auto ELFOrError = object::ELF64LEFile::create(MemBuffer.getBuffer());
@@ -305,6 +315,12 @@
   const object::ELF64LEFile ELFObj = ELFOrError.get();
   ArrayRef Sections = cantFail(ELFObj.sections());
   KernelInfoReader Reader(KernelInfoMap);
+
+  // Read the code object version from ELF image header
+  auto Header = ELFObj.getHeader();
+  ELFABIVersion = (uint8_t)(Header.e_ident[ELF::EI_ABIVERSION]);
+  DP("ELFABIVERSION Version: %u\n", ELFABIVersion);
+
   for (const auto &S : Sections) {
 if (S.sh_type != ELF::SHT_NOTE)
   continue;
Index: openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp
===
--- openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp
+++ openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp
@@ -381,6 +381,9 @@
   /// Get the executable.
   hsa_executable_t getExecutable() const { return Executable; }
 
+  /// Get to Code Object Version of the ELF
+  uint16_t getELFABIVersion() const { return ELFABIVersion; }
+
   /// Find an HSA device symbol by its name on the executable.
   Expected
   findDeviceSymbol(GenericDeviceTy &Device, StringRef SymbolName) const;
@@ -401,6 +404,7 @@
   hsa_executable_t Executable;
   hsa_code_object_t CodeObject;
   StringMap KernelInfoMap;
+  uint16_t ELFABIVersion;
 };
 
 /// Class implementing the AMDGPU kernel functionalities which derives from the
@@ -408,8 +412,7 @@
 struct AMDGPUKernelTy : public GenericKernelTy {
   /// Create an AMDGPU kernel with a name and an execution mode.
   AMDGPUKernelTy(const char *Name, OMPTgtExecModeFlags ExecutionMode)
-  : GenericKernelTy(Name, ExecutionMode),
-ImplicitArgsSize(sizeof(utils::AMDGPUImplicitArgsTy)) {}
+  : GenericKernelTy(Name, ExecutionMode) {}
 
   /// Initialize the AMDGPU kernel.
   Error initImpl(GenericDeviceTy &Device, DeviceImageTy &Image) override {
@@ -450,6 +453,9 @@
 // TODO: Read the kernel descriptor for the max threads per block. May be
 // read from the image.
 
+ImplicitArgsSize = utils::getImplicitArgsSize(AMDImage.getELFABIVersion());
+DP("ELFABIVersion: %d\n", AMDImage.getELFABIVersion());

[PATCH] D158223: [clang] Add clang::unnamed_addr attribute that marks globals' address as not significant

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: erichkeane.
aaron.ballman added a comment.

Hmmm, so this is effectively making a per-declaration version of 
`-fmerge-all-constants`? (How does this interact with the `common` attribute, 
if at all?)




Comment at: clang/include/clang/Basic/AttrDocs.td:1416-1417
+not significant. This allows global constants with the same contents to be
+merged. This can break global pointer identity, i.e. two different globals have
+the same address.
+

What happens for tentative definitions where the value isn't known? e.g.,
```
[[clang::unnamed_addr]] int i1, i2;
```

What happens if the types are similar but not the same?
```
[[clang::unnamed_addr]] signed int i1 = 32;
[[clang::unnamed_addr]] unsigned int i2 = 32;
```

Should we diagnose taking the address of such an attributed variable so users 
have some hope of spotting the non-conforming situations?

Does this attribute have impacts across translation unit boundaries (perhaps 
only when doing LTO) or only within a single TU?

What does this attribute do in C++ in the presence of constructors and 
destructors? e.g.,
```
struct S {
  S();
  ~S();
};

[[clang::unnamed_addr]] S s1, s2; // Are these merged and there's only one 
ctor/dtor call?
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158223

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

[PATCH] D158515: [include-cleaner] Make handling of enum constants similar to members

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

We were treating enum constants more like regular decls, which results
in ignoring type aliases/exports.
This patch brings the handling to be closer to member-like decls, with
one caveat. When we encounter reference to an enum constant we still
report an explicit reference to the particular enum constant, as
otherwise we might not see any references to the enum itself.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158515

Files:
  clang-tools-extra/include-cleaner/lib/WalkAST.cpp
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -309,6 +309,14 @@
 namespace ns { using ::$explicit^Foo; }
 template<> struct ns::Foo {};)cpp",
"ns::^Foo x;");
+  testWalk(R"cpp(
+namespace ns { enum class foo { bar }; }
+using ns::$implicit^foo;)cpp",
+   "auto x = foo::^bar;");
+  testWalk(R"cpp(
+namespace ns { enum foo { bar }; }
+using ns::foo::$explicit^bar;)cpp",
+   "auto x = ^bar;");
 }
 
 TEST(WalkAST, Using) {
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -126,13 +126,27 @@
   }
 
   bool VisitDeclRefExpr(DeclRefExpr *DRE) {
-// Static class members are handled here, as they don't produce 
MemberExprs.
-if (DRE->getFoundDecl()->isCXXClassMember()) {
-  if (auto *Qual = DRE->getQualifier())
-report(DRE->getLocation(), Qual->getAsRecordDecl(), RefType::Implicit);
-} else {
-  report(DRE->getLocation(), DRE->getFoundDecl());
+auto *FD = DRE->getFoundDecl();
+// For refs to non-meber-like decls, use the found decl.
+if (!FD->isCXXClassMember() && !llvm::isa(FD)) {
+  report(DRE->getLocation(), FD);
+  return true;
+}
+// For refs to member-like decls, report an implicit ref to the container.
+if (auto *Qual = DRE->getQualifier()) {
+  if (auto *Ty = Qual->getAsType()) {
+report(DRE->getLocation(), getMemberProvider(QualType(Ty, 0)),
+   RefType::Implicit);
+  }
+  return true;
 }
+// If the ref is without a qualifier, and is a member, ignore it. As it is
+// available in current context due to some other construct (e.g. base
+// specifiers, using decls) that has to spell the name explicitly.
+// If it's an enum constant, it must be due to prior decl. Report 
references
+// to it instead.
+if (llvm::isa(FD))
+  report(DRE->getLocation(), FD);
 return true;
   }
 


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -309,6 +309,14 @@
 namespace ns { using ::$explicit^Foo; }
 template<> struct ns::Foo {};)cpp",
"ns::^Foo x;");
+  testWalk(R"cpp(
+namespace ns { enum class foo { bar }; }
+using ns::$implicit^foo;)cpp",
+   "auto x = foo::^bar;");
+  testWalk(R"cpp(
+namespace ns { enum foo { bar }; }
+using ns::foo::$explicit^bar;)cpp",
+   "auto x = ^bar;");
 }
 
 TEST(WalkAST, Using) {
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -126,13 +126,27 @@
   }
 
   bool VisitDeclRefExpr(DeclRefExpr *DRE) {
-// Static class members are handled here, as they don't produce MemberExprs.
-if (DRE->getFoundDecl()->isCXXClassMember()) {
-  if (auto *Qual = DRE->getQualifier())
-report(DRE->getLocation(), Qual->getAsRecordDecl(), RefType::Implicit);
-} else {
-  report(DRE->getLocation(), DRE->getFoundDecl());
+auto *FD = DRE->getFoundDecl();
+// For refs to non-meber-like decls, use the found decl.
+if (!FD->isCXXClassMember() && !llvm::isa(FD)) {
+  report(DRE->getLocation(), FD);
+  return true;
+}
+// For refs to member-like decls, report an implicit ref to the container.
+if (auto *Qual = DRE->getQualifier()) {
+  if (auto *Ty = Qual->getAsType()) {
+report(DRE->getLocation(), getMemberProvider(QualType(Ty, 0)),
+   RefType::Implicit);
+  }
+  return true;
 }
+// If the ref is without a qualifier, and is a

[PATCH] D158293: [NFC][Clang] Fix static code analyzer concern about null value dereference

[Soumi Manna via Phabricator via cfe-commits]

Manna updated this revision to Diff 552347.
Manna retitled this revision from "[NFC][CLANG] Fix potential dereferencing of 
null return values" to "[NFC][Clang] Fix static code analyzer concern about 
null value dereference".
Manna added a comment.
Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, 
a.sidorin, baloghadamsoftware.

CurLexer is dereferenced and should not be null in 
clang::​Preprocessor::​SkipExcludedConditionalBlock(clang::​SourceLocation, 
clang::​SourceLocation, bool, bool, clang::​SourceLocation)

This patch adds  NULL value check for pointer CurLexer.


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

https://reviews.llvm.org/D158293

Files:
  clang/lib/Lex/PPDirectives.cpp


Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -547,7 +547,7 @@
   } SkippingRangeState(*this);
 
   while (true) {
-if (CurLexer->isDependencyDirectivesLexer()) {
+if (CurLexer && CurLexer->isDependencyDirectivesLexer()) {
   CurLexer->LexDependencyDirectiveTokenWhileSkipping(Tok);
 } else {
   SkippingRangeState.beginLexPass();


Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -547,7 +547,7 @@
   } SkippingRangeState(*this);
 
   while (true) {
-if (CurLexer->isDependencyDirectivesLexer()) {
+if (CurLexer && CurLexer->isDependencyDirectivesLexer()) {
   CurLexer->LexDependencyDirectiveTokenWhileSkipping(Tok);
 } else {
   SkippingRangeState.beginLexPass();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151567: [Support] Report EISDIR when opening a directory

[Alison Zhang via Phabricator via cfe-commits]

azhan92 updated this revision to Diff 552353.
azhan92 retitled this revision from "[Support] Report EISDIR when opening a 
directory " to "[Support] Report EISDIR when opening a directory".
azhan92 added a comment.

Add unit test to verify patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151567

Files:
  llvm/lib/Support/Unix/Path.inc
  llvm/test/tools/llvm-symbolizer/input-file-err.test
  llvm/unittests/Support/CommandLineTest.cpp
  llvm/unittests/Support/Path.cpp


Index: llvm/unittests/Support/Path.cpp
===
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -1287,6 +1287,22 @@
 }
 #endif
 
+TEST_F(FileSystemTest, OpenDirectoryAsFile) {
+  ASSERT_NO_ERROR(fs::create_directory(Twine(TestDirectory)));
+  ASSERT_EQ(fs::create_directory(Twine(TestDirectory), false),
+errc::file_exists);
+
+  int FD;
+  std::error_code EC;
+  EC = fs::openFileForRead(Twine(TestDirectory), FD);
+  ASSERT_EQ(EC, errc::is_a_directory);
+  EC = fs::openFileForWrite(Twine(TestDirectory), FD);
+  ASSERT_EQ(EC, errc::is_a_directory);
+
+  ASSERT_NO_ERROR(fs::remove_directories(Twine(TestDirectory)));
+  ::close(FD);
+}
+
 TEST_F(FileSystemTest, Remove) {
   SmallString<64> BaseDir;
   SmallString<64> Paths[4];
Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -1060,7 +1060,6 @@
   ASSERT_STREQ(Argv[0], "clang");
   ASSERT_STREQ(Argv[1], AFileExp.c_str());
 
-#if !defined(_AIX) && !defined(__MVS__)
   std::string ADirExp = std::string("@") + std::string(ADir.path());
   Argv = {"clang", ADirExp.c_str()};
   Res = cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv);
@@ -1068,7 +1067,6 @@
   ASSERT_EQ(2U, Argv.size());
   ASSERT_STREQ(Argv[0], "clang");
   ASSERT_STREQ(Argv[1], ADirExp.c_str());
-#endif
 }
 
 TEST(CommandLineTest, SetDefaultValue) {
Index: llvm/test/tools/llvm-symbolizer/input-file-err.test
===
--- llvm/test/tools/llvm-symbolizer/input-file-err.test
+++ llvm/test/tools/llvm-symbolizer/input-file-err.test
@@ -1,5 +1,3 @@
-Failing on AIX due to D153595. The test expects a different error message from 
the one given on AIX.
-XFAIL: target={{.*}}-aix{{.*}}
 RUN: not llvm-addr2line -e %p/Inputs/nonexistent 0x12 2>&1 | FileCheck %s 
--check-prefix=CHECK-NONEXISTENT-A2L -DMSG=%errc_ENOENT
 RUN: not llvm-addr2line -e %p/Inputs/nonexistent 2>&1 | FileCheck %s 
--check-prefix=CHECK-NONEXISTENT-A2L -DMSG=%errc_ENOENT
 CHECK-NONEXISTENT-A2L: llvm-addr2line{{.*}}: error: 
'{{.*}}Inputs/nonexistent': [[MSG]]
Index: llvm/lib/Support/Unix/Path.inc
===
--- llvm/lib/Support/Unix/Path.inc
+++ llvm/lib/Support/Unix/Path.inc
@@ -1019,6 +1019,13 @@
   auto Open = [&]() { return ::open(P.begin(), OpenFlags, Mode); };
   if ((ResultFD = sys::RetryAfterSignal(-1, Open)) < 0)
 return std::error_code(errno, std::generic_category());
+if (Access == FA_Read) {
+struct stat Status;
+if (fstat(ResultFD, &Status) == -1)
+  return std::error_code(errno, std::generic_category());
+if (S_ISDIR(Status.st_mode))
+  return make_error_code(errc::is_a_directory);
+}
 #ifndef O_CLOEXEC
   if (!(Flags & OF_ChildInherit)) {
 int r = fcntl(ResultFD, F_SETFD, FD_CLOEXEC);


Index: llvm/unittests/Support/Path.cpp
===
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -1287,6 +1287,22 @@
 }
 #endif
 
+TEST_F(FileSystemTest, OpenDirectoryAsFile) {
+  ASSERT_NO_ERROR(fs::create_directory(Twine(TestDirectory)));
+  ASSERT_EQ(fs::create_directory(Twine(TestDirectory), false),
+errc::file_exists);
+
+  int FD;
+  std::error_code EC;
+  EC = fs::openFileForRead(Twine(TestDirectory), FD);
+  ASSERT_EQ(EC, errc::is_a_directory);
+  EC = fs::openFileForWrite(Twine(TestDirectory), FD);
+  ASSERT_EQ(EC, errc::is_a_directory);
+
+  ASSERT_NO_ERROR(fs::remove_directories(Twine(TestDirectory)));
+  ::close(FD);
+}
+
 TEST_F(FileSystemTest, Remove) {
   SmallString<64> BaseDir;
   SmallString<64> Paths[4];
Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -1060,7 +1060,6 @@
   ASSERT_STREQ(Argv[0], "clang");
   ASSERT_STREQ(Argv[1], AFileExp.c_str());
 
-#if !defined(_AIX) && !defined(__MVS__)
   std::string ADirExp = std::string("@") + std::string(ADir.path());
   Argv = {"clang", ADirExp.c_str()};
   Res = cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv);
@@ -1068,7 +1067,6 @@
   ASSERT_

[clang-tools-extra] 8c21544 - [clangd] Bump timeouts for LSPServerTests

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2023-08-22T16:20:15+02:00
New Revision: 8c21544286a0cf5eba7df67e23e3b7265364e752

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

LOG: [clangd] Bump timeouts for LSPServerTests

We seem to be hitting limits in some windows build bots, see
https://github.com/clangd/clangd/issues/1712#issuecomment-1686478931.

So bumping the timeouts to 60 seconds and completely dropping them for sync
requests. As mentioned in the comment above, this should improve things,
considering even the tests that don't touch any complicated scheduler is
failing.

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

Added: 


Modified: 
clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
clang-tools-extra/clangd/unittests/LSPClient.cpp
clang-tools-extra/clangd/unittests/LSPClient.h

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp 
b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
index 4c6a238bbb261f..15d4be8dd9ba91 100644
--- a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -8,19 +8,38 @@
 
 #include "Annotations.h"
 #include "ClangdLSPServer.h"
+#include "ClangdServer.h"
+#include "ConfigProvider.h"
+#include "Diagnostics.h"
+#include "FeatureModule.h"
+#include "LSPBinder.h"
 #include "LSPClient.h"
-#include "Protocol.h"
 #include "TestFS.h"
+#include "support/Function.h"
 #include "support/Logger.h"
 #include "support/TestTracer.h"
+#include "support/Threading.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/FunctionExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/JSON.h"
+#include "llvm/Support/raw_ostream.h"
 #include "llvm/Testing/Support/Error.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
 #include 
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -358,7 +377,7 @@ TEST_F(LSPTest, FeatureModulesThreadingTest) {
   Client.notify("increment", nullptr);
   Client.notify("increment", nullptr);
   Client.notify("increment", nullptr);
-  EXPECT_THAT_EXPECTED(Client.call("sync", nullptr).take(), Succeeded());
+  Client.sync();
   EXPECT_EQ(3, FeatureModules.get()->getSync());
   // Throw some work on the queue to make sure shutdown blocks on it.
   Client.notify("increment", nullptr);

diff  --git a/clang-tools-extra/clangd/unittests/LSPClient.cpp 
b/clang-tools-extra/clangd/unittests/LSPClient.cpp
index 9361c0e29c91e7..4d8ba137e8c55d 100644
--- a/clang-tools-extra/clangd/unittests/LSPClient.cpp
+++ b/clang-tools-extra/clangd/unittests/LSPClient.cpp
@@ -12,21 +12,36 @@
 #include "Transport.h"
 #include "support/Logger.h"
 #include "support/Threading.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include "gtest/gtest.h"
 #include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
 #include 
 #include 
+#include 
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
 
 llvm::Expected clang::clangd::LSPClient::CallResult::take() 
{
   std::unique_lock Lock(Mu);
-  if (!clangd::wait(Lock, CV, timeoutSeconds(10),
+  static constexpr size_t TimeoutSecs = 60;
+  if (!clangd::wait(Lock, CV, timeoutSeconds(TimeoutSecs),
 [this] { return Value.has_value(); })) {
-ADD_FAILURE() << "No result from call after 10 seconds!";
+ADD_FAILURE() << "No result from call after " << TimeoutSecs << " 
seconds!";
 return llvm::json::Value(nullptr);
   }
   auto Res = std::move(*Value);

diff  --git a/clang-tools-extra/clangd/unittests/LSPClient.h 
b/clang-tools-extra/clangd/unittests/LSPClient.h
index be8bd42b84df13..3d459076321aca 100644
--- a/clang-tools-extra/clangd/unittests/LSPClient.h
+++ b/clang-tools-extra/clangd/unittests/LSPClient.h
@@ -9,12 +9,14 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_LSPCLIENT_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_LSPCLIENT_H
 
+#include "llvm/ADT/StringRef.h"
+#include 
 #include 
 #include 
-#include 
-#include 
+#include 
 #include 
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -32,7 +34,7 @@ class LSPClient {
   class CallResult {
   public:
 ~CallResult();
-// Blocks up to 10 seconds for the result to be ready.
+// Blocks up to 60 seconds for the result to be ready.
 // Records a test failure if there was no reply.
 llvm::Expected take

[PATCH] D158426: [clangd] Bump timeouts for LSPServerTests

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8c21544286a0: [clangd] Bump timeouts for LSPServerTests 
(authored by kadircet).

Changed prior to commit:
  https://reviews.llvm.org/D158426?vs=552284&id=552354#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158426

Files:
  clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
  clang-tools-extra/clangd/unittests/LSPClient.cpp
  clang-tools-extra/clangd/unittests/LSPClient.h

Index: clang-tools-extra/clangd/unittests/LSPClient.h
===
--- clang-tools-extra/clangd/unittests/LSPClient.h
+++ clang-tools-extra/clangd/unittests/LSPClient.h
@@ -9,12 +9,14 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_LSPCLIENT_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_LSPCLIENT_H
 
+#include "llvm/ADT/StringRef.h"
+#include 
 #include 
 #include 
-#include 
-#include 
+#include 
 #include 
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -32,7 +34,7 @@
   class CallResult {
   public:
 ~CallResult();
-// Blocks up to 10 seconds for the result to be ready.
+// Blocks up to 60 seconds for the result to be ready.
 // Records a test failure if there was no reply.
 llvm::Expected take();
 // Like take(), but records a test failure if the result was an error.
Index: clang-tools-extra/clangd/unittests/LSPClient.cpp
===
--- clang-tools-extra/clangd/unittests/LSPClient.cpp
+++ clang-tools-extra/clangd/unittests/LSPClient.cpp
@@ -12,21 +12,36 @@
 #include "Transport.h"
 #include "support/Logger.h"
 #include "support/Threading.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include "gtest/gtest.h"
 #include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
 #include 
 #include 
+#include 
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
 
 llvm::Expected clang::clangd::LSPClient::CallResult::take() {
   std::unique_lock Lock(Mu);
-  if (!clangd::wait(Lock, CV, timeoutSeconds(10),
+  static constexpr size_t TimeoutSecs = 60;
+  if (!clangd::wait(Lock, CV, timeoutSeconds(TimeoutSecs),
 [this] { return Value.has_value(); })) {
-ADD_FAILURE() << "No result from call after 10 seconds!";
+ADD_FAILURE() << "No result from call after " << TimeoutSecs << " seconds!";
 return llvm::json::Value(nullptr);
   }
   auto Res = std::move(*Value);
Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -8,19 +8,38 @@
 
 #include "Annotations.h"
 #include "ClangdLSPServer.h"
+#include "ClangdServer.h"
+#include "ConfigProvider.h"
+#include "Diagnostics.h"
+#include "FeatureModule.h"
+#include "LSPBinder.h"
 #include "LSPClient.h"
-#include "Protocol.h"
 #include "TestFS.h"
+#include "support/Function.h"
 #include "support/Logger.h"
 #include "support/TestTracer.h"
+#include "support/Threading.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/FunctionExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/JSON.h"
+#include "llvm/Support/raw_ostream.h"
 #include "llvm/Testing/Support/Error.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
 #include 
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -358,7 +377,7 @@
   Client.notify("increment", nullptr);
   Client.notify("increment", nullptr);
   Client.notify("increment", nullptr);
-  EXPECT_THAT_EXPECTED(Client.call("sync", nullptr).take(), Succeeded());
+  Client.sync();
   EXPECT_EQ(3, FeatureModules.get()->getSync());
   // Throw some work on the queue to make sure shutdown blocks on it.
   Client.notify("increment", nullptr);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158293: [NFC][Clang] Fix static code analyzer concern about null value dereference

[Soumi Manna via Phabricator via cfe-commits]

Manna added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2009-2010
(Line.MightBeFunctionDecl || Line.InPPDirective) &&
-   Current.NestingLevel == 0 &&
+   Current.NestingLevel == 0 && Current.Previous &&
!Current.Previous->isOneOf(tok::kw_operator, tok::identifier)) {
   // not auto operator->() -> xxx;

owenpan wrote:
> tahonermann wrote:
> > owenpan wrote:
> > > `Current.Previous` can't be null here because `AutoFound` is `true`.
> > Could you please elaborate on why you believe it is safe to move the check 
> > of `Current.Previous` inside the body of the `if` statement? Doing so will 
> > short circuit the remaining `else if` cases such that `Current.setType()` 
> > will not be called at all. It isn't obvious to me that those cases should 
> > not be considered if the previous token was not one of `kw_operator` or 
> > `identifier`. This looks like it has potential to change behavior.
> > 
> > The change that was originally proposed is clearly safe.
> > Could you please elaborate on why you believe it is safe to move the check 
> > of `Current.Previous` inside the body of the `if` statement? Doing so will 
> > short circuit the remaining `else if` cases such that `Current.setType()` 
> > will not be called at all. It isn't obvious to me that those cases should 
> > not be considered if the previous token was not one of `kw_operator` or 
> > `identifier`. This looks like it has potential to change behavior.
> 
> Ahh, you are right.
> 
> > The change that was originally proposed is clearly safe.
> 
> My point that `Previous` can't be null still stands. So we should either make 
> no changes here or add an assertion just before the `if` statement at line 
> 1991 above.
Thank you @owenpan and @tahonermann for reviews and feedbacks, I have removed 
my previous change since Current.Previous can't be null here.


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

https://reviews.llvm.org/D158293

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

[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics

[Mital Ashok via Phabricator via cfe-commits]

MitalAshok updated this revision to Diff 552356.
MitalAshok added a comment.

Use C's definition of type compatibility even in C++ mode (to not warn about 
enums promoted to their underlying types)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156054

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/FormatString.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CXX/drs/dr7xx.cpp
  clang/test/CodeGen/xcore-abi.c
  clang/test/Sema/format-pointer.c
  clang/test/Sema/format-strings-pedantic.c
  clang/test/Sema/varargs.c
  clang/test/SemaCXX/format-strings-0x-nopedantic.cpp
  clang/test/SemaCXX/varargs.cpp
  clang/www/cxx_dr_status.html

Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -4373,7 +4373,7 @@
 https://cplusplus.github.io/CWG/issues/722.html";>722
 CD2
 Can nullptr be passed to an ellipsis?
-Unknown
+Clang 18
   
   
 https://cplusplus.github.io/CWG/issues/726.html";>726
Index: clang/test/SemaCXX/varargs.cpp
===
--- clang/test/SemaCXX/varargs.cpp
+++ clang/test/SemaCXX/varargs.cpp
@@ -31,7 +31,7 @@
 // Ensure the correct behavior for promotable type UB checking.
 void promotable(int a, ...) {
   enum Unscoped1 { One = 0x7FFF };
-  (void)__builtin_va_arg(ap, Unscoped1); // ok
+  (void)__builtin_va_arg(ap, Unscoped1); // expected-warning {{second argument to 'va_arg' is of promotable type 'Unscoped1'; this va_arg has undefined behavior because arguments will be promoted to 'int'}}
 
   enum Unscoped2 { Two = 0x };
   (void)__builtin_va_arg(ap, Unscoped2); // ok
@@ -55,6 +55,24 @@
   (void)__builtin_va_arg(ap, unsigned int);
 
   (void)__builtin_va_arg(ap, bool); // expected-warning {{second argument to 'va_arg' is of promotable type 'bool'; this va_arg has undefined behavior because arguments will be promoted to 'int'}}
+
+  (void)__builtin_va_arg(ap, float); // expected-warning {{second argument to 'va_arg' is of promotable type 'float'; this va_arg has undefined behavior because arguments will be promoted to 'double'}}
+  (void)__builtin_va_arg(ap, __fp16); // expected-warning {{second argument to 'va_arg' is of promotable type '__fp16'; this va_arg has undefined behavior because arguments will be promoted to 'double'}}
+
+#if __cplusplus >= 201103L
+  (void)__builtin_va_arg(ap, decltype(nullptr)); // expected-warning {{second argument to 'va_arg' is of promotable type 'decltype(nullptr)' (aka 'std::nullptr_t'); this va_arg has undefined behavior because arguments will be promoted to 'void *'}}
+#endif
+
+  (void)__builtin_va_arg(ap, int[3]); // expected-warning {{second argument to 'va_arg' is of promotable type 'int[3]'; this va_arg has undefined behavior because arguments will be promoted to 'int *'}}
+  (void)__builtin_va_arg(ap, const int[3]); // expected-warning {{second argument to 'va_arg' is of promotable type 'const int[3]'; this va_arg has undefined behavior because arguments will be promoted to 'const int *'}}
+
+  // _BitInts aren't promoted
+  (void)__builtin_va_arg(ap, _BitInt(7));
+  (void)__builtin_va_arg(ap, unsigned _BitInt(7));
+  (void)__builtin_va_arg(ap, _BitInt(32));
+  (void)__builtin_va_arg(ap, unsigned _BitInt(32));
+  (void)__builtin_va_arg(ap, _BitInt(33));
+  (void)__builtin_va_arg(ap, unsigned _BitInt(33));
 }
 
 #if __cplusplus >= 201103L
Index: clang/test/SemaCXX/format-strings-0x-nopedantic.cpp
===
--- clang/test/SemaCXX/format-strings-0x-nopedantic.cpp
+++ clang/test/SemaCXX/format-strings-0x-nopedantic.cpp
@@ -5,6 +5,7 @@
 extern int printf(const char *restrict, ...);
 }
 
-void f(char *c) {
+void f(char *c, int *q) {
   printf("%p", c);
+  printf("%p", q);
 }
Index: clang/test/Sema/varargs.c
===
--- clang/test/Sema/varargs.c
+++ clang/test/Sema/varargs.c
@@ -75,6 +75,14 @@
 (void)__builtin_va_arg(args, enum E); // Don't warn here in C
 (void)__builtin_va_arg(args, short); // expected-warning {{second argument to 'va_arg' is of promotable type 'short'}}
 (void)__builtin_va_arg(args, char); // expected-warning {{second argument to 'va_arg' is of promotable type 'char'}}
+
+// _BitInts aren't promoted
+(void)__builtin_va_arg(args, _BitInt(7));
+(void)__builtin_va_arg(args, unsigned _BitInt(7));
+(void)__builtin_va_arg(args, _BitInt(32)); // OK
+(void)__builtin_va_arg(args, unsigned _BitInt(32)); // OK
+(void)__builtin_va_arg(args, _BitInt(33)); // OK
+(void)__builtin_va_arg(args, unsigned _BitInt(33)); // OK
 }
 
 void f10(int a, ...) {
@@ -121,3 +129,18 @@
   __builtin_va_list va;
   va_start(va, e);
 }
+
+void f15(__builtin_va_list args) {
+  (void)__builtin_va_a

[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics

[Mital Ashok via Phabricator via cfe-commits]

MitalAshok updated this revision to Diff 552357.
MitalAshok added a comment.

clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156054

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/FormatString.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CXX/drs/dr7xx.cpp
  clang/test/CodeGen/xcore-abi.c
  clang/test/Sema/format-pointer.c
  clang/test/Sema/format-strings-pedantic.c
  clang/test/Sema/varargs.c
  clang/test/SemaCXX/format-strings-0x-nopedantic.cpp
  clang/test/SemaCXX/varargs.cpp
  clang/www/cxx_dr_status.html

Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -4373,7 +4373,7 @@
 https://cplusplus.github.io/CWG/issues/722.html";>722
 CD2
 Can nullptr be passed to an ellipsis?
-Unknown
+Clang 18
   
   
 https://cplusplus.github.io/CWG/issues/726.html";>726
Index: clang/test/SemaCXX/varargs.cpp
===
--- clang/test/SemaCXX/varargs.cpp
+++ clang/test/SemaCXX/varargs.cpp
@@ -31,7 +31,7 @@
 // Ensure the correct behavior for promotable type UB checking.
 void promotable(int a, ...) {
   enum Unscoped1 { One = 0x7FFF };
-  (void)__builtin_va_arg(ap, Unscoped1); // ok
+  (void)__builtin_va_arg(ap, Unscoped1); // expected-warning {{second argument to 'va_arg' is of promotable type 'Unscoped1'; this va_arg has undefined behavior because arguments will be promoted to 'int'}}
 
   enum Unscoped2 { Two = 0x };
   (void)__builtin_va_arg(ap, Unscoped2); // ok
@@ -55,6 +55,24 @@
   (void)__builtin_va_arg(ap, unsigned int);
 
   (void)__builtin_va_arg(ap, bool); // expected-warning {{second argument to 'va_arg' is of promotable type 'bool'; this va_arg has undefined behavior because arguments will be promoted to 'int'}}
+
+  (void)__builtin_va_arg(ap, float); // expected-warning {{second argument to 'va_arg' is of promotable type 'float'; this va_arg has undefined behavior because arguments will be promoted to 'double'}}
+  (void)__builtin_va_arg(ap, __fp16); // expected-warning {{second argument to 'va_arg' is of promotable type '__fp16'; this va_arg has undefined behavior because arguments will be promoted to 'double'}}
+
+#if __cplusplus >= 201103L
+  (void)__builtin_va_arg(ap, decltype(nullptr)); // expected-warning {{second argument to 'va_arg' is of promotable type 'decltype(nullptr)' (aka 'std::nullptr_t'); this va_arg has undefined behavior because arguments will be promoted to 'void *'}}
+#endif
+
+  (void)__builtin_va_arg(ap, int[3]); // expected-warning {{second argument to 'va_arg' is of promotable type 'int[3]'; this va_arg has undefined behavior because arguments will be promoted to 'int *'}}
+  (void)__builtin_va_arg(ap, const int[3]); // expected-warning {{second argument to 'va_arg' is of promotable type 'const int[3]'; this va_arg has undefined behavior because arguments will be promoted to 'const int *'}}
+
+  // _BitInts aren't promoted
+  (void)__builtin_va_arg(ap, _BitInt(7));
+  (void)__builtin_va_arg(ap, unsigned _BitInt(7));
+  (void)__builtin_va_arg(ap, _BitInt(32));
+  (void)__builtin_va_arg(ap, unsigned _BitInt(32));
+  (void)__builtin_va_arg(ap, _BitInt(33));
+  (void)__builtin_va_arg(ap, unsigned _BitInt(33));
 }
 
 #if __cplusplus >= 201103L
Index: clang/test/SemaCXX/format-strings-0x-nopedantic.cpp
===
--- clang/test/SemaCXX/format-strings-0x-nopedantic.cpp
+++ clang/test/SemaCXX/format-strings-0x-nopedantic.cpp
@@ -5,6 +5,7 @@
 extern int printf(const char *restrict, ...);
 }
 
-void f(char *c) {
+void f(char *c, int *q) {
   printf("%p", c);
+  printf("%p", q);
 }
Index: clang/test/Sema/varargs.c
===
--- clang/test/Sema/varargs.c
+++ clang/test/Sema/varargs.c
@@ -75,6 +75,14 @@
 (void)__builtin_va_arg(args, enum E); // Don't warn here in C
 (void)__builtin_va_arg(args, short); // expected-warning {{second argument to 'va_arg' is of promotable type 'short'}}
 (void)__builtin_va_arg(args, char); // expected-warning {{second argument to 'va_arg' is of promotable type 'char'}}
+
+// _BitInts aren't promoted
+(void)__builtin_va_arg(args, _BitInt(7));
+(void)__builtin_va_arg(args, unsigned _BitInt(7));
+(void)__builtin_va_arg(args, _BitInt(32)); // OK
+(void)__builtin_va_arg(args, unsigned _BitInt(32)); // OK
+(void)__builtin_va_arg(args, _BitInt(33)); // OK
+(void)__builtin_va_arg(args, unsigned _BitInt(33)); // OK
 }
 
 void f10(int a, ...) {
@@ -121,3 +129,18 @@
   __builtin_va_list va;
   va_start(va, e);
 }
+
+void f15(__builtin_va_list args) {
+  (void)__builtin_va_arg(args, const int);
+  (void)__builtin_va_arg(args, const volatile int);
+  (void)__builtin_va_arg(args, v

[PATCH] D158227: [clang] EditedSource::applyRewrites - useless call

[Soumi Manna via Phabricator via cfe-commits]

Manna added a comment.

Thank you @aaron.ballman for reviews and feedbacks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158227

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

[PATCH] D158516: [clang][Interp] Only lazily visit constant globals

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

tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, shafik, cor3ntin.
Herald added a project: All.
tbaeder 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/D158516

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/test/AST/Interp/c.c


Index: clang/test/AST/Interp/c.c
===
--- clang/test/AST/Interp/c.c
+++ clang/test/AST/Interp/c.c
@@ -14,11 +14,15 @@
// pedantic-expected-warning {{not an integer 
constant expression}}
 _Static_assert(!!1, "");
 
-/// FIXME: Should also be rejected in the new interpreter
-int a = (1 == 1 ? 5 : 3);
+int a = (1 == 1 ? 5 : 3); // expected-note {{declared here}} \
+  // pedantic-expected-note {{declared here}}
 _Static_assert(a == 5, ""); // ref-error {{not an integral constant 
expression}} \
 // pedantic-ref-error {{not an integral constant 
expression}} \
-// pedantic-expected-warning {{not an integer 
constant expression}}
+// expected-error {{not an integral constant 
expression}} \
+// expected-note {{read of non-const variable}} \
+// pedantic-expected-error {{not an integral 
constant expression}} \
+// pedantic-expected-note {{read of non-const 
variable}}
+
 
 const int b = 3;
 _Static_assert(b == 3, ""); // pedantic-ref-warning {{not an integer constant 
expression}} \
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -2639,7 +2639,8 @@
   // This happens in C.
   if (!Ctx.getLangOpts().CPlusPlus) {
 if (const auto *VD = dyn_cast(D);
-VD && VD->hasGlobalStorage() && VD->getAnyInitializer()) {
+VD && VD->hasGlobalStorage() && VD->getAnyInitializer() &&
+VD->getType().isConstQualified()) {
   if (!this->visitVarDecl(VD))
 return false;
   // Retry.


Index: clang/test/AST/Interp/c.c
===
--- clang/test/AST/Interp/c.c
+++ clang/test/AST/Interp/c.c
@@ -14,11 +14,15 @@
// pedantic-expected-warning {{not an integer constant expression}}
 _Static_assert(!!1, "");
 
-/// FIXME: Should also be rejected in the new interpreter
-int a = (1 == 1 ? 5 : 3);
+int a = (1 == 1 ? 5 : 3); // expected-note {{declared here}} \
+  // pedantic-expected-note {{declared here}}
 _Static_assert(a == 5, ""); // ref-error {{not an integral constant expression}} \
 // pedantic-ref-error {{not an integral constant expression}} \
-// pedantic-expected-warning {{not an integer constant expression}}
+// expected-error {{not an integral constant expression}} \
+// expected-note {{read of non-const variable}} \
+// pedantic-expected-error {{not an integral constant expression}} \
+// pedantic-expected-note {{read of non-const variable}}
+
 
 const int b = 3;
 _Static_assert(b == 3, ""); // pedantic-ref-warning {{not an integer constant expression}} \
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -2639,7 +2639,8 @@
   // This happens in C.
   if (!Ctx.getLangOpts().CPlusPlus) {
 if (const auto *VD = dyn_cast(D);
-VD && VD->hasGlobalStorage() && VD->getAnyInitializer()) {
+VD && VD->hasGlobalStorage() && VD->getAnyInitializer() &&
+VD->getType().isConstQualified()) {
   if (!this->visitVarDecl(VD))
 return false;
   // Retry.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics

[Mital Ashok via Phabricator via cfe-commits]

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



Comment at: clang/test/SemaCXX/varargs.cpp:34
   enum Unscoped1 { One = 0x7FFF };
-  (void)__builtin_va_arg(ap, Unscoped1); // ok
+  (void)__builtin_va_arg(ap, Unscoped1); // expected-warning {{second argument 
to 'va_arg' is of promotable type 'Unscoped1'; this va_arg has undefined 
behavior because arguments will be promoted to 'int'}}
 

Unscoped1 is promoted to int when passed to a variadic function.

The underlying type for Unscoped1 is unsigned int, so only Unscoped1 and 
unsigned int are compatible, not Unscoped1 and int. An Unscoped1 passed to a 
variadic function must be retrieved via va_arg(ap, int).



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156054

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

[PATCH] D157684: [clang][ASTImporter] Repeated friend templates are partially imported

[Ding Fei via Phabricator via cfe-commits]

danix800 added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:4114
   SmallVector ImportedEquivalentFriends;
-
-  while (ImportedFriend) {
-bool Match = false;
-if (D->getFriendDecl() && ImportedFriend->getFriendDecl()) {
-  Match =
-  IsStructuralMatch(D->getFriendDecl(), 
ImportedFriend->getFriendDecl(),
-/*Complain=*/false);
-} else if (D->getFriendType() && ImportedFriend->getFriendType()) {
-  Match = Importer.IsStructurallyEquivalent(
-  D->getFriendType()->getType(),
-  ImportedFriend->getFriendType()->getType(), /*Complain=*/false);
-}
-if (Match)
+  for (auto *ImportedFriend : RD->friends())
+if (IsEquivalentFriend(Importer, D, ImportedFriend))

balazske wrote:
> `auto` should not be used here, this loop could be replaced by some generic 
> "algorithm" function call (`llvm::copy_if`).
There's no trivial predicate so the result code might be:
```
  llvm::copy_if(RD->friends(), std::back_inserter(ImportedEquivalentFriends),
[&](FriendDecl *ImportedFriend) {
  return IsEquivalentFriend(Importer, D, ImportedFriend);
});
```
which is not that 'intuitive' as the for-range version.

I'll pertain to the original one with `auto` replaced with explicit type. 
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157684

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

[PATCH] D158135: [Clang][CodeGen] Add __builtin_bcopy

[Carlos Eduardo Seo via Phabricator via cfe-commits]

cseo updated this revision to Diff 552370.
cseo added a comment.

Fix builtin definition and tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158135

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/AST/Decl.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/Analysis/bstring.c
  clang/test/Analysis/security-syntax-checks.m
  clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-macros.c

Index: clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-macros.c
===
--- clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-macros.c
+++ clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-macros.c
@@ -167,15 +167,15 @@
 }
 
 // 64BIT-LABEL: @testbcopy(
-// 64BIT: call void @bcopy(ptr noundef {{%.*}}, ptr noundef {{%.*}}, i64 noundef {{%.*}})
+// 64BIT: call void @llvm.memmove.p0.p0.i64(ptr align 1 {{%.*}}, ptr align 1 {{%.*}}, i64 {{%.*}}, i1 false)
 // 64BIT-NEXT:ret void
 //
 // 32BIT-LABEL: @testbcopy(
-// 32BIT: call void @bcopy(ptr noundef {{%.*}}, ptr noundef {{%.*}}, i32 noundef {{%.*}})
+// 32BIT: call void @llvm.memmove.p0.p0.i32(ptr align 1 {{%.*}}, ptr align 1 {{%.*}}, i32 {{%.*}}, i1 false)
 // 32BIT-NEXT:ret void
 //
 void testbcopy(const void *src, void *dest, size_t n) {
-  __bcopy(src, dest, n);
+  bcopy(src, dest, n);
 }
 
 // 64BIT-LABEL: @testbzero(
Index: clang/test/Analysis/security-syntax-checks.m
===
--- clang/test/Analysis/security-syntax-checks.m
+++ clang/test/Analysis/security-syntax-checks.m
@@ -77,9 +77,9 @@
 }
 
 // Obsolete function bcopy
-void bcopy(void *, void *, size_t);
+void bcopy(const void *, void *, size_t);
 
-void test_bcopy(void *a, void *b, size_t n) {
+void test_bcopy(const void *a, void *b, size_t n) {
   bcopy(a, b, n); // expected-warning{{The bcopy() function is obsoleted by memcpy() or memmove(}}
 }
 
Index: clang/test/Analysis/bstring.c
===
--- clang/test/Analysis/bstring.c
+++ clang/test/Analysis/bstring.c
@@ -483,8 +483,7 @@
 //===--===
 
 #define bcopy BUILTIN(bcopy)
-// __builtin_bcopy is not defined with const in Builtins.def.
-void bcopy(/*const*/ void *s1, void *s2, size_t n);
+void bcopy(const void *s1, void *s2, size_t n);
 
 
 void bcopy0 (void) {
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -3561,6 +3561,20 @@
 Builder.CreateMemSet(Dest, Builder.getInt8(0), SizeVal, false);
 return RValue::get(nullptr);
   }
+
+  case Builtin::BIbcopy:
+  case Builtin::BI__builtin_bcopy: {
+Address Dest = EmitPointerWithAlignment(E->getArg(1));
+Address Src = EmitPointerWithAlignment(E->getArg(0));
+Value *SizeVal = EmitScalarExpr(E->getArg(2));
+EmitNonNullArgCheck(RValue::get(Dest.getPointer()), E->getArg(1)->getType(),
+E->getArg(1)->getExprLoc(), FD, 0);
+EmitNonNullArgCheck(RValue::get(Src.getPointer()), E->getArg(0)->getType(),
+E->getArg(0)->getExprLoc(), FD, 1);
+Builder.CreateMemMove(Dest, Src, SizeVal, false);
+return RValue::get(Dest.getPointer());
+  }
+
   case Builtin::BImemcpy:
   case Builtin::BI__builtin_memcpy:
   case Builtin::BImempcpy:
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4356,6 +4356,10 @@
   case Builtin::BIbzero:
 return Builtin::BIbzero;
 
+  case Builtin::BI__builtin_bcopy:
+  case Builtin::BIbcopy:
+return Builtin::BIbcopy;
+
   case Builtin::BIfree:
 return Builtin::BIfree;
 
@@ -4387,6 +4391,8 @@
 return Builtin::BIstrlen;
   if (FnInfo->isStr("bzero"))
 return Builtin::BIbzero;
+  if (FnInfo->isStr("bcopy"))
+return Builtin::BIbcopy;
 } else if (isInStdNamespace()) {
   if (FnInfo->isStr("free"))
 return Builtin::BIfree;
Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -560,7 +560,7 @@
 BUILTIN(__builtin_stdarg_start, "vA.", "nt")
 BUILTIN(__builtin_assume_aligned, "v*vC*z.", "nctE")
 BUILTIN(__builtin_bcmp, "ivC*vC*z", "FnE")
-BUILTIN(__builtin_bcopy, "vv*v*z", "n")
+BUILTIN(__builtin_bcopy, "vvC*v*z", "nF")
 BUILTIN(__builtin_bzero, "vv*z", "nF")
 BUILTIN(__builtin_free, "vv*", "nF")
 BUILTIN(__builtin_malloc, "v*z", "nF")
@@ -1156,6 +1156,7 @@
 LIBBUILTIN(index, "c*cC*i",   "f", STRINGS_H, ALL_GNU_LANGUAGES)
 LIBBUILTIN(rindex, "c*cC*i",  "f", STRINGS_H, ALL_GNU_LANGUAGES)
 LIBBUILTIN(bzero, "vv*z",   

[PATCH] D155776: [NFC] Add checks for self-assignment.

[Tom Honermann via Phabricator via cfe-commits]

tahonermann accepted this revision.
tahonermann added a comment.

Thank you for following up, Sindhu! Looks good!


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

https://reviews.llvm.org/D155776

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

[PATCH] D158488: [NFC] Initialize member pointers to nullptr.

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added inline comments.



Comment at: clang/include/clang/AST/Stmt.h:2606-2607
   enum { INIT, CONDVAR, COND, INC, BODY, END_EXPR };
-  Stmt* SubExprs[END_EXPR]; // SubExprs[INIT] is an expression or declstmt.
+  Stmt *SubExprs[END_EXPR] = {
+  nullptr}; // SubExprs[INIT] is an expression or declstmt.
   SourceLocation LParenLoc, RParenLoc;

aaron.ballman wrote:
> I don't think this initialization is necessary. The constructor for `ForStmt` 
> initializes all of the valid elements: 
> https://github.com/llvm/llvm-project/blob/87c5a3e203ae3643bc13c9a13917b92a657f4358/clang/lib/AST/Stmt.cpp#L1024
> 
> The `EmptyShell` constructor does not initialize anything but that's because 
> it is piecemeal initialized by the AST importer, but all of its fields are 
> also initialized: 
> https://github.com/llvm/llvm-project/blob/87c5a3e203ae3643bc13c9a13917b92a657f4358/clang/lib/Serialization/ASTReaderStmt.cpp#L296
Declarations like this are common in the AST. I'm curious while static analysis 
flagged this one in particular. Perhaps it identified a path where one or more 
of the elements don't get initialized? If so, this would be a good find and a 
fix should be applied to that path.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158488

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

[PATCH] D157684: [clang][ASTImporter] Repeated friend templates are partially imported

[Ding Fei via Phabricator via cfe-commits]

danix800 updated this revision to Diff 552372.
danix800 added a comment.

1. Rebase;
2. Replace `auto` with explicit type;
3. Use local var without interrupting `Importer` state.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157684

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

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4054,6 +4054,25 @@
  ->lookup(ToRecordOfFriend->getDeclName())
  .empty());
   }
+
+  void testRepeatedFriendImport(const char *Code) {
+Decl *ToTu = getToTuDecl(Code, Lang_CXX03);
+Decl *FromTu = getTuDecl(Code, Lang_CXX03, "from.cc");
+
+auto *ToFriend1 = FirstDeclMatcher().match(ToTu, friendDecl());
+auto *ToFriend2 = LastDeclMatcher().match(ToTu, friendDecl());
+auto *FromFriend1 =
+FirstDeclMatcher().match(FromTu, friendDecl());
+auto *FromFriend2 =
+LastDeclMatcher().match(FromTu, friendDecl());
+
+FriendDecl *ToImportedFriend1 = Import(FromFriend1, Lang_CXX03);
+FriendDecl *ToImportedFriend2 = Import(FromFriend2, Lang_CXX03);
+
+EXPECT_NE(ToImportedFriend1, ToImportedFriend2);
+EXPECT_EQ(ToFriend1, ToImportedFriend1);
+EXPECT_EQ(ToFriend2, ToImportedFriend2);
+  }
 };
 
 TEST_P(ImportFriendClasses, ImportOfFriendRecordDoesNotMergeDefinition) {
@@ -4395,21 +4414,7 @@
 friend class X;
   };
   )";
-  Decl *ToTu = getToTuDecl(Code, Lang_CXX03);
-  Decl *FromTu = getTuDecl(Code, Lang_CXX03, "from.cc");
-
-  auto *ToFriend1 = FirstDeclMatcher().match(ToTu, friendDecl());
-  auto *ToFriend2 = LastDeclMatcher().match(ToTu, friendDecl());
-  auto *FromFriend1 =
-  FirstDeclMatcher().match(FromTu, friendDecl());
-  auto *FromFriend2 = LastDeclMatcher().match(FromTu, friendDecl());
-
-  FriendDecl *ToImportedFriend1 = Import(FromFriend1, Lang_CXX03);
-  FriendDecl *ToImportedFriend2 = Import(FromFriend2, Lang_CXX03);
-
-  EXPECT_NE(ToImportedFriend1, ToImportedFriend2);
-  EXPECT_EQ(ToFriend1, ToImportedFriend1);
-  EXPECT_EQ(ToFriend2, ToImportedFriend2);
+  testRepeatedFriendImport(Code);
 }
 
 TEST_P(ImportFriendClasses, ImportOfRepeatedFriendDecl) {
@@ -4420,21 +4425,31 @@
 friend void f();
   };
   )";
-  Decl *ToTu = getToTuDecl(Code, Lang_CXX03);
-  Decl *FromTu = getTuDecl(Code, Lang_CXX03, "from.cc");
-
-  auto *ToFriend1 = FirstDeclMatcher().match(ToTu, friendDecl());
-  auto *ToFriend2 = LastDeclMatcher().match(ToTu, friendDecl());
-  auto *FromFriend1 =
-  FirstDeclMatcher().match(FromTu, friendDecl());
-  auto *FromFriend2 = LastDeclMatcher().match(FromTu, friendDecl());
+  testRepeatedFriendImport(Code);
+}
 
-  FriendDecl *ToImportedFriend1 = Import(FromFriend1, Lang_CXX03);
-  FriendDecl *ToImportedFriend2 = Import(FromFriend2, Lang_CXX03);
+TEST_P(ImportFriendClasses, ImportOfRepeatedFriendFunctionTemplateDecl) {
+  const char *Code =
+  R"(
+template 
+class Container {
+  template  friend void m();
+  template  friend void m();
+};
+  )";
+  testRepeatedFriendImport(Code);
+}
 
-  EXPECT_NE(ToImportedFriend1, ToImportedFriend2);
-  EXPECT_EQ(ToFriend1, ToImportedFriend1);
-  EXPECT_EQ(ToFriend2, ToImportedFriend2);
+TEST_P(ImportFriendClasses, ImportOfRepeatedFriendClassTemplateDecl) {
+  const char *Code =
+  R"(
+template 
+class Container {
+  template  friend class X;
+  template  friend class X;
+};
+  )";
+  testRepeatedFriendImport(Code);
 }
 
 TEST_P(ASTImporterOptionSpecificTestBase, FriendFunInClassTemplate) {
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -4079,22 +4079,34 @@
   unsigned int IndexOfDecl;
 };
 
-template 
-static FriendCountAndPosition getFriendCountAndPosition(
-const FriendDecl *FD,
-llvm::function_ref GetCanTypeOrDecl) {
+static bool IsEquivalentFriend(ASTImporter &Importer, FriendDecl *FD1,
+   FriendDecl *FD2) {
+  if ((!FD1->getFriendType()) != (!FD2->getFriendType()))
+return false;
+
+  if (TypeSourceInfo *TSI = FD1->getFriendType())
+return Importer.IsStructurallyEquivalent(
+TSI->getType(), FD2->getFriendType()->getType(), /*Complain=*/false);
+
+  ASTImporter::NonEquivalentDeclSet NonEquivalentDecls;
+  StructuralEquivalenceContext Ctx(
+  FD1->getASTContext(), FD2->getASTContext(), NonEquivalentDecls,
+  StructuralEquivalenceKind::Default,
+  /* StrictTypeSpelling = */ false, /* Complain = */ false);
+  return Ctx.IsEquivalent(FD1, FD2);
+}
+
+static FriendCountAndPosition getFriendCountAndPosition(ASTImporter &Importer,
+   

[PATCH] D158507: [Flang][Driver] Add support for fomit-frame-pointer 1/2

[victorkingi via Phabricator via cfe-commits]

victorkingi updated this revision to Diff 552375.
victorkingi added a comment.

Added m64 visibility to flang in Options.td


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158507

Files:
  clang/include/clang/Driver/Options.td
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/fomit-frame-pointer.f90

Index: flang/test/Driver/fomit-frame-pointer.f90
===
--- /dev/null
+++ flang/test/Driver/fomit-frame-pointer.f90
@@ -0,0 +1,12 @@
+! Test no error emitted with 'fomit-frame-pointer' flag.
+
+! Test opt_record flags get generated for fc1
+! RUN: %flang -fomit-frame-pointer %s
+
+program forttest
+implicit none
+integer :: n
+
+n = 1
+
+end program forttest
Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -57,6 +57,7 @@
 ! HELP-NEXT: -fno-stack-arrays   Allocate array temporaries on the heap (default)
 ! HELP-NEXT: -fno-version-loops-for-stride
 ! HELP-NEXT: Do not create unit-strided loops (default)
+! HELP-NEXT: -fomit-frame-pointerOmit the frame pointer from functions that don't need it. Some stack unwinding cases, such as profilers and sanitizers, may prefer specifying -fno-omit-frame-pointer. On many targets, -O1 and higher omit the frame pointer by default. -m[no-]omit-leaf-frame-pointer takes precedence for leaf functions
 ! HELP-NEXT: -fopenacc   Enable OpenACC
 ! HELP-NEXT: -fopenmp-target-debug   Enable debugging in the OpenMP offloading device RTL
 ! HELP-NEXT: -fopenmp-targets=
@@ -219,6 +220,7 @@
 ! HELP-FC1-NEXT: -load  Load the named plugin (dynamic shared object)
 ! HELP-FC1-NEXT: -menable-no-infsAllow optimization to assume there are no infinities.
 ! HELP-FC1-NEXT: -menable-no-nansAllow optimization to assume there are no NaNs.
+! HELP-FC1-NEXT: -mframe-pointer= Specify which frame pointers to retain.
 ! HELP-FC1-NEXT: -mllvm   Additional arguments to forward to LLVM's option processing
 ! HELP-FC1-NEXT: -mmlir   Additional arguments to forward to MLIR's option processing
 ! HELP-FC1-NEXT: -module-dirPut MODULE files in 
Index: flang/test/Driver/driver-help-hidden.f90
===
--- flang/test/Driver/driver-help-hidden.f90
+++ flang/test/Driver/driver-help-hidden.f90
@@ -65,6 +65,7 @@
 ! CHECK-NEXT: -fno-stack-arrays   Allocate array temporaries on the heap (default)
 ! CHECK-NEXT: -fno-version-loops-for-stride
 ! CHECK-NEXT: Do not create unit-strided loops (default)
+! CHECK-NEXT: -fomit-frame-pointerOmit the frame pointer from functions that don't need it. Some stack unwinding cases, such as profilers and sanitizers, may prefer specifying -fno-omit-frame-pointer. On many targets, -O1 and higher omit the frame pointer by default. -m[no-]omit-leaf-frame-pointer takes precedence for leaf functions
 ! CHECK-NEXT: -fopenacc   Enable OpenACC
 ! CHECK-NEXT: -fopenmp-assume-no-nested-parallelism
 ! CHECK-NEXT: Assert no nested parallel regions in the GPU
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3061,7 +3061,8 @@
 def fno_objc_legacy_dispatch : Flag<["-"], "fno-objc-legacy-dispatch">, Group;
 def fno_objc_weak : Flag<["-"], "fno-objc-weak">, Group,
   Visibility<[ClangOption, CC1Option]>;
-def fno_omit_frame_pointer : Flag<["-"], "fno-omit-frame-pointer">, Group;
+def fno_omit_frame_pointer : Flag<["-"], "fno-omit-frame-pointer">, Group,
+  Visibility<[ClangOption, FlangOption]>;
 defm operator_names : BoolFOption<"operator-names",
   LangOpts<"CXXOperatorNames">, Default,
   NegFlag>;
 
 def fomit_frame_pointer : Flag<["-"], "fomit-frame-pointer">, Group,
+  Visibility<[ClangOption, FlangOption]>,
   HelpText<"Omit the frame pointer from functions that don't need it. "
   "Some stack unwinding cases, such as profilers and sanitizers, may prefer specifying -fno-omit-frame-pointer. "
   "On many targets, -O1 and higher omit the frame pointer by default. "
@@ -4154,7 +4156,7 @@
   HelpText<"Enable hexagon-qdsp6 backward compatibility">,
   MarshallingInfoFlag>;
 def m64 : Flag<["-"], "m64">, Group, Flags<[NoXarchOption]>,
-  Visibility<[ClangOption, CLOption, DXCOption]>;
+  Visibility<[ClangOption, CLOption, DXCOption, FlangOption]>;
 def maix64 : Flag<["-"], "maix64">, Group, Flags<[NoXarchOption]>;
 def mx32 : Flag<["-"], "mx32">, Group, Flags<[NoXarchOption]>,
   Visibility<[ClangOption, CLOption, DXCOption]>;
@@ -6615,10 +6617,6 @@
 def mdebug_pass : Separate<["-"], "mdebug-pass">,
  

[PATCH] D157747: Support Unicode Microsoft predefined macros

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D157747#4581933 , @cor3ntin wrote:

> This does not appear to be valid under MSVC and I don't think we should 
> support features that MS does not (Nor can i find any documentation that it 
> ought to work)
> https://godbolt.org/z/7Te3YYeb9

Agreed -- we typically do not want to make extensions to Microsoft's extensions.

> Supporting the same concatenation behavior MSVC does have seem like a cleaner 
> approach. We would not have an many new tokens.
> Before you do anything though. I'd like the feedback of more folks.
>
> I can't find any documentation for LPREFIX, but it seems in the general case, 
> it can be emulated that way https://godbolt.org/z/dr7MjMs99
>
> In terms of breaking change, it's only an issue if L/u/etc are existing macro 
> at the point of use.

It's a bit hard to reason about `__LPREFIX` given there's no documentation for 
it and it's not a macro that expands to anything we can poke at. However, 
https://github.com/llvm/llvm-project/issues/27402 shows there is interest in 
having Clang support it, so I weakly lean towards that option, though it may be 
more effort to reverse engineer how it works for various edge cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157747

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

[PATCH] D158519: [clangd] Move diagnostics from modules to main sources file

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin created this revision.
DmitryPolukhin added reviewers: adamcz, sammccall, kadircet.
DmitryPolukhin added a project: clang.
Herald added subscribers: arphaman, javed.absar.
Herald added a project: All.
DmitryPolukhin requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

After https://reviews.llvm.org/D85753 these diagnostics silently dropped
so the user may not have an idea what is wrong. This diff moves
diagnostics to the start of the main file same as it happens with
errors in the command line. Also added prefix give to make it clear that the
location is not original.

Test Plan: check-clangd


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158519

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/unittests/ModulesTests.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp


Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -1079,7 +1079,8 @@
   ElementsAre(AllOf(
   Field(&Diag::ID, Eq(diag::err_drv_unknown_argument)),
   Field(&Diag::Name, Eq("drv_unknown_argument")),
-  Field(&Diag::Message, "unknown argument: '-fsome-unknown-flag'";
+  Field(&Diag::Message,
+"in command line: unknown argument: '-fsome-unknown-flag'";
 }
 
 TEST_F(TUSchedulerTests, CommandLineWarnings) {
Index: clang-tools-extra/clangd/unittests/ModulesTests.cpp
===
--- clang-tools-extra/clangd/unittests/ModulesTests.cpp
+++ clang-tools-extra/clangd/unittests/ModulesTests.cpp
@@ -18,6 +18,12 @@
 namespace clangd {
 namespace {
 
+using ::testing::ElementsAre;
+
+MATCHER_P(diag, Desc, "") {
+  return llvm::StringRef(arg.Message).contains(Desc);
+}
+
 TEST(Modules, TextualIncludeInPreamble) {
   TestTU TU = TestTU::withCode(R"cpp(
 #include "Textual.h"
@@ -88,7 +94,13 @@
 )modulemap";
 
   // Test that we do not crash.
-  TU.build();
+  auto AST = TU.build();
+
+  // Test expected diagnostics.
+  EXPECT_THAT(AST.getDiagnostics(),
+  ElementsAre(diag("in implicitly built module: module M does not "
+   "depend on a module exporting 'non-modular.h'"),
+  diag("could not build module 'M'")));
 }
 
 // Unknown module formats are a fatal failure for clang. Ensure we don't crash.
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -672,14 +672,14 @@
 
 void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
   const clang::Diagnostic &Info) {
-  // If the diagnostic was generated for a different SourceManager, skip it.
+  bool FromModule = false;
+  // If the diagnostic was generated for a different SourceManager.
   // This happens when a module is imported and needs to be implicitly built.
   // The compilation of that module will use the same StoreDiags, but different
-  // SourceManager.
+  // SourceManager. Treat them as coming from command line.
   if (OrigSrcMgr && Info.hasSourceManager() &&
   OrigSrcMgr != &Info.getSourceManager()) {
-IgnoreDiagnostics::log(DiagLevel, Info);
-return;
+FromModule = true;
   }
 
   DiagnosticConsumer::HandleDiagnostic(DiagLevel, Info);
@@ -687,7 +687,7 @@
   Info.getDiags()->getDiagnosticIDs()->isDefaultMappingAsError(
   Info.getID());
 
-  if (Info.getLocation().isInvalid()) {
+  if (Info.getLocation().isInvalid() || FromModule) {
 // Handle diagnostics coming from command-line arguments. The source 
manager
 // is *not* available at this point, so we cannot use it.
 if (!OriginallyError) {
@@ -702,6 +702,11 @@
 LastDiagOriginallyError = OriginallyError;
 LastDiag->ID = Info.getID();
 fillNonLocationData(DiagLevel, Info, *LastDiag);
+// Update message to mention original location.
+const char *Prefix =
+FromModule ? "in implicitly built module" : "in command line";
+LastDiag->Message = llvm::formatv("{0}: {1}", Prefix, LastDiag->Message);
+
 LastDiag->InsideMainFile = true;
 // Put it at the start of the main file, for a lack of a better place.
 LastDiag->Range.start = Position{0, 0};


Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -1079,7 +1079,8 @@
   ElementsAre(AllOf(
   Field(&Diag::ID, Eq(diag::err_drv_unknown_argument)),
   Field(&Diag::Name, Eq("drv_unknown_ar

[PATCH] D158488: [NFC] Initialize member pointers to nullptr.

[Sindhu Chittireddy via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/AST/Stmt.h:2606-2607
   enum { INIT, CONDVAR, COND, INC, BODY, END_EXPR };
-  Stmt* SubExprs[END_EXPR]; // SubExprs[INIT] is an expression or declstmt.
+  Stmt *SubExprs[END_EXPR] = {
+  nullptr}; // SubExprs[INIT] is an expression or declstmt.
   SourceLocation LParenLoc, RParenLoc;

tahonermann wrote:
> aaron.ballman wrote:
> > I don't think this initialization is necessary. The constructor for 
> > `ForStmt` initializes all of the valid elements: 
> > https://github.com/llvm/llvm-project/blob/87c5a3e203ae3643bc13c9a13917b92a657f4358/clang/lib/AST/Stmt.cpp#L1024
> > 
> > The `EmptyShell` constructor does not initialize anything but that's 
> > because it is piecemeal initialized by the AST importer, but all of its 
> > fields are also initialized: 
> > https://github.com/llvm/llvm-project/blob/87c5a3e203ae3643bc13c9a13917b92a657f4358/clang/lib/Serialization/ASTReaderStmt.cpp#L296
> Declarations like this are common in the AST. I'm curious while static 
> analysis flagged this one in particular. Perhaps it identified a path where 
> one or more of the elements don't get initialized? If so, this would be a 
> good find and a fix should be applied to that path.
I will follow-up with this one more and open a different PR if necessary. 
Removing this change. 



Comment at: clang/include/clang/AST/Stmt.h:2606-2607
   enum { INIT, CONDVAR, COND, INC, BODY, END_EXPR };
-  Stmt* SubExprs[END_EXPR]; // SubExprs[INIT] is an expression or declstmt.
+  Stmt *SubExprs[END_EXPR] = {
+  nullptr}; // SubExprs[INIT] is an expression or declstmt.
   SourceLocation LParenLoc, RParenLoc;

schittir wrote:
> tahonermann wrote:
> > aaron.ballman wrote:
> > > I don't think this initialization is necessary. The constructor for 
> > > `ForStmt` initializes all of the valid elements: 
> > > https://github.com/llvm/llvm-project/blob/87c5a3e203ae3643bc13c9a13917b92a657f4358/clang/lib/AST/Stmt.cpp#L1024
> > > 
> > > The `EmptyShell` constructor does not initialize anything but that's 
> > > because it is piecemeal initialized by the AST importer, but all of its 
> > > fields are also initialized: 
> > > https://github.com/llvm/llvm-project/blob/87c5a3e203ae3643bc13c9a13917b92a657f4358/clang/lib/Serialization/ASTReaderStmt.cpp#L296
> > Declarations like this are common in the AST. I'm curious while static 
> > analysis flagged this one in particular. Perhaps it identified a path where 
> > one or more of the elements don't get initialized? If so, this would be a 
> > good find and a fix should be applied to that path.
> I will follow-up with this one more and open a different PR if necessary. 
> Removing this change. 
Thank you for this analysis!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158488

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

[PATCH] D158488: [NFC] Initialize member pointers to nullptr.

[Sindhu Chittireddy via Phabricator via cfe-commits]

schittir updated this revision to Diff 552380.
schittir added a comment.

Remove unnecessary initialization.


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

https://reviews.llvm.org/D158488

Files:
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp


Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -931,7 +931,7 @@
   (void)OutlinedFnID;
 
   // Return value of the runtime offloading call.
-  Value *Return;
+  Value *Return = nullptr;
 
   // Arguments for the target kernel.
   SmallVector ArgsVector;


Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -931,7 +931,7 @@
   (void)OutlinedFnID;
 
   // Return value of the runtime offloading call.
-  Value *Return;
+  Value *Return = nullptr;
 
   // Arguments for the target kernel.
   SmallVector ArgsVector;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158507: [Flang][Driver] Add support for fomit-frame-pointer 1/2

[victorkingi via Phabricator via cfe-commits]

victorkingi updated this revision to Diff 552381.
victorkingi added a comment.

revert addition of m64 visibility in Options.td


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158507

Files:
  clang/include/clang/Driver/Options.td
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/fomit-frame-pointer.f90


Index: flang/test/Driver/fomit-frame-pointer.f90
===
--- /dev/null
+++ flang/test/Driver/fomit-frame-pointer.f90
@@ -0,0 +1,12 @@
+! Test no error emitted with 'fomit-frame-pointer' flag.
+
+! Test opt_record flags get generated for fc1
+! RUN: %flang -fomit-frame-pointer %s
+
+program forttest
+implicit none
+integer :: n
+
+n = 1
+
+end program forttest
Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -57,6 +57,7 @@
 ! HELP-NEXT: -fno-stack-arrays   Allocate array temporaries on the heap 
(default)
 ! HELP-NEXT: -fno-version-loops-for-stride
 ! HELP-NEXT: Do not create unit-strided loops (default)
+! HELP-NEXT: -fomit-frame-pointerOmit the frame pointer from functions 
that don't need it. Some stack unwinding cases, such as profilers and 
sanitizers, may prefer specifying -fno-omit-frame-pointer. On many targets, -O1 
and higher omit the frame pointer by default. -m[no-]omit-leaf-frame-pointer 
takes precedence for leaf functions
 ! HELP-NEXT: -fopenacc   Enable OpenACC
 ! HELP-NEXT: -fopenmp-target-debug   Enable debugging in the OpenMP offloading 
device RTL
 ! HELP-NEXT: -fopenmp-targets=
@@ -219,6 +220,7 @@
 ! HELP-FC1-NEXT: -load  Load the named plugin (dynamic shared 
object)
 ! HELP-FC1-NEXT: -menable-no-infsAllow optimization to assume there 
are no infinities.
 ! HELP-FC1-NEXT: -menable-no-nansAllow optimization to assume there 
are no NaNs.
+! HELP-FC1-NEXT: -mframe-pointer= Specify which frame pointers to 
retain.
 ! HELP-FC1-NEXT: -mllvm   Additional arguments to forward to 
LLVM's option processing
 ! HELP-FC1-NEXT: -mmlir   Additional arguments to forward to 
MLIR's option processing
 ! HELP-FC1-NEXT: -module-dirPut MODULE files in 
Index: flang/test/Driver/driver-help-hidden.f90
===
--- flang/test/Driver/driver-help-hidden.f90
+++ flang/test/Driver/driver-help-hidden.f90
@@ -65,6 +65,7 @@
 ! CHECK-NEXT: -fno-stack-arrays   Allocate array temporaries on the heap 
(default)
 ! CHECK-NEXT: -fno-version-loops-for-stride
 ! CHECK-NEXT: Do not create unit-strided loops 
(default)
+! CHECK-NEXT: -fomit-frame-pointerOmit the frame pointer from functions 
that don't need it. Some stack unwinding cases, such as profilers and 
sanitizers, may prefer specifying -fno-omit-frame-pointer. On many targets, -O1 
and higher omit the frame pointer by default. -m[no-]omit-leaf-frame-pointer 
takes precedence for leaf functions
 ! CHECK-NEXT: -fopenacc   Enable OpenACC
 ! CHECK-NEXT: -fopenmp-assume-no-nested-parallelism
 ! CHECK-NEXT: Assert no nested parallel regions in the 
GPU
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3061,7 +3061,8 @@
 def fno_objc_legacy_dispatch : Flag<["-"], "fno-objc-legacy-dispatch">, 
Group;
 def fno_objc_weak : Flag<["-"], "fno-objc-weak">, Group,
   Visibility<[ClangOption, CC1Option]>;
-def fno_omit_frame_pointer : Flag<["-"], "fno-omit-frame-pointer">, 
Group;
+def fno_omit_frame_pointer : Flag<["-"], "fno-omit-frame-pointer">, 
Group,
+  Visibility<[ClangOption, FlangOption]>;
 defm operator_names : BoolFOption<"operator-names",
   LangOpts<"CXXOperatorNames">, Default,
   NegFlag>;
 
 def fomit_frame_pointer : Flag<["-"], "fomit-frame-pointer">, Group,
+  Visibility<[ClangOption, FlangOption]>,
   HelpText<"Omit the frame pointer from functions that don't need it. "
   "Some stack unwinding cases, such as profilers and sanitizers, may prefer 
specifying -fno-omit-frame-pointer. "
   "On many targets, -O1 and higher omit the frame pointer by default. "
@@ -6615,10 +6617,6 @@
 def mdebug_pass : Separate<["-"], "mdebug-pass">,
   HelpText<"Enable additional debug output">,
   MarshallingInfoString>;
-def mframe_pointer_EQ : Joined<["-"], "mframe-pointer=">,
-  HelpText<"Specify which frame pointers to retain.">, 
Values<"all,non-leaf,none">,
-  NormalizedValuesScope<"CodeGenOptions::FramePointerKind">, 
NormalizedValues<["All", "NonLeaf", "None"]>,
-  MarshallingInfoEnum, "None">;
 def mabi_EQ_ieeelongdouble : Flag<["-"], "mabi=ieeelongdouble">,
   HelpText<"U

[PATCH] D155858: Add a concept AST node.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Hi!  I was away on vacation, so I didn't get a chance to take a look at this.  
I'm currently digging my way through my 'back to work' tasks, but will keep 
this on my list of things to review ASAP.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155858

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

[PATCH] D155776: [NFC] Add checks for self-assignment.

[Sindhu Chittireddy via Phabricator via cfe-commits]

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

Thank you for the review, @aaron.ballman and @tahonermann 
Investigating the build failure...


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

https://reviews.llvm.org/D155776

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

[PATCH] D158285: [NFC][CLANG] Fix wrong orders of function arguments positions

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added inline comments.



Comment at: clang/lib/AST/Interp/InterpBlock.cpp:94-95
 
 DeadBlock::DeadBlock(DeadBlock *&Root, Block *Blk)
-: Root(Root), B(Blk->Desc, Blk->IsStatic, Blk->IsExtern, /*isDead=*/true) {
+: Root(Root), B(Blk->Desc, Blk->IsExtern, Blk->IsStatic, /*isDead=*/true) {
   // Add the block to the chain of dead blocks.

steakhal wrote:
> I think this is a TP.
> I find this unfortunate, that while all the ˙Block` ctors accepts the 
> parameters in the order how the backing fields are defined - except for the 
> `isDead` ctor overload, where the `IsExtern` and `IsStatic`.
> 
> To address this, I'd recommend not swapping the parameters at the callsite, 
> but rather fix the ctor overload to expect the parameters in the right order 
> as the rest of the ctors do. And of course, check all the callsites to this 
> weird constructor to fix them as well.
> 
> WDYT @tbaeder
I would be surprised if it makes sense for both `IsExtern` and `IsStatic` to be 
true. Perhaps `Block` could be modified to hold a single data member of type 
`StorageClass` that is asserted to be one of `SC_None`, `SC_Extern`, or 
`SC_Static`?



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1468-1477
   std::optional getRangeForNegatedSymSym(const SymSymExpr *SSE) {
 return getRangeForNegatedExpr(
 [SSE, State = this->State]() -> SymbolRef {
   if (SSE->getOpcode() == BO_Sub)
 return State->getSymbolManager().getSymSymExpr(
-SSE->getRHS(), BO_Sub, SSE->getLHS(), SSE->getType());
+SSE->getLHS(), BO_Sub, SSE->getRHS(), SSE->getType());
   return nullptr;

steakhal wrote:
> Now this is within my realm.
> This code applies the following transformation: `-(X - Y)  =>  (Y - X)` .
> Consequently, this is intentional.
Ideally, this change would have tripped up a test. Do we have tests that 
attempt to verify source locations such that one could be added? I know testing 
source locations is difficult and tedious.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158285

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

[PATCH] D103048: [IR] make -stack-alignment= into a module attr

[Ben Gamari via Phabricator via cfe-commits]

bgamari added a comment.
Herald added a project: All.

@nickdesaulniers, this appears to be missing the corresponding documentation in 
the LLVM language reference. As someone who maintains GHC 
's LLVM backend, this documentation is invaluable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103048

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

[PATCH] D158521: [Flang][Driver] Add visibility for Flang for m64 option

[victorkingi via Phabricator via cfe-commits]

victorkingi created this revision.
Herald added a reviewer: sscalpone.
Herald added a project: All.
victorkingi 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/D158521

Files:
  clang/include/clang/Driver/Options.td


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -4156,7 +4156,7 @@
   HelpText<"Enable hexagon-qdsp6 backward compatibility">,
   MarshallingInfoFlag>;
 def m64 : Flag<["-"], "m64">, Group, Flags<[NoXarchOption]>,
-  Visibility<[ClangOption, CLOption, DXCOption]>;
+  Visibility<[ClangOption, CLOption, DXCOption, FlangOption]>;
 def maix64 : Flag<["-"], "maix64">, Group, Flags<[NoXarchOption]>;
 def mx32 : Flag<["-"], "mx32">, Group, Flags<[NoXarchOption]>,
   Visibility<[ClangOption, CLOption, DXCOption]>;


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -4156,7 +4156,7 @@
   HelpText<"Enable hexagon-qdsp6 backward compatibility">,
   MarshallingInfoFlag>;
 def m64 : Flag<["-"], "m64">, Group, Flags<[NoXarchOption]>,
-  Visibility<[ClangOption, CLOption, DXCOption]>;
+  Visibility<[ClangOption, CLOption, DXCOption, FlangOption]>;
 def maix64 : Flag<["-"], "maix64">, Group, Flags<[NoXarchOption]>;
 def mx32 : Flag<["-"], "mx32">, Group, Flags<[NoXarchOption]>,
   Visibility<[ClangOption, CLOption, DXCOption]>;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158488: [NFC] Initialize member pointers to nullptr.

[Tom Honermann via Phabricator via cfe-commits]

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

Looks good to me.




Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:934
   // Return value of the runtime offloading call.
-  Value *Return;
+  Value *Return = nullptr;
 

aaron.ballman wrote:
> This is necessary to initialize because `emitTargetKernel()` has an early 
> return which does not initialize the passed reference to this object.
`Return` is passed to `Builder.CreateIsNotNull()`. I briefly inspected and it 
looks like it handles a null argument, so I agree this looks like the right fix.


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

https://reviews.llvm.org/D158488

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