[PATCH] D125418: [Arm64EC 6/?] Implement C/C++ mangling for Arm64EC function definitions.

2022-08-29 Thread chenglin.bi via Phabricator via cfe-commits
bcl5980 added a comment.

Another thing we need consider here is this case:

  #pragma pack(push, 1)
  struct b64 {
  char a[64];
  };
  #pragma pack(pop)
  
  typedef b64 (fptrtype)(int a);
  
  b64 f(void* p, int a) {
  return ((fptrtype*)p)(a);
  }

For now we generate exit_thunk with type void f(void* sret(b64) ret, int a)

  $iexit_thunk$cdecl$v$i8i8:  // @"$iexit_thunk$cdecl$v$i8i8"
  .seh_proc $iexit_thunk$cdecl$v$i8i8
  // %bb.0:
sub sp, sp, #48
.seh_stackalloc 48
stp x29, x30, [sp, #32] // 16-byte Folded Spill
.seh_save_fplr  32
add x29, sp, #32
.seh_add_fp 32
.seh_endprologue
mov w1, w0
mov x0, x8
adrpx8, __os_arm64x_dispatch_call_no_redirect
ldr x8, [x8, :lo12:__os_arm64x_dispatch_call_no_redirect]
blr x8
.seh_startepilogue
ldp x29, x30, [sp, #32] // 16-byte Folded Reload
.seh_save_fplr  32
add sp, sp, #48
.seh_stackalloc 48
.seh_endepilogue
ret
.seh_endfunclet
.seh_endproc
  // -- End function
.globl  f
.deff;
.scl2;
.type   32;
.endef

But it looks Microsoft generate exit thunk with type void* f(int a)

  |$iexit_thunk$cdecl$i8$i8| PROC
  |$LN2|
pacibsp
stp fp,lr,[sp,#-0x10]!
mov fp,sp
sub sp,sp,#0x20
adrpx8,__os_arm64x_dispatch_call_no_redirect
ldr xip0,[x8,__os_arm64x_dispatch_call_no_redirect]
blr xip0
mov x0,x8
add sp,sp,#0x20
ldp fp,lr,[sp],#0x10
autibsp
ret
  
ENDP  ; |$iexit_thunk$cdecl$i8$i8|

But based on clang x86 on Windows, we also generate the function type with void 
f(void* sret(b64) ret, int a).
It looks clang is different from MSVC even in x86 ABI.
Do we need to follow MSVC to generate $iexit_thunk$cdecl$i8$i8 ? Or just follow 
clang's ABI and ignore the difference?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125418

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


[PATCH] D132742: [X86][BF16] Add type mangling for Windows

2022-08-29 Thread Freddy, Ye via Phabricator via cfe-commits
FreddyYe accepted this revision.
FreddyYe 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/D132742/new/

https://reviews.llvm.org/D132742

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


[clang] 279ba53 - [Fuchsia][CMake] Disable LLVM plugin support

2022-08-29 Thread Petr Hosek via cfe-commits

Author: Petr Hosek
Date: 2022-08-29T07:26:01Z
New Revision: 279ba539a1dad609c8293b39a641d33814a2e2be

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

LOG: [Fuchsia][CMake] Disable LLVM plugin support

We already disable plugin support in Clang, disable LLVM side as well
since we don't support plugins in Fuchsia toolchain.

Added: 


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

Removed: 




diff  --git a/clang/cmake/caches/Fuchsia-stage2.cmake 
b/clang/cmake/caches/Fuchsia-stage2.cmake
index 566df27c1d1cc..d0bdeb3249797 100644
--- a/clang/cmake/caches/Fuchsia-stage2.cmake
+++ b/clang/cmake/caches/Fuchsia-stage2.cmake
@@ -14,6 +14,7 @@ set(LLVM_ENABLE_LIBEDIT OFF CACHE BOOL "")
 set(LLVM_ENABLE_LLD ON CACHE BOOL "")
 set(LLVM_ENABLE_LTO ON CACHE BOOL "")
 set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR ON CACHE BOOL "")
+set(LLVM_ENABLE_PLUGINS OFF CACHE BOOL "")
 set(LLVM_ENABLE_TERMINFO OFF CACHE BOOL "")
 set(LLVM_ENABLE_UNWIND_TABLES OFF CACHE BOOL "")
 set(LLVM_ENABLE_Z3_SOLVER OFF CACHE BOOL "")



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


[PATCH] D132030: [analyzer] Pass correct bldrCtx to computeObjectUnderConstruction

2022-08-29 Thread Tomasz Kamiński via Phabricator via cfe-commits
tomasz-kaminski-sonarsource updated this revision to Diff 456268.
tomasz-kaminski-sonarsource added a comment.

- Pass BldrCtx to handleConstructionContext


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132030

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  clang/test/Analysis/copy-elision.cpp

Index: clang/test/Analysis/copy-elision.cpp
===
--- clang/test/Analysis/copy-elision.cpp
+++ clang/test/Analysis/copy-elision.cpp
@@ -20,6 +20,7 @@
 #endif
 
 void clang_analyzer_eval(bool);
+void clang_analyzer_dump(int);
 
 namespace variable_functional_cast_crash {
 
@@ -418,3 +419,31 @@
 }
 
 } // namespace address_vector_tests
+
+namespace arg_directly_from_return_in_loop {
+
+struct Result {
+  int value;
+};
+
+Result create() {
+  return Result{10};
+}
+
+int accessValue(Result r) {
+  return r.value;
+}
+
+void test() {
+  for (int i = 0; i < 3; ++i) {
+int v = accessValue(create());
+if (i == 0) {
+  clang_analyzer_dump(v); // expected-warning {{10 S32b}}
+} else {
+  clang_analyzer_dump(v); // expected-warning {{10 S32b}}
+  // was {{reg_${{[0-9]+}} }} for C++11
+}
+  }
+}
+
+} // namespace arg_directly_from_return_in_loop
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -720,9 +720,9 @@
 SVal Target;
 assert(RTC->getStmt() == Call.getOriginExpr());
 EvalCallOptions CallOpts; // FIXME: We won't really need those.
-std::tie(State, Target) =
-handleConstructionContext(Call.getOriginExpr(), State, LCtx,
-  RTC->getConstructionContext(), CallOpts);
+std::tie(State, Target) = handleConstructionContext(
+Call.getOriginExpr(), State, currBldrCtx, LCtx,
+RTC->getConstructionContext(), CallOpts);
 const MemRegion *TargetR = Target.getAsRegion();
 assert(TargetR);
 // Invalidate the region so that it didn't look uninitialized. If this is
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -111,9 +111,15 @@
   return LValue;
 }
 
+// In case when the prvalue is returned from the function (kind is one of
+// SimpleReturnedValueKind, CXX17ElidedCopyReturnedValueKind), then
+// it's materialization happens in context of the caller.
+// We pass BldrCtx explicitly, as currBldrCtx always refers to callee's context.
 SVal ExprEngine::computeObjectUnderConstruction(
-const Expr *E, ProgramStateRef State, const LocationContext *LCtx,
-const ConstructionContext *CC, EvalCallOptions &CallOpts, unsigned Idx) {
+const Expr *E, ProgramStateRef State, const NodeBuilderContext *BldrCtx,
+const LocationContext *LCtx, const ConstructionContext *CC,
+EvalCallOptions &CallOpts, unsigned Idx) {
+
   SValBuilder &SVB = getSValBuilder();
   MemRegionManager &MRMgr = SVB.getRegionManager();
   ASTContext &ACtx = SVB.getContext();
@@ -209,8 +215,11 @@
   CallerLCtx = CallerLCtx->getParent();
   assert(!isa(CallerLCtx));
 }
+
+NodeBuilderContext CallerBldrCtx(getCoreEngine(),
+ SFC->getCallSiteBlock(), CallerLCtx);
 return computeObjectUnderConstruction(
-cast(SFC->getCallSite()), State, CallerLCtx,
+cast(SFC->getCallSite()), State, &CallerBldrCtx, CallerLCtx,
 RTC->getConstructionContext(), CallOpts);
   } else {
 // We are on the top frame of the analysis. We do not know where is the
@@ -250,7 +259,7 @@
   EvalCallOptions PreElideCallOpts = CallOpts;
 
   SVal V = computeObjectUnderConstruction(
-  TCC->getConstructorAfterElision(), State, LCtx,
+  TCC->getConstructorAfterElision(), State, BldrCtx, LCtx,
   TCC->getConstructionContextAfterElision(), CallOpts);
 
   // FIXME: This definition of "copy elision has not failed" is unreliable.
@@ -318,7 +327,7 @@
   CallEventManager &CEMgr = getStateManager().getCallEventManager();
   auto getArgLoc = [&](CallEventRef<> Caller) -> Optional {
 const LocationContext *FutureSFC =
-Caller->getCalleeStackFrame(currBldrCtx->blockCount());
+Caller->getCalleeStackFrame(BldrCtx->blockCount());
 // Return early if we are unable to reliably foresee
 // 

[PATCH] D132030: [analyzer] Pass correct bldrCtx to computeObjectUnderConstruction

2022-08-29 Thread Tomasz Kamiński via Phabricator via cfe-commits
tomasz-kaminski-sonarsource marked an inline comment as done.
tomasz-kaminski-sonarsource added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h:753
 
-SVal V = computeObjectUnderConstruction(E, State, LCtx, CC, CallOpts, Idx);
+SVal V = computeObjectUnderConstruction(E, State, currBldrCtx, LCtx, CC,
+CallOpts, Idx);

NoQ wrote:
> tomasz-kaminski-sonarsource wrote:
> > tomasz-kaminski-sonarsource wrote:
> > > NoQ wrote:
> > > > You probably want an updated builder context here as well. This 
> > > > function should be a simple wrapper, it should be completely 
> > > > interchangeable with calling both functions directly.
> > > Could you please elaborate more? I would see a reason to create a context 
> > > here if I would expect that `currBlrdCtx` refers to a different `Block` 
> > > that we want to perform construction in. And there is no indication on 
> > > another `Block` being inplay here, and I would construct `NodeBlockCtx` 
> > > with same block as `currBldrCtx`.
> > > In other words,  I expect this function to be `handeConstructionContext` 
> > > in current `Block`. 
> > Or to say it differently, I expect `BldCtx` not being `currBldrCtx` to be 
> > an unusual situation, that is limited to the construction of return value. 
> > Thus having it in `convenience` would only make it more currbesome.
> No-no, I mean, it's not any concrete bug that I see, it's maintaining API 
> contracts to make the code easy to understand and make it harder to make 
> mistakes in the future.
> 
> The function `handleConstructionContext()` , by design, is supposed to be 
> equivalent to calling `computeObjectUnderConstruction()` and 
> `updateObjectsUnderConstruction()`. Whenever someone has to perform both 
> steps, they can combine them together with this convenience wrapper.
> 
> After your patch, they won't be able to combine the two calls in a situation 
> when they need to pass a non-current builder context to 
> `computeObjectUnderConstruction()`. I hope they'll be able to figure out that 
> they need to add another parameter but I'm worried that they may think "oh 
> this big function doesn't need that parameter, let's drop it". Because the 
> bug you've found is quite subtle, it's easy to miss why we even need to pass 
> that parameter.
> 
> So I suggest to add the new parameter to `handleConstructionContext()` as 
> well, and pass it manually on all call sites.
Change applied.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132030

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


[PATCH] D130793: [clang-tidy] adjust treating of array-of-pointers when 'AnalyzePointers' is deactivated

2022-08-29 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-values.cpp:27-30
+  for (const int *p_local1 : p_local0) {
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'p_local1' of type 
'const int *' can be declared 'const'
+  // CHECK-FIXES: for (const int *const p_local1 : p_local0)
+  }

njames93 wrote:
> Wouldn't this behaviour be expected before the changes added in this patch, 
> as p_local1 isn't an array type.
the behaviour for `p_local1` does not change with the patch and is tested/moved 
here.

only `p_local0` has a behaviour change. this test file activates the analysis 
for `pointers-as-values`, so it must be diagnosed in this test file.
in the other test-file `-values.cpp` `p_local0` is not diagnosed, making it 
`np_local0`.

originally, the tests for the range based for loops lived in the `values` part 
of the tests. As this is incorrect for the pointer-arrays I moved them into 
this file instead (for pointer arrays).
the tests here mostly verify that the variables are properly matched and 
filtered. The actual analysis has its own unit tests for these cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130793

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


[PATCH] D132829: [clang][Interp] Handle ImplictValueInitExprs

2022-08-29 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, shafik, erichkeane, tahonermann.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

I don't have a test case handy for them since I'm not sure how to trigger them 
reliably, but they are easy enough to implement and I ran into them while 
working on array fillers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132829

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/Opcodes.td


Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -203,6 +203,7 @@
 // [] -> [Integer]
 def Zero : Opcode {
   let Types = [AluTypeClass];
+  let HasGroup = 1;
 }
 
 // [] -> [Pointer]
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -67,6 +67,7 @@
   bool VisitDeclRefExpr(const DeclRefExpr *E);
   bool VisitArraySubscriptExpr(const ArraySubscriptExpr *E);
   bool VisitInitListExpr(const InitListExpr *E);
+  bool VisitImplicitValueInitExpr(const ImplicitValueInitExpr *E);
 
 protected:
   bool visitExpr(const Expr *E) override;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -259,6 +259,14 @@
   return true;
 }
 
+template 
+bool ByteCodeExprGen::VisitImplicitValueInitExpr(const 
ImplicitValueInitExpr *E) {
+  if (Optional T = classify(E))
+return this->emitZero(*T, E);
+
+  return false;
+}
+
 template 
 bool ByteCodeExprGen::discard(const Expr *E) {
   OptionScope Scope(this, /*NewDiscardResult=*/true);


Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -203,6 +203,7 @@
 // [] -> [Integer]
 def Zero : Opcode {
   let Types = [AluTypeClass];
+  let HasGroup = 1;
 }
 
 // [] -> [Pointer]
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -67,6 +67,7 @@
   bool VisitDeclRefExpr(const DeclRefExpr *E);
   bool VisitArraySubscriptExpr(const ArraySubscriptExpr *E);
   bool VisitInitListExpr(const InitListExpr *E);
+  bool VisitImplicitValueInitExpr(const ImplicitValueInitExpr *E);
 
 protected:
   bool visitExpr(const Expr *E) override;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -259,6 +259,14 @@
   return true;
 }
 
+template 
+bool ByteCodeExprGen::VisitImplicitValueInitExpr(const ImplicitValueInitExpr *E) {
+  if (Optional T = classify(E))
+return this->emitZero(*T, E);
+
+  return false;
+}
+
 template 
 bool ByteCodeExprGen::discard(const Expr *E) {
   OptionScope Scope(this, /*NewDiscardResult=*/true);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] a845d8f - [X86][BF16] Add type mangling for Windows

2022-08-29 Thread Phoebe Wang via cfe-commits

Author: Phoebe Wang
Date: 2022-08-29T16:12:26+08:00
New Revision: a845d8fc57b6b09198bcb104f060925298034636

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

LOG: [X86][BF16] Add type mangling for Windows

Reviewed By: FreddyYe

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

Added: 


Modified: 
clang/lib/AST/MicrosoftMangle.cpp
clang/test/CodeGen/X86/bfloat-mangle.cpp

Removed: 




diff  --git a/clang/lib/AST/MicrosoftMangle.cpp 
b/clang/lib/AST/MicrosoftMangle.cpp
index 09075e60142a7..e58fedb6fa54c 100644
--- a/clang/lib/AST/MicrosoftMangle.cpp
+++ b/clang/lib/AST/MicrosoftMangle.cpp
@@ -2469,6 +2469,10 @@ void MicrosoftCXXNameMangler::mangleType(const 
BuiltinType *T, Qualifiers,
   Out << "$halff@";
 break;
 
+  case BuiltinType::BFloat16:
+mangleArtificialTagType(TTK_Struct, "__bf16", {"__clang"});
+break;
+
 #define SVE_TYPE(Name, Id, SingletonId) \
   case BuiltinType::Id:
 #include "clang/Basic/AArch64SVEACLETypes.def"
@@ -2501,7 +2505,6 @@ void MicrosoftCXXNameMangler::mangleType(const 
BuiltinType *T, Qualifiers,
   case BuiltinType::SatUShortFract:
   case BuiltinType::SatUFract:
   case BuiltinType::SatULongFract:
-  case BuiltinType::BFloat16:
   case BuiltinType::Ibm128:
   case BuiltinType::Float128: {
 DiagnosticsEngine &Diags = Context.getDiags();

diff  --git a/clang/test/CodeGen/X86/bfloat-mangle.cpp 
b/clang/test/CodeGen/X86/bfloat-mangle.cpp
index 2892a76d8d910..acc6c280f2e8e 100644
--- a/clang/test/CodeGen/X86/bfloat-mangle.cpp
+++ b/clang/test/CodeGen/X86/bfloat-mangle.cpp
@@ -1,5 +1,8 @@
-// RUN: %clang_cc1 -triple i386-unknown-unknown -target-feature +sse2 
-emit-llvm -o - %s | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -target-feature +sse2 
-emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple i386-unknown-unknown -target-feature +sse2 
-emit-llvm -o - %s | FileCheck %s --check-prefixes=LINUX
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -target-feature +sse2 
-emit-llvm -o - %s | FileCheck %s --check-prefixes=LINUX
+// RUN: %clang_cc1 -triple i386-windows-msvc -target-feature +sse2 -emit-llvm 
-o - %s | FileCheck %s --check-prefixes=WINDOWS
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -target-feature +sse2 
-emit-llvm -o - %s | FileCheck %s --check-prefixes=WINDOWS
 
-// CHECK: define {{.*}}void @_Z3foou6__bf16(bfloat noundef %b)
+// LINUX: define {{.*}}void @_Z3foou6__bf16(bfloat noundef %b)
+// WINDOWS: define {{.*}}void @"?foo@@YAXU__bf16@__clang@@@Z"(bfloat noundef 
%b)
 void foo(__bf16 b) {}



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


[PATCH] D132742: [X86][BF16] Add type mangling for Windows

2022-08-29 Thread Phoebe Wang via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa845d8fc57b6: [X86][BF16] Add type mangling for Windows 
(authored by pengfei).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132742

Files:
  clang/lib/AST/MicrosoftMangle.cpp
  clang/test/CodeGen/X86/bfloat-mangle.cpp


Index: clang/test/CodeGen/X86/bfloat-mangle.cpp
===
--- clang/test/CodeGen/X86/bfloat-mangle.cpp
+++ clang/test/CodeGen/X86/bfloat-mangle.cpp
@@ -1,5 +1,8 @@
-// RUN: %clang_cc1 -triple i386-unknown-unknown -target-feature +sse2 
-emit-llvm -o - %s | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -target-feature +sse2 
-emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple i386-unknown-unknown -target-feature +sse2 
-emit-llvm -o - %s | FileCheck %s --check-prefixes=LINUX
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -target-feature +sse2 
-emit-llvm -o - %s | FileCheck %s --check-prefixes=LINUX
+// RUN: %clang_cc1 -triple i386-windows-msvc -target-feature +sse2 -emit-llvm 
-o - %s | FileCheck %s --check-prefixes=WINDOWS
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -target-feature +sse2 
-emit-llvm -o - %s | FileCheck %s --check-prefixes=WINDOWS
 
-// CHECK: define {{.*}}void @_Z3foou6__bf16(bfloat noundef %b)
+// LINUX: define {{.*}}void @_Z3foou6__bf16(bfloat noundef %b)
+// WINDOWS: define {{.*}}void @"?foo@@YAXU__bf16@__clang@@@Z"(bfloat noundef 
%b)
 void foo(__bf16 b) {}
Index: clang/lib/AST/MicrosoftMangle.cpp
===
--- clang/lib/AST/MicrosoftMangle.cpp
+++ clang/lib/AST/MicrosoftMangle.cpp
@@ -2469,6 +2469,10 @@
   Out << "$halff@";
 break;
 
+  case BuiltinType::BFloat16:
+mangleArtificialTagType(TTK_Struct, "__bf16", {"__clang"});
+break;
+
 #define SVE_TYPE(Name, Id, SingletonId) \
   case BuiltinType::Id:
 #include "clang/Basic/AArch64SVEACLETypes.def"
@@ -2501,7 +2505,6 @@
   case BuiltinType::SatUShortFract:
   case BuiltinType::SatUFract:
   case BuiltinType::SatULongFract:
-  case BuiltinType::BFloat16:
   case BuiltinType::Ibm128:
   case BuiltinType::Float128: {
 DiagnosticsEngine &Diags = Context.getDiags();


Index: clang/test/CodeGen/X86/bfloat-mangle.cpp
===
--- clang/test/CodeGen/X86/bfloat-mangle.cpp
+++ clang/test/CodeGen/X86/bfloat-mangle.cpp
@@ -1,5 +1,8 @@
-// RUN: %clang_cc1 -triple i386-unknown-unknown -target-feature +sse2 -emit-llvm -o - %s | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -target-feature +sse2 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple i386-unknown-unknown -target-feature +sse2 -emit-llvm -o - %s | FileCheck %s --check-prefixes=LINUX
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -target-feature +sse2 -emit-llvm -o - %s | FileCheck %s --check-prefixes=LINUX
+// RUN: %clang_cc1 -triple i386-windows-msvc -target-feature +sse2 -emit-llvm -o - %s | FileCheck %s --check-prefixes=WINDOWS
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -target-feature +sse2 -emit-llvm -o - %s | FileCheck %s --check-prefixes=WINDOWS
 
-// CHECK: define {{.*}}void @_Z3foou6__bf16(bfloat noundef %b)
+// LINUX: define {{.*}}void @_Z3foou6__bf16(bfloat noundef %b)
+// WINDOWS: define {{.*}}void @"?foo@@YAXU__bf16@__clang@@@Z"(bfloat noundef %b)
 void foo(__bf16 b) {}
Index: clang/lib/AST/MicrosoftMangle.cpp
===
--- clang/lib/AST/MicrosoftMangle.cpp
+++ clang/lib/AST/MicrosoftMangle.cpp
@@ -2469,6 +2469,10 @@
   Out << "$halff@";
 break;
 
+  case BuiltinType::BFloat16:
+mangleArtificialTagType(TTK_Struct, "__bf16", {"__clang"});
+break;
+
 #define SVE_TYPE(Name, Id, SingletonId) \
   case BuiltinType::Id:
 #include "clang/Basic/AArch64SVEACLETypes.def"
@@ -2501,7 +2505,6 @@
   case BuiltinType::SatUShortFract:
   case BuiltinType::SatUFract:
   case BuiltinType::SatULongFract:
-  case BuiltinType::BFloat16:
   case BuiltinType::Ibm128:
   case BuiltinType::Float128: {
 DiagnosticsEngine &Diags = Context.getDiags();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D132742: [X86][BF16] Add type mangling for Windows

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

Thanks @FreddyYe


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132742

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


[PATCH] D132830: [clangd] Avoid crash when printing call to string literal operator template in hover

2022-08-29 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision.
nridge added reviewers: hokein, kadircet, sammccall.
Herald added a subscriber: arphaman.
Herald added a project: All.
nridge requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added projects: clang, clang-tools-extra.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132830

Files:
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang/lib/AST/StmtPrinter.cpp


Index: clang/lib/AST/StmtPrinter.cpp
===
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -1993,7 +1993,7 @@
   cast(DRE->getDecl())->getTemplateSpecializationArgs();
 assert(Args);
 
-if (Args->size() != 1) {
+if (Args->size() != 1 || Args->get(0).getKind() != TemplateArgument::Pack) 
{
   const TemplateParameterList *TPL = nullptr;
   if (!DRE->hadMultipleCandidates())
 if (const auto *TD = dyn_cast(DRE->getDecl()))
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -3200,6 +3200,24 @@
   EXPECT_EQ(*HI->Value, "&bar");
 }
 
+TEST(Hover, ClassNTTPNoCrash) {
+  Annotations T(R"cpp(
+  struct A {
+template  constexpr A(const char (&)[N]) : n(N) {}
+unsigned n;
+  };
+  template  constexpr auto operator""_a() { return a.n; }
+  constexpr auto w^aldo = "abc"_a;
+  )cpp");
+  TestTU TU = TestTU::withCode(T.code());
+  TU.ExtraArgs.push_back("-std=c++20");
+  auto AST = TU.build();
+  auto HI = getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
+  ASSERT_TRUE(HI);
+  EXPECT_EQ(HI->Definition, "constexpr auto waldo = operator\"\"_a<{4}>()");
+  EXPECT_EQ(*HI->Value, "4");
+}
+
 TEST(Hover, DisableShowAKA) {
   Annotations T(R"cpp(
 using m_int = int;


Index: clang/lib/AST/StmtPrinter.cpp
===
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -1993,7 +1993,7 @@
   cast(DRE->getDecl())->getTemplateSpecializationArgs();
 assert(Args);
 
-if (Args->size() != 1) {
+if (Args->size() != 1 || Args->get(0).getKind() != TemplateArgument::Pack) {
   const TemplateParameterList *TPL = nullptr;
   if (!DRE->hadMultipleCandidates())
 if (const auto *TD = dyn_cast(DRE->getDecl()))
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -3200,6 +3200,24 @@
   EXPECT_EQ(*HI->Value, "&bar");
 }
 
+TEST(Hover, ClassNTTPNoCrash) {
+  Annotations T(R"cpp(
+  struct A {
+template  constexpr A(const char (&)[N]) : n(N) {}
+unsigned n;
+  };
+  template  constexpr auto operator""_a() { return a.n; }
+  constexpr auto w^aldo = "abc"_a;
+  )cpp");
+  TestTU TU = TestTU::withCode(T.code());
+  TU.ExtraArgs.push_back("-std=c++20");
+  auto AST = TU.build();
+  auto HI = getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
+  ASSERT_TRUE(HI);
+  EXPECT_EQ(HI->Definition, "constexpr auto waldo = operator\"\"_a<{4}>()");
+  EXPECT_EQ(*HI->Value, "4");
+}
+
 TEST(Hover, DisableShowAKA) {
   Annotations T(R"cpp(
 using m_int = int;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D132831: [clang][Interp] Handle SubstNonTypeTemplateParmExprs

2022-08-29 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, shafik, erichkeane, tahonermann.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

These are also easy to add and used in a unit test that I'd like to get working 
with the new interpreter.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132831

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/test/AST/Interp/functions.cpp


Index: clang/test/AST/Interp/functions.cpp
===
--- clang/test/AST/Interp/functions.cpp
+++ clang/test/AST/Interp/functions.cpp
@@ -65,3 +65,11 @@
   return recursion(i);
 }
 static_assert(recursion(10) == 0, "");
+
+
+template
+constexpr decltype(N) getNum() {
+  return N;
+}
+static_assert(getNum<-2>() == -2, "");
+static_assert(getNum<10>() == 10, "");
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -68,6 +68,7 @@
   bool VisitArraySubscriptExpr(const ArraySubscriptExpr *E);
   bool VisitInitListExpr(const InitListExpr *E);
   bool VisitImplicitValueInitExpr(const ImplicitValueInitExpr *E);
+  bool VisitSubstNonTypeTemplateParmExpr(const SubstNonTypeTemplateParmExpr 
*E);
 
 protected:
   bool visitExpr(const Expr *E) override;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -267,6 +267,12 @@
   return false;
 }
 
+template 
+bool ByteCodeExprGen::VisitSubstNonTypeTemplateParmExpr(
+const SubstNonTypeTemplateParmExpr *E) {
+  return this->visit(E->getReplacement());
+}
+
 template 
 bool ByteCodeExprGen::discard(const Expr *E) {
   OptionScope Scope(this, /*NewDiscardResult=*/true);


Index: clang/test/AST/Interp/functions.cpp
===
--- clang/test/AST/Interp/functions.cpp
+++ clang/test/AST/Interp/functions.cpp
@@ -65,3 +65,11 @@
   return recursion(i);
 }
 static_assert(recursion(10) == 0, "");
+
+
+template
+constexpr decltype(N) getNum() {
+  return N;
+}
+static_assert(getNum<-2>() == -2, "");
+static_assert(getNum<10>() == 10, "");
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -68,6 +68,7 @@
   bool VisitArraySubscriptExpr(const ArraySubscriptExpr *E);
   bool VisitInitListExpr(const InitListExpr *E);
   bool VisitImplicitValueInitExpr(const ImplicitValueInitExpr *E);
+  bool VisitSubstNonTypeTemplateParmExpr(const SubstNonTypeTemplateParmExpr *E);
 
 protected:
   bool visitExpr(const Expr *E) override;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -267,6 +267,12 @@
   return false;
 }
 
+template 
+bool ByteCodeExprGen::VisitSubstNonTypeTemplateParmExpr(
+const SubstNonTypeTemplateParmExpr *E) {
+  return this->visit(E->getReplacement());
+}
+
 template 
 bool ByteCodeExprGen::discard(const Expr *E) {
   OptionScope Scope(this, /*NewDiscardResult=*/true);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D132832: [clang][Interp] Handle missing local initializers better

2022-08-29 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, shafik, erichkeane, tahonermann.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is illegal in a constexpr context. We can already figure that out,
but we'd still run into an assertion later on when trying to visit the
missing initializer or run the invalid function.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132832

Files:
  clang/lib/AST/Interp/ByteCodeEmitter.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/Function.h
  clang/test/AST/Interp/cxx20.cpp

Index: clang/test/AST/Interp/cxx20.cpp
===
--- clang/test/AST/Interp/cxx20.cpp
+++ clang/test/AST/Interp/cxx20.cpp
@@ -2,8 +2,6 @@
 // RUN: %clang_cc1 -std=c++20 -verify=ref %s
 
 
-// expected-no-diagnostics
-// ref-no-diagnostics
 constexpr int getMinus5() {
   int a = 10;
   a = -5;
@@ -53,3 +51,12 @@
   return v;
 }
 //static_assert(pointerAssign2() == 12, ""); TODO
+
+
+constexpr int unInitLocal() {
+  int a;
+  return a; // ref-note{{read of uninitialized object}}
+}
+static_assert(unInitLocal() == 0, ""); // expected-error {{not an integral constant expression}} \
+   // ref-error {{not an integral constant expression}} \
+   // ref-note {{in call to 'unInitLocal()'}}
Index: clang/lib/AST/Interp/Function.h
===
--- clang/lib/AST/Interp/Function.h
+++ clang/lib/AST/Interp/Function.h
@@ -112,6 +112,9 @@
   /// Checks if the function is a constructor.
   bool isConstructor() const { return isa(F); }
 
+  /// Checks if the function is fully done compiling.
+  bool isFullyCompiled() const { return IsFullyCompiled; }
+
 private:
   /// Construct a function representing an actual function.
   Function(Program &P, const FunctionDecl *F, unsigned ArgSize,
@@ -128,6 +131,8 @@
 IsValid = true;
   }
 
+  void setIsFullyCompiled(bool FC) { IsFullyCompiled = FC; }
+
 private:
   friend class Program;
   friend class ByteCodeEmitter;
@@ -154,6 +159,9 @@
   llvm::DenseMap Params;
   /// Flag to indicate if the function is valid.
   bool IsValid = false;
+  /// Flag to indicate if the function is done being
+  /// compile to bytecode.
+  bool IsFullyCompiled = false;
 
 public:
   /// Dumps the disassembled bytecode to \c llvm::errs().
Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -238,12 +238,17 @@
 
   // Integers, pointers, primitives.
   if (Optional T = this->classify(VD->getType())) {
+const Expr *Init = VD->getInit();
 auto Offset =
 this->allocateLocalPrimitive(VD, *T, VD->getType().isConstQualified());
+
+if (!Init)
+  return false;
+
 // Compile the initialiser in its own scope.
-{
+if (Init) {
   ExprScope Scope(this);
-  if (!this->visit(VD->getInit()))
+  if (!this->visit(Init))
 return false;
 }
 // Set the value.
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -699,6 +699,14 @@
 }
 assert(Func);
 
+// If the function is being compiled right now, this is a recursive call.
+// In that case, the function can't be valid yet, even though it will be
+// later.
+// If the function is already fully compiled but not constexpr, it was
+// found to be faulty earlier on, so bail out.
+if (Func->isFullyCompiled() && !Func->isConstexpr())
+  return false;
+
 QualType ReturnType = E->getCallReturnType(Ctx.getASTContext());
 Optional T = classify(ReturnType);
 
Index: clang/lib/AST/Interp/ByteCodeEmitter.cpp
===
--- clang/lib/AST/Interp/ByteCodeEmitter.cpp
+++ clang/lib/AST/Interp/ByteCodeEmitter.cpp
@@ -62,8 +62,10 @@
 // Return a dummy function if compilation failed.
 if (BailLocation)
   return llvm::make_error(*BailLocation);
-else
+else {
+  Func->setIsFullyCompiled(true);
   return Func;
+}
   } else {
 // Create scopes from descriptors.
 llvm::SmallVector Scopes;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D132833: [clangd] Fail more gracefully if QueryDriverDatabase cannot determine file type

2022-08-29 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision.
nridge added reviewers: kadircet, sammccall.
Herald added a subscriber: arphaman.
Herald added a project: All.
nridge requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Currently, QueryDriverDatabase returns an empty compile command
if it could not determine the file type.

This failure mode is unnecessarily destructive; it's better to
just return the incoming compiler command, which is still more
likely to be useful than an empty command.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132833

Files:
  clang-tools-extra/clangd/QueryDriverDatabase.cpp


Index: clang-tools-extra/clangd/QueryDriverDatabase.cpp
===
--- clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -341,7 +341,7 @@
   auto Type = driver::types::lookupTypeForExtension(Ext);
   if (Type == driver::types::TY_INVALID) {
 elog("System include extraction: invalid file type for {0}", Ext);
-return {};
+return Cmd;
   }
   Lang = driver::types::getTypeName(Type);
 }


Index: clang-tools-extra/clangd/QueryDriverDatabase.cpp
===
--- clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -341,7 +341,7 @@
   auto Type = driver::types::lookupTypeForExtension(Ext);
   if (Type == driver::types::TY_INVALID) {
 elog("System include extraction: invalid file type for {0}", Ext);
-return {};
+return Cmd;
   }
   Lang = driver::types::getTypeName(Type);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D132832: [clang][Interp] Handle missing local initializers better

2022-08-29 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 456281.

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

https://reviews.llvm.org/D132832

Files:
  clang/lib/AST/Interp/ByteCodeEmitter.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/Function.h
  clang/test/AST/Interp/cxx20.cpp

Index: clang/test/AST/Interp/cxx20.cpp
===
--- clang/test/AST/Interp/cxx20.cpp
+++ clang/test/AST/Interp/cxx20.cpp
@@ -2,8 +2,6 @@
 // RUN: %clang_cc1 -std=c++20 -verify=ref %s
 
 
-// expected-no-diagnostics
-// ref-no-diagnostics
 constexpr int getMinus5() {
   int a = 10;
   a = -5;
@@ -53,3 +51,12 @@
   return v;
 }
 //static_assert(pointerAssign2() == 12, ""); TODO
+
+
+constexpr int unInitLocal() {
+  int a;
+  return a; // ref-note{{read of uninitialized object}}
+}
+static_assert(unInitLocal() == 0, ""); // expected-error {{not an integral constant expression}} \
+   // ref-error {{not an integral constant expression}} \
+   // ref-note {{in call to 'unInitLocal()'}}
Index: clang/lib/AST/Interp/Function.h
===
--- clang/lib/AST/Interp/Function.h
+++ clang/lib/AST/Interp/Function.h
@@ -112,6 +112,9 @@
   /// Checks if the function is a constructor.
   bool isConstructor() const { return isa(F); }
 
+  /// Checks if the function is fully done compiling.
+  bool isFullyCompiled() const { return IsFullyCompiled; }
+
 private:
   /// Construct a function representing an actual function.
   Function(Program &P, const FunctionDecl *F, unsigned ArgSize,
@@ -128,6 +131,8 @@
 IsValid = true;
   }
 
+  void setIsFullyCompiled(bool FC) { IsFullyCompiled = FC; }
+
 private:
   friend class Program;
   friend class ByteCodeEmitter;
@@ -154,6 +159,9 @@
   llvm::DenseMap Params;
   /// Flag to indicate if the function is valid.
   bool IsValid = false;
+  /// Flag to indicate if the function is done being
+  /// compile to bytecode.
+  bool IsFullyCompiled = false;
 
 public:
   /// Dumps the disassembled bytecode to \c llvm::errs().
Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -238,12 +238,17 @@
 
   // Integers, pointers, primitives.
   if (Optional T = this->classify(VD->getType())) {
+const Expr *Init = VD->getInit();
 auto Offset =
 this->allocateLocalPrimitive(VD, *T, VD->getType().isConstQualified());
+
+if (!Init)
+  return false;
+
 // Compile the initialiser in its own scope.
-{
+if (Init) {
   ExprScope Scope(this);
-  if (!this->visit(VD->getInit()))
+  if (!this->visit(Init))
 return false;
 }
 // Set the value.
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -699,6 +699,14 @@
 }
 assert(Func);
 
+// If the function is being compiled right now, this is a recursive call.
+// In that case, the function can't be valid yet, even though it will be
+// later.
+// If the function is already fully compiled but not constexpr, it was
+// found to be faulty earlier on, so bail out.
+if (Func->isFullyCompiled() && !Func->isConstexpr())
+  return false;
+
 QualType ReturnType = E->getCallReturnType(Ctx.getASTContext());
 Optional T = classify(ReturnType);
 
Index: clang/lib/AST/Interp/ByteCodeEmitter.cpp
===
--- clang/lib/AST/Interp/ByteCodeEmitter.cpp
+++ clang/lib/AST/Interp/ByteCodeEmitter.cpp
@@ -62,8 +62,10 @@
 // Return a dummy function if compilation failed.
 if (BailLocation)
   return llvm::make_error(*BailLocation);
-else
+else {
+  Func->setIsFullyCompiled(true);
   return Func;
+}
   } else {
 // Create scopes from descriptors.
 llvm::SmallVector Scopes;
@@ -74,6 +76,7 @@
 // Set the function's code.
 Func->setCode(NextLocalOffset, std::move(Code), std::move(SrcMap),
   std::move(Scopes));
+Func->setIsFullyCompiled(true);
 return Func;
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

2022-08-29 Thread dongjunduo via Phabricator via cfe-commits
dongjunduo updated this revision to Diff 456282.
dongjunduo marked 2 inline comments as done.
dongjunduo added a comment.

Restyle variables' name and comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/check-time-trace.cpp
  clang/tools/driver/cc1_main.cpp

Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -256,17 +256,10 @@
   llvm::TimerGroup::clearAll();
 
   if (llvm::timeTraceProfilerEnabled()) {
-SmallString<128> Path(Clang->getFrontendOpts().OutputFile);
-llvm::sys::path::replace_extension(Path, "json");
-if (!Clang->getFrontendOpts().TimeTracePath.empty()) {
-  // replace the suffix to '.json' directly
-  SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
-  if (llvm::sys::fs::is_directory(TracePath))
-llvm::sys::path::append(TracePath, llvm::sys::path::filename(Path));
-  Path.assign(TracePath);
-}
+SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
+assert(!TracePath.empty() && "`-ftime-trace=` is empty");
 if (auto profilerOutput = Clang->createOutputFile(
-Path.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
+TracePath.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
 /*useTemporary=*/false)) {
   llvm::timeTraceProfilerWrite(*profilerOutput);
   profilerOutput.reset();
Index: clang/test/Driver/check-time-trace.cpp
===
--- clang/test/Driver/check-time-trace.cpp
+++ clang/test/Driver/check-time-trace.cpp
@@ -1,3 +1,8 @@
+// RUN: rm -rf %T/exe && mkdir %T/exe
+// RUN: %clangxx -ftime-trace -ftime-trace-granularity=0 -o %T/exe/check-time-trace %s
+// RUN: cat %T/exe/check-time-trace*.json \
+// RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
+// RUN:   | FileCheck %s
 // RUN: %clangxx -S -ftime-trace -ftime-trace-granularity=0 -o %T/check-time-trace %s
 // RUN: cat %T/check-time-trace.json \
 // RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4658,6 +4658,108 @@
   llvm_unreachable("invalid phase in ConstructPhaseAction");
 }
 
+// Infer data storing path of the options `-ftime-trace`, `-ftime-trace=`
+void InferTimeTracePath(Compilation &C) {
+  bool HasTimeTrace =
+  C.getArgs().getLastArg(options::OPT_ftime_trace) != nullptr;
+  bool HasTimeTraceFile =
+  C.getArgs().getLastArg(options::OPT_ftime_trace_EQ) != nullptr;
+  // Whether `-ftime-trace` or `-ftime-trace=` are specified
+  if (!HasTimeTrace && !HasTimeTraceFile)
+return;
+
+  // If `-ftime-trace=` is specified, TracePath is the .
+  // Else if there is a linking job, TracePath is the parent path of .exe,
+  // then the OutputFile's name may be appended to it.
+  // Else, TracePath is "",
+  // then the full OutputFile's path may be appended to it.
+  SmallString<128> TracePath("");
+
+  if (HasTimeTraceFile) {
+TracePath = SmallString<128>(
+C.getArgs().getLastArg(options::OPT_ftime_trace_EQ)->getValue());
+  } else {
+// Get linking executable file's parent path as TracePath's parent path,
+// default is ".". Filename may be determined and added into TracePath then.
+//
+// e.g. executable file's path: /usr/local/a.out
+//  its parent's path:  /usr/local
+for (auto &J : C.getJobs()) {
+  if (J.getSource().getKind() == Action::LinkJobClass) {
+assert(!J.getOutputFilenames().empty() &&
+   "linking output filename is empty");
+auto OutputFilePath =
+SmallString<128>(J.getOutputFilenames()[0].c_str());
+if (llvm::sys::path::has_parent_path(OutputFilePath)) {
+  TracePath = llvm::sys::path::parent_path(OutputFilePath);
+} else {
+  TracePath = SmallString<128>(".");
+}
+break;
+  }
+}
+  }
+
+  // Add or replace the modified -ftime-trace` to all clang jobs
+  for (auto &J : C.getJobs()) {
+if (J.getSource().getKind() == Action::AssembleJobClass ||
+J.getSource().getKind() == Action::BackendJobClass ||
+J.getSource().getKind() == Action::CompileJobClass) {
+  SmallString<128> TracePathReal = TracePath;
+  SmallString<128> OutputPath(J.getOutputFilenames()[0].c_str());
+  std::string arg = std::string("-ftime-trace=");
+  if (!HasTimeTraceFile) {
+if (TracePathReal.empty()) {
+  // /xxx/yyy.o => /xxx/yyy.json
+  llvm:

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

2022-08-29 Thread dongjunduo via Phabricator via cfe-commits
dongjunduo added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:4702
+
+  // Add or replace -ftime-trace` to the correct one to all clang jobs
+  for (auto &J : C.getJobs()) {

Whitney wrote:
> Do you mean Add or replace the modified `-ftime-trace=` to all clang 
> jobs?
Right



Comment at: clang/lib/Driver/Driver.cpp:4739
+
+  // replace `-ftime-trace=`
+  auto &JArgs = J.getArguments();

Whitney wrote:
> should we also replace `-ftime-trace`?
The work before here is to infer the correct path to store the time-trace file.

After that, the  in `-ftime-trace=` should be replaced by the 
infered correct path.

We do not need to replace `-ftime-trace` then.



Comment at: clang/test/Driver/check-time-trace.cpp:4
+// RUN: %clangxx -ftime-trace -ftime-trace-granularity=0 -o 
%T/exe/check-time-trace %s
+// RUN: cat %T/exe/check-time-trace*.json \
+// RUN:   | %python -c 'import json, sys; 
json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \

Whitney wrote:
> what may be between `check-time-trace` and `.json`?
If we use `-ftime-trace` but no `-ftime-trace=` to compile the source 
like "check-time-trace.cpp" to a executable file `check-time-trace`, the 
`check-time-trace.cpp` should be compiled to 
`check-time-trace-[random-string].o`, then linked to the `check-time-trace` by 
the linker. This random string is introduced by clang's own default logic.

The `-ftime-trace` records the time cost details of compilng source file to the 
object file (.cpp -> .o). If the time-trace file name isn't be specified, its 
default name is [object file's name].json, which is corresponding the object 
file.

Demo:

Cmd: `clang++ -ftime-trace -ftime-trace-granularity=0 -o check-time-trace 
check-time-trace.cpp`
Output: `check-time-trace`, `check-time-trace-a40601.json`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

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


[PATCH] D132661: [clang] Make guard(nocf) attribute available only for Windows

2022-08-29 Thread Martin Storsjö via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG00d648bdb5a8: [clang] Make guard(nocf) attribute available 
only for Windows (authored by alvinhochun, committed by mstorsjo).

Changed prior to commit:
  https://reviews.llvm.org/D132661?vs=455820&id=456283#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132661

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Attr.td
  clang/test/Sema/attr-guard_nocf.c


Index: clang/test/Sema/attr-guard_nocf.c
===
--- clang/test/Sema/attr-guard_nocf.c
+++ clang/test/Sema/attr-guard_nocf.c
@@ -2,10 +2,14 @@
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify -std=c++11 
-fsyntax-only -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -verify -fsyntax-only %s
 // RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -verify -std=c++11 
-fsyntax-only -x c++ %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -verify -std=c++11 
-fsyntax-only -x c++ %s
 
 // The x86_64-w64-windows-gnu version tests mingw target, which relies on
 // __declspec(...) being defined as __attribute__((...)) by compiler built-in.
 
+#if defined(_WIN32)
+
 // Function definition.
 __declspec(guard(nocf)) void testGuardNoCF(void) { // no warning
 }
@@ -35,3 +39,9 @@
 // 'guard' Attribute argument must be a supported identifier.
 __declspec(guard(cf)) void testGuardNoCFInvalidParam(void) { // 
expected-warning {{'guard' attribute argument not supported: 'cf'}}
 }
+
+#else
+
+__attribute((guard(nocf))) void testGNUStyleGuardNoCF(void) {} // 
expected-warning {{unknown attribute 'guard' ignored}}
+
+#endif
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -399,6 +399,9 @@
 def TargetX86 : TargetArch<["x86"]>;
 def TargetAnyX86 : TargetArch<["x86", "x86_64"]>;
 def TargetWebAssembly : TargetArch<["wasm32", "wasm64"]>;
+def TargetWindows : TargetSpec {
+  let OSes = ["Win32"];
+}
 def TargetHasDLLImportExport : TargetSpec {
   let CustomCode = [{ Target.getTriple().hasDLLImportExport() }];
 }
@@ -3494,7 +3497,7 @@
   let Documentation = [MSAllocatorDocs];
 }
 
-def CFGuard : InheritableAttr {
+def CFGuard : InheritableAttr, TargetSpecificAttr {
   // Currently only the __declspec(guard(nocf)) modifier is supported. In 
future
   // we might also want to support __declspec(guard(suppress)).
   let Spellings = [Declspec<"guard">, Clang<"guard">];
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -146,7 +146,8 @@
   ``[[clang::guard(nocf)]]``, which is equivalent to 
``__declspec(guard(nocf))``
   when using the MSVC environment. This is to support enabling Windows Control
   Flow Guard checks with the ability to disable them for specific functions 
when
-  using the MinGW environment.
+  using the MinGW environment. This attribute is only available for Windows
+  targets.
 
 Windows Support
 ---


Index: clang/test/Sema/attr-guard_nocf.c
===
--- clang/test/Sema/attr-guard_nocf.c
+++ clang/test/Sema/attr-guard_nocf.c
@@ -2,10 +2,14 @@
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify -std=c++11 -fsyntax-only -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -verify -fsyntax-only %s
 // RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -verify -std=c++11 -fsyntax-only -x c++ %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -verify -std=c++11 -fsyntax-only -x c++ %s
 
 // The x86_64-w64-windows-gnu version tests mingw target, which relies on
 // __declspec(...) being defined as __attribute__((...)) by compiler built-in.
 
+#if defined(_WIN32)
+
 // Function definition.
 __declspec(guard(nocf)) void testGuardNoCF(void) { // no warning
 }
@@ -35,3 +39,9 @@
 // 'guard' Attribute argument must be a supported identifier.
 __declspec(guard(cf)) void testGuardNoCFInvalidParam(void) { // expected-warning {{'guard' attribute argument not supported: 'cf'}}
 }
+
+#else
+
+__attribute((guard(nocf))) void testGNUStyleGuardNoCF(void) {} // expected-warning {{unknown attribute 'guard' ignored}}
+
+#endif
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -399,6 +399,9 @@
 def TargetX86 : TargetArch<["x86"]>;
 def TargetAnyX86 : TargetArch<["x86", "x86_64"]>;
 def TargetWebAssembly : TargetArch<["wasm32", "wasm64"]>;
+def TargetWindows : TargetSpec {
+  let OSes = ["Win

[clang] 00d648b - [clang] Make guard(nocf) attribute available only for Windows

2022-08-29 Thread Martin Storsjö via cfe-commits

Author: Alvin Wong
Date: 2022-08-29T11:30:44+03:00
New Revision: 00d648bdb5a8b71785269b4851b651c883de2cd9

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

LOG: [clang] Make guard(nocf) attribute available only for Windows

Control Flow Guard is only supported on Windows target, therefore there
is no point to make it an accepted attribute for other targets.

Reviewed By: rnk, aaron.ballman

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/Attr.td
clang/test/Sema/attr-guard_nocf.c

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b19a81e848387..cdcb6007ab836 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -146,7 +146,8 @@ Attribute Changes in Clang
   ``[[clang::guard(nocf)]]``, which is equivalent to 
``__declspec(guard(nocf))``
   when using the MSVC environment. This is to support enabling Windows Control
   Flow Guard checks with the ability to disable them for specific functions 
when
-  using the MinGW environment.
+  using the MinGW environment. This attribute is only available for Windows
+  targets.
 
 Windows Support
 ---

diff  --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index f962f84f3b7a7..3b0e3e2971803 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -399,6 +399,9 @@ def TargetRISCV : TargetArch<["riscv32", "riscv64"]>;
 def TargetX86 : TargetArch<["x86"]>;
 def TargetAnyX86 : TargetArch<["x86", "x86_64"]>;
 def TargetWebAssembly : TargetArch<["wasm32", "wasm64"]>;
+def TargetWindows : TargetSpec {
+  let OSes = ["Win32"];
+}
 def TargetHasDLLImportExport : TargetSpec {
   let CustomCode = [{ Target.getTriple().hasDLLImportExport() }];
 }
@@ -3494,7 +3497,7 @@ def MSAllocator : InheritableAttr {
   let Documentation = [MSAllocatorDocs];
 }
 
-def CFGuard : InheritableAttr {
+def CFGuard : InheritableAttr, TargetSpecificAttr {
   // Currently only the __declspec(guard(nocf)) modifier is supported. In 
future
   // we might also want to support __declspec(guard(suppress)).
   let Spellings = [Declspec<"guard">, Clang<"guard">];

diff  --git a/clang/test/Sema/attr-guard_nocf.c 
b/clang/test/Sema/attr-guard_nocf.c
index 7d7708e766473..c40884f45b7b7 100644
--- a/clang/test/Sema/attr-guard_nocf.c
+++ b/clang/test/Sema/attr-guard_nocf.c
@@ -2,10 +2,14 @@
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify -std=c++11 
-fsyntax-only -x c++ %s
 // RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -verify -fsyntax-only %s
 // RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -verify -std=c++11 
-fsyntax-only -x c++ %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -verify -std=c++11 
-fsyntax-only -x c++ %s
 
 // The x86_64-w64-windows-gnu version tests mingw target, which relies on
 // __declspec(...) being defined as __attribute__((...)) by compiler built-in.
 
+#if defined(_WIN32)
+
 // Function definition.
 __declspec(guard(nocf)) void testGuardNoCF(void) { // no warning
 }
@@ -35,3 +39,9 @@ __declspec(guard(nocf, nocf)) void 
testGuardNoCFTooManyParams(void) { // expecte
 // 'guard' Attribute argument must be a supported identifier.
 __declspec(guard(cf)) void testGuardNoCFInvalidParam(void) { // 
expected-warning {{'guard' attribute argument not supported: 'cf'}}
 }
+
+#else
+
+__attribute((guard(nocf))) void testGNUStyleGuardNoCF(void) {} // 
expected-warning {{unknown attribute 'guard' ignored}}
+
+#endif



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


[PATCH] D132810: [clang][MinGW] Add `-mguard=cf` and `-mguard=cf-nochecks`

2022-08-29 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment.

Looks reasonable to me, but I'd appreciate input from people more familiar with 
adding new options to the GCC style driver about option naming.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132810

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


[clang] c04eab8 - [Flang] Use find_program() to find clang-tblgen

2022-08-29 Thread Nikita Popov via cfe-commits

Author: Nikita Popov
Date: 2022-08-29T11:09:25+02:00
New Revision: c04eab8c78e517210c7641551ec008b09bfe20d0

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

LOG: [Flang] Use find_program() to find clang-tblgen

There are two scenarios here:

1. Standalone flang build, where we use an installed clang-tblgen
   binary. We need to use find_package() to find it.
2. Combined build of clang and flang, where we want to use the
   path specified in CLANG_TABLEGEN_EXE during the clang build --
   however, this variable was previously not exported.

The new implementation matches what is done for mlir-tblgen.

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

Added: 


Modified: 
clang/CMakeLists.txt
flang/docs/CMakeLists.txt

Removed: 




diff  --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index 2b1e968da3960..1064cfd0a35a3 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -484,6 +484,9 @@ option(CLANG_INCLUDE_TESTS
 
 add_subdirectory(utils/TableGen)
 
+# Export CLANG_TABLEGEN_EXE for use by flang docs.
+set(CLANG_TABLEGEN_EXE "${CLANG_TABLEGEN_EXE}" CACHE INTERNAL "")
+
 add_subdirectory(include)
 
 # All targets below may depend on all tablegen'd files.

diff  --git a/flang/docs/CMakeLists.txt b/flang/docs/CMakeLists.txt
index 770343cd29b80..3414b8e3acc46 100644
--- a/flang/docs/CMakeLists.txt
+++ b/flang/docs/CMakeLists.txt
@@ -126,7 +126,7 @@ if (LLVM_ENABLE_SPHINX)
 ARGS ${CMAKE_CURRENT_BINARY_DIR}/Source/FIR/CreateFIRLangRef.py)
 
   # CLANG_TABLEGEN_EXE variable needs to be set for clang_tablegen to run 
without error
-  set(CLANG_TABLEGEN_EXE clang-tblgen)
+  find_program(CLANG_TABLEGEN_EXE "clang-tblgen" ${LLVM_TOOLS_BINARY_DIR} 
NO_DEFAULT_PATH)
   gen_rst_file_from_td(FlangCommandLineReference.rst -gen-opt-docs 
FlangOptionsDocs.td docs-flang-html)
 endif()
 if (${SPHINX_OUTPUT_MAN})



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


[PATCH] D131475: [Flang] Use find_program() to find clang-tblgen

2022-08-29 Thread Nikita Popov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc04eab8c78e5: [Flang] Use find_program() to find 
clang-tblgen (authored by nikic).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131475

Files:
  clang/CMakeLists.txt
  flang/docs/CMakeLists.txt


Index: flang/docs/CMakeLists.txt
===
--- flang/docs/CMakeLists.txt
+++ flang/docs/CMakeLists.txt
@@ -126,7 +126,7 @@
 ARGS ${CMAKE_CURRENT_BINARY_DIR}/Source/FIR/CreateFIRLangRef.py)
 
   # CLANG_TABLEGEN_EXE variable needs to be set for clang_tablegen to run 
without error
-  set(CLANG_TABLEGEN_EXE clang-tblgen)
+  find_program(CLANG_TABLEGEN_EXE "clang-tblgen" ${LLVM_TOOLS_BINARY_DIR} 
NO_DEFAULT_PATH)
   gen_rst_file_from_td(FlangCommandLineReference.rst -gen-opt-docs 
FlangOptionsDocs.td docs-flang-html)
 endif()
 if (${SPHINX_OUTPUT_MAN})
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -484,6 +484,9 @@
 
 add_subdirectory(utils/TableGen)
 
+# Export CLANG_TABLEGEN_EXE for use by flang docs.
+set(CLANG_TABLEGEN_EXE "${CLANG_TABLEGEN_EXE}" CACHE INTERNAL "")
+
 add_subdirectory(include)
 
 # All targets below may depend on all tablegen'd files.


Index: flang/docs/CMakeLists.txt
===
--- flang/docs/CMakeLists.txt
+++ flang/docs/CMakeLists.txt
@@ -126,7 +126,7 @@
 ARGS ${CMAKE_CURRENT_BINARY_DIR}/Source/FIR/CreateFIRLangRef.py)
 
   # CLANG_TABLEGEN_EXE variable needs to be set for clang_tablegen to run without error
-  set(CLANG_TABLEGEN_EXE clang-tblgen)
+  find_program(CLANG_TABLEGEN_EXE "clang-tblgen" ${LLVM_TOOLS_BINARY_DIR} NO_DEFAULT_PATH)
   gen_rst_file_from_td(FlangCommandLineReference.rst -gen-opt-docs FlangOptionsDocs.td docs-flang-html)
 endif()
 if (${SPHINX_OUTPUT_MAN})
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -484,6 +484,9 @@
 
 add_subdirectory(utils/TableGen)
 
+# Export CLANG_TABLEGEN_EXE for use by flang docs.
+set(CLANG_TABLEGEN_EXE "${CLANG_TABLEGEN_EXE}" CACHE INTERNAL "")
+
 add_subdirectory(include)
 
 # All targets below may depend on all tablegen'd files.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] b5b7503 - [docs] improve documentation for misc-const-correctness

2022-08-29 Thread Jonas Toth via cfe-commits

Author: Jonas Toth
Date: 2022-08-29T11:20:47+02:00
New Revision: b5b750346346bfe95c7c6b1cceacc6cfccc8f4f4

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

LOG: [docs] improve documentation for misc-const-correctness

Improve the documentation for 'misc-const-correctness' to:

- include better examples
- improve the english
- fix links to other checks that were broken due to the directory-layout changes
- mention the limitation that the check does not run on `C` code.

Addresses #56749, #56958

Reviewed By: njames93

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

Added: 


Modified: 
clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst

Removed: 




diff  --git 
a/clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst 
b/clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst
index 76d283da37dd1..cc6ae33a414d2 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst
@@ -4,12 +4,12 @@ misc-const-correctness
 ==
 
 This check implements detection of local variables which could be declared as
-``const``, but are not. Declaring variables as ``const`` is required or 
recommended by many
+``const`` but are not. Declaring variables as ``const`` is required or 
recommended by many
 coding guidelines, such as:
 `CppCoreGuidelines ES.25 
`_
 and `AUTOSAR C++14 Rule A7-1-1 (6.7.1 Specifiers) 
`_.
 
-Please note that this analysis is type-based only. Variables that are not 
modified
+Please note that this check's analysis is type-based only. Variables that are 
not modified
 but used to create a non-const handle that might escape the scope are not 
diagnosed
 as potential ``const``.
 
@@ -18,25 +18,29 @@ as potential ``const``.
   // Declare a variable, which is not ``const`` ...
   int i = 42;
   // but use it as read-only. This means that `i` can be declared ``const``.
-  int result = i * i;
+  int result = i * i;   // Before transformation
+  int const result = i * i; // After transformation
 
-The check can analyzes values, pointers and references but not (yet) pointees:
+The check can analyze values, pointers and references but not (yet) pointees:
 
 .. code-block:: c++
 
   // Normal values like built-ins or objects.
-  int potential_const_int = 42; // 'const int potential_const_int = 42' 
suggestion.
+  int potential_const_int = 42;   // Before transformation
+  int const potential_const_int = 42; // After transformation
   int copy_of_value = potential_const_int;
 
-  MyClass could_be_const; // 'const MyClass could_be_const' suggestion;
+  MyClass could_be_const;   // Before transformation
+  MyClass const could_be_const; // After transformation
   could_be_const.const_qualified_method();
 
   // References can be declared const as well.
-  int &reference_value = potential_const_int; // 'const int &reference_value' 
suggestion.
+  int &reference_value = potential_const_int;   // Before transformation
+  int const& reference_value = potential_const_int; // After transformation
   int another_copy = reference_value;
 
   // The similar semantics of pointers are not (yet) analyzed.
-  int *pointer_variable = &potential_const_int; // Not 'const int 
*pointer_variable' suggestion.
+  int *pointer_variable = &potential_const_int; // _NO_ 'const int 
*pointer_variable' suggestion.
   int last_copy = *pointer_variable;
 
 The automatic code transformation is only applied to variables that are 
declared in single
@@ -44,18 +48,20 @@ declarations. You may want to prepare your code base with
 `readability-isolate-declaration <../readability/isolate-declaration.html>`_ 
first.
 
 Note that there is the check
-`cppcoreguidelines-avoid-non-const-global-variables 
`_
+`cppcoreguidelines-avoid-non-const-global-variables 
<../cppcoreguidelines/avoid-non-const-global-variables.html>`_
 to enforce ``const`` correctness on all globals.
 
 Known Limitations
 -
 
+The check does not run on `C` code.
+
 The check will not analyze templated variables or variables that are 
instantiation dependent.
 Different instantiations can result in 
diff erent ``const`` correctness properties and in general it
-is not possible to find all instantiations of a template. It might be used 
diff erently in an
-independent translation unit.
+is not possible to find all instantiations of a template. The template might 
be used 
diff erently in
+an independent tran

[PATCH] D132244: [docs] improve documentation for misc-const-correctness

2022-08-29 Thread Jonas Toth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb5b750346346: [docs] improve documentation for 
misc-const-correctness (authored by JonasToth).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132244

Files:
  clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst

Index: clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst
+++ clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst
@@ -4,12 +4,12 @@
 ==
 
 This check implements detection of local variables which could be declared as
-``const``, but are not. Declaring variables as ``const`` is required or recommended by many
+``const`` but are not. Declaring variables as ``const`` is required or recommended by many
 coding guidelines, such as:
 `CppCoreGuidelines ES.25 `_
 and `AUTOSAR C++14 Rule A7-1-1 (6.7.1 Specifiers) `_.
 
-Please note that this analysis is type-based only. Variables that are not modified
+Please note that this check's analysis is type-based only. Variables that are not modified
 but used to create a non-const handle that might escape the scope are not diagnosed
 as potential ``const``.
 
@@ -18,25 +18,29 @@
   // Declare a variable, which is not ``const`` ...
   int i = 42;
   // but use it as read-only. This means that `i` can be declared ``const``.
-  int result = i * i;
+  int result = i * i;   // Before transformation
+  int const result = i * i; // After transformation
 
-The check can analyzes values, pointers and references but not (yet) pointees:
+The check can analyze values, pointers and references but not (yet) pointees:
 
 .. code-block:: c++
 
   // Normal values like built-ins or objects.
-  int potential_const_int = 42; // 'const int potential_const_int = 42' suggestion.
+  int potential_const_int = 42;   // Before transformation
+  int const potential_const_int = 42; // After transformation
   int copy_of_value = potential_const_int;
 
-  MyClass could_be_const; // 'const MyClass could_be_const' suggestion;
+  MyClass could_be_const;   // Before transformation
+  MyClass const could_be_const; // After transformation
   could_be_const.const_qualified_method();
 
   // References can be declared const as well.
-  int &reference_value = potential_const_int; // 'const int &reference_value' suggestion.
+  int &reference_value = potential_const_int;   // Before transformation
+  int const& reference_value = potential_const_int; // After transformation
   int another_copy = reference_value;
 
   // The similar semantics of pointers are not (yet) analyzed.
-  int *pointer_variable = &potential_const_int; // Not 'const int *pointer_variable' suggestion.
+  int *pointer_variable = &potential_const_int; // _NO_ 'const int *pointer_variable' suggestion.
   int last_copy = *pointer_variable;
 
 The automatic code transformation is only applied to variables that are declared in single
@@ -44,18 +48,20 @@
 `readability-isolate-declaration <../readability/isolate-declaration.html>`_ first.
 
 Note that there is the check
-`cppcoreguidelines-avoid-non-const-global-variables `_
+`cppcoreguidelines-avoid-non-const-global-variables <../cppcoreguidelines/avoid-non-const-global-variables.html>`_
 to enforce ``const`` correctness on all globals.
 
 Known Limitations
 -
 
+The check does not run on `C` code.
+
 The check will not analyze templated variables or variables that are instantiation dependent.
 Different instantiations can result in different ``const`` correctness properties and in general it
-is not possible to find all instantiations of a template. It might be used differently in an
-independent translation unit.
+is not possible to find all instantiations of a template. The template might be used differently in
+an independent translation unit.
 
-Pointees can not be analyzed for constness yet. The following code is shows this limitation.
+Pointees can not be analyzed for constness yet. The following code shows this limitation.
 
 .. code-block:: c++
 
@@ -74,15 +80,35 @@
 Options
 ---
 
-.. option:: AnalyzeValues (default = 1)
+.. option:: AnalyzeValues (default = true)
 
   Enable or disable the analysis of ordinary value variables, like ``int i = 42;``
 
-.. option:: AnalyzeReferences (default = 1)
+  .. code-block:: c++
+
+// Warning
+int i = 42;
+// No warning
+int const i = 42;
+
+// Warning
+int a[] = {42, 42, 42};
+// No warning
+int const a[] = {42, 42, 42};
+
+.. opt

[PATCH] D132830: [clangd] Avoid crash when printing call to string literal operator template in hover

2022-08-29 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

can you move the test into 
`llvm/llvm-project/clang/unittests/AST/StmtPrinterTest.cpp` ?

while there, also a test on pack-extended user defined literals would be nice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132830

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


[PATCH] D131388: [docs] Add "Standard C++ Modules"

2022-08-29 Thread Aaron Siddhartha Mondal via Phabricator via cfe-commits
aaronmondal accepted this revision.
aaronmondal added a comment.

Thanks for addressing my comment. I think I overlooked the part about 
`-fmodules-embed-all-files` 😅

I'm fine with this revision 😊


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

https://reviews.llvm.org/D131388

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


[clang] efc76a1 - [analyzer] Silence GCC warnings about unused variables. NFC.

2022-08-29 Thread Martin Storsjö via cfe-commits

Author: Martin Storsjö
Date: 2022-08-29T13:26:13+03:00
New Revision: efc76a1ac5f910776091a48947ca1e90e9068845

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

LOG: [analyzer] Silence GCC warnings about unused variables. NFC.

Use `isa()` instead of `Type *Var = dyn_cast()`
when the result of the cast isn't used.

Added: 


Modified: 
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp 
b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 586e89ef2a174..656a7c1fe590a 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1217,7 +1217,7 @@ void ExprEngine::ProcessAutomaticObjDtor(const 
CFGAutomaticObjDtor Dtor,
   }
 
   unsigned Idx = 0;
-  if (const auto *AT = dyn_cast(varType)) {
+  if (isa(varType)) {
 SVal ElementCount;
 std::tie(state, Idx) = prepareStateForArrayDestruction(
 state, Region, varType, LCtx, &ElementCount);
@@ -1368,7 +1368,7 @@ void ExprEngine::ProcessMemberDtor(const CFGMemberDtor D,
   SVal FieldVal = State->getLValue(Member, ThisLoc);
 
   unsigned Idx = 0;
-  if (const auto *AT = dyn_cast(T)) {
+  if (isa(T)) {
 SVal ElementCount;
 std::tie(State, Idx) = prepareStateForArrayDestruction(
 State, FieldVal.getAsRegion(), T, LCtx, &ElementCount);



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


[PATCH] D71387: pass -mabi to LTO linker only in RISC-V targets, enable RISC-V LTO

2022-08-29 Thread Lewis Revill via Phabricator via cfe-commits
lewis-revill added a comment.
Herald added subscribers: sunshaoce, pcwang-thead, VincentWu, luke957, 
StephenFan, arichardson.
Herald added a project: All.

I believe this patch is still relevant/necessary when using LTO for RISCV, so 
can I ask if @khchen is able to update it to rebase/address the feedback? If 
not, are there are any objections to me commandeering this revision to get it 
landed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71387

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


[PATCH] D132843: [RISCV] Ensure target features get passed to the LTO linker for RISC-V

2022-08-29 Thread Lewis Revill via Phabricator via cfe-commits
lewis-revill created this revision.
lewis-revill added reviewers: efriedma, lenary, jrtc27, asb.
Herald added subscribers: sunshaoce, VincentWu, luke957, ormris, StephenFan, 
vkmr, frasercrmck, evandro, luismarques, apazos, sameer.abuasal, simoncook, 
s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, 
rogfer01, steven_wu, edward-jones, zzheng, shiva0217, kito-cheng, niosHD, 
sabuasal, johnrusso, rbar, hiraditya, arichardson, inglorion.
Herald added a project: All.
lewis-revill requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, eopXD, MaskRay.
Herald added a project: clang.

This patch fixes an issue whereby the LTO linker would not receive any 
information about target features, so things like architecture extensions would 
not be accounted for during codegen.

There is perhaps an outstanding question as to why this is required versus 
obtaining the target features from bitcode. This is the approach I implemented 
for now, but would be happy to look into whether I can make it work via bitcode 
if desired.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132843

Files:
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.h
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/lto.c


Index: clang/test/Driver/lto.c
===
--- clang/test/Driver/lto.c
+++ clang/test/Driver/lto.c
@@ -155,3 +155,8 @@
 // RISCV-SPEC-ABI-2: "-plugin-opt=-target-abi=ilp32d"
 // RISCV-SPEC-ABI-3: "-plugin-opt=-target-abi=lp64"
 // RISCV-SPEC-ABI-4: "-plugin-opt=-target-abi=lp64f"
+
+// RUN: %clang -target riscv32-unknown-elf %s -fuse-ld=gold -flto -mrelax \
+// RUN:   -### 2>&1 | FileCheck %s --check-prefix=RV32-RELAX-ATTR
+//
+// RV32-RELAX-ATTR: "-plugin-opt=-mattr=+relax"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -637,6 +637,7 @@
   case llvm::Triple::riscv32:
   case llvm::Triple::riscv64: {
 riscv::addRISCVTargetABIArgs(ToolChain, Args, CmdArgs);
+riscv::addRISCVTargetFeatureArgs(ToolChain, Args, CmdArgs);
 break;
   }
   }
Index: clang/lib/Driver/ToolChains/Arch/RISCV.h
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.h
+++ clang/lib/Driver/ToolChains/Arch/RISCV.h
@@ -30,6 +30,10 @@
 void addRISCVTargetABIArgs(const ToolChain &ToolChain,
const llvm::opt::ArgList &Args,
llvm::opt::ArgStringList &CmdArgs);
+
+void addRISCVTargetFeatureArgs(const ToolChain &ToolChain,
+   const llvm::opt::ArgList &Args,
+   llvm::opt::ArgStringList &CmdArgs);
 } // end namespace riscv
 } // namespace tools
 } // end namespace driver
Index: clang/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -313,3 +313,15 @@
   CmdArgs.push_back(
   Args.MakeArgString(Twine("-plugin-opt=-target-abi=") + ABIName));
 }
+
+void riscv::addRISCVTargetFeatureArgs(const ToolChain &ToolChain,
+  const llvm::opt::ArgList &Args,
+  llvm::opt::ArgStringList &CmdArgs) {
+  const Driver &D = ToolChain.getDriver();
+
+  // Pass a plugin-opt for each of the target features to the LTO linker.
+  std::vector Features;
+  riscv::getRISCVTargetFeatures(D, ToolChain.getTriple(), Args, Features);
+  for (StringRef F : Features)
+CmdArgs.push_back(Args.MakeArgString("-plugin-opt=-mattr=" + F));
+}


Index: clang/test/Driver/lto.c
===
--- clang/test/Driver/lto.c
+++ clang/test/Driver/lto.c
@@ -155,3 +155,8 @@
 // RISCV-SPEC-ABI-2: "-plugin-opt=-target-abi=ilp32d"
 // RISCV-SPEC-ABI-3: "-plugin-opt=-target-abi=lp64"
 // RISCV-SPEC-ABI-4: "-plugin-opt=-target-abi=lp64f"
+
+// RUN: %clang -target riscv32-unknown-elf %s -fuse-ld=gold -flto -mrelax \
+// RUN:   -### 2>&1 | FileCheck %s --check-prefix=RV32-RELAX-ATTR
+//
+// RV32-RELAX-ATTR: "-plugin-opt=-mattr=+relax"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -637,6 +637,7 @@
   case llvm::Triple::riscv32:
   case llvm::Triple::riscv64: {
 riscv::addRISCVTargetABIArgs(ToolChain, Args, CmdArgs);
+riscv::addRISCVTargetFeatureArgs(ToolChain, Args, CmdArgs);
 break;
   }
   }
Index: clang/lib/Driver/ToolChains/Arch/RISCV.h
===
--- clang/lib/Driver/ToolChains/Arch/RISC

[PATCH] D129833: Use @llvm.threadlocal.address intrinsic to access TLS

2022-08-29 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment.

In D129833#3727881 , @ChuanqiXu wrote:

> And I am working on adding Align properties. But I meet problems since the 
> alignment of threadlocal_address intrinsic depends on its argument so we 
> can't set the alignment for its declaration and we probably need to set the 
> alignment for its call, which means we need to set the alignment when in 
> IRBuilder. Do you think this is good?

I think that would be fine. Alternatively, it could be inferred in InstCombine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129833

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


[PATCH] D131154: FoldingRanges: Handle LineFoldingsOnly clients.

2022-08-29 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:202
 FR.endLine = End.line;
+FR.startCharacter = Start.character;
 FR.endCharacter = End.character;

can you put it back to its previous location (i.e. right after startline)



Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:251
+  // Always show last line of a block comment.
+  if (StartPosition(*LastComment).line != EndPosition(*LastComment).line) {
+End.line--;

i think this will end up trimming the foldings for cases like:

```
// foo\
bar\
baz
```

can we check the comment introducer instead? i.e. `//` vs `/*`. also let's 
amend the startposition to be `Startoffset +2` similar to what we do with 
braces, to make sure starting sentinel is not included in the folding range.



Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:391
+/* No folding for this comment.
+*/ int b_token=0;
+  )cpp",

usaxena95 wrote:
> kadircet wrote:
> > it'd be nice to have a test case like:
> > ```
> > template <[[typename foo, class bar]]> struct baz {[[]]};
> > ```
> > 
> > which isn't useful for line-only folding clients, but something we should 
> > be able to support going forward.
> I don't understand. We do not return any single-line ranges.
> I think lines are never long enough to have helpful foldings (atleast in 
> formatted code). It would only clutter  IMO.
> I don't understand. We do not return any single-line ranges.

I think that's just a convenient solution we went with today because it's 
convenient and easier. i don't think that's the final UX we want to have.

> I think lines are never long enough to have helpful foldings (atleast in 
> formatted code). It would only clutter IMO.

Even if that was the case template arguments can get really long, especially in 
the presence of SFINAE, so there's definitely value in folding them.

Also just to be clear, i wasn't suggesting to have the implementation for them. 
I was just saying let's leave the testcases here, in disabled state, to remind 
us to have support for them in the future (and properly handle even for single 
line case).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131154

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


[PATCH] D132147: [clang][dataflow] Refactor `TestingSupport.h`

2022-08-29 Thread weiyi via Phabricator via cfe-commits
wyt updated this revision to Diff 456305.
wyt marked 5 inline comments as done.
wyt added a comment.

Address comments: add const qualifiers where applicable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132147

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

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1240,43 +1240,46 @@
 /*IgnoreSmartPointerDereference=*/true};
 std::vector Diagnostics;
 llvm::Error Error = checkDataflow(
-SourceCode, FuncMatcher,
-[Options](ASTContext &Ctx, Environment &) {
-  return UncheckedOptionalAccessModel(Ctx, Options);
+/*AI:AnalysisInputs=*/{
+.Code = SourceCode,
+.TargetFuncMatcher = FuncMatcher,
+.MakeAnalysis =
+[Options](ASTContext &Ctx, Environment &) {
+  return UncheckedOptionalAccessModel(Ctx, Options);
+},
+.PostVisitCFG =
+[&Diagnostics,
+ Diagnoser = UncheckedOptionalAccessDiagnoser(Options)](
+ASTContext &Ctx, const CFGElement &Elt,
+const TypeErasedDataflowAnalysisState &State) mutable {
+  auto Stmt = Elt.getAs();
+  if (!Stmt) {
+return;
+  }
+  auto StmtDiagnostics =
+  Diagnoser.diagnose(Ctx, Stmt->getStmt(), State.Env);
+  llvm::move(StmtDiagnostics, std::back_inserter(Diagnostics));
+},
+.ASTBuildArgs = {"-fsyntax-only", "-std=c++17",
+ "-Wno-undefined-inline"},
+.ASTBuildVirtualMappedFiles = FileContents,
 },
-[&Diagnostics, Diagnoser = UncheckedOptionalAccessDiagnoser(Options)](
-ASTContext &Ctx, const CFGStmt &Stmt,
-const TypeErasedDataflowAnalysisState &State) mutable {
-  auto StmtDiagnostics =
-  Diagnoser.diagnose(Ctx, Stmt.getStmt(), State.Env);
-  llvm::move(StmtDiagnostics, std::back_inserter(Diagnostics));
-},
-[&Diagnostics](AnalysisData AnalysisData) {
-  auto &SrcMgr = AnalysisData.ASTCtx.getSourceManager();
-
+/*VerifyResults=*/[&Diagnostics](
+  const llvm::DenseMap
+  &Annotations,
+  const AnalysisOutputs &AO) {
   llvm::DenseSet AnnotationLines;
-  for (const auto &Pair : AnalysisData.Annotations) {
-auto *Stmt = Pair.getFirst();
-AnnotationLines.insert(
-SrcMgr.getPresumedLineNumber(Stmt->getBeginLoc()));
-// We add both the begin and end locations, so that if the
-// statement spans multiple lines then the test will fail.
-//
-// FIXME: Going forward, we should change this to instead just
-// get the single line number from the annotation itself, rather
-// than looking at the statement it's attached to.
-AnnotationLines.insert(
-SrcMgr.getPresumedLineNumber(Stmt->getEndLoc()));
+  for (const auto &[Line, _] : Annotations) {
+AnnotationLines.insert(Line);
   }
-
+  auto &SrcMgr = AO.ASTCtx.getSourceManager();
   llvm::DenseSet DiagnosticLines;
   for (SourceLocation &Loc : Diagnostics) {
 DiagnosticLines.insert(SrcMgr.getPresumedLineNumber(Loc));
   }
 
   EXPECT_THAT(DiagnosticLines, ContainerEq(AnnotationLines));
-},
-{"-fsyntax-only", "-std=c++17", "-Wno-undefined-inline"}, FileContents);
+});
 if (Error)
   FAIL() << llvm::toString(std::move(Error));
   }
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -56,47 +56,134 @@
 
 namespace test {
 
-// Returns assertions based on annotations that are present after statements in
-// `AnnotatedCode`.
-llvm::Expected>
-buildStatementToAnnotationMapping(const FunctionDecl *Func,
-  llvm::Annotations AnnotatedCode);
-
-struct AnalysisData {
+/// Contains data structures required and produced by a dataflow analysis run.
+struct AnalysisOutputs {
+  /// Input code that is analyzed. Points within the code may be mar

[PATCH] D132377: [clang][dataflow] Add `SetupTest` parameter for `AnalysisInputs`.

2022-08-29 Thread weiyi via Phabricator via cfe-commits
wyt updated this revision to Diff 456307.
wyt marked an inline comment as done.
wyt added a comment.

Propagate change from parent patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132377

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

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -94,6 +94,11 @@
   /// takes as argument the AST generated from the code being analyzed and the
   /// initial state from which the analysis starts with.
   std::function MakeAnalysis;
+  /// Optional. If provided, this function is executed immediately before
+  /// running the dataflow analysis to allow for additional setup. All fields in
+  /// the `AnalysisOutputs` argument will be initialized except for the
+  /// `BlockStates` field which is only computed later during the analysis.
+  std::function SetupTest = nullptr;
   /// Optional. If provided, this function is applied on each CFG element after
   /// the analysis has been run.
   std::function
-getAnnotationLinesAndContent(const AnalysisOutputs &AO);
-
-// FIXME: Return a string map instead of a vector of pairs.
-//
-/// Returns the analysis states at each annotated statement in `AO.Code`.
-template 
-llvm::Expected>>>
-getAnnotationStates(const AnalysisOutputs &AO) {
-  using StateT = DataflowAnalysisState;
-  // FIXME: Extend to annotations on non-statement constructs.
-  // Get annotated statements.
-  llvm::Expected>
-  MaybeStmtToAnnotations =
-  buildStatementToAnnotationMapping(AO.Target, AO.Code);
-  if (!MaybeStmtToAnnotations)
-return MaybeStmtToAnnotations.takeError();
-  auto &StmtToAnnotations = *MaybeStmtToAnnotations;
-
-  // Compute a map from statement annotations to the state computed
-  // for the program point immediately after the annotated statement.
-  std::vector> Results;
-  for (const CFGBlock *Block : AO.CFCtx.getCFG()) {
-// Skip blocks that were not evaluated.
-if (!AO.BlockStates[Block->getBlockID()])
-  continue;
-
-transferBlock(
-AO.CFCtx, AO.BlockStates, *Block, AO.InitEnv, AO.Analysis,
-[&Results,
- &StmtToAnnotations](const clang::CFGElement &Element,
- const TypeErasedDataflowAnalysisState &State) {
-  auto Stmt = Element.getAs();
-  if (!Stmt)
-return;
-  auto It = StmtToAnnotations.find(Stmt->getStmt());
-  if (It == StmtToAnnotations.end())
-return;
-  auto *Lattice =
-  llvm::any_cast(&State.Lattice.Value);
-  Results.emplace_back(It->second, StateT{*Lattice, State.Env});
-});
-  }
-
-  return Results;
-}
+buildLineToAnnotationMapping(SourceManager &SM,
+ llvm::Annotations AnnotatedCode);
 
 /// Runs dataflow specified from `AI.MakeAnalysis` and `AI.PostVisitCFG` on the
 /// body of the function that matches `AI.TargetFuncMatcher` in `AI.Code`.
@@ -174,9 +135,9 @@
 ///
 ///   `VerifyResults` must be provided.
 template 
-llvm::Error checkDataflow(
-AnalysisInputs AI,
-std::function VerifyResults) {
+llvm::Error
+checkDataflow(AnalysisInputs AI,
+  std::function VerifyResults) {
   // Build AST context from code.
   llvm::Annotations AnnotatedCode(AI.Code);
   auto Unit = tooling::buildASTFromCodeWithArgs(
@@ -210,7 +171,7 @@
 return MaybeCFCtx.takeError();
   auto &CFCtx = *MaybeCFCtx;
 
-  // Initialize states and run dataflow analysis.
+  // Initialize states for running dataflow analysis.
   DataflowAnalysisContext DACtx(std::make_unique());
   Environment InitEnv(DACtx, *Target);
   auto Analysis = AI.MakeAnalysis(Context, InitEnv);
@@ -225,19 +186,28 @@
 };
   }
 
-  // If successful, the run returns a mapping from block IDs to the
-  // post-analysis states for the CFG blocks that have been evaluated.
+  // Additional test setup.
+  AnalysisOutputs AO{AnnotatedCode, Context, Target, CFCtx,
+ Analysis,  InitEnv, {}};
+  if (AI.SetupTest) {
+auto Error = AI.SetupTest(AO);
+if (Error) {
+  return Error;
+}
+  }
+
+  // If successful, the dataflow analysis returns a mapping from block IDs to
+  // the post-analysis states for the CFG blocks that have been evaluated.
   llvm::Expected>>
   MaybeBlockStates = runTypeErasedDataflowAnalysis(CFCtx, Analysis, InitEnv,
PostVisitCFGClosure);
   if (!MaybeBlockStates)
 return MaybeBlockStates.takeError();
-  auto &BlockStates = *MaybeBlockStates;
+  AO.BlockStates = *MaybeBlockStates;
 
   // Verify dataflow analysis outputs.
-  AnalysisOutputs AO{AnnotatedCode, Context, Target,

[PATCH] D132377: [clang][dataflow] Add `SetupTest` parameter for `AnalysisInputs`.

2022-08-29 Thread weiyi via Phabricator via cfe-commits
wyt added inline comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:88
+/// Arguments for building the dataflow analysis.
+template  struct AnalysisInputs {
+  /// Input code that is analyzed.

sgatev wrote:
> Why move this? It makes it hard to tell if there are other changes. If there 
> are other changes, let's keep it where it is to have a clean diff and move it 
> in a separate commit.
It was moved as the new parameter declared uses `AnalysisOutputs`, hence 
`AnalysisOutputs` needs to come before `AnalysisInputs`. The move is now fixed 
by reordering these two struct declarations in the parent patch where they were 
first introduced.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132377

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


[PATCH] D132763: [clang][dataflow] Use `StringMap` for storing analysis states at annotated points instead of `vector>`.

2022-08-29 Thread weiyi via Phabricator via cfe-commits
wyt updated this revision to Diff 456308.
wyt marked 6 inline comments as done.
wyt added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132763

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

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -428,12 +428,12 @@
std::pair>>
Results,
ASTContext &ASTCtx) {
-ASSERT_THAT(Results, ElementsAre(Pair("p4", _), Pair("p3", _),
- Pair("p2", _), Pair("p1", _)));
-const Environment &Env1 = Results[3].second.Env;
-const Environment &Env2 = Results[2].second.Env;
-const Environment &Env3 = Results[1].second.Env;
-const Environment &Env4 = Results[0].second.Env;
+ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _),
+ Pair("p3", _), Pair("p4", _)));
+const Environment &Env1 = Results[0].second.Env;
+const Environment &Env2 = Results[1].second.Env;
+const Environment &Env3 = Results[2].second.Env;
+const Environment &Env4 = Results[3].second.Env;
 
 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
 ASSERT_THAT(FooDecl, NotNull());
@@ -581,10 +581,10 @@
  Results,
  ASTContext &ASTCtx) {
 ASSERT_THAT(Results,
-ElementsAre(Pair("p3", _), Pair("p2", _), Pair("p1", _)));
-const Environment &Env1 = Results[2].second.Env;
+ElementsAre(Pair("p1", _), Pair("p2", _), Pair("p3", _)));
+const Environment &Env1 = Results[0].second.Env;
 const Environment &Env2 = Results[1].second.Env;
-const Environment &Env3 = Results[0].second.Env;
+const Environment &Env3 = Results[2].second.Env;
 
 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
 ASSERT_THAT(FooDecl, NotNull());
@@ -624,12 +624,12 @@
  std::pair>>
  Results,
  ASTContext &ASTCtx) {
-ASSERT_THAT(Results, ElementsAre(Pair("p4", _), Pair("p3", _),
- Pair("p2", _), Pair("p1", _)));
-const Environment &Env1 = Results[3].second.Env;
-const Environment &Env2 = Results[2].second.Env;
-const Environment &Env3 = Results[1].second.Env;
-const Environment &Env4 = Results[0].second.Env;
+ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _),
+ Pair("p3", _), Pair("p4", _)));
+const Environment &Env1 = Results[0].second.Env;
+const Environment &Env2 = Results[1].second.Env;
+const Environment &Env3 = Results[2].second.Env;
+const Environment &Env4 = Results[3].second.Env;
 
 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
 ASSERT_THAT(FooDecl, NotNull());
@@ -753,14 +753,14 @@
 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
 ASSERT_THAT(FooDecl, NotNull());
 
-ASSERT_THAT(Results, ElementsAre(Pair("p2", _), Pair("p1", _)));
+ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _)));
 
-const Environment &Env1 = Results[1].second.Env;
+const Environment &Env1 = Results[0].second.Env;
 auto *FooVal1 =
 cast(Env1.getValue(*FooDecl, SkipPast::None));
 EXPECT_TRUE(Env1.flowConditionImplies(*FooVal1));
 
-const Environment &Env2 = Results[0].second.Env;
+const Environment &Env2 = Results[1].second.Env;
 auto *FooVal2 =
 cast(Env2.getValue(*FooDecl, SkipPast::None));
 EXPECT_FALSE(Env2.flowConditionImplies(*FooVal2));
@@ -787,14 +787,14 @@
 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
 ASSERT_THAT(FooDecl, NotNull());
 
-ASSERT_THAT(Results, ElementsAre(Pair("p2", _), Pair("p1", _)));
+ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _)));
 
-const Environment &Env1 = Results[1].second.Env;
+const Environment &Env1 = Results[0].second.Env;
 auto *FooVal1 =
 

[PATCH] D132763: [clang][dataflow] Use `StringMap` for storing analysis states at annotated points instead of `vector>`.

2022-08-29 Thread weiyi via Phabricator via cfe-commits
wyt added inline comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:363-365
+std::sort(AnnotationStatesAsVector.begin(),
+  AnnotationStatesAsVector.end(),
+  [](auto a, auto b) { return a.first < b.first; });

gribozavr2 wrote:
> Please use llvm::sort. You also don't need to specify the comparator, the 
> default comparator for strings is exactly the same.
> 
> 
Used llvm::sort.

The default comparator for pairs also tries to compare the second field which 
is undefined for DataflowAnalysisState. And considering that annotations are 
unique here, it should be sufficient to just compare the strings here - hence 
the specified comparator that just checks the first of the pair.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132763

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


[PATCH] D132745: [clang] Fix ambiguous use of `report_fatal_error`.

2022-08-29 Thread weiyi via Phabricator via cfe-commits
wyt updated this revision to Diff 456311.
wyt marked an inline comment as done.
wyt added a comment.

Address comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132745

Files:
  clang/lib/Basic/SanitizerSpecialCaseList.cpp


Index: clang/lib/Basic/SanitizerSpecialCaseList.cpp
===
--- clang/lib/Basic/SanitizerSpecialCaseList.cpp
+++ clang/lib/Basic/SanitizerSpecialCaseList.cpp
@@ -33,7 +33,7 @@
   std::string Error;
   if (auto SSCL = create(Paths, VFS, Error))
 return SSCL;
-  llvm::report_fatal_error(Error);
+  llvm::report_fatal_error(StringRef(Error));
 }
 
 void SanitizerSpecialCaseList::createSanitizerSections() {


Index: clang/lib/Basic/SanitizerSpecialCaseList.cpp
===
--- clang/lib/Basic/SanitizerSpecialCaseList.cpp
+++ clang/lib/Basic/SanitizerSpecialCaseList.cpp
@@ -33,7 +33,7 @@
   std::string Error;
   if (auto SSCL = create(Paths, VFS, Error))
 return SSCL;
-  llvm::report_fatal_error(Error);
+  llvm::report_fatal_error(StringRef(Error));
 }
 
 void SanitizerSpecialCaseList::createSanitizerSections() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D131475: [Flang] Use find_program() to find clang-tblgen

2022-08-29 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski added a comment.

Thanks for the quick fix, makes sense! 👍🏻


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131475

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


[PATCH] D132592: [Clang] Implement function attribute nouwtable

2022-08-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM, though please add a release note for the new attribute.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132592

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


[clang] 123062e - Expose QualType::getUnqualifiedType in libclang

2022-08-29 Thread Aaron Ballman via cfe-commits

Author: Luca Di Sera
Date: 2022-08-29T08:16:18-04:00
New Revision: 123062ec2f3e45bb1614a63bcb79c22527d9b914

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

LOG: Expose QualType::getUnqualifiedType in libclang

The method is now wrapped by clang_getUnqualifiedType.

A declaration for clang_getUnqualifiedType was added to
clang-c/Index.h to expose it to user of the library.

An implementation for clang_getUnqualifiedType was introduced in
CXType.cpp that wraps the equivalent method of the underlying
QualType of a CXType.

An export symbol was added to libclang.map under the new version entry
LLVM_16.

A test was added to LibclangTest.cpp that tests the removal of
qualifiers for some CXTypes.

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang-c/Index.h
clang/tools/libclang/CXType.cpp
clang/tools/libclang/libclang.map
clang/unittests/libclang/LibclangTest.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index cdcb6007ab836..ef3950501f963 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -259,7 +259,8 @@ clang-extdef-mapping
 libclang
 
 
-- ...
+- Introduced the new function `clang_getUnqualifiedType`, which mimics
+  the behavior of `QualType::getUnqualifiedType` for `CXType`.
 
 Static Analyzer
 ---

diff  --git a/clang/include/clang-c/Index.h b/clang/include/clang-c/Index.h
index b897910624de8..41efc57823d3c 100644
--- a/clang/include/clang-c/Index.h
+++ b/clang/include/clang-c/Index.h
@@ -3760,6 +3760,43 @@ CINDEX_LINKAGE CXString clang_getTypedefName(CXType CT);
  */
 CINDEX_LINKAGE CXType clang_getPointeeType(CXType T);
 
+/**
+ * Retrieve the unqualified variant of the given type, removing as
+ * little sugar as possible.
+ *
+ * For example, given the following series of typedefs:
+ *
+ * \code
+ * typedef int Integer;
+ * typedef const Integer CInteger;
+ * typedef CInteger DifferenceType;
+ * \endcode
+ *
+ * Executing \c clang_getUnqualifiedType() on a \c CXType that
+ * represents \c DifferenceType, will desugar to a type representing
+ * \c Integer, that has no qualifiers.
+ *
+ * And, executing \c clang_getUnqualifiedType() on the type of the
+ * first argument of the following function declaration:
+ *
+ * \code
+ * void foo(const int);
+ * \endcode
+ *
+ * Will return a type representing \c int, removing the \c const
+ * qualifier.
+ *
+ * Sugar over array types is not desugared.
+ *
+ * A type can be checked for qualifiers with \c
+ * clang_isConstQualifiedType(), \c clang_isVolatileQualifiedType()
+ * and \c clang_isRestrictQualifiedType().
+ *
+ * A type that resulted from a call to \c clang_getUnqualifiedType
+ * will return \c false for all of the above calls.
+ */
+CINDEX_LINKAGE CXType clang_getUnqualifiedType(CXType CT);
+
 /**
  * Return the cursor for the declaration of the given type.
  */

diff  --git a/clang/tools/libclang/CXType.cpp b/clang/tools/libclang/CXType.cpp
index 5f5a63ddaaca6..e50a471b94098 100644
--- a/clang/tools/libclang/CXType.cpp
+++ b/clang/tools/libclang/CXType.cpp
@@ -484,6 +484,10 @@ CXType clang_getPointeeType(CXType CT) {
   return MakeCXType(T, GetTU(CT));
 }
 
+CXType clang_getUnqualifiedType(CXType CT) {
+  return MakeCXType(GetQualType(CT).getUnqualifiedType(), GetTU(CT));
+}
+
 CXCursor clang_getTypeDeclaration(CXType CT) {
   if (CT.kind == CXType_Invalid)
 return cxcursor::MakeCXCursorInvalid(CXCursor_NoDeclFound);

diff  --git a/clang/tools/libclang/libclang.map 
b/clang/tools/libclang/libclang.map
index 716e2474966d5..9ed81722fa341 100644
--- a/clang/tools/libclang/libclang.map
+++ b/clang/tools/libclang/libclang.map
@@ -405,6 +405,11 @@ LLVM_13 {
   local: *;
 };
 
+LLVM_16 {
+  global:
+clang_getUnqualifiedType;
+};
+
 # Example of how to add a new symbol version entry.  If you do add a new symbol
 # version, please update the example to depend on the version you added.
 # LLVM_X {

diff  --git a/clang/unittests/libclang/LibclangTest.cpp 
b/clang/unittests/libclang/LibclangTest.cpp
index fc3ad43b495cf..e6084859ade71 100644
--- a/clang/unittests/libclang/LibclangTest.cpp
+++ b/clang/unittests/libclang/LibclangTest.cpp
@@ -843,6 +843,54 @@ TEST_F(LibclangParseTest, 
clang_Cursor_hasVarDeclExternalStorageTrue) {
   },
   nullptr);
 }
+
+TEST_F(LibclangParseTest, clang_getUnqualifiedTypeRemovesQualifiers) {
+  std::string Header = "header.h";
+  WriteFile(Header, "void foo1(const int);\n"
+"void foo2(volatile int);\n"
+"void foo3(const volatile int);\n"
+"void foo4(int* const);\n"
+"void foo5(int* volatile);\n"
+"void foo6(int* re

[PATCH] D132749: Expose QualType::getUnqualifiedType in libclang

2022-08-29 Thread Aaron Ballman via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG123062ec2f3e: Expose QualType::getUnqualifiedType in 
libclang (authored by diseraluca, committed by aaron.ballman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132749

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang-c/Index.h
  clang/tools/libclang/CXType.cpp
  clang/tools/libclang/libclang.map
  clang/unittests/libclang/LibclangTest.cpp

Index: clang/unittests/libclang/LibclangTest.cpp
===
--- clang/unittests/libclang/LibclangTest.cpp
+++ clang/unittests/libclang/LibclangTest.cpp
@@ -843,6 +843,54 @@
   },
   nullptr);
 }
+
+TEST_F(LibclangParseTest, clang_getUnqualifiedTypeRemovesQualifiers) {
+  std::string Header = "header.h";
+  WriteFile(Header, "void foo1(const int);\n"
+"void foo2(volatile int);\n"
+"void foo3(const volatile int);\n"
+"void foo4(int* const);\n"
+"void foo5(int* volatile);\n"
+"void foo6(int* restrict);\n"
+"void foo7(int* const volatile);\n"
+"void foo8(int* volatile restrict);\n"
+"void foo9(int* const restrict);\n"
+"void foo10(int* const volatile restrict);\n");
+
+  auto is_qualified = [](CXType type) -> bool {
+return clang_isConstQualifiedType(type) ||
+   clang_isVolatileQualifiedType(type) ||
+   clang_isRestrictQualifiedType(type);
+  };
+
+  auto from_CXString = [](CXString cx_string) -> std::string {
+std::string string{clang_getCString(cx_string)};
+
+clang_disposeString(cx_string);
+
+return string;
+  };
+
+  ClangTU = clang_parseTranslationUnit(Index, Header.c_str(), nullptr, 0,
+   nullptr, 0, TUFlags);
+
+  Traverse([&is_qualified, &from_CXString](CXCursor cursor, CXCursor) {
+if (clang_getCursorKind(cursor) == CXCursor_FunctionDecl) {
+  CXType arg_type = clang_getArgType(clang_getCursorType(cursor), 0);
+  EXPECT_TRUE(is_qualified(arg_type))
+  << "Input data '" << from_CXString(clang_getCursorSpelling(cursor))
+  << "' first argument does not have a qualified type.";
+
+  CXType unqualified_arg_type = clang_getUnqualifiedType(arg_type);
+  EXPECT_FALSE(is_qualified(unqualified_arg_type))
+  << "The type '" << from_CXString(clang_getTypeSpelling(arg_type))
+  << "' was not unqualified after a call to clang_getUnqualifiedType.";
+}
+
+return CXChildVisit_Continue;
+  });
+}
+
 class LibclangRewriteTest : public LibclangParseTest {
 public:
   CXRewriter Rew = nullptr;
Index: clang/tools/libclang/libclang.map
===
--- clang/tools/libclang/libclang.map
+++ clang/tools/libclang/libclang.map
@@ -405,6 +405,11 @@
   local: *;
 };
 
+LLVM_16 {
+  global:
+clang_getUnqualifiedType;
+};
+
 # Example of how to add a new symbol version entry.  If you do add a new symbol
 # version, please update the example to depend on the version you added.
 # LLVM_X {
Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -484,6 +484,10 @@
   return MakeCXType(T, GetTU(CT));
 }
 
+CXType clang_getUnqualifiedType(CXType CT) {
+  return MakeCXType(GetQualType(CT).getUnqualifiedType(), GetTU(CT));
+}
+
 CXCursor clang_getTypeDeclaration(CXType CT) {
   if (CT.kind == CXType_Invalid)
 return cxcursor::MakeCXCursorInvalid(CXCursor_NoDeclFound);
Index: clang/include/clang-c/Index.h
===
--- clang/include/clang-c/Index.h
+++ clang/include/clang-c/Index.h
@@ -3760,6 +3760,43 @@
  */
 CINDEX_LINKAGE CXType clang_getPointeeType(CXType T);
 
+/**
+ * Retrieve the unqualified variant of the given type, removing as
+ * little sugar as possible.
+ *
+ * For example, given the following series of typedefs:
+ *
+ * \code
+ * typedef int Integer;
+ * typedef const Integer CInteger;
+ * typedef CInteger DifferenceType;
+ * \endcode
+ *
+ * Executing \c clang_getUnqualifiedType() on a \c CXType that
+ * represents \c DifferenceType, will desugar to a type representing
+ * \c Integer, that has no qualifiers.
+ *
+ * And, executing \c clang_getUnqualifiedType() on the type of the
+ * first argument of the following function declaration:
+ *
+ * \code
+ * void foo(const int);
+ * \endcode
+ *
+ * Will return a type representing \c int, removing the \c const
+ * qualifier.
+ *
+ * Sugar over array types is not desugared.
+ *
+ * A type can be checked for qualifiers with \c
+ * clang_isConstQualifiedType(), \c clang_isVolatileQualifiedTy

[PATCH] D132756: [clang][dataflow] Refactor `TypeErasedDataflowAnalysisTest` - replace usage of the deprecated overload of `checkDataflow`.

2022-08-29 Thread weiyi via Phabricator via cfe-commits
wyt updated this revision to Diff 456317.
wyt marked 2 inline comments as done.
wyt added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132756

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

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -24,8 +24,10 @@
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Testing/ADT/StringMapEntry.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -42,12 +44,10 @@
 using namespace dataflow;
 using namespace test;
 using namespace ast_matchers;
-using ::testing::_;
-using ::testing::ElementsAre;
+using llvm::IsStringMapEntry;
+using ::testing::DescribeMatcher;
 using ::testing::IsEmpty;
-using ::testing::IsNull;
 using ::testing::NotNull;
-using ::testing::Pair;
 using ::testing::Test;
 using ::testing::UnorderedElementsAre;
 
@@ -130,7 +130,8 @@
 }
 
 struct FunctionCallLattice {
-  llvm::SmallSet CalledFunctions;
+  using FunctionSet = llvm::SmallSet;
+  FunctionSet CalledFunctions;
 
   bool operator==(const FunctionCallLattice &Other) const {
 return CalledFunctions == Other.CalledFunctions;
@@ -197,16 +198,24 @@
 
 ASSERT_THAT_ERROR(
 test::checkDataflow(
-Code, "target",
-[](ASTContext &C, Environment &) {
-  return FunctionCallAnalysis(C);
+/*AI:AnalysisInputs=*/
+{
+.Code = Code,
+.TargetFuncMatcher = ast_matchers::hasName("target"),
+.MakeAnalysis =
+[](ASTContext &C, Environment &) {
+  return FunctionCallAnalysis(C);
+},
+.ASTBuildArgs = {"-fsyntax-only", "-std=c++17"},
+.ASTBuildVirtualMappedFiles = FilesContents,
 },
+/*VerifyResults=*/
 [&Expectations](
-llvm::ArrayRef>>
-Results,
-ASTContext &) { EXPECT_THAT(Results, Expectations); },
-{"-fsyntax-only", "-std=c++17"}, FilesContents),
+const llvm::StringMap<
+DataflowAnalysisState> &Results,
+const AnalysisOutputs &) {
+  EXPECT_THAT(Results, Expectations);
+}),
 llvm::Succeeded());
   }
 };
@@ -214,12 +223,16 @@
 MATCHER_P(HoldsFunctionCallLattice, m,
   ((negation ? "doesn't hold" : "holds") +
llvm::StringRef(" a lattice element that ") +
-   ::testing::DescribeMatcher(m, negation))
+   DescribeMatcher(m))
   .str()) {
   return ExplainMatchResult(m, arg.Lattice, result_listener);
 }
 
-MATCHER_P(HasCalledFunctions, m, "") {
+MATCHER_P(HasCalledFunctions, m,
+  ((negation ? "doesn't hold" : "holds") +
+   llvm::StringRef(" a set of called functions that ") +
+   DescribeMatcher(m))
+  .str()) {
   return ExplainMatchResult(m, arg.CalledFunctions, result_listener);
 }
 
@@ -233,9 +246,9 @@
   // [[p]]
 }
   )";
-  runDataflow(Code, UnorderedElementsAre(
-Pair("p", HoldsFunctionCallLattice(HasCalledFunctions(
-  UnorderedElementsAre("foo", "bar"));
+  runDataflow(Code, UnorderedElementsAre(IsStringMapEntry(
+"p", HoldsFunctionCallLattice(HasCalledFunctions(
+ UnorderedElementsAre("foo", "bar"));
 }
 
 TEST_F(NoreturnDestructorTest, ConditionalOperatorLeftBranchReturns) {
@@ -248,9 +261,9 @@
   // [[p]]
 }
   )";
-  runDataflow(Code, UnorderedElementsAre(
-Pair("p", HoldsFunctionCallLattice(HasCalledFunctions(
-  UnorderedElementsAre("foo"));
+  runDataflow(Code, UnorderedElementsAre(IsStringMapEntry(
+"p", HoldsFunctionCallLattice(HasCalledFunctions(
+ UnorderedElementsAre("foo"));
 }
 
 TEST_F(NoreturnDestructorTest, ConditionalOperatorRightBranchReturns) {
@@ -263,9 +276,9 @@
   // [[p]]
 }
   )";
-  runDataflow(Code, UnorderedElementsAre(
-Pair("p", HoldsFunctionCallLattice(HasCalledFunctions(
-  UnorderedElementsAre("foo"));
+  runDataflow(Code, UnorderedElementsAre(IsStringMapEntry(
+"p", HoldsFunctionCallLat

[PATCH] D132791: Fix formatting in release notes

2022-08-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM! I did not test building the docs, but the changes all look correct based 
on my experience with RST.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132791

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


[PATCH] D132763: [clang][dataflow] Use `StringMap` for storing analysis states at annotated points instead of `vector>`.

2022-08-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision.
gribozavr2 added inline comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:302
+AnnotationStates.insert({It->second, StateT{*Lattice, State.Env}});
+assert(InsertSuccess);
   };

Please add `(void)InsertSuccess;` to silence an "unused variable" warning in 
release builds.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132763

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


[PATCH] D131479: Handle explicitly defaulted consteval special members.

2022-08-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D131479#3753586 , @Mordante wrote:

> In D131479#3753533 , @aaron.ballman 
> wrote:
>
>> In D131479#3753462 , @Mordante 
>> wrote:
>>
>>> This change breaks the libc++ test 
>>> `libcxx/test/std/utilities/optional/optional.object/optional.object.ctor/move.fail.cpp`.
>>>  
>>> (https://buildkite.com/llvm-project/libcxx-ci/builds/13149#0182d0d4-341a-4b41-b67f-12937f41e6d5)
>>>
>>> Looking at the description of this patch it should only affect `consteval` 
>>> functions, but it seems to affect `constexpr` functions too. Can you have a 
>>> look?
>>
>> Agreed that we should take a look, but it's interesting to note that 
>> libstdc++ seems to have a different behavior than libc++ here: 
>> https://godbolt.org/z/6cWhf6GYj (the last three compiler panes show Clang 14 
>> libstd++, Clang 14 libc++, and Clang trunk libc++). How sure are you that 
>> the libc++ test is actually correct, because (without checking the standard, 
>> so take with a giant grain of salt) this seems like it might have fixed a 
>> bug in libc++? The release note says this is expected to impact constexpr 
>> and consteval default special member functions, so behavioral changes to 
>> `constexpr` aren't unexpected (though I agree that the commit message didn't 
>> mention `constexpr` so the changes may seem surprising).
>
> Thanks for confirming it's indeed intended to affect `constexpr` too! It was 
> a bit confusing since it also referred to C++14, which didn't have 
> `consteval`. I wasn't entirely sure about the status of libc++, but since the 
> patch gave of a mixed message I wanted make sure that `constexpr` was 
> intended.

It was a good question to bring up!

> I've done some further digging and according to cppreference the paper 
> P0602R4 was a DR against C++20. This was proposed in the paper.
> Looking at the changes in the addressing this paper I see no C++ version 
> checks https://reviews.llvm.org/D32385
> In a followup introduces tests using C++ version checks 
> https://reviews.llvm.org/D54772
> The failing test was introduced in 308624127fd6cc36558b6eee4d4ffa4e215a074e 
> before the paper was voted in
>
> So I think there's indeed a bug in libc++ which was hidden since Clang hadn't 
> implemented some DRs. I will look at the libc++ side what needs to be done to 
> fix the CI failures.

Great sleuth work! Thank you for the investigation!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131479

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


[PATCH] D132848: [clang] Fix checking for emulated TLS in shouldAssumeDSOLocal in CodeGen

2022-08-29 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo created this revision.
mstorsjo added a reviewer: aaron.ballman.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: clang.

We shouldn't just check the clang options, we also should check
`Triple::hasDefaultEmulatedTLS()`.

This doesn't make any testable difference in this commit, because
the check for emulated TLS is within a block for
`TT.isWindowsGNUEnvironment()`, and `hasDefaultEmulatedTLS()` returns
false for any such environment - but it makes the code from
0e4cf807aeaf54a10e02176498a7df13ac722b37 
 / D102970 
 more correct and
generic.

Some mingw distributions carry a downstream patch, that enables
emulated TLS by default for mingw targets in `hasDefaultEmulatedTLS()`

- and for such cases, this patch does make a difference and fixes the

detection of emulated TLS, if it is implicitly enabled.

I'm open for better suggestions on where to place the `useEmulatedTLS()`
helper function; it's a parallel to the preexisting
`TargetMachine::useEmulatedTLS()` which does the same, within llvm.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132848

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h


Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -746,6 +746,12 @@
 
   const TargetCodeGenInfo &getTargetCodeGenInfo();
 
+  bool useEmulatedTLS() const {
+if (CodeGenOpts.ExplicitEmulatedTLS)
+  return CodeGenOpts.EmulatedTLS;
+return getTriple().hasDefaultEmulatedTLS();
+  }
+
   CodeGenTypes &getTypes() { return Types; }
 
   CodeGenVTables &getVTables() { return VTables; }
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1154,7 +1154,7 @@
 // such variables can't be marked as DSO local. (Native TLS variables
 // can't be dllimported at all, though.)
 if (GV->isDeclarationForLinker() && isa(GV) &&
-(!GV->isThreadLocal() || CGM.getCodeGenOpts().EmulatedTLS))
+(!GV->isThreadLocal() || CGM.useEmulatedTLS()))
   return false;
   }
 


Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -746,6 +746,12 @@
 
   const TargetCodeGenInfo &getTargetCodeGenInfo();
 
+  bool useEmulatedTLS() const {
+if (CodeGenOpts.ExplicitEmulatedTLS)
+  return CodeGenOpts.EmulatedTLS;
+return getTriple().hasDefaultEmulatedTLS();
+  }
+
   CodeGenTypes &getTypes() { return Types; }
 
   CodeGenVTables &getVTables() { return VTables; }
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1154,7 +1154,7 @@
 // such variables can't be marked as DSO local. (Native TLS variables
 // can't be dllimported at all, though.)
 if (GV->isDeclarationForLinker() && isa(GV) &&
-(!GV->isThreadLocal() || CGM.getCodeGenOpts().EmulatedTLS))
+(!GV->isThreadLocal() || CGM.useEmulatedTLS()))
   return false;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D128095: [clang] Improve diagnostics for expansion length mismatch

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



Comment at: clang/lib/Sema/SemaTemplateVariadic.cpp:103-106
+VisitSubstNonTypeTemplateParmPackExpr(SubstNonTypeTemplateParmPackExpr *E) 
{
+  Unexpanded.push_back({E, E->getParameterPackLocation()});
+  return true;
+}

Do we need to handle `FunctionParmPackExpr` as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128095

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


[PATCH] D131154: FoldingRanges: Handle LineFoldingsOnly clients.

2022-08-29 Thread Utkarsh Saxena via Phabricator via cfe-commits
usaxena95 updated this revision to Diff 456321.
usaxena95 marked 3 inline comments as done.
usaxena95 added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131154

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticSelection.cpp
  clang-tools-extra/clangd/SemanticSelection.h
  clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
@@ -196,7 +196,7 @@
   ElementsAre(SourceAnnotations.range("empty")));
 }
 
-TEST(FoldingRanges, All) {
+TEST(FoldingRanges, ASTAll) {
   const char *Tests[] = {
   R"cpp(
 #define FOO int foo() {\
@@ -265,7 +265,7 @@
   }
 }
 
-TEST(FoldingRangesPseudoParser, All) {
+TEST(FoldingRanges, PseudoParserWithoutLineFoldings) {
   const char *Tests[] = {
   R"cpp(
 #define FOO int foo() {\
@@ -336,36 +336,98 @@
 ]]};
   )cpp",
   R"cpp(
-[[/* Multi 
+/*[[ Multi 
   * line
   *  comment 
-  */]]
+  ]]*/
   )cpp",
   R"cpp(
-[[// Comment
+//[[ Comment
 // 1]]
 
-[[// Comment
+//[[ Comment
 // 2]]
 
 // No folding for single line comment.
 
-[[/* comment 3
-*/]]
+/*[[ comment 3
+]]*/
 
-[[/* comment 4
-*/]]
+/*[[ comment 4
+]]*/
   )cpp",
   };
   for (const char *Test : Tests) {
 auto T = Annotations(Test);
-EXPECT_THAT(
-gatherFoldingRanges(llvm::cantFail(getFoldingRanges(T.code().str(,
-UnorderedElementsAreArray(T.ranges()))
+EXPECT_THAT(gatherFoldingRanges(llvm::cantFail(getFoldingRanges(
+T.code().str(), /*LineFoldingsOnly=*/false))),
+UnorderedElementsAreArray(T.ranges()))
 << Test;
   }
 }
 
+TEST(FoldingRanges, PseudoParserLineFoldingsOnly) {
+  const char *Tests[] = {
+  R"cpp(
+void func(int a) {[[
+a++;]]
+}
+  )cpp",
+  R"cpp(
+// Always exclude last line for brackets.
+void func(int a) {[[
+  if(a == 1) {[[
+a++;]]
+  } else if (a == 2){[[
+a--;]]
+  } else {  // No folding for 2 line bracketed ranges.
+  }]]
+}
+  )cpp",
+  R"cpp(
+/*[[ comment
+* comment]]
+*/
+
+/* No folding for this comment.
+*/
+
+// No folding for this comment.
+
+//[[ 2 single line comment.
+// 2 single line comment.]]
+
+//[[ >=2 line comments.
+// >=2 line comments.
+// >=2 line comments.]]
+
+//[[ foo\
+bar\
+baz]]
+  )cpp",
+  // FIXME: Support folding template arguments.
+  // R"cpp(
+  // template <[[typename foo, class bar]]> struct baz {};
+  // )cpp",
+
+  };
+  auto StripColumns = [](const std::vector &Ranges) {
+std::vector Res;
+for (Range R : Ranges) {
+  R.start.character = R.end.character = 0;
+  Res.push_back(R);
+}
+return Res;
+  };
+  for (const char *Test : Tests) {
+auto T = Annotations(Test);
+EXPECT_THAT(
+StripColumns(gatherFoldingRanges(llvm::cantFail(
+getFoldingRanges(T.code().str(), /*LineFoldingsOnly=*/true,
+UnorderedElementsAreArray(StripColumns(T.ranges(
+<< Test;
+  }
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/SemanticSelection.h
===
--- clang-tools-extra/clangd/SemanticSelection.h
+++ clang-tools-extra/clangd/SemanticSelection.h
@@ -33,7 +33,7 @@
 /// Returns a list of ranges whose contents might be collapsible in an editor.
 /// This version uses the pseudoparser which does not require the AST.
 llvm::Expected>
-getFoldingRanges(const std::string &Code);
+getFoldingRanges(const std::string &Code, bool LineFoldingOnly);
 
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/SemanticSelection.cpp
===
--- clang-tools-extra/clangd/SemanticSelection.cpp
+++ clang-tools-extra/clangd/SemanticSelection.cpp
@@ -179,7 +179,7 @@
 // statement bodies).
 // Related issue: https://github.com/clangd/clangd/issues/310
 llvm::Expected>
-getFoldingRanges(const std::string &Code) {
+getFoldingRanges(const std::string &Code, bool LineFoldingOnly) {
   aut

[PATCH] D132763: [clang][dataflow] Use `StringMap` for storing analysis states at annotated points instead of `vector>`.

2022-08-29 Thread weiyi via Phabricator via cfe-commits
wyt updated this revision to Diff 456323.
wyt marked an inline comment as done.
wyt added a comment.

Address comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132763

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

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -428,12 +428,12 @@
std::pair>>
Results,
ASTContext &ASTCtx) {
-ASSERT_THAT(Results, ElementsAre(Pair("p4", _), Pair("p3", _),
- Pair("p2", _), Pair("p1", _)));
-const Environment &Env1 = Results[3].second.Env;
-const Environment &Env2 = Results[2].second.Env;
-const Environment &Env3 = Results[1].second.Env;
-const Environment &Env4 = Results[0].second.Env;
+ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _),
+ Pair("p3", _), Pair("p4", _)));
+const Environment &Env1 = Results[0].second.Env;
+const Environment &Env2 = Results[1].second.Env;
+const Environment &Env3 = Results[2].second.Env;
+const Environment &Env4 = Results[3].second.Env;
 
 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
 ASSERT_THAT(FooDecl, NotNull());
@@ -581,10 +581,10 @@
  Results,
  ASTContext &ASTCtx) {
 ASSERT_THAT(Results,
-ElementsAre(Pair("p3", _), Pair("p2", _), Pair("p1", _)));
-const Environment &Env1 = Results[2].second.Env;
+ElementsAre(Pair("p1", _), Pair("p2", _), Pair("p3", _)));
+const Environment &Env1 = Results[0].second.Env;
 const Environment &Env2 = Results[1].second.Env;
-const Environment &Env3 = Results[0].second.Env;
+const Environment &Env3 = Results[2].second.Env;
 
 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
 ASSERT_THAT(FooDecl, NotNull());
@@ -624,12 +624,12 @@
  std::pair>>
  Results,
  ASTContext &ASTCtx) {
-ASSERT_THAT(Results, ElementsAre(Pair("p4", _), Pair("p3", _),
- Pair("p2", _), Pair("p1", _)));
-const Environment &Env1 = Results[3].second.Env;
-const Environment &Env2 = Results[2].second.Env;
-const Environment &Env3 = Results[1].second.Env;
-const Environment &Env4 = Results[0].second.Env;
+ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _),
+ Pair("p3", _), Pair("p4", _)));
+const Environment &Env1 = Results[0].second.Env;
+const Environment &Env2 = Results[1].second.Env;
+const Environment &Env3 = Results[2].second.Env;
+const Environment &Env4 = Results[3].second.Env;
 
 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
 ASSERT_THAT(FooDecl, NotNull());
@@ -753,14 +753,14 @@
 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
 ASSERT_THAT(FooDecl, NotNull());
 
-ASSERT_THAT(Results, ElementsAre(Pair("p2", _), Pair("p1", _)));
+ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _)));
 
-const Environment &Env1 = Results[1].second.Env;
+const Environment &Env1 = Results[0].second.Env;
 auto *FooVal1 =
 cast(Env1.getValue(*FooDecl, SkipPast::None));
 EXPECT_TRUE(Env1.flowConditionImplies(*FooVal1));
 
-const Environment &Env2 = Results[0].second.Env;
+const Environment &Env2 = Results[1].second.Env;
 auto *FooVal2 =
 cast(Env2.getValue(*FooDecl, SkipPast::None));
 EXPECT_FALSE(Env2.flowConditionImplies(*FooVal2));
@@ -787,14 +787,14 @@
 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
 ASSERT_THAT(FooDecl, NotNull());
 
-ASSERT_THAT(Results, ElementsAre(Pair("p2", _), Pair("p1", _)));
+ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _)));
 
-const Environment &Env1 = Results[1].second.Env;
+const Environment &Env1 = Results[0].second.Env;
 auto *FooVal1 =
  

[PATCH] D128095: [clang] Improve diagnostics for expansion length mismatch

2022-08-29 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaTemplateVariadic.cpp:103-106
+VisitSubstNonTypeTemplateParmPackExpr(SubstNonTypeTemplateParmPackExpr *E) 
{
+  Unexpanded.push_back({E, E->getParameterPackLocation()});
+  return true;
+}

aaron.ballman wrote:
> Do we need to handle `FunctionParmPackExpr` as well?
Right, that and SubstTemplateTemplateParmPack are the two missing cases we 
could handle here, and perhaps that would allow us to get rid of that 'from 
outer parameter packs' diagnostics completely.

Would you prefer to handle everything in one patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128095

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


[PATCH] D128095: [clang] Improve diagnostics for expansion length mismatch

2022-08-29 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

just a pair of minor changes I'd like to see, otherwise this LGTM.




Comment at: clang/lib/Sema/SemaTemplateVariadic.cpp:103-106
+VisitSubstNonTypeTemplateParmPackExpr(SubstNonTypeTemplateParmPackExpr *E) 
{
+  Unexpanded.push_back({E, E->getParameterPackLocation()});
+  return true;
+}

mizvekov wrote:
> aaron.ballman wrote:
> > Do we need to handle `FunctionParmPackExpr` as well?
> Right, that and SubstTemplateTemplateParmPack are the two missing cases we 
> could handle here, and perhaps that would allow us to get rid of that 'from 
> outer parameter packs' diagnostics completely.
> 
> Would you prefer to handle everything in one patch?
I think I'm alright doing those in a followup patch.



Comment at: clang/lib/Sema/SemaTemplateVariadic.cpp:723
+  const auto *ND = ParmPack.first.get();
+  if (isa(ND)) {
+// Figure out whether we're instantiating to an argument pack or not.

Can you please make this an `else if` at the level higher?  The 'else' after 
this ends up being so small that it gets lost trying to figure out what is 
going on here.

So:

`} else if (const auto *VD = dyn_cast(ParmPack.first.get()))`



Comment at: clang/lib/Sema/SemaTemplateVariadic.cpp:865
+  const auto *ND = Unexpanded[I].first.get();
   if (isa(ND)) {
 // Function parameter pack or init-capture pack.

same comment here, see if the 'vardecl' logic can be moved up a level in the 
'if' tree.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128095

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


[PATCH] D128095: [clang] Improve diagnostics for expansion length mismatch

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



Comment at: clang/lib/Sema/SemaTemplateVariadic.cpp:103-106
+VisitSubstNonTypeTemplateParmPackExpr(SubstNonTypeTemplateParmPackExpr *E) 
{
+  Unexpanded.push_back({E, E->getParameterPackLocation()});
+  return true;
+}

erichkeane wrote:
> mizvekov wrote:
> > aaron.ballman wrote:
> > > Do we need to handle `FunctionParmPackExpr` as well?
> > Right, that and SubstTemplateTemplateParmPack are the two missing cases we 
> > could handle here, and perhaps that would allow us to get rid of that 'from 
> > outer parameter packs' diagnostics completely.
> > 
> > Would you prefer to handle everything in one patch?
> I think I'm alright doing those in a followup patch.
I'm also fine handling it in a follow-up if the changes are involved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128095

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


[PATCH] D131154: FoldingRanges: Handle LineFoldingsOnly clients.

2022-08-29 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:254
+// Remove the ending sentinal "*/" from the block comment range.
+if (Code.substr(EndOffset(*LastComment) - 2, 2) == "*/") {
+  End.character -= 2;

this is actually not true, we might have `// foo */`, so we need to check if it 
has `/*` in the beginning again.

also can we have it in an explicit variable instead `bool IsBlockComment`



Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:259
+  // Always show last line of a block comment.
+  if (Code.substr(StartOffset(*LastComment), 2) == "/*" &&
+  StartPosition(*LastComment).line != EndPosition(*LastComment).line) {

so it actually gets pretty annoying with mixture of block and line comments 
back to back, but also with multiple block comments back to back, e.g:

```
/* foo
  * bar*/
```

vs

```
/* foo */
/* bar */
```

vs

```
// foo
/* bar */
```

which is rare, so i don't think it's worth handling. but at least to make it 
consistent, rather than checking for the start/end lines of the lastcomment, we 
should actually be looking at the whole range and whether we've started the 
range with a blockcomment. (sorry for noticing it too late)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131154

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


[PATCH] D131632: [clang] Enable output of SARIF diagnostics

2022-08-29 Thread Mikael Holmén via Phabricator via cfe-commits
uabelho added inline comments.



Comment at: clang/test/Frontend/sarif-diagnostics.cpp:8
+// Omit filepath to llvm project directory
+// CHECK: 
clang/test/Frontend/sarif-diagnostics.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":12}}}],"message":{"text":"'main'
 must return 
'int'"},"ruleId":"3465","ruleIndex":0},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":11,"startColumn":11,"startLine":13}}}],"message":{"text":"use
 of undeclared identifier 
'hello'"},"ruleId":"4604","ruleIndex":1},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":17,"startColumn":17,"startLine":15}}}],"message":{"text":"invalid
 digit 'a' in decimal 
constant"},"ruleId":"898","ruleIndex":2},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":5,"startColumn":5,"startLine":19}}}],"message":{"text":"misleading
 indentation; statement is not part of the previous 
'if'"},"ruleId":"1806","ruleIndex":3},{"level":"note","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":3,"startColumn":3,"startLine":17}}}],"message":{"text":"previous
 statement is 
here"},"ruleId":"1730","ruleIndex":4},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"startColumn":10,"startLine":18}}}],"message":{"text":"unused
 variable 
'Yes'"},"ruleId":"6539","ruleIndex":5},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":12,"startColumn":12,"startLine":21}}}],"message":{"text":"use
 of undeclared identifier 
'hi'"},"ruleId":"4604","ruleIndex":6},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":23}}}],"message":{"text":"extraneous
 closing brace 
('}')"},"ruleId":"1399","ruleIndex":7},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":6,"endLine":27,"startColumn":5,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"endLine":27,"startColumn":9,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":7,"startColumn":7,"startLine":27}}}],"message":{"text":"invalid
 operands to binary expression ('t1' and 
't1')"},"ruleId":"4539","ruleIndex":8}],"tool":{"driver":{"fullName":"","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"clang","rules":[{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"3465","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4604","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"898","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"1806","name":""},{"defaultConfiguration":{"enabled":true,"level":"note","rank":-1},"fullDescription":{"text":""},"id":"1730","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"6539","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4604","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"1399","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4539","name":""}],"version":"16.0.0"}}}],"version":"2.1.0"}
+// CHECK: 2 warnings and 6 errors generated.

dyung wrote:
> dyung wrote:
> > Can this CHECK line be broken into smaller pieces? This test is failing on 
> > our internal bot, and I'm going crosseyed trying to figure out what the 
> > difference is because the line is 3741 characters long!
> Is there any significance to the "ruleId" and "Id" values in this test? If 
> not, can we just use a regex to check for a number? Our internal build with 
> private changes is failing due to slightly different numbers in some of the 
> "ruleId" fields
+1 on this. We also get different "ruleId" and "Id" values downstream and it's 
quite painful trying to figure out where the diffs are in a +3700 character 
CHECK.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

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


[PATCH] D131979: [clang][UBSan] Fix __builtin_assume_aligned crash

2022-08-29 Thread Wang Yihan via Phabricator via cfe-commits
yihanaa added a comment.

pin~


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131979

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


[PATCH] D132829: [clang][Interp] Handle ImplictValueInitExprs

2022-08-29 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision.
erichkeane added a comment.
This revision is now accepted and ready to land.

I assume we're going to see more patches in the future in this space as you 
discover examples/tests for this, but in order to unblock other tests, this is 
good enough for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132829

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


[PATCH] D132352: Introduce noread_thread_id to address the thread identification problem in coroutines

2022-08-29 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment.

Okay, this is a bit tricky because we have three different things:

1. The noread_thread_id attribute, the lack of which was causing issues with 
intrinsics in the previous version
2. The meaning of the readnone (etc) attributes, which for pragmatic reasons 
has to exclude thread IDs for now
3. The meaning of doesNotReadMemory() etc queries, which in the previous 
version included thread ID accesses, but in the new version require a separate 
call

I think my question here would be why this did not stick with the previous 
implementation approach that also affects doesNotReadMemory and AA queries (and 
thus makes everything "automatically correct"), and only added the 
noread_thread_id attribute to make intrinsic handling more precise?

My general vision for this area was that after D130896 
, we would add ThreadID as an additional 
ModRef location, which gets removed for non-presplit-coroutines due to being 
constant. This would follow the interpretation that the thread ID is part of 
"memory" though, which kind of goes against the approach here.


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

https://reviews.llvm.org/D132352

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


[PATCH] D132832: [clang][Interp] Handle missing local initializers better

2022-08-29 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:245
+
+if (!Init)
+  return false;

Would be nice to test this before the work to allocate Offset?



Comment at: clang/lib/AST/Interp/Function.h:163
+  /// Flag to indicate if the function is done being
+  /// compile to bytecode.
+  bool IsFullyCompiled = false;




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

https://reviews.llvm.org/D132832

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


[clang] b345be1 - Make this test slightly less fragile; NFC

2022-08-29 Thread Aaron Ballman via cfe-commits

Author: Aaron Ballman
Date: 2022-08-29T09:41:23-04:00
New Revision: b345be177d03166add391f090fd0288a23413934

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

LOG: Make this test slightly less fragile; NFC

The rule IDs are not stable, so this uses a regex for the rule ids
instead of concrete values. It also moves the CHECK lines below the
code so that it's easier to modify the test in the future. It also
breaks the CHECK lines into multiple lines to improve the performance
of the test and aid in debugging failures. Finally, it adds a comment
to the top of the test explaining that things are still rather fragile.

Added: 


Modified: 
clang/test/Frontend/sarif-diagnostics.cpp

Removed: 




diff  --git a/clang/test/Frontend/sarif-diagnostics.cpp 
b/clang/test/Frontend/sarif-diagnostics.cpp
index 0ff9bfdd10d84..c7512bd1777f4 100644
--- a/clang/test/Frontend/sarif-diagnostics.cpp
+++ b/clang/test/Frontend/sarif-diagnostics.cpp
@@ -1,14 +1,14 @@
 // RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif %s > %t 
2>&1 || true
 // RUN: FileCheck -dump-input=always %s --input-file=%t
-// CHECK: warning: diagnostic formatting in SARIF mode is currently unstable 
[-Wsarif-format-unstable]
-// CHECK: 
{"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":
-// Omit exact length of this file
-// CHECK: ,"location":{"index":0,"uri":"file://
-// Omit filepath to llvm project directory
-// CHECK: 
clang/test/Frontend/sarif-diagnostics.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":12}}}],"message":{"text":"'main'
 must return 
'int'"},"ruleId":"3465","ruleIndex":0},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":11,"startColumn":11,"startLine":13}}}],"message":{"text":"use
 of undeclared identifier 
'hello'"},"ruleId":"4604","ruleIndex":1},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":17,"startColumn":17,"startLine":15}}}],"message":{"text":"invalid
 digit 'a' in decimal 
constant"},"ruleId":"898","ruleIndex":2},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":5,"startColumn":5,"startLine":19}}}],"message":{"text":"misleading
 indentation; statement is not part of the previous 
'if'"},"ruleId":"1806","ruleIndex":3},{"level":"note","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":3,"startColumn":3,"startLine":17}}}],"message":{"text":"previous
 statement is 
here"},"ruleId":"1730","ruleIndex":4},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"startColumn":10,"startLine":18}}}],"message":{"text":"unused
 variable 
'Yes'"},"ruleId":"6539","ruleIndex":5},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":12,"startColumn":12,"startLine":21}}}],"message":{"text":"use
 of undeclared identifier 
'hi'"},"ruleId":"4604","ruleIndex":6},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":23}}}],"message":{"text":"extraneous
 closing brace 
('}')"},"ruleId":"1399","ruleIndex":7},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":6,"endLine":27,"startColumn":5,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"endLine":27,"startColumn":9,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":7,"startColumn":7,"startLine":27}}}],"message":{"text":"invalid
 operands to binary expression ('t1' and 
't1')"},"ruleId":"4539","ruleIndex":8}],"tool":{"driver":{"fullName":"","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"clang","rules":[{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"3465","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4604","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"898","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"1806","name":""},{"defaultConfiguration":{"enabled":true,"level":"note","rank":-1},"fullDescription":{"text":""},"id":"1730","name":""},{"defaultConfiguration":{"enabled":true,"level"

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

2022-08-29 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

just my 2 cents




Comment at: clang-tools-extra/clang-tidy/readability/UseEarlyExitsCheck.cpp:66
+  if (needsParensAfterUnaryNegation(Condition)) {
+Diag << FixItHint::CreateInsertion(Condition->getBeginLoc(), "!(")
+ << FixItHint::CreateInsertion(

did you consider comma expressions?

`if (myDirtyCode(), myCondition && yourCondition)`. It seems to me, that the 
transformation would be incorrect.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability/use-early-exits.rst:55
+
+  If `true`, split up conditions with cunjunctions (``&&``) into multiple 
+  ``if`` statements. Default value is `false`.

typo `cunjunctions -> conjunctions`



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability/use-early-exits.rst:63
+void Process(bool A, bool B) {
+  if (A && B) {
+// Long processing.

if this option is false, the transformation would be `if(!(A && B))`, right?

should demorgan rules be applied or at least be mentioned here? I think 
transforming to `if (!A || !B)` is at least a viable option for enough users.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability/use-early-exits.rst:112
+
+  If `true`, the check will only report ifs which contain nested control 
blocks.
+  Default value is `false`

maybe highlight the `if` as code with double quotes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130181

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


[PATCH] D131632: [clang] Enable output of SARIF diagnostics

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



Comment at: clang/test/Frontend/sarif-diagnostics.cpp:8
+// Omit filepath to llvm project directory
+// CHECK: 
clang/test/Frontend/sarif-diagnostics.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":12}}}],"message":{"text":"'main'
 must return 
'int'"},"ruleId":"3465","ruleIndex":0},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":11,"startColumn":11,"startLine":13}}}],"message":{"text":"use
 of undeclared identifier 
'hello'"},"ruleId":"4604","ruleIndex":1},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":17,"startColumn":17,"startLine":15}}}],"message":{"text":"invalid
 digit 'a' in decimal 
constant"},"ruleId":"898","ruleIndex":2},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":5,"startColumn":5,"startLine":19}}}],"message":{"text":"misleading
 indentation; statement is not part of the previous 
'if'"},"ruleId":"1806","ruleIndex":3},{"level":"note","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":3,"startColumn":3,"startLine":17}}}],"message":{"text":"previous
 statement is 
here"},"ruleId":"1730","ruleIndex":4},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"startColumn":10,"startLine":18}}}],"message":{"text":"unused
 variable 
'Yes'"},"ruleId":"6539","ruleIndex":5},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":12,"startColumn":12,"startLine":21}}}],"message":{"text":"use
 of undeclared identifier 
'hi'"},"ruleId":"4604","ruleIndex":6},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":23}}}],"message":{"text":"extraneous
 closing brace 
('}')"},"ruleId":"1399","ruleIndex":7},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":6,"endLine":27,"startColumn":5,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"endLine":27,"startColumn":9,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":7,"startColumn":7,"startLine":27}}}],"message":{"text":"invalid
 operands to binary expression ('t1' and 
't1')"},"ruleId":"4539","ruleIndex":8}],"tool":{"driver":{"fullName":"","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"clang","rules":[{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"3465","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4604","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"898","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"1806","name":""},{"defaultConfiguration":{"enabled":true,"level":"note","rank":-1},"fullDescription":{"text":""},"id":"1730","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"6539","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4604","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"1399","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4539","name":""}],"version":"16.0.0"}}}],"version":"2.1.0"}
+// CHECK: 2 warnings and 6 errors generated.

uabelho wrote:
> dyung wrote:
> > dyung wrote:
> > > Can this CHECK line be broken into smaller pieces? This test is failing 
> > > on our internal bot, and I'm going crosseyed trying to figure out what 
> > > the difference is because the line is 3741 characters long!
> > Is there any significance to the "ruleId" and "Id" values in this test? If 
> > not, can we just use a regex to check for a number? Our internal build with 
> > private changes is failing due to slightly different numbers in some of the 
> > "ruleId" fields
> +1 on this. We also get different "ruleId" and "Id" values downstream and 
> it's quite painful trying to figure out where the diffs are in a +3700 
> character CHECK.
I ran into this problem this morning as well. I've improved the situation 
somewhat in b345be177d03166add391f090fd0288a23413934, but I think we really 
could use some better diff tooling than FileCheck for diffing between two JSON 
files.

The test can still be made less fragile despite the changes I made, which wer

[PATCH] D132832: [clang][Interp] Handle missing local initializers better

2022-08-29 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:245
+
+if (!Init)
+  return false;

erichkeane wrote:
> Would be nice to test this before the work to allocate Offset?
Ah, sure. Good catch.


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

https://reviews.llvm.org/D132832

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


[PATCH] D132832: [clang][Interp] Handle missing local initializers better

2022-08-29 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 456328.
tbaeder marked 2 inline comments as done.

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

https://reviews.llvm.org/D132832

Files:
  clang/lib/AST/Interp/ByteCodeEmitter.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/Function.h
  clang/test/AST/Interp/cxx20.cpp

Index: clang/test/AST/Interp/cxx20.cpp
===
--- clang/test/AST/Interp/cxx20.cpp
+++ clang/test/AST/Interp/cxx20.cpp
@@ -2,8 +2,6 @@
 // RUN: %clang_cc1 -std=c++20 -verify=ref %s
 
 
-// expected-no-diagnostics
-// ref-no-diagnostics
 constexpr int getMinus5() {
   int a = 10;
   a = -5;
@@ -53,3 +51,12 @@
   return v;
 }
 //static_assert(pointerAssign2() == 12, ""); TODO
+
+
+constexpr int unInitLocal() {
+  int a;
+  return a; // ref-note{{read of uninitialized object}}
+}
+static_assert(unInitLocal() == 0, ""); // expected-error {{not an integral constant expression}} \
+   // ref-error {{not an integral constant expression}} \
+   // ref-note {{in call to 'unInitLocal()'}}
Index: clang/lib/AST/Interp/Function.h
===
--- clang/lib/AST/Interp/Function.h
+++ clang/lib/AST/Interp/Function.h
@@ -112,6 +112,9 @@
   /// Checks if the function is a constructor.
   bool isConstructor() const { return isa(F); }
 
+  /// Checks if the function is fully done compiling.
+  bool isFullyCompiled() const { return IsFullyCompiled; }
+
 private:
   /// Construct a function representing an actual function.
   Function(Program &P, const FunctionDecl *F, unsigned ArgSize,
@@ -128,6 +131,8 @@
 IsValid = true;
   }
 
+  void setIsFullyCompiled(bool FC) { IsFullyCompiled = FC; }
+
 private:
   friend class Program;
   friend class ByteCodeEmitter;
@@ -154,6 +159,9 @@
   llvm::DenseMap Params;
   /// Flag to indicate if the function is valid.
   bool IsValid = false;
+  /// Flag to indicate if the function is done being
+  /// compiled to bytecode.
+  bool IsFullyCompiled = false;
 
 public:
   /// Dumps the disassembled bytecode to \c llvm::errs().
Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -238,12 +238,18 @@
 
   // Integers, pointers, primitives.
   if (Optional T = this->classify(VD->getType())) {
+const Expr *Init = VD->getInit();
+
+if (!Init)
+  return false;
+
 auto Offset =
 this->allocateLocalPrimitive(VD, *T, VD->getType().isConstQualified());
-// Compile the initialiser in its own scope.
+
+// Compile the initializer in its own scope.
 {
   ExprScope Scope(this);
-  if (!this->visit(VD->getInit()))
+  if (!this->visit(Init))
 return false;
 }
 // Set the value.
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -699,6 +699,14 @@
 }
 assert(Func);
 
+// If the function is being compiled right now, this is a recursive call.
+// In that case, the function can't be valid yet, even though it will be
+// later.
+// If the function is already fully compiled but not constexpr, it was
+// found to be faulty earlier on, so bail out.
+if (Func->isFullyCompiled() && !Func->isConstexpr())
+  return false;
+
 QualType ReturnType = E->getCallReturnType(Ctx.getASTContext());
 Optional T = classify(ReturnType);
 
Index: clang/lib/AST/Interp/ByteCodeEmitter.cpp
===
--- clang/lib/AST/Interp/ByteCodeEmitter.cpp
+++ clang/lib/AST/Interp/ByteCodeEmitter.cpp
@@ -62,8 +62,10 @@
 // Return a dummy function if compilation failed.
 if (BailLocation)
   return llvm::make_error(*BailLocation);
-else
+else {
+  Func->setIsFullyCompiled(true);
   return Func;
+}
   } else {
 // Create scopes from descriptors.
 llvm::SmallVector Scopes;
@@ -74,6 +76,7 @@
 // Set the function's code.
 Func->setCode(NextLocalOffset, std::move(Code), std::move(SrcMap),
   std::move(Scopes));
+Func->setIsFullyCompiled(true);
 return Func;
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D132786: [clang-tidy] Fix a false positive in bugprone-assignment-in-if-condition

2022-08-29 Thread dodohand via Phabricator via cfe-commits
dodohand added a comment.

IMO you have just introduced a bug, not fixed one.
A lambda expression in an if statement condition clause is exactly the kind of 
thing that this checker was designed to flag.
The BARR group coding guideline, with which this is intended to comply in 
spirit, makes no exception for lambda expressions. (see 8.2c of 
https://barrgroup.com/sites/default/files/barr_c_coding_standard_2018.pdf)

It would seem to me that rather than this change as a default behavior, it 
would be better if @njames93 created an option flag for this checker which 
enabled this exclusion as a special case. This would preserve the intent of the 
original check, while also allowing the use case which njames93 has in mind.

Is it possible to retroactively reject/un-commit a contribution?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132786

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


[PATCH] D131939: [clang-tidy] Add performance-expensive-flat-container-operation check

2022-08-29 Thread Nicolas van Kempen via Phabricator via cfe-commits
nicovank added a comment.

Ping.

I feel like Phabricator is not sending notifications for this patch like it 
usually does (I'm not getting any emails).
I'll create a new, identical patch tomorrow if there's still no activity.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131939

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


[PATCH] D131939: [clang-tidy] Add performance-expensive-flat-container-operation check

2022-08-29 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment.

I got notification.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131939

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


[PATCH] D132851: Further update -Wbitfield-constant-conversion for 1-bit bitfield

2022-08-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman created this revision.
aaron.ballman added reviewers: ShawnZhong, thakis, clang-language-wg, 
erichkeane.
Herald added a project: All.
aaron.ballman requested review of this revision.
Herald added a project: clang.

https://reviews.llvm.org/D131255 (82afc9b169a67e8b8a1862fb9c41a2cd974d6691 
) began 
warning about conversion causing data loss for a single-bit bit-field. However, 
after landing the changes, there were reports about significant false positives 
from some code bases.

This alters the approach taken in that patch by introducing a new warning group 
(`-Wsingle-bit-bitfield-constant-conversion`) which is grouped under 
`-Wbitfield-constant-conversion` to allow users to selectively disable the 
single-bit warning without losing the other constant conversion warnings.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132851

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CXX/drs/dr6xx.cpp
  clang/test/Sema/constant-conversion.c

Index: clang/test/Sema/constant-conversion.c
===
--- clang/test/Sema/constant-conversion.c
+++ clang/test/Sema/constant-conversion.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-apple-darwin %s
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,one-bit -triple x86_64-apple-darwin %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-single-bit-bitfield-constant-conversion -verify -triple x86_64-apple-darwin %s
 
 // This file tests -Wconstant-conversion, a subcategory of -Wconversion
 // which is on by default.
@@ -23,7 +24,7 @@
   s.b = -2; // expected-warning {{implicit truncation from 'int' to bit-field changes value from -2 to 0}}
   s.b = -1; // no-warning
   s.b = 0;  // no-warning
-  s.b = 1;  // expected-warning {{implicit truncation from 'int' to bit-field changes value from 1 to -1}}
+  s.b = 1;  // one-bit-warning {{implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1}}
   s.b = 2;  // expected-warning {{implicit truncation from 'int' to bit-field changes value from 2 to 0}}
 }
 
Index: clang/test/CXX/drs/dr6xx.cpp
===
--- clang/test/CXX/drs/dr6xx.cpp
+++ clang/test/CXX/drs/dr6xx.cpp
@@ -911,9 +911,9 @@
 namespace dr675 { // dr675: dup 739
   template struct A { T n : 1; };
 #if __cplusplus >= 201103L
-  static_assert(A{1}.n < 0, ""); // expected-warning {{implicit truncation from 'int' to bit-field changes value from 1 to -1}}
-  static_assert(A{1}.n < 0, ""); // expected-warning {{implicit truncation from 'int' to bit-field changes value from 1 to -1}}
-  static_assert(A{1}.n < 0, ""); // expected-warning {{implicit truncation from 'int' to bit-field changes value from 1 to -1}}
+  static_assert(A{1}.n < 0, ""); // expected-warning {{implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1}}
+  static_assert(A{1}.n < 0, ""); // expected-warning {{implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1}}
+  static_assert(A{1}.n < 0, ""); // expected-warning {{implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1}}
 #endif
 }
 
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13067,9 +13067,12 @@
   std::string PrettyValue = toString(Value, 10);
   std::string PrettyTrunc = toString(TruncatedValue, 10);
 
-  S.Diag(InitLoc, diag::warn_impcast_bitfield_precision_constant)
-<< PrettyValue << PrettyTrunc << OriginalInit->getType()
-<< Init->getSourceRange();
+  bool IsOneBit = FieldWidth == 1 && Value == 1;
+  S.Diag(InitLoc, IsOneBit
+  ? diag::warn_impcast_single_bit_bitield_precision_constant
+  : diag::warn_impcast_bitfield_precision_constant)
+  << PrettyValue << PrettyTrunc << OriginalInit->getType()
+  << Init->getSourceRange();
 
   return true;
 }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3846,6 +3846,9 @@
 def warn_impcast_integer_precision_constant : Warning<
   "implicit conversion from %2 to %3 changes value from %0 to %1">,
   InGroup;
+def warn_impcast_single_bit_bitield_precision_constant : Warning<
+  "implicit truncation from %2 to a one-bit wide bit-field changes value from "
+  "%0 to %1">, InGroup;
 def warn_impcast_bitfield_precision_constant : Warning<
   "implicit truncation from %2 to bit-field changes value from %0 to %1">,
   InGroup;
Index: clang/include/clang/Basic/Dia

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

2022-08-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D131255#3729304 , @aaron.ballman 
wrote:

> After some more thought and some offline discussions, I think I have a 
> reasonable way forward: let's add `-Wsingle-bit-bitfield-constant-conversion` 
> as a new warning group under `-Wbitfield-constant-conversion` that controls 
> the diagnostic for one-bit bitfields. The diagnostic behavior here is 
> pedantically correct and will help catch some bugs for folks, but there's 
> enough existing code using `1` and not misbehaving (probably because they're 
> not inspecting the value except in a boolean context) that having separate 
> control seems useful.
>
> What do others think?

I went ahead and posted https://reviews.llvm.org/D132851.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131255

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


[PATCH] D131939: [clang-tidy] Add performance-expensive-flat-container-operation check

2022-08-29 Thread Nicolas van Kempen via Phabricator via cfe-commits
nicovank added a comment.

In D131939#3755399 , @Eugene.Zelenko 
wrote:

> I got notification.

Thank you. I got an email for this message but not my own updates, which 
usually also CC me 🤷.
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131939

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


[PATCH] D131154: FoldingRanges: Handle LineFoldingsOnly clients.

2022-08-29 Thread Utkarsh Saxena via Phabricator via cfe-commits
usaxena95 updated this revision to Diff 456335.
usaxena95 marked 3 inline comments as done.
usaxena95 added a comment.

Addressed comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131154

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticSelection.cpp
  clang-tools-extra/clangd/SemanticSelection.h
  clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
@@ -196,7 +196,7 @@
   ElementsAre(SourceAnnotations.range("empty")));
 }
 
-TEST(FoldingRanges, All) {
+TEST(FoldingRanges, ASTAll) {
   const char *Tests[] = {
   R"cpp(
 #define FOO int foo() {\
@@ -265,7 +265,7 @@
   }
 }
 
-TEST(FoldingRangesPseudoParser, All) {
+TEST(FoldingRanges, PseudoParserWithoutLineFoldings) {
   const char *Tests[] = {
   R"cpp(
 #define FOO int foo() {\
@@ -336,36 +336,123 @@
 ]]};
   )cpp",
   R"cpp(
-[[/* Multi 
+/*[[ Multi 
   * line
   *  comment 
-  */]]
+  ]]*/
   )cpp",
   R"cpp(
-[[// Comment
+//[[ Comment
 // 1]]
 
-[[// Comment
+//[[ Comment
 // 2]]
 
 // No folding for single line comment.
 
-[[/* comment 3
-*/]]
+/*[[ comment 3
+]]*/
 
-[[/* comment 4
-*/]]
+/*[[ comment 4
+]]*/
+
+/*[[ foo */
+/* bar ]]*/
+
+/*[[ foo */
+// baz
+/* bar ]]*/
+
+/*[[ foo */
+/* bar*/
+// baz]]
+
+//[[ foo
+/* bar */]]
   )cpp",
   };
   for (const char *Test : Tests) {
 auto T = Annotations(Test);
-EXPECT_THAT(
-gatherFoldingRanges(llvm::cantFail(getFoldingRanges(T.code().str(,
-UnorderedElementsAreArray(T.ranges()))
+EXPECT_THAT(gatherFoldingRanges(llvm::cantFail(getFoldingRanges(
+T.code().str(), /*LineFoldingsOnly=*/false))),
+UnorderedElementsAreArray(T.ranges()))
 << Test;
   }
 }
 
+TEST(FoldingRanges, PseudoParserLineFoldingsOnly) {
+  const char *Tests[] = {
+  R"cpp(
+void func(int a) {[[
+a++;]]
+}
+  )cpp",
+  R"cpp(
+// Always exclude last line for brackets.
+void func(int a) {[[
+  if(a == 1) {[[
+a++;]]
+  } else if (a == 2){[[
+a--;]]
+  } else {  // No folding for 2 line bracketed ranges.
+  }]]
+}
+  )cpp",
+  R"cpp(
+/*[[ comment
+* comment]]
+*/
+
+/* No folding for this comment.
+*/
+
+// No folding for this comment.
+
+//[[ 2 single line comment.
+// 2 single line comment.]]
+
+//[[ >=2 line comments.
+// >=2 line comments.
+// >=2 line comments.]]
+
+//[[ foo\
+bar\
+baz]]
+
+/*[[ foo */
+/* bar */]]
+/* baz */
+
+/*[[ foo */
+/* bar]]
+* This does not fold me */
+
+//[[ foo
+/* bar */]]
+  )cpp",
+  // FIXME: Support folding template arguments.
+  // R"cpp(
+  // template <[[typename foo, class bar]]> struct baz {};
+  // )cpp",
+
+  };
+  auto StripColumns = [](const std::vector &Ranges) {
+std::vector Res;
+for (Range R : Ranges) {
+  R.start.character = R.end.character = 0;
+  Res.push_back(R);
+}
+return Res;
+  };
+  for (const char *Test : Tests) {
+auto T = Annotations(Test);
+EXPECT_THAT(
+StripColumns(gatherFoldingRanges(llvm::cantFail(
+getFoldingRanges(T.code().str(), /*LineFoldingsOnly=*/true,
+UnorderedElementsAreArray(StripColumns(T.ranges(
+<< Test;
+  }
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/SemanticSelection.h
===
--- clang-tools-extra/clangd/SemanticSelection.h
+++ clang-tools-extra/clangd/SemanticSelection.h
@@ -33,7 +33,7 @@
 /// Returns a list of ranges whose contents might be collapsible in an editor.
 /// This version uses the pseudoparser which does not require the AST.
 llvm::Expected>
-getFoldingRanges(const std::string &Code);
+getFoldingRanges(const std::string &Code, bool LineFoldingOnly);
 
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/SemanticSelection.cpp
===

[PATCH] D132851: Further update -Wbitfield-constant-conversion for 1-bit bitfield

2022-08-29 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

LGTM, questioning whether we want to do some macro introspection in C mode (as 
the CORRECT way to use these 1 bit-bitfields is as a bool), but at least this 
gives the person the ability to disable this warning.




Comment at: clang/test/Sema/constant-conversion.c:27
   s.b = 0;  // no-warning
-  s.b = 1;  // expected-warning {{implicit truncation from 'int' to bit-field 
changes value from 1 to -1}}
+  s.b = 1;  // one-bit-warning {{implicit truncation from 'int' to a one-bit 
wide bit-field changes value from 1 to -1}}
   s.b = 2;  // expected-warning {{implicit truncation from 'int' to bit-field 
changes value from 2 to 0}}

Can you add a test that shows this isn't diagnosed:

`s.b = true;`?

Thanks to stdbool.h, this of course might have to be in the C++ test (or C23).  
Makes me question whether we need to do some detection to see if someone has 
written the above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132851

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


[PATCH] D123630: [WIP] Remove connection between 'ffast-math' and 'ffp-contract'.

2022-08-29 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 456337.

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

https://reviews.llvm.org/D123630

Files:
  clang/docs/UsersManual.rst
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/ffp-contract-option.c
  clang/test/Driver/fp-contract.c

Index: clang/test/Driver/fp-contract.c
===
--- /dev/null
+++ clang/test/Driver/fp-contract.c
@@ -0,0 +1,114 @@
+// Test that -ffp-contract is set to the right value when combined with
+// the options -ffast-math and -fno-fast-math.
+
+// RUN: %clang -### -ffast-math -c %s 2>&1  \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+// CHECK-FPC-FAST: "-ffp-contract=fast"
+
+// RUN: %clang -### -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+// CHECK-FPC-ON:   "-ffp-contract=on"
+
+// RUN: %clang -### -ffast-math -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-OFF %s
+// CHECK-FPC-OFF:  "-ffp-contract=off"
+
+// RUN: %clang -### -ffast-math -ffp-contract=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=fast -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+// RUN: %clang -### -ffp-contract=on -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+// RUN: %clang -### -ffp-contract=off -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=fast -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+// RUN: %clang -### -ffp-contract=on -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+// RUN: %clang -### -ffp-contract=off -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-OFF %s
+
+
+// RUN: %clang -### -ffast-math -ffp-contract=fast -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=on -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-OFF %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=on -ffp-contract=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=off -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=off -ffp-contract=fast \
+// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=on -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=off -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-OFF %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=fast -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffast-math -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -fno-fast-math -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -fno-fast-math -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -fno-fast-math -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-OFF %s
+
+// RUN: %clang -### -fno-fast-math -ffp-contract=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=fast -fno-fast-math -ffp-contract=on \
+// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffp-contract=fast -fno-fast-math -ffp-contract=off \
+// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-OFF %s
+
+// RUN: %clang -### -ffp-contract=off -fno-fast-math -ffp-contract=fast \
+// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=off -fno-fast-math -ffp-contract=on \
+// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffp-contract=on -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=off -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=fast -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=on -ffast-math -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffp-contract=off -ffast-math -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-OFF %s
+
+// RUN: %clang -### -ffp-contract=fast -ffast-math -fno-

[PATCH] D132821: [clang][Parse] Fix crash when emitting template diagnostic

2022-08-29 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments.



Comment at: clang/test/Parser/cxx-concept-declaration.cpp:3
 // Support parsing of concepts
 // Disabled for now.
 

I guess we can drop these two lines now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132821

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


[PATCH] D132851: Further update -Wbitfield-constant-conversion for 1-bit bitfield

2022-08-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 456343.
aaron.ballman added a comment.

Added a test case for `true` in C++, and added similar logic for C to reduce 
the chance for false positives. If the user uses `true` (the macro from 
stdbool.h), they're signaling an intent that the field is used in boolean 
contexts. The macro expands to `1` (per spec) but should not be diagnosed 
because the actual value stored only needs to be nonzero to have the correct 
boolean semantics.


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

https://reviews.llvm.org/D132851

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CXX/drs/dr6xx.cpp
  clang/test/Sema/constant-conversion.c
  clang/test/SemaCXX/constant-conversion.cpp

Index: clang/test/SemaCXX/constant-conversion.cpp
===
--- clang/test/SemaCXX/constant-conversion.cpp
+++ clang/test/SemaCXX/constant-conversion.cpp
@@ -22,3 +22,12 @@
   char ok3 = true ? 0 : 9 + 1;
   char ok4 = true ? 0 : nines() + 1;
 }
+
+void test_bitfield() {
+  struct S {
+int one_bit : 1;
+  } s;
+
+  s.one_bit = 1;// expected-warning {{implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1}}
+  s.one_bit = true; // no-warning
+}
Index: clang/test/Sema/constant-conversion.c
===
--- clang/test/Sema/constant-conversion.c
+++ clang/test/Sema/constant-conversion.c
@@ -1,4 +1,7 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-apple-darwin %s
+// RUN: %clang_cc1 -fsyntax-only -ffreestanding -verify=expected,one-bit -triple x86_64-apple-darwin %s
+// RUN: %clang_cc1 -fsyntax-only -ffreestanding -Wno-single-bit-bitfield-constant-conversion -verify -triple x86_64-apple-darwin %s
+
+#include 
 
 // This file tests -Wconstant-conversion, a subcategory of -Wconversion
 // which is on by default.
@@ -19,12 +22,14 @@
 int b : 1;  // The only valid values are 0 and -1.
   } s;
 
-  s.b = -3; // expected-warning {{implicit truncation from 'int' to bit-field changes value from -3 to -1}}
-  s.b = -2; // expected-warning {{implicit truncation from 'int' to bit-field changes value from -2 to 0}}
-  s.b = -1; // no-warning
-  s.b = 0;  // no-warning
-  s.b = 1;  // expected-warning {{implicit truncation from 'int' to bit-field changes value from 1 to -1}}
-  s.b = 2;  // expected-warning {{implicit truncation from 'int' to bit-field changes value from 2 to 0}}
+  s.b = -3;// expected-warning {{implicit truncation from 'int' to bit-field changes value from -3 to -1}}
+  s.b = -2;// expected-warning {{implicit truncation from 'int' to bit-field changes value from -2 to 0}}
+  s.b = -1;// no-warning
+  s.b = 0; // no-warning
+  s.b = 1; // one-bit-warning {{implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1}}
+  s.b = true;  // no-warning (we suppress it manually to reduce false positives)
+  s.b = false; // no-warning
+  s.b = 2; // expected-warning {{implicit truncation from 'int' to bit-field changes value from 2 to 0}}
 }
 
 enum Test2 { K_zero, K_one };
Index: clang/test/CXX/drs/dr6xx.cpp
===
--- clang/test/CXX/drs/dr6xx.cpp
+++ clang/test/CXX/drs/dr6xx.cpp
@@ -911,9 +911,9 @@
 namespace dr675 { // dr675: dup 739
   template struct A { T n : 1; };
 #if __cplusplus >= 201103L
-  static_assert(A{1}.n < 0, ""); // expected-warning {{implicit truncation from 'int' to bit-field changes value from 1 to -1}}
-  static_assert(A{1}.n < 0, ""); // expected-warning {{implicit truncation from 'int' to bit-field changes value from 1 to -1}}
-  static_assert(A{1}.n < 0, ""); // expected-warning {{implicit truncation from 'int' to bit-field changes value from 1 to -1}}
+  static_assert(A{1}.n < 0, ""); // expected-warning {{implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1}}
+  static_assert(A{1}.n < 0, ""); // expected-warning {{implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1}}
+  static_assert(A{1}.n < 0, ""); // expected-warning {{implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1}}
 #endif
 }
 
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13044,9 +13044,19 @@
   }
 
   llvm::APSInt Value = Result.Val.getInt();
-
   unsigned OriginalWidth = Value.getBitWidth();
 
+  // In C, the macro 'true' from stdbool.h will evaluate to '1'; To reduce
+  // false positives where the user is demonstrating they intend to use the
+  // bit-field as a Boolean, check to see if the value is 1 and we're assigning
+  // to a one-bit bit-field to see

[PATCH] D132853: [clang] Fix -Warray-bound interaction with -fstrict-flex-arrays=1

2022-08-29 Thread serge via Phabricator via cfe-commits
serge-sans-paille created this revision.
serge-sans-paille added reviewers: sberg, kees.
Herald added a project: All.
serge-sans-paille 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/D132853

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp


Index: clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp
===
--- clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp
+++ clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -verify -fstrict-flex-arrays=2 %s
+// RUN: %clang_cc1 -verify=relaxed -fstrict-flex-arrays=1 %s
+// RUN: %clang_cc1 -verify=relaxed,strict -fstrict-flex-arrays=2 %s
 
 // We cannot know for sure the size of a flexible array.
 struct t {
@@ -9,16 +10,26 @@
   s2->a[2] = 0; // no-warning
 }
 
-// Under -fstrict-flex-arrays `a` is not a flexible array.
+// Under -fstrict-flex-arrays={1,2} `a` is not a flexible array
+struct t0 {
+  int f;
+  int a[10]; // relaxed-note {{array 'a' declared here}}
+};
+void test0(t0 *s2) {
+  s2->a[12] = 0; // relaxed-warning {{array index 12 is past the end of the 
array (which contains 10 elements)}}
+}
+
+
+// Under -fstrict-flex-arrays=2 `a` is not a flexible array, but it is under 
-fstrict-flex-arrays=1
 struct t1 {
   int f;
-  int a[1]; // expected-note {{array 'a' declared here}}
+  int a[1]; // strict-note {{array 'a' declared here}}
 };
 void test1(t1 *s2) {
-  s2->a[2] = 0; // expected-warning {{array index 2 is past the end of the 
array (which contains 1 element)}}
+  s2->a[2] = 0; // strict-warning {{array index 2 is past the end of the array 
(which contains 1 element)}}
 }
 
-// Under -fstrict-flex-arrays `a` is a flexible array.
+// Under -fstrict-flex-arrays={1,2} `a` is a flexible array.
 struct t2 {
   int f;
   int a[0];
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -15852,7 +15852,7 @@
   if (StrictFlexArraysLevel >= 2 && Size != 0)
 return false;
 
-  if (StrictFlexArraysLevel == 1 && Size.ule(1))
+  if (StrictFlexArraysLevel == 1 && Size.uge(2))
 return false;
 
   // FIXME: While the default -fstrict-flex-arrays=0 permits Size>1 trailing


Index: clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp
===
--- clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp
+++ clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -verify -fstrict-flex-arrays=2 %s
+// RUN: %clang_cc1 -verify=relaxed -fstrict-flex-arrays=1 %s
+// RUN: %clang_cc1 -verify=relaxed,strict -fstrict-flex-arrays=2 %s
 
 // We cannot know for sure the size of a flexible array.
 struct t {
@@ -9,16 +10,26 @@
   s2->a[2] = 0; // no-warning
 }
 
-// Under -fstrict-flex-arrays `a` is not a flexible array.
+// Under -fstrict-flex-arrays={1,2} `a` is not a flexible array
+struct t0 {
+  int f;
+  int a[10]; // relaxed-note {{array 'a' declared here}}
+};
+void test0(t0 *s2) {
+  s2->a[12] = 0; // relaxed-warning {{array index 12 is past the end of the array (which contains 10 elements)}}
+}
+
+
+// Under -fstrict-flex-arrays=2 `a` is not a flexible array, but it is under -fstrict-flex-arrays=1
 struct t1 {
   int f;
-  int a[1]; // expected-note {{array 'a' declared here}}
+  int a[1]; // strict-note {{array 'a' declared here}}
 };
 void test1(t1 *s2) {
-  s2->a[2] = 0; // expected-warning {{array index 2 is past the end of the array (which contains 1 element)}}
+  s2->a[2] = 0; // strict-warning {{array index 2 is past the end of the array (which contains 1 element)}}
 }
 
-// Under -fstrict-flex-arrays `a` is a flexible array.
+// Under -fstrict-flex-arrays={1,2} `a` is a flexible array.
 struct t2 {
   int f;
   int a[0];
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -15852,7 +15852,7 @@
   if (StrictFlexArraysLevel >= 2 && Size != 0)
 return false;
 
-  if (StrictFlexArraysLevel == 1 && Size.ule(1))
+  if (StrictFlexArraysLevel == 1 && Size.uge(2))
 return false;
 
   // FIXME: While the default -fstrict-flex-arrays=0 permits Size>1 trailing
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D132786: [clang-tidy] Fix a false positive in bugprone-assignment-in-if-condition

2022-08-29 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

In D132786#3755390 , @dodohand wrote:

> IMO you have just introduced a bug, not fixed one.
> A lambda expression in an if statement condition clause is exactly the kind 
> of thing that this checker was designed to flag.
> The BARR group coding guideline, with which this is intended to comply in 
> spirit, makes no exception for lambda expressions. (see 8.2c of 
> https://barrgroup.com/sites/default/files/barr_c_coding_standard_2018.pdf)
>
> It would seem to me that rather than this change as a default behavior, it 
> would be better if @njames93 created an option flag for this checker which 
> enabled this exclusion as a special case. This would preserve the intent of 
> the original check, while also allowing the use case which njames93 has in 
> mind.
>
> Is it possible to retroactively reject/un-commit a contribution?

Granted that the coding guidelines this was intended for are designed for 
writing embedded C code, there was never any need to consider lambdas. As such, 
this change just makes the check a little bit more useful on C++ codebases.
Going by the name of this, it gives the impression that we are trying to detect 
bugs where an equality comparison was intended, or handle the case when an 
assignment isn't executed due to things like short circuiting rules. Neither of 
these use cases are expected when the assignment appears in the body of a 
lambda 
I'd be happy to support this as an option though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132786

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


[PATCH] D131464: [test] Make tests pass regardless of gnu++14/gnu++17 default

2022-08-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

I like the direction this is going; I ran into some questions on the tests 
about whether we should use a range or not and other small things, but I think 
this is getting close.




Comment at: clang/test/CXX/temp/temp.res/temp.local/p3.cpp:2
+// RUN: %clang_cc1 -verify=expected,precxx17 %stdcxx_98-14 %s
+// RUN: %clang_cc1 -verify=expected,cxx17 -std=c++17 %s
 

Should this be C++17+?



Comment at: clang/test/CodeGen/typedef_alignment_mismatch_warning.cpp:2
+// RUN: %clang_cc1 %s -fsyntax-only -verify=expected,precxx17 %stdcxx_11-14 
-fdata-sections -fcolor-diagnostics
+// RUN: %clang_cc1 %s -fsyntax-only -verify -std=c++17 -fdata-sections 
-fcolor-diagnostics
 

Same question here about C++17+?



Comment at: clang/test/CodeGenCXX/exception-spec-decay.cpp:1
-// RUN: %clang_cc1 -fcxx-exceptions -fexceptions %s -triple=i686-unknown-linux 
-emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %stdcxx_98- -fcxx-exceptions -fexceptions 
-Wno-dynamic-exception-spec %s -triple=i686-unknown-linux -emit-llvm -o - | 
FileCheck %s
 typedef int Array[10];

Should we drop the `%stdcxx_98-` entirely from tests and not have any `-std` 
flag (e.g., no such flags tells lit to run the test in all language modes, 
eventually)?



Comment at: clang/test/CodeGenCXX/override-bit-field-layout.cpp:2
+// RUN: %clang_cc1 %stdcxx_98-14 -w -triple=x86_64-pc-win32 -fms-compatibility 
-fdump-record-layouts-simple 
-foverride-record-layout=%S/Inputs/override-bit-field-layout.layout %s | 
FileCheck %s --check-prefixes=CHECK,PRE17
+// RUN: %clang_cc1 -std=c++17 -w -triple=x86_64-pc-win32 -fms-compatibility 
-fdump-record-layouts-simple 
-foverride-record-layout=%S/Inputs/override-bit-field-layout.layout %s | 
FileCheck %s --check-prefixes=CHECK,CXX17
 

17+?



Comment at: clang/test/CodeGenCXX/override-layout.cpp:1-9
+// RUN: %clang_cc1 -std=c++14 -w -fdump-record-layouts-simple %s > %t.layouts
+// RUN: %clang_cc1 -std=c++14 -w -fdump-record-layouts-simple %s > %t.before
+// RUN: %clang_cc1 -std=c++14 -w -DPACKED= -DALIGNED16= 
-fdump-record-layouts-simple -foverride-record-layout=%t.layouts %s > %t.after
 // RUN: diff -u %t.before %t.after
-// RUN: FileCheck %s < %t.after
+// RUN: FileCheck --check-prefixes=CHECK,PRE17 %s < %t.after
+
+// RUN: %clang_cc1 -std=c++17 -w -fdump-record-layouts-simple %s > %t.layouts

Pre 14? Post 17?



Comment at: clang/test/Layout/ms-x86-vtordisp.cpp:1-3
+// RUN: %clang_cc1 -std=c++14 -fno-rtti -fms-extensions -emit-llvm-only 
-triple i686-pc-win32 -fdump-record-layouts -fsyntax-only %s 2>&1 \
 // RUN:| FileCheck %s
+// RUN: %clang_cc1 -std=c++14 -fno-rtti -fms-extensions -emit-llvm-only 
-triple x86_64-pc-win32 -fdump-record-layouts -fsyntax-only %s 2>/dev/null \

Is this test specific to C++14?



Comment at: clang/test/Parser/cxx-casting.cpp:1-3
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++14 %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s

LG given that lit doesn't run multiple tests  yet, so switching to a range 
would lose coverage.



Comment at: clang/test/SemaCXX/libstdcxx_is_pod_hack.cpp:1
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++14 %s
 

Is this one only for C++14 or should there be a range used instead?



Comment at: llvm/utils/lit/lit/llvm/config.py:573
+if t.endswith('-'):
+t += '2b'
+l = clang_std_values.index(t[0:2])

aaron.ballman wrote:
> Maybe we can use `clang_std_values[-1]` so we don't have to hardcode this?
Thoughts?



Comment at: llvm/utils/lit/lit/llvm/config.py:579
+l = h - clang_std_group % (h-l+1)
+self.config.substitutions.append((s, '-std=c++' + 
clang_std_values[l]))
+

MaskRay wrote:
> aaron.ballman wrote:
> > One thing we should consider is whether we want to run in *all* the 
> > specified language modes instead of just the newest mode. This will make 
> > running tests slower because we'll run significantly more of them, and it 
> > might get awkward if a lot of tests change behavior in the different 
> > language modes, so I don't suggest it as part of this patch.
> This is difficult in lit. Will answer in my main comment.
It's unfortunate that it's difficult in lit. I'm fine punting on that work for 
now, but I think we should try to invest in it (or are you saying "difficult" 
as in "not worth the effort"?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131464

___
cfe-commits mailing list
cfe

[PATCH] D131154: FoldingRanges: Handle LineFoldingsOnly clients.

2022-08-29 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

thanks, lgtm!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131154

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


[PATCH] D132855: [OpenMP] Extend the lit test for uses_allocators in target region

2022-08-29 Thread Animesh Kumar via Phabricator via cfe-commits
animeshk-amd created this revision.
animeshk-amd added reviewers: saiislam, JonChesterfield.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
animeshk-amd requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

Although the `uses_allocators` clause was already present in the
test, it didn't have CHECK lines for the respective IR. Few RUN
lines are fixed based upon the OpenMP constructs being tested.
Respective CHECK lines are added for various allocators. This test
is inspired from the test 5.0/target/test_target_allocate.c of the
SOLLVE repo: https://github.com/SOLLVE/sollve_vv


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132855

Files:
  clang/test/OpenMP/target_uses_allocators.c


Index: clang/test/OpenMP/target_uses_allocators.c
===
--- clang/test/OpenMP/target_uses_allocators.c
+++ clang/test/OpenMP/target_uses_allocators.c
@@ -1,7 +1,7 @@
 // Test host codegen.
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=50  -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-emit-llvm %s -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-64
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=50  -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -x c++ -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -x c++ -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -x c++ -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
 
 // expected-no-diagnostics
 #ifndef HEADER
@@ -42,3 +42,59 @@
 }
 
 #endif
+
+// CHECK: %[[#R0:]] = call i32 @__kmpc_global_thread_num(ptr @1)
+// CHECK-NEXT: store i64 %x, ptr %x.addr, align 8
+// CHECK-NEXT: %.x..void.addr = call ptr @__kmpc_alloc(i32 %[[#R0]], i64 4, 
ptr null)
+// CHECK-NEXT: %[[#R1:]] = load i32, ptr %x.addr, align 4
+// CHECK-NEXT: store i32 %[[#R1]], ptr %.x..void.addr, align 4
+// CHECK-NEXT: call void @__kmpc_free(i32 %[[#R0]], ptr %.x..void.addr, ptr 
null)
+
+// CHECK: %[[#R0:]] = call i32 @__kmpc_global_thread_num(ptr @1)
+// CHECK-NEXT: store i64 %x, ptr %x.addr, align 8
+// CHECK-NEXT: %.x..void.addr = call ptr @__kmpc_alloc(i32 %[[#R0]], i64 4, 
ptr inttoptr (i64 1 to ptr))
+// CHECK-NEXT: %[[#R1:]] = load i32, ptr %x.addr, align 4
+// CHECK-NEXT: store i32 %[[#R1]], ptr %.x..void.addr, align 4
+// CHECK-NEXT: call void @__kmpc_free(i32 %[[#R0]], ptr %.x..void.addr, ptr 
inttoptr (i64 1 to ptr))
+
+// CHECK: %[[#R0:]] = call i32 @__kmpc_global_thread_num(ptr @1)
+// CHECK-NEXT: store i64 %x, ptr %x.addr, align 8
+// CHECK-NEXT: %.x..void.addr = call ptr @__kmpc_alloc(i32 %[[#R0]], i64 4, 
ptr inttoptr (i64 2 to ptr))
+// CHECK-NEXT: %[[#R1:]] = load i32, ptr %x.addr, align 4
+// CHECK-NEXT: store i32 %[[#R1]], ptr %.x..void.addr, align 4
+// CHECK-NEXT: call void @__kmpc_free(i32 %[[#R0]], ptr %.x..void.addr, ptr 
inttoptr (i64 2 to ptr))
+
+// CHECK: %[[#R0:]] = call i32 @__kmpc_global_thread_num(ptr @1)
+// CHECK-NEXT: store i64 %x, ptr %x.addr, align 8
+// CHECK-NEXT: %.x..void.addr = call ptr @__kmpc_alloc(i32 %[[#R0]], i64 4, 
ptr inttoptr (i64 3 to ptr))
+// CHECK-NEXT: %[[#R1:]] = load i32, ptr %x.addr, align 4
+// CHECK-NEXT: store i32 %[[#R1]], ptr %.x..void.addr, align 4
+// CHECK-NEXT: call void @__kmpc_free(i32 %[[#R0]], ptr %.x..void.addr, ptr 
inttoptr (i64 3 to ptr))
+
+// CHECK: %[[#R0:]] = call i32 @__kmpc_global_thread_num(ptr @1)
+// CHECK-NEXT: store i64 %x, ptr %x.addr, align 8
+// CHECK-NEXT: %.x..void.addr = call ptr @__kmpc_alloc(i32 %[[#R0]], i64 4, 
ptr inttoptr (i64 4 to ptr))
+// CHECK-NEXT: %[[#R1:]] = load i32, ptr %x.addr, align 4
+// CHECK-NEXT: store i32 %[[#R1]], ptr %.x..void.addr, align 4
+// CHECK-NEXT: call void @__kmpc_free(i32 %[[#R0]], ptr %.x..void.addr, ptr 
inttoptr (i64 4 to ptr))
+
+// CHECK: %[[#R0:]] = call i32 @__kmpc_global_thread_num(ptr @1)
+// CHECK-NEXT: store i64 %x, ptr %x.addr, align 8
+// CHECK-NEXT: %.x..void.addr = call ptr @__kmpc_alloc(i32 %[[#R0]], i64 4, 
ptr inttoptr (i64 5 to ptr))
+// CHECK-NEXT: %[[#R1:]] = load i32, ptr %x.addr, align 4
+// CHECK-NEXT: store i32 %[[#R1]], ptr %.x..void.addr, align 4
+// CHECK-NEXT: call void @__kmpc_free(i32 %[[#R0]], ptr %.x..void.addr, ptr 
inttoptr (i64 5 to ptr))
+
+// CHECK: %[[#R0:]] = call i32 @__kmpc_global_thread_num(ptr @1)
+// CHECK-NEXT: store i64 %x, ptr %x.addr, align 8
+// CHECK-NEXT: %.x..void.addr = call ptr @__kmpc_alloc(i32 %[[#R0]], i64 4, 
ptr inttoptr (i64 6 to ptr))
+// CHECK-NEXT: %[[#R1:]] = load i32, ptr %x.addr, align 

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

2022-08-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Precommit CI found a relevant failure:

  FAIL: Clang :: AST/Interp/arrays.cpp (67 of 15858)
   TEST 'Clang :: AST/Interp/arrays.cpp' FAILED 

  Script:
  --
  : 'RUN: at line 1';   
c:\ws\w5\llvm-project\premerge-checks\build\bin\clang.exe -cc1 
-internal-isystem 
c:\ws\w5\llvm-project\premerge-checks\build\lib\clang\16.0.0\include 
-nostdsysteminc -fexperimental-new-constant-interpreter -verify 
C:\ws\w5\llvm-project\premerge-checks\clang\test\AST\Interp\arrays.cpp
  : 'RUN: at line 2';   
c:\ws\w5\llvm-project\premerge-checks\build\bin\clang.exe -cc1 
-internal-isystem 
c:\ws\w5\llvm-project\premerge-checks\build\lib\clang\16.0.0\include 
-nostdsysteminc -verify=ref 
C:\ws\w5\llvm-project\premerge-checks\clang\test\AST\Interp\arrays.cpp
  --
  Exit Code: 1
  
  Command Output (stdout):
  --
  $ ":" "RUN: at line 1"
  $ "c:\ws\w5\llvm-project\premerge-checks\build\bin\clang.exe" "-cc1" 
"-internal-isystem" 
"c:\ws\w5\llvm-project\premerge-checks\build\lib\clang\16.0.0\include" 
"-nostdsysteminc" "-fexperimental-new-constant-interpreter" "-verify" 
"C:\ws\w5\llvm-project\premerge-checks\clang\test\AST\Interp\arrays.cpp"
  # command stderr:
  error: 'note' diagnostics expected but not seen: 
File C:\ws\w5\llvm-project\premerge-checks\clang\test\AST\Interp\arrays.cpp 
Line 49: 5 == 4
  1 error generated.
  
  error: command failed with exit status: 1
  
  --
  
  




Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:43
 protected:
-  // Emitters for opcodes of various arities.
-  using NullaryFn = bool (ByteCodeExprGen::*)(const SourceInfo &);
-  using UnaryFn = bool (ByteCodeExprGen::*)(PrimType, const SourceInfo &);
-  using BinaryFn = bool (ByteCodeExprGen::*)(PrimType, PrimType,
- const SourceInfo &);
-
-  // Aliases for types defined in the emitter.
-  using LabelTy = typename Emitter::LabelTy;
-  using AddrTy = typename Emitter::AddrTy;
-
-  // Reference to a function generating the pointer of an initialized object.s
+  // Reference to a function generating the pointer of an initialized object.
   using InitFnRef = std::function;

tbaeder wrote:
> Yes, a few of these cleanups, renames and added doc comments aren't necessary 
> for this patch. I will remove them if you want to, I just don't want to 
> rebase against `main` all the time to push NFC patches.
Please remove them; we want to keep changes logically separate per our 
developer policy. The biggest reason is: we sometimes need to revert changes 
and there's no reason to lose the good (unrelated) work at the same time, but 
also, it helps code reviewers to focus on the important parts without getting 
lost tracking which changes are for what reason.



Comment at: clang/test/AST/Interp/arrays.cpp:10
+};
+
+static_assert(foo[0][0] == nullptr, "");

I'd like to see a test for designated initialization of arrays, including 
reinitialization: https://godbolt.org/z/xfjW8xPfE


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

https://reviews.llvm.org/D132727

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


[PATCH] D132829: [clang][Interp] Handle ImplictValueInitExprs

2022-08-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

> I don't have a test case handy for them since I'm not sure how to trigger 
> them reliably, but they are easy enough to implement and I ran into them 
> while working on array fillers.

I think we should have test coverage for this if possible. 
https://godbolt.org/z/75jGrcT7v shows an example, but it might be that it's 
hard to test that example without other interpreter support first (in which 
case, you should add the example but comment out any bits that need it so that 
the test passes, along with a FIXME comment).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132829

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


[PATCH] D132821: [clang][Parse] Fix crash when emitting template diagnostic

2022-08-29 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:779
+  "|refers to a variable template||refers to a concept|"
+  "refers to a concept template}1">;
 def err_id_after_template_in_nested_name_spec : Error<

Is "concept template" a term of art? I have never heard of that before.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132821

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


[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

2022-08-29 Thread Whitney Tsang via Phabricator via cfe-commits
Whitney added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:4702
+
+  // Add or replace -ftime-trace` to the correct one to all clang jobs
+  for (auto &J : C.getJobs()) {

dongjunduo wrote:
> Whitney wrote:
> > Do you mean Add or replace the modified `-ftime-trace=` to all clang 
> > jobs?
> Right
ic, can you please have the comment updated?



Comment at: clang/lib/Driver/Driver.cpp:4739
+
+  // replace `-ftime-trace=`
+  auto &JArgs = J.getArguments();

dongjunduo wrote:
> Whitney wrote:
> > should we also replace `-ftime-trace`?
> The work before here is to infer the correct path to store the time-trace 
> file.
> 
> After that, the  in `-ftime-trace=` should be replaced by the 
> infered correct path.
> 
> We do not need to replace `-ftime-trace` then.
What happens when `-ftime-trace` is given by the user? Do you have both 
`-ftime-trace=` and `-ftime-trace` as arguments?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

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


[PATCH] D132821: [clang][Parse] Fix crash when emitting template diagnostic

2022-08-29 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:779
+  "|refers to a variable template||refers to a concept|"
+  "refers to a concept template}1">;
 def err_id_after_template_in_nested_name_spec : Error<

cjdb wrote:
> Is "concept template" a term of art? I have never heard of that before.
The enum member is called `clang::TNK_Concept_template`, so I made up "concept 
template" from that. Not sure what to use instead, would just "refers to a 
concept" be better?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132821

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


[clang] d346eb7 - [clang] Fix ambiguous use of `report_fatal_error`.

2022-08-29 Thread Wei Yi Tee via cfe-commits

Author: Wei Yi Tee
Date: 2022-08-29T15:32:49Z
New Revision: d346eb7bf08c7780bb80426eabc6b5f81490e9ae

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

LOG: [clang] Fix ambiguous use of `report_fatal_error`.

`report_fatal_error` is overloaded on `StringRef` and `Twine &`, therefore 
passing a `std::string` argument leads to ambiguity as it is convertible to 
either type.

Reviewed By: gribozavr2, sgatev

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

Added: 


Modified: 
clang/lib/Basic/SanitizerSpecialCaseList.cpp

Removed: 




diff  --git a/clang/lib/Basic/SanitizerSpecialCaseList.cpp 
b/clang/lib/Basic/SanitizerSpecialCaseList.cpp
index 5bf8d39ffd95b..2dbf04c6ede97 100644
--- a/clang/lib/Basic/SanitizerSpecialCaseList.cpp
+++ b/clang/lib/Basic/SanitizerSpecialCaseList.cpp
@@ -33,7 +33,7 @@ SanitizerSpecialCaseList::createOrDie(const 
std::vector &Paths,
   std::string Error;
   if (auto SSCL = create(Paths, VFS, Error))
 return SSCL;
-  llvm::report_fatal_error(Error);
+  llvm::report_fatal_error(StringRef(Error));
 }
 
 void SanitizerSpecialCaseList::createSanitizerSections() {



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


[PATCH] D132745: [clang] Fix ambiguous use of `report_fatal_error`.

2022-08-29 Thread weiyi via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd346eb7bf08c: [clang] Fix ambiguous use of 
`report_fatal_error`. (authored by wyt).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132745

Files:
  clang/lib/Basic/SanitizerSpecialCaseList.cpp


Index: clang/lib/Basic/SanitizerSpecialCaseList.cpp
===
--- clang/lib/Basic/SanitizerSpecialCaseList.cpp
+++ clang/lib/Basic/SanitizerSpecialCaseList.cpp
@@ -33,7 +33,7 @@
   std::string Error;
   if (auto SSCL = create(Paths, VFS, Error))
 return SSCL;
-  llvm::report_fatal_error(Error);
+  llvm::report_fatal_error(StringRef(Error));
 }
 
 void SanitizerSpecialCaseList::createSanitizerSections() {


Index: clang/lib/Basic/SanitizerSpecialCaseList.cpp
===
--- clang/lib/Basic/SanitizerSpecialCaseList.cpp
+++ clang/lib/Basic/SanitizerSpecialCaseList.cpp
@@ -33,7 +33,7 @@
   std::string Error;
   if (auto SSCL = create(Paths, VFS, Error))
 return SSCL;
-  llvm::report_fatal_error(Error);
+  llvm::report_fatal_error(StringRef(Error));
 }
 
 void SanitizerSpecialCaseList::createSanitizerSections() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D132851: Further update -Wbitfield-constant-conversion for 1-bit bitfield

2022-08-29 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision.
erichkeane added a comment.
This revision is now accepted and ready to land.

2 nits, otherwise LGTM.




Comment at: clang/lib/Sema/SemaChecking.cpp:13080
 
-  S.Diag(InitLoc, diag::warn_impcast_bitfield_precision_constant)
-<< PrettyValue << PrettyTrunc << OriginalInit->getType()
-<< Init->getSourceRange();
+  bool IsOneBit = FieldWidth == 1 && Value == 1;
+  S.Diag(InitLoc, IsOneBit

Might consider hoisting this line above the macro-checking, and just use it in 
both cases.  Also, perhaps a more descriptive name?  
`IsInvalidSignedOneBitAssignment` ? 


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

https://reviews.llvm.org/D132851

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


[PATCH] D132831: [clang][Interp] Handle SubstNonTypeTemplateParmExprs

2022-08-29 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik accepted this revision.
shafik added a comment.
This revision is now accepted and ready to land.

Makes sense to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132831

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


[PATCH] D132832: [clang][Interp] Handle missing local initializers better

2022-08-29 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik accepted this revision.
shafik added a comment.

LGTM


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

https://reviews.llvm.org/D132832

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


[PATCH] D132017: [clang][analyzer] Add errno modeling to StreamChecker

2022-08-29 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added a reviewer: martong.
balazske added inline comments.
Herald added a subscriber: rnkovacs.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:537
+  errno_modeling::getNoteTagForStdSuccess(
+  C, cast(Call.getDecl())->getNameAsString()));
   C.addTransition(StateNull);

steakhal wrote:
> I believe, `getDecl()` might return null, e.g. for calling a function pointer.
In this case the function should be known (we know that `fopen` is the called 
function, if the name is known the `Decl` should be known too), I think it can 
not be null here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132017

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


[PATCH] D127973: [analyzer] Eval construction of non POD type arrays.

2022-08-29 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:493
+  if (auto DS = dyn_cast_or_null(Item.getStmtOrNull())) {
+if (auto VD = dyn_cast_or_null(DS->getSingleDecl()))
+  E = dyn_cast(VD->getInit());

Hmm, when we are building our Cfgs, we explode DeclStatements with multiple 
declarations into separate DeclStatements, I'd expect `getSingleDecl` to never 
return `nullptr`.  Maybe we can remove the `or_null`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127973

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


[PATCH] D132831: [clang][Interp] Handle SubstNonTypeTemplateParmExprs

2022-08-29 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:273
+const SubstNonTypeTemplateParmExpr *E) {
+  return this->visit(E->getReplacement());
+}

Is there nothing special that has to happen when these are reference 
parameters?  Can you at least add a test for that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132831

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


[PATCH] D131639: [OpenMP] Remove 'stdbool.h' from OpenMP header wrappers

2022-08-29 Thread Ivan Rodriguez via Phabricator via cfe-commits
ivanrodriguez3753 added a comment.

In D131639#3749633 , @jhuber6 wrote:

> I think it's perfectly reasonable to include system files as part of a 
> toolchain.

I think it comes down to a matter of inconveniencing the user versus the 
developer. We usually go with the latter. I don't particularly agree with your 
statement I quoted above, but I can't say I'm an expert in this area. I'll see 
what others have to say. I've already pulled this into our fork so it doesn't 
make a difference to us, but I did think it would be useful to get input from 
the community on whether or not we should pull this in to upstream.

Thanks for your input and responses.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131639

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


[PATCH] D132851: Further update -Wbitfield-constant-conversion for 1-bit bitfield

2022-08-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 456366.
aaron.ballman added a comment.

Update based on review feedback.


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

https://reviews.llvm.org/D132851

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CXX/drs/dr6xx.cpp
  clang/test/Sema/constant-conversion.c
  clang/test/SemaCXX/constant-conversion.cpp

Index: clang/test/SemaCXX/constant-conversion.cpp
===
--- clang/test/SemaCXX/constant-conversion.cpp
+++ clang/test/SemaCXX/constant-conversion.cpp
@@ -22,3 +22,12 @@
   char ok3 = true ? 0 : 9 + 1;
   char ok4 = true ? 0 : nines() + 1;
 }
+
+void test_bitfield() {
+  struct S {
+int one_bit : 1;
+  } s;
+
+  s.one_bit = 1;// expected-warning {{implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1}}
+  s.one_bit = true; // no-warning
+}
Index: clang/test/Sema/constant-conversion.c
===
--- clang/test/Sema/constant-conversion.c
+++ clang/test/Sema/constant-conversion.c
@@ -1,4 +1,7 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-apple-darwin %s
+// RUN: %clang_cc1 -fsyntax-only -ffreestanding -verify=expected,one-bit -triple x86_64-apple-darwin %s
+// RUN: %clang_cc1 -fsyntax-only -ffreestanding -Wno-single-bit-bitfield-constant-conversion -verify -triple x86_64-apple-darwin %s
+
+#include 
 
 // This file tests -Wconstant-conversion, a subcategory of -Wconversion
 // which is on by default.
@@ -19,12 +22,14 @@
 int b : 1;  // The only valid values are 0 and -1.
   } s;
 
-  s.b = -3; // expected-warning {{implicit truncation from 'int' to bit-field changes value from -3 to -1}}
-  s.b = -2; // expected-warning {{implicit truncation from 'int' to bit-field changes value from -2 to 0}}
-  s.b = -1; // no-warning
-  s.b = 0;  // no-warning
-  s.b = 1;  // expected-warning {{implicit truncation from 'int' to bit-field changes value from 1 to -1}}
-  s.b = 2;  // expected-warning {{implicit truncation from 'int' to bit-field changes value from 2 to 0}}
+  s.b = -3;// expected-warning {{implicit truncation from 'int' to bit-field changes value from -3 to -1}}
+  s.b = -2;// expected-warning {{implicit truncation from 'int' to bit-field changes value from -2 to 0}}
+  s.b = -1;// no-warning
+  s.b = 0; // no-warning
+  s.b = 1; // one-bit-warning {{implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1}}
+  s.b = true;  // no-warning (we suppress it manually to reduce false positives)
+  s.b = false; // no-warning
+  s.b = 2; // expected-warning {{implicit truncation from 'int' to bit-field changes value from 2 to 0}}
 }
 
 enum Test2 { K_zero, K_one };
Index: clang/test/CXX/drs/dr6xx.cpp
===
--- clang/test/CXX/drs/dr6xx.cpp
+++ clang/test/CXX/drs/dr6xx.cpp
@@ -911,9 +911,9 @@
 namespace dr675 { // dr675: dup 739
   template struct A { T n : 1; };
 #if __cplusplus >= 201103L
-  static_assert(A{1}.n < 0, ""); // expected-warning {{implicit truncation from 'int' to bit-field changes value from 1 to -1}}
-  static_assert(A{1}.n < 0, ""); // expected-warning {{implicit truncation from 'int' to bit-field changes value from 1 to -1}}
-  static_assert(A{1}.n < 0, ""); // expected-warning {{implicit truncation from 'int' to bit-field changes value from 1 to -1}}
+  static_assert(A{1}.n < 0, ""); // expected-warning {{implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1}}
+  static_assert(A{1}.n < 0, ""); // expected-warning {{implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1}}
+  static_assert(A{1}.n < 0, ""); // expected-warning {{implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1}}
 #endif
 }
 
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13047,6 +13047,18 @@
 
   unsigned OriginalWidth = Value.getBitWidth();
 
+  // In C, the macro 'true' from stdbool.h will evaluate to '1'; To reduce
+  // false positives where the user is demonstrating they intend to use the
+  // bit-field as a Boolean, check to see if the value is 1 and we're assigning
+  // to a one-bit bit-field to see if the value came from a macro named 'true'.
+  bool OneAssignedToOneBitBitfield = FieldWidth == 1 && Value == 1;
+  if (OneAssignedToOneBitBitfield && !S.LangOpts.CPlusPlus) {
+SourceLocation MaybeMacroLoc = OriginalInit->getBeginLoc();
+if (S.SourceMgr.isInSystemMacro(MaybeMacroLoc) &&
+S.findMacroSpelling(MaybeMacroLoc, "true"))
+  return false;
+  }
+
   if (!Value.isSigned() || Value.isNe

[PATCH] D132136: [clang] Perform implicit lvalue-to-rvalue cast with new interpreter

2022-08-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D132136#3753290 , @tbaeder wrote:

> In D132136#3751702 , @erichkeane 
> wrote:
>
>> Would be great if we had a better test here... is there anything we can do 
>> to validate this is happening other than checking for that one note?
>
> `EvaluateAsRValue` is called from `Expr::EvaluateAsRValue()`, so I think it 
> would be possible to write a unittest for this. But I think that would be a 
> lot of effort just to test this. There is even 
> `unittests/AST/EvaluateAsRValueTest.cpp` already, but it tests the wrong 
> thing :(

The existing test coverage being wrong seems like all the more reason to add 
correct test coverage. LValue to RValue conversions are important to get right 
(lol here's a wonderful demonstration of where we didn't bother to see if we 
got it right that I accidentally stumbled into when trying to give you a 
constexpr test case: https://godbolt.org/z/bdxbers3M), especially because 
they're going to impact which overload gets called when picking between an `&&` 
and `&` overload.


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

https://reviews.llvm.org/D132136

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


[PATCH] D132851: Further update -Wbitfield-constant-conversion for 1-bit bitfield

2022-08-29 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision.
erichkeane added a comment.

Still LGTM.


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

https://reviews.llvm.org/D132851

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


[PATCH] D132831: [clang][Interp] Handle SubstNonTypeTemplateParmExprs

2022-08-29 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:273
+const SubstNonTypeTemplateParmExpr *E) {
+  return this->visit(E->getReplacement());
+}

erichkeane wrote:
> Is there nothing special that has to happen when these are reference 
> parameters?  Can you at least add a test for that?
Umm, I don't really do references yet (I haven't tested them at all and I don't 
think they are implemented).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132831

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


[PATCH] D132821: [clang][Parse] Fix crash when emitting template diagnostic

2022-08-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Please be sure to add a release note for the fix as well.




Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:776-779
   "identifier followed by '<' indicates a class template specialization but "
   "%0 %select{does not refer to a template|refers to a function template|"
-  "|refers to a variable template||refers to a concept}1">;
+  "|refers to a variable template||refers to a concept|"
+  "refers to a concept template}1">;

I think the issue is that `TNK_Undeclared_template` was added but this 
diagnostic was not updated accordingly. As best I can tell, 
ParseDeclCXX.cpp:1715-1725 makes that kind unused in this diagnostic as well.



Comment at: clang/test/Parser/cxx-concept-declaration.cpp:3
 // Support parsing of concepts
 // Disabled for now.
 

tbaeder wrote:
> I guess we can drop these two lines now.
Agreed (feel free to land as an NFC commit)



Comment at: clang/test/Parser/cxx-concept-declaration.cpp:6-10
+template concept C1 = true;
+
+
+
+template




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132821

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


[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

2022-08-29 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 456376.

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

https://reviews.llvm.org/D132727

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Pointer.cpp
  clang/lib/AST/Interp/Program.cpp
  clang/test/AST/Interp/arrays.cpp

Index: clang/test/AST/Interp/arrays.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/arrays.cpp
@@ -0,0 +1,83 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
+// RUN: %clang_cc1 -verify=ref %s
+
+constexpr int m = 3;
+constexpr const int *foo[][5] = {
+  {nullptr, &m, nullptr, nullptr, nullptr},
+  {nullptr, nullptr, &m, nullptr, nullptr},
+  {nullptr, nullptr, nullptr, &m, nullptr},
+};
+
+static_assert(foo[0][0] == nullptr, "");
+static_assert(foo[0][1] == &m, "");
+static_assert(foo[0][2] == nullptr, "");
+static_assert(foo[0][3] == nullptr, "");
+static_assert(foo[0][4] == nullptr, "");
+static_assert(foo[1][0] == nullptr, "");
+static_assert(foo[1][1] == nullptr, "");
+static_assert(foo[1][2] == &m, "");
+static_assert(foo[1][3] == nullptr, "");
+static_assert(foo[1][4] == nullptr, "");
+static_assert(foo[2][0] == nullptr, "");
+static_assert(foo[2][1] == nullptr, "");
+static_assert(foo[2][2] == nullptr, "");
+static_assert(foo[2][3] == &m, "");
+static_assert(foo[2][4] == nullptr, "");
+
+
+/// A init list for a primitive value.
+constexpr int f{5};
+static_assert(f == 5, "");
+
+
+constexpr int getElement(int i) {
+  int values[] = {1, 4, 9, 16, 25, 36};
+  return values[i];
+}
+static_assert(getElement(1) == 4, "");
+static_assert(getElement(5) == 36, "");
+
+
+template
+constexpr T getElementOf(T* array, int i) {
+  return array[i];
+}
+static_assert(getElementOf(foo[0], 1) == &m, "");
+
+
+constexpr int data[] = {5, 4, 3, 2, 1};
+static_assert(data[0] == 4, ""); // expected-error{{failed}} \
+ // expected-note{{5 == 4}} \
+ // ref-error{{failed}} \
+ // ref-note{{5 == 4}}
+
+
+constexpr int dynamic[] = {
+  f, 3, 2 + 5, data[3], *getElementOf(foo[2], 3)
+};
+static_assert(dynamic[0] == f, "");
+static_assert(dynamic[3] == 2, "");
+
+
+constexpr int dependent[4] = {
+  0, 1, dependent[0], dependent[1]
+};
+static_assert(dependent[2] == dependent[0], "");
+static_assert(dependent[3] == dependent[1], "");
+
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wc99-extensions"
+#pragma clang diagnostic ignored "-Winitializer-overrides"
+constexpr int DI[] = {
+  [0] = 10,
+  [1] = 20,
+  30,
+  40,
+  [1] = 50
+};
+static_assert(DI[0] == 10, "");
+static_assert(DI[1] == 50, "");
+static_assert(DI[2] == 30, "");
+static_assert(DI[3] == 40, "");
+#pragma clang diagnostic pop
Index: clang/lib/AST/Interp/Program.cpp
===
--- clang/lib/AST/Interp/Program.cpp
+++ clang/lib/AST/Interp/Program.cpp
@@ -334,14 +334,15 @@
   } else {
 // Arrays of composites. In this case, the array is a list of pointers,
 // followed by the actual elements.
-Descriptor *Desc =
+Descriptor *ElemDesc =
 createDescriptor(D, ElemTy.getTypePtr(), IsConst, IsTemporary);
-if (!Desc)
+if (!ElemDesc)
   return nullptr;
-InterpSize ElemSize = Desc->getAllocSize() + sizeof(InlineDescriptor);
+InterpSize ElemSize =
+ElemDesc->getAllocSize() + sizeof(InlineDescriptor);
 if (std::numeric_limits::max() / ElemSize <= NumElems)
   return {};
-return allocateDescriptor(D, Desc, NumElems, IsConst, IsTemporary,
+return allocateDescriptor(D, ElemDesc, NumElems, IsConst, IsTemporary,
   IsMutable);
   }
 }
Index: clang/lib/AST/Interp/Pointer.cpp
===
--- clang/lib/AST/Interp/Pointer.cpp
+++ clang/lib/AST/Interp/Pointer.cpp
@@ -106,7 +106,7 @@
 
   // Build the path into the object.
   Pointer Ptr = *this;
-  while (Ptr.isField()) {
+  while (Ptr.isField() || Ptr.isArrayElement()) {
 if (Ptr.isArrayElement()) {
   Path.push_back(APValue::LValuePathEntry::ArrayIndex(Ptr.getIndex()));
   Ptr = Ptr.getArray();
@@ -154,7 +154,8 @@
 void Pointer::initialize() const {
   assert(Pointee && "Cannot initialize null pointer");
   Descriptor *Desc = getFieldDesc();
-  if (Desc->isPrimitiveArray()) {
+
+  if (Desc->isArray() || Desc->isPrimitiveArray()) {
 if (!Pointee->IsStatic) {
   // Primitive array initializer.
   InitMap *&Map = getInitMap();
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Inter

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

2022-08-29 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added a comment.

In D132727#3755571 , @aaron.ballman 
wrote:

> Precommit CI found a relevant failure:

That needs the lvalue-to-rvalue conversion patch first.


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

https://reviews.llvm.org/D132727

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


  1   2   3   >