[clang] 49d3118 - [clang-format] Missing space after cast in a macro

[via cfe-commits]

Author: mydeveloperday
Date: 2022-01-06T08:07:03Z
New Revision: 49d311874edc928831ccaddd621801a4dbee580d

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

LOG: [clang-format] Missing space after cast in a macro

https://github.com/llvm/llvm-project/issues/52979

Though SpaceAfterCStyleCast is set to true, clang-format 13 does not add a 
space after (void *) here:

```
```

This patch addresses that

Fixes: #52979

Reviewed By: curdeius, HazardyKnusperkeks, owenpan

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

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index ec9bfdb0b2a76..5241685630a5d 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -1883,9 +1883,10 @@ class AnnotatingParser {
 
 FormatToken *LeftOfParens = Tok.MatchingParen->getPreviousNonComment();
 if (LeftOfParens) {
-  // If there is a closing parenthesis left of the current parentheses,
-  // look past it as these might be chained casts.
-  if (LeftOfParens->is(tok::r_paren)) {
+  // If there is a closing parenthesis left of the current
+  // parentheses, look past it as these might be chained casts.
+  if (LeftOfParens->is(tok::r_paren) &&
+  LeftOfParens->isNot(TT_CastRParen)) {
 if (!LeftOfParens->MatchingParen ||
 !LeftOfParens->MatchingParen->Previous)
   return false;

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index d90c3d3a291f6..85ce3171bbc89 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -10150,6 +10150,15 @@ TEST_F(FormatTest, FormatsCasts) {
"(aa *)(aa +\n"
"   bb);");
 
+  verifyFormat("#define CONF_BOOL(x) (bool *)(void *)(x)");
+  verifyFormat("#define CONF_BOOL(x) (bool *)(x)");
+  verifyFormat("#define CONF_BOOL(x) (bool)(x)");
+  verifyFormat("bool *y = (bool *)(void *)(x);");
+  verifyFormat("#define CONF_BOOL(x) (bool *)(void *)(int)(x)");
+  verifyFormat("bool *y = (bool *)(void *)(int)(x);");
+  verifyFormat("#define CONF_BOOL(x) (bool *)(void *)(int)foo(x)");
+  verifyFormat("bool *y = (bool *)(void *)(int)foo(x);");
+
   // These are not casts.
   verifyFormat("void f(int *) {}");
   verifyFormat("f(foo)->b;");
@@ -14661,6 +14670,11 @@ TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
"  break;\n"
"}",
Spaces);
+  verifyFormat("#define CONF_BOOL(x) ( bool * ) ( void * ) (x)", Spaces);
+  verifyFormat("#define CONF_BOOL(x) ( bool * ) (x)", Spaces);
+  verifyFormat("#define CONF_BOOL(x) ( bool ) (x)", Spaces);
+  verifyFormat("bool *y = ( bool * ) ( void * ) (x);", Spaces);
+  verifyFormat("bool *y = ( bool * ) (x);", Spaces);
 
   // Run subset of tests again with:
   Spaces.SpacesInCStyleCastParentheses = false;
@@ -14680,6 +14694,11 @@ TEST_F(FormatTest, ConfigurableSpacesInParentheses) {
   verifyFormat("size_t idx = (a->foo)(a - 1);", Spaces);
   verifyFormat("size_t idx = (*foo)(a - 1);", Spaces);
   verifyFormat("size_t idx = (*(foo))(a - 1);", Spaces);
+  verifyFormat("#define CONF_BOOL(x) (bool *) (void *) (x)", Spaces);
+  verifyFormat("#define CONF_BOOL(x) (bool *) (void *) (int) (x)", Spaces);
+  verifyFormat("bool *y = (bool *) (void *) (x);", Spaces);
+  verifyFormat("bool *y = (bool *) (void *) (int) (x);", Spaces);
+  verifyFormat("bool *y = (bool *) (void *) (int) foo(x);", Spaces);
   Spaces.ColumnLimit = 80;
   Spaces.IndentWidth = 4;
   Spaces.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;



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

[PATCH] D116592: [clang-format] Missing space after cast in a macro

[MyDeveloperDay 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 rG49d311874edc: [clang-format] Missing space after cast in a 
macro (authored by MyDeveloperDay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116592

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10150,6 +10150,15 @@
"(aa *)(aa +\n"
"   bb);");
 
+  verifyFormat("#define CONF_BOOL(x) (bool *)(void *)(x)");
+  verifyFormat("#define CONF_BOOL(x) (bool *)(x)");
+  verifyFormat("#define CONF_BOOL(x) (bool)(x)");
+  verifyFormat("bool *y = (bool *)(void *)(x);");
+  verifyFormat("#define CONF_BOOL(x) (bool *)(void *)(int)(x)");
+  verifyFormat("bool *y = (bool *)(void *)(int)(x);");
+  verifyFormat("#define CONF_BOOL(x) (bool *)(void *)(int)foo(x)");
+  verifyFormat("bool *y = (bool *)(void *)(int)foo(x);");
+
   // These are not casts.
   verifyFormat("void f(int *) {}");
   verifyFormat("f(foo)->b;");
@@ -14661,6 +14670,11 @@
"  break;\n"
"}",
Spaces);
+  verifyFormat("#define CONF_BOOL(x) ( bool * ) ( void * ) (x)", Spaces);
+  verifyFormat("#define CONF_BOOL(x) ( bool * ) (x)", Spaces);
+  verifyFormat("#define CONF_BOOL(x) ( bool ) (x)", Spaces);
+  verifyFormat("bool *y = ( bool * ) ( void * ) (x);", Spaces);
+  verifyFormat("bool *y = ( bool * ) (x);", Spaces);
 
   // Run subset of tests again with:
   Spaces.SpacesInCStyleCastParentheses = false;
@@ -14680,6 +14694,11 @@
   verifyFormat("size_t idx = (a->foo)(a - 1);", Spaces);
   verifyFormat("size_t idx = (*foo)(a - 1);", Spaces);
   verifyFormat("size_t idx = (*(foo))(a - 1);", Spaces);
+  verifyFormat("#define CONF_BOOL(x) (bool *) (void *) (x)", Spaces);
+  verifyFormat("#define CONF_BOOL(x) (bool *) (void *) (int) (x)", Spaces);
+  verifyFormat("bool *y = (bool *) (void *) (x);", Spaces);
+  verifyFormat("bool *y = (bool *) (void *) (int) (x);", Spaces);
+  verifyFormat("bool *y = (bool *) (void *) (int) foo(x);", Spaces);
   Spaces.ColumnLimit = 80;
   Spaces.IndentWidth = 4;
   Spaces.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1883,9 +1883,10 @@
 
 FormatToken *LeftOfParens = Tok.MatchingParen->getPreviousNonComment();
 if (LeftOfParens) {
-  // If there is a closing parenthesis left of the current parentheses,
-  // look past it as these might be chained casts.
-  if (LeftOfParens->is(tok::r_paren)) {
+  // If there is a closing parenthesis left of the current
+  // parentheses, look past it as these might be chained casts.
+  if (LeftOfParens->is(tok::r_paren) &&
+  LeftOfParens->isNot(TT_CastRParen)) {
 if (!LeftOfParens->MatchingParen ||
 !LeftOfParens->MatchingParen->Previous)
   return false;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10150,6 +10150,15 @@
"(aa *)(aa +\n"
"   bb);");
 
+  verifyFormat("#define CONF_BOOL(x) (bool *)(void *)(x)");
+  verifyFormat("#define CONF_BOOL(x) (bool *)(x)");
+  verifyFormat("#define CONF_BOOL(x) (bool)(x)");
+  verifyFormat("bool *y = (bool *)(void *)(x);");
+  verifyFormat("#define CONF_BOOL(x) (bool *)(void *)(int)(x)");
+  verifyFormat("bool *y = (bool *)(void *)(int)(x);");
+  verifyFormat("#define CONF_BOOL(x) (bool *)(void *)(int)foo(x)");
+  verifyFormat("bool *y = (bool *)(void *)(int)foo(x);");
+
   // These are not casts.
   verifyFormat("void f(int *) {}");
   verifyFormat("f(foo)->b;");
@@ -14661,6 +14670,11 @@
"  break;\n"
"}",
Spaces);
+  verifyFormat("#define CONF_BOOL(x) ( bool * ) ( void * ) (x)", Spaces);
+  verifyFormat("#define CONF_BOOL(x) ( bool * ) (x)", Spaces);
+  verifyFormat("#define CONF_BOOL(x) ( bool ) (x)", Spaces);
+  verifyFormat("bool *y = ( bool * ) ( void * ) (x);", Spaces);
+  verifyFormat("bool *y = ( bool * ) (x);", Spaces);
 
   // Run subset of tests again with:
   Spaces.SpacesInCStyleCastParentheses = false;
@@ -14680,6 +14694,11 @@
   verifyFormat("size_t idx = (a->foo)(a - 1);", Spaces);
   verifyFormat("size

[PATCH] D114601: Read path to CUDA from env. variable CUDA_PATH on Windows

[Mojca Miklavec via Phabricator via cfe-commits]

mojca added a comment.

In D114601#3223155 , @tra wrote:

> Ping. @mojca, do you need help landing the patch?

Yes, please. I don't have commit access yet.
You can attribute it to mojca at macports.org, for example.

We also need a fix for unit tests on the CI in order to find the files, but I 
cannot help with that as I don't know how that works.


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

https://reviews.llvm.org/D114601

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

[PATCH] D116726: [clang-format] Fix a crash (assertion) in qualifier alignment when matching template closer is null

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: owenpan, curdeius, HazardyKnusperkeks, JohelEGP.
MyDeveloperDay added projects: clang, clang-format.
MyDeveloperDay requested review of this revision.

https://github.com/llvm/llvm-project/issues/53008

  template  using A = quantity /**/, 1>;

the presence of the comment between identifier and template opener seems to be 
causing the qualifier alignment to fail


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116726

Files:
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/unittests/Format/QualifierFixerTest.cpp


Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -818,5 +818,18 @@
   EXPECT_EQ(ReplacementCount, 0);
 }
 
+TEST_F(QualifierFixerTest, QualifierTemplates) {
+
+  FormatStyle Style = getLLVMStyle();
+  Style.QualifierAlignment = FormatStyle::QAS_Custom;
+  Style.QualifierOrder = {"static", "const", "type"};
+
+  ReplacementCount = 0;
+  EXPECT_EQ(ReplacementCount, 0);
+  verifyFormat("template  using A = quantity /**/, 1>;",
+   Style);
+  EXPECT_EQ(ReplacementCount, 0);
+}
+
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/QualifierAlignmentFixer.cpp
===
--- clang/lib/Format/QualifierAlignmentFixer.cpp
+++ clang/lib/Format/QualifierAlignmentFixer.cpp
@@ -334,7 +334,8 @@
 Next->Previous->startsSequence(tok::identifier, TT_TemplateOpener)) {
   // Read from to the end of the TemplateOpener to
   // TemplateCloser const ArrayRef a; const ArrayRef &a;
-  assert(Next->MatchingParen && "Missing template closer");
+  if (!Next->MatchingParen)
+return Next;
   Next = Next->MatchingParen->Next;
 
   // Move to the end of any template class members e.g.


Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -818,5 +818,18 @@
   EXPECT_EQ(ReplacementCount, 0);
 }
 
+TEST_F(QualifierFixerTest, QualifierTemplates) {
+
+  FormatStyle Style = getLLVMStyle();
+  Style.QualifierAlignment = FormatStyle::QAS_Custom;
+  Style.QualifierOrder = {"static", "const", "type"};
+
+  ReplacementCount = 0;
+  EXPECT_EQ(ReplacementCount, 0);
+  verifyFormat("template  using A = quantity /**/, 1>;",
+   Style);
+  EXPECT_EQ(ReplacementCount, 0);
+}
+
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/QualifierAlignmentFixer.cpp
===
--- clang/lib/Format/QualifierAlignmentFixer.cpp
+++ clang/lib/Format/QualifierAlignmentFixer.cpp
@@ -334,7 +334,8 @@
 Next->Previous->startsSequence(tok::identifier, TT_TemplateOpener)) {
   // Read from to the end of the TemplateOpener to
   // TemplateCloser const ArrayRef a; const ArrayRef &a;
-  assert(Next->MatchingParen && "Missing template closer");
+  if (!Next->MatchingParen)
+return Next;
   Next = Next->MatchingParen->Next;
 
   // Move to the end of any template class members e.g.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114601: Read path to CUDA from env. variable CUDA_PATH on Windows

[Mojca Miklavec via Phabricator via cfe-commits]

mojca added a comment.

Also, I would like to get to do some further "baby steps" towards better 
support of CUDA on Windows in particular, but I would need some guidelines.
I requested a special channel that would allow a bit of discussion 
https://discord.com/channels/636084430946959380/.
What would be the best way to shortly discuss the options?


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

https://reviews.llvm.org/D114601

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

[clang] e8b98a5 - [CodeGen] Emit elementtype attributes for indirect inline asm constraints

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2022-01-06T09:29:22+01:00
New Revision: e8b98a5216dbfdaa31f7016955f9586cef94a626

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

LOG: [CodeGen] Emit elementtype attributes for indirect inline asm constraints

This implements the clang side of D116531. The elementtype
attribute is added for all indirect constraints (*) and tests are
updated accordingly.

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

Added: 


Modified: 
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/CodeGen/CGObjCMac.cpp
clang/lib/CodeGen/CGStmt.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/test/CodeGen/RISCV/riscv-inline-asm.c
clang/test/CodeGen/SystemZ/systemz-inline-asm.c
clang/test/CodeGen/aarch64-inline-asm.c
clang/test/CodeGen/asm-inout.c
clang/test/CodeGen/asm.c
clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond-64bit-only.c
clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond.c
clang/test/CodeGen/inline-asm-x86-flag-output.c
clang/test/CodeGen/matrix-type.c
clang/test/CodeGen/mips-constraints-mem.c
clang/test/CodeGen/mips-inline-asm-modifiers.c
clang/test/CodeGen/mips-inline-asm.c
clang/test/CodeGen/mozilla-ms-inline-asm.c
clang/test/CodeGen/ms-inline-asm-64.c
clang/test/CodeGen/ms-inline-asm-static-variable.c
clang/test/CodeGen/ms-inline-asm.c
clang/test/CodeGen/ms-inline-asm.cpp
clang/test/CodeGen/ms-intrinsics.c
clang/test/CodeGen/mult-alt-generic.c
clang/test/CodeGen/mult-alt-x86.c
clang/test/CodeGen/ppc64-inline-asm.c
clang/test/CodeGenCXX/ms-inline-asm-fields.cpp
clang/test/CodeGenObjC/exceptions.m
clang/test/CodeGenObjC/synchronized.m

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index c1541ff0c846b..50f59a2abab85 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -1060,7 +1060,10 @@ static llvm::Value 
*emitPPCLoadReserveIntrinsic(CodeGenFunction &CGF,
 
   llvm::InlineAsm *IA =
   llvm::InlineAsm::get(FTy, Asm, Constraints, /*hasSideEffects=*/true);
-  return CGF.Builder.CreateCall(IA, {Addr});
+  llvm::CallInst *CI = CGF.Builder.CreateCall(IA, {Addr});
+  CI->addParamAttr(
+  0, Attribute::get(CGF.getLLVMContext(), Attribute::ElementType, 
RetType));
+  return CI;
 }
 
 namespace {

diff  --git a/clang/lib/CodeGen/CGObjCMac.cpp b/clang/lib/CodeGen/CGObjCMac.cpp
index 425d1a793439a..d769574c1f5ef 100644
--- a/clang/lib/CodeGen/CGObjCMac.cpp
+++ b/clang/lib/CodeGen/CGObjCMac.cpp
@@ -4370,7 +4370,11 @@ FragileHazards::FragileHazards(CodeGenFunction &CGF) : 
CGF(CGF) {
 void FragileHazards::emitWriteHazard() {
   if (Locals.empty()) return;
 
-  CGF.EmitNounwindRuntimeCall(WriteHazard, Locals);
+  llvm::CallInst *Call = CGF.EmitNounwindRuntimeCall(WriteHazard, Locals);
+  for (auto Pair : llvm::enumerate(Locals))
+Call->addParamAttr(Pair.index(), llvm::Attribute::get(
+CGF.getLLVMContext(), llvm::Attribute::ElementType,
+cast(Pair.value())->getAllocatedType()));
 }
 
 void FragileHazards::emitReadHazard(CGBuilderTy &Builder) {
@@ -4378,6 +4382,10 @@ void FragileHazards::emitReadHazard(CGBuilderTy 
&Builder) {
   llvm::CallInst *call = Builder.CreateCall(ReadHazard, Locals);
   call->setDoesNotThrow();
   call->setCallingConv(CGF.getRuntimeCC());
+  for (auto Pair : llvm::enumerate(Locals))
+call->addParamAttr(Pair.index(), llvm::Attribute::get(
+Builder.getContext(), llvm::Attribute::ElementType,
+cast(Pair.value())->getAllocatedType()));
 }
 
 /// Emit read hazards in all the protected blocks, i.e. all the blocks

diff  --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index ef0068cd3b0c9..feff8a2c178a3 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -2109,42 +2109,35 @@ AddVariableConstraints(const std::string &Constraint, 
const Expr &AsmExpr,
   return (EarlyClobber ? "&{" : "{") + Register.str() + "}";
 }
 
-llvm::Value*
-CodeGenFunction::EmitAsmInputLValue(const TargetInfo::ConstraintInfo &Info,
-LValue InputValue, QualType InputType,
-std::string &ConstraintStr,
-SourceLocation Loc) {
-  llvm::Value *Arg;
+std::pair CodeGenFunction::EmitAsmInputLValue(
+const TargetInfo::ConstraintInfo &Info, LValue InputValue,
+QualType InputType, std::string &ConstraintStr, SourceLocation Loc) {
   if (Info.allowsRegister() || !Info.allowsMemory()) {
-if (CodeGenFunction::hasScalarEvaluationKind(InputType)) {
-  Arg = EmitLoadOfLValue(InputValue, Loc).getScalarVal();
-} else {
-  llvm::Type *Ty = ConvertType(InputType);
-  uint64_t Si

[PATCH] D116666: [CodeGen] Emit elementtype attributes for indirect inline asm constraints

[Nikita Popov 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 rGe8b98a5216db: [CodeGen] Emit elementtype attributes for 
indirect inline asm constraints (authored by nikic).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D11?vs=397585&id=397805#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D11

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/RISCV/riscv-inline-asm.c
  clang/test/CodeGen/SystemZ/systemz-inline-asm.c
  clang/test/CodeGen/aarch64-inline-asm.c
  clang/test/CodeGen/asm-inout.c
  clang/test/CodeGen/asm.c
  clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond-64bit-only.c
  clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond.c
  clang/test/CodeGen/inline-asm-x86-flag-output.c
  clang/test/CodeGen/matrix-type.c
  clang/test/CodeGen/mips-constraints-mem.c
  clang/test/CodeGen/mips-inline-asm-modifiers.c
  clang/test/CodeGen/mips-inline-asm.c
  clang/test/CodeGen/mozilla-ms-inline-asm.c
  clang/test/CodeGen/ms-inline-asm-64.c
  clang/test/CodeGen/ms-inline-asm-static-variable.c
  clang/test/CodeGen/ms-inline-asm.c
  clang/test/CodeGen/ms-inline-asm.cpp
  clang/test/CodeGen/ms-intrinsics.c
  clang/test/CodeGen/mult-alt-generic.c
  clang/test/CodeGen/mult-alt-x86.c
  clang/test/CodeGen/ppc64-inline-asm.c
  clang/test/CodeGenCXX/ms-inline-asm-fields.cpp
  clang/test/CodeGenObjC/exceptions.m
  clang/test/CodeGenObjC/synchronized.m

Index: clang/test/CodeGenObjC/synchronized.m
===
--- clang/test/CodeGenObjC/synchronized.m
+++ clang/test/CodeGenObjC/synchronized.m
@@ -32,7 +32,7 @@
   // CHECK:  call i32 @_setjmp
   @synchronized(a) {
 // This is unreachable, but the optimizers can't know that.
-// CHECK: call void asm sideeffect "", "=*m,=*m,=*m"(i8** nonnull [[A]], i8** nonnull [[SYNC]]
+// CHECK: call void asm sideeffect "", "=*m,=*m,=*m"(i8** nonnull elementtype(i8*) [[A]], i8** nonnull elementtype(i8*) [[SYNC]]
 // CHECK: call i32 @objc_sync_exit
 // CHECK: call i8* @objc_exception_extract
 // CHECK: call void @objc_exception_throw
Index: clang/test/CodeGenObjC/exceptions.m
===
--- clang/test/CodeGenObjC/exceptions.m
+++ clang/test/CodeGenObjC/exceptions.m
@@ -54,14 +54,14 @@
   // CHECK-NEXT:   br i1 [[CAUGHT]]
   @try {
 // Landing pad.  Note that we elide the re-enter.
-// CHECK:  call void asm sideeffect "", "=*m,=*m"(i32* nonnull [[X]]
+// CHECK:  call void asm sideeffect "", "=*m,=*m"(i32* nonnull elementtype(i32) [[X]]
 // CHECK-NEXT: call i8* @objc_exception_extract
 // CHECK-NEXT: [[T1:%.*]] = load i32, i32* [[X]]
 // CHECK-NEXT: [[T2:%.*]] = add nsw i32 [[T1]], -1
 
 // CHECK: store i32 6, i32* [[X]]
 x++;
-// CHECK-NEXT: call void asm sideeffect "", "*m,*m"(i32* nonnull [[X]]
+// CHECK-NEXT: call void asm sideeffect "", "*m,*m"(i32* nonnull elementtype(i32) [[X]]
 // CHECK-NEXT: call void @foo()
 // CHECK-NEXT: call void @objc_exception_try_exit
 // CHECK-NEXT: [[T:%.*]] = load i32, i32* [[X]]
Index: clang/test/CodeGenCXX/ms-inline-asm-fields.cpp
===
--- clang/test/CodeGenCXX/ms-inline-asm-fields.cpp
+++ clang/test/CodeGenCXX/ms-inline-asm-fields.cpp
@@ -24,7 +24,7 @@
 
 extern "C" int test_namespace_global() {
 // CHECK: define{{.*}} i32 @test_namespace_global()
-// CHECK: call i32 asm sideeffect inteldialect "mov eax, $1", "{{.*}}"(i32* getelementptr inbounds (%struct.A, %struct.A* @_ZN4asdf8a_globalE, i32 0, i32 2, i32 1))
+// CHECK: call i32 asm sideeffect inteldialect "mov eax, $1", "{{.*}}"(i32* elementtype(i32) getelementptr inbounds (%struct.A, %struct.A* @_ZN4asdf8a_globalE, i32 0, i32 2, i32 1))
 // CHECK: ret i32
   __asm mov eax, asdf::a_global.a3.b2
 }
@@ -53,4 +53,4 @@
 // CHECK: %[[P:.*]] = alloca %"struct.make_storage_type::type", align 4
 // CHECK: %[[B:.*]] = getelementptr inbounds %"struct.make_storage_type::type", %"struct.make_storage_type::type"* %[[P]], i32 0, i32 0
 // CHECK: %[[X:.*]] = getelementptr inbounds %"struct.make_storage_type::type::B", %"struct.make_storage_type::type::B"* %[[B]], i32 0, i32 1
-// CHECK: call void asm sideeffect inteldialect "mov edx, dword ptr $0", "*m,~{edx},~{dirflag},~{fpsr},~{flags}"(i32* %[[X]])
+// CHECK: call void asm sideeffect inteldialect "mov edx, dword ptr $0", "*m,~{edx},~{dirflag},~{fpsr},~{flags}"(i32* elementtype(i32) %[[X]])
Index: clang/test/CodeGen/ppc64-inline-asm.c
===
--- clang/test/CodeGe

[PATCH] D116351: Update Bug report URL to Github Issues

[James Henderson via Phabricator via cfe-commits]

jhenderson accepted this revision.
jhenderson added a comment.

LGTM, from my point of view.


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

https://reviews.llvm.org/D116351

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

[PATCH] D116726: [clang-format] Fix a crash (assertion) in qualifier alignment when matching template closer is null

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 397806.
MyDeveloperDay added a comment.

The cause is because Next is a comment, not a template opener, handle that case.


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

https://reviews.llvm.org/D116726

Files:
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/unittests/Format/QualifierFixerTest.cpp


Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -818,5 +818,19 @@
   EXPECT_EQ(ReplacementCount, 0);
 }
 
+TEST_F(QualifierFixerTest, QualifierTemplates) {
+
+  FormatStyle Style = getLLVMStyle();
+  Style.QualifierAlignment = FormatStyle::QAS_Custom;
+  Style.QualifierOrder = {"static", "const", "type"};
+
+  ReplacementCount = 0;
+  EXPECT_EQ(ReplacementCount, 0);
+  verifyFormat("template  using A = quantity, 1>;", Style);
+  verifyFormat("template  using A = quantity /**/, 1>;",
+   Style);
+  EXPECT_EQ(ReplacementCount, 0);
+}
+
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/QualifierAlignmentFixer.cpp
===
--- clang/lib/Format/QualifierAlignmentFixer.cpp
+++ clang/lib/Format/QualifierAlignmentFixer.cpp
@@ -334,6 +334,8 @@
 Next->Previous->startsSequence(tok::identifier, TT_TemplateOpener)) {
   // Read from to the end of the TemplateOpener to
   // TemplateCloser const ArrayRef a; const ArrayRef &a;
+  if (Next->is(TT_BlockComment) && Next->Next)
+Next = Next->Next;
   assert(Next->MatchingParen && "Missing template closer");
   Next = Next->MatchingParen->Next;
 


Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -818,5 +818,19 @@
   EXPECT_EQ(ReplacementCount, 0);
 }
 
+TEST_F(QualifierFixerTest, QualifierTemplates) {
+
+  FormatStyle Style = getLLVMStyle();
+  Style.QualifierAlignment = FormatStyle::QAS_Custom;
+  Style.QualifierOrder = {"static", "const", "type"};
+
+  ReplacementCount = 0;
+  EXPECT_EQ(ReplacementCount, 0);
+  verifyFormat("template  using A = quantity, 1>;", Style);
+  verifyFormat("template  using A = quantity /**/, 1>;",
+   Style);
+  EXPECT_EQ(ReplacementCount, 0);
+}
+
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/QualifierAlignmentFixer.cpp
===
--- clang/lib/Format/QualifierAlignmentFixer.cpp
+++ clang/lib/Format/QualifierAlignmentFixer.cpp
@@ -334,6 +334,8 @@
 Next->Previous->startsSequence(tok::identifier, TT_TemplateOpener)) {
   // Read from to the end of the TemplateOpener to
   // TemplateCloser const ArrayRef a; const ArrayRef &a;
+  if (Next->is(TT_BlockComment) && Next->Next)
+Next = Next->Next;
   assert(Next->MatchingParen && "Missing template closer");
   Next = Next->MatchingParen->Next;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116726: [clang-format] Fix a crash (assertion) in qualifier alignment when matching template closer is null

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/unittests/Format/QualifierFixerTest.cpp:830
+  verifyFormat("template  using A = quantity, 1>;", Style);
+  verifyFormat("template  using A = quantity /**/, 1>;",
+   Style);

Could you test with a line comment too?



Comment at: clang/unittests/Format/QualifierFixerTest.cpp:830
+  verifyFormat("template  using A = quantity, 1>;", Style);
+  verifyFormat("template  using A = quantity /**/, 1>;",
+   Style);

curdeius wrote:
> Could you test with a line comment too?
And what about multiple comments? Maybe we need a while loop instead of if?


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

https://reviews.llvm.org/D116726

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

[PATCH] D116726: [clang-format] Fix a crash (assertion) in qualifier alignment when matching template closer is null

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/unittests/Format/QualifierFixerTest.cpp:830
+  verifyFormat("template  using A = quantity, 1>;", Style);
+  verifyFormat("template  using A = quantity /**/, 1>;",
+   Style);

curdeius wrote:
> curdeius wrote:
> > Could you test with a line comment too?
> And what about multiple comments? Maybe we need a while loop instead of if?
And test with non-empty comments too.


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

https://reviews.llvm.org/D116726

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

[PATCH] D116701: Respect -fsanitize-memory-param-retval flag.

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

>> Implementation of flag introduced in https://reviews.llvm.org/D116633

It's not implementation, the goal of -sanitize-memory-param-retval to setup 
Msan pass with proper parameters
This part usability improvement to avoid one flag, which I slightly like, but 
@eugenis don't.
I don't think this part part is blocking us, we can discuss it later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116701

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

[PATCH] D116633: Add -fsanitize-address-param-retval to clang.

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

we probably should to move clang parts from D116634 
 into this patch?




Comment at: clang/include/clang/Basic/CodeGenOptions.def:234
  ///< destructors are emitted.
+CODEGENOPT(SanitizeMemoryParamRetval, 1, 0) /// eugenis wrote:
> > vitalybuka wrote:
> > > vitalybuka wrote:
> > > > @eugenis Would be better to force CodeGenOpts.EnableNoundefAttrs if 
> > > > SanitizeMemoryParamRetval is provided?
> > > > @eugenis Would be better to force CodeGenOpts.EnableNoundefAttrs if 
> > > > SanitizeMemoryParamRetval is provided?
> > > 
> > > To clarify, checking SanitizeMemoryParamRetval here as-is is LGTM, unless 
> > > @eugenis or someone else thinks EnableNoundefAttrs reset is better.
> > I don't think this is right at all. An argument being noundef has nothing 
> > to do at all with memory sanitization. It affects optimization, too. 
> > SanitizeMemoryParamRetval should only affect what MemorySanitizerPass does 
> > with noundef attributes.
> Is the problem the form of the new code?
>   - instead of introducing a single new flag to flip 4 others, should there 
> be 1 flag to pick up the CodeGen changes, and another for the Sanitizer?  (Is 
> 2 flags better than 4?)
>   - another option is have the changes propagate in the other direction: use 
> the flag (-fsanitize-memory-param-retval), to passes along '-mllvm 
> -enable_noundef_analysis=1' to CodeGen (via SanitizerArgs.addArgs).
> 
> OR is there a problem forcing EnableNoundefAttrs based on an "unrelated" flag?
>   - then leave existing code, just don't do anything fancy to change 
> EnableNoundefAttrs.
> I don't think this is right at all. An argument being noundef has nothing to 
> do at all with memory sanitization. It affects optimization, too. 
> SanitizeMemoryParamRetval should only affect what MemorySanitizerPass does 
> with noundef attributes.

What is the point of running SanitizeMemoryParamRetval==1 && 
EnableNoundefAttrs==0?
Are we going to show warning or error and ask to set thee redundant options? 
Then why not just automate this for the user?
Also please note that HasStrictReturn already check SanitizerKind::Memory and 
adjust NoUndef attributes.

On the other hand, if EnableNoundefAttrs is going to be true by default in 
future, this conversation is not important, we can have two flags.




Comment at: clang/lib/CodeGen/CGCall.cpp:2384
+if ((CodeGenOpts.EnableNoundefAttrs ||
+ CodeGenOpts.SanitizeMemoryParamRetval) &&
+ArgNoUndef)

vitalybuka wrote:
> kda wrote:
> > eugenis wrote:
> > > vitalybuka wrote:
> > > > vitalybuka wrote:
> > > > > @eugenis Would be better to force CodeGenOpts.EnableNoundefAttrs if 
> > > > > SanitizeMemoryParamRetval is provided?
> > > > > @eugenis Would be better to force CodeGenOpts.EnableNoundefAttrs if 
> > > > > SanitizeMemoryParamRetval is provided?
> > > > 
> > > > To clarify, checking SanitizeMemoryParamRetval here as-is is LGTM, 
> > > > unless @eugenis or someone else thinks EnableNoundefAttrs reset is 
> > > > better.
> > > I don't think this is right at all. An argument being noundef has nothing 
> > > to do at all with memory sanitization. It affects optimization, too. 
> > > SanitizeMemoryParamRetval should only affect what MemorySanitizerPass 
> > > does with noundef attributes.
> > Is the problem the form of the new code?
> >   - instead of introducing a single new flag to flip 4 others, should there 
> > be 1 flag to pick up the CodeGen changes, and another for the Sanitizer?  
> > (Is 2 flags better than 4?)
> >   - another option is have the changes propagate in the other direction: 
> > use the flag (-fsanitize-memory-param-retval), to passes along '-mllvm 
> > -enable_noundef_analysis=1' to CodeGen (via SanitizerArgs.addArgs).
> > 
> > OR is there a problem forcing EnableNoundefAttrs based on an "unrelated" 
> > flag?
> >   - then leave existing code, just don't do anything fancy to change 
> > EnableNoundefAttrs.
> > I don't think this is right at all. An argument being noundef has nothing 
> > to do at all with memory sanitization. It affects optimization, too. 
> > SanitizeMemoryParamRetval should only affect what MemorySanitizerPass does 
> > with noundef attributes.
> 
> What is the point of running SanitizeMemoryParamRetval==1 && 
> EnableNoundefAttrs==0?
> Are we going to show warning or error and ask to set thee redundant options? 
> Then why not just automate this for the user?
> Also please note that HasStrictReturn already check SanitizerKind::Memory and 
> adjust NoUndef attributes.
> 
> On the other hand, if EnableNoundefAttrs is going to be true by default in 
> future, this conversation is not important, we can have two flags.
> 
> Is the problem the form of the new code?
>   - instead of introducing a single new flag to flip 4 others, should there 
> be 1 flag to pick up the CodeGen changes, and another for t

[PATCH] D112913: Misleading bidirectional detection

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 397813.
serge-sans-paille added a comment.

Fix some parts of the bidi algorithm, and add extra test cases


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

https://reviews.llvm.org/D112913

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp
  clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-misleading-bidirectional.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-misleading-bidirectional.cpp

Index: clang-tools-extra/docs/clang-tidy/checks/misc-misleading-bidirectional.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-misleading-bidirectional.rst
@@ -0,0 +1,21 @@
+.. title:: clang-tidy - misc-misleading-bidirectional
+
+misc-misleading-bidirectional
+=
+
+Warn about unterminated bidirectional unicode sequence, detecting potential attack
+as described in the `Trojan Source `_ attack.
+
+Example:
+
+.. code-block:: c++
+
+#include 
+
+int main() {
+bool isAdmin = false;
+/*‮ } ⁦if (isAdmin)⁩ ⁦ begin admins only */
+std::cout << "You are an admin.\n";
+/* end admins only ‮ { ⁦*/
+return 0;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -213,6 +213,7 @@
`llvmlibc-implementation-in-namespace `_,
`llvmlibc-restrict-system-libc-headers `_, "Yes"
`misc-definitions-in-headers `_, "Yes"
+   `misc-misleading-bidirectional `_,
`misc-misleading-identifier `_,
`misc-misplaced-const `_,
`misc-new-delete-overloads `_,
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -120,6 +120,10 @@
   Reports identifiers whose names are too short. Currently checks local
   variables and function parameters only.
 
+- New :doc:`misc-misleading-bidirectional ` check.
+
+  Inspects string literal and comments for unterminated bidirectional Unicode
+  characters.
 
 New check aliases
 ^
Index: clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.h
@@ -0,0 +1,38 @@
+//===--- MisleadingBidirectionalCheck.h - clang-tidy *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISLEADINGBIDIRECTIONALCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISLEADINGBIDIRECTIONALCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+class MisleadingBidirectionalCheck : public ClangTidyCheck {
+public:
+  MisleadingBidirectionalCheck(StringRef Name, ClangTidyContext *Context);
+  ~MisleadingBidirectionalCheck();
+
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override;
+
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  class MisleadingBidirectionalHandler;
+  std::unique_ptr Handler;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISLEADINGBIDIRECTIONALCHECK_H
Index: clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp
@@ -0,0 +1,142 @@
+//===--- MisleadingBidirectional.cpp - clang-tidy -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MisleadingBidirectional.h"
+
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Preprocessor.h"
+#include "llvm/Support/ConvertUTF.h"
+
+using namespace c

[PATCH] D116638: [clang-format] Fix ignoring JavaScriptWrapImport when ColumnWidth: 0

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I would say for the `ColumnLimit:0` case, we don't have to wrap a single import 
like this: for `JavaScriptWrapImports :true`

  import {
 Get
  }  from '@nestjs/common';

For more than one import then I'd say it should do:

  import {
 Get,
 Req
  }  from '@nestjs/common';

What tells me that is the "s" at the end of "Imports" of  
`JavaScriptWrapImports` meaning more than one. i.e. we don't wrap on 1 but we 
do on 2 and above

For  `ColumnLimit: 0`  and `JavaScriptWrapImports: false`  then I think that 
means we shouldn't touch the imports at all.

This is becuase in this case we don't really want "true/false" we want 
"Leave/Always/Never", I don't think we should assume `false` means `Never` and 
hence its time to disambiguate.

This is part of our natural lifecycle of all options: (which tend to follow 
this patter)

1. they start as booleans
2. they become enums
3. they become structs

It probably means we've got to the point where they should be changed to an 
enum so we don't have to guess what all our 100,000's of users will want but 
give them the power to do what they need.


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

https://reviews.llvm.org/D116638

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

[PATCH] D116351: Update Bug report URL to Github Issues

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

It looks like @asl isn't here and many experienced guys accepted this. So I 
think it might should be good to commit this one.


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

https://reviews.llvm.org/D116351

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

[PATCH] D116316: [clang-format] Add an experimental option to remove optional control statement braces in LLVM C++ code

[MyDeveloperDay via Phabricator via cfe-commits]

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

Owen I think we should push ahead with this rather than D95168: [clang-format] 
Add Insert/Remove Braces option  as I've 
looked at what you've done here I'm reminded that the removal and insertion are 
likely mutually exclusive operations.

There is no doubt that insertion is desired (so I'd like to see us do that 
afterwards) but I now think that could be in a completely separate pass as you 
originally suggested.

LGTM


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

https://reviews.llvm.org/D116316

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

[libunwind] bbce75e - Update Bug report URL to Github Issues

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2022-01-06T17:33:25+08:00
New Revision: bbce75e352be0637305a1b59ac5eca7175bceece

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

LOG: Update Bug report URL to Github Issues

Although we moved to Github Issues. The bug report message refers to
Bugzilla still. This patch tries to update these URLs.

Reviewed By: MaskRay, Quuxplusone, jhenderson, libunwind, libc++

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

Added: 


Modified: 
clang-tools-extra/docs/clang-doc.rst
clang/docs/CommandGuide/clang.rst
clang/www/c_status.html
clang/www/cxx_status.html
clang/www/get_involved.html
clang/www/get_started.html
clang/www/menu.html.incl
libcxx/docs/index.rst
libunwind/docs/index.rst
lldb/docs/index.rst
llvm/CMakeLists.txt
llvm/docs/CommandGuide/llvm-install-name-tool.rst
llvm/docs/CommandGuide/llvm-libtool-darwin.rst
llvm/docs/CommandGuide/llvm-lipo.rst
llvm/docs/CommandGuide/llvm-objcopy.rst
llvm/docs/CommandGuide/llvm-objdump.rst
llvm/docs/CommandGuide/llvm-otool.rst
llvm/docs/CommandGuide/llvm-size.rst
llvm/docs/CommandGuide/llvm-strings.rst
llvm/docs/CommandGuide/llvm-strip.rst
llvm/utils/gn/secondary/clang/include/clang/Config/BUILD.gn
llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
utils/bazel/llvm-project-overlay/clang/include/clang/Config/config.h
utils/bazel/llvm-project-overlay/llvm/include/llvm/Config/config.h

Removed: 




diff  --git a/clang-tools-extra/docs/clang-doc.rst 
b/clang-tools-extra/docs/clang-doc.rst
index 9be8a8dc31d45..2db3e914ac8a7 100644
--- a/clang-tools-extra/docs/clang-doc.rst
+++ b/clang-tools-extra/docs/clang-doc.rst
@@ -12,7 +12,7 @@ source code and comments.
 
 The tool is in a very early development stage, so you might encounter bugs and
 crashes. Submitting reports with information about how to reproduce the issue
-to `the LLVM bugtracker `_ will definitely help the
+to `the LLVM bug tracker `_ will 
definitely help the
 project. If you have any ideas or suggestions, please to put a feature request
 there.
 

diff  --git a/clang/docs/CommandGuide/clang.rst 
b/clang/docs/CommandGuide/clang.rst
index a24e138e86a7d..6797020d1b68e 100644
--- a/clang/docs/CommandGuide/clang.rst
+++ b/clang/docs/CommandGuide/clang.rst
@@ -662,7 +662,7 @@ ENVIRONMENT
 BUGS
 
 
-To report bugs, please visit .  Most bug reports should
+To report bugs, please visit .  
Most bug reports should
 include preprocessed source files (use the :option:`-E` option) and the full
 output of the compiler, along with information to reproduce.
 

diff  --git a/clang/www/c_status.html b/clang/www/c_status.html
index 561f061c6449f..42bc57d969ad8 100644
--- a/clang/www/c_status.html
+++ b/clang/www/c_status.html
@@ -72,8 +72,8 @@ C Support in Clang
 reports, but we do not currently track our DR status (help with
 tracking DR status is appreciated).
 
-The https://bugs.llvm.org/";>LLVM bug tracker contains a
-Clang C component that tracks known bugs with Clang's language
+The https://github.com/llvm/llvm-project/issues/";>LLVM bug 
tracker uses
+the "c", "c11", "c18", and "c2x" labels to track known bugs with Clang's 
language
 conformance.
 
 C89 implementation status

diff  --git a/clang/www/cxx_status.html b/clang/www/cxx_status.html
index 3cf12ff477215..5005ec6c08e4c 100755
--- a/clang/www/cxx_status.html
+++ b/clang/www/cxx_status.html
@@ -76,9 +76,9 @@ C++ Support in Clang
 Specifications that will help drive the future of the C++ programming
 language.
 
-The https://bugs.llvm.org/";>LLVM bug tracker contains Clang
-C++ components that track known bugs with Clang's language conformance in
-each language mode.
+The https://github.com/llvm/llvm-project/issues/";>LLVM bug 
tracker uses
+the "c++" label, as well as mode-specific labels such as "c++11", "c++14",
+and so on, to track known bugs with Clang's language conformance.
 
 C++98 implementation status
 

diff  --git a/clang/www/get_involved.html b/clang/www/get_involved.html
index 96784108ce6e6..d6eddb8227a54 100755
--- a/clang/www/get_involved.html
+++ b/clang/www/get_involved.html
@@ -65,7 +65,7 @@ Follow what's going on
 
 If you're looking for something to work on, check out our Open Projects page or look through the https://bugs.llvm.org/";>Bugzilla bug database.
+href="https://github.com/llvm/llvm-project/issues/";>LLVM bug tracker.
 
 Contributing Extensions to Clang
 

diff  --git a/clang/www/get_started.html b/clang/www/get_started.html
index bc9629d7e2ff2..ab5f7fac6a6c9 100755
--- a/clang/www/get_started.html
+++ b/clang/www/get_started.html
@@ -19,

[PATCH] D116351: Update Bug report URL to Github Issues

[Chuanqi Xu via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbbce75e352be: Update Bug report URL to Github Issues 
(authored by ChuanqiXu).

Changed prior to commit:
  https://reviews.llvm.org/D116351?vs=397763&id=397817#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116351

Files:
  clang-tools-extra/docs/clang-doc.rst
  clang/docs/CommandGuide/clang.rst
  clang/www/c_status.html
  clang/www/cxx_status.html
  clang/www/get_involved.html
  clang/www/get_started.html
  clang/www/menu.html.incl
  libcxx/docs/index.rst
  libunwind/docs/index.rst
  lldb/docs/index.rst
  llvm/CMakeLists.txt
  llvm/docs/CommandGuide/llvm-install-name-tool.rst
  llvm/docs/CommandGuide/llvm-libtool-darwin.rst
  llvm/docs/CommandGuide/llvm-lipo.rst
  llvm/docs/CommandGuide/llvm-objcopy.rst
  llvm/docs/CommandGuide/llvm-objdump.rst
  llvm/docs/CommandGuide/llvm-otool.rst
  llvm/docs/CommandGuide/llvm-size.rst
  llvm/docs/CommandGuide/llvm-strings.rst
  llvm/docs/CommandGuide/llvm-strip.rst
  llvm/utils/gn/secondary/clang/include/clang/Config/BUILD.gn
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  utils/bazel/llvm-project-overlay/clang/include/clang/Config/config.h
  utils/bazel/llvm-project-overlay/llvm/include/llvm/Config/config.h

Index: utils/bazel/llvm-project-overlay/llvm/include/llvm/Config/config.h
===
--- utils/bazel/llvm-project-overlay/llvm/include/llvm/Config/config.h
+++ utils/bazel/llvm-project-overlay/llvm/include/llvm/Config/config.h
@@ -24,7 +24,7 @@
 #include "llvm/Config/llvm-config.h"
 
 /* Bug report URL. */
-#define BUG_REPORT_URL "https://bugs.llvm.org/";
+#define BUG_REPORT_URL "https://github.com/llvm/llvm-project/issues/";
 
 /* Define to 1 to enable backtraces, and to 0 otherwise. */
 #define ENABLE_BACKTRACES 1
@@ -332,7 +332,7 @@
 /* LTDL_SHLIB_EXT defined in Bazel */
 
 /* Define to the address where bug reports for this package should be sent. */
-#define PACKAGE_BUGREPORT "https://bugs.llvm.org/";
+#define PACKAGE_BUGREPORT "https://github.com/llvm/llvm-project/issues/";
 
 /* Define to the full name of this package. */
 #define PACKAGE_NAME "LLVM"
Index: utils/bazel/llvm-project-overlay/clang/include/clang/Config/config.h
===
--- utils/bazel/llvm-project-overlay/clang/include/clang/Config/config.h
+++ utils/bazel/llvm-project-overlay/clang/include/clang/Config/config.h
@@ -20,7 +20,7 @@
 #define CLANG_CONFIG_H
 
 /* Bug report URL. */
-#define BUG_REPORT_URL "https://bugs.llvm.org/";
+#define BUG_REPORT_URL "https://github.com/llvm/llvm-project/issues/";
 
 /* Default to -fPIE and -pie on Linux. */
 #define CLANG_DEFAULT_PIE_ON_LINUX 0
Index: llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
@@ -72,7 +72,7 @@
   input = "config.h.cmake"
   output = "$target_gen_dir/config.h"
   values = [
-"BUG_REPORT_URL=https://bugs.llvm.org/";,
+"BUG_REPORT_URL=https://github.com/llvm/llvm-project/issues/";,
 "ENABLE_BACKTRACES=1",
 "ENABLE_CRASH_OVERRIDES=1",
 "BACKTRACE_HEADER=execinfo.h",
@@ -120,7 +120,7 @@
 "LLVM_VERSION_INFO=",
 "LLVM_VERSION_PRINTER_SHOW_HOST_TARGET_INFO=1",
 "LLVM_WINDOWS_PREFER_FORWARD_SLASH=",
-"PACKAGE_BUGREPORT=https://bugs.llvm.org/";,
+"PACKAGE_BUGREPORT=https://github.com/llvm/llvm-project/issues/";,
 "PACKAGE_NAME=LLVM",
 "PACKAGE_STRING=LLVM ${llvm_version}git",
 "PACKAGE_VERSION=${llvm_version}git",
Index: llvm/utils/gn/secondary/clang/include/clang/Config/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/include/clang/Config/BUILD.gn
+++ llvm/utils/gn/secondary/clang/include/clang/Config/BUILD.gn
@@ -8,7 +8,7 @@
   input = "config.h.cmake"
   output = "$target_gen_dir/config.h"
   values = [
-"BUG_REPORT_URL=https://bugs.llvm.org/";,
+"BUG_REPORT_URL=https://github.com/llvm/llvm-project/issues/";,
 "CLANG_DEFAULT_PIE_ON_LINUX=",
 "CLANG_DEFAULT_LINKER=",
 "CLANG_DEFAULT_STD_C=",
Index: llvm/docs/CommandGuide/llvm-strip.rst
===
--- llvm/docs/CommandGuide/llvm-strip.rst
+++ llvm/docs/CommandGuide/llvm-strip.rst
@@ -194,7 +194,7 @@
 BUGS
 
 
-To report bugs, please visit .
+To report bugs, please visit .
 
 SEE ALSO
 
Index: llvm/docs/CommandGuide/llvm-strings.rst
===

[PATCH] D116316: [clang-format] Add an experimental option to remove optional control statement braces in LLVM C++ code

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D116316#3224591 , @MyDeveloperDay 
wrote:

> Owen I think we should push ahead with this rather than D95168: 
> [clang-format] Add Insert/Remove Braces option 
>  as I've looked at what you've done here I'm 
> reminded that the removal and insertion are likely mutually exclusive 
> operations.
>
> There is no doubt that insertion is desired (so I'd like to see us do that 
> afterwards) but I now think that could be in a completely separate pass as 
> you originally suggested.
>
> LGTM

Thanks! I will wait a couple of days in case other reviewers have some final 
requests.


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

https://reviews.llvm.org/D116316

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

[PATCH] D116731: [Clang] Make Clang copy its CMake modules into the build dir

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski created this revision.
Herald added subscribers: Chia-hungDuan, rriddle, mgorny.
awarzynski requested review of this revision.
Herald added subscribers: cfe-commits, stephenneuendorffer.
Herald added a project: clang.

LLVM has a documented mechanism for passing configuration information to
an out of tree project using CMake. See
https://llvm.org/docs/CMake.html#embedding-llvm-in-your-project.
Similar logic applies to "standalone" builds of other sub-projects
within LLVM that depend on each other. For example, a standalone build
of Flang will use this mechanism to acquire Clang's configuration.

Currently, the relevant CMake modules for Clang will only be copied into
the installation directory. This means that in order to configure a
standalone build of Flang, one has to first build and then install
Clang. This is not required for LLVM nor for MLIR - other sub-projects
that Flang depends on (i.e. the CMake modules for LLVM and MLIR are
available in the build dir, so installation is not needed).

This change removes the need for installing Clang in order to access its
configuration. It makes sure that the required CMake modules are copied
into the build directory. This will make Clang behave consistently with
LLVM and MLIR in this respect. It will also simplify building Flang as
standalone sub-project.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116731

Files:
  clang/cmake/modules/CMakeLists.txt


Index: clang/cmake/modules/CMakeLists.txt
===
--- clang/cmake/modules/CMakeLists.txt
+++ clang/cmake/modules/CMakeLists.txt
@@ -28,6 +28,14 @@
 set(CLANG_CONFIG_CMAKE_DIR)
 set(CLANG_CONFIG_LLVM_CMAKE_DIR)
 
+# For compatibility with projects that include(ClangConfig)
+# via CMAKE_MODULE_PATH, place API modules next to it.
+file(COPY .
+  DESTINATION ${clang_cmake_builddir}
+  FILES_MATCHING PATTERN *.cmake
+  PATTERN CMakeFiles EXCLUDE
+  )
+
 # Generate ClangConfig.cmake for the install tree.
 set(CLANG_CONFIG_CODE "
 # Compute the installation prefix from this LLVMConfig.cmake file location.


Index: clang/cmake/modules/CMakeLists.txt
===
--- clang/cmake/modules/CMakeLists.txt
+++ clang/cmake/modules/CMakeLists.txt
@@ -28,6 +28,14 @@
 set(CLANG_CONFIG_CMAKE_DIR)
 set(CLANG_CONFIG_LLVM_CMAKE_DIR)
 
+# For compatibility with projects that include(ClangConfig)
+# via CMAKE_MODULE_PATH, place API modules next to it.
+file(COPY .
+  DESTINATION ${clang_cmake_builddir}
+  FILES_MATCHING PATTERN *.cmake
+  PATTERN CMakeFiles EXCLUDE
+  )
+
 # Generate ClangConfig.cmake for the install tree.
 set(CLANG_CONFIG_CODE "
 # Compute the installation prefix from this LLVMConfig.cmake file location.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116378: [clang-tidy] Disable clang-tidy warnings from system macros

[Salman Javed via Phabricator via cfe-commits]

salman-javed-nz added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp:23-40
+namespace {
+class VaArgPPCallbacks : public PPCallbacks {
+public:
+  VaArgPPCallbacks(ProTypeVarargCheck *Check) : Check(Check) {}
+
+  void MacroExpands(const Token &MacroNameTok, const MacroDefinition &MD,
+SourceRange Range, const MacroArgs *Args) override {

Should this be in another patch?

Line 722 in ClangTidyDiagnosticConsumer.cpp makes it so that clang-tidy filters 
warnings from system macros. This would benefit all checks.

This VaArgPPCallBack change here only benefits 
`cppcoreguidelines-pro-type-vararg`.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp:46-78
+"__builtin_isgreater",
+"__builtin_isgreaterequal",
 "__builtin_isless",
-"__builtin_islessequal", 
-"__builtin_islessgreater", 
+"__builtin_islessequal",
+"__builtin_islessgreater",
 "__builtin_isunordered",
+"__builtin_fpclassify",

Formatting changed for code not directly involved in this patch. Should be 
moved to a separate NFC commit that you don't have to run by us.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116378

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

[PATCH] D116731: [Clang] Make Clang copy its CMake modules into the build dir

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

For a bit of context - this came up in a discussion for 
https://reviews.llvm.org/D116566/.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116731

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

[PATCH] D112913: Misleading bidirectional detection

[Xidorn Quan via Phabricator via cfe-commits]

upsuper added a comment.

I think the core algorithm looks correct now. I'll leave the code review to 
LLVM reviewers. Thanks.


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

https://reviews.llvm.org/D112913

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

[PATCH] D116599: Simplify AttrBuilder storage for target dependent attributes

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 397831.
serge-sans-paille added a comment.

Minor updates


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

https://reviews.llvm.org/D116599

Files:
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  llvm/include/llvm/CodeGen/IndirectThunks.h
  llvm/include/llvm/IR/Attributes.h
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/CodeGen/Analysis.cpp
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
  llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
  llvm/lib/Transforms/Coroutines/CoroSplit.cpp
  llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
  llvm/lib/Transforms/Utils/CallPromotionUtils.cpp
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
  llvm/tools/bugpoint/CrashDebugger.cpp
  llvm/tools/llvm-reduce/deltas/ReduceAttributes.cpp
  llvm/unittests/ExecutionEngine/Orc/IndirectionUtilsTest.cpp
  llvm/unittests/IR/AttributesTest.cpp
  llvm/unittests/IR/InstructionsTest.cpp
  mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

Index: mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
===
--- mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -746,7 +746,7 @@
 return func.emitError(
 "llvm.align attribute attached to LLVM non-pointer argument");
   llvmArg.addAttrs(
-  llvm::AttrBuilder().addAlignmentAttr(llvm::Align(attr.getInt(;
+  llvm::AttrBuilder(llvmArg.getContext()).addAlignmentAttr(llvm::Align(attr.getInt(;
 }
 
 if (auto attr = func.getArgAttrOfType(argIdx, "llvm.sret")) {
@@ -754,7 +754,7 @@
   if (!argTy.isa())
 return func.emitError(
 "llvm.sret attribute attached to LLVM non-pointer argument");
-  llvmArg.addAttrs(llvm::AttrBuilder().addStructRetAttr(
+  llvmArg.addAttrs(llvm::AttrBuilder(llvmArg.getContext()).addStructRetAttr(
   llvmArg.getType()->getPointerElementType()));
 }
 
@@ -763,7 +763,7 @@
   if (!argTy.isa())
 return func.emitError(
 "llvm.byval attribute attached to LLVM non-pointer argument");
-  llvmArg.addAttrs(llvm::AttrBuilder().addByValAttr(
+  llvmArg.addAttrs(llvm::AttrBuilder(llvmArg.getContext()).addByValAttr(
   llvmArg.getType()->getPointerElementType()));
 }
 
Index: llvm/unittests/IR/InstructionsTest.cpp
===
--- llvm/unittests/IR/InstructionsTest.cpp
+++ llvm/unittests/IR/InstructionsTest.cpp
@@ -614,7 +614,7 @@
 
   // Test cloning an attribute.
   {
-AttrBuilder AB;
+AttrBuilder AB(C);
 AB.addAttribute(Attribute::ReadOnly);
 Call->setAttributes(
 AttributeList::get(C, AttributeList::FunctionIndex, AB));
@@ -633,7 +633,7 @@
   std::unique_ptr Call(
   CallInst::Create(FnTy, Callee, Args, OldBundle, "result"));
   Call->setTailCallKind(CallInst::TailCallKind::TCK_NoTail);
-  AttrBuilder AB;
+  AttrBuilder AB(C);
   AB.addAttribute(Attribute::Cold);
   Call->setAttributes(AttributeList::get(C, AttributeList::FunctionIndex, AB));
   Call->setDebugLoc(DebugLoc(MDNode::get(C, None)));
@@ -662,7 +662,7 @@
   std::unique_ptr Invoke(
   InvokeInst::Create(FnTy, Callee, NormalDest.get(), UnwindDest.get(), Args,
  OldBundle, "result"));
-  AttrBuilder AB;
+  AttrBuilder AB(C);
   AB.addAttribute(Attribute::Cold);
   Invoke->setAttributes(
   AttributeList::get(C, AttributeList::FunctionIndex, AB));
Index: llvm/unittests/IR/AttributesTest.cpp
===
--- llvm/unittests/IR/AttributesTest.cpp
+++ llvm/unittests/IR/AttributesTest.cpp
@@ -62,9 +62,9 @@
 TEST(Attributes, AddAttributes) {
   LLVMContext C;
   AttributeList AL;
-  AttrBuilder B;
+  AttrBuilder B(C);
   B.addAttribute(Attribute::NoReturn);
-  AL = AL.addFnAttributes(C, AttributeSet::get(C, B));
+  AL = AL.addFnAttributes(C, AttrBuilder(C, AttributeSet::get(C, B)));
   EXPECT_TRUE(AL.hasFnAttr(Attribute::NoReturn));
   B.clear();
   B.addAttribute(Attribute::SExt);
@@ -78,12 +78,12 @@
 
   Attribute AlignAttr = Attribute::getWithAlignment(C, Align(8));
   Attribute StackAlignAttr = Attribute::getWithStackAlignment(C, Align(32));
-  AttrBuilder B_align_readonly;
+  AttrBuilder B_align_readonly(C);
   B_align_readonly.addAttribute(AlignAttr);
   B_align_readonly.addAttribute(Attribu

[PATCH] D115867: [C++20] [Coroutines] Warning for always_inline coroutine

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 397835.
ChuanqiXu added a comment.
Herald added a reviewer: aaron.ballman.

Address comments:

- Update warning message as Mathias's suggestion.
- Update document.


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

https://reviews.llvm.org/D115867

Files:
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaCoroutine.cpp
  clang/test/SemaCXX/coroutines.cpp


Index: clang/test/SemaCXX/coroutines.cpp
===
--- clang/test/SemaCXX/coroutines.cpp
+++ clang/test/SemaCXX/coroutines.cpp
@@ -1460,3 +1460,13 @@
   co_await missing_await_suspend{}; // expected-error {{no member named 
'await_suspend' in 'missing_await_suspend'}}
   co_await missing_await_resume{}; // expected-error {{no member named 
'await_resume' in 'missing_await_resume'}}
 }
+
+__attribute__((__always_inline__))
+void warn_always_inline() { // expected-warning {{a coroutine may be split 
into pieces and not every piece of the coroutine can be guaranteed to be 
inlined}}
+  co_await suspend_always{};
+}
+
+[[gnu::always_inline]]
+void warn_gnu_always_inline() { // expected-warning {{a coroutine may be split 
into pieces and not every piece of the coroutine can be guaranteed to be 
inlined}}
+  co_await suspend_always{};
+}
Index: clang/lib/Sema/SemaCoroutine.cpp
===
--- clang/lib/Sema/SemaCoroutine.cpp
+++ clang/lib/Sema/SemaCoroutine.cpp
@@ -1081,6 +1081,15 @@
 return;
   }
 
+  // The coroutine marked always inline might not be inlined properly in 
current
+  // compiler support. Since the coroutine would be splitted into pieces. And
+  // the call to resume() and destroy() functions might be indirect call. Also
+  // the ramp function wouldn't get inlined under O0 due to pipeline ordering
+  // problems. It might be different to what users expects to. Emit a warning 
to
+  // tell it.
+  if (FD->hasAttr())
+Diag(FD->getLocation(), diag::warn_always_inline_coroutine);
+
   // [stmt.return.coroutine]p1:
   //   A coroutine shall not enclose a return statement ([stmt.return]).
   if (Fn->FirstReturnLoc.isValid()) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11105,6 +11105,10 @@
 def note_coroutine_function_declare_noexcept : Note<
   "must be declared with 'noexcept'"
 >;
+def warn_always_inline_coroutine : Warning<
+  "a coroutine may be split into pieces and not every piece of the coroutine 
can be guaranteed to be inlined"
+  >,
+  InGroup;
 } // end of coroutines issue category
 
 let CategoryName = "Documentation Issue" in {
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -58,7 +58,9 @@
   DiagGroup<"deprecated-experimental-coroutine">;
 def DeprecatedCoroutine :
   DiagGroup<"deprecated-coroutine", [DeprecatedExperimentalCoroutine]>;
-def Coroutine : DiagGroup<"coroutine", [CoroutineMissingUnhandledException, 
DeprecatedCoroutine]>;
+def AlwaysInlineCoroutine :
+  DiagGroup<"always-inline-coroutine">;
+def Coroutine : DiagGroup<"coroutine", [CoroutineMissingUnhandledException, 
DeprecatedCoroutine, AlwaysInlineCoroutine]>;
 def ObjCBoolConstantConversion : DiagGroup<"objc-bool-constant-conversion">;
 def ConstantConversion : DiagGroup<"constant-conversion",
[BitFieldConstantConversion,
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -6137,6 +6137,10 @@
 
 Does not guarantee that inline substitution actually occurs.
 
+Note that a coroutine function wouldn't get inlined in O0 if it is marked with 
"always_inline".
+In other optimization levels, only the first part of the coroutine - "ramp 
function"`_
+can be guaranteed to be inlined.
+
 See also `the Microsoft Docs on Inline Functions`_, `the GCC Common Function
 Attribute docs`_, and `the GCC Inline docs`_.
 


Index: clang/test/SemaCXX/coroutines.cpp
===
--- clang/test/SemaCXX/coroutines.cpp
+++ clang/test/SemaCXX/coroutines.cpp
@@ -1460,3 +1460,13 @@
   co_await missing_await_suspend{}; // expected-error {{no member named 'await_suspend' in 'missing_await_suspend'}}
   co_await missing_await_resume{}; // expected-error {{no member named 'await_resume' in 'missing_await_resume'}}
 }
+
+__attribute__((__always_inline__))
+void warn_always_inline() { //

[PATCH] D116316: [clang-format] Add an experimental option to remove optional control statement braces in LLVM C++ code

[Marek Kurdej via Phabricator via cfe-commits]

curdeius accepted this revision.
curdeius added a comment.

LGTM with some nits.




Comment at: clang/lib/Format/UnwrappedLineParser.cpp:435-454
+bool UnwrappedLineParser::precededByCommentOrPPDirective() const {
+  const size_t size = Lines.size();
+  if (size > 0 && Lines[size - 1].InPPDirective)
+return true;
+#if 1
+  const unsigned Position = Tokens->getPosition();
+  if (Position == 0)

Please remove unused code.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:485
+if (FormatTok->isNot(tok::r_brace) || StatementCount != 1 ||
+IsPrecededByCommentOrPPDirective || // StartsWithBrace ||
+precededByCommentOrPPDirective()) {

Please remove unused code.



Comment at: clang/unittests/Format/FormatTest.cpp:23224
+
+  EXPECT_EQ("if (a)\n"
+"  if (b)\n"

owenpan wrote:
> owenpan wrote:
> > MyDeveloperDay wrote:
> > > any reason these can't be verifyFormats?
> > Did you mean to add the expected part as a separate case? I don't think it 
> > would add any value if there are no braces to remove in the first place?
> > Did you mean to add the expected part as a separate case? I don't think it 
> > would add any value if there are no braces to remove in the first place?
> 
> Nvm. I think you wanted something like `verifyFormat(PostformatCode, 
> PreformatCode, Style)`? Yes, I could do that, but I would have to relax the 
> restriction to calling `BracesRemover()` in Format.cpp, i.e. checking 
> `isCpp()` instead.
:+1: you should be able to use the 2-argument version of `verifyFormat`, no?


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

https://reviews.llvm.org/D116316

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

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

[Elvis Stansvik via Phabricator via cfe-commits]

estan added a comment.

@carlosgalvezp Oh yes, that would be even better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88833

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

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

[Elvis Stansvik via Phabricator via cfe-commits]

estan added a comment.

In D88833#3222829 , @estan wrote:

> Sounds good @aaron.ballman, let's wait for @fiesh.
>
> Though I realize now that the scope of this patch is probably not enough to 
> solve a problem we have in our code base. The check will warn about (for 
> example) things like this:
>
> In a third party lib outside our control:
>
>   void f(double out[3]);
>
> In our code:
>
>   double out[3];
>   f(out);
>
> Include paths for the third party lib are added with -isystem.
>
> Am I right that we're still going to get warnings for this with this patch?
>
> Full disclosure. The third party lib is VTK, which is littered with APIs like 
> these.

@aaron.ballman BTW, would you guys be open to a patch that makes the check not 
warn in cases like the above, or at least an option to make it not warn?

We like the spirit of this check, to make sure we use gsl::array_view/std::span 
instead of (T *data, int size) style interfaces in our own APIs. But we cannot 
change the (T*) APIs of our third party libraries. Having to litter our code 
with hundreds of casts is not very nice. It would be great if there was an 
option to tell the check to ignore all decays that occur in calls to functions 
in system headers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88833

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

[PATCH] D115936: [Clang] Add isInNamespace() to check if a Decl in a specific namespace

[Jun Zhang via Phabricator via cfe-commits]

junaire updated this revision to Diff 397842.
junaire added a comment.

After Aarron's comments, I realize what I really want to do is add a new
ASTMatcher that matches declarations what in the top level namespace like std.

This update fixed previous broken one, added some comments and unit tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115936

Files:
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/AST/DeclBase.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -3478,6 +3478,33 @@
   EXPECT_TRUE(matches("namespace {}", namespaceDecl(isAnonymous(;
 }
 
+TEST_P(ASTMatchersTest, InTopNamespace) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  EXPECT_TRUE(notMatches(
+  "namespace boost {"
+  "  namespace detail {"
+  "class shared_ptr {};"
+  "  }"
+  "}",
+  cxxRecordDecl(hasName("shared_ptr"), isInTopNamespace("boost";
+
+  EXPECT_TRUE(
+  matches("namespace boost {"
+  "  class shared_ptr {};"
+  "}",
+  cxxRecordDecl(hasName("shared_ptr"), isInTopNamespace("boost";
+
+  EXPECT_TRUE(
+  matches("namespace boost {"
+  "  inline namespace __1 {"
+  "class shared_ptr {};"
+  "  }"
+  "}",
+  cxxRecordDecl(hasName("shared_ptr"), isInTopNamespace("boost";
+}
+
 TEST_P(ASTMatchersTest, InStdNamespace) {
   if (!GetParam().isCXX()) {
 return;
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -391,9 +391,11 @@
   return false;
 }
 
-bool Decl::isInStdNamespace() const {
+bool Decl::isInStdNamespace() const { return isInTopNamespace("std"); }
+
+bool Decl::isInTopNamespace(llvm::StringRef Namespace) const {
   const DeclContext *DC = getDeclContext();
-  return DC && DC->isStdNamespace();
+  return DC && DC->isTopNamespace(Namespace);
 }
 
 TranslationUnitDecl *Decl::getTranslationUnitDecl() {
@@ -1123,20 +1125,21 @@
  cast(this)->isInline();
 }
 
-bool DeclContext::isStdNamespace() const {
+bool DeclContext::isStdNamespace() const { return isTopNamespace("std"); }
+bool DeclContext::isTopNamespace(llvm::StringRef Namespace) const {
   if (!isNamespace())
 return false;
 
   const auto *ND = cast(this);
   if (ND->isInline()) {
-return ND->getParent()->isStdNamespace();
+return ND->getParent()->isTopNamespace(Namespace);
   }
 
   if (!getParent()->getRedeclContext()->isTranslationUnit())
 return false;
 
   const IdentifierInfo *II = ND->getIdentifier();
-  return II && II->isStr("std");
+  return II && II->isStr(Namespace);
 }
 
 bool DeclContext::isDependentContext() const {
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -7675,6 +7675,29 @@
   return Node.isAnonymousNamespace();
 }
 
+/// Matches declarations in the top level namespace, but not in nested
+/// namespaces. This is because nested namespaces are designed for
+/// implementation details, we don't need to match them.
+///
+/// Given
+/// \code
+///   class shared_ptr {};
+///   namespace boost {
+/// class shared_ptr {}; #1
+/// namespace detail {
+///   class shared_ptr {};
+/// }
+/// inline namespace __1 {
+///   class shared_ptr {}; #2
+/// }
+///   }
+/// \endcode
+/// cxxRecordDecl(hasName("shared_ptr"), isInTopNamespace("boost")) will match
+/// #1 and #2.
+AST_MATCHER_P(Decl, isInTopNamespace, llvm::StringRef, Namespace) {
+  return Node.isInTopNamespace(Namespace);
+}
+
 /// Matches declarations in the namespace `std`, but not in nested namespaces.
 ///
 /// Given
Index: clang/include/clang/AST/DeclBase.h
===
--- clang/include/clang/AST/DeclBase.h
+++ clang/include/clang/AST/DeclBase.h
@@ -462,6 +462,10 @@
 
   bool isInAnonymousNamespace() const;
 
+  /// Helper to check if a Decl in a specific top level namespace, while inline
+  /// namespaces will be skipped.
+  bool isInTopNamespace(llvm::StringRef) const;
+
   bool isInStdNamespace() const;
 
   ASTContext &getASTContext() const LLVM_READONLY;
@@ -1943,6 +1947,8 @@
 
   bool isNamespace() const { return getDeclKind() == Decl::Namespace; }
 
+  bool isTopNamespace(llvm::StringRef Namespace) const;
+
   bool isStdNamespace() const;
 
   bool isInlineNamespace() const;
___
cfe-comm

[PATCH] D106518: [RISCV] Disable EEW=64 for index values when XLEN=32.

[Jianjian Guan via Phabricator via cfe-commits]

jacquesguan added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106518

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

[PATCH] D116736: [Clang] Add __builtin_reduce_or and __builtin_reduce_and

[Jun Zhang via Phabricator via cfe-commits]

junaire created this revision.
junaire added a reviewer: fhahn.
junaire requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch implements two builtins specified in D111529 
.
The last __builtin_reduce_add will be seperated into another one.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116736

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

Index: clang/test/Sema/builtins-reduction-math.c
===
--- clang/test/Sema/builtins-reduction-math.c
+++ clang/test/Sema/builtins-reduction-math.c
@@ -52,3 +52,37 @@
   i = __builtin_reduce_xor(v);
   // expected-error@-1 {{1st argument must be a vector of integers (was 'float4' (vector of 4 'float' values))}}
 }
+
+void test_builtin_reduce_or(int i, float4 v, int3 iv) {
+  struct Foo s = __builtin_reduce_or(iv);
+  // expected-error@-1 {{initializing 'struct Foo' with an expression of incompatible type 'int'}}
+
+  i = __builtin_reduce_or();
+  // expected-error@-1 {{too few arguments to function call, expected 1, have 0}}
+
+  i = __builtin_reduce_or(iv, iv);
+  // expected-error@-1 {{too many arguments to function call, expected 1, have 2}}
+
+  i = __builtin_reduce_or(i);
+  // expected-error@-1 {{1st argument must be a vector of integers (was 'int')}}
+
+  i = __builtin_reduce_or(v);
+  // expected-error@-1 {{1st argument must be a vector of integers (was 'float4' (vector of 4 'float' values))}}
+}
+
+void test_builtin_reduce_and(int i, float4 v, int3 iv) {
+  struct Foo s = __builtin_reduce_and(iv);
+  // expected-error@-1 {{initializing 'struct Foo' with an expression of incompatible type 'int'}}
+
+  i = __builtin_reduce_and();
+  // expected-error@-1 {{too few arguments to function call, expected 1, have 0}}
+
+  i = __builtin_reduce_and(iv, iv);
+  // expected-error@-1 {{too many arguments to function call, expected 1, have 2}}
+
+  i = __builtin_reduce_and(i);
+  // expected-error@-1 {{1st argument must be a vector of integers (was 'int')}}
+
+  i = __builtin_reduce_and(v);
+  // expected-error@-1 {{1st argument must be a vector of integers (was 'float4' (vector of 4 'float' values))}}
+}
Index: clang/test/CodeGen/builtins-reduction-math.c
===
--- clang/test/CodeGen/builtins-reduction-math.c
+++ clang/test/CodeGen/builtins-reduction-math.c
@@ -68,3 +68,25 @@
   // CHECK-NEXT: call i32 @llvm.vector.reduce.xor.v4i32(<4 x i32> [[VU1]])
   unsigned r3 = __builtin_reduce_xor(vu1);
 }
+
+void test_builtin_reduce_or(si8 vi1, u4 vu1) {
+
+  // CHECK:  [[VI1:%.+]] = load <8 x i16>, <8 x i16>* %vi1.addr, align 16
+  // CHECK-NEXT: call i16 @llvm.vector.reduce.or.v8i16(<8 x i16> [[VI1]])
+  short r2 = __builtin_reduce_or(vi1);
+
+  // CHECK:  [[VU1:%.+]] = load <4 x i32>, <4 x i32>* %vu1.addr, align 16
+  // CHECK-NEXT: call i32 @llvm.vector.reduce.or.v4i32(<4 x i32> [[VU1]])
+  unsigned r3 = __builtin_reduce_or(vu1);
+}
+
+void test_builtin_reduce_and(si8 vi1, u4 vu1) {
+
+  // CHECK:  [[VI1:%.+]] = load <8 x i16>, <8 x i16>* %vi1.addr, align 16
+  // CHECK-NEXT: call i16 @llvm.vector.reduce.and.v8i16(<8 x i16> [[VI1]])
+  short r2 = __builtin_reduce_and(vi1);
+
+  // CHECK:  [[VU1:%.+]] = load <4 x i32>, <4 x i32>* %vu1.addr, align 16
+  // CHECK-NEXT: call i32 @llvm.vector.reduce.and.v4i32(<4 x i32> [[VU1]])
+  unsigned r3 = __builtin_reduce_and(vu1);
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -2232,8 +2232,10 @@
 break;
   }
 
-  // __builtin_reduce_xor supports vector of integers only.
-  case Builtin::BI__builtin_reduce_xor: {
+  // This builtins support vector of integers only.
+  case Builtin::BI__builtin_reduce_xor:
+  case Builtin::BI__builtin_reduce_or:
+  case Builtin::BI__builtin_reduce_and: {
 if (PrepareBuiltinReduceMathOneArgCall(TheCall))
   return ExprError();
 
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -3208,6 +3208,12 @@
   case Builtin::BI__builtin_reduce_xor:
 return RValue::get(emitUnaryBuiltin(
 *this, E, llvm::Intrinsic::vector_reduce_xor, "rdx.xor"));
+  case Builtin::BI__builtin_reduce_or:
+return RValue::get(emitUnaryBuiltin(
+*this, E, llvm::Intrinsic::vector_reduce_or, "rdx.or"));
+  case Builtin::BI__builtin_reduce_and:
+return RValue::get(emitUnaryBuiltin(
+*this, E, llvm::Intrinsic::vector_reduce_and, "rdx.and"));
 
   case Builtin::BI__builtin_matrix_transpose: {
 const auto *MatrixTy = E->getArg(

[PATCH] D116155: [clang][AST][ASTImporter] Set record to complete during import of its members.

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:2021
+  // fields imported) at that time without multiple AST import passes.
+  To->setCompleteDefinition(true);
   // Complete the definition even if error is returned.

balazske wrote:
> balazske wrote:
> > shafik wrote:
> > > So `DefinitionCompleterScopeExit` will run 
> > > `To->setCompleteDefinition(false);`  after this function exits but this 
> > > will be in effect during the import the base classes. I don't see how the 
> > > tests you added hit that code.
> > Should the test `CompleteRecordBeforeImporting` not do the  import in 
> > minimal mode? The comment says minimal but in `ASTImporter` minimal mode is 
> > not set. The test will fail because this added line. But I think it should 
> > work to call the `To->setCompleteDefinition` here only in non-minimal mode.
> And remove the later added line 2088 `To->setCompleteDefinition(false);`.
> Should the test CompleteRecordBeforeImporting not do the import in minimal 
> mode?

Yes, that should, however, I can confirm it probably does not (see the `false` 
arg below):
```
Importer.reset(Creator(ToAST->getASTContext(), ToAST->getFileManager(),
   Unit->getASTContext(), Unit->getFileManager(), false,
   SharedState));

```

So, first we should fix the test case `CompleteRecordBeforeImporting` to set up 
the ASTImporter in minimal mode.

Then, we should call the To->setCompleteDefinition here only in non-minimal 
mode as you suggests. Once again, an ugly branching because of the "minimal" 
mode, we should really get rid of that (and hope that Raphael patch evolves in 
D101950).



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116155

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

[PATCH] D116726: [clang-format] Fix a crash (assertion) in qualifier alignment when matching template closer is null

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 397853.
MyDeveloperDay marked 3 inline comments as done.
MyDeveloperDay added a comment.

Use getNextNonComment() which has some const-ness knock ons (but probably not a 
bad thing)

address the review concerns by adding more tests (which indeed highlighted new 
failure scenarios! Thanks @curdeius)


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

https://reviews.llvm.org/D116726

Files:
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/lib/Format/QualifierAlignmentFixer.h
  clang/unittests/Format/QualifierFixerTest.cpp

Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -818,5 +818,53 @@
   EXPECT_EQ(ReplacementCount, 0);
 }
 
+TEST_F(QualifierFixerTest, QualifierTemplates) {
+  FormatStyle Style = getLLVMStyle();
+  Style.QualifierAlignment = FormatStyle::QAS_Custom;
+  Style.QualifierOrder = {"static", "const", "type"};
+
+  ReplacementCount = 0;
+  EXPECT_EQ(ReplacementCount, 0);
+  verifyFormat("template  using A = quantity, 1>;", Style);
+  verifyFormat("template  using A = quantity /**/, 1>;",
+   Style);
+  verifyFormat("template  using A = quantity /* */, 1>;",
+   Style);
+  verifyFormat("template  using A = quantity /*foo*/, 1>;",
+   Style);
+  verifyFormat("template  using A = quantity /**/ /**/, 1>;",
+   Style);
+  verifyFormat("template  using A = quantity, 1>;",
+   Style);
+  verifyFormat("template  using A = /**/ quantity, 1>;",
+   Style);
+  EXPECT_EQ(ReplacementCount, 0);
+  verifyFormat("template \n"
+   "using A = quantity // foo\n"
+   ", 1>;",
+   Style);
+
+  ReplacementCount = 0;
+  Style.QualifierOrder = {"type", "static", "const"};
+  verifyFormat("template  using A = quantity, 1>;", Style);
+  verifyFormat("template  using A = quantity /**/, 1>;",
+   Style);
+  verifyFormat("template  using A = quantity /* */, 1>;",
+   Style);
+  verifyFormat("template  using A = quantity /*foo*/, 1>;",
+   Style);
+  verifyFormat("template  using A = quantity /**/ /**/, 1>;",
+   Style);
+  verifyFormat("template  using A = quantity, 1>;",
+   Style);
+  verifyFormat("template  using A = /**/ quantity, 1>;",
+   Style);
+  EXPECT_EQ(ReplacementCount, 0);
+  verifyFormat("template \n"
+   "using A = quantity // foo\n"
+   ", 1>;",
+   Style);
+}
+
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/QualifierAlignmentFixer.h
===
--- clang/lib/Format/QualifierAlignmentFixer.h
+++ clang/lib/Format/QualifierAlignmentFixer.h
@@ -72,17 +72,19 @@
 
   static tok::TokenKind getTokenFromQualifier(const std::string &Qualifier);
 
-  FormatToken *analyzeRight(const SourceManager &SourceMgr,
-const AdditionalKeywords &Keywords,
-tooling::Replacements &Fixes, FormatToken *Tok,
-const std::string &Qualifier,
-tok::TokenKind QualifierType);
-
-  FormatToken *analyzeLeft(const SourceManager &SourceMgr,
-   const AdditionalKeywords &Keywords,
-   tooling::Replacements &Fixes, FormatToken *Tok,
-   const std::string &Qualifier,
-   tok::TokenKind QualifierType);
+  const FormatToken *analyzeRight(const SourceManager &SourceMgr,
+  const AdditionalKeywords &Keywords,
+  tooling::Replacements &Fixes,
+  const FormatToken *Tok,
+  const std::string &Qualifier,
+  tok::TokenKind QualifierType);
+
+  const FormatToken *analyzeLeft(const SourceManager &SourceMgr,
+ const AdditionalKeywords &Keywords,
+ tooling::Replacements &Fixes,
+ const FormatToken *Tok,
+ const std::string &Qualifier,
+ tok::TokenKind QualifierType);
 
   // is the Token a simple or qualifier type
   static bool isQualifierOrType(const FormatToken *Tok,
Index: clang/lib/Format/QualifierAlignmentFixer.cpp
===
--- clang/lib/Format/QualifierAlignmentFixer.cpp
+++ clang/lib/Format/QualifierAlignmentFixer.cpp
@@ -204,9 +204,9 @@
   replaceToken(SourceMgr, Fixes, Range, NewText);
 }
 
-FormatToken *LeftRightQualifierAlignmentFixer::analyzeRight(
+const FormatToken *LeftRightQualifierAlignmentFixer::analyzeRight(
 const SourceMan

[PATCH] D116378: [clang-tidy] Disable clang-tidy warnings from system macros

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp:23-40
+namespace {
+class VaArgPPCallbacks : public PPCallbacks {
+public:
+  VaArgPPCallbacks(ProTypeVarargCheck *Check) : Check(Check) {}
+
+  void MacroExpands(const Token &MacroNameTok, const MacroDefinition &MD,
+SourceRange Range, const MacroArgs *Args) override {

salman-javed-nz wrote:
> Should this be in another patch?
> 
> Line 722 in ClangTidyDiagnosticConsumer.cpp makes it so that clang-tidy 
> filters warnings from system macros. This would benefit all checks.
> 
> This VaArgPPCallBack change here only benefits 
> `cppcoreguidelines-pro-type-vararg`.
The change to `ClangTidyDiagnosticConsumer.cpp` broke this check, because it 
was checking for the use of `vararg` indirectly, checking for the use of 
`__builtin_vararg`, which was expanded from the system macro `vararg`. This 
patch updates the code so it checks directly what the rule is about "do not use 
`vararg`".  I should perhaps have been more clear about that in the commit 
message :) 



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp:46-78
+"__builtin_isgreater",
+"__builtin_isgreaterequal",
 "__builtin_isless",
-"__builtin_islessequal", 
-"__builtin_islessgreater", 
+"__builtin_islessequal",
+"__builtin_islessgreater",
 "__builtin_isunordered",
+"__builtin_fpclassify",

salman-javed-nz wrote:
> Formatting changed for code not directly involved in this patch. Should be 
> moved to a separate NFC commit that you don't have to run by us.
Yes, it's a bit unfortunate, this was an automatic change on save from my 
editor. Is it really worth it to revert this change though? I will need to 
disable this feature on my IDE, revert each line manually by adding a space, 
push, then re-enable the feature. Sounds like a lot of time spent on reverting 
a change that anyway improves the state of the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116378

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

[PATCH] D116638: [clang-format] Fix ignoring JavaScriptWrapImport when ColumnWidth: 0

[Andrey Mishchenko via Phabricator via cfe-commits]

andmis added a comment.

Thanks for the feedback. Two things:

1. Force-breaking at >= 2 imports and not breaking at 1 import feels has the 
advantage of being simple to state, implement, document, and test, but I don't 
think it's actually the behavior people will want. For example, the original 
bug reporter apparently thought about it too and apparently came up with the 
same semantics I tried to implement as his idea of "the right thing": 
https://github.com/llvm/llvm-project/issues/52935.
2. The issue isn't whether we use an enum or a boolean. The issue is that it is 
not clear what the semantics should be of "keep input file's breaking 
decisions". If `SortIncludes: true`, what should "keep" do here?

  import {
A,
B, C,
  } from 'url';
  
  import {
A,
C, B,
  } from 'url';

In my opinion, this could just be merged, pending a larger revisit: (1) very 
few people use `ColumnLimit: 0`, at least with JS code, else we would hear more 
complaints, since import wrapping is currently really not good in this case, 
(2) this patch applies the correct fix in the case `ColumnLimit: 0`, 
`JavaScriptWrapImports: false`, (3) this patch admittedly does not completely 
solve `ColumnLimit: 0`, `JavaScriptWrapImports: true`, but is not a regression 
there, either.


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

https://reviews.llvm.org/D116638

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

[PATCH] D114787: [clang][PR51931] Enable `-Wdeclaration-after-statement` for all C versions

[Markus Böck via Phabricator via cfe-commits]

zero9178 updated this revision to Diff 397857.
zero9178 added a comment.

Addressed reviewer comments: Added tests for cases when the diagnostic should 
not be emitted.


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

https://reviews.llvm.org/D114787

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaStmt.cpp
  clang/test/Sema/warn-mixed-decls.c


Index: clang/test/Sema/warn-mixed-decls.c
===
--- /dev/null
+++ clang/test/Sema/warn-mixed-decls.c
@@ -0,0 +1,23 @@
+/* RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -pedantic %s
+ */
+/* RUN: %clang_cc1 -fsyntax-only -verify -std=c99 
-Wdeclaration-after-statement %s
+ */
+
+/* Should not emit diagnostic when not pedantic, not enabled or in C++ Code*/
+/* RUN: %clang_cc1 -fsyntax-only -verify=none -std=c89 %s
+ */
+/* RUN: %clang_cc1 -fsyntax-only -verify=none -std=c99 %s
+ */
+/* RUN: %clang_cc1 -fsyntax-only -verify=none -x c++ %s
+ */
+/* RUN: %clang_cc1 -fsyntax-only -verify=none -x c++ 
-Wdeclaration-after-statement %s
+ */
+
+/* none-no-diagnostics */
+
+int foo(int i)
+{
+  i += 1;
+  int f = i; /* expected-warning {{ISO C90 forbids mixing declarations and 
code}}*/
+  return f;
+}
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -410,9 +410,10 @@
ArrayRef Elts, bool isStmtExpr) {
   const unsigned NumElts = Elts.size();
 
-  // If we're in C89 mode, check that we don't have any decls after stmts.  If
-  // so, emit an extension diagnostic.
-  if (!getLangOpts().C99 && !getLangOpts().CPlusPlus) {
+  // If we're in C mode, check that we don't have any decls after stmts.  If
+  // so, emit an extension diagnostic in C89 and potentially a warning in later
+  // versions.
+  if (!getLangOpts().CPlusPlus) {
 // Note that __extension__ can be around a decl.
 unsigned i = 0;
 // Skip over all declarations.
@@ -425,7 +426,8 @@
 
 if (i != NumElts) {
   Decl *D = *cast(Elts[i])->decl_begin();
-  Diag(D->getLocation(), diag::ext_mixed_decls_code);
+  Diag(D->getLocation(), !getLangOpts().C99 ? diag::ext_mixed_decls_code
+: diag::warn_mixed_decls_code);
 }
   }
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9863,7 +9863,10 @@
 
 def ext_mixed_decls_code : Extension<
   "ISO C90 forbids mixing declarations and code">,
-  InGroup>;
+  InGroup;
+def warn_mixed_decls_code : Warning<
+  ext_mixed_decls_code.Text>,
+  InGroup, DefaultIgnore;
 
 def err_non_local_variable_decl_in_for : Error<
   "declaration of non-local variable in 'for' loop">;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -241,6 +241,7 @@
 
 def EmptyBody : DiagGroup<"empty-body">;
 def Exceptions : DiagGroup<"exceptions">;
+def DeclarationAfterStatement : DiagGroup<"declaration-after-statement">;
 
 def GNUEmptyInitializer : DiagGroup<"gnu-empty-initializer">;
 def GNUEmptyStruct : DiagGroup<"gnu-empty-struct">;


Index: clang/test/Sema/warn-mixed-decls.c
===
--- /dev/null
+++ clang/test/Sema/warn-mixed-decls.c
@@ -0,0 +1,23 @@
+/* RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -pedantic %s
+ */
+/* RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wdeclaration-after-statement %s
+ */
+
+/* Should not emit diagnostic when not pedantic, not enabled or in C++ Code*/
+/* RUN: %clang_cc1 -fsyntax-only -verify=none -std=c89 %s
+ */
+/* RUN: %clang_cc1 -fsyntax-only -verify=none -std=c99 %s
+ */
+/* RUN: %clang_cc1 -fsyntax-only -verify=none -x c++ %s
+ */
+/* RUN: %clang_cc1 -fsyntax-only -verify=none -x c++ -Wdeclaration-after-statement %s
+ */
+
+/* none-no-diagnostics */
+
+int foo(int i)
+{
+  i += 1;
+  int f = i; /* expected-warning {{ISO C90 forbids mixing declarations and code}}*/
+  return f;
+}
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -410,9 +410,10 @@
ArrayRef Elts, bool isStmtExpr) {
   const unsigned NumElts = Elts.size();
 
-  // If we're in C89 mode, check that we don't have any decls after stmts.  If
-  // so, emit an extension diagnostic.
-  if (!getLangOpts().C99 && !getLangOpts().CPlusPlus) {
+  // If we're in C mode, check that we don't have any decls after stmts.  If
+  // so, emit an extension diagnostic in C89 

[PATCH] D114787: [clang][PR51931] Enable `-Wdeclaration-after-statement` for all C versions

[Markus Böck via Phabricator via cfe-commits]

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



Comment at: clang/test/Sema/warn-mixed-decls.c:1-4
+/* RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -pedantic %s
+ */
+/* RUN: %clang_cc1 -fsyntax-only -verify -std=c99 
-Wdeclaration-after-statement %s
+ */

aaron.ballman wrote:
> I'd also like to see RUN lines for when we expect the diagnostic to not be 
> enabled:
> ```
> /* RUN: %clang_cc1 -fsyntax-only -verify=none -std=c99 %s */
> /* RUN: %clang_cc1 -fsyntax-only -verify=none -x c++ %s */
> /* RUN: %clang_cc1 -fsyntax-only -verify=none -x c++ 
> -Wdeclaration-after-statement %s */
> 
> /* none-no-diagnostics */
> ```
> I should note that the last RUN line will give different behavior between 
> Clang and GCC: https://godbolt.org/z/o1PKo7dhM, but I think that's a more 
> general issue that doesn't need to be addressed in this patch. (We don't have 
> a way to flag a diagnostic as requiring a particular language mode.)
The `*/`  on the next line seems to be necessary so as lit seems to otherwise 
add `*/` to the command line of the `RUN` command. At least this is the case on 
my Windows machine.  


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

https://reviews.llvm.org/D114787

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

[PATCH] D116636: [WIP] Enable `-Wstrict-calls-without-prototype` by default

[Aaron Ballman via Phabricator via cfe-commits]

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

I don't see a justification for enabling this diagnostic by default in C89 
mode. I can imagine users wanting to opt into this behavior for C89, but it 
certainly shouldn't be diagnosing correct code by default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116636

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

[PATCH] D116635: Add warning to detect when calls passing arguments are made to functions without prototypes.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5529
+def warn_call_function_without_prototype : Warning<
+  "calling function %0 with arguments when function has no prototype">, 
InGroup<
+  DiagGroup<"strict-calls-without-prototype">>, DefaultIgnore;

This diagnostic doesn't tell me what's wrong with the code (and in fact, 
there's very possibly nothing wrong with the code whatsoever). Further, why 
does calling a function *with no arguments* matter when it has no prototype? I 
would imagine this should flag any call to a function without a prototype given 
that the function without a prototype may still expect arguments. e.g.,
```
// Header.h
int f();

// Source.c
int f(a) int a; { ... }

// Caller.c
#include "Header.h"

int main() {
  return f();
}
```
I think the diagnostic text should be updated to something more like `cannot 
verify %0 is being called with the correct number or %plural{1:type|:types}1 of 
arguments because it was declared without a prototype` (or something along 
those lines that explains what's wrong with the code).



Comment at: clang/lib/Sema/SemaExpr.cpp:6391
 
+  // Diagnose calls that pass arguments to functions without a prototype
+  if (!LangOpts.CPlusPlus) {





Comment at: clang/test/Sema/warn-calls-without-prototype.c:39
+  return a + b +c;
+}
+

NoQ wrote:
> delcypher wrote:
> > delcypher wrote:
> > > delcypher wrote:
> > > > @NoQ Any ideas about this?  It seems kind of weird that when merging 
> > > > `not_a_prototype3` prototype with the K&R style definition of 
> > > > `not_a_prototype3` that the resulting FunctionDecl we see at the call 
> > > > site in `call_to_function_without_prototype3` is marked as not having a 
> > > > prototype.
> > > > 
> > > > If I flip the order (see `not_a_prototype6`) then the merged 
> > > > declaration is marked as having a prototype.
> > > > 
> > > > I'm not sure if this is a bug in `Sema::MergeFunctionDecl` or if this 
> > > > just a peculiarity of K&R style function definitions.
> > > I suspect the problem might be here in `Sema::MergeFunctionDecl`.
> > > 
> > > ```lang=c++
> > >// C: Function types need to be compatible, not identical. This handles
> > >   // duplicate function decls like "void f(int); void f(enum X);" 
> > > properly.
> > >   if (!getLangOpts().CPlusPlus &&
> > >   Context.typesAreCompatible(OldQType, NewQType)) {
> > > const FunctionType *OldFuncType = OldQType->getAs();
> > > const FunctionType *NewFuncType = NewQType->getAs();
> > > const FunctionProtoType *OldProto = nullptr;
> > > if (MergeTypeWithOld && isa(NewFuncType) &&
> > > (OldProto = dyn_cast(OldFuncType))) {
> > >   // The old declaration provided a function prototype, but the
> > >   // new declaration does not. Merge in the prototype.
> > > ```
> > > 
> > > ` isa(NewFuncType)` is false in this particular 
> > > case, however `New` doesn't have a prototype (i.e. `New->hasPrototype()` 
> > > is false). One fix might be to replace 
> > > `isa(NewFuncType)` with 
> > > 
> > > ```
> > > (isa(NewFuncType) || !New->hasPrototype())
> > > ```
> > > 
> > > However, I don't really know this code well enough to know if that's the 
> > > right fix.
> > Okay. I think the above would actually be the wrong location for a fix 
> > because in this case we don't need to go down the path that synthesizes the 
> > parameters because we already know them for both `old` and `new` in this 
> > situation.
> > 
> > Instead I think the change would have to be in 
> > `Sema::MergeCompatibleFunctionDecls` to do something like.
> > 
> > ```lang=c++
> >   // If New is a K&R function definition it will be marked
> >   // as not having a prototype. If `Old` has a prototype
> >   // then to "merge" we should mark the K&R function as having a prototype.
> >   if (!getLangOpts().CPlusPlus && Old->hasPrototype() && 
> > !New->hasPrototype())
> > New->setHasInheritedPrototype(); 
> > ```
> > 
> > What I'm not sure about is if this is semantically the right thing to do. 
> > Thoughts?
> Ok dunno but I definitely find this whole thing surprising. I'd expect this 
> example to be the entirely normal situation for this code, where it sees that 
> the new declaration has no prototype so it "inherits" it from the old 
> declaration. But you're saying that
> 
> > `isa(NewFuncType)` is false in this particular case
> 
> Where does that proto-type come from then? I only see this code affecting the 
> type
> ```lang=c++
>3523   if (RequiresAdjustment) {
>3524 const FunctionType *AdjustedType = 
> New->getType()->getAs();
>3525 AdjustedType = Context.adjustFunctionType(AdjustedType, 
> NewTypeInfo);
>3526 New->setType(QualType(AdjustedType, 0));
>3527 NewQType = Context.getCanonicalType(New->getType());
>3528   }
> ```
> which doesn't seem to touch no-proto-types:
> 

[PATCH] D116329: [clang-check] Adjust argument adjusters for clang-check to strip options blocking the static analyzer

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang/tools/clang-check/ClangCheck.cpp:217
+if (!AnalyzerOutput.empty()) {
+  Tool.appendArgumentsAdjuster(getInsertArgumentAdjuster("-o"));
+  Tool.appendArgumentsAdjuster(

you can pass a vector to getInsertArgumentsAdjuster



Comment at: clang/tools/clang-check/ClangCheck.cpp:225
+//
+// The syntax-only adjuster can also help us to remove other options that
+// trigger output generation, e.g. -save-temps. Besides, to enable the

You've removed the mention of the default syntax-only adjuster, so this comment 
seems confusing.

"The syntax-only adjuster is installed by default.
Good: It strips options that trigger extra output, like --save-temps.
Bad: We don't want the -fsyntax-only arg itself, as it suppresses analyzer 
output"

Then strip -fsyntax-only, then add -analyze?



Comment at: clang/tools/clang-check/ClangCheck.cpp:236
+  AdjustedArgs.emplace_back(Arg);
+} else if (!HasAnalyze) {
+  AdjustedArgs.emplace_back("--analyze");

this seems like a particularly confusing way to say "if (Arg == "-fsyntax-only 
&& !HasAnalyze)"

The "replace first -fsyntax-only with -analyze, strip others" seems more 
clearly expressed as "strip -fsyntax-only" and "add -analyze" as separate steps.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116329

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

[PATCH] D116378: [clang-tidy] Disable clang-tidy warnings from system macros

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp updated this revision to Diff 397859.
carlosgalvezp added a comment.

Revert formatting, clarify necessary changes to existing check in the commit 
message.


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

https://reviews.llvm.org/D116378

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/file-filter/system/system-header.h
  clang-tools-extra/test/clang-tidy/infrastructure/file-filter.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/file-filter.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/file-filter.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/file-filter.cpp
@@ -9,6 +9,8 @@
 //   file-filter\header*.h due to code order between '/' and '\\'.
 // RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -system-headers %s -- -I %S/Inputs/file-filter/system/.. -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK4 %s
 // RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -system-headers -quiet %s -- -I %S/Inputs/file-filter/system/.. -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK4-QUIET %s
+// RUN: clang-tidy -checks='-*,cppcoreguidelines-pro-type-cstyle-cast' -header-filter='.*' -system-headers %s -- -I %S/Inputs/file-filter/system/.. -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK5 %s
+// RUN: clang-tidy -checks='-*,cppcoreguidelines-pro-type-cstyle-cast' -header-filter='.*' %s -- -I %S/Inputs/file-filter/system/.. -isystem %S/Inputs/file-filter/system 2>&1 | FileCheck --check-prefix=CHECK5-NO-SYSTEM-HEADERS %s
 
 #include "header1.h"
 // CHECK-NOT: warning:
@@ -71,3 +73,8 @@
 // CHECK4-NOT: Suppressed {{.*}} warnings
 // CHECK4-NOT: Use -header-filter=.* {{.*}}
 // CHECK4-QUIET-NOT: Suppressed
+
+int x = 123;
+auto x_ptr = TO_FLOAT_PTR(&x);
+// CHECK5: :[[@LINE-1]]:14: warning: do not use C-style cast to convert between unrelated types
+// CHECK5-NO-SYSTEM-HEADERS-NOT: :[[@LINE-2]]:14: warning: do not use C-style cast to convert between unrelated types
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/file-filter/system/system-header.h
===
--- clang-tools-extra/test/clang-tidy/infrastructure/Inputs/file-filter/system/system-header.h
+++ clang-tools-extra/test/clang-tidy/infrastructure/Inputs/file-filter/system/system-header.h
@@ -1 +1,3 @@
 class A0 { A0(int); };
+
+#define TO_FLOAT_PTR(x) ((float *)(x))
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -67,10 +67,13 @@
 Improvements to clang-tidy
 --
 
-- Added support for globbing in `NOLINT*` expressions, to simplify suppressing
+- Ignore warnings from macros defined in system headers, if not using the
+  ``-system-headers`` flag.
+
+- Added support for globbing in ``NOLINT*`` expressions, to simplify suppressing
   multiple warnings in the same line.
 
-- Added support for `NOLINTBEGIN` ... `NOLINTEND` comments to suppress
+- Added support for ``NOLINTBEGIN`` ... ``NOLINTEND`` comments to suppress
   Clang-Tidy warnings over multiple lines.
 
 - Generalized the `modernize-use-default-member-init` check to handle non-default
@@ -151,7 +154,7 @@
 
 - Fixed a false positive in :doc:`fuchsia-trailing-return
   ` for C++17 deduction guides.
-  
+
 - Fixed a false positive in :doc:`bugprone-throw-keyword-missing
   ` when creating an exception object
   using placement new
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.h
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.h
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.h
@@ -28,6 +28,8 @@
 return LangOpts.CPlusPlus;
   }
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 };
 
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp
@@ -11,12 +11,33 @@
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMat

[PATCH] D116378: [clang-tidy] Disable clang-tidy warnings from system macros

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp:23-40
+namespace {
+class VaArgPPCallbacks : public PPCallbacks {
+public:
+  VaArgPPCallbacks(ProTypeVarargCheck *Check) : Check(Check) {}
+
+  void MacroExpands(const Token &MacroNameTok, const MacroDefinition &MD,
+SourceRange Range, const MacroArgs *Args) override {

carlosgalvezp wrote:
> salman-javed-nz wrote:
> > Should this be in another patch?
> > 
> > Line 722 in ClangTidyDiagnosticConsumer.cpp makes it so that clang-tidy 
> > filters warnings from system macros. This would benefit all checks.
> > 
> > This VaArgPPCallBack change here only benefits 
> > `cppcoreguidelines-pro-type-vararg`.
> The change to `ClangTidyDiagnosticConsumer.cpp` broke this check, because it 
> was checking for the use of `vararg` indirectly, checking for the use of 
> `__builtin_vararg`, which was expanded from the system macro `vararg`. This 
> patch updates the code so it checks directly what the rule is about "do not 
> use `vararg`".  I should perhaps have been more clear about that in the 
> commit message :) 
Messed up names above, I meant `va_arg` and `__builtin_va_arg`.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp:46-78
+"__builtin_isgreater",
+"__builtin_isgreaterequal",
 "__builtin_isless",
-"__builtin_islessequal", 
-"__builtin_islessgreater", 
+"__builtin_islessequal",
+"__builtin_islessgreater",
 "__builtin_isunordered",
+"__builtin_fpclassify",

carlosgalvezp wrote:
> salman-javed-nz wrote:
> > Formatting changed for code not directly involved in this patch. Should be 
> > moved to a separate NFC commit that you don't have to run by us.
> Yes, it's a bit unfortunate, this was an automatic change on save from my 
> editor. Is it really worth it to revert this change though? I will need to 
> disable this feature on my IDE, revert each line manually by adding a space, 
> push, then re-enable the feature. Sounds like a lot of time spent on 
> reverting a change that anyway improves the state of the code.
Fixed.


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

https://reviews.llvm.org/D116378

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

[PATCH] D114787: [clang][PR51931] Enable `-Wdeclaration-after-statement` for all C versions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9865
 def ext_mixed_decls_code : Extension<
   "ISO C90 forbids mixing declarations and code">,
+  InGroup;

In the other review, I left a comment about the diagnostic text: 
https://reviews.llvm.org/D115094#3176985

Since we're cleaning this up, I think we should reword this diagnostic so it 
follows newer conventions. I think the extension diagnostic should read: 
`mixing declarations and code is a C99 extension` and the default ignore 
warning should read `mixing declarations and code is incompatible with 
standards before C99`. (This also helpfully removes the `ISO C90` wording, 
which is confused about the name of the standard.)

Typically, we'd put the default ignore warning under a new `CPre99Compat` 
diagnostic group (spelled `pre-c99-compat`) as we do with other precompat 
diagnostics, but the goal here is to match GCC's behavior and so the existing 
warning group seems fine to me (I don't think we want warnings in multiple 
groups, but that's possibly an option if it matters in the future).



Comment at: clang/test/Sema/warn-mixed-decls.c:1-4
+/* RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -pedantic %s
+ */
+/* RUN: %clang_cc1 -fsyntax-only -verify -std=c99 
-Wdeclaration-after-statement %s
+ */

zero9178 wrote:
> aaron.ballman wrote:
> > I'd also like to see RUN lines for when we expect the diagnostic to not be 
> > enabled:
> > ```
> > /* RUN: %clang_cc1 -fsyntax-only -verify=none -std=c99 %s */
> > /* RUN: %clang_cc1 -fsyntax-only -verify=none -x c++ %s */
> > /* RUN: %clang_cc1 -fsyntax-only -verify=none -x c++ 
> > -Wdeclaration-after-statement %s */
> > 
> > /* none-no-diagnostics */
> > ```
> > I should note that the last RUN line will give different behavior between 
> > Clang and GCC: https://godbolt.org/z/o1PKo7dhM, but I think that's a more 
> > general issue that doesn't need to be addressed in this patch. (We don't 
> > have a way to flag a diagnostic as requiring a particular language mode.)
> The `*/`  on the next line seems to be necessary so as lit seems to otherwise 
> add `*/` to the command line of the `RUN` command. At least this is the case 
> on my Windows machine.  
Huh, today I learned. :-D Thanks for letting me know!


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

https://reviews.llvm.org/D114787

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

[PATCH] D116726: [clang-format] Fix a crash (assertion) in qualifier alignment when matching template closer is null

[Marek Kurdej via Phabricator via cfe-commits]

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

LGTM! Thanks :).


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

https://reviews.llvm.org/D116726

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

[PATCH] D116726: [clang-format] Fix a crash (assertion) in qualifier alignment when matching template closer is null

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Just a last thought, you can maybe minimize some of the test cases.
It seems that even:

  using X = Y /**/ <>;

reproduces the issue.


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

https://reviews.llvm.org/D116726

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

[PATCH] D114425: [clang] Add __builtin_bswap128

[Nikolas Klauser via Phabricator via cfe-commits]

philnik updated this revision to Diff 397866.
philnik marked 3 inline comments as done.
philnik added a comment.

- Addressed comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114425

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Builtins.def
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins.cpp
  clang/test/Sema/constant-builtins-2.c


Index: clang/test/Sema/constant-builtins-2.c
===
--- clang/test/Sema/constant-builtins-2.c
+++ clang/test/Sema/constant-builtins-2.c
@@ -216,6 +216,9 @@
 int h3 = __builtin_bswap16(0x1234) == 0x3412 ? 1 : f();
 int h4 = __builtin_bswap32(0x1234) == 0x3412 ? 1 : f();
 int h5 = __builtin_bswap64(0x1234) == 0x3412 ? 1 : f();
+#if defined(__SIZEOF_INT128__)
+int h6 = __builtin_bswap128(0x1234) == (((__int128)0x3412) << 112) ? 1 : f();
+#endif
 extern long int bi0;
 extern __typeof__(__builtin_expect(0, 0)) bi0;
 
Index: clang/test/CodeGen/builtins.cpp
===
--- clang/test/CodeGen/builtins.cpp
+++ clang/test/CodeGen/builtins.cpp
@@ -20,6 +20,10 @@
 decltype(__builtin_bswap32(0)) bswap32 = 42;
 extern uint64_t bswap64;
 decltype(__builtin_bswap64(0)) bswap64 = 42;
+#ifdef __SIZEOF_INT128__
+extern __uint128_t bswap128;
+decltype(__builtin_bswap128(0)) bswap128 = 42;
+#endif
 
 #ifdef __clang__
 extern uint8_t bitrev8;
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -2925,7 +2925,10 @@
   }
   case Builtin::BI__builtin_bswap16:
   case Builtin::BI__builtin_bswap32:
-  case Builtin::BI__builtin_bswap64: {
+  case Builtin::BI__builtin_bswap64:
+  case Builtin::BI__builtin_bswap128: {
+if (!Target.hasInt128Type() && BuiltinIDIfNoAsmLabel == 
Builtin::BI__builtin_bswap128)
+  CGM.ErrorUnsupported(E, "__builtin_bswap128");
 return RValue::get(emitUnaryBuiltin(*this, E, Intrinsic::bswap));
   }
   case Builtin::BI__builtin_bitreverse8:
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -11751,7 +11751,8 @@
 
   case Builtin::BI__builtin_bswap16:
   case Builtin::BI__builtin_bswap32:
-  case Builtin::BI__builtin_bswap64: {
+  case Builtin::BI__builtin_bswap64:
+  case Builtin::BI__builtin_bswap128: {
 APSInt Val;
 if (!EvaluateInteger(E->getArg(0), Val, Info))
   return false;
Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -512,6 +512,7 @@
 BUILTIN(__builtin_bswap16, "UsUs", "nc")
 BUILTIN(__builtin_bswap32, "UZiUZi", "nc")
 BUILTIN(__builtin_bswap64, "UWiUWi", "nc")
+BUILTIN(__builtin_bswap128, "ULLLiULLLi", "nc")
 
 BUILTIN(__builtin_bitreverse8, "UcUc", "nc")
 BUILTIN(__builtin_bitreverse16, "UsUs", "nc")
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -67,6 +67,8 @@
   the base path of the current config file. See :ref:`configuration-files` for
   details.
 
+- The builtin ``__builtin_bswap128`` was added.
+
 New Compiler Flags
 --
 


Index: clang/test/Sema/constant-builtins-2.c
===
--- clang/test/Sema/constant-builtins-2.c
+++ clang/test/Sema/constant-builtins-2.c
@@ -216,6 +216,9 @@
 int h3 = __builtin_bswap16(0x1234) == 0x3412 ? 1 : f();
 int h4 = __builtin_bswap32(0x1234) == 0x3412 ? 1 : f();
 int h5 = __builtin_bswap64(0x1234) == 0x3412 ? 1 : f();
+#if defined(__SIZEOF_INT128__)
+int h6 = __builtin_bswap128(0x1234) == (((__int128)0x3412) << 112) ? 1 : f();
+#endif
 extern long int bi0;
 extern __typeof__(__builtin_expect(0, 0)) bi0;
 
Index: clang/test/CodeGen/builtins.cpp
===
--- clang/test/CodeGen/builtins.cpp
+++ clang/test/CodeGen/builtins.cpp
@@ -20,6 +20,10 @@
 decltype(__builtin_bswap32(0)) bswap32 = 42;
 extern uint64_t bswap64;
 decltype(__builtin_bswap64(0)) bswap64 = 42;
+#ifdef __SIZEOF_INT128__
+extern __uint128_t bswap128;
+decltype(__builtin_bswap128(0)) bswap128 = 42;
+#endif
 
 #ifdef __clang__
 extern uint8_t bitrev8;
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -2925,7 +2925,10 @@
   }
   case Builtin::BI__builtin_bswap16:
   case Builtin::BI__builtin_bswap32:
-  case Builtin::BI__builtin_bswap64: {
+  case Builtin::BI_

[PATCH] D116638: [clang-format] Fix ignoring JavaScriptWrapImport when ColumnWidth: 0

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

> (1) very few people use... I don't think it's actually the behavior people 
> will want

Subjective or Objective opinion?

https://github.com/search?o=desc&q=%22ColumnLimit%3A+0%22&s=indexed&type=Code

95,000+ occurrences of "ColumnLimit" in github YAML files and multiple 
`ColumnLimit: 0` even on the first page  and https://www.hyrumslaw.com/ 
suggests we shouldn't make that kind of assumption

From my recollection @mprobst is the original author of this and they are on 
this review as a reviewer so I tend to bow to their better judgement. its 
possible we never thought about the ColumnLimit: 0 case originally.


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

https://reviews.llvm.org/D116638

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

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D55134#3224339 , @ychen wrote:

> FWIW, ASTUnit seems to save an effective triple while the current AST uses 
> the nominal triple. (for example, `armv7a-xx-xx-eabihf` vs 
> `armv7a-xx-xx-eabi`). I'm not sure if there is a good solution other than 
> using heuristics to allow some differences.

@ychen, thanks for the notice! I hope we'll have time to address the issue soon.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55134

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

[PATCH] D116638: [clang-format] Fix ignoring JavaScriptWrapImport when ColumnWidth: 0

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/include/clang/Format/Format.h:2620
+  ///
+  /// * If ``ColumnWidth`` is 0 (no limit on the number of columns),
+  ///   then import statements will keep the number of lines they

Please change all occurrences of `ColumnWidth` (old name) to `ColumnLimit`.



Comment at: clang/unittests/Format/FormatTestJS.cpp:1990
   Style.ColumnLimit = 40;
-  // Using this version of verifyFormat because test::messUp hides the issue.
+  Style.JavaScriptWrapImports = true;
   verifyFormat("import {\n"

andmis wrote:
> mprobst wrote:
> > curdeius wrote:
> > > It's already true, cf. line 1977. Remove this line.
> > FWIW, I think the tests might be more readable if each configuration 
> > ({true, false} x {col: 0, col: 40}) explicitly set all the options, even if 
> > it's a bit redundant.
> I've reformatted them in two big blocks: one where `ColumnLimit > 0` and one 
> where `ColumnLimit: 0`. So there are no more redundant option set statements.
> 
> This is the most-readable way to format the tests IMO because when 
> `ColumnLimit > 0`, we can use the simple `verifyFormat` (where you just pass 
> the expected result and it messes it up, formats, and checks) because 
> clang-format will do a full reflow. But for the `ColumnLimit: 0` tests, we 
> need to use the other `verifyFormat` (where we have input and expected output 
> explicitly), and it's convenient to put a comment explaining what's happening 
> at the top of the `ColumnLimit: 0` block.
It's ok for me too, but I'd like to either put all the involved options each 
time, or minimalistic changes, but not a mix of the two.


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

https://reviews.llvm.org/D116638

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

[PATCH] D116513: [clang-tidy] Fix bugs in misc-unused-parameters for Constructors calls site

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp:157-159
 // CHECK-FIXES: C() {}
   C(int i) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:9: warning

mehdi_amini wrote:
> aaron.ballman wrote:
> > I think this fix is incorrect and should not be applied; it changes the 
> > meaning of the interface from having a converting constructor to having a 
> > default constructor. I think that needs to be optional behavior because 
> > it's a pretty invasive change to apply automatically. This patch builds on 
> > top of the existing incorrect behavior, but would be fine if it was only 
> > applied when the option to modify constructors is enabled.
> I'm not against an option, but I'd like to get to a default behavior that is 
> "safe". So I guess my first patch should be to undo the transformation 
> happening here in the test already.
> We can either never touch any C++ constructor or try to find a conservative 
> logic for it.
> I mentioned in the other review that we may avoid touching a Ctor with a 
> single parameter. 
> 
> Then we also can't do it for a Ctor with two parameters as it'll turn it into 
> a converting ctor. Unless you can eliminate both parameters, in which case it 
> is become a default ctor (which can conflict with an existing one, in which 
> case it can be just deleted?)
> 
> I'm not against an option, but I'd like to get to a default behavior that is 
> "safe".

Definitely agreed!

> So I guess my first patch should be to undo the transformation happening here 
> in the test already.

I think that's a good approach.

> We can either never touch any C++ constructor or try to find a conservative 
> logic for it.

Initially, I'd say let's not touch any C++ constructors. We may be able to find 
conservative logic for it, but that'll take time to get right.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116513

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

[PATCH] D116638: [clang-format] Fix ignoring JavaScriptWrapImport when ColumnWidth: 0

[Andrey Mishchenko via Phabricator via cfe-commits]

andmis added a comment.

My guess that `ColumnLimit: 0` is rarely used for JS is based on the objective 
fact that JS import formatting is (IMO very) buggy with the column limit set 
that way, and it took several years for us to hear a bug report about it. And 
"we should not make assumptions about what people will want" applies just as 
well to the proposal of force-wrapping at >= 2 imports.

I agree that `AlwaysWrap`/`NeverWrap`/`LeaveExistingWraps` is clearer and 
better than `true`/`false`. But we still have no answer for the semantics of 
`LeaveExistingWraps` in edge cases. Are we saying that we should not merge this 
bugfix without figuring out the answer to the semantics question and/or 
changing the option to an enum?


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

https://reviews.llvm.org/D116638

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

[PATCH] D116512: [clang-tidy] Limit non-Strict mode to public functions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc-unused-parameters.rst:43
+   a human reader, and there's basically no place for a bug to hide. On the 
other
+   hand for non-public functions, all the call-sites are visible and the 
parameter
+   can be eliminated entirely.

mehdi_amini wrote:
> aaron.ballman wrote:
> > Call sites are not always visible for protected functions, so this seems a 
> > bit suspicious. The changes are missing test coverage for that situation.
> You're using `public` for "access control" while I was using the linkage 
> aspect: my reasoning is that if a method isn't "externally visible" from the 
> current translation unit, you see all the call-sites. This is orthogonal to 
> public/private/protected as far as I know.
> 
> I am likely missing a check for "isDefinedInMainFile" (or whatever the api is 
> called) to not flag included headers.
> You're using public for "access control" while I was using the linkage 
> aspect: my reasoning is that if a method isn't "externally visible" from the 
> current translation unit, you see all the call-sites.

Oh, thanks for pointing out that I was confused! I'm not used to "public" when 
describing linkage, usually that's "external" or "externally visible". Any 
appetite for rewording in those terms? Something like "On the other hand, for 
functions with internal linkage, all the call sites are visible and parameters 
can be safely removed."



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp:157-159
+// CHECK-FIXES: C() {}
+  C(int i) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning

mehdi_amini wrote:
> aaron.ballman wrote:
> > I think this demonstrates a bad fix -- this changes code meaning from being 
> > a converting constructor to being a default constructor, which are very 
> > different things.
> Oh you're right: so we can't do it for a Ctor with a single parameter...
> 
> But we also can't do it for a Ctor with two parameters as it'll turn it into 
> a converting ctor. Unless you can eliminate both parameters, in which case it 
> is become a default ctor (which can conflict with an existing one, in which 
> case it can be just deleted?)
Yeah, I think we need to not transform ctors at all currently.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116512

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

[PATCH] D113049: [AIX] Disable tests that fail because of no 64-bit XCOFF object file support

[Jake Egan via Phabricator via cfe-commits]

Jake-Egan added inline comments.



Comment at: clang/test/ASTMerge/codegen-body/test.c:1
+// UNSUPPORTED: powerpc64-ibm-aix
 // RUN: %clang_cc1 -emit-pch -o %t.1.ast %S/Inputs/body1.c

Jake-Egan wrote:
> shchenz wrote:
> > Jake-Egan wrote:
> > > shchenz wrote:
> > > > Do we still need this? -emit-obj is already excluded?
> > > Excluding this test causes regressions because LIT can't find any tests 
> > > at `/codegen-body` directory:
> > > ```
> > > Clang :: utils/update_cc_test_checks/global-hex-value-regex.test
> > > Clang :: utils/update_cc_test_checks/global-value-regex.test
> > > ```
> > > I figure it's easier to mark this test unsupported. 
> > Is it possible to add `expected-no-diagnostics` for any of the above two 
> > RUN lines? So we will have a test point and be able to exclude this case?
> Unfortunately adding `expected-no-diagnostics` fix the issue. The existing 
> `expected-no-diagnostics` is already applied to all the RUN lines.
> fix the issue
I meant *doesn't* fix the issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113049

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

[PATCH] D116256: [-fms-extensions] Make some exception specification warnings/errors compatible with what cl.exe does

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang/test/SemaCXX/MicrosoftCompatibility.cpp:4
 // RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -std=c++11 
-Wmicrosoft -verify -fms-compatibility -fexceptions -fcxx-exceptions 
-fms-compatibility-version=19.00
 // RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -std=c++11 
-Wmicrosoft -verify -fms-compatibility -fexceptions -fcxx-exceptions 
-fms-compatibility-version=18.00
 

Could you add a run line with `-std=c++17`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116256

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

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

[Vaibhav Yenamandra via Phabricator via cfe-commits]

vaibhav.y added a comment.

ping: Requesting review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109701

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

[PATCH] D115183: [clang][HeaderSearch] Support framework includes in suggestPath...

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 397882.
dgoldman added a comment.

Improve double-quotes.m test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115183

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Modules/double-quotes.m
  clang/unittests/Lex/HeaderSearchTest.cpp

Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -47,6 +47,15 @@
 Search.AddSearchPath(DL, /*isAngled=*/false);
   }
 
+  void addSystemFrameworkSearchDir(llvm::StringRef Dir) {
+VFS->addFile(Dir, 0, llvm::MemoryBuffer::getMemBuffer(""), /*User=*/None,
+ /*Group=*/None, llvm::sys::fs::file_type::directory_file);
+auto DE = FileMgr.getOptionalDirectoryRef(Dir);
+assert(DE);
+auto DL = DirectoryLookup(*DE, SrcMgr::C_System, /*isFramework=*/true);
+Search.AddSystemSearchPath(DL);
+  }
+
   void addHeaderMap(llvm::StringRef Filename,
 std::unique_ptr Buf,
 bool isAngled = false) {
@@ -155,6 +164,44 @@
 "y/z/t.h");
 }
 
+TEST_F(HeaderSearchTest, SdkFramework) {
+  addSystemFrameworkSearchDir(
+  "/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk/Frameworks/");
+  bool IsSystem = false;
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics(
+"/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/"
+"Frameworks/AppKit.framework/Headers/NSView.h",
+/*WorkingDir=*/"",
+/*MainFile=*/"", &IsSystem),
+"AppKit/AppKit.h");
+  EXPECT_TRUE(IsSystem);
+}
+
+TEST_F(HeaderSearchTest, SdkIntraFramework) {
+  addSystemFrameworkSearchDir(
+  "/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk/Frameworks/");
+  bool IsSystem = false;
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics(
+"/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/"
+"Frameworks/AppKit.framework/Headers/NSView.h",
+/*WorkingDir=*/"",
+"/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/"
+"Frameworks/AppKit.framework/Headers/NSViewController.h",
+&IsSystem),
+"AppKit/NSView.h");
+  EXPECT_TRUE(IsSystem);
+}
+
+TEST_F(HeaderSearchTest, NestedFramework) {
+  addSystemFrameworkSearchDir("/Platforms/MacOSX/Frameworks");
+  EXPECT_EQ(Search.suggestPathToFileForDiagnostics(
+"/Platforms/MacOSX/Frameworks/AppKit.framework/Frameworks/"
+"Sub.framework/Headers/Sub.h",
+/*WorkingDir=*/"",
+/*MainFile=*/""),
+"Sub/Sub.h");
+}
+
 // Helper struct with null terminator character to make MemoryBuffer happy.
 template 
 struct NullTerminatedFile : public FileTy {
Index: clang/test/Modules/double-quotes.m
===
--- clang/test/Modules/double-quotes.m
+++ clang/test/Modules/double-quotes.m
@@ -24,8 +24,17 @@
 // because they only show up under the module A building context.
 // RUN: FileCheck --input-file=%t/stderr %s
 // CHECK: double-quoted include "A0.h" in framework header, expected angle-bracketed instead
+// CHECK: #include "A0.h"
+// CHECK:  ^~
+// CHECK: 
 // CHECK: double-quoted include "B.h" in framework header, expected angle-bracketed instead
+// CHECK: #include "B.h"
+// CHECK:  ^
+// CHECK: 
 // CHECK: double-quoted include "B.h" in framework header, expected angle-bracketed instead
+// CHECK: #import "B.h" // Included from Z.h & A.h
+// CHECK: ^
+// CHECK: 
 
 #import "A.h"
 #import 
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -721,7 +721,8 @@
 }
 
 static bool isFrameworkStylePath(StringRef Path, bool &IsPrivateHeader,
- SmallVectorImpl &FrameworkName) {
+ SmallVectorImpl &FrameworkName,
+ SmallVectorImpl &IncludeSpelling) {
   using namespace llvm::sys;
   path::const_iterator I = path::begin(Path);
   path::const_iterator E = path::end(Path);
@@ -737,15 +738,22 @@
   // and some other variations among these lines.
   int FoundComp = 0;
   while (I != E) {
-if (*I == "Headers")
+if (*I == "Headers") {
   ++FoundComp;
-if (I->endswith(".framework")) {
-  FrameworkName.append(I->begin(), I->end());
-  ++FoundComp;
-}
-if (*I == "PrivateHeaders") {
+} else if (*I == "PrivateHeaders") {
   ++FoundComp;
   IsPrivateHeader = true;
+} else if (I->endswith(".framework")) {
+  StringRef Name = I->drop_back(10); // Drop .framework
+  // Need to reset the strings and 

[PATCH] D116673: [Clang][NVPTX]Add NVPTX intrinsics and builtins for CUDA PTX cvt sm80 instructions

[Jack Kirk via Phabricator via cfe-commits]

JackAKirk updated this revision to Diff 397884.
JackAKirk added a comment.

Made suggested change to naming convention.

Added a few missing lines from the original patch.


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

https://reviews.llvm.org/D116673

Files:
  clang/include/clang/Basic/BuiltinsNVPTX.def
  clang/test/CodeGen/builtins-nvptx.c
  llvm/include/llvm/IR/IntrinsicsNVVM.td
  llvm/lib/Target/NVPTX/MCTargetDesc/NVPTXInstPrinter.cpp
  llvm/lib/Target/NVPTX/NVPTX.h
  llvm/lib/Target/NVPTX/NVPTXInstrInfo.td
  llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
  llvm/test/CodeGen/NVPTX/convert-sm80.ll

Index: llvm/test/CodeGen/NVPTX/convert-sm80.ll
===
--- /dev/null
+++ llvm/test/CodeGen/NVPTX/convert-sm80.ll
@@ -0,0 +1,136 @@
+; RUN: llc < %s -march=nvptx64 -mcpu=sm_80 -mattr=+ptx70 | FileCheck %s
+
+
+; CHECK-LABEL: cvt_rn_bf16x2_f32
+define i32 @cvt_rn_bf16x2_f32(float %f1, float %f2) {
+
+; CHECK: cvt.rn.bf16x2.f32
+  %val = call i32 @llvm.nvvm.ff2bf16x2.rn(float %f1, float %f2);
+
+ret i32 %val
+}
+
+; CHECK-LABEL: cvt_rn_relu_bf16x2_f32
+define i32 @cvt_rn_relu_bf16x2_f32(float %f1, float %f2) {
+
+; CHECK: cvt.rn.relu.bf16x2.f32
+%val = call i32 @llvm.nvvm.ff2bf16x2.rn.relu(float %f1, float %f2);
+
+ret i32 %val
+}
+
+; CHECK-LABEL: cvt_rz_bf16x2_f32
+define i32 @cvt_rz_bf16x2_f32(float %f1, float %f2) {
+
+; CHECK: cvt.rz.bf16x2.f32
+  %val = call i32 @llvm.nvvm.ff2bf16x2.rz(float %f1, float %f2);
+
+ret i32 %val
+}
+
+; CHECK-LABEL: cvt_rz_relu_bf16x2_f32
+define i32 @cvt_rz_relu_bf16x2_f32(float %f1, float %f2) {
+
+; CHECK: cvt.rz.relu.bf16x2.f32
+%val = call i32 @llvm.nvvm.ff2bf16x2.rz.relu(float %f1, float %f2);
+
+ret i32 %val
+}
+
+declare i32 @llvm.nvvm.ff2bf16x2.rn(float, float)
+declare i32 @llvm.nvvm.ff2bf16x2.rn.relu(float, float)
+declare i32 @llvm.nvvm.ff2bf16x2.rz(float, float)
+declare i32 @llvm.nvvm.ff2bf16x2.rz.relu(float, float)
+
+; CHECK-LABEL: cvt_rn_f16x2_f32
+define <2 x half> @cvt_rn_f16x2_f32(float %f1, float %f2) {
+
+; CHECK: cvt.rn.f16x2.f32
+  %val = call <2 x half> @llvm.nvvm.ff2f16x2.rn(float %f1, float %f2);
+
+ret <2 x half> %val
+}
+
+; CHECK-LABEL: cvt_rn_relu_f16x2_f32
+define <2 x half> @cvt_rn_relu_f16x2_f32(float %f1, float %f2) {
+
+; CHECK: cvt.rn.relu.f16x2.f32
+%val = call <2 x half> @llvm.nvvm.ff2f16x2.rn.relu(float %f1, float %f2);
+
+ret <2 x half> %val
+}
+
+; CHECK-LABEL: cvt_rz_f16x2_f32
+define <2 x half> @cvt_rz_f16x2_f32(float %f1, float %f2) {
+
+; CHECK: cvt.rz.f16x2.f32
+  %val = call <2 x half> @llvm.nvvm.ff2f16x2.rz(float %f1, float %f2);
+
+ret <2 x half> %val
+}
+
+; CHECK-LABEL: cvt_rz_relu_f16x2_f32
+define <2 x half> @cvt_rz_relu_f16x2_f32(float %f1, float %f2) {
+
+; CHECK: cvt.rz.relu.f16x2.f32
+%val = call <2 x half> @llvm.nvvm.ff2f16x2.rz.relu(float %f1, float %f2);
+
+ret <2 x half> %val
+}
+
+declare <2 x half> @llvm.nvvm.ff2f16x2.rn(float, float)
+declare <2 x half> @llvm.nvvm.ff2f16x2.rn.relu(float, float)
+declare <2 x half> @llvm.nvvm.ff2f16x2.rz(float, float)
+declare <2 x half> @llvm.nvvm.ff2f16x2.rz.relu(float, float)
+
+; CHECK-LABEL: cvt_rn_bf16_f32
+define i16 @cvt_rn_bf16_f32(float %f1) {
+
+; CHECK: cvt.rn.bf16.f32
+  %val = call i16 @llvm.nvvm.f2bf16.rn(float %f1);
+
+ret i16 %val
+}
+
+; CHECK-LABEL: cvt_rn_relu_bf16_f32
+define i16 @cvt_rn_relu_bf16_f32(float %f1) {
+
+; CHECK: cvt.rn.relu.bf16.f32
+%val = call i16 @llvm.nvvm.f2bf16.rn.relu(float %f1);
+
+ret i16 %val
+}
+
+; CHECK-LABEL: cvt_rz_bf16_f32
+define i16 @cvt_rz_bf16_f32(float %f1) {
+
+; CHECK: cvt.rz.bf16.f32
+  %val = call i16 @llvm.nvvm.f2bf16.rz(float %f1);
+
+ret i16 %val
+}
+
+; CHECK-LABEL: cvt_rz_relu_bf16_f32
+define i16 @cvt_rz_relu_bf16_f32(float %f1) {
+
+; CHECK: cvt.rz.relu.bf16.f32
+%val = call i16 @llvm.nvvm.f2bf16.rz.relu(float %f1);
+
+ret i16 %val
+}
+
+declare i16 @llvm.nvvm.f2bf16.rn(float)
+declare i16 @llvm.nvvm.f2bf16.rn.relu(float)
+declare i16 @llvm.nvvm.f2bf16.rz(float)
+declare i16 @llvm.nvvm.f2bf16.rz.relu(float)
+
+; CHECK-LABEL: cvt_rna_tf32_f32
+define i32 @cvt_rna_tf32_f32(float %f1) {
+
+; CHECK: cvt.rna.tf32.f32
+  %val = call i32 @llvm.nvvm.f2tf32.rna(float %f1);
+
+ret i32 %val
+}
+
+declare i32 @llvm.nvvm.f2tf32.rna(float)
Index: llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
===
--- llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
+++ llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
@@ -1046,6 +1046,38 @@
 def : Pat<(int_nvvm_ui2f_rp Int32Regs:$a),
   (CVT_f32_u32 Int32Regs:$a, CvtRP)>;
 
+def : Pat<(int_nvvm_ff2bf16x2_rn Float32Regs:$a, Float32Regs:$b),
+  (CVT_bf16x2_f32 Float32Regs:$a, Float32Regs:$b, CvtRN)>;
+def : Pat<(int_nvvm_ff2bf16x2_rn_relu Float32Regs:$a, Float32Regs:$b),
+  (CVT_bf16x2_f32 Float32Regs:$a, Float32Regs:$b, CvtRN_RELU)>;
+def : Pat<(int_nvvm_ff2bf16x2_rz Float32Regs:$a, Float32Regs:$b),
+  (CVT_bf16x2_f32 Float32Regs:$a, Float32Regs:$b, C

[PATCH] D112913: Misleading bidirectional detection

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 397885.
serge-sans-paille added a comment.

rebased on main branch


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

https://reviews.llvm.org/D112913

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp
  clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-misleading-bidirectional.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-misleading-bidirectional.cpp

Index: clang-tools-extra/docs/clang-tidy/checks/misc-misleading-bidirectional.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-misleading-bidirectional.rst
@@ -0,0 +1,21 @@
+.. title:: clang-tidy - misc-misleading-bidirectional
+
+misc-misleading-bidirectional
+=
+
+Warn about unterminated bidirectional unicode sequence, detecting potential attack
+as described in the `Trojan Source `_ attack.
+
+Example:
+
+.. code-block:: c++
+
+#include 
+
+int main() {
+bool isAdmin = false;
+/*‮ } ⁦if (isAdmin)⁩ ⁦ begin admins only */
+std::cout << "You are an admin.\n";
+/* end admins only ‮ { ⁦*/
+return 0;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -212,7 +212,8 @@
`llvmlibc-implementation-in-namespace `_,
`llvmlibc-restrict-system-libc-headers `_, "Yes"
`misc-definitions-in-headers `_, "Yes"
-   `misc-misleading-identifier `_,
+   `misc-misleading-bidirectional `_,
+   `misc-misleading-identifier `_,
`misc-misplaced-const `_,
`misc-new-delete-overloads `_,
`misc-no-recursion `_,
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -120,6 +120,10 @@
   Reports identifiers whose names are too short. Currently checks local
   variables and function parameters only.
 
+- New :doc:`misc-misleading-bidirectional ` check.
+
+  Inspects string literal and comments for unterminated bidirectional Unicode
+  characters.
 
 New check aliases
 ^
Index: clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.h
@@ -0,0 +1,38 @@
+//===--- MisleadingBidirectionalCheck.h - clang-tidy *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISLEADINGBIDIRECTIONALCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISLEADINGBIDIRECTIONALCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+class MisleadingBidirectionalCheck : public ClangTidyCheck {
+public:
+  MisleadingBidirectionalCheck(StringRef Name, ClangTidyContext *Context);
+  ~MisleadingBidirectionalCheck();
+
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override;
+
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  class MisleadingBidirectionalHandler;
+  std::unique_ptr Handler;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISLEADINGBIDIRECTIONALCHECK_H
Index: clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/MisleadingBidirectional.cpp
@@ -0,0 +1,142 @@
+//===--- MisleadingBidirectional.cpp - clang-tidy -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MisleadingBidirectional.h"
+
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Preprocessor.h"
+#include "llvm/Support/ConvertUTF.

[PATCH] D112913: Misleading bidirectional detection

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

Thanks @upsuper!


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

https://reviews.llvm.org/D112913

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

[clang] d5ba066 - [clang][lex] NFC: Move some HeaderSearch functions to .cpp file

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2022-01-06T16:32:02+01:00
New Revision: d5ba066cb6641d1923dca90bb4e1a1cecbcd02b7

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

LOG: [clang][lex] NFC: Move some HeaderSearch functions to .cpp file

Added: 


Modified: 
clang/include/clang/Lex/HeaderSearch.h
clang/lib/Lex/HeaderSearch.cpp

Removed: 




diff  --git a/clang/include/clang/Lex/HeaderSearch.h 
b/clang/include/clang/Lex/HeaderSearch.h
index b3445703f7823..0482bf6c053f1 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -281,27 +281,10 @@ class HeaderSearch {
   /// Interface for setting the file search paths.
   void SetSearchPaths(std::vector dirs, unsigned angledDirIdx,
   unsigned systemDirIdx, bool noCurDirSearch,
-  llvm::DenseMap searchDirToHSEntry) {
-assert(angledDirIdx <= systemDirIdx && systemDirIdx <= dirs.size() &&
-"Directory indices are unordered");
-SearchDirs = std::move(dirs);
-SearchDirsUsage.assign(SearchDirs.size(), false);
-AngledDirIdx = angledDirIdx;
-SystemDirIdx = systemDirIdx;
-NoCurDirSearch = noCurDirSearch;
-SearchDirToHSEntry = std::move(searchDirToHSEntry);
-//LookupFileCache.clear();
-  }
+  llvm::DenseMap searchDirToHSEntry);
 
   /// Add an additional search path.
-  void AddSearchPath(const DirectoryLookup &dir, bool isAngled) {
-unsigned idx = isAngled ? SystemDirIdx : AngledDirIdx;
-SearchDirs.insert(SearchDirs.begin() + idx, dir);
-SearchDirsUsage.insert(SearchDirsUsage.begin() + idx, false);
-if (!isAngled)
-  AngledDirIdx++;
-SystemDirIdx++;
-  }
+  void AddSearchPath(const DirectoryLookup &dir, bool isAngled);
 
   /// Set the list of system header prefixes.
   void SetSystemHeaderPrefixes(ArrayRef> P) {

diff  --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 5cb009dd68d1d..f9d61ecef7964 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -109,6 +109,30 @@ void HeaderSearch::PrintStats() {
<< NumSubFrameworkLookups << " subframework lookups.\n";
 }
 
+void HeaderSearch::SetSearchPaths(
+std::vector dirs, unsigned int angledDirIdx,
+unsigned int systemDirIdx, bool noCurDirSearch,
+llvm::DenseMap searchDirToHSEntry) {
+  assert(angledDirIdx <= systemDirIdx && systemDirIdx <= dirs.size() &&
+ "Directory indices are unordered");
+  SearchDirs = std::move(dirs);
+  SearchDirsUsage.assign(SearchDirs.size(), false);
+  AngledDirIdx = angledDirIdx;
+  SystemDirIdx = systemDirIdx;
+  NoCurDirSearch = noCurDirSearch;
+  SearchDirToHSEntry = std::move(searchDirToHSEntry);
+  //LookupFileCache.clear();
+}
+
+void HeaderSearch::AddSearchPath(const DirectoryLookup &dir, bool isAngled) {
+  unsigned idx = isAngled ? SystemDirIdx : AngledDirIdx;
+  SearchDirs.insert(SearchDirs.begin() + idx, dir);
+  SearchDirsUsage.insert(SearchDirsUsage.begin() + idx, false);
+  if (!isAngled)
+AngledDirIdx++;
+  SystemDirIdx++;
+}
+
 std::vector HeaderSearch::computeUserEntryUsage() const {
   std::vector UserEntryUsage(HSOpts->UserEntries.size());
   for (unsigned I = 0, E = SearchDirsUsage.size(); I < E; ++I) {



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

[PATCH] D112913: Misleading bidirectional detection

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Has this been tested against any large code bases that use bidirectional 
characters to see what the false positive rate is? Also, have you seen the 
latest Unicode guidance on this topic: 
https://unicode.org/L2/L2022/22007-avoiding-spoof.pdf to make sure we're 
following along?


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

https://reviews.llvm.org/D112913

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

[PATCH] D115521: [Templight] Don't display empty strings for names of unnamed template parameters

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/Frontend/FrontendActions.cpp:501
+
+if (const auto *Decl = dyn_cast(NamedTemplate)) {
+  if (const auto *R = dyn_cast(Decl)) {

martong wrote:
> Should this handle `EnumDecl`s as well? An enum declaration itself cannot be 
> a primary template, however, it can be 1) a member of a specialization of a 
> templated class 2)  an instantiation of a member enumeration of a class 
> template specialization.
> 
> 
Should this handle variable templates (VarTemplateDecl) as well? Or it  is not 
possible to have unnamed variable templates?



Comment at: clang/lib/Frontend/FrontendActions.cpp:501-510
+if (const auto *Decl = dyn_cast(NamedTemplate)) {
+  if (const auto *R = dyn_cast(Decl)) {
+if (R->isLambda()) {
+  OS << "lambda at ";
+  Decl->getLocation().print(OS, TheSema.getSourceManager());
+  return;
+}

Should this handle `EnumDecl`s as well? An enum declaration itself cannot be a 
primary template, however, it can be 1) a member of a specialization of a 
templated class 2)  an instantiation of a member enumeration of a class 
template specialization.




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

https://reviews.llvm.org/D115521

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

[PATCH] D116337: [clang] set __NO_MATH_ERRNO__ if -fno-math-errno

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

It looks like precommit CI is still failing on `Clang :: 
Preprocessor/predefined-macros.c` on Windows (the Debian failures look 
unrelated, but you should double-check just to be sure).

https://buildkite.com/llvm-project/premerge-checks/builds/72399#4ba61c84-15ff-462e-b861-8b7be5b09eb4

Also, when you submit the patch, can you use `-U` to provide a bunch of 
context in the patch file? That makes code review easier within Phab.


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

https://reviews.llvm.org/D116337

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

[clang] f558acf - Reland "[clang][ObjC] Add fix it for missing methods in impl"

[David Goldman via cfe-commits]

Author: David Goldman
Date: 2022-01-06T10:55:02-05:00
New Revision: f558acf49201a843df3f0d3dd7e77bb5ea91568c

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

LOG: Reland "[clang][ObjC] Add fix it for missing methods in impl"

This reverts commit 37be74885946f18dbeb70343ad659924c61d2549/
relands https://reviews.llvm.org/D116417 now that the internal
issue has been fixed.

Added: 
clang/test/FixIt/fixit-objc-missing-method-impl.m

Modified: 
clang/lib/Sema/SemaDeclObjC.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp
index d6e659e17069c..d4fefc3d18d8a 100644
--- a/clang/lib/Sema/SemaDeclObjC.cpp
+++ b/clang/lib/Sema/SemaDeclObjC.cpp
@@ -2212,9 +2212,8 @@ void 
Sema::CheckImplementationIvars(ObjCImplementationDecl *ImpDecl,
 Diag(IVI->getLocation(), diag::err_inconsistent_ivar_count);
 }
 
-static void WarnUndefinedMethod(Sema &S, SourceLocation ImpLoc,
-ObjCMethodDecl *method,
-bool &IncompleteImpl,
+static void WarnUndefinedMethod(Sema &S, ObjCImplDecl *Impl,
+ObjCMethodDecl *method, bool &IncompleteImpl,
 unsigned DiagID,
 NamedDecl *NeededFor = nullptr) {
   // No point warning no definition of method which is 'unavailable'.
@@ -2227,10 +2226,19 @@ static void WarnUndefinedMethod(Sema &S, SourceLocation 
ImpLoc,
   // separate warnings.  We will give that approach a try, as that
   // matches what we do with protocols.
   {
-const Sema::SemaDiagnosticBuilder &B = S.Diag(ImpLoc, DiagID);
+const Sema::SemaDiagnosticBuilder &B = S.Diag(Impl->getLocation(), DiagID);
 B << method;
 if (NeededFor)
   B << NeededFor;
+
+// Add an empty definition at the end of the @implementation.
+std::string FixItStr;
+llvm::raw_string_ostream Out(FixItStr);
+method->print(Out, Impl->getASTContext().getPrintingPolicy());
+Out << " {\n}\n\n";
+
+SourceLocation Loc = Impl->getAtEndRange().getBegin();
+B << FixItHint::CreateInsertion(Loc, FixItStr);
   }
 
   // Issue a note to the original declaration.
@@ -2679,14 +2687,10 @@ static void findProtocolsWithExplicitImpls(const 
ObjCInterfaceDecl *Super,
 
 /// CheckProtocolMethodDefs - This routine checks unimplemented methods
 /// Declared in protocol, and those referenced by it.
-static void CheckProtocolMethodDefs(Sema &S,
-SourceLocation ImpLoc,
-ObjCProtocolDecl *PDecl,
-bool& IncompleteImpl,
-const Sema::SelectorSet &InsMap,
-const Sema::SelectorSet &ClsMap,
-ObjCContainerDecl *CDecl,
-LazyProtocolNameSet &ProtocolsExplictImpl) 
{
+static void CheckProtocolMethodDefs(
+Sema &S, ObjCImplDecl *Impl, ObjCProtocolDecl *PDecl, bool &IncompleteImpl,
+const Sema::SelectorSet &InsMap, const Sema::SelectorSet &ClsMap,
+ObjCContainerDecl *CDecl, LazyProtocolNameSet &ProtocolsExplictImpl) {
   ObjCCategoryDecl *C = dyn_cast(CDecl);
   ObjCInterfaceDecl *IDecl = C ? C->getClassInterface()
: dyn_cast(CDecl);
@@ -2773,9 +2777,8 @@ static void CheckProtocolMethodDefs(Sema &S,
   if (C || MethodInClass->isPropertyAccessor())
 continue;
 unsigned DIAG = diag::warn_unimplemented_protocol_method;
-if (!S.Diags.isIgnored(DIAG, ImpLoc)) {
-  WarnUndefinedMethod(S, ImpLoc, method, IncompleteImpl, DIAG,
-  PDecl);
+if (!S.Diags.isIgnored(DIAG, Impl->getLocation())) {
+  WarnUndefinedMethod(S, Impl, method, IncompleteImpl, DIAG, 
PDecl);
 }
   }
 }
@@ -2796,15 +2799,15 @@ static void CheckProtocolMethodDefs(Sema &S,
 continue;
 
   unsigned DIAG = diag::warn_unimplemented_protocol_method;
-  if (!S.Diags.isIgnored(DIAG, ImpLoc)) {
-WarnUndefinedMethod(S, ImpLoc, method, IncompleteImpl, DIAG, PDecl);
+  if (!S.Diags.isIgnored(DIAG, Impl->getLocation())) {
+WarnUndefinedMethod(S, Impl, method, IncompleteImpl, DIAG, PDecl);
   }
 }
   }
   // Check on this protocols's referenced protocols, recursively.
   for (auto *PI : PDecl->protocols())
-CheckProtocolMethodDefs(S, ImpLoc, PI, IncompleteImpl, InsMap, ClsMap,
-CDecl, ProtocolsExplictImpl);
+CheckProtocolMethodDefs(S, Impl, PI, IncompleteImpl, InsMap, ClsMap, CDecl,
+ProtocolsExplictImpl);
 }
 
 /// MatchAllMethodDeclarations -

[PATCH] D116748: [AArch64][ARM][Clang] PerfMon Extension Added

[Mubashar Ahmad via Phabricator via cfe-commits]

mubashar_ created this revision.
mubashar_ added a reviewer: lenary.
Herald added subscribers: hiraditya, kristof.beyls.
mubashar_ requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Performance monitor architecture extension has been added to clang.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116748

Files:
  clang/test/Driver/aarch64-perfmon.c
  clang/test/Driver/arm-perfmon.c
  llvm/include/llvm/Support/AArch64TargetParser.def
  llvm/include/llvm/Support/AArch64TargetParser.h
  llvm/include/llvm/Support/ARMTargetParser.def
  llvm/include/llvm/Support/ARMTargetParser.h
  llvm/lib/Support/AArch64TargetParser.cpp
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -728,7 +728,8 @@
   {"sb", "nosb", "+sb", "-sb"},
   {"i8mm", "noi8mm", "+i8mm", "-i8mm"},
   {"mve", "nomve", "+mve", "-mve"},
-  {"mve.fp", "nomve.fp", "+mve.fp", "-mve.fp"}};
+  {"mve.fp", "nomve.fp", "+mve.fp", "-mve.fp"},
+  {"pmuv3p4", "nopmuv3p4", "+perfmon", "-perfmon"}};
 
   for (unsigned i = 0; i < array_lengthof(ArchExt); i++) {
 EXPECT_EQ(StringRef(ArchExt[i][2]), ARM::getArchExtFeature(ArchExt[i][0]));
@@ -1438,7 +1439,7 @@
 AArch64::AEK_SVE2SHA3, AArch64::AEK_SVE2BITPERM,
 AArch64::AEK_RCPC, AArch64::AEK_FP16FML,
 AArch64::AEK_SME,  AArch64::AEK_SMEF64,
-AArch64::AEK_SMEI64 };
+AArch64::AEK_SMEI64,   AArch64::AEK_PERFMON};
 
   std::vector Features;
 
@@ -1473,6 +1474,7 @@
   EXPECT_TRUE(llvm::is_contained(Features, "+sme"));
   EXPECT_TRUE(llvm::is_contained(Features, "+sme-f64"));
   EXPECT_TRUE(llvm::is_contained(Features, "+sme-i64"));
+  EXPECT_TRUE(llvm::is_contained(Features, "+perfmon"));
 }
 
 TEST(TargetParserTest, AArch64ArchFeatures) {
@@ -1521,6 +1523,7 @@
   {"sme", "nosme", "+sme", "-sme"},
   {"sme-f64", "nosme-f64", "+sme-f64", "-sme-f64"},
   {"sme-i64", "nosme-i64", "+sme-i64", "-sme-i64"},
+  {"pmuv3p4", "nopmuv3p4", "+perfmon", "-perfmon"},
 };
 
   for (unsigned i = 0; i < array_lengthof(ArchExt); i++) {
Index: llvm/lib/Support/AArch64TargetParser.cpp
===
--- llvm/lib/Support/AArch64TargetParser.cpp
+++ llvm/lib/Support/AArch64TargetParser.cpp
@@ -114,6 +114,8 @@
 Features.push_back("+sme-f64");
   if (Extensions & AArch64::AEK_SMEI64)
 Features.push_back("+sme-i64");
+  if (Extensions & AArch64::AEK_PERFMON)
+Features.push_back("+perfmon");
 
   return true;
 }
Index: llvm/include/llvm/Support/ARMTargetParser.h
===
--- llvm/include/llvm/Support/ARMTargetParser.h
+++ llvm/include/llvm/Support/ARMTargetParser.h
@@ -60,6 +60,7 @@
   AEK_CDECP6 =  1 << 28,
   AEK_CDECP7 =  1 << 29,
   AEK_PACBTI =  1 << 30,
+  AEK_PERFMON = 1ULL << 31,
   // Unsupported extensions.
   AEK_OS   =1ULL << 59,
   AEK_IWMMXT   =1ULL << 60,
Index: llvm/include/llvm/Support/ARMTargetParser.def
===
--- llvm/include/llvm/Support/ARMTargetParser.def
+++ llvm/include/llvm/Support/ARMTargetParser.def
@@ -213,6 +213,7 @@
 ARM_ARCH_EXT_NAME("cdecp6",   ARM::AEK_CDECP6,   "+cdecp6",  "-cdecp6")
 ARM_ARCH_EXT_NAME("cdecp7",   ARM::AEK_CDECP7,   "+cdecp7",  "-cdecp7")
 ARM_ARCH_EXT_NAME("pacbti",   ARM::AEK_PACBTI,   "+pacbti",  "-pacbti")
+ARM_ARCH_EXT_NAME("pmuv3p4",  ARM::AEK_PERFMON,  "+perfmon", "-perfmon")
 #undef ARM_ARCH_EXT_NAME
 
 #ifndef ARM_HW_DIV_NAME
Index: llvm/include/llvm/Support/AArch64TargetParser.h
===
--- llvm/include/llvm/Support/AArch64TargetParser.h
+++ llvm/include/llvm/Support/AArch64TargetParser.h
@@ -69,6 +69,7 @@
   AEK_SME = 1ULL << 37,
   AEK_SMEF64 =  1ULL << 38,
   AEK_SMEI64 =  1ULL << 39,
+  AEK_PERFMON = 1ULL << 40,
 };
 
 enum class ArchKind {
Index: llvm/include/llvm/Support/AArch64TargetParser.def
===
--- llvm/include/llvm/Support/AArch64TargetParser.def
+++ llvm/include/llvm/Support/AArch64TargetParser.def
@@ -144,6 +144,7 @@
 AARCH64_ARCH_EXT_NAME("sme",  AArch64::AEK_SME, "+sme",   "-sme")
 AARCH64_ARCH_EXT_NAME("sme-f64",  AArch64::AEK_SMEF64,  "+sme-f64", "-sme-f64")
 AARCH64_ARCH_EXT_NAME("sme-i64",  AArch64::AEK_SMEI64,  "+sme-i64", "-sme-i64")
+AARCH64_ARCH_EXT_NAME("pmuv3p4",  AArch

[clang] e3e8799 - [AST] ASTContext::mergeTypes - pull out repeated getAs<> calls. NFC.

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-01-06T16:04:36Z
New Revision: e3e8799bebd0e624de21b9e93e358b9396bf4ace

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

LOG: [AST] ASTContext::mergeTypes - pull out repeated getAs<> calls. NFC.

Avoids static-analyzer null dereference warnings.

Added: 


Modified: 
clang/lib/AST/ASTContext.cpp

Removed: 




diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index ac6f98e91f755..3ecbdc1eb77b0 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -9819,12 +9819,13 @@ QualType ASTContext::mergeTypes(QualType LHS, QualType 
RHS,
   // designates the object or function denoted by the reference, and the
   // expression is an lvalue unless the reference is an rvalue reference and
   // the expression is a function call (possibly inside parentheses).
-  if (LangOpts.OpenMP && LHS->getAs() &&
-  RHS->getAs() && LHS->getTypeClass() == 
RHS->getTypeClass())
-return mergeTypes(LHS->getAs()->getPointeeType(),
-  RHS->getAs()->getPointeeType(),
+  auto *LHSRefTy = LHS->getAs();
+  auto *RHSRefTy = RHS->getAs();
+  if (LangOpts.OpenMP && LHSRefTy && RHSRefTy &&
+  LHS->getTypeClass() == RHS->getTypeClass())
+return mergeTypes(LHSRefTy->getPointeeType(), RHSRefTy->getPointeeType(),
   OfBlockPointer, Unqualified, BlockReturnType);
-  if (LHS->getAs() || RHS->getAs())
+  if (LHSRefTy || RHSRefTy)
 return {};
 
   if (Unqualified) {



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

[PATCH] D116748: [AArch64][ARM][Clang] PerfMon Extension Added

[Sam Elliott via Phabricator via cfe-commits]

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

I'm happy with this patch, but you could update the description to be clearer 
that you're adding an option to allow PMU v3.4 to be enabled/disabled 
separately to the architecture - not adding the pmu extension itself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116748

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

[PATCH] D116726: [clang-format] Fix a crash (assertion) in qualifier alignment when matching template closer is null

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 397898.
MyDeveloperDay added a comment.

Simplify down the test to make it easier to read, keep perhaps slightly 
duplicated tests to just ensure we have the coverage


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

https://reviews.llvm.org/D116726

Files:
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/lib/Format/QualifierAlignmentFixer.h
  clang/unittests/Format/QualifierFixerTest.cpp

Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -818,5 +818,45 @@
   EXPECT_EQ(ReplacementCount, 0);
 }
 
+TEST_F(QualifierFixerTest, QualifierTemplates) {
+  FormatStyle Style = getLLVMStyle();
+  Style.QualifierAlignment = FormatStyle::QAS_Custom;
+  Style.QualifierOrder = {"static", "const", "type"};
+
+  ReplacementCount = 0;
+  EXPECT_EQ(ReplacementCount, 0);
+  verifyFormat("using A = B<>;", Style);
+  verifyFormat("using A = B /**/<>;", Style);
+  verifyFormat("template  using A = B, 1>;", Style);
+  verifyFormat("template  using A = B /**/, 1>;", Style);
+  verifyFormat("template  using A = B /* */, 1>;", Style);
+  verifyFormat("template  using A = B /*foo*/, 1>;", Style);
+  verifyFormat("template  using A = B /**/ /**/, 1>;", Style);
+  verifyFormat("template  using A = B, 1>;", Style);
+  verifyFormat("template  using A = /**/ B, 1>;", Style);
+  EXPECT_EQ(ReplacementCount, 0);
+  verifyFormat("template \n"
+   "using A = B // foo\n"
+   ", 1>;",
+   Style);
+
+  ReplacementCount = 0;
+  Style.QualifierOrder = {"type", "static", "const"};
+  verifyFormat("using A = B<>;", Style);
+  verifyFormat("using A = B /**/<>;", Style);
+  verifyFormat("template  using A = B, 1>;", Style);
+  verifyFormat("template  using A = B /**/, 1>;", Style);
+  verifyFormat("template  using A = B /* */, 1>;", Style);
+  verifyFormat("template  using A = B /*foo*/, 1>;", Style);
+  verifyFormat("template  using A = B /**/ /**/, 1>;", Style);
+  verifyFormat("template  using A = B, 1>;", Style);
+  verifyFormat("template  using A = /**/ B, 1>;", Style);
+  EXPECT_EQ(ReplacementCount, 0);
+  verifyFormat("template \n"
+   "using A = B // foo\n"
+   ", 1>;",
+   Style);
+}
+
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/QualifierAlignmentFixer.h
===
--- clang/lib/Format/QualifierAlignmentFixer.h
+++ clang/lib/Format/QualifierAlignmentFixer.h
@@ -72,17 +72,19 @@
 
   static tok::TokenKind getTokenFromQualifier(const std::string &Qualifier);
 
-  FormatToken *analyzeRight(const SourceManager &SourceMgr,
-const AdditionalKeywords &Keywords,
-tooling::Replacements &Fixes, FormatToken *Tok,
-const std::string &Qualifier,
-tok::TokenKind QualifierType);
-
-  FormatToken *analyzeLeft(const SourceManager &SourceMgr,
-   const AdditionalKeywords &Keywords,
-   tooling::Replacements &Fixes, FormatToken *Tok,
-   const std::string &Qualifier,
-   tok::TokenKind QualifierType);
+  const FormatToken *analyzeRight(const SourceManager &SourceMgr,
+  const AdditionalKeywords &Keywords,
+  tooling::Replacements &Fixes,
+  const FormatToken *Tok,
+  const std::string &Qualifier,
+  tok::TokenKind QualifierType);
+
+  const FormatToken *analyzeLeft(const SourceManager &SourceMgr,
+ const AdditionalKeywords &Keywords,
+ tooling::Replacements &Fixes,
+ const FormatToken *Tok,
+ const std::string &Qualifier,
+ tok::TokenKind QualifierType);
 
   // is the Token a simple or qualifier type
   static bool isQualifierOrType(const FormatToken *Tok,
Index: clang/lib/Format/QualifierAlignmentFixer.cpp
===
--- clang/lib/Format/QualifierAlignmentFixer.cpp
+++ clang/lib/Format/QualifierAlignmentFixer.cpp
@@ -204,9 +204,9 @@
   replaceToken(SourceMgr, Fixes, Range, NewText);
 }
 
-FormatToken *LeftRightQualifierAlignmentFixer::analyzeRight(
+const FormatToken *LeftRightQualifierAlignmentFixer::analyzeRight(
 const SourceManager &SourceMgr, const AdditionalKeywords &Keywords,
-tooling::Replacements &Fixes, FormatToken *Tok,
+tooling::Replacements &Fixes, const FormatToken *Tok,
 const std::string &Qualifier, tok::TokenKind QualifierType) {
   // We only need to think

[PATCH] D116750: [clang][lex] Keep search directory indices up-to-date

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith, ahoppen.
Herald added a subscriber: mgorny.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The `HeaderSearch::SearchDirToHSEntry` map references `SearchDir` instances by 
their index in `HeaderSearch::SearchDirs`. When adding new search directory, 
update the indices.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116750

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/unittests/Lex/CMakeLists.txt
  clang/unittests/Lex/HeaderSearchTest.cpp

Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -15,6 +15,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/TargetOptions.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/HeaderSearchOptions.h"
 #include "clang/Serialization/InMemoryModuleCache.h"
@@ -24,6 +25,12 @@
 namespace clang {
 namespace {
 
+static std::shared_ptr createTargetOptions() {
+  auto TargetOpts = std::make_shared();
+  TargetOpts->Triple = "x86_64-apple-darwin11.1.0";
+  return TargetOpts;
+}
+
 // The test fixture.
 class HeaderSearchTest : public ::testing::Test {
 protected:
@@ -31,12 +38,10 @@
   : VFS(new llvm::vfs::InMemoryFileSystem), FileMgr(FileMgrOpts, VFS),
 DiagID(new DiagnosticIDs()),
 Diags(DiagID, new DiagnosticOptions, new IgnoringDiagConsumer()),
-SourceMgr(Diags, FileMgr), TargetOpts(new TargetOptions),
+SourceMgr(Diags, FileMgr), TargetOpts(createTargetOptions()),
+Target(TargetInfo::CreateTargetInfo(Diags, TargetOpts)),
 Search(std::make_shared(), SourceMgr, Diags,
-   LangOpts, Target.get()) {
-TargetOpts->Triple = "x86_64-apple-darwin11.1.0";
-Target = TargetInfo::CreateTargetInfo(Diags, TargetOpts);
-  }
+   LangOpts, Target.get()) {}
 
   void addSearchDir(llvm::StringRef Dir) {
 VFS->addFile(Dir, 0, llvm::MemoryBuffer::getMemBuffer(""), /*User=*/None,
@@ -47,6 +52,27 @@
 Search.AddSearchPath(DL, /*isAngled=*/false);
   }
 
+  void setSearchDirs(llvm::ArrayRef QuotedDirs,
+ llvm::ArrayRef AngledDirs) {
+auto AddPath = [&](StringRef Dir, bool IsAngled) {
+  VFS->addFile(Dir, 0, llvm::MemoryBuffer::getMemBuffer(""), /*User=*/None,
+   /*Group=*/None, llvm::sys::fs::file_type::directory_file);
+  auto Group = IsAngled ? frontend::IncludeDirGroup::Angled
+: frontend::IncludeDirGroup::Quoted;
+  Search.getHeaderSearchOpts().AddPath(Dir, Group,
+   /*IsFramework=*/false,
+   /*IgnoreSysRoot=*/true);
+};
+
+for (llvm::StringRef Dir : QuotedDirs)
+  AddPath(Dir, /*IsAngled=*/false);
+for (llvm::StringRef Dir : AngledDirs)
+  AddPath(Dir, /*IsAngled=*/true);
+
+clang::ApplyHeaderSearchOptions(Search, Search.getHeaderSearchOpts(),
+LangOpts, Target->getTriple());
+  }
+
   void addHeaderMap(llvm::StringRef Filename,
 std::unique_ptr Buf,
 bool isAngled = false) {
@@ -63,6 +89,17 @@
 Search.AddSearchPath(DL, isAngled);
   }
 
+  void createModule(StringRef Mod) {
+std::string ModDir = ("/" + Mod).str();
+std::string ModHeader = (Mod + ".h").str();
+VFS->addFile(
+ModDir + "/module.modulemap", 0,
+llvm::MemoryBuffer::getMemBufferCopy(
+("module " + Mod + " { header \"" + ModHeader + "\" }").str()));
+VFS->addFile(ModDir + "/" + ModHeader, 0,
+ llvm::MemoryBuffer::getMemBuffer(""));
+  }
+
   IntrusiveRefCntPtr VFS;
   FileSystemOptions FileMgrOpts;
   FileManager FileMgr;
@@ -224,5 +261,35 @@
   EXPECT_EQ(FI->Framework.str(), "Foo");
 }
 
+TEST_F(HeaderSearchTest, SearchPathUsage) {
+  Search.getHeaderSearchOpts().ImplicitModuleMaps = true;
+
+  setSearchDirs(/*QuotedDirs=*/{"/M0"}, /*AngledDirs=*/{"/M2", "/M3"});
+  createModule("M0");
+  createModule("M2");
+  createModule("M3");
+
+  {
+Module *M2 = Search.lookupModule("M2");
+EXPECT_NE(M2, nullptr);
+std::vector ExpectedSearchDirUsageAfterM2{false, true, false};
+EXPECT_EQ(Search.getSearchDirUsage(), ExpectedSearchDirUsageAfterM2);
+std::vector ExpectedUserEntryUsageAfterM2{false, true, false};
+EXPECT_EQ(Search.computeUserEntryUsage(), ExpectedUserEntryUsageAfterM2);
+  }
+
+  addSearchDir("/M1");
+  createModule("M1");
+
+  {
+Module *M1 = Search.lookupModule("M1");
+EXPECT_NE(M1, nullptr);
+std::vector ExpectedSearchDirUsageAfterM1{false, true, true, false};
+EXPECT_EQ(Search.getSearchDirUsage(), ExpectedSe

[PATCH] D116751: [clang][lex] NFC: Extract module creation into function

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith, ahoppen.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch extracts the construction of `Module` within `ModuleMap` into a 
separate function.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116751

Files:
  clang/include/clang/Lex/ModuleMap.h
  clang/lib/Lex/ModuleMap.cpp

Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -104,6 +104,13 @@
   llvm_unreachable("unknown header kind");
 }
 
+Module *ModuleMap::makeModule(StringRef Name, SourceLocation DefinitionLoc,
+  Module *Parent, bool IsFramework, bool IsExplicit,
+  unsigned VisibilityID) {
+  return new Module(Name, DefinitionLoc, Parent, IsFramework, IsExplicit,
+VisibilityID);
+}
+
 Module::ExportDecl
 ModuleMap::resolveExport(Module *Mod,
  const Module::UnresolvedExportDecl &Unresolved,
@@ -821,7 +828,7 @@
 return std::make_pair(Sub, false);
 
   // Create a new module with this name.
-  Module *Result = new Module(Name, SourceLocation(), Parent, IsFramework,
+  Module *Result = makeModule(Name, SourceLocation(), Parent, IsFramework,
   IsExplicit, NumCreatedModules++);
   if (!Parent) {
 if (LangOpts.CurrentModule == Name)
@@ -834,7 +841,7 @@
 
 Module *ModuleMap::createGlobalModuleFragmentForModuleUnit(SourceLocation Loc,
Module *Parent) {
-  auto *Result = new Module("", Loc, Parent, /*IsFramework*/ false,
+  auto *Result = makeModule("", Loc, Parent, /*IsFramework*/ false,
 /*IsExplicit*/ true, NumCreatedModules++);
   Result->Kind = Module::GlobalModuleFragment;
   // If the created module isn't owned by a parent, send it to PendingSubmodules
@@ -848,7 +855,7 @@
 ModuleMap::createPrivateModuleFragmentForInterfaceUnit(Module *Parent,
SourceLocation Loc) {
   auto *Result =
-  new Module("", Loc, Parent, /*IsFramework*/ false,
+  makeModule("", Loc, Parent, /*IsFramework*/ false,
  /*IsExplicit*/ true, NumCreatedModules++);
   Result->Kind = Module::PrivateModuleFragment;
   return Result;
@@ -861,7 +868,7 @@
   assert(!Modules[Name] && "redefining existing module");
 
   auto *Result =
-  new Module(Name, Loc, nullptr, /*IsFramework*/ false,
+  makeModule(Name, Loc, nullptr, /*IsFramework*/ false,
  /*IsExplicit*/ false, NumCreatedModules++);
   Result->Kind = Module::ModuleInterfaceUnit;
   Modules[Name] = SourceModule = Result;
@@ -888,13 +895,13 @@
   assert(!Modules[Name] && "redefining existing module");
 
   auto *Result =
-  new Module(Name, SourceLocation(), nullptr, /*IsFramework*/ false,
+  makeModule(Name, SourceLocation(), nullptr, /*IsFramework*/ false,
  /*IsExplicit*/ false, NumCreatedModules++);
   Result->Kind = Module::ModuleInterfaceUnit;
   Modules[Name] = SourceModule = Result;
 
   for (const Module::Header &H : Headers) {
-auto *M = new Module(H.NameAsWritten, SourceLocation(), Result,
+auto *M = makeModule(H.NameAsWritten, SourceLocation(), Result,
  /*IsFramework*/ false,
  /*IsExplicit*/ true, NumCreatedModules++);
 // Header modules are implicitly 'export *'.
@@ -1023,7 +1030,7 @@
   if (!UmbrellaHeader)
 return nullptr;
 
-  Module *Result = new Module(ModuleName, SourceLocation(), Parent,
+  Module *Result = makeModule(ModuleName, SourceLocation(), Parent,
   /*IsFramework=*/true, /*IsExplicit=*/false,
   NumCreatedModules++);
   InferredModuleAllowedBy[Result] = ModuleMapFile;
@@ -1116,7 +1123,7 @@
 
   // Create a new module with this name.
   Module *Result =
-  new Module(Name, SourceLocation(), /*Parent=*/nullptr, IsFramework,
+  makeModule(Name, SourceLocation(), /*Parent=*/nullptr, IsFramework,
  /*IsExplicit=*/false, NumCreatedModules++);
   Result->ShadowingModule = ShadowingModule;
   Result->markUnavailable(/*Unimportable*/true);
Index: clang/include/clang/Lex/ModuleMap.h
===
--- clang/include/clang/Lex/ModuleMap.h
+++ clang/include/clang/Lex/ModuleMap.h
@@ -279,6 +279,11 @@
   /// map.
   llvm::DenseMap ParsedModuleMap;
 
+  /// Creates new Module.
+  Module *makeModule(StringRef Name, SourceLocation DefinitionLoc,
+ Module *Parent, bool IsFramework, bool IsExplicit,
+ unsigned VisibilityID);
+
   /// Resolve the given export declaration into an actual export
   /// declaration.
   ///
__

[PATCH] D113676: WIP: [clang][lex] Fix search path usage remark with modules

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 397902.
jansvoboda11 added a comment.

Rebase on top of extracted patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113676

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/ModuleMap.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/ModuleMap.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/test/Preprocessor/search-path-usage-modules.m
  clang/test/Preprocessor/search-path-usage.m

Index: clang/test/Preprocessor/search-path-usage.m
===
--- clang/test/Preprocessor/search-path-usage.m
+++ clang/test/Preprocessor/search-path-usage.m
@@ -128,19 +128,3 @@
 // expected-remark-re {{search path used: '{{.*}}/b-missing.hmap'}}
 #endif
 #endif
-
-// Check that search paths with module maps are reported.
-//
-// RUN: mkdir %t/modulemap_abs
-// RUN: sed "s|DIR|%/S/Inputs/search-path-usage|g"\
-// RUN:   %S/Inputs/search-path-usage/modulemap_abs/module.modulemap.template \
-// RUN: > %t/modulemap_abs/module.modulemap
-// RUN: %clang_cc1 -Eonly %s -Rsearch-path-usage   \
-// RUN:   -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules \
-// RUN:   -I %t/modulemap_abs  \
-// RUN:   -I %S/Inputs/search-path-usage/a \
-// RUN:   -DMODMAP_ABS -verify
-#ifdef MODMAP_ABS
-@import b; // \
-// expected-remark-re {{search path used: '{{.*}}/modulemap_abs'}}
-#endif
Index: clang/test/Preprocessor/search-path-usage-modules.m
===
--- /dev/null
+++ clang/test/Preprocessor/search-path-usage-modules.m
@@ -0,0 +1,40 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -Eonly %t/test-both.m   -I %t/sp1 -I %t/sp2 -Rsearch-path-usage \
+// RUN:   -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache 2>&1 | FileCheck %t/test-both.m
+// RUN: %clang_cc1 -Eonly %t/test-both.m   -I %t/sp2 -I %t/sp1 -Rsearch-path-usage \
+// RUN:   -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache 2>&1 | FileCheck %t/test-both.m
+
+// RUN: %clang_cc1 -Eonly %t/test-first.m  -I %t/sp2 -I %t/sp1 -Rsearch-path-usage \
+// RUN:   -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache 2>&1 | FileCheck %t/test-first.m
+// RUN: %clang_cc1 -Eonly %t/test-second.m -I %t/sp1 -I %t/sp2 -Rsearch-path-usage \
+// RUN:   -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache 2>&1 | FileCheck %t/test-second.m
+
+//--- sp1/module.modulemap
+module mod1 { header "mod1.h" }
+//--- sp1/mod1.h
+int module1();
+
+//--- sp2/module.modulemap
+module mod2 { header "mod2.h" }
+//--- sp2/mod2.h
+int module2();
+
+//--- test-both.m
+@import mod1;
+@import mod2;
+// CHECK: search path used: '{{.*}}/sp1'
+// CHECK: search path used: '{{.*}}/sp2'
+
+//--- test-first.m
+@import mod1;
+// CHECK-NOT: search path used: '{{.*}}/sp2'
+// CHECK: search path used: '{{.*}}/sp1'
+// CHECK-NOT: search path used: '{{.*}}/sp2'
+
+//--- test-second.m
+@import mod2;
+// CHECK-NOT: search path used: '{{.*}}/sp1'
+// CHECK: search path used: '{{.*}}/sp2'
+// CHECK-NOT: search path used: '{{.*}}/sp1'
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -3907,7 +3907,7 @@
 // An implicitly-loaded module file should have its module listed in some
 // module map file that we've already loaded.
 Module *M =
-PP.getHeaderSearchInfo().lookupModule(F.ModuleName, F.ImportLoc);
+PP.getHeaderSearchInfo().lookupModule(F.ModuleName, SourceLocation());
 auto &Map = PP.getHeaderSearchInfo().getModuleMap();
 const FileEntry *ModMap = M ? Map.getModuleMapFileForUniquing(M) : nullptr;
 // Don't emit module relocation error if we have -fno-validate-pch
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -107,8 +107,13 @@
 Module *ModuleMap::makeModule(StringRef Name, SourceLocation DefinitionLoc,
   Module *Parent, bool IsFramework, bool IsExplicit,
   unsigned VisibilityID) {
-  return new Module(Name, DefinitionLoc, Parent, IsFramework, IsExplicit,
-VisibilityID);
+  Module *Result = new Module(Name, DefinitionLoc, Parent, IsFramework,
+  IsExplicit, VisibilityID);
+
+  for (const auto &Callback : Callbacks)
+Callback->moduleMapModuleCreated(Result);
+
+  return Result;
 }
 
 Module::ExportDecl
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.

[clang] 2ccf0b7 - Fix build failure with GCC 11 in C++20 mode

[Gabor Marton via cfe-commits]

Author: Evgeny Mandrikov
Date: 2022-01-06T17:20:26+01:00
New Revision: 2ccf0b76bcaf0895e04f14e3ff53c59dd96f9f0f

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

LOG: Fix build failure with GCC 11 in C++20 mode

See https://wg21.link/cwg2237

Reviewed By: shafik, dexonsmith

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

Added: 


Modified: 
clang/lib/AST/ASTImporter.cpp
llvm/include/llvm/CodeGen/LiveInterval.h
llvm/include/llvm/Support/BinaryStreamArray.h
llvm/lib/Passes/StandardInstrumentations.cpp

Removed: 




diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 7f78da10e0b36..457465e87d935 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -8409,8 +8409,8 @@ Expected 
ASTImporter::Import(TypeSourceInfo *FromTSI) {
 // and destructed after it is created. The construction already performs the
 // import of the data.
 template  struct AttrArgImporter {
-  AttrArgImporter(const AttrArgImporter &) = delete;
-  AttrArgImporter(AttrArgImporter &&) = default;
+  AttrArgImporter(const AttrArgImporter &) = delete;
+  AttrArgImporter(AttrArgImporter &&) = default;
   AttrArgImporter &operator=(const AttrArgImporter &) = delete;
   AttrArgImporter &operator=(AttrArgImporter &&) = default;
 
@@ -8429,8 +8429,8 @@ template  struct AttrArgImporter {
 // is used by the attribute classes. This object should be created once for the
 // array data to be imported (the array size is not imported, just copied).
 template  struct AttrArgArrayImporter {
-  AttrArgArrayImporter(const AttrArgArrayImporter &) = delete;
-  AttrArgArrayImporter(AttrArgArrayImporter &&) = default;
+  AttrArgArrayImporter(const AttrArgArrayImporter &) = delete;
+  AttrArgArrayImporter(AttrArgArrayImporter &&) = default;
   AttrArgArrayImporter &operator=(const AttrArgArrayImporter &) = delete;
   AttrArgArrayImporter &operator=(AttrArgArrayImporter &&) = default;
 

diff  --git a/llvm/include/llvm/CodeGen/LiveInterval.h 
b/llvm/include/llvm/CodeGen/LiveInterval.h
index 923a45821dd4b..51ffe28074345 100644
--- a/llvm/include/llvm/CodeGen/LiveInterval.h
+++ b/llvm/include/llvm/CodeGen/LiveInterval.h
@@ -724,7 +724,7 @@ namespace llvm {
   T *P;
 
 public:
-  SingleLinkedListIterator(T *P) : P(P) {}
+  SingleLinkedListIterator(T *P) : P(P) {}
 
   SingleLinkedListIterator &operator++() {
 P = P->Next;

diff  --git a/llvm/include/llvm/Support/BinaryStreamArray.h 
b/llvm/include/llvm/Support/BinaryStreamArray.h
index 85d29be26ca96..c3e0db4dcff09 100644
--- a/llvm/include/llvm/Support/BinaryStreamArray.h
+++ b/llvm/include/llvm/Support/BinaryStreamArray.h
@@ -323,7 +323,7 @@ class FixedStreamArrayIterator
   FixedStreamArrayIterator(const FixedStreamArray &Array, uint32_t Index)
   : Array(Array), Index(Index) {}
 
-  FixedStreamArrayIterator(const FixedStreamArrayIterator &Other)
+  FixedStreamArrayIterator(const FixedStreamArrayIterator &Other)
   : Array(Other.Array), Index(Other.Index) {}
   FixedStreamArrayIterator &
   operator=(const FixedStreamArrayIterator &Other) {

diff  --git a/llvm/lib/Passes/StandardInstrumentations.cpp 
b/llvm/lib/Passes/StandardInstrumentations.cpp
index 23c825c787132..c42b1cb26f138 100644
--- a/llvm/lib/Passes/StandardInstrumentations.cpp
+++ b/llvm/lib/Passes/StandardInstrumentations.cpp
@@ -441,7 +441,7 @@ const Module *getModuleForComparison(Any IR) {
 
 } // namespace
 
-template  ChangeReporter::~ChangeReporter() {
+template  ChangeReporter::~ChangeReporter() {
   assert(BeforeStack.empty() && "Problem with Change Printer stack.");
 }
 



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

[PATCH] D115355: Fix build failure with GCC 11 in C++20 mode

[Gabor Marton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2ccf0b76bcaf: Fix build failure with GCC 11 in C++20 mode 
(authored by Godin, committed by martong).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115355

Files:
  clang/lib/AST/ASTImporter.cpp
  llvm/include/llvm/CodeGen/LiveInterval.h
  llvm/include/llvm/Support/BinaryStreamArray.h
  llvm/lib/Passes/StandardInstrumentations.cpp


Index: llvm/lib/Passes/StandardInstrumentations.cpp
===
--- llvm/lib/Passes/StandardInstrumentations.cpp
+++ llvm/lib/Passes/StandardInstrumentations.cpp
@@ -441,7 +441,7 @@
 
 } // namespace
 
-template  ChangeReporter::~ChangeReporter() {
+template  ChangeReporter::~ChangeReporter() {
   assert(BeforeStack.empty() && "Problem with Change Printer stack.");
 }
 
Index: llvm/include/llvm/Support/BinaryStreamArray.h
===
--- llvm/include/llvm/Support/BinaryStreamArray.h
+++ llvm/include/llvm/Support/BinaryStreamArray.h
@@ -323,7 +323,7 @@
   FixedStreamArrayIterator(const FixedStreamArray &Array, uint32_t Index)
   : Array(Array), Index(Index) {}
 
-  FixedStreamArrayIterator(const FixedStreamArrayIterator &Other)
+  FixedStreamArrayIterator(const FixedStreamArrayIterator &Other)
   : Array(Other.Array), Index(Other.Index) {}
   FixedStreamArrayIterator &
   operator=(const FixedStreamArrayIterator &Other) {
Index: llvm/include/llvm/CodeGen/LiveInterval.h
===
--- llvm/include/llvm/CodeGen/LiveInterval.h
+++ llvm/include/llvm/CodeGen/LiveInterval.h
@@ -724,7 +724,7 @@
   T *P;
 
 public:
-  SingleLinkedListIterator(T *P) : P(P) {}
+  SingleLinkedListIterator(T *P) : P(P) {}
 
   SingleLinkedListIterator &operator++() {
 P = P->Next;
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -8409,8 +8409,8 @@
 // and destructed after it is created. The construction already performs the
 // import of the data.
 template  struct AttrArgImporter {
-  AttrArgImporter(const AttrArgImporter &) = delete;
-  AttrArgImporter(AttrArgImporter &&) = default;
+  AttrArgImporter(const AttrArgImporter &) = delete;
+  AttrArgImporter(AttrArgImporter &&) = default;
   AttrArgImporter &operator=(const AttrArgImporter &) = delete;
   AttrArgImporter &operator=(AttrArgImporter &&) = default;
 
@@ -8429,8 +8429,8 @@
 // is used by the attribute classes. This object should be created once for the
 // array data to be imported (the array size is not imported, just copied).
 template  struct AttrArgArrayImporter {
-  AttrArgArrayImporter(const AttrArgArrayImporter &) = delete;
-  AttrArgArrayImporter(AttrArgArrayImporter &&) = default;
+  AttrArgArrayImporter(const AttrArgArrayImporter &) = delete;
+  AttrArgArrayImporter(AttrArgArrayImporter &&) = default;
   AttrArgArrayImporter &operator=(const AttrArgArrayImporter &) = delete;
   AttrArgArrayImporter &operator=(AttrArgArrayImporter &&) = default;
 


Index: llvm/lib/Passes/StandardInstrumentations.cpp
===
--- llvm/lib/Passes/StandardInstrumentations.cpp
+++ llvm/lib/Passes/StandardInstrumentations.cpp
@@ -441,7 +441,7 @@
 
 } // namespace
 
-template  ChangeReporter::~ChangeReporter() {
+template  ChangeReporter::~ChangeReporter() {
   assert(BeforeStack.empty() && "Problem with Change Printer stack.");
 }
 
Index: llvm/include/llvm/Support/BinaryStreamArray.h
===
--- llvm/include/llvm/Support/BinaryStreamArray.h
+++ llvm/include/llvm/Support/BinaryStreamArray.h
@@ -323,7 +323,7 @@
   FixedStreamArrayIterator(const FixedStreamArray &Array, uint32_t Index)
   : Array(Array), Index(Index) {}
 
-  FixedStreamArrayIterator(const FixedStreamArrayIterator &Other)
+  FixedStreamArrayIterator(const FixedStreamArrayIterator &Other)
   : Array(Other.Array), Index(Other.Index) {}
   FixedStreamArrayIterator &
   operator=(const FixedStreamArrayIterator &Other) {
Index: llvm/include/llvm/CodeGen/LiveInterval.h
===
--- llvm/include/llvm/CodeGen/LiveInterval.h
+++ llvm/include/llvm/CodeGen/LiveInterval.h
@@ -724,7 +724,7 @@
   T *P;
 
 public:
-  SingleLinkedListIterator(T *P) : P(P) {}
+  SingleLinkedListIterator(T *P) : P(P) {}
 
   SingleLinkedListIterator &operator++() {
 P = P->Next;
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -8409,8 +8409,8 @@
 // and destructed after it is created. The con

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

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

balazske added a comment.

The functions `asctime` and `asctime_r` are discouraged according to CERT 
MSC33-C rule. These could be added to this check as well. There is a clang SA 
checker `SecuritySyntaxChecker` that contains other obsolete functions (and the 
whole check looks like it can be done in clang-tidy).


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

https://reviews.llvm.org/D91000

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

[PATCH] D104830: AST: Create __va_list in the std namespace even in C.

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

Ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104830

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

[PATCH] D113676: [clang][lex] Fix search path usage remark with modules

[Jan Svoboda via Phabricator via cfe-commits]

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



Comment at: clang/lib/Lex/HeaderSearch.cpp:95
+  if (HS.CurrentSearchPathIdx != ~0U)
+HS.ModuleToSearchDirIdx[M] = HS.CurrentSearchPathIdx;
+}

ahoppen wrote:
> These indices will be out of date if the search paths are changed via 
> `HeaderSearch::SetSearchPaths` or `HeaderSearch::AddSearchPath` after the 
> first module map has been loaded. One could probably adjust the indices in 
> `AddSearchPath` but maybe it’s easier to not use the index into `SearchDirs` 
> as the key. An alternative suggestion would be to use 
> `std::shared_ptr` instead, changing some of the data 
> structures as follows:
> ```
> - std::vector SearchDirs
> + std::vector> SearchDirs
> 
> - std::vector SearchDirsUsage;
> + std::map, bool> SearchDirsUsage; 
> 
> - llvm::DenseMap ModuleToSearchDirIdx;
> + llvm::DenseMap> 
> ModuleToSearchDirIdx;
> 
> - llvm::DenseMap SearchDirToHSEntry;
> + llvm::DenseMap, unsigned> 
> SearchDirToHSEntry; 
> ```
Fixed in D116750.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113676

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

[PATCH] D116670: [ASan] Driver changes to always link-in asan_static library.

[Kirill Stoimenov via Phabricator via cfe-commits]

kstoimenov updated this revision to Diff 397908.
kstoimenov added a comment.

Lint if statement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116670

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/sanitizer-ld.c


Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -22,7 +22,7 @@
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-ASAN-NO-LINK-RUNTIME-LINUX %s
 //
-// CHECK-ASAN-NO-LINK-RUNTIME-LINUX-NOT: libclang_rt.asan
+// CHECK-ASAN-NO-LINK-RUNTIME-LINUX-NOT: libclang_rt.asan-x86_64
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target i386-unknown-linux -fuse-ld=ld -fsanitize=address 
-shared-libsan \
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -826,6 +826,10 @@
   if (SanArgs.needsStatsRt() && SanArgs.linkRuntimes())
 StaticRuntimes.push_back("stats_client");
 
+  // Always link the static runtime regardless of DSO or executable.
+  if (SanArgs.needsAsanRt())
+HelperStaticRuntimes.push_back("asan_static");
+
   // Collect static runtimes.
   if (Args.hasArg(options::OPT_shared)) {
 // Don't link static runtimes into DSOs.


Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -22,7 +22,7 @@
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-ASAN-NO-LINK-RUNTIME-LINUX %s
 //
-// CHECK-ASAN-NO-LINK-RUNTIME-LINUX-NOT: libclang_rt.asan
+// CHECK-ASAN-NO-LINK-RUNTIME-LINUX-NOT: libclang_rt.asan-x86_64
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target i386-unknown-linux -fuse-ld=ld -fsanitize=address -shared-libsan \
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -826,6 +826,10 @@
   if (SanArgs.needsStatsRt() && SanArgs.linkRuntimes())
 StaticRuntimes.push_back("stats_client");
 
+  // Always link the static runtime regardless of DSO or executable.
+  if (SanArgs.needsAsanRt())
+HelperStaticRuntimes.push_back("asan_static");
+
   // Collect static runtimes.
   if (Args.hasArg(options::OPT_shared)) {
 // Don't link static runtimes into DSOs.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116731: [Clang] Make Clang copy its CMake modules into the build dir

[Pete Steinfeld via Phabricator via cfe-commits]

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

Thanks for doing this!

After adopting this change, I did an in tree build followed by an out of tree 
build -- both without creating or using the install area.  Both builds were 
successful and ran `check-flang` without problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116731

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

[PATCH] D116549: [OpenMP][Clang] Allow passing target features in ISA trait for metadirective clause

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam added a comment.

In D116549#3218281 , @jdoerfert wrote:

> Can you make the test check for the diagnose message? Also, do we have a test 
> to verify an isa trait is properly handled?

I don't see a point in adding a diagnostic message when a feature is not found 
valid for a certain target. Because if it satisfies one of the host or device 
compilation, then it will definitely fail in the other.
I have added new tests to check that isa trait is handled properly in the next 
revision.




Comment at: clang/lib/Parse/ParseOpenMP.cpp:2533
+std::function DiagUnknownTrait = [this, Loc](
+StringRef ISATrait) {};
+TargetOMPContext OMPCtx(ASTContext, std::move(DiagUnknownTrait),

jdoerfert wrote:
> Why doesn't this diagnose nothing?
Because an isa-feature will fail at least once, for either host compilation or 
device compilation. So, no point in always giving a warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116549

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

[PATCH] D116549: [OpenMP][Clang] Allow passing target features in ISA trait for metadirective clause

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam updated this revision to Diff 397912.
saiislam added a comment.
Herald added a subscriber: jvesely.

Added target specific tests for ISA traits, for CPU as well as GPU.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116549

Files:
  clang/lib/Parse/ParseOpenMP.cpp
  clang/test/OpenMP/amdgcn_target_codegen.cpp
  clang/test/OpenMP/metadirective_implementation_codegen.c
  clang/test/OpenMP/metadirective_implementation_codegen.cpp

Index: clang/test/OpenMP/metadirective_implementation_codegen.cpp
===
--- clang/test/OpenMP/metadirective_implementation_codegen.cpp
+++ clang/test/OpenMP/metadirective_implementation_codegen.cpp
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-linux -emit-llvm %s -fexceptions -fcxx-exceptions -o - -fsanitize-address-use-after-scope | FileCheck %s
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple aarch64-unknown-linux -emit-llvm %s -fexceptions -fcxx-exceptions -o - -fsanitize-address-use-after-scope | FileCheck %s
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple ppc64le-unknown-linux -emit-llvm %s -fexceptions -fcxx-exceptions -o - -fsanitize-address-use-after-scope | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-linux -emit-llvm %s -fexceptions -fcxx-exceptions -o - -fsanitize-address-use-after-scope -target-cpu x86-64| FileCheck %s -check-prefixes=SUPPORTEDISA,UNSUPPORTEDISA
 // expected-no-diagnostics
 
 #ifndef HEADER
@@ -35,6 +36,12 @@
: parallel) default(parallel for)
   for (int i = 0; i < 100; i++)
 ;
+#pragma omp metadirective when(device = {isa("sse2")} \
+   : parallel) default(single)
+  bar();
+#pragma omp metadirective when(device = {isa("some-unsupported-feature")} \
+   : parallel) default(single)
+  bar();
 }
 
 // CHECK-LABEL: void @_Z3foov()
@@ -44,6 +51,10 @@
 // CHECK: @__kmpc_fork_call(%struct.ident_t* {{.+}}, i32 0, void (i32*, i32*, ...)* bitcast (void (i32*, i32*)* [[OUTLINED_5:@.+]] to void
 // CHECK: @__kmpc_fork_call(%struct.ident_t* {{.+}}, i32 0, void (i32*, i32*, ...)* bitcast (void (i32*, i32*)* [[OUTLINED_6:@.+]] to void
 // CHECK: @__kmpc_fork_call(%struct.ident_t* {{.+}}, i32 0, void (i32*, i32*, ...)* bitcast (void (i32*, i32*)* [[OUTLINED_7:@.+]] to void
+// SUPPORTEDISA: ...) @__kmpc_fork_call(
+// UNSUPPORTEDISA: call i32 @__kmpc_single
+// UNSUPPORTEDISA:  @_Z3barv
+// UNSUPPORTEDISA: call void @__kmpc_end_single
 // CHECK: ret void
 
 // CHECK: define internal void [[OUTLINED_2]](
@@ -73,4 +84,7 @@
 // NO-CHECK: call void @__kmpc_for_static_fini
 // CHECK: ret void
 
+// SUPPORTEDISA: define internal void @.omp_outlined..6(
+// SUPPORTEDISA: @_Z3barv
+// SUPPORTEDISA: ret void
 #endif
Index: clang/test/OpenMP/metadirective_implementation_codegen.c
===
--- clang/test/OpenMP/metadirective_implementation_codegen.c
+++ clang/test/OpenMP/metadirective_implementation_codegen.c
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -verify -fopenmp -x c -triple x86_64-unknown-linux -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -verify -fopenmp -x c -triple aarch64-unknown-linux -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -verify -fopenmp -x c -triple ppc64le-unknown-linux -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -x c -triple x86_64-unknown-linux -emit-llvm -target-cpu x86-64 %s -o - | FileCheck %s -check-prefixes=SUPPORTEDISA,UNSUPPORTEDISA
 // expected-no-diagnostics
 
 #ifndef HEADER
@@ -35,10 +36,20 @@
: parallel) default(parallel for)
   for (int i = 0; i < 100; i++)
 ;
+#pragma omp metadirective when(device = {isa("sse2")} \
+   : parallel) default(single)
+  bar();
+#pragma omp metadirective when(device = {isa("some-unsupported-feature")} \
+   : parallel) default(single)
+  bar();
 }
 
 // CHECK: void @foo()
 // CHECK-COUNT-6: ...) @__kmpc_fork_call(
+// SUPPORTEDISA: ...) @__kmpc_fork_call(
+// UNSUPPORTEDISA: call i32 @__kmpc_single
+// UNSUPPORTEDISA: @bar
+// UNSUPPORTEDISA: call void @__kmpc_end_single
 // CHECK: ret void
 
 // CHECK: define internal void @.omp_outlined.(
@@ -68,4 +79,7 @@
 // NO-CHECK: call void @__kmpc_for_static_fini
 // CHECK: ret void
 
+// SUPPORTEDISA: define internal void @.omp_outlined..6(
+// SUPPORTEDISA: @bar
+// SUPPORTEDISA: ret void
 #endif
Index: clang/test/OpenMP/amdgcn_target_codegen.cpp
===
--- clang/test/OpenMP/amdgcn_target_codegen.cpp
+++ clang/test/OpenMP/amdgcn_target_codegen.cpp
@@ -2,6 +2,7 @@
 
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown -fopenmp-targets=amdgcn-amd-amdhsa -emit-llvm-bc %s -o %t-pp

[PATCH] D113676: [clang][lex] Fix search path usage remark with modules

[Jan Svoboda via Phabricator via cfe-commits]

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

I'm starting to think it could be fine to keep doing things as outlined by this 
patch: only marking search paths that discovered a module as used when its 
`Module` gets returned from `HeaderSearch` (and ignoring `Module`s returned 
`ModuleMap`).

I think this could work okay for the purposes of dependency scanning.

If the scanning phase fails with an error, the diagnostics will be presented to 
the user and the actual compile won't be carried out at all - the pruning 
optimization won't kick in.
Other kinds of diagnostics (that might be discarded/changed by pruning search 
paths) will be still visible in the build log of the scanning phase - this 
might be good enough.
And the preprocessor seems to use `HeaderSearch` when handling include/import 
directives without touching `ModuleMap`, which should work.

If we document this properly in `computeUserEntryUsage()` I think that may be 
fine. Any thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113676

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

[PATCH] D112913: Misleading bidirectional detection

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@aaron.ballman unfortunately I don't know any of those. If I recall correctly 
we found no software in the RedHat collection actually using those control 
characters :-/
My understanding is that we are inline with the document you mention, except 
the fact that 1. We don't allow LRM character in plain text and 2. we do not 
check if a string / comment sequence ends in RTL state, but that actually 
requires another algorithm :-/


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

https://reviews.llvm.org/D112913

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

[PATCH] D112913: Misleading bidirectional detection

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

Note that if there's a consensus on it, I can implement LRM character support.


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

https://reviews.llvm.org/D112913

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

[PATCH] D116753: Default to -fno-math-errno for musl too

[Alex Xu (Hello71) via Phabricator via cfe-commits]

alxu created this revision.
alxu added a project: clang.
alxu requested review of this revision.
Herald added a subscriber: cfe-commits.

musl does not set errno in math functions: 
https://wiki.musl-libc.org/mathematical-library.html, 
https://github.com/ifduyue/musl/blob/cfdfd5ea3ce14c6abf7fb22a531f3d99518b5a1b/include/math.h#L28.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116753

Files:
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/fast-math.c


Index: clang/test/Driver/fast-math.c
===
--- clang/test/Driver/fast-math.c
+++ clang/test/Driver/fast-math.c
@@ -102,6 +102,8 @@
 // RUN:   | FileCheck --check-prefix=CHECK-NO-MATH-ERRNO %s
 // RUN: %clang -### -target x86_64-linux-android -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-MATH-ERRNO %s
+// RUN: %clang -### -target x86_64-linux-musl -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NO-MATH-ERRNO %s
 // RUN: %clang -### -target amdgcn-amd-amdhsa -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-MATH-ERRNO %s
 // RUN: %clang -### -target amdgcn-amd-amdpal -c %s 2>&1 \
Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -681,7 +681,7 @@
 }
 
 bool Linux::IsMathErrnoDefault() const {
-  if (getTriple().isAndroid())
+  if (getTriple().isAndroid() || getTriple().isMusl())
 return false;
   return Generic_ELF::IsMathErrnoDefault();
 }


Index: clang/test/Driver/fast-math.c
===
--- clang/test/Driver/fast-math.c
+++ clang/test/Driver/fast-math.c
@@ -102,6 +102,8 @@
 // RUN:   | FileCheck --check-prefix=CHECK-NO-MATH-ERRNO %s
 // RUN: %clang -### -target x86_64-linux-android -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-MATH-ERRNO %s
+// RUN: %clang -### -target x86_64-linux-musl -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NO-MATH-ERRNO %s
 // RUN: %clang -### -target amdgcn-amd-amdhsa -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-MATH-ERRNO %s
 // RUN: %clang -### -target amdgcn-amd-amdpal -c %s 2>&1 \
Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -681,7 +681,7 @@
 }
 
 bool Linux::IsMathErrnoDefault() const {
-  if (getTriple().isAndroid())
+  if (getTriple().isAndroid() || getTriple().isMusl())
 return false;
   return Generic_ELF::IsMathErrnoDefault();
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

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

steakhal added a comment.

In D91000#3225369 , @balazske wrote:

> The functions `asctime` and `asctime_r` are discouraged according to CERT 
> MSC33-C rule. These could be added to this check as well. There is a clang SA 
> checker `SecuritySyntaxChecker` that contains other obsolete functions (and 
> the whole check looks like it can be done in clang-tidy).

+1


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

https://reviews.llvm.org/D91000

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

[PATCH] D106518: [RISCV] Disable EEW=64 for index values when XLEN=32.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:5475
 
+  if (IndexVT.getVectorElementType() == MVT::i64 && XLenVT == MVT::i32) {
+report_fatal_error("The V extension does not support EEW=64 for index "

Can we truncate the index to nvxXi32 instead of erroring? Would that allow us 
to preserve more test cases?



Comment at: llvm/test/CodeGen/RISCV/rvv/fixed-vectors-masked-gather.ll:1033
 
-define <8 x i64> @mgather_baseidx_sext_v8i8_v8i64(i64* %base, <8 x i8> %idxs, 
<8 x i1> %m, <8 x i64> %passthru) {
-; RV32-LABEL: mgather_baseidx_sext_v8i8_v8i64:

Can these test cases be preserved in an rv64 only test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106518

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

[PATCH] D116755: Revert "[CodeGen] Mark fma as const for Android"

[Alex Xu (Hello71) via Phabricator via cfe-commits]

alxu created this revision.
alxu added a project: clang.
Herald added a subscriber: danielkiss.
alxu requested review of this revision.
Herald added a subscriber: cfe-commits.

This code is intended to give a special exception for platforms which set errno 
in some math functions but not fma. This does not apply to Android, which does 
not set errno in any math functions 
(https://cs.android.com/android/platform/superproject/+/master:bionic/libc/include/math.h;drc=master;l=59).
 The correct implementation for Android is to set -fno-math-errno by default, 
which was done in https://reviews.llvm.org/D51068. Therefore, this special 
exception is no longer needed for Android. Deleting it slightly reduces code 
complexity, clang executable size, compile time, and test time.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116755

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGen/math-builtins.c


Index: clang/test/CodeGen/math-builtins.c
===
--- clang/test/CodeGen/math-builtins.c
+++ clang/test/CodeGen/math-builtins.c
@@ -1,7 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -w -S -o - -emit-llvm
  %s | FileCheck %s -check-prefix=NO__ERRNO
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -w -S -o - -emit-llvm 
-fmath-errno %s | FileCheck %s -check-prefix=HAS_ERRNO
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown-gnu -w -S -o - -emit-llvm 
-fmath-errno %s | FileCheck %s --check-prefix=HAS_ERRNO_GNU
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown-android -w -S -o - 
-emit-llvm -fmath-errno %s | FileCheck %s --check-prefix=HAS_ERRNO_ANDROID
 // RUN: %clang_cc1 -triple x86_64-unknown-windows-msvc -w -S -o - -emit-llvm 
-fmath-errno %s | FileCheck %s --check-prefix=HAS_ERRNO_WIN
 
 // Test attributes and codegen of math builtins.
@@ -345,10 +344,6 @@
 // HAS_ERRNO_GNU: declare float @llvm.fma.f32(float, float, float) 
[[READNONE_INTRINSIC]]
 // HAS_ERRNO_GNU: declare x86_fp80 @llvm.fma.f80(x86_fp80, x86_fp80, x86_fp80) 
[[READNONE_INTRINSIC]]
 
-// HAS_ERRNO_ANDROID: declare double @llvm.fma.f64(double, double, double) 
[[READNONE_INTRINSIC:#[0-9]+]]
-// HAS_ERRNO_ANDROID: declare float @llvm.fma.f32(float, float, float) 
[[READNONE_INTRINSIC]]
-// HAS_ERRNO_ANDROID: declare x86_fp80 @llvm.fma.f80(x86_fp80, x86_fp80, 
x86_fp80) [[READNONE_INTRINSIC]]
-
 // HAS_ERRNO_WIN: declare double @llvm.fma.f64(double, double, double) 
[[READNONE_INTRINSIC:#[0-9]+]]
 // HAS_ERRNO_WIN: declare float @llvm.fma.f32(float, float, float) 
[[READNONE_INTRINSIC]]
 // Long double is just double on win, so no f80 use/declaration.
@@ -696,6 +691,5 @@
 // HAS_ERRNO: attributes [[READNONE]] = { {{.*}}readnone{{.*}} }
 
 // HAS_ERRNO_GNU: attributes [[READNONE_INTRINSIC]] = { {{.*}}readnone{{.*}} }
-// HAS_ERRNO_ANDROID: attributes [[READNONE_INTRINSIC]] = { 
{{.*}}readnone{{.*}} }
 // HAS_ERRNO_WIN: attributes [[READNONE_INTRINSIC]] = { {{.*}}readnone{{.*}} }
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -15191,11 +15191,11 @@
 Context.BuiltinInfo.isConstWithoutErrno(BuiltinID))
   FD->addAttr(ConstAttr::CreateImplicit(Context, FD->getLocation()));
 
-// We make "fma" on some platforms const because we know it does not set
+// We make "fma" on GNU or Windows const because we know it does not set
 // errno in those environments even though it could set errno based on the
 // C standard.
 const llvm::Triple &Trip = Context.getTargetInfo().getTriple();
-if ((Trip.isGNUEnvironment() || Trip.isAndroid() || Trip.isOSMSVCRT()) &&
+if ((Trip.isGNUEnvironment() || Trip.isOSMSVCRT()) &&
 !FD->hasAttr()) {
   switch (BuiltinID) {
   case Builtin::BI__builtin_fma:


Index: clang/test/CodeGen/math-builtins.c
===
--- clang/test/CodeGen/math-builtins.c
+++ clang/test/CodeGen/math-builtins.c
@@ -1,7 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -w -S -o - -emit-llvm  %s | FileCheck %s -check-prefix=NO__ERRNO
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -w -S -o - -emit-llvm -fmath-errno %s | FileCheck %s -check-prefix=HAS_ERRNO
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown-gnu -w -S -o - -emit-llvm -fmath-errno %s | FileCheck %s --check-prefix=HAS_ERRNO_GNU
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown-android -w -S -o - -emit-llvm -fmath-errno %s | FileCheck %s --check-prefix=HAS_ERRNO_ANDROID
 // RUN: %clang_cc1 -triple x86_64-unknown-windows-msvc -w -S -o - -emit-llvm -fmath-errno %s | FileCheck %s --check-prefix=HAS_ERRNO_WIN
 
 // Test attributes and codegen of math builtins.
@@ -345,10 +344,6 @@
 // HAS_ERRNO_GNU: declare float @llvm.fma.f32(float, float, float) [[READNONE_INTRINSIC]]
 // HAS_ERRNO_GNU: declare x86_fp80 @llvm.fma.f80(x86_fp80, x86_fp80

[PATCH] D116726: [clang-format] Fix a crash (assertion) in qualifier alignment when matching template closer is null

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Cool. Thanks!


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

https://reviews.llvm.org/D116726

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

[PATCH] D116337: [clang] set __NO_MATH_ERRNO__ if -fno-math-errno

[Alex Xu (Hello71) via Phabricator via cfe-commits]

alxu updated this revision to Diff 397922.
alxu added a comment.

Correct logic.


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

https://reviews.llvm.org/D116337

Files:
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Preprocessor/predefined-macros.c


Index: clang/test/Preprocessor/predefined-macros.c
===
--- clang/test/Preprocessor/predefined-macros.c
+++ clang/test/Preprocessor/predefined-macros.c
@@ -60,6 +60,15 @@
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-FAST-MATH
 // CHECK-FAST-MATH: #define __FAST_MATH__ 1
 // CHECK-FAST-MATH: #define __FINITE_MATH_ONLY__ 1
+// CHECK-FAST-MATH: #define __NO_MATH_ERRNO__ 1
+//
+// RUN: %clang_cc1 %s -E -dM -fmath-errno -o - \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-MATH-ERRNO
+// CHECK-MATH-ERRNO-NOT: __NO_MATH_ERRNO__
+//
+// RUN: %clang %s -E -dM -fno-math-errno -o - \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-NO-MATH-ERRNO
+// CHECK-NO-MATH-ERRNO: #define __NO_MATH_ERRNO__ 1
 //
 // RUN: %clang_cc1 %s -E -dM -ffinite-math-only -o - \
 // RUN:   | FileCheck -match-full-lines %s 
--check-prefix=CHECK-FINITE-MATH-ONLY
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -1039,6 +1039,9 @@
 
   Builder.defineMacro("__USER_LABEL_PREFIX__", TI.getUserLabelPrefix());
 
+  if (!LangOpts.MathErrno)
+Builder.defineMacro("__NO_MATH_ERRNO__");
+
   if (LangOpts.FastMath || LangOpts.FiniteMathOnly)
 Builder.defineMacro("__FINITE_MATH_ONLY__", "1");
   else


Index: clang/test/Preprocessor/predefined-macros.c
===
--- clang/test/Preprocessor/predefined-macros.c
+++ clang/test/Preprocessor/predefined-macros.c
@@ -60,6 +60,15 @@
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-FAST-MATH
 // CHECK-FAST-MATH: #define __FAST_MATH__ 1
 // CHECK-FAST-MATH: #define __FINITE_MATH_ONLY__ 1
+// CHECK-FAST-MATH: #define __NO_MATH_ERRNO__ 1
+//
+// RUN: %clang_cc1 %s -E -dM -fmath-errno -o - \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-MATH-ERRNO
+// CHECK-MATH-ERRNO-NOT: __NO_MATH_ERRNO__
+//
+// RUN: %clang %s -E -dM -fno-math-errno -o - \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-NO-MATH-ERRNO
+// CHECK-NO-MATH-ERRNO: #define __NO_MATH_ERRNO__ 1
 //
 // RUN: %clang_cc1 %s -E -dM -ffinite-math-only -o - \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-FINITE-MATH-ONLY
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -1039,6 +1039,9 @@
 
   Builder.defineMacro("__USER_LABEL_PREFIX__", TI.getUserLabelPrefix());
 
+  if (!LangOpts.MathErrno)
+Builder.defineMacro("__NO_MATH_ERRNO__");
+
   if (LangOpts.FastMath || LangOpts.FiniteMathOnly)
 Builder.defineMacro("__FINITE_MATH_ONLY__", "1");
   else
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114206: [Clang][ScanDeps] Use the virtual path for module maps

[Sam Elliott via Phabricator via cfe-commits]

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

@Bigcheese I am sorry - after a lot of further investigation, it was due to an 
odd interaction with some downstream changes. I will re-land your changes, with 
correct attribution to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114206

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

[PATCH] D116549: [OpenMP][Clang] Allow passing target features in ISA trait for metadirective clause

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.



Comment at: clang/lib/Parse/ParseOpenMP.cpp:2533
+std::function DiagUnknownTrait = [this, Loc](
+StringRef ISATrait) {};
+TargetOMPContext OMPCtx(ASTContext, std::move(DiagUnknownTrait),

saiislam wrote:
> jdoerfert wrote:
> > Why doesn't this diagnose nothing?
> Because an isa-feature will fail at least once, for either host compilation 
> or device compilation. So, no point in always giving a warning.
That is debatable. 

First, if I compile for a single architecture there is no device compilation 
and it should warn.
Second, if I place the metadirective into a declare variant function or add a 
`kind(...)` selector to it it will also not warn even if you have multiple 
architectures.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116549

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

[PATCH] D116755: Revert "[CodeGen] Mark fma as const for Android"

[Pirama Arumuga Nainar via Phabricator via cfe-commits]

pirama accepted this revision.
pirama added a subscriber: srhines.
pirama added a comment.
This revision is now accepted and ready to land.

Thanks for the cleanup here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116755

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

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for your patience! I finally had the chance to go through this a bit 
more. I identified a bunch of tiny nits (the review feedback may look scary, 
but most should be trivial changes), but a few larger things about the design 
as well that are worth thinking about.

One thing that I'm still a bit worried about is validating the output document. 
The unit tests are a good start and the sort of thing we need for this 
functionality, but I'm also worried we won't find substantial design issues 
until we finally get SARIF results out of Clang or the static analyzer so we 
can see whether tools can actually *import* the SARIF we produce. I don't think 
we need in-tree tests for that sort of thing, but it'd definitely make me feel 
a lot more comfortable if we had external evidence that we're producing valid 
SARIF given that this is an exchange format. That said, I also don't want this 
to become a cumbersome patch for you to work on or for us to review, so I'm not 
certain what the best answer is here yet.




Comment at: clang/include/clang/Basic/Sarif.h:46
+
+using namespace llvm;
+

Having thought about this a bit more, I think this line should be removed 
because it's within a header file, so all includes of this header file will be 
impacted and likely without knowing it. I'm very sorry for the churn, but I 
think this should go and the `llvm::` prefixes should be brought back within 
the header file. Within the source file, it's fine to use the `using`.



Comment at: clang/include/clang/Basic/Sarif.h:112
+
+  explicit SarifArtifact(const SarifArtifactLocation &Loc) : Location(Loc) {}
+

Should we delete the default ctor?



Comment at: clang/include/clang/Basic/Sarif.h:162
+public:
+  static ThreadFlow create() { return {}; }
+

One question this raises for me is whether we should be enforcing invariants 
from the SARIF spec as part of this interface or not. Currently, you can create 
a thread flow that has no importance or a rule without a name/id, etc. That 
means it's pretty easy to create SARIF that won't validate properly. One 
possible way to alleviate this would be for the `create()` methods/ctors to 
require these be set when creating the objects. However, I can imagine there 
will be times when that is awkward due to following the builder pattern with 
the interface. Another option would be to have some validation methods on each 
of the interfaces and the whole tree gets validated after construction, but 
this could have performance impacts.

What are your thoughts?



Comment at: clang/include/clang/Basic/Sarif.h:248
+  // chosen because it has to be non-negative, and because the JSON encoder
+  // used requires this be a type that can be safely promoted to \c int64_t
+  uint32_t RuleIdx;





Comment at: clang/include/clang/Basic/Sarif.h:339
+public:
+  /// Create a new empty SARIF document
+  SarifDocumentWriter() : Closed(true){};





Comment at: clang/include/clang/Basic/Sarif.h:340
+  /// Create a new empty SARIF document
+  SarifDocumentWriter() : Closed(true){};
+

Once you use the default ctor, there's no way to associate language options 
with the document writer. Is it wise to expose this constructor? (If we didn't, 
then we could store the `LangOptions` as a const reference instead of making a 
copy in the other constructor. Given that we never mutate the options, I think 
that's a win.)



Comment at: clang/include/clang/Basic/Sarif.h:342
+
+  /// Create a new empty SARIF document with the given language options
+  SarifDocumentWriter(const LangOptions &LangOpts)





Comment at: clang/include/clang/Basic/Sarif.h:344
+  SarifDocumentWriter(const LangOptions &LangOpts)
+  : LangOpts(LangOpts), Closed(true) {}
+





Comment at: clang/include/clang/Basic/Sarif.h:346
+
+  /// Release resources held by this SARIF document
+  ~SarifDocumentWriter() = default;





Comment at: clang/include/clang/Basic/Sarif.h:400
+private:
+  /// Langauge options to use for the current SARIF document
+  const LangOptions LangOpts;





Comment at: clang/include/clang/Basic/Sarif.h:406
+  /// This could be a document that is freshly created, or has recently
+  /// finished writing to a previous run
+  bool Closed;





Comment at: clang/include/clang/Basic/Sarif.h:407
+  /// finished writing to a previous run
+  bool Closed;
+





Comment at: clang/include/clang/Basic/Sarif.h:80-82
+  static SarifArtifactLocation create(StringRef URI) {
+return SarifArtifactLocation{URI};
+  }

vaibhav.y wrote:
> aaron.ballman wrote:
> > One thing that's worth 

[PATCH] D116256: [-fms-extensions] Make some exception specification warnings/errors compatible with what cl.exe does

[Amy Huang via Phabricator via cfe-commits]

akhuang updated this revision to Diff 397930.
akhuang marked an inline comment as done.
akhuang added a comment.

Add std=c++17 to the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116256

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/test/SemaCXX/MicrosoftCompatibility.cpp


Index: clang/test/SemaCXX/MicrosoftCompatibility.cpp
===
--- clang/test/SemaCXX/MicrosoftCompatibility.cpp
+++ clang/test/SemaCXX/MicrosoftCompatibility.cpp
@@ -2,6 +2,8 @@
 // RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -std=c++11 
-Wmicrosoft -verify -fms-compatibility -fexceptions -fcxx-exceptions 
-fms-compatibility-version=19.27
 // RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -std=c++11 
-Wmicrosoft -verify -fms-compatibility -fexceptions -fcxx-exceptions 
-fms-compatibility-version=19.00
 // RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -std=c++11 
-Wmicrosoft -verify -fms-compatibility -fexceptions -fcxx-exceptions 
-fms-compatibility-version=18.00
+// RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -std=c++17 
-Wmicrosoft -verify -fms-compatibility -fexceptions -fcxx-exceptions
+
 
 #if defined(_HAS_CHAR16_T_LANGUAGE_SUPPORT) && _HAS_CHAR16_T_LANGUAGE_SUPPORT
 char16_t x;
@@ -350,6 +352,7 @@
 void foo(); // expected-note {{previous declaration}}
 void foo() throw(); // expected-warning {{exception specification in 
declaration does not match previous declaration}}
 
+#if __cplusplus < 201703L
 void r6() throw(...); // expected-note {{previous declaration}}
 void r6() throw(int); // expected-warning {{exception specification in 
declaration does not match previous declaration}}
 
@@ -362,6 +365,7 @@
   virtual void f2() throw(...);
   virtual void f3();
 };
+#endif
 
 class A {
   virtual ~A() throw();
@@ -377,14 +381,14 @@
 #endif
 };
 
-}
+void f4() throw(); // expected-note {{previous declaration is here}}
+void f4() {}   // expected-warning {{'f4' is missing exception 
specification 'throw()'}}
 
-namespace PR25265 {
-struct S {
-  int fn() throw(); // expected-note {{previous declaration is here}}
-};
+__declspec(nothrow) void f5();
+void f5() {}
 
-int S::fn() { return 0; } // expected-warning {{is missing exception 
specification}}
+void f6() noexcept; // expected-note {{previous declaration is here}}
+void f6() {}// expected-error {{'f6' is missing exception 
specification 'noexcept'}}
 }
 
 namespace PR43265 {
Index: clang/lib/Sema/SemaExceptionSpec.cpp
===
--- clang/lib/Sema/SemaExceptionSpec.cpp
+++ clang/lib/Sema/SemaExceptionSpec.cpp
@@ -391,9 +391,8 @@
 NewProto->getExtProtoInfo().withExceptionSpec(ESI)));
   }
 
-  if (getLangOpts().MSVCCompat && ESI.Type != EST_DependentNoexcept) {
-// Allow missing exception specifications in redeclarations as an 
extension.
-DiagID = diag::ext_ms_missing_exception_specification;
+  if (getLangOpts().MSVCCompat && isDynamicExceptionSpec(ESI.Type)) {
+DiagID = diag::ext_missing_exception_specification;
 ReturnValueOnError = false;
   } else if (New->isReplaceableGlobalAllocationFunction() &&
  ESI.Type != EST_DependentNoexcept) {
@@ -402,6 +401,10 @@
 DiagID = diag::ext_missing_exception_specification;
 ReturnValueOnError = false;
   } else if (ESI.Type == EST_NoThrow) {
+// Don't emit any warning for missing 'nothrow' in MSVC.
+if (getLangOpts().MSVCCompat) {
+  return false;
+}
 // Allow missing attribute 'nothrow' in redeclarations, since this is a 
very
 // common omission.
 DiagID = diag::ext_missing_exception_specification;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1695,9 +1695,6 @@
 def ext_missing_exception_specification : ExtWarn<
   err_missing_exception_specification.Text>,
   InGroup>;
-def ext_ms_missing_exception_specification : ExtWarn<
-  err_missing_exception_specification.Text>,
-  InGroup;
 def err_noexcept_needs_constant_expression : Error<
   "argument to noexcept specifier must be a constant expression">;
 def err_exception_spec_not_parsed : Error<


Index: clang/test/SemaCXX/MicrosoftCompatibility.cpp
===
--- clang/test/SemaCXX/MicrosoftCompatibility.cpp
+++ clang/test/SemaCXX/MicrosoftCompatibility.cpp
@@ -2,6 +2,8 @@
 // RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -std=c++11 -Wmicrosoft -verify -fms-compatibility -fexceptions -fcxx-exceptions -fms-compatibility-version=19.27
 // RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -std=c++11 -Wmicrosoft -verify -fms-compatibility -fexceptions -f

[PATCH] D116337: [clang] set __NO_MATH_ERRNO__ if -fno-math-errno

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Getting closer! Now there are new CI pipeline failures (some tests look like 
they need updating).


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

https://reviews.llvm.org/D116337

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

[PATCH] D116337: [clang] set __NO_MATH_ERRNO__ if -fno-math-errno

[Alex Xu (Hello71) via Phabricator via cfe-commits]

alxu updated this revision to Diff 397937.
alxu added a comment.
Herald added subscribers: aheejin, dschuff.

Update init.c, init-aarch64.c tests.


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

https://reviews.llvm.org/D116337

Files:
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Preprocessor/init-aarch64.c
  clang/test/Preprocessor/init.c
  clang/test/Preprocessor/predefined-macros.c


Index: clang/test/Preprocessor/predefined-macros.c
===
--- clang/test/Preprocessor/predefined-macros.c
+++ clang/test/Preprocessor/predefined-macros.c
@@ -60,6 +60,15 @@
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-FAST-MATH
 // CHECK-FAST-MATH: #define __FAST_MATH__ 1
 // CHECK-FAST-MATH: #define __FINITE_MATH_ONLY__ 1
+// CHECK-FAST-MATH: #define __NO_MATH_ERRNO__ 1
+//
+// RUN: %clang_cc1 %s -E -dM -fmath-errno -o - \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-MATH-ERRNO
+// CHECK-MATH-ERRNO-NOT: __NO_MATH_ERRNO__
+//
+// RUN: %clang %s -E -dM -fno-math-errno -o - \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-NO-MATH-ERRNO
+// CHECK-NO-MATH-ERRNO: #define __NO_MATH_ERRNO__ 1
 //
 // RUN: %clang_cc1 %s -E -dM -ffinite-math-only -o - \
 // RUN:   | FileCheck -match-full-lines %s 
--check-prefix=CHECK-FINITE-MATH-ONLY
Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -1691,6 +1691,7 @@
 // WEBASSEMBLY64-NEXT:#define __LONG_MAX__ 9223372036854775807L
 // WEBASSEMBLY64-NEXT:#define __LP64__ 1
 // WEBASSEMBLY-NEXT:#define __NO_INLINE__ 1
+// WEBASSEMBLY-NEXT:#define __NO_MATH_ERRNO__ 1
 // WEBASSEMBLY-NEXT:#define __OBJC_BOOL_IS_BOOL 0
 // WEBASSEMBLY-NEXT:#define __OPENCL_MEMORY_SCOPE_ALL_SVM_DEVICES 3
 // WEBASSEMBLY-NEXT:#define __OPENCL_MEMORY_SCOPE_DEVICE 2
Index: clang/test/Preprocessor/init-aarch64.c
===
--- clang/test/Preprocessor/init-aarch64.c
+++ clang/test/Preprocessor/init-aarch64.c
@@ -192,6 +192,7 @@
 // AARCH64-NEXT: #define __LONG_MAX__ 9223372036854775807L
 // AARCH64-NEXT: #define __LP64__ 1
 // AARCH64-NEXT: #define __NO_INLINE__ 1
+// AARCH64-NEXT: #define __NO_MATH_ERRNO__ 1
 // AARCH64-NEXT: #define __OBJC_BOOL_IS_BOOL 0
 // AARCH64-NEXT: #define __OPENCL_MEMORY_SCOPE_ALL_SVM_DEVICES 3
 // AARCH64-NEXT: #define __OPENCL_MEMORY_SCOPE_DEVICE 2
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -1039,6 +1039,9 @@
 
   Builder.defineMacro("__USER_LABEL_PREFIX__", TI.getUserLabelPrefix());
 
+  if (!LangOpts.MathErrno)
+Builder.defineMacro("__NO_MATH_ERRNO__");
+
   if (LangOpts.FastMath || LangOpts.FiniteMathOnly)
 Builder.defineMacro("__FINITE_MATH_ONLY__", "1");
   else


Index: clang/test/Preprocessor/predefined-macros.c
===
--- clang/test/Preprocessor/predefined-macros.c
+++ clang/test/Preprocessor/predefined-macros.c
@@ -60,6 +60,15 @@
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-FAST-MATH
 // CHECK-FAST-MATH: #define __FAST_MATH__ 1
 // CHECK-FAST-MATH: #define __FINITE_MATH_ONLY__ 1
+// CHECK-FAST-MATH: #define __NO_MATH_ERRNO__ 1
+//
+// RUN: %clang_cc1 %s -E -dM -fmath-errno -o - \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-MATH-ERRNO
+// CHECK-MATH-ERRNO-NOT: __NO_MATH_ERRNO__
+//
+// RUN: %clang %s -E -dM -fno-math-errno -o - \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-NO-MATH-ERRNO
+// CHECK-NO-MATH-ERRNO: #define __NO_MATH_ERRNO__ 1
 //
 // RUN: %clang_cc1 %s -E -dM -ffinite-math-only -o - \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-FINITE-MATH-ONLY
Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -1691,6 +1691,7 @@
 // WEBASSEMBLY64-NEXT:#define __LONG_MAX__ 9223372036854775807L
 // WEBASSEMBLY64-NEXT:#define __LP64__ 1
 // WEBASSEMBLY-NEXT:#define __NO_INLINE__ 1
+// WEBASSEMBLY-NEXT:#define __NO_MATH_ERRNO__ 1
 // WEBASSEMBLY-NEXT:#define __OBJC_BOOL_IS_BOOL 0
 // WEBASSEMBLY-NEXT:#define __OPENCL_MEMORY_SCOPE_ALL_SVM_DEVICES 3
 // WEBASSEMBLY-NEXT:#define __OPENCL_MEMORY_SCOPE_DEVICE 2
Index: clang/test/Preprocessor/init-aarch64.c
===
--- clang/test/Preprocessor/init-aarch64.c
+++ clang/test/Preprocessor/init-aarch64.c
@@ -192,6 +192,7 @@
 // AARCH64-NEXT: #define __LONG_MAX__ 9223372036854775807L
 // AARCH64-NEXT: #define __LP64__ 1
 // AARCH64-NEXT: #define __NO_INLINE__ 1
+// AARCH64-NEXT: #define __NO_MATH_ERRNO__ 1
 // AARCH64-NEXT: #define __

[PATCH] D116337: [clang] set __NO_MATH_ERRNO__ if -fno-math-errno

[Alex Xu (Hello71) via Phabricator via cfe-commits]

alxu updated this revision to Diff 397938.
alxu added a comment.

Re-add context lines.


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

https://reviews.llvm.org/D116337

Files:
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Preprocessor/init-aarch64.c
  clang/test/Preprocessor/init.c
  clang/test/Preprocessor/predefined-macros.c


Index: clang/test/Preprocessor/predefined-macros.c
===
--- clang/test/Preprocessor/predefined-macros.c
+++ clang/test/Preprocessor/predefined-macros.c
@@ -60,6 +60,15 @@
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-FAST-MATH
 // CHECK-FAST-MATH: #define __FAST_MATH__ 1
 // CHECK-FAST-MATH: #define __FINITE_MATH_ONLY__ 1
+// CHECK-FAST-MATH: #define __NO_MATH_ERRNO__ 1
+//
+// RUN: %clang_cc1 %s -E -dM -fmath-errno -o - \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-MATH-ERRNO
+// CHECK-MATH-ERRNO-NOT: __NO_MATH_ERRNO__
+//
+// RUN: %clang %s -E -dM -fno-math-errno -o - \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-NO-MATH-ERRNO
+// CHECK-NO-MATH-ERRNO: #define __NO_MATH_ERRNO__ 1
 //
 // RUN: %clang_cc1 %s -E -dM -ffinite-math-only -o - \
 // RUN:   | FileCheck -match-full-lines %s 
--check-prefix=CHECK-FINITE-MATH-ONLY
Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -1691,6 +1691,7 @@
 // WEBASSEMBLY64-NEXT:#define __LONG_MAX__ 9223372036854775807L
 // WEBASSEMBLY64-NEXT:#define __LP64__ 1
 // WEBASSEMBLY-NEXT:#define __NO_INLINE__ 1
+// WEBASSEMBLY-NEXT:#define __NO_MATH_ERRNO__ 1
 // WEBASSEMBLY-NEXT:#define __OBJC_BOOL_IS_BOOL 0
 // WEBASSEMBLY-NEXT:#define __OPENCL_MEMORY_SCOPE_ALL_SVM_DEVICES 3
 // WEBASSEMBLY-NEXT:#define __OPENCL_MEMORY_SCOPE_DEVICE 2
Index: clang/test/Preprocessor/init-aarch64.c
===
--- clang/test/Preprocessor/init-aarch64.c
+++ clang/test/Preprocessor/init-aarch64.c
@@ -192,6 +192,7 @@
 // AARCH64-NEXT: #define __LONG_MAX__ 9223372036854775807L
 // AARCH64-NEXT: #define __LP64__ 1
 // AARCH64-NEXT: #define __NO_INLINE__ 1
+// AARCH64-NEXT: #define __NO_MATH_ERRNO__ 1
 // AARCH64-NEXT: #define __OBJC_BOOL_IS_BOOL 0
 // AARCH64-NEXT: #define __OPENCL_MEMORY_SCOPE_ALL_SVM_DEVICES 3
 // AARCH64-NEXT: #define __OPENCL_MEMORY_SCOPE_DEVICE 2
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -1039,6 +1039,9 @@
 
   Builder.defineMacro("__USER_LABEL_PREFIX__", TI.getUserLabelPrefix());
 
+  if (!LangOpts.MathErrno)
+Builder.defineMacro("__NO_MATH_ERRNO__");
+
   if (LangOpts.FastMath || LangOpts.FiniteMathOnly)
 Builder.defineMacro("__FINITE_MATH_ONLY__", "1");
   else


Index: clang/test/Preprocessor/predefined-macros.c
===
--- clang/test/Preprocessor/predefined-macros.c
+++ clang/test/Preprocessor/predefined-macros.c
@@ -60,6 +60,15 @@
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-FAST-MATH
 // CHECK-FAST-MATH: #define __FAST_MATH__ 1
 // CHECK-FAST-MATH: #define __FINITE_MATH_ONLY__ 1
+// CHECK-FAST-MATH: #define __NO_MATH_ERRNO__ 1
+//
+// RUN: %clang_cc1 %s -E -dM -fmath-errno -o - \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-MATH-ERRNO
+// CHECK-MATH-ERRNO-NOT: __NO_MATH_ERRNO__
+//
+// RUN: %clang %s -E -dM -fno-math-errno -o - \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-NO-MATH-ERRNO
+// CHECK-NO-MATH-ERRNO: #define __NO_MATH_ERRNO__ 1
 //
 // RUN: %clang_cc1 %s -E -dM -ffinite-math-only -o - \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-FINITE-MATH-ONLY
Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -1691,6 +1691,7 @@
 // WEBASSEMBLY64-NEXT:#define __LONG_MAX__ 9223372036854775807L
 // WEBASSEMBLY64-NEXT:#define __LP64__ 1
 // WEBASSEMBLY-NEXT:#define __NO_INLINE__ 1
+// WEBASSEMBLY-NEXT:#define __NO_MATH_ERRNO__ 1
 // WEBASSEMBLY-NEXT:#define __OBJC_BOOL_IS_BOOL 0
 // WEBASSEMBLY-NEXT:#define __OPENCL_MEMORY_SCOPE_ALL_SVM_DEVICES 3
 // WEBASSEMBLY-NEXT:#define __OPENCL_MEMORY_SCOPE_DEVICE 2
Index: clang/test/Preprocessor/init-aarch64.c
===
--- clang/test/Preprocessor/init-aarch64.c
+++ clang/test/Preprocessor/init-aarch64.c
@@ -192,6 +192,7 @@
 // AARCH64-NEXT: #define __LONG_MAX__ 9223372036854775807L
 // AARCH64-NEXT: #define __LP64__ 1
 // AARCH64-NEXT: #define __NO_INLINE__ 1
+// AARCH64-NEXT: #define __NO_MATH_ERRNO__ 1
 // AARCH64-NEXT: #define __OBJC_BOOL_IS_BOOL 0
 // AARCH64-NEXT: #define __OPENCL_MEMO

[PATCH] D116635: Add warning to detect when calls passing arguments are made to functions without prototypes.

[Dan Liew via Phabricator via cfe-commits]

delcypher added inline comments.



Comment at: clang/test/Sema/warn-calls-without-prototype.c:39
+  return a + b +c;
+}
+

NoQ wrote:
> delcypher wrote:
> > delcypher wrote:
> > > delcypher wrote:
> > > > @NoQ Any ideas about this?  It seems kind of weird that when merging 
> > > > `not_a_prototype3` prototype with the K&R style definition of 
> > > > `not_a_prototype3` that the resulting FunctionDecl we see at the call 
> > > > site in `call_to_function_without_prototype3` is marked as not having a 
> > > > prototype.
> > > > 
> > > > If I flip the order (see `not_a_prototype6`) then the merged 
> > > > declaration is marked as having a prototype.
> > > > 
> > > > I'm not sure if this is a bug in `Sema::MergeFunctionDecl` or if this 
> > > > just a peculiarity of K&R style function definitions.
> > > I suspect the problem might be here in `Sema::MergeFunctionDecl`.
> > > 
> > > ```lang=c++
> > >// C: Function types need to be compatible, not identical. This handles
> > >   // duplicate function decls like "void f(int); void f(enum X);" 
> > > properly.
> > >   if (!getLangOpts().CPlusPlus &&
> > >   Context.typesAreCompatible(OldQType, NewQType)) {
> > > const FunctionType *OldFuncType = OldQType->getAs();
> > > const FunctionType *NewFuncType = NewQType->getAs();
> > > const FunctionProtoType *OldProto = nullptr;
> > > if (MergeTypeWithOld && isa(NewFuncType) &&
> > > (OldProto = dyn_cast(OldFuncType))) {
> > >   // The old declaration provided a function prototype, but the
> > >   // new declaration does not. Merge in the prototype.
> > > ```
> > > 
> > > ` isa(NewFuncType)` is false in this particular 
> > > case, however `New` doesn't have a prototype (i.e. `New->hasPrototype()` 
> > > is false). One fix might be to replace 
> > > `isa(NewFuncType)` with 
> > > 
> > > ```
> > > (isa(NewFuncType) || !New->hasPrototype())
> > > ```
> > > 
> > > However, I don't really know this code well enough to know if that's the 
> > > right fix.
> > Okay. I think the above would actually be the wrong location for a fix 
> > because in this case we don't need to go down the path that synthesizes the 
> > parameters because we already know them for both `old` and `new` in this 
> > situation.
> > 
> > Instead I think the change would have to be in 
> > `Sema::MergeCompatibleFunctionDecls` to do something like.
> > 
> > ```lang=c++
> >   // If New is a K&R function definition it will be marked
> >   // as not having a prototype. If `Old` has a prototype
> >   // then to "merge" we should mark the K&R function as having a prototype.
> >   if (!getLangOpts().CPlusPlus && Old->hasPrototype() && 
> > !New->hasPrototype())
> > New->setHasInheritedPrototype(); 
> > ```
> > 
> > What I'm not sure about is if this is semantically the right thing to do. 
> > Thoughts?
> Ok dunno but I definitely find this whole thing surprising. I'd expect this 
> example to be the entirely normal situation for this code, where it sees that 
> the new declaration has no prototype so it "inherits" it from the old 
> declaration. But you're saying that
> 
> > `isa(NewFuncType)` is false in this particular case
> 
> Where does that proto-type come from then? I only see this code affecting the 
> type
> ```lang=c++
>3523   if (RequiresAdjustment) {
>3524 const FunctionType *AdjustedType = 
> New->getType()->getAs();
>3525 AdjustedType = Context.adjustFunctionType(AdjustedType, 
> NewTypeInfo);
>3526 New->setType(QualType(AdjustedType, 0));
>3527 NewQType = Context.getCanonicalType(New->getType());
>3528   }
> ```
> which doesn't seem to touch no-proto-types:
> ```lang=c++
>3094 const FunctionType *ASTContext::adjustFunctionType(const FunctionType 
> *T,
>3095
> FunctionType::ExtInfo Info) {
>3096   if (T->getExtInfo() == Info)
>3097 return T;
>3098
>3099   QualType Result;
>3100   if (const auto *FNPT = dyn_cast(T)) {
>3101 Result = getFunctionNoProtoType(FNPT->getReturnType(), Info);
>...
>3107   }
>3108
>3109   return cast(Result.getTypePtr());
>3110 }
> ```
> So it sounds like `New->getType()` was already with prototype from the start? 
> Maybe whatever code has set that type should also have set the 
> `HasInheritedPrototype` flag?
> Ok dunno but I definitely find this whole thing surprising. I'd expect this 
> example to be the entirely normal situation for this code, where it sees that 
> the new declaration has no prototype so it "inherits" it from the old 
> declaration. But you're saying that
> 
> `isa(NewFuncType) `is false in this particular case

Yep.

```
(lldb) p NewFuncType->dump()
FunctionProtoType 0x128829430 'int (int, int)' cdecl
|-BuiltinType 0x12782bf10 'int'
|-BuiltinType 0x12782bf10 'int'
`-BuiltinType 0x12782bf10 'int'
(lldb) p OldFuncType->dump()
FunctionProtoType 0x128829430 'int (int, int)' cdecl

[PATCH] D116378: [clang-tidy] Disable clang-tidy warnings from system macros

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a minor nit, thanks! This was a good catch.




Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp:32-33
+  Check->diag(MacroNameTok.getLocation(),
+  "do not use va_arg to define c-style vararg functions; "
+  "use variadic templates instead");
+}

Rather than duplicate the diagnostic text, I think we should hoist the text 
into a constant string that's used in both places.

That makes it easier to fix the diagnostic (in a follow up) to properly quote 
`va_arg` and convert the grammar to singular form instead of plural (e.g, `do 
not use 'va_arg' to define a C-style vararg function; use a variadic template 
instead`).


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

https://reviews.llvm.org/D116378

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