[clang] 5b769ff - [NFC] Add an invalid test case for C++20 Modules

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2023-03-06T14:39:32+08:00
New Revision: 5b769ff3e6a47eb2ffe8aa9e86f701ef046cf0a9

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

LOG: [NFC] Add an invalid test case for C++20 Modules

Address https://github.com/llvm/llvm-project/issues/61150.

The test is intended to show the polluted operator&& will affect the ODR
checking and the ODR checking here is correct.

Added: 
clang/test/Modules/polluted-operator.cppm

Modified: 


Removed: 




diff  --git a/clang/test/Modules/polluted-operator.cppm 
b/clang/test/Modules/polluted-operator.cppm
new file mode 100644
index 0..b24464aa6ad21
--- /dev/null
+++ b/clang/test/Modules/polluted-operator.cppm
@@ -0,0 +1,57 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/a.cppm -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 %t/b.cppm -fprebuilt-module-path=%t 
-emit-module-interface -o %t/b.pcm -verify
+
+//--- foo.h
+
+namespace std
+{
+template
+void operator &&(_Dom1 __v, _Dom1 __w)
+{ 
+return;
+}
+}
+
+//--- bar.h
+namespace std 
+{
+  template
+struct _Traits
+{
+  static constexpr bool _S_copy_ctor =
+   (__is_trivial(_Types) && ...);
+};
+
+  template
+struct variant
+{
+  void
+  swap(variant& __rhs)
+  noexcept((__is_trivial(_Types) && ...))
+  {
+  }
+};
+}
+
+//--- a.cppm
+module;
+// The operator&& defined in 'foo.h' will pollute the 
+// expression '__is_trivial(_Types) && ...' in bar.h
+#include "foo.h"
+#include "bar.h"
+export module a;
+
+//--- b.cppm
+module;
+#include "bar.h"
+export module b;
+import a;
+
+// expected-error@* {{has 
diff erent definitions in 
diff erent modules; first 
diff erence is defined here found data member '_S_copy_ctor' with an 
initializer}}
+// expected-note@* {{but in 'a.' found data member '_S_copy_ctor' with 
a 
diff erent initializer}}
+// expected-error@* {{from module 'a.' is not present in definition of 
'variant<_Types...>' provided earlier}}
+// expected-note@* {{declaration of 'swap' does not match}}



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

[PATCH] D145346: [NFC] Format printstmt

[xufei via Phabricator via cfe-commits]

zkkxu updated this revision to Diff 502510.
zkkxu added a comment.



1. Updating D145346 : [NFC] Format printstmt #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

up


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145346

Files:
  clang/lib/AST/Stmt.cpp


Index: clang/lib/AST/Stmt.cpp
===
--- clang/lib/AST/Stmt.cpp
+++ clang/lib/AST/Stmt.cpp
@@ -102,24 +102,21 @@
   getStmtInfoTableEntry(Stmt::NullStmtClass);
 
   unsigned sum = 0;
+  unsigned total_bytes = 0;
   llvm::errs() << "\n*** Stmt/Expr Stats:\n";
   for (int i = 0; i != Stmt::lastStmtConstant+1; i++) {
 if (StmtClassInfo[i].Name == nullptr) continue;
 sum += StmtClassInfo[i].Counter;
-  }
-  llvm::errs() << "  " << sum << " stmts/exprs total.\n";
-  sum = 0;
-  for (int i = 0; i != Stmt::lastStmtConstant+1; i++) {
-if (StmtClassInfo[i].Name == nullptr) continue;
 if (StmtClassInfo[i].Counter == 0) continue;
 llvm::errs() << "" << StmtClassInfo[i].Counter << " "
  << StmtClassInfo[i].Name << ", " << StmtClassInfo[i].Size
  << " each (" << StmtClassInfo[i].Counter*StmtClassInfo[i].Size
  << " bytes)\n";
-sum += StmtClassInfo[i].Counter*StmtClassInfo[i].Size;
+total_bytes += StmtClassInfo[i].Counter*StmtClassInfo[i].Size;
   }
-
-  llvm::errs() << "Total bytes = " << sum << "\n";
+  
+  llvm::errs() << "  " << sum << " stmts/exprs total.\n";
+  llvm::errs() << "Total bytes = " << total_bytes << "\n";
 }
 
 void Stmt::addStmtClass(StmtClass s) {


Index: clang/lib/AST/Stmt.cpp
===
--- clang/lib/AST/Stmt.cpp
+++ clang/lib/AST/Stmt.cpp
@@ -102,24 +102,21 @@
   getStmtInfoTableEntry(Stmt::NullStmtClass);
 
   unsigned sum = 0;
+  unsigned total_bytes = 0;
   llvm::errs() << "\n*** Stmt/Expr Stats:\n";
   for (int i = 0; i != Stmt::lastStmtConstant+1; i++) {
 if (StmtClassInfo[i].Name == nullptr) continue;
 sum += StmtClassInfo[i].Counter;
-  }
-  llvm::errs() << "  " << sum << " stmts/exprs total.\n";
-  sum = 0;
-  for (int i = 0; i != Stmt::lastStmtConstant+1; i++) {
-if (StmtClassInfo[i].Name == nullptr) continue;
 if (StmtClassInfo[i].Counter == 0) continue;
 llvm::errs() << "" << StmtClassInfo[i].Counter << " "
  << StmtClassInfo[i].Name << ", " << StmtClassInfo[i].Size
  << " each (" << StmtClassInfo[i].Counter*StmtClassInfo[i].Size
  << " bytes)\n";
-sum += StmtClassInfo[i].Counter*StmtClassInfo[i].Size;
+total_bytes += StmtClassInfo[i].Counter*StmtClassInfo[i].Size;
   }
-
-  llvm::errs() << "Total bytes = " << sum << "\n";
+  
+  llvm::errs() << "  " << sum << " stmts/exprs total.\n";
+  llvm::errs() << "Total bytes = " << total_bytes << "\n";
 }
 
 void Stmt::addStmtClass(StmtClass s) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144454: Add builtin for llvm set rounding

[xiongji90 via Phabricator via cfe-commits]

xiongji90 updated this revision to Diff 502507.
xiongji90 added a comment.

Update the builtin name to __builtin_set_flt_rounds and restrict it to be used 
only on x86, x86_64, arm, aarch64 target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144454

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins.c


Index: clang/test/CodeGen/builtins.c
===
--- clang/test/CodeGen/builtins.c
+++ clang/test/CodeGen/builtins.c
@@ -278,6 +278,9 @@
 
   res = __builtin_flt_rounds();
   // CHECK: call i32 @llvm.get.rounding(
+
+  __builtin_set_flt_rounds(1);
+  // CHECK: call void @llvm.set.rounding(i32 1)
 }
 
 // CHECK-LABEL: define{{.*}} void @test_float_builtin_ops
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -2146,6 +2146,14 @@
   return ExprError();
 break;
 
+  case Builtin::BI__builtin_set_flt_rounds:
+if (CheckBuiltinTargetInSupported(*this, BuiltinID, TheCall,
+  {llvm::Triple::x86, llvm::Triple::x86_64,
+   llvm::Triple::arm, llvm::Triple::thumb,
+   llvm::Triple::aarch64}))
+  return ExprError();
+break;
+
   case Builtin::BI__builtin_isgreater:
   case Builtin::BI__builtin_isgreaterequal:
   case Builtin::BI__builtin_isless:
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -3361,6 +3361,14 @@
 return RValue::get(Result);
   }
 
+  case Builtin::BI__builtin_set_flt_rounds: {
+Function *F = CGM.getIntrinsic(Intrinsic::set_rounding);
+
+Value *V = EmitScalarExpr(E->getArg(0));
+Builder.CreateCall(F, V);
+return RValue::get(nullptr);
+  }
+
   case Builtin::BI__builtin_fpclassify: {
 CodeGenFunction::CGFPOptionsRAII FPOptsRAII(*this, E);
 // FIXME: for strictfp/IEEE-754 we need to not trap on SNaN here.
Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -392,6 +392,7 @@
 
 // Access to floating point environment
 BUILTIN(__builtin_flt_rounds, "i", "n")
+BUILTIN(__builtin_set_flt_rounds, "vi", "n")
 
 // C99 complex builtins
 BUILTIN(__builtin_cabs, "dXd", "Fne")


Index: clang/test/CodeGen/builtins.c
===
--- clang/test/CodeGen/builtins.c
+++ clang/test/CodeGen/builtins.c
@@ -278,6 +278,9 @@
 
   res = __builtin_flt_rounds();
   // CHECK: call i32 @llvm.get.rounding(
+
+  __builtin_set_flt_rounds(1);
+  // CHECK: call void @llvm.set.rounding(i32 1)
 }
 
 // CHECK-LABEL: define{{.*}} void @test_float_builtin_ops
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -2146,6 +2146,14 @@
   return ExprError();
 break;
 
+  case Builtin::BI__builtin_set_flt_rounds:
+if (CheckBuiltinTargetInSupported(*this, BuiltinID, TheCall,
+  {llvm::Triple::x86, llvm::Triple::x86_64,
+   llvm::Triple::arm, llvm::Triple::thumb,
+   llvm::Triple::aarch64}))
+  return ExprError();
+break;
+
   case Builtin::BI__builtin_isgreater:
   case Builtin::BI__builtin_isgreaterequal:
   case Builtin::BI__builtin_isless:
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -3361,6 +3361,14 @@
 return RValue::get(Result);
   }
 
+  case Builtin::BI__builtin_set_flt_rounds: {
+Function *F = CGM.getIntrinsic(Intrinsic::set_rounding);
+
+Value *V = EmitScalarExpr(E->getArg(0));
+Builder.CreateCall(F, V);
+return RValue::get(nullptr);
+  }
+
   case Builtin::BI__builtin_fpclassify: {
 CodeGenFunction::CGFPOptionsRAII FPOptsRAII(*this, E);
 // FIXME: for strictfp/IEEE-754 we need to not trap on SNaN here.
Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -392,6 +392,7 @@
 
 // Access to floating point environment
 BUILTIN(__builtin_flt_rounds, "i", "n")
+BUILTIN(__builtin_set_flt_rounds, "vi", "n")
 
 // C99 complex builtins
 BUILTIN(__builtin_cabs, "dXd", "Fne")
___
cfe-commits 

[PATCH] D145343: [AMDGPU] Emit predefined macro `__AMDGCN_CUMODE_OPTION`

[Brian Sumner via Phabricator via cfe-commits]

b-sumner added a comment.

In D145343#4170305 , @yaxunl wrote:

> In D145343#4170250 , @arsenm wrote:
>
>> I think exposing whether or not the flag was used is weird/broken, as is 
>> including _OPTION in the name. Should just define to whether it's enabled or 
>> not
>
> I agree. @b-sumner What do you think?

I think applications may need to check if CUMode is enabled at compile time and 
select code based on that.  But a concern has been raised about compiling such 
source with an older compiler which is not setting the macro regardless of 
whether -mcumode was used.   The conservative approach here would be to only 
define a macro only if -mcumode is used, and define nothing if it is not used.  
Then, when using an old compiler, the code will assume -mno-cumode which is 
always fine to do.


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

https://reviews.llvm.org/D145343

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

[PATCH] D145346: [clang][ast]: refactor printstmt merge the two for loop

[xufei via Phabricator via cfe-commits]

zkkxu created this revision.
zkkxu added reviewers: arcbbb, craig.topper.
Herald added a project: All.
zkkxu 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/D145346

Files:
  clang/lib/AST/Stmt.cpp


Index: clang/lib/AST/Stmt.cpp
===
--- clang/lib/AST/Stmt.cpp
+++ clang/lib/AST/Stmt.cpp
@@ -104,17 +104,20 @@
   unsigned sum = 0;
   unsigned total_bytes = 0;
   llvm::errs() << "\n*** Stmt/Expr Stats:\n";
-  for (int i = 0; i != Stmt::lastStmtConstant+1; i++) {
-if (StmtClassInfo[i].Name == nullptr) continue;
+  for (int i = 0; i != Stmt::lastStmtConstant + 1; i++) {
+if (StmtClassInfo[i].Name == nullptr)
+  continue;
 sum += StmtClassInfo[i].Counter;
-if (StmtClassInfo[i].Counter == 0) continue;
+if (StmtClassInfo[i].Counter == 0)
+  continue;
 llvm::errs() << "" << StmtClassInfo[i].Counter << " "
  << StmtClassInfo[i].Name << ", " << StmtClassInfo[i].Size
- << " each (" << StmtClassInfo[i].Counter*StmtClassInfo[i].Size
+ << " each ("
+ << StmtClassInfo[i].Counter * StmtClassInfo[i].Size
  << " bytes)\n";
-total_bytes += StmtClassInfo[i].Counter*StmtClassInfo[i].Size;
+total_bytes += StmtClassInfo[i].Counter * StmtClassInfo[i].Size;
   }
-  
+
   llvm::errs() << "  " << sum << " stmts/exprs total.\n";
   llvm::errs() << "Total bytes = " << total_bytes << "\n";
 }


Index: clang/lib/AST/Stmt.cpp
===
--- clang/lib/AST/Stmt.cpp
+++ clang/lib/AST/Stmt.cpp
@@ -104,17 +104,20 @@
   unsigned sum = 0;
   unsigned total_bytes = 0;
   llvm::errs() << "\n*** Stmt/Expr Stats:\n";
-  for (int i = 0; i != Stmt::lastStmtConstant+1; i++) {
-if (StmtClassInfo[i].Name == nullptr) continue;
+  for (int i = 0; i != Stmt::lastStmtConstant + 1; i++) {
+if (StmtClassInfo[i].Name == nullptr)
+  continue;
 sum += StmtClassInfo[i].Counter;
-if (StmtClassInfo[i].Counter == 0) continue;
+if (StmtClassInfo[i].Counter == 0)
+  continue;
 llvm::errs() << "" << StmtClassInfo[i].Counter << " "
  << StmtClassInfo[i].Name << ", " << StmtClassInfo[i].Size
- << " each (" << StmtClassInfo[i].Counter*StmtClassInfo[i].Size
+ << " each ("
+ << StmtClassInfo[i].Counter * StmtClassInfo[i].Size
  << " bytes)\n";
-total_bytes += StmtClassInfo[i].Counter*StmtClassInfo[i].Size;
+total_bytes += StmtClassInfo[i].Counter * StmtClassInfo[i].Size;
   }
-  
+
   llvm::errs() << "  " << sum << " stmts/exprs total.\n";
   llvm::errs() << "Total bytes = " << total_bytes << "\n";
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145343: [AMDGPU] Emit predefined macro `__AMDGCN_CUMODE_OPTION`

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D145343#4170250 , @arsenm wrote:

> I think exposing whether or not the flag was used is weird/broken, as is 
> including _OPTION in the name. Should just define to whether it's enabled or 
> not

I agree. @b-sumner What do you think?


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

https://reviews.llvm.org/D145343

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

[PATCH] D145345: [HIP] Fix regression about `__fp16` args and return value

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added a reviewer: tra.
Herald added subscribers: kosarev, kerbowa, jvesely.
Herald added a project: All.
yaxunl requested review of this revision.

HIP allows `__fp16` as function arguments and return value by passing
`-fallow-half-arguments-and-returns` to clang through hipcc.

https://reviews.llvm.org/D133885 removed `-fallow-half-arguments-and-returns`
and add a TargetInfo member to control it.

This caused regressions in some HIP apps 
(https://github.com/ROCm-Developer-Tools/HIP/issues/3178).


https://reviews.llvm.org/D145345

Files:
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/test/SemaCUDA/fp16-arg-return.cu


Index: clang/test/SemaCUDA/fp16-arg-return.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/fp16-arg-return.cu
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -emit-llvm -o - -triple amdgcn-amd-amdhsa -fcuda-is-device 
-fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+__fp16 testFP16AsArgAndReturn(__fp16 x) {
+  return x;
+}
Index: clang/lib/Basic/Targets/AMDGPU.cpp
===
--- clang/lib/Basic/Targets/AMDGPU.cpp
+++ clang/lib/Basic/Targets/AMDGPU.cpp
@@ -416,6 +416,7 @@
   }
 
   MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
+  HalfArgsAndReturns = true;
 }
 
 void AMDGPUTargetInfo::adjust(DiagnosticsEngine , LangOptions ) {


Index: clang/test/SemaCUDA/fp16-arg-return.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/fp16-arg-return.cu
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -emit-llvm -o - -triple amdgcn-amd-amdhsa -fcuda-is-device -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+__fp16 testFP16AsArgAndReturn(__fp16 x) {
+  return x;
+}
Index: clang/lib/Basic/Targets/AMDGPU.cpp
===
--- clang/lib/Basic/Targets/AMDGPU.cpp
+++ clang/lib/Basic/Targets/AMDGPU.cpp
@@ -416,6 +416,7 @@
   }
 
   MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
+  HalfArgsAndReturns = true;
 }
 
 void AMDGPUTargetInfo::adjust(DiagnosticsEngine , LangOptions ) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145344: [clang-format] Don't annotate left brace of class as FunctionLBrace

[Owen Pan via Phabricator via cfe-commits]

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

The `l_brace` of class/struct/union was incorrectly annotated as 
`TT_FunctionLBrace` in the presence of attributes. This in turn would cause the 
`RemoveSemicolon` option to remove the semicolon at the end of the declaration, 
resulting in invalid code being generated.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145344

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -345,6 +345,10 @@
   Tokens = annotate("const class {} c;");
   EXPECT_EQ(Tokens.size(), 7u) << Tokens;
   EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_ClassLBrace);
+
+  Tokens = annotate("class [[deprecated(\"\")]] C { int i; };");
+  EXPECT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[10], tok::l_brace, TT_ClassLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsStructs) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25233,6 +25233,11 @@
"}",
Style);
 
+  verifyFormat("class [[deprecated(\"\")]] C {\n"
+   "  int i;\n"
+   "};",
+   Style);
+
   verifyIncompleteFormat("class C final [[deprecated(l]] {});", Style);
 
   // These tests are here to show a problem that may not be easily
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -3701,13 +3701,13 @@
 void UnwrappedLineParser::parseRecord(bool ParseAsExpr) {
   const FormatToken  = *FormatTok;
   nextToken();
+  handleAttributes();
 
   // The actual identifier can be a nested name specifier, and in macros
   // it is often token-pasted.
-  // An [[attribute]] can be before the identifier.
   while (FormatTok->isOneOf(tok::identifier, tok::coloncolon, tok::hashhash,
 tok::kw___attribute, tok::kw___declspec,
-tok::kw_alignas, tok::l_square, tok::r_square) ||
+tok::kw_alignas) ||
  ((Style.Language == FormatStyle::LK_Java || Style.isJavaScript()) &&
   FormatTok->isOneOf(tok::period, tok::comma))) {
 if (Style.isJavaScript() &&
@@ -3725,15 +3725,10 @@
 FormatTok->is(tok::identifier) &&
 FormatTok->TokenText != FormatTok->TokenText.upper();
 nextToken();
-// We can have macros or attributes in between 'class' and the class name.
+// We can have macros in between 'class' and the class name.
 if (!IsNonMacroIdentifier) {
   if (FormatTok->is(tok::l_paren)) {
 parseParens();
-  } else if (FormatTok->is(TT_AttributeSquare)) {
-parseSquare();
-// Consume the closing TT_AttributeSquare.
-if (FormatTok->Next && FormatTok->is(TT_AttributeSquare))
-  nextToken();
   }
 }
   }


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -345,6 +345,10 @@
   Tokens = annotate("const class {} c;");
   EXPECT_EQ(Tokens.size(), 7u) << Tokens;
   EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_ClassLBrace);
+
+  Tokens = annotate("class [[deprecated(\"\")]] C { int i; };");
+  EXPECT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[10], tok::l_brace, TT_ClassLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsStructs) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25233,6 +25233,11 @@
"}",
Style);
 
+  verifyFormat("class [[deprecated(\"\")]] C {\n"
+   "  int i;\n"
+   "};",
+   Style);
+
   verifyIncompleteFormat("class C final [[deprecated(l]] {});", Style);
 
   // These tests are here to show a problem that may not be easily
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -3701,13 +3701,13 @@
 void 

[PATCH] D145343: [AMDGPU] Emit predefined macro `__AMDGCN_CUMODE_OPTION`

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

I think exposing whether or not the flag was used is weird/broken, as is 
including _OPTION in the name. Should just define to whether it's enabled or not


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

https://reviews.llvm.org/D145343

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

[PATCH] D145343: [AMDGPU] Emit predefined macro `__AMDGCN_CUMODE_OPTION`

[Yaxun Liu via Phabricator via cfe-commits]

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

Predefine `__AMDGCN_CUMODE_OPTION` as 1 or 0 when -mcumode or -mno-cumode
is specified.

This is needed for implementing device functions like `__smid` 
(https://github.com/ROCm-Developer-Tools/hipamd/blob/312dff7b794337aa040be0691acc78e9f968a8d2/include/hip/amd_detail/amd_device_functions.h#L957)


https://reviews.llvm.org/D145343

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


Index: clang/test/Driver/hip-macros.hip
===
--- clang/test/Driver/hip-macros.hip
+++ clang/test/Driver/hip-macros.hip
@@ -19,3 +19,14 @@
 // RUN:   -mwavefrontsize64 %s 2>&1 | FileCheck --check-prefixes=WAVE64 %s
 // WAVE64-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
 // WAVE32-DAG: #define __AMDGCN_WAVEFRONT_SIZE 32
+
+// RUN: %clang -E -dM --offload-arch=gfx1030 --cuda-device-only -nogpuinc 
-nogpulib \
+// RUN:   %s 2>&1 | FileCheck --check-prefix=CUMODE-DEFAULT %s
+// RUN: %clang -E -dM --offload-arch=gfx1030 --cuda-device-only -nogpuinc 
-nogpulib -mcumode \
+// RUN:   %s 2>&1 | FileCheck --check-prefix=CUMODE-ON %s
+// RUN: %clang -E -dM --offload-arch=gfx1030 --cuda-device-only -nogpuinc 
-nogpulib -mno-cumode \
+// RUN:   %s 2>&1 | FileCheck --check-prefix=CUMODE-OFF %s
+// CUMODE-DEFAULT-NOT: #define __AMDGCN_CUMODE_OPTION
+// CUMODE-DEFAULT-NOT: #define __AMDGCN_CUMODE_OPTION
+// CUMODE-ON-DAG: #define __AMDGCN_CUMODE_OPTION 1
+// CUMODE-OFF-DAG: #define __AMDGCN_CUMODE_OPTION 0
Index: clang/test/Driver/amdgpu-macros.cl
===
--- clang/test/Driver/amdgpu-macros.cl
+++ clang/test/Driver/amdgpu-macros.cl
@@ -156,3 +156,14 @@
 // RUN:   -mwavefrontsize64 %s 2>&1 | FileCheck --check-prefix=WAVE64 %s
 // WAVE64-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
 // WAVE32-DAG: #define __AMDGCN_WAVEFRONT_SIZE 32
+
+// RUN: %clang -E -dM -target amdgcn -mcpu=gfx1030 \
+// RUN:   %s 2>&1 | FileCheck --check-prefix=CUMODE-DEFAULT %s
+// RUN: %clang -E -dM -target amdgcn -mcpu=gfx1030 -mcumode \
+// RUN:   %s 2>&1 | FileCheck --check-prefix=CUMODE-ON %s
+// RUN: %clang -E -dM -target amdgcn -mcpu=gfx1030 -mno-cumode \
+// RUN:   %s 2>&1 | FileCheck --check-prefix=CUMODE-OFF %s
+// CUMODE-DEFAULT-NOT: #define __AMDGCN_CUMODE_OPTION
+// CUMODE-DEFAULT-NOT: #define __AMDGCN_CUMODE_OPTION
+// CUMODE-ON-DAG: #define __AMDGCN_CUMODE_OPTION 1
+// CUMODE-OFF-DAG: #define __AMDGCN_CUMODE_OPTION 0
Index: clang/lib/Basic/Targets/AMDGPU.h
===
--- clang/lib/Basic/Targets/AMDGPU.h
+++ clang/lib/Basic/Targets/AMDGPU.h
@@ -43,6 +43,9 @@
   unsigned GPUFeatures;
   unsigned WavefrontSize;
 
+  /// User explicitly specify -mcumode or -mno-cumode.
+  std::optional CUModeOpt;
+
   /// Target ID is device name followed by optional feature name postfixed
   /// by plus or minus sign delimitted by colon, e.g. gfx908:xnack+:sramecc-.
   /// If the target ID contains feature+, map it to true.
@@ -443,6 +446,10 @@
   assert(F.front() == '+' || F.front() == '-');
   if (F == "+wavefrontsize64")
 WavefrontSize = 64;
+  else if (F == "+cumode")
+CUModeOpt = true;
+  else if (F == "-cumode")
+CUModeOpt = false;
   bool IsOn = F.front() == '+';
   StringRef Name = StringRef(F).drop_front();
   if (!llvm::is_contained(TargetIDFeatures, Name))
Index: clang/lib/Basic/Targets/AMDGPU.cpp
===
--- clang/lib/Basic/Targets/AMDGPU.cpp
+++ clang/lib/Basic/Targets/AMDGPU.cpp
@@ -487,6 +487,8 @@
 Builder.defineMacro("FP_FAST_FMA");
 
   Builder.defineMacro("__AMDGCN_WAVEFRONT_SIZE", Twine(WavefrontSize));
+  if (CUModeOpt)
+Builder.defineMacro("__AMDGCN_CUMODE_OPTION", Twine(CUModeOpt.value()));
 }
 
 void AMDGPUTargetInfo::setAuxTarget(const TargetInfo *Aux) {


Index: clang/test/Driver/hip-macros.hip
===
--- clang/test/Driver/hip-macros.hip
+++ clang/test/Driver/hip-macros.hip
@@ -19,3 +19,14 @@
 // RUN:   -mwavefrontsize64 %s 2>&1 | FileCheck --check-prefixes=WAVE64 %s
 // WAVE64-DAG: #define __AMDGCN_WAVEFRONT_SIZE 64
 // WAVE32-DAG: #define __AMDGCN_WAVEFRONT_SIZE 32
+
+// RUN: %clang -E -dM --offload-arch=gfx1030 --cuda-device-only -nogpuinc -nogpulib \
+// RUN:   %s 2>&1 | FileCheck --check-prefix=CUMODE-DEFAULT %s
+// RUN: %clang -E -dM --offload-arch=gfx1030 --cuda-device-only -nogpuinc -nogpulib -mcumode \
+// RUN:   %s 2>&1 | FileCheck --check-prefix=CUMODE-ON %s
+// RUN: %clang -E -dM --offload-arch=gfx1030 --cuda-device-only -nogpuinc -nogpulib 

[clang] 7f8d844 - [CodeGen] guarantee variable templates are initialized in the reverse instantiation order

[Yuanfang Chen via cfe-commits]

Author: Yuanfang Chen
Date: 2023-03-05T15:31:23-08:00
New Revision: 7f8d844df5e91f8f689d0e9658e811d21bc4a605

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

LOG: [CodeGen] guarantee variable templates are initialized in the reverse 
instantiation order

Following up D127259.

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

Added: 
clang/test/CodeGenCXX/static-init-variable-template.cpp

Modified: 
clang/lib/Sema/SemaExpr.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index c3e2b6fdeaf1f..56abde04eaf0d 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -19888,16 +19888,11 @@ static void DoMarkVarDeclReferenced(
   DRE->setDecl(DRE->getDecl());
 else if (auto *ME = dyn_cast_or_null(E))
   ME->setMemberDecl(ME->getMemberDecl());
-  } else if (FirstInstantiation ||
- isa(Var)) {
-// FIXME: For a specialization of a variable template, we don't
-// distinguish between "declaration and type implicitly instantiated"
-// and "implicit instantiation of definition requested", so we have
-// no direct way to avoid enqueueing the pending instantiation
-// multiple times.
+  } else if (FirstInstantiation) {
 SemaRef.PendingInstantiations
 .push_back(std::make_pair(Var, PointOfInstantiation));
   } else {
+bool Inserted = false;
 for (auto  : SemaRef.SavedPendingInstantiations) {
   auto Iter = llvm::find_if(
   I, [Var](const Sema::PendingImplicitInstantiation ) {
@@ -19906,9 +19901,19 @@ static void DoMarkVarDeclReferenced(
   if (Iter != I.end()) {
 SemaRef.PendingInstantiations.push_back(*Iter);
 I.erase(Iter);
+Inserted = true;
 break;
   }
 }
+
+// FIXME: For a specialization of a variable template, we don't
+// distinguish between "declaration and type implicitly instantiated"
+// and "implicit instantiation of definition requested", so we have
+// no direct way to avoid enqueueing the pending instantiation
+// multiple times.
+if (isa(Var) && !Inserted)
+  SemaRef.PendingInstantiations
+.push_back(std::make_pair(Var, PointOfInstantiation));
   }
 }
   }

diff  --git a/clang/test/CodeGenCXX/static-init-variable-template.cpp 
b/clang/test/CodeGenCXX/static-init-variable-template.cpp
new file mode 100644
index 0..672ab11059a30
--- /dev/null
+++ b/clang/test/CodeGenCXX/static-init-variable-template.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -std=c++14 -S -emit-llvm -disable-llvm-passes -o - %s 
-triple x86_64-linux-gnu | FileCheck %s
+
+template int Fib = Fib + Fib;
+template<> int Fib<0> = 0;
+template<> int Fib<1> = 1;
+int f = Fib<5>;
+
+template int Fib2 = Fib2 + Fib2;
+template<> int Fib2<0> = 0;
+template<> int Fib2<1> = 1;
+int f2 = Fib2<5>;
+
+// CHECK: @llvm.global_ctors = appending global [9 x { i32, ptr, ptr }] [
+// CHECK-SAME: { i32, ptr, ptr } { i32 65535, ptr @__cxx_global_var_init.4, 
ptr @_Z3FibILi2EE },
+// CHECK-SAME: { i32, ptr, ptr } { i32 65535, ptr @__cxx_global_var_init.3, 
ptr @_Z3FibILi3EE },
+// CHECK-SAME: { i32, ptr, ptr } { i32 65535, ptr @__cxx_global_var_init.5, 
ptr @_Z3FibILi4EE },
+// CHECK-SAME: { i32, ptr, ptr } { i32 65535, ptr @__cxx_global_var_init.2,  
ptr @_Z3FibILi5EE },
+// CHECK-SAME: { i32, ptr, ptr } { i32 65535, ptr @__cxx_global_var_init.8, 
ptr @_Z4Fib2ILi2EE },
+// CHECK-SAME: { i32, ptr, ptr } { i32 65535, ptr @__cxx_global_var_init.9, 
ptr @_Z4Fib2ILi3EE },
+// CHECK-SAME: { i32, ptr, ptr } { i32 65535, ptr @__cxx_global_var_init.7,  
ptr @_Z4Fib2ILi4EE },
+// CHECK-SAME: { i32, ptr, ptr } { i32 65535, ptr @__cxx_global_var_init.6, 
ptr @_Z4Fib2ILi5EE },
+// CHECK-SAME: { i32, ptr, ptr } { i32 65535, ptr 
@_GLOBAL__sub_I_static_init_variable_template.cpp, ptr null }



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

[PATCH] D144672: [Sanitizers] Error when attempting to use `static-lsan` with `TSan` or `Asan` on darwin

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach updated this revision to Diff 502474.
dmaclach added a comment.

Moved to `REQUIRES: asan-static-runtime`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144672

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/sanitizer-ld.c
  compiler-rt/test/asan/TestCases/replaceable_new_delete.cpp
  compiler-rt/test/asan/TestCases/replaceable_new_delete_shared.cpp
  compiler-rt/test/asan/TestCases/replaceable_new_delete_static.cpp

Index: compiler-rt/test/asan/TestCases/replaceable_new_delete_static.cpp
===
--- /dev/null
+++ compiler-rt/test/asan/TestCases/replaceable_new_delete_static.cpp
@@ -0,0 +1,35 @@
+// Ensure that operator new/delete are still replaceable using static-libsan.
+
+// FIXME: Weak symbols aren't supported on Windows, although some code in
+// compiler-rt already exists to solve this problem. We should probably define
+// the new/delete interceptors as "weak" using those workarounds as well.
+// UNSUPPORTED: target={{.*windows.*}}
+
+// darwin only supports `shared-libsan`.
+// REQUIRES: asan-static-runtime
+
+// RUN: %clangxx %s -o %t -fsanitize=address -static-libsan && not %run %t 2>&1 | FileCheck %s
+
+#include 
+#include 
+#include 
+
+void *operator new[](size_t size) {
+  fprintf(stderr, "replaced new\n");
+  return malloc(size);
+}
+
+void operator delete[](void *ptr) noexcept {
+  fprintf(stderr, "replaced delete\n");
+  return free(ptr);
+}
+
+int main(int argc, char **argv) {
+  // CHECK: replaced new
+  char *x = new char[5];
+  // CHECK: replaced delete
+  delete[] x;
+  // CHECK: ERROR: AddressSanitizer
+  *x = 13;
+  return 0;
+}
Index: compiler-rt/test/asan/TestCases/replaceable_new_delete_shared.cpp
===
--- compiler-rt/test/asan/TestCases/replaceable_new_delete_shared.cpp
+++ compiler-rt/test/asan/TestCases/replaceable_new_delete_shared.cpp
@@ -1,4 +1,4 @@
-// Ensure that operator new/delete are still replaceable.
+// Ensure that operator new/delete are still replaceable using shared-libsan.
 
 // FIXME: Weak symbols aren't supported on Windows, although some code in
 // compiler-rt already exists to solve this problem. We should probably define
@@ -6,7 +6,6 @@
 // UNSUPPORTED: target={{.*windows.*}}
 
 // RUN: %clangxx %s -o %t -fsanitize=address -shared-libsan && not %run %t 2>&1 | FileCheck %s
-// RUN: %clangxx %s -o %t -fsanitize=address -static-libsan && not %run %t 2>&1 | FileCheck %s
 
 #include 
 #include 
Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -457,6 +457,18 @@
 // RUN:   | FileCheck --check-prefix=CHECK-UBSAN-STATIC-DARWIN %s
 // CHECK-UBSAN-STATIC-DARWIN: {{.*}}error: static UndefinedBehaviorSanitizer runtime is not supported on darwin
 
+// RUN: %clang -fsanitize=address -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin -fuse-ld=ld -static-libsan \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-ASAN-STATIC-DARWIN %s
+// CHECK-ASAN-STATIC-DARWIN: {{.*}}error: static AddressSanitizer runtime is not supported on darwin
+
+// RUN: %clang -fsanitize=thread -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin -fuse-ld=ld -static-libsan \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-TSAN-STATIC-DARWIN %s
+// CHECK-TSAN-STATIC-DARWIN: {{.*}}error: static ThreadSanitizer runtime is not supported on darwin
+
 // RUN: %clang -fsanitize=address,undefined -### %s 2>&1 \
 // RUN: --target=i386-unknown-linux -fuse-ld=ld \
 // RUN: -resource-dir=%S/Inputs/resource_dir \
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1426,24 +1426,42 @@
 
   const SanitizerArgs  = getSanitizerArgs(Args);
 
-  if (!Sanitize.needsSharedRt() && Sanitize.needsUbsanRt()) {
-getDriver().Diag(diag::err_drv_unsupported_static_ubsan_darwin);
-return;
+  if (!Sanitize.needsSharedRt()) {
+const char *sanitizer = nullptr;
+if (Sanitize.needsUbsanRt()) {
+  sanitizer = "UndefinedBehaviorSanitizer";
+} else if (Sanitize.needsAsanRt()) {
+  sanitizer = "AddressSanitizer";
+} else if (Sanitize.needsTsanRt()) {
+  sanitizer = "ThreadSanitizer";
+}
+if (sanitizer) {
+  getDriver().Diag(diag::err_drv_unsupported_static_sanitizer_darwin)
+  << sanitizer;
+  return;
+}
   }
 
   if (Sanitize.linkRuntimes()) {
-if (Sanitize.needsAsanRt())
+if (Sanitize.needsAsanRt()) {
+  assert(Sanitize.needsSharedRt() &&

[PATCH] D144135: [clang-tidy] Add performance-enum-size check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL planned changes to this revision.
PiotrZSL added a comment.

Option need to be changed from Regex to List


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144135

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

[PATCH] D144036: [clang-tidy] Add bugprone-enum-to-bool-conversion check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL planned changes to this revision.
PiotrZSL added a comment.

Documentation to be updated, option need to be changed to list


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144036

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

[PATCH] D144828: [clang-tidy] Add misc-header-include-cycle check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL planned changes to this revision.
PiotrZSL added a comment.

Option need to be added


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144828

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

[PATCH] D144828: [clang-tidy] Add misc-header-include-cycle check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

TODO: Add option: ExcludeHeadersRegexp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144828

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

[PATCH] D144828: [clang-tidy] Add misc-header-include-cycle check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 502464.
PiotrZSL added a comment.

Added support for --include
Added support for self-include of source file
Added more tests
Reworked include stack.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144828

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp
  clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.h
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc/header-include-cycle.rst
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.first-d.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.first.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.fourth-d.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.fourth.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.second-d.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.second.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.self-d.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.self-i.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.self-n.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.self-o.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.self.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.third-d.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.third.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/system/header-include-cycle.first-s.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/system/header-include-cycle.fourth-s.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/system/header-include-cycle.second-s.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/system/header-include-cycle.self-s.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/system/header-include-cycle.third-s.hpp
  clang-tools-extra/test/clang-tidy/checkers/misc/header-include-cycle.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc/header-include-cycle.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc/header-include-cycle.cpp
@@ -0,0 +1,54 @@
+// RUN: rm -rf %T/misc-header-include-cycle-headers
+// RUN: mkdir %T/misc-header-include-cycle-headers
+// RUN: cp -r %S/Inputs/header-include-cycle* %T/misc-header-include-cycle-headers/
+// RUN: mkdir %T/misc-header-include-cycle-headers/system
+// RUN: cp -r %S/Inputs/system/header-include-cycle* %T/misc-header-include-cycle-headers/system
+// RUN: clang-tidy %s -checks='-*,misc-header-include-cycle' -header-filter=.* -- \
+// RUN: -I%T/misc-header-include-cycle-headers -isystem %T/misc-header-include-cycle-headers/system \
+// RUN: --include %T/misc-header-include-cycle-headers/header-include-cycle.self-i.hpp | FileCheck %s \
+// RUN: -check-prefix=CHECK-MESSAGES -implicit-check-not="{{warning|error|note}}:"
+// RUN: rm -rf %T/misc-header-include-cycle-headers
+
+#ifndef MAIN_GUARD
+#define MAIN_GUARD
+
+#include 
+// CHECK-MESSAGES: header-include-cycle.fourth-d.hpp:3:10: warning: circular header file dependency detected while including 'header-include-cycle.first-d.hpp', please check the include path [misc-header-include-cycle]
+// CHECK-MESSAGES: header-include-cycle.third-d.hpp:3:10: note: 'header-include-cycle.fourth-d.hpp' included from here
+// CHECK-MESSAGES: header-include-cycle.second-d.hpp:3:10: note: 'header-include-cycle.third-d.hpp' included from here
+// CHECK-MESSAGES: header-include-cycle.first-d.hpp:3:10: note: 'header-include-cycle.second-d.hpp' included from here
+// CHECK-MESSAGES: header-include-cycle.cpp:[[@LINE-5]]:10: note: 'header-include-cycle.first-d.hpp' included from here
+
+#include 
+// CHECK-MESSAGES: header-include-cycle.fourth.hpp:2:10: warning: circular header file dependency detected while including 'header-include-cycle.first.hpp', please check the include path [misc-header-include-cycle]
+// CHECK-MESSAGES: header-include-cycle.third.hpp:2:10: note: 'header-include-cycle.fourth.hpp' included from here
+// CHECK-MESSAGES: header-include-cycle.second.hpp:2:10: note: 'header-include-cycle.third.hpp' included from here
+// CHECK-MESSAGES: header-include-cycle.first.hpp:2:10: note: 'header-include-cycle.second.hpp' included from here
+// CHECK-MESSAGES: header-include-cycle.cpp:[[@LINE-5]]:10: note: 'header-include-cycle.first.hpp' included from here
+
+#include 
+// CHECK-MESSAGES: 

[PATCH] D144828: [clang-tidy] Add misc-header-include-cycle check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL marked 4 inline comments as done.
PiotrZSL added a comment.

"But maybe when we emit a warning for a specified include, we could put it in a 
set to not warn on that include again"
I don't think so, in such case we hide include paths, and most of the time 
header guards should help here in reduction of cyclic dependences.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144828

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

[PATCH] D145321: [clang-tidy] altera-id-dependent-backward-branch: fix assignment notes

[Egor Suvorov via Phabricator via cfe-commits]

yeputons-gh added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp:126
   IdDepFieldsMap[Field] = IdDependencyRecord(
   Field, Statement->getBeginLoc(),
   Twine("assignment of ID-dependent field ") + Field->getNameAsString());

This line was already correct prior to this patch.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp:42
+// CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is 
ID-dependent due to variable reference to 'ThreadIDAssigned' and may cause 
performance degradation [altera-id-dependent-backward-branch]
+// CHECK-NOTES: :[[@LINE-4]]:3: note: assignment of ID-dependent variable 
ThreadIDAssigned
+accumulator++;

It previously pointed at `int ThreadIDAssigned = 0`, not `ThreadIDAssigned = 
get_local_id(0) * 2`.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp:61-63
-  for (int i = 0; i < ThreadID2; i++) {
-// CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is 
ID-dependent due to variable reference to 'ThreadID2' and may cause performance 
degradation [altera-id-dependent-backward-branch]
-// CHECK-NOTES: :[[@LINE-4]]:3: note: inferred assignment of ID-dependent 
value from ID-dependent variable ThreadID

This became `ThreadIDVarFromVar`, see above.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp:180-210
+  int NotThreadIDVarFromVar = NotThreadID * 2;
+  for (int i = 0; i < NotThreadIDVarFromVar; i++) {
+accumulator++;
+  }
+
+  int NotThreadIDVarAssignFromVar;
+  NotThreadIDVarAssignFromVar = NotThreadID * 2;

I mirror all id-dependent tests with non-id-dependent to make sure 
https://github.com/llvm/llvm-project/issues/52790 does not creep back.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145321

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

[PATCH] D145304: [clang-tidy] altera-id-dependent-backward-branch: refactor test

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

In D145304#4169810 , @yeputons-gh 
wrote:

> Absolutely, give we few minutes. Maybe it conflicts with the previous 
> revision that has already landed?

I believe so. I'm quite new to arcanist so I might be missing some tricks to 
handled chained patches. Rebase worked and I pushed now thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145304

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

[PATCH] D145304: [clang-tidy] altera-id-dependent-backward-branch: refactor test

[Carlos Galvez 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 rG9fda83222431: [clang-tidy] 
altera-id-dependent-backward-branch: refactor test (authored by yeputons-gh, 
committed by carlosgalvezp).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145304

Files:
  
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
@@ -1,9 +1,5 @@
 // RUN: %check_clang_tidy %s altera-id-dependent-backward-branch %t -- 
-header-filter=.* "--" -cl-std=CL1.2 -c
 
-typedef struct ExampleStruct {
-  int IDDepField;
-} ExampleStruct;
-
 void error() {
   //  Conditional Expressions 
   int accumulator = 0;
@@ -32,39 +28,24 @@
 accumulator++;
   }
 
-  ExampleStruct Example;
-  Example.IDDepField = get_local_id(0);
-
-  //  Inferred Assignments 
-  int ThreadID2 = ThreadID * get_local_size(0);
-
-  int ThreadID3 = Example.IDDepField; // OK: not used in any loops
-
-  ExampleStruct UnusedStruct = {
-  ThreadID * 2 // OK: not used in any loops
-  };
-
-  for (int i = 0; i < ThreadID2; i++) {
-// CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is 
ID-dependent due to variable reference to 'ThreadID2' and may cause performance 
degradation [altera-id-dependent-backward-branch]
-// CHECK-NOTES: :[[@LINE-10]]:3: note: inferred assignment of ID-dependent 
value from ID-dependent variable ThreadID
-accumulator++;
-  }
-
   do {
 accumulator++;
   } while (j < ThreadID);
   // CHECK-NOTES: :[[@LINE-1]]:12: warning: backward branch (do loop) is 
ID-dependent due to variable reference to 'ThreadID' and may cause performance 
degradation [altera-id-dependent-backward-branch]
-  // CHECK-NOTES: :[[@LINE-30]]:3: note: assignment of ID-dependent variable 
ThreadID
+  // CHECK-NOTES: :[[@LINE-12]]:3: note: assignment of ID-dependent variable 
ThreadID
+
+  struct { int IDDepField; } Example;
+  Example.IDDepField = get_local_id(0);
 
   for (int i = 0; i < Example.IDDepField; i++) {
 // CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is 
ID-dependent due to member reference to 'IDDepField' and may cause performance 
degradation [altera-id-dependent-backward-branch]
-// CHECK-NOTES: :[[@LINE-25]]:3: note: assignment of ID-dependent field 
IDDepField
+// CHECK-NOTES: :[[@LINE-4]]:3: note: assignment of ID-dependent field 
IDDepField
 accumulator++;
   }
 
   while (j < Example.IDDepField) {
 // CHECK-NOTES: :[[@LINE-1]]:10: warning: backward branch (while loop) is 
ID-dependent due to member reference to 'IDDepField' and may cause performance 
degradation [altera-id-dependent-backward-branch]
-// CHECK-NOTES: :[[@LINE-31]]:3: note: assignment of ID-dependent field 
IDDepField
+// CHECK-NOTES: :[[@LINE-10]]:3: note: assignment of ID-dependent field 
IDDepField
 accumulator++;
   }
 
@@ -72,7 +53,22 @@
 accumulator++;
   } while (j < Example.IDDepField);
   // CHECK-NOTES: :[[@LINE-1]]:12: warning: backward branch (do loop) is 
ID-dependent due to member reference to 'IDDepField' and may cause performance 
degradation [altera-id-dependent-backward-branch]
-  // CHECK-NOTES: :[[@LINE-39]]:3: note: assignment of ID-dependent field 
IDDepField
+  // CHECK-NOTES: :[[@LINE-18]]:3: note: assignment of ID-dependent field 
IDDepField
+
+  //  Inferred Assignments 
+  int ThreadID2 = ThreadID * 2;
+
+  for (int i = 0; i < ThreadID2; i++) {
+// CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is 
ID-dependent due to variable reference to 'ThreadID2' and may cause performance 
degradation [altera-id-dependent-backward-branch]
+// CHECK-NOTES: :[[@LINE-4]]:3: note: inferred assignment of ID-dependent 
value from ID-dependent variable ThreadID
+accumulator++;
+  }
+
+  //  Unused Inferred Assignments 
+  int UnusedThreadID = Example.IDDepField; // OK: not used in any loops
+
+  struct { int IDDepField; } UnusedStruct;
+  UnusedStruct.IDDepField = ThreadID * 2; // OK: not used in any loops
 }
 
 void success() {


Index: clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
@@ -1,9 +1,5 @@
 // RUN: %check_clang_tidy %s altera-id-dependent-backward-branch %t -- -header-filter=.* "--" -cl-std=CL1.2 -c
 
-typedef 

[clang-tools-extra] 9fda832 - [clang-tidy] altera-id-dependent-backward-branch: refactor test

[Carlos Galvez via cfe-commits]

Author: Egor Suvorov
Date: 2023-03-05T18:22:10Z
New Revision: 9fda8322243168cbfcb78c4cf80afa838473a573

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

LOG: [clang-tidy] altera-id-dependent-backward-branch: refactor test

This is a preparation for future fixes that need more tests.

* Put all "Inferred Assignments" testing at the end
* Group together ID-dependent variable/member testing
* Group together unused inferred assignments
* Use a literal instead of `get_local_size` which may have special meaning
* Create a new `struct` for each variable because analysis is done per field,
  not per field per instance.

Depends on D145303

Reviewed By: carlosgalvezp

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

Added: 


Modified: 

clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp

Removed: 




diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
index 0c7a20ad6a179..c0a75fae98d75 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
@@ -1,9 +1,5 @@
 // RUN: %check_clang_tidy %s altera-id-dependent-backward-branch %t -- 
-header-filter=.* "--" -cl-std=CL1.2 -c
 
-typedef struct ExampleStruct {
-  int IDDepField;
-} ExampleStruct;
-
 void error() {
   //  Conditional Expressions 
   int accumulator = 0;
@@ -32,39 +28,24 @@ void error() {
 accumulator++;
   }
 
-  ExampleStruct Example;
-  Example.IDDepField = get_local_id(0);
-
-  //  Inferred Assignments 
-  int ThreadID2 = ThreadID * get_local_size(0);
-
-  int ThreadID3 = Example.IDDepField; // OK: not used in any loops
-
-  ExampleStruct UnusedStruct = {
-  ThreadID * 2 // OK: not used in any loops
-  };
-
-  for (int i = 0; i < ThreadID2; i++) {
-// CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is 
ID-dependent due to variable reference to 'ThreadID2' and may cause performance 
degradation [altera-id-dependent-backward-branch]
-// CHECK-NOTES: :[[@LINE-10]]:3: note: inferred assignment of ID-dependent 
value from ID-dependent variable ThreadID
-accumulator++;
-  }
-
   do {
 accumulator++;
   } while (j < ThreadID);
   // CHECK-NOTES: :[[@LINE-1]]:12: warning: backward branch (do loop) is 
ID-dependent due to variable reference to 'ThreadID' and may cause performance 
degradation [altera-id-dependent-backward-branch]
-  // CHECK-NOTES: :[[@LINE-30]]:3: note: assignment of ID-dependent variable 
ThreadID
+  // CHECK-NOTES: :[[@LINE-12]]:3: note: assignment of ID-dependent variable 
ThreadID
+
+  struct { int IDDepField; } Example;
+  Example.IDDepField = get_local_id(0);
 
   for (int i = 0; i < Example.IDDepField; i++) {
 // CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is 
ID-dependent due to member reference to 'IDDepField' and may cause performance 
degradation [altera-id-dependent-backward-branch]
-// CHECK-NOTES: :[[@LINE-25]]:3: note: assignment of ID-dependent field 
IDDepField
+// CHECK-NOTES: :[[@LINE-4]]:3: note: assignment of ID-dependent field 
IDDepField
 accumulator++;
   }
 
   while (j < Example.IDDepField) {
 // CHECK-NOTES: :[[@LINE-1]]:10: warning: backward branch (while loop) is 
ID-dependent due to member reference to 'IDDepField' and may cause performance 
degradation [altera-id-dependent-backward-branch]
-// CHECK-NOTES: :[[@LINE-31]]:3: note: assignment of ID-dependent field 
IDDepField
+// CHECK-NOTES: :[[@LINE-10]]:3: note: assignment of ID-dependent field 
IDDepField
 accumulator++;
   }
 
@@ -72,7 +53,22 @@ void error() {
 accumulator++;
   } while (j < Example.IDDepField);
   // CHECK-NOTES: :[[@LINE-1]]:12: warning: backward branch (do loop) is 
ID-dependent due to member reference to 'IDDepField' and may cause performance 
degradation [altera-id-dependent-backward-branch]
-  // CHECK-NOTES: :[[@LINE-39]]:3: note: assignment of ID-dependent field 
IDDepField
+  // CHECK-NOTES: :[[@LINE-18]]:3: note: assignment of ID-dependent field 
IDDepField
+
+  //  Inferred Assignments 
+  int ThreadID2 = ThreadID * 2;
+
+  for (int i = 0; i < ThreadID2; i++) {
+// CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is 
ID-dependent due to variable reference to 'ThreadID2' and may cause performance 
degradation [altera-id-dependent-backward-branch]
+// CHECK-NOTES: :[[@LINE-4]]:3: note: inferred assignment of ID-dependent 
value from ID-dependent variable ThreadID
+accumulator++;
+  }
+
+  //  Unused Inferred Assignments 
+  int UnusedThreadID = 

[PATCH] D145321: [clang-tidy] altera-id-dependent-backward-branch: fix direct assignment notes

[Egor Suvorov via Phabricator via cfe-commits]

yeputons-gh updated this revision to Diff 502447.
yeputons-gh added a comment.

Test inferred assignments and fix notes for them as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145321

Files:
  clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
@@ -34,6 +34,15 @@
   // CHECK-NOTES: :[[@LINE-1]]:12: warning: backward branch (do loop) is ID-dependent due to variable reference to 'ThreadID' and may cause performance degradation [altera-id-dependent-backward-branch]
   // CHECK-NOTES: :[[@LINE-12]]:3: note: assignment of ID-dependent variable ThreadID
 
+  int ThreadIDAssigned = 0;
+  ThreadIDAssigned = get_local_id(0) * 2;
+
+  for (int i = 0; i < ThreadIDAssigned; i++) {
+// CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is ID-dependent due to variable reference to 'ThreadIDAssigned' and may cause performance degradation [altera-id-dependent-backward-branch]
+// CHECK-NOTES: :[[@LINE-4]]:3: note: assignment of ID-dependent variable ThreadIDAssigned
+accumulator++;
+  }
+
   struct { int IDDepField; } Example;
   Example.IDDepField = get_local_id(0);
 
@@ -56,11 +65,49 @@
   // CHECK-NOTES: :[[@LINE-18]]:3: note: assignment of ID-dependent field IDDepField
 
   //  Inferred Assignments 
-  int ThreadID2 = ThreadID * 2;
+  int ThreadIDVarFromVar = ThreadID * 2;
+  for (int i = 0; i < ThreadIDVarFromVar; i++) {
+// CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is ID-dependent due to variable reference to 'ThreadIDVarFromVar' and may cause performance degradation [altera-id-dependent-backward-branch]
+// CHECK-NOTES: :[[@LINE-3]]:28: note: inferred assignment of ID-dependent value from ID-dependent variable ThreadID
+accumulator++;
+  }
+
+  int ThreadIDVarAssignFromVar;
+  ThreadIDVarAssignFromVar = ThreadID * 2;
+  for (int i = 0; i < ThreadIDVarAssignFromVar; i++) {
+// CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is ID-dependent due to variable reference to 'ThreadIDVarAssignFromVar' and may cause performance degradation [altera-id-dependent-backward-branch]
+// CHECK-NOTES: :[[@LINE-3]]:30: note: inferred assignment of ID-dependent value from ID-dependent variable ThreadID
+accumulator++;
+  }
+
+  int ThreadIDVarFromMember = Example.IDDepField * 2;
+  for (int i = 0; i < ThreadIDVarFromMember; i++) {
+// CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is ID-dependent due to variable reference to 'ThreadIDVarFromMember' and may cause performance degradation [altera-id-dependent-backward-branch]
+// CHECK-NOTES: :[[@LINE-3]]:31: note: inferred assignment of ID-dependent value from ID-dependent member IDDepField
+accumulator++;
+  }
+
+  int ThreadIDVarAssignFromMember;
+  ThreadIDVarAssignFromMember = Example.IDDepField * 2;
+  for (int i = 0; i < ThreadIDVarAssignFromMember; i++) {
+// CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is ID-dependent due to variable reference to 'ThreadIDVarAssignFromMember' and may cause performance degradation [altera-id-dependent-backward-branch]
+// CHECK-NOTES: :[[@LINE-3]]:33: note: inferred assignment of ID-dependent value from ID-dependent member IDDepField
+accumulator++;
+  }
+
+  struct { int IDDepFieldFromVar; } ExampleFromVar;
+  ExampleFromVar.IDDepFieldFromVar = ThreadID * 2;
+  for (int i = 0; i < ExampleFromVar.IDDepFieldFromVar; i++) {
+// CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is ID-dependent due to member reference to 'IDDepFieldFromVar' and may cause performance degradation [altera-id-dependent-backward-branch]
+// CHECK-NOTES: :[[@LINE-3]]:38: note: inferred assignment of ID-dependent member from ID-dependent variable ThreadID
+accumulator++;
+  }
 
-  for (int i = 0; i < ThreadID2; i++) {
-// CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is ID-dependent due to variable reference to 'ThreadID2' and may cause performance degradation [altera-id-dependent-backward-branch]
-// CHECK-NOTES: :[[@LINE-4]]:3: note: inferred assignment of ID-dependent value from ID-dependent variable ThreadID
+  struct { int IDDepFieldFromMember; } ExampleFromMember;
+  ExampleFromMember.IDDepFieldFromMember = Example.IDDepField * 2;
+  for (int i = 0; i < ExampleFromMember.IDDepFieldFromMember; i++) {
+// CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is ID-dependent due to member reference to 

[PATCH] D145321: [clang-tidy] altera-id-dependent-backward-branch: fix direct assignment notes

[Egor Suvorov via Phabricator via cfe-commits]

yeputons-gh created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
yeputons-gh requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Previously a note was always issued on a variable declaration itself. Now it
points to the actual assignment of `get_local_id` to the variable, like with
fields.

Depends on D145305 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145321

Files:
  clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
@@ -34,6 +34,15 @@
   // CHECK-NOTES: :[[@LINE-1]]:12: warning: backward branch (do loop) is 
ID-dependent due to variable reference to 'ThreadID' and may cause performance 
degradation [altera-id-dependent-backward-branch]
   // CHECK-NOTES: :[[@LINE-12]]:3: note: assignment of ID-dependent variable 
ThreadID
 
+  int ThreadIDAssigned = 0;
+  ThreadIDAssigned = get_local_id(0) * 2;
+
+  for (int i = 0; i < ThreadIDAssigned; i++) {
+// CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is 
ID-dependent due to variable reference to 'ThreadIDAssigned' and may cause 
performance degradation [altera-id-dependent-backward-branch]
+// CHECK-NOTES: :[[@LINE-4]]:3: note: assignment of ID-dependent variable 
ThreadIDAssigned
+accumulator++;
+  }
+
   struct { int IDDepField; } Example;
   Example.IDDepField = get_local_id(0);
 
@@ -107,6 +116,13 @@
 accumulator++;
   } while (j < NotThreadID);
 
+  int NotThreadIDAssigned = 0;
+  NotThreadIDAssigned = foo(0) * 2;
+
+  for (int i = 0; i < NotThreadIDAssigned; i++) {
+accumulator++;
+  }
+
   struct { int NotIDDepField; } Example;
   Example.NotIDDepField = foo(0);
 
Index: clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
===
--- clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
+++ clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
@@ -114,7 +114,7 @@
   const VarDecl *Variable) {
   // Record that this variable is thread-dependent.
   IdDepVarsMap[Variable] =
-  IdDependencyRecord(Variable, Variable->getBeginLoc(),
+  IdDependencyRecord(Variable, Statement->getBeginLoc(),
  Twine("assignment of ID-dependent variable ") +
  Variable->getNameAsString());
 }


Index: clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
@@ -34,6 +34,15 @@
   // CHECK-NOTES: :[[@LINE-1]]:12: warning: backward branch (do loop) is ID-dependent due to variable reference to 'ThreadID' and may cause performance degradation [altera-id-dependent-backward-branch]
   // CHECK-NOTES: :[[@LINE-12]]:3: note: assignment of ID-dependent variable ThreadID
 
+  int ThreadIDAssigned = 0;
+  ThreadIDAssigned = get_local_id(0) * 2;
+
+  for (int i = 0; i < ThreadIDAssigned; i++) {
+// CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is ID-dependent due to variable reference to 'ThreadIDAssigned' and may cause performance degradation [altera-id-dependent-backward-branch]
+// CHECK-NOTES: :[[@LINE-4]]:3: note: assignment of ID-dependent variable ThreadIDAssigned
+accumulator++;
+  }
+
   struct { int IDDepField; } Example;
   Example.IDDepField = get_local_id(0);
 
@@ -107,6 +116,13 @@
 accumulator++;
   } while (j < NotThreadID);
 
+  int NotThreadIDAssigned = 0;
+  NotThreadIDAssigned = foo(0) * 2;
+
+  for (int i = 0; i < NotThreadIDAssigned; i++) {
+accumulator++;
+  }
+
   struct { int NotIDDepField; } Example;
   Example.NotIDDepField = foo(0);
 
Index: clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
===
--- clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
+++ clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
@@ -114,7 +114,7 @@
   const VarDecl *Variable) {
   // Record that this variable is thread-dependent.
   IdDepVarsMap[Variable] =
-  IdDependencyRecord(Variable, Variable->getBeginLoc(),
+

[PATCH] D145305: [clang-tidy] altera-id-dependent-backward-branch: do not mark all variables

[Egor Suvorov via Phabricator via cfe-commits]

yeputons-gh updated this revision to Diff 502445.
yeputons-gh added a comment.

Rebased on top of the recent `main`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145305

Files:
  clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
  clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h
  
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
@@ -71,6 +71,8 @@
   UnusedStruct.IDDepField = ThreadID * 2; // OK: not used in any loops
 }
 
+int foo(int);
+
 void success() {
   int accumulator = 0;
 
@@ -79,4 +81,57 @@
   accumulator++;
 }
   }
+
+  //  Conditional Expressions 
+  for (int i = 0; i < foo(0); i++) {
+accumulator++;
+  }
+
+  int j = 0;
+  while (j < foo(0)) {
+accumulator++;
+  }
+
+  do {
+accumulator++;
+  } while (j < foo(0));
+
+  //  Assignments 
+  int NotThreadID = foo(0);
+
+  while (j < NotThreadID) {
+accumulator++;
+  }
+
+  do {
+accumulator++;
+  } while (j < NotThreadID);
+
+  struct { int NotIDDepField; } Example;
+  Example.NotIDDepField = foo(0);
+
+  for (int i = 0; i < Example.NotIDDepField; i++) {
+accumulator++;
+  }
+
+  while (j < Example.NotIDDepField) {
+accumulator++;
+  }
+
+  do {
+accumulator++;
+  } while (j < Example.NotIDDepField);
+
+  //  Inferred Assignments 
+  int NotThreadID2 = NotThreadID * 2;
+
+  for (int i = 0; i < NotThreadID2; i++) {
+accumulator++;
+  }
+
+  //  Unused Inferred Assignments 
+  int UnusedNotThreadID = Example.NotIDDepField;
+
+  struct { int NotIDDepField; } UnusedStruct;
+  UnusedStruct.NotIDDepField = NotThreadID * 2;
 }
Index: clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h
===
--- clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h
+++ clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h
@@ -56,14 +56,14 @@
   void saveIdDepField(const Stmt *Statement, const FieldDecl *Field);
   /// Stores the location an ID-dependent variable is created from a reference
   /// to another ID-dependent variable or field in IdDepVarsMap.
-  void saveIdDepVarFromReference(const DeclRefExpr *RefExpr,
- const MemberExpr *MemExpr,
- const VarDecl *PotentialVar);
+  void saveIdDepVarFromPotentialReference(const DeclRefExpr *RefExpr,
+  const MemberExpr *MemExpr,
+  const VarDecl *PotentialVar);
   /// Stores the location an ID-dependent field is created from a reference to
   /// another ID-dependent variable or field in IdDepFieldsMap.
-  void saveIdDepFieldFromReference(const DeclRefExpr *RefExpr,
-   const MemberExpr *MemExpr,
-   const FieldDecl *PotentialField);
+  void saveIdDepFieldFromPotentialReference(const DeclRefExpr *RefExpr,
+const MemberExpr *MemExpr,
+const FieldDecl *PotentialField);
   /// Returns the loop type.
   LoopType getLoopType(const Stmt *Loop);
 
Index: clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
===
--- clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
+++ clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
@@ -127,7 +127,7 @@
   Twine("assignment of ID-dependent field ") + Field->getNameAsString());
 }
 
-void IdDependentBackwardBranchCheck::saveIdDepVarFromReference(
+void IdDependentBackwardBranchCheck::saveIdDepVarFromPotentialReference(
 const DeclRefExpr *RefExpr, const MemberExpr *MemExpr,
 const VarDecl *PotentialVar) {
   // If the variable is already in IdDepVarsMap, ignore it.
@@ -140,20 +140,26 @@
   if (RefExpr) {
 const auto *RefVar = dyn_cast(RefExpr->getDecl());
 // If variable isn't ID-dependent, but RefVar is.
-if (IdDepVarsMap.find(RefVar) != IdDepVarsMap.end())
+if (IdDepVarsMap.find(RefVar) != IdDepVarsMap.end()) {
   StringStream << "variable " << RefVar->getNameAsString();
+  IdDepVarsMap[PotentialVar] = IdDependencyRecord(
+  PotentialVar, PotentialVar->getBeginLoc(), Message);
+  return; // Optional, as we only match only one of `RefExpr` or `MemExpr`
+}
   }
   if (MemExpr) {
 const auto *RefField = dyn_cast(MemExpr->getMemberDecl());
 // If variable isn't 

[PATCH] D145304: [clang-tidy] altera-id-dependent-backward-branch: refactor test

[Egor Suvorov via Phabricator via cfe-commits]

yeputons-gh updated this revision to Diff 502442.
yeputons-gh added a comment.

Rebased on top of the recent 'main'.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145304

Files:
  
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
@@ -1,9 +1,5 @@
 // RUN: %check_clang_tidy %s altera-id-dependent-backward-branch %t -- 
-header-filter=.* "--" -cl-std=CL1.2 -c
 
-typedef struct ExampleStruct {
-  int IDDepField;
-} ExampleStruct;
-
 void error() {
   //  Conditional Expressions 
   int accumulator = 0;
@@ -32,39 +28,24 @@
 accumulator++;
   }
 
-  ExampleStruct Example;
-  Example.IDDepField = get_local_id(0);
-
-  //  Inferred Assignments 
-  int ThreadID2 = ThreadID * get_local_size(0);
-
-  int ThreadID3 = Example.IDDepField; // OK: not used in any loops
-
-  ExampleStruct UnusedStruct = {
-  ThreadID * 2 // OK: not used in any loops
-  };
-
-  for (int i = 0; i < ThreadID2; i++) {
-// CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is 
ID-dependent due to variable reference to 'ThreadID2' and may cause performance 
degradation [altera-id-dependent-backward-branch]
-// CHECK-NOTES: :[[@LINE-10]]:3: note: inferred assignment of ID-dependent 
value from ID-dependent variable ThreadID
-accumulator++;
-  }
-
   do {
 accumulator++;
   } while (j < ThreadID);
   // CHECK-NOTES: :[[@LINE-1]]:12: warning: backward branch (do loop) is 
ID-dependent due to variable reference to 'ThreadID' and may cause performance 
degradation [altera-id-dependent-backward-branch]
-  // CHECK-NOTES: :[[@LINE-30]]:3: note: assignment of ID-dependent variable 
ThreadID
+  // CHECK-NOTES: :[[@LINE-12]]:3: note: assignment of ID-dependent variable 
ThreadID
+
+  struct { int IDDepField; } Example;
+  Example.IDDepField = get_local_id(0);
 
   for (int i = 0; i < Example.IDDepField; i++) {
 // CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is 
ID-dependent due to member reference to 'IDDepField' and may cause performance 
degradation [altera-id-dependent-backward-branch]
-// CHECK-NOTES: :[[@LINE-25]]:3: note: assignment of ID-dependent field 
IDDepField
+// CHECK-NOTES: :[[@LINE-4]]:3: note: assignment of ID-dependent field 
IDDepField
 accumulator++;
   }
 
   while (j < Example.IDDepField) {
 // CHECK-NOTES: :[[@LINE-1]]:10: warning: backward branch (while loop) is 
ID-dependent due to member reference to 'IDDepField' and may cause performance 
degradation [altera-id-dependent-backward-branch]
-// CHECK-NOTES: :[[@LINE-31]]:3: note: assignment of ID-dependent field 
IDDepField
+// CHECK-NOTES: :[[@LINE-10]]:3: note: assignment of ID-dependent field 
IDDepField
 accumulator++;
   }
 
@@ -72,7 +53,22 @@
 accumulator++;
   } while (j < Example.IDDepField);
   // CHECK-NOTES: :[[@LINE-1]]:12: warning: backward branch (do loop) is 
ID-dependent due to member reference to 'IDDepField' and may cause performance 
degradation [altera-id-dependent-backward-branch]
-  // CHECK-NOTES: :[[@LINE-39]]:3: note: assignment of ID-dependent field 
IDDepField
+  // CHECK-NOTES: :[[@LINE-18]]:3: note: assignment of ID-dependent field 
IDDepField
+
+  //  Inferred Assignments 
+  int ThreadID2 = ThreadID * 2;
+
+  for (int i = 0; i < ThreadID2; i++) {
+// CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is 
ID-dependent due to variable reference to 'ThreadID2' and may cause performance 
degradation [altera-id-dependent-backward-branch]
+// CHECK-NOTES: :[[@LINE-4]]:3: note: inferred assignment of ID-dependent 
value from ID-dependent variable ThreadID
+accumulator++;
+  }
+
+  //  Unused Inferred Assignments 
+  int UnusedThreadID = Example.IDDepField; // OK: not used in any loops
+
+  struct { int IDDepField; } UnusedStruct;
+  UnusedStruct.IDDepField = ThreadID * 2; // OK: not used in any loops
 }
 
 void success() {


Index: clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
@@ -1,9 +1,5 @@
 // RUN: %check_clang_tidy %s altera-id-dependent-backward-branch %t -- -header-filter=.* "--" -cl-std=CL1.2 -c
 
-typedef struct ExampleStruct {
-  int IDDepField;
-} ExampleStruct;
-
 void error() {
   //  Conditional Expressions 
   int accumulator = 0;
@@ -32,39 +28,24 @@
 

[PATCH] D145304: [clang-tidy] altera-id-dependent-backward-branch: refactor test

[Egor Suvorov via Phabricator via cfe-commits]

yeputons-gh added a comment.

Absolutely, give we few minutes. Maybe it conflicts with the previous revision 
that has already landed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145304

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

[PATCH] D145304: [clang-tidy] altera-id-dependent-backward-branch: refactor test

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

It seems I'm having trouble to download the patch with arcanist, would you mind 
rebasing on top of the main branch? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145304

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

[PATCH] D145304: [clang-tidy] altera-id-dependent-backward-branch: refactor test

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp accepted this revision.
carlosgalvezp 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/D145304/new/

https://reviews.llvm.org/D145304

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

[PATCH] D145313: [clang-tidy] Make readability-container-size-empty check using header

[Mike Crowe via Phabricator via cfe-commits]

mikecrowe added a comment.

In D145313#4169790 , @carlosgalvezp 
wrote:

> Could you upload the patch with full context? I believe you need to do 
> something like `git show HEAD -U99` as per the guidelines 
> . Otherwise `arc diff` should do the 
> job automatically. Reason I ask is that I cannot see the new line numbers 
> referred to by the `note` comments - Phab says "Context not available".

Of course. It had been sufficiently long since I last uploaded a new diff that 
I forgot that part of the instructions and just used `git format-patch` for 
this batch. I shall try to learn how to use `arc diff` because the copying and 
pasting of diffs was getting rather tedious and I knew I was bound to make a 
mistake with it eventually.

Thanks!


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

https://reviews.llvm.org/D145313

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

[PATCH] D145312: [clang-tidy] Make readability-string-compare check use header

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability/string-compare.cpp:4-8
-template 
-class allocator {};
-template 
-class char_traits {};
-template , typename A = 
std::allocator>

Would it make sense to add these to the new header?


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

https://reviews.llvm.org/D145312

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

[PATCH] D145313: [clang-tidy] Make readability-container-size-empty check using header

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

Could you upload the patch with full context? I believe you need to do 
something like `git show HEAD -U99` as per the guidelines 
. Otherwise `arc diff` should do the 
job automatically. Reason I ask is that I cannot see the new line numbers 
referred to by the `note` comments - Phab says "Context not available".


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

https://reviews.llvm.org/D145313

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

[PATCH] D145310: [clang-tidy] Make readability-container-data-pointer use header

[Carlos Galvez via Phabricator via cfe-commits]

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

LGTM, would you mind rebasing to get the pre-merge jobs green?


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

https://reviews.llvm.org/D145310

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

[PATCH] D144216: [clang-tidy] Extract string header from redundant-string-cstr checker

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

In D144216#4169781 , @mikecrowe wrote:

> In D144216#4169772 , @carlosgalvezp 
> wrote:
>
>> In D144216#4169764 , @mikecrowe 
>> wrote:
>>
 I will double check that this is true once my current build is complete.
>>>
>>> Yes, it's true. I stuck a `#error` in 
>>> `clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string.h` and I 
>>> saw the expected error from a file including ``.
>>
>> Awesome, thanks for checking, I wasn't aware we already had `string.h`. Then 
>> if we do have headers named the same as the standard headers, would it make 
>> sense to name this header `cstring` instead of `string`?
>
> I think you mean rename `string.h` to `cstring`. (`string` clearly has to be 
> just `string` if it's the standard C++ `string` header.)
>
> A bit of searching in the existing checks shows that `string.h` is included 
> by C checks (`bugprone/signal-handler*.c`) and also by checks that 
> deliberately want `string.h` so they can suggest switching to `cstring` 
> (`modernize/deprecated-headers*`) so it looks like it's necessary to keep 
> `string.h`. If you wish, a `cstring` wrapper that just includes `string.h` 
> could be created, but it really ought to put everything in `namespace std` 
> too. (If so, I think this would probably be better done as a separate change.)

You are absolutely right, I don't know why I mixed `cstring` with `string`, 
nevermind! Pushed now, will look at the follow up patches soon. Thanks again 
for the fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144216

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

[PATCH] D144216: [clang-tidy] Extract string header from redundant-string-cstr checker

[Carlos Galvez via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGae25e2f19dec: [clang-tidy] Extract string header from 
redundant-string-cstr checker (authored by mikecrowe, committed by 
carlosgalvezp).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144216

Files:
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
  
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
@@ -1,71 +1,5 @@
-// RUN: %check_clang_tidy %s readability-redundant-string-cstr %t
-
-typedef unsigned __INT16_TYPE__ char16;
-typedef unsigned __INT32_TYPE__ char32;
-typedef __SIZE_TYPE__ size;
-
-namespace std {
-template 
-class allocator {};
-template 
-class char_traits {};
-template 
-struct basic_string {
-  typedef basic_string _Type;
-  basic_string();
-  basic_string(const C *p, const A  = A());
-
-  ~basic_string();
-
-  const C *c_str() const;
-  const C *data() const;
-
-  _Type& append(const C *s);
-  _Type& append(const C *s, size n);
-  _Type& assign(const C *s);
-  _Type& assign(const C *s, size n);
-
-  int compare(const _Type&) const;
-  int compare(const C* s) const;
-  int compare(size pos, size len, const _Type&) const;
-  int compare(size pos, size len, const C* s) const;
-
-  size find(const _Type& str, size pos = 0) const;
-  size find(const C* s, size pos = 0) const;
-  size find(const C* s, size pos, size n) const;
-
-  _Type& insert(size pos, const _Type& str);
-  _Type& insert(size pos, const C* s);
-  _Type& insert(size pos, const C* s, size n);
-
-  _Type& operator+=(const _Type& str);
-  _Type& operator+=(const C* s);
-  _Type& operator=(const _Type& str);
-  _Type& operator=(const C* s);
-};
-
-typedef basic_string, std::allocator> string;
-typedef basic_string, std::allocator> wstring;
-typedef basic_string, std::allocator> u16string;
-typedef basic_string, std::allocator> u32string;
-
-template 
-struct basic_string_view {
-  basic_string_view(const C* s);
-};
-typedef basic_string_view> string_view;
-typedef basic_string_view> wstring_view;
-typedef basic_string_view> u16string_view;
-typedef basic_string_view> u32string_view;
-}
-
-std::string operator+(const std::string&, const std::string&);
-std::string operator+(const std::string&, const char*);
-std::string operator+(const char*, const std::string&);
-
-bool operator==(const std::string&, const std::string&);
-bool operator==(const std::string&, const char*);
-bool operator==(const char*, const std::string&);
+// RUN: %check_clang_tidy %s readability-redundant-string-cstr %t -- -- -isystem %clang_tidy_headers
+#include 
 
 namespace llvm {
 struct StringRef {
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
@@ -0,0 +1,74 @@
+#ifndef _STRING_
+#define _STRING_
+
+// For size_t
+#include 
+
+typedef unsigned __INT16_TYPE__ char16;
+typedef unsigned __INT32_TYPE__ char32;
+
+namespace std {
+template 
+class allocator {};
+template 
+class char_traits {};
+template 
+struct basic_string {
+  typedef size_t size_type;
+  typedef basic_string _Type;
+  basic_string();
+  basic_string(const C *p, const A  = A());
+
+  ~basic_string();
+
+  const C *c_str() const;
+  const C *data() const;
+
+  _Type& append(const C *s);
+  _Type& append(const C *s, size_type n);
+  _Type& assign(const C *s);
+  _Type& assign(const C *s, size_type n);
+
+  int compare(const _Type&) const;
+  int compare(const C* s) const;
+  int compare(size_type pos, size_type len, const _Type&) const;
+  int compare(size_type pos, size_type len, const C* s) const;
+
+  size_type find(const _Type& str, size_type pos = 0) const;
+  size_type find(const C* s, size_type pos = 0) const;
+  size_type find(const C* s, size_type pos, size_type n) const;
+
+  _Type& insert(size_type pos, const _Type& str);
+  _Type& insert(size_type pos, const C* s);
+  _Type& insert(size_type pos, const C* s, size_type n);
+
+  _Type& operator+=(const _Type& str);
+  _Type& operator+=(const C* s);
+  _Type& operator=(const _Type& str);
+  _Type& operator=(const C* s);
+};
+
+typedef basic_string, std::allocator> string;
+typedef basic_string, std::allocator> wstring;
+typedef basic_string, std::allocator> u16string;
+typedef basic_string, std::allocator> u32string;
+
+template 
+struct basic_string_view {
+  basic_string_view(const C* s);
+};
+typedef basic_string_view> string_view;
+typedef basic_string_view> wstring_view;
+typedef basic_string_view> 

[clang-tools-extra] ae25e2f - [clang-tidy] Extract string header from redundant-string-cstr checker

[Carlos Galvez via cfe-commits]

Author: Mike Crowe
Date: 2023-03-05T17:16:20Z
New Revision: ae25e2f19decb94198301f0726ee613f945cc405

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

LOG: [clang-tidy] Extract string header from redundant-string-cstr checker

In preparation for using the implementation of basic_string and
basic_string_view from redundant-string-cstr.cpp in other checks, let's
extract it to a header.

Using the existing  to provide size_t, using that to define
size_type and using it rather than the previous "size" type makes it
easier to provide the size() method later and makes the implementation
closer to the standard.

Reviewed By: carlosgalvezp

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

Added: 
clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string

Modified: 

clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp

Removed: 




diff  --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string 
b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
new file mode 100644
index 0..614480968f2aa
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
@@ -0,0 +1,74 @@
+#ifndef _STRING_
+#define _STRING_
+
+// For size_t
+#include 
+
+typedef unsigned __INT16_TYPE__ char16;
+typedef unsigned __INT32_TYPE__ char32;
+
+namespace std {
+template 
+class allocator {};
+template 
+class char_traits {};
+template 
+struct basic_string {
+  typedef size_t size_type;
+  typedef basic_string _Type;
+  basic_string();
+  basic_string(const C *p, const A  = A());
+
+  ~basic_string();
+
+  const C *c_str() const;
+  const C *data() const;
+
+  _Type& append(const C *s);
+  _Type& append(const C *s, size_type n);
+  _Type& assign(const C *s);
+  _Type& assign(const C *s, size_type n);
+
+  int compare(const _Type&) const;
+  int compare(const C* s) const;
+  int compare(size_type pos, size_type len, const _Type&) const;
+  int compare(size_type pos, size_type len, const C* s) const;
+
+  size_type find(const _Type& str, size_type pos = 0) const;
+  size_type find(const C* s, size_type pos = 0) const;
+  size_type find(const C* s, size_type pos, size_type n) const;
+
+  _Type& insert(size_type pos, const _Type& str);
+  _Type& insert(size_type pos, const C* s);
+  _Type& insert(size_type pos, const C* s, size_type n);
+
+  _Type& operator+=(const _Type& str);
+  _Type& operator+=(const C* s);
+  _Type& operator=(const _Type& str);
+  _Type& operator=(const C* s);
+};
+
+typedef basic_string, std::allocator> 
string;
+typedef basic_string, 
std::allocator> wstring;
+typedef basic_string, std::allocator> 
u16string;
+typedef basic_string, std::allocator> 
u32string;
+
+template 
+struct basic_string_view {
+  basic_string_view(const C* s);
+};
+typedef basic_string_view> string_view;
+typedef basic_string_view> wstring_view;
+typedef basic_string_view> u16string_view;
+typedef basic_string_view> u32string_view;
+
+std::string operator+(const std::string&, const std::string&);
+std::string operator+(const std::string&, const char*);
+std::string operator+(const char*, const std::string&);
+
+bool operator==(const std::string&, const std::string&);
+bool operator==(const std::string&, const char*);
+bool operator==(const char*, const std::string&);
+}
+
+#endif // _STRING_

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
index ed50ad16f2423..2b6d290a4aff6 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
@@ -1,71 +1,5 @@
-// RUN: %check_clang_tidy %s readability-redundant-string-cstr %t
-
-typedef unsigned __INT16_TYPE__ char16;
-typedef unsigned __INT32_TYPE__ char32;
-typedef __SIZE_TYPE__ size;
-
-namespace std {
-template 
-class allocator {};
-template 
-class char_traits {};
-template 
-struct basic_string {
-  typedef basic_string _Type;
-  basic_string();
-  basic_string(const C *p, const A  = A());
-
-  ~basic_string();
-
-  const C *c_str() const;
-  const C *data() const;
-
-  _Type& append(const C *s);
-  _Type& append(const C *s, size n);
-  _Type& assign(const C *s);
-  _Type& assign(const C *s, size n);
-
-  int compare(const _Type&) const;
-  int compare(const C* s) const;
-  int compare(size pos, size len, const _Type&) const;
-  int compare(size pos, size len, const C* s) const;
-
-  size find(const _Type& str, size pos = 0) const;
-  size find(const C* s, size pos = 0) const;
-  size find(const C* s, size pos, size n) const;
-
-  _Type& insert(size pos, const _Type& str);
-  _Type& insert(size pos, const C* s);
-  

[PATCH] D144216: [clang-tidy] Extract string header from redundant-string-cstr checker

[Mike Crowe via Phabricator via cfe-commits]

mikecrowe added a comment.

In D144216#4169772 , @carlosgalvezp 
wrote:

> In D144216#4169764 , @mikecrowe 
> wrote:
>
>>> I will double check that this is true once my current build is complete.
>>
>> Yes, it's true. I stuck a `#error` in 
>> `clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string.h` and I 
>> saw the expected error from a file including ``.
>
> Awesome, thanks for checking, I wasn't aware we already had `string.h`. Then 
> if we do have headers named the same as the standard headers, would it make 
> sense to name this header `cstring` instead of `string`?

I think you mean rename `string.h` to `cstring`. (`string` clearly has to be 
just `string` if it's the standard C++ `string` header.)

A bit of searching in the existing checks shows that `string.h` is included by 
C checks (`bugprone/signal-handler*.c`) and also by checks that deliberately 
want `string.h` so they can suggest switching to `cstring` 
(`modernize/deprecated-headers*`) so it looks like it's necessary to keep 
`string.h`. If you wish, a `cstring` wrapper that just includes `string.h` 
could be created, but it really ought to put everything in `namespace std` too. 
(If so, I think this would probably be better done as a separate change.)


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

https://reviews.llvm.org/D144216

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

[PATCH] D144216: [clang-tidy] Extract string header from redundant-string-cstr checker

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

In D144216#4169764 , @mikecrowe wrote:

>> I will double check that this is true once my current build is complete.
>
> Yes, it's true. I stuck a `#error` in 
> `clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string.h` and I 
> saw the expected error from a file including ``.

Awesome, thanks for checking, I wasn't aware we already had `string.h`. Then if 
we do have headers named the same as the standard headers, would it make sense 
to name this header `cstring` instead of `string`?


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

https://reviews.llvm.org/D144216

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

[PATCH] D145125: [RISCV] Make D extension imply F extension.

[Alex Bradbury via Phabricator via cfe-commits]

asb accepted this revision.
asb added a comment.

I think this is in line with how we now handle implication of specs - when the 
code was first written IIRC we were much stricter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145125

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

[PATCH] D144216: [clang-tidy] Extract string header from redundant-string-cstr checker

[Mike Crowe via Phabricator via cfe-commits]

mikecrowe added a comment.

> I will double check that this is true once my current build is complete.

Yes, it's true. I stuck a `#error` in 
`clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string.h` and I saw 
the expected error from a file including ``.


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

https://reviews.llvm.org/D144216

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

[PATCH] D144216: [clang-tidy] Extract string header from redundant-string-cstr checker

[Mike Crowe via Phabricator via cfe-commits]

mikecrowe added a comment.

> The purpose of this header is to not include any standard header, and yet 
> this is done in line 5 so it kinda defeats the purpose.

I believe that this `` header is including 
`clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string.h` rather 
than the system `string.h` header. (That's why I'm including `string.h` to get 
`size_t` rather than the more obvious `stdlib.h`.) If we're going to have sets 
of "standard-but-not-standard" headers for these checks then it feels like it 
ought to be safe to use them.

I will double check that this is true once my current build is complete.

Mike.


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

https://reviews.llvm.org/D144216

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

[PATCH] D145303: clang-tidy altera-id-dependent-backward-branch: print notes after warning

[Carlos Galvez via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc6d195b366c8: clang-tidy 
altera-id-dependent-backward-branch: print notes after warning (authored by 
yeputons-gh, committed by carlosgalvezp).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145303

Files:
  clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
@@ -27,8 +27,8 @@
   int ThreadID = get_local_id(0);
 
   while (j < ThreadID) {
-// CHECK-NOTES: :[[@LINE-3]]:3: note: assignment of ID-dependent variable 
ThreadID
-// CHECK-NOTES: :[[@LINE-2]]:10: warning: backward branch (while loop) is 
ID-dependent due to variable reference to 'ThreadID' and may cause performance 
degradation [altera-id-dependent-backward-branch]
+// CHECK-NOTES: :[[@LINE-1]]:10: warning: backward branch (while loop) is 
ID-dependent due to variable reference to 'ThreadID' and may cause performance 
degradation [altera-id-dependent-backward-branch]
+// CHECK-NOTES: :[[@LINE-4]]:3: note: assignment of ID-dependent variable 
ThreadID
 accumulator++;
   }
 
@@ -45,34 +45,34 @@
   };
 
   for (int i = 0; i < ThreadID2; i++) {
-// CHECK-NOTES: :[[@LINE-9]]:3: note: inferred assignment of ID-dependent 
value from ID-dependent variable ThreadID
-// CHECK-NOTES: :[[@LINE-2]]:19: warning: backward branch (for loop) is 
ID-dependent due to variable reference to 'ThreadID2' and may cause performance 
degradation [altera-id-dependent-backward-branch]
+// CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is 
ID-dependent due to variable reference to 'ThreadID2' and may cause performance 
degradation [altera-id-dependent-backward-branch]
+// CHECK-NOTES: :[[@LINE-10]]:3: note: inferred assignment of ID-dependent 
value from ID-dependent variable ThreadID
 accumulator++;
   }
 
   do {
 accumulator++;
   } while (j < ThreadID);
-  // CHECK-NOTES: :[[@LINE-29]]:3: note: assignment of ID-dependent variable 
ThreadID
-  // CHECK-NOTES: :[[@LINE-2]]:12: warning: backward branch (do loop) is 
ID-dependent due to variable reference to 'ThreadID' and may cause performance 
degradation [altera-id-dependent-backward-branch]
+  // CHECK-NOTES: :[[@LINE-1]]:12: warning: backward branch (do loop) is 
ID-dependent due to variable reference to 'ThreadID' and may cause performance 
degradation [altera-id-dependent-backward-branch]
+  // CHECK-NOTES: :[[@LINE-30]]:3: note: assignment of ID-dependent variable 
ThreadID
 
   for (int i = 0; i < Example.IDDepField; i++) {
-// CHECK-NOTES: :[[@LINE-24]]:3: note: assignment of ID-dependent field 
IDDepField
-// CHECK-NOTES: :[[@LINE-2]]:19: warning: backward branch (for loop) is 
ID-dependent due to member reference to 'IDDepField' and may cause performance 
degradation [altera-id-dependent-backward-branch]
+// CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is 
ID-dependent due to member reference to 'IDDepField' and may cause performance 
degradation [altera-id-dependent-backward-branch]
+// CHECK-NOTES: :[[@LINE-25]]:3: note: assignment of ID-dependent field 
IDDepField
 accumulator++;
   }
 
   while (j < Example.IDDepField) {
-// CHECK-NOTES: :[[@LINE-30]]:3: note: assignment of ID-dependent field 
IDDepField
-// CHECK-NOTES: :[[@LINE-2]]:10: warning: backward branch (while loop) is 
ID-dependent due to member reference to 'IDDepField' and may cause performance 
degradation [altera-id-dependent-backward-branch]
+// CHECK-NOTES: :[[@LINE-1]]:10: warning: backward branch (while loop) is 
ID-dependent due to member reference to 'IDDepField' and may cause performance 
degradation [altera-id-dependent-backward-branch]
+// CHECK-NOTES: :[[@LINE-31]]:3: note: assignment of ID-dependent field 
IDDepField
 accumulator++;
   }
 
   do {
 accumulator++;
   } while (j < Example.IDDepField);
-  // CHECK-NOTES: :[[@LINE-38]]:3: note: assignment of ID-dependent field 
IDDepField
-  // CHECK-NOTES: :[[@LINE-2]]:12: warning: backward branch (do loop) is 
ID-dependent due to member reference to 'IDDepField' and may cause performance 
degradation [altera-id-dependent-backward-branch]
+  // CHECK-NOTES: :[[@LINE-1]]:12: warning: backward branch (do loop) is 
ID-dependent due to member reference to 'IDDepField' and may cause performance 
degradation [altera-id-dependent-backward-branch]
+  // CHECK-NOTES: :[[@LINE-39]]:3: note: assignment of ID-dependent field 
IDDepField
 }
 
 void success() {

[clang-tools-extra] c6d195b - clang-tidy altera-id-dependent-backward-branch: print notes after warning

[Carlos Galvez via cfe-commits]

Author: Egor Suvorov
Date: 2023-03-05T16:06:55Z
New Revision: c6d195b366c8256184ff40c5e46339eed96b4a81

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

LOG: clang-tidy altera-id-dependent-backward-branch: print notes after warning

In Clang notes are typically printed after a corresponding warning, not before.
For example, all notes issued before the first warning are ignored.

Running `clang-tidy --checks=-*,altera-id-dependent-backward-branch a.cpp` on 
the following code:
```
long get_local_id(int);
void error() {
  int ThreadID = get_local_id(0);
  for (int i = 0; i < ThreadID; i++) {
  }
  for (int i = 0; i < ThreadID; i++) {
  }
}
```
results in two warnings and a single note in between.

Reviewed By: carlosgalvezp

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp

clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp 
b/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
index 66e6d97f500ce..f9225f6ba7f47 100644
--- a/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
+++ b/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
@@ -241,18 +241,17 @@ void IdDependentBackwardBranchCheck::check(
 IdDependencyRecord *IdDepVar = hasIdDepVar(CondExpr);
 IdDependencyRecord *IdDepField = hasIdDepField(CondExpr);
 if (IdDepVar) {
-  // Change one of these to a Note
-  diag(IdDepVar->Location, IdDepVar->Message, DiagnosticIDs::Note);
   diag(CondExpr->getBeginLoc(),
"backward branch (%select{do|while|for}0 loop) is ID-dependent due "
"to variable reference to %1 and may cause performance degradation")
   << Type << IdDepVar->VariableDeclaration;
+  diag(IdDepVar->Location, IdDepVar->Message, DiagnosticIDs::Note);
 } else if (IdDepField) {
-  diag(IdDepField->Location, IdDepField->Message, DiagnosticIDs::Note);
   diag(CondExpr->getBeginLoc(),
"backward branch (%select{do|while|for}0 loop) is ID-dependent due "
"to member reference to %1 and may cause performance degradation")
   << Type << IdDepField->FieldDeclaration;
+  diag(IdDepField->Location, IdDepField->Message, DiagnosticIDs::Note);
 }
   }
 }

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
index 68f4658a7dc9b..0c7a20ad6a179 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
@@ -27,8 +27,8 @@ void error() {
   int ThreadID = get_local_id(0);
 
   while (j < ThreadID) {
-// CHECK-NOTES: :[[@LINE-3]]:3: note: assignment of ID-dependent variable 
ThreadID
-// CHECK-NOTES: :[[@LINE-2]]:10: warning: backward branch (while loop) is 
ID-dependent due to variable reference to 'ThreadID' and may cause performance 
degradation [altera-id-dependent-backward-branch]
+// CHECK-NOTES: :[[@LINE-1]]:10: warning: backward branch (while loop) is 
ID-dependent due to variable reference to 'ThreadID' and may cause performance 
degradation [altera-id-dependent-backward-branch]
+// CHECK-NOTES: :[[@LINE-4]]:3: note: assignment of ID-dependent variable 
ThreadID
 accumulator++;
   }
 
@@ -45,34 +45,34 @@ void error() {
   };
 
   for (int i = 0; i < ThreadID2; i++) {
-// CHECK-NOTES: :[[@LINE-9]]:3: note: inferred assignment of ID-dependent 
value from ID-dependent variable ThreadID
-// CHECK-NOTES: :[[@LINE-2]]:19: warning: backward branch (for loop) is 
ID-dependent due to variable reference to 'ThreadID2' and may cause performance 
degradation [altera-id-dependent-backward-branch]
+// CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is 
ID-dependent due to variable reference to 'ThreadID2' and may cause performance 
degradation [altera-id-dependent-backward-branch]
+// CHECK-NOTES: :[[@LINE-10]]:3: note: inferred assignment of ID-dependent 
value from ID-dependent variable ThreadID
 accumulator++;
   }
 
   do {
 accumulator++;
   } while (j < ThreadID);
-  // CHECK-NOTES: :[[@LINE-29]]:3: note: assignment of ID-dependent variable 
ThreadID
-  // CHECK-NOTES: :[[@LINE-2]]:12: warning: backward branch (do loop) is 
ID-dependent due to variable reference to 'ThreadID' and may cause performance 
degradation [altera-id-dependent-backward-branch]
+  // CHECK-NOTES: :[[@LINE-1]]:12: warning: backward branch (do loop) is 

[PATCH] D144216: [clang-tidy] Extract string header from redundant-string-cstr checker

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string:4-5
+
+// For size_t
+#include 
+

Sorry I just noticed this - should we keep using the `__SYZE_TYPE__` macro that 
existed in the previous patch? The purpose of this header is to not include any 
standard header, and yet this is done in line 5 so it kinda defeats the purpose.

I.e add the following under line 8:

```
typedef __SIZE_TYPE__ size_t
```

And then use that in line 17.


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

https://reviews.llvm.org/D144216

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

[PATCH] D144216: [clang-tidy] Extract string header from redundant-string-cstr checker

[Mike Crowe via Phabricator via cfe-commits]

mikecrowe added a comment.

> Sounds good, should we land this?

If you're happy to do so, then so am I.

> If you don't have commit rights, please let us know Github name and email for 
> attribution.

These are my first code contributions to LLVM, so I don't really know the 
process. I don't believe that I have commit rights. My GitHub username is 
"mikecrowe" and my email address is "m...@mcrowe.com".

Thanks!

Mike.


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

https://reviews.llvm.org/D144216

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

[PATCH] D143375: clang-tidy: Count template constructors in modernize-use-default-member-init

[Carlos Galvez via Phabricator via cfe-commits]

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

Thanks for the fix!


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

https://reviews.llvm.org/D143375

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

[PATCH] D145303: clang-tidy altera-id-dependent-backward-branch: print notes after warning

[Egor Suvorov via Phabricator via cfe-commits]

yeputons-gh added a comment.

Hi, @carlosgalvezp ! I don't have any commit rights for the LLVM project, so I 
probably need help. My GitHub handle is `yeputons` 
(https://github.com/yeputons/), my email for attribution is 
`egor.suvo...@gmail.com` (that's my first name, last name)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145303

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

[PATCH] D144216: [clang-tidy] Extract string header from redundant-string-cstr checker

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp accepted this revision.
carlosgalvezp added a comment.

Sounds good, should we land this? If you don't have commit rights, please let 
us know Github name and email for attribution.


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

https://reviews.llvm.org/D144216

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

[PATCH] D145303: clang-tidy altera-id-dependent-backward-branch: print notes after warning

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

Thanks for the fix! Can you land the patch or would need that we do it for you? 
If so please provide Github name and email for attribution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145303

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

[PATCH] D145319: [clangd] Refine logic on $0 in completion snippets

[Younan Zhang via Phabricator via cfe-commits]

zyounan created this revision.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
zyounan added reviewers: nridge, kadircet.
zyounan published this revision for review.
zyounan added inline comments.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.



Comment at: clang-tools-extra/clangd/CodeCompletionStrings.cpp:130
+  return false;
+return true;
+  }();

I was cringed that if we should refine the logic based on `CursorKind`: It is 
from libclang; The meaning is sometimes kind of opaque (to me, I don't know it 
very clearly TBH) like `CXCursor_NotImplemented`...


We have a workaround from D128621  that makes 
$0 no longer being
a placeholder to conform a vscode feature. However, we have to
refine the logic as it may suppress the last parameter placeholder
for constructor of base class because not all patterns of completion
are compound statements.

This fixes clangd/clangd#1479


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145319

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/CodeCompletionStrings.cpp
  clang-tools-extra/clangd/CodeCompletionStrings.h
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -3450,6 +3450,22 @@
   EXPECT_THAT(Results.Completions,
   Contains(AllOf(named("while_foo"),
  snippetSuffix("(${1:int a}, ${2:int b})";
+
+  Results = completions(R"cpp(
+struct Base {
+  Base(int a, int b) {}
+};
+
+struct Derived : Base {
+  Derived() : Base^
+};
+  )cpp",
+/*IndexSymbols=*/{}, Options);
+  // Constructors from base classes are a kind of pattern that shouldn't end
+  // with $0.
+  EXPECT_THAT(Results.Completions,
+  Contains(AllOf(named("Base"),
+ snippetSuffix("(${1:int a}, ${2:int b})";
 }
 
 TEST(CompletionTest, WorksWithNullType) {
Index: clang-tools-extra/clangd/CodeCompletionStrings.h
===
--- clang-tools-extra/clangd/CodeCompletionStrings.h
+++ clang-tools-extra/clangd/CodeCompletionStrings.h
@@ -47,7 +47,8 @@
 void getSignature(const CodeCompletionString , std::string *Signature,
   std::string *Snippet,
   std::string *RequiredQualifiers = nullptr,
-  bool CompletingPattern = false);
+  bool CompletingPattern = false,
+  const CodeCompletionResult *CCR = nullptr);
 
 /// Assembles formatted documentation for a completion result. This includes
 /// documentation comments and other relevant information like annotations.
Index: clang-tools-extra/clangd/CodeCompletionStrings.cpp
===
--- clang-tools-extra/clangd/CodeCompletionStrings.cpp
+++ clang-tools-extra/clangd/CodeCompletionStrings.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "CodeCompletionStrings.h"
+#include "clang-c/Index.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/RawCommentList.h"
 #include "clang/Basic/SourceManager.h"
@@ -96,7 +97,7 @@
 
 void getSignature(const CodeCompletionString , std::string *Signature,
   std::string *Snippet, std::string *RequiredQualifiers,
-  bool CompletingPattern) {
+  bool CompletingPattern, const CodeCompletionResult *CCR) {
   // Placeholder with this index will be ${0:…} to mark final cursor position.
   // Usually we do not add $0, so the cursor is placed at end of completed text.
   unsigned CursorSnippetArg = std::numeric_limits::max();
@@ -111,6 +112,23 @@
   return C.Kind == CodeCompletionString::CK_Placeholder;
 });
   }
+  // If the result kind of CCR is `RK_Pattern`, it doesn't always mean we're
+  // completing a chunk of statements.  Constructors defined in base class, for
+  // example, are considered as a type of pattern, with the cursor type set to
+  // CXCursor_Constructor.
+  // We have to discriminate these cases manually in order to avoid providing
+  // incorrect placeholder `$0` which should have been a normal parameter.
+  bool ShouldPatchPlaceholder0 = CompletingPattern && [CCR] {
+if (!CCR)
+  return true;
+// The process of CCR construction employs `clang::getCursorKindForDecl` to
+// obtain cursor kind for Decls, otherwise CursorKind would be set by
+// constructor. Note that the default value is CXCursor_NotImplemented.
+if (CCR->CursorKind == CXCursorKind::CXCursor_Constructor ||
+   

[clang] 5e12002 - Revert "[clang][Interp] Support destructors"

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2023-03-05T13:18:13+01:00
New Revision: 5e12002c6cea7601073888c2281525131caa77e3

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

LOG: Revert "[clang][Interp] Support destructors"

This reverts commit 78e4237460bf58f3d6b75f275e0424f38e3b1d04.

This breaks the memory sanitizer builder:
https://lab.llvm.org/buildbot/#/builders/5/builds/31959

Added: 


Modified: 
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeExprGen.h
clang/lib/AST/Interp/ByteCodeStmtGen.cpp
clang/test/AST/Interp/cxx20.cpp

Removed: 




diff  --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp 
b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index 488d07e09d9b5..bc682c92c143f 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -26,10 +26,10 @@ namespace clang {
 namespace interp {
 
 /// Scope used to handle temporaries in toplevel variable declarations.
-template  class DeclScope final : public VariableScope 
{
+template  class DeclScope final : public LocalScope {
 public:
   DeclScope(ByteCodeExprGen *Ctx, const ValueDecl *VD)
-  : VariableScope(Ctx), Scope(Ctx->P, VD) {}
+  : LocalScope(Ctx), Scope(Ctx->P, VD) {}
 
   void addExtended(const Scope::Local ) override {
 return this->addLocal(Local);
@@ -1857,80 +1857,6 @@ void ByteCodeExprGen::emitCleanup() {
 C->emitDestruction();
 }
 
-/// When calling this, we have a pointer of the local-to-destroy
-/// on the stack.
-/// Emit destruction of record types (or arrays of record types).
-/// FIXME: Handle virtual destructors.
-template 
-bool ByteCodeExprGen::emitRecordDestruction(const Descriptor *Desc) {
-  assert(Desc);
-  assert(!Desc->isPrimitive());
-  assert(!Desc->isPrimitiveArray());
-
-  // Arrays.
-  if (Desc->isArray()) {
-const Descriptor *ElemDesc = Desc->ElemDesc;
-const Record *ElemRecord = ElemDesc->ElemRecord;
-assert(ElemRecord); // This is not a primitive array.
-
-if (const CXXDestructorDecl *Dtor = ElemRecord->getDestructor();
-Dtor && !Dtor->isTrivial()) {
-  for (ssize_t I = Desc->getNumElems() - 1; I >= 0; --I) {
-if (!this->emitConstUint64(I, SourceInfo{}))
-  return false;
-if (!this->emitArrayElemPtrUint64(SourceInfo{}))
-  return false;
-if (!this->emitRecordDestruction(Desc->ElemDesc))
-  return false;
-  }
-}
-return this->emitPopPtr(SourceInfo{});
-  }
-
-  const Record *R = Desc->ElemRecord;
-  assert(R);
-  // First, destroy all fields.
-  for (const Record::Field  : llvm::reverse(R->fields())) {
-const Descriptor *D = Field.Desc;
-if (!D->isPrimitive() && !D->isPrimitiveArray()) {
-  if (!this->emitDupPtr(SourceInfo{}))
-return false;
-  if (!this->emitGetPtrField(Field.Offset, SourceInfo{}))
-return false;
-  if (!this->emitRecordDestruction(D))
-return false;
-}
-  }
-
-  // FIXME: Unions need to be handled 
diff erently here. We don't want to
-  //   call the destructor of its members.
-
-  // Now emit the destructor and recurse into base classes.
-  if (const CXXDestructorDecl *Dtor = R->getDestructor();
-  Dtor && !Dtor->isTrivial()) {
-const Function *DtorFunc = getFunction(Dtor);
-if (DtorFunc && DtorFunc->isConstexpr()) {
-  assert(DtorFunc->hasThisPointer());
-  assert(DtorFunc->getNumParams() == 1);
-  if (!this->emitDupPtr(SourceInfo{}))
-return false;
-  if (!this->emitCall(DtorFunc, SourceInfo{}))
-return false;
-}
-  }
-
-  for (const Record::Base  : llvm::reverse(R->bases())) {
-if (!this->emitGetPtrBase(Base.Offset, SourceInfo{}))
-  return false;
-if (!this->emitRecordDestruction(Base.Desc))
-  return false;
-  }
-  // FIXME: Virtual bases.
-
-  // Remove the instance pointer.
-  return this->emitPopPtr(SourceInfo{});
-}
-
 namespace clang {
 namespace interp {
 

diff  --git a/clang/lib/AST/Interp/ByteCodeExprGen.h 
b/clang/lib/AST/Interp/ByteCodeExprGen.h
index 4d929278f29c8..231f39ff8bd6d 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.h
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -256,8 +256,6 @@ class ByteCodeExprGen : public 
ConstStmtVisitor, bool>,
 return FPO.getRoundingMode();
   }
 
-  bool emitRecordDestruction(const Descriptor *Desc);
-
 protected:
   /// Variable to storage mapping.
   llvm::DenseMap Locals;
@@ -335,20 +333,9 @@ template  class LocalScope : public 
VariableScope {
 this->Ctx->Descriptors[*Idx].emplace_back(Local);
   }
 
-  /// Emit destruction of the local variable. This includes
-  /// object destructors.
   void emitDestruction() override {
 if (!Idx)
   return;
-// Emit destructor calls for local variables of record
-// type with 

[PATCH] D144903: [X86] Drop single use check for freeze(undef) in LowerAVXCONCAT_VECTORS

[Manuel Brito via Phabricator via cfe-commits]

ManuelJBrito added a comment.

It seems the build failure was caused by a known crash  
https://github.com/llvm/llvm-project/issues/55263. I tried to find some 
workaround but unsuccessfully.

So I'm thinking I can drop the end-to-end tests for now and commit them when 
the crash is fixed and for now just rely on the separate frontend and backend 
tests for regressions.

Is this OK with you @RKSimon ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144903

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

[PATCH] D137070: [clang][Interp] Support destructors

[Timm Bäder 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 rG78e4237460bf: [clang][Interp] Support destructors (authored 
by tbaeder).

Changed prior to commit:
  https://reviews.llvm.org/D137070?vs=494660=502409#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137070

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  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
@@ -271,3 +271,250 @@
// ref-error {{must have constant destruction}} \
// ref-note {{in call to}}
 };
+
+namespace Destructors {
+
+  class Inc final {
+  public:
+int 
+constexpr Inc(int ) : I(I) {}
+constexpr ~Inc() {
+  I++;
+}
+  };
+
+  class Dec final {
+  public:
+int 
+constexpr Dec(int ) : I(I) {}
+constexpr ~Dec() {
+  I--;
+}
+  };
+
+
+
+  constexpr int m() {
+int i = 0;
+{
+  Inc f1(i);
+  Inc f2(i);
+  Inc f3(i);
+}
+return i;
+  }
+  static_assert(m() == 3, "");
+
+
+  constexpr int C() {
+int i = 0;
+
+while (i < 10) {
+  Inc inc(i);
+  continue;
+  Dec dec(i);
+}
+return i;
+  }
+  static_assert(C() == 10, "");
+
+
+  constexpr int D() {
+int i = 0;
+
+{
+  Inc i1(i);
+  {
+Inc i2(i);
+return i;
+  }
+}
+
+return i;
+  }
+  static_assert(D() == 0, "");
+
+  constexpr int E() {
+int i = 0;
+
+for(;;) {
+  Inc i1(i);
+  break;
+}
+return i;
+  }
+  static_assert(E() == 1, "");
+
+
+  /// FIXME: This should be rejected, since we call the destructor
+  ///   twice. However, GCC doesn't care either.
+  constexpr int ManualDtor() {
+int i = 0;
+{
+  Inc I(i); // ref-note {{destroying object 'I' whose lifetime has already ended}}
+  I.~Inc();
+}
+return i;
+  }
+  static_assert(ManualDtor() == 1, ""); // expected-error {{static assertion failed}} \
+// expected-note {{evaluates to '2 == 1'}} \
+// ref-error {{not an integral constant expression}} \
+// ref-note {{in call to 'ManualDtor()'}}
+
+  constexpr void doInc(int ) {
+Inc I(i);
+return;
+  }
+  constexpr int testInc() {
+int i = 0;
+doInc(i);
+return i;
+  }
+  static_assert(testInc() == 1, "");
+  constexpr void doInc2(int ) {
+Inc I(i);
+// No return statement.
+  }
+   constexpr int testInc2() {
+int i = 0;
+doInc2(i);
+return i;
+  }
+  static_assert(testInc2() == 1, "");
+
+
+  namespace DtorOrder {
+class A {
+  public:
+  int 
+  constexpr A(int ) : I(I) {}
+  constexpr ~A() {
+I = 1337;
+  }
+};
+
+class B : public A {
+  public:
+  constexpr B(int ) : A(I) {}
+  constexpr ~B() {
+I = 42;
+  }
+};
+
+constexpr int foo() {
+  int i = 0;
+  {
+B b(i);
+  }
+  return i;
+}
+
+static_assert(foo() == 1337);
+  }
+
+  class FieldDtor1 {
+  public:
+Inc I1;
+Inc I2;
+constexpr FieldDtor1(int ) : I1(I), I2(I){}
+  };
+
+  constexpr int foo2() {
+int i = 0;
+{
+  FieldDtor1 FD1(i);
+}
+return i;
+  }
+
+  static_assert(foo2() == 2);
+
+  class FieldDtor2 {
+  public:
+Inc Incs[3];
+constexpr FieldDtor2(int )  : Incs{Inc(I), Inc(I), Inc(I)} {}
+  };
+
+  constexpr int foo3() {
+int i = 0;
+{
+  FieldDtor2 FD2(i);
+}
+return i;
+  }
+
+  static_assert(foo3() == 3);
+
+  struct ArrD {
+int index;
+int *arr;
+int 
+constexpr ~ArrD() {
+  arr[p] = index;
+  ++p;
+}
+  };
+  constexpr bool ArrayOrder() {
+int order[3] = {0, 0, 0};
+int p = 0;
+{
+  ArrD ds[3] = {
+{1, order, p},
+{2, order, p},
+{3, order, p},
+  };
+  // ds will be destroyed.
+}
+return order[0] == 3 && order[1] == 2 && order[2] == 1;
+  }
+  static_assert(ArrayOrder());
+
+
+  // Static members aren't destroyed.
+  class Dec2 {
+  public:
+int A = 0;
+constexpr ~Dec2() {
+  A++;
+}
+  };
+  class Foo {
+  public:
+static constexpr Dec2 a;
+static Dec2 b;
+  };
+  static_assert(Foo::a.A == 0);
+  constexpr bool f() {
+Foo f;
+return true;
+  }
+  static_assert(Foo::a.A == 0);
+  static_assert(f());
+  static_assert(Foo::a.A == 0);
+
+
+  struct NotConstexpr {
+NotConstexpr() {}
+~NotConstexpr() {}
+  };
+
+  struct Outer {
+constexpr Outer() = default;
+constexpr ~Outer();
+
+

[clang] 78e4237 - [clang][Interp] Support destructors

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2023-03-05T10:02:42+01:00
New Revision: 78e4237460bf58f3d6b75f275e0424f38e3b1d04

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

LOG: [clang][Interp] Support destructors

Use the existing local variable cleanup infrastructure to implement
destruction.

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

Added: 


Modified: 
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeExprGen.h
clang/lib/AST/Interp/ByteCodeStmtGen.cpp
clang/test/AST/Interp/cxx20.cpp

Removed: 




diff  --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp 
b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index bc682c92c143f..488d07e09d9b5 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -26,10 +26,10 @@ namespace clang {
 namespace interp {
 
 /// Scope used to handle temporaries in toplevel variable declarations.
-template  class DeclScope final : public LocalScope {
+template  class DeclScope final : public VariableScope 
{
 public:
   DeclScope(ByteCodeExprGen *Ctx, const ValueDecl *VD)
-  : LocalScope(Ctx), Scope(Ctx->P, VD) {}
+  : VariableScope(Ctx), Scope(Ctx->P, VD) {}
 
   void addExtended(const Scope::Local ) override {
 return this->addLocal(Local);
@@ -1857,6 +1857,80 @@ void ByteCodeExprGen::emitCleanup() {
 C->emitDestruction();
 }
 
+/// When calling this, we have a pointer of the local-to-destroy
+/// on the stack.
+/// Emit destruction of record types (or arrays of record types).
+/// FIXME: Handle virtual destructors.
+template 
+bool ByteCodeExprGen::emitRecordDestruction(const Descriptor *Desc) {
+  assert(Desc);
+  assert(!Desc->isPrimitive());
+  assert(!Desc->isPrimitiveArray());
+
+  // Arrays.
+  if (Desc->isArray()) {
+const Descriptor *ElemDesc = Desc->ElemDesc;
+const Record *ElemRecord = ElemDesc->ElemRecord;
+assert(ElemRecord); // This is not a primitive array.
+
+if (const CXXDestructorDecl *Dtor = ElemRecord->getDestructor();
+Dtor && !Dtor->isTrivial()) {
+  for (ssize_t I = Desc->getNumElems() - 1; I >= 0; --I) {
+if (!this->emitConstUint64(I, SourceInfo{}))
+  return false;
+if (!this->emitArrayElemPtrUint64(SourceInfo{}))
+  return false;
+if (!this->emitRecordDestruction(Desc->ElemDesc))
+  return false;
+  }
+}
+return this->emitPopPtr(SourceInfo{});
+  }
+
+  const Record *R = Desc->ElemRecord;
+  assert(R);
+  // First, destroy all fields.
+  for (const Record::Field  : llvm::reverse(R->fields())) {
+const Descriptor *D = Field.Desc;
+if (!D->isPrimitive() && !D->isPrimitiveArray()) {
+  if (!this->emitDupPtr(SourceInfo{}))
+return false;
+  if (!this->emitGetPtrField(Field.Offset, SourceInfo{}))
+return false;
+  if (!this->emitRecordDestruction(D))
+return false;
+}
+  }
+
+  // FIXME: Unions need to be handled 
diff erently here. We don't want to
+  //   call the destructor of its members.
+
+  // Now emit the destructor and recurse into base classes.
+  if (const CXXDestructorDecl *Dtor = R->getDestructor();
+  Dtor && !Dtor->isTrivial()) {
+const Function *DtorFunc = getFunction(Dtor);
+if (DtorFunc && DtorFunc->isConstexpr()) {
+  assert(DtorFunc->hasThisPointer());
+  assert(DtorFunc->getNumParams() == 1);
+  if (!this->emitDupPtr(SourceInfo{}))
+return false;
+  if (!this->emitCall(DtorFunc, SourceInfo{}))
+return false;
+}
+  }
+
+  for (const Record::Base  : llvm::reverse(R->bases())) {
+if (!this->emitGetPtrBase(Base.Offset, SourceInfo{}))
+  return false;
+if (!this->emitRecordDestruction(Base.Desc))
+  return false;
+  }
+  // FIXME: Virtual bases.
+
+  // Remove the instance pointer.
+  return this->emitPopPtr(SourceInfo{});
+}
+
 namespace clang {
 namespace interp {
 

diff  --git a/clang/lib/AST/Interp/ByteCodeExprGen.h 
b/clang/lib/AST/Interp/ByteCodeExprGen.h
index 231f39ff8bd6d..4d929278f29c8 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.h
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -256,6 +256,8 @@ class ByteCodeExprGen : public 
ConstStmtVisitor, bool>,
 return FPO.getRoundingMode();
   }
 
+  bool emitRecordDestruction(const Descriptor *Desc);
+
 protected:
   /// Variable to storage mapping.
   llvm::DenseMap Locals;
@@ -333,9 +335,20 @@ template  class LocalScope : public 
VariableScope {
 this->Ctx->Descriptors[*Idx].emplace_back(Local);
   }
 
+  /// Emit destruction of the local variable. This includes
+  /// object destructors.
   void emitDestruction() override {
 if (!Idx)
   return;
+// Emit destructor calls for local variables of record
+// type with a destructor.
+for 

[clang] 33ba940 - [clang][Interp][NFCI] Support more expression in initializers

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2023-03-05T09:57:52+01:00
New Revision: 33ba940de0401c91b4d7e12d98972e6ba0f0f662

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

LOG: [clang][Interp][NFCI] Support more expression in initializers

Added: 


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

Removed: 




diff  --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp 
b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index 1740c5dd939e3..bc682c92c143f 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -1337,6 +1337,8 @@ bool 
ByteCodeExprGen::visitArrayInitializer(const Expr *Initializer) {
 return true;
   } else if (const auto *CLE = dyn_cast(Initializer)) {
 return visitInitializer(CLE->getInitializer());
+  } else if (const auto *EWC = dyn_cast(Initializer)) {
+return visitInitializer(EWC->getSubExpr());
   }
 
   assert(false && "Unknown expression for array initialization");
@@ -1412,6 +1414,8 @@ bool 
ByteCodeExprGen::visitRecordInitializer(const Expr *Initializer) {
 return this->visitInitializer(DIE->getExpr());
   } else if (const auto *CE = dyn_cast(Initializer)) {
 return this->visitInitializer(CE->getSubExpr());
+  } else if (const auto *CE = dyn_cast(Initializer)) {
+return this->visitInitializer(CE->getSubExpr());
   }
 
   return false;



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