[PATCH] D113708: [clang-tidy] Fix `bugprone-use-after-move` check to also consider moves in constructor initializers

[Carlos Galvez via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc3e3c762098e: [clang-tidy] Fix `bugprone-use-after-move` 
check to also consider moves in… (authored by fwolff, committed by 
carlosgalvezp).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113708

Files:
  clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -1338,3 +1338,15 @@
   Foo Other{std::move(Bar)};
 }
 } // namespace UnevalContext
+
+class PR38187 {
+public:
+  PR38187(std::string val) : val_(std::move(val)) {
+val.empty();
+// CHECK-NOTES: [[@LINE-1]]:5: warning: 'val' used after it was moved
+// CHECK-NOTES: [[@LINE-3]]:30: note: move occurred here
+  }
+
+private:
+  std::string val_;
+};
Index: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -129,8 +129,12 @@
   Visited.clear();
 
   const CFGBlock *Block = BlockMap->blockContainingStmt(MovingCall);
-  if (!Block)
-return false;
+  if (!Block) {
+// This can happen if MovingCall is in a constructor initializer, which is
+// not included in the CFG because the CFG is built only from the function
+// body.
+Block = &TheCFG->getEntry();
+  }
 
   return findInternal(Block, MovingCall, MovedVariable, TheUseAfterMove);
 }


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -1338,3 +1338,15 @@
   Foo Other{std::move(Bar)};
 }
 } // namespace UnevalContext
+
+class PR38187 {
+public:
+  PR38187(std::string val) : val_(std::move(val)) {
+val.empty();
+// CHECK-NOTES: [[@LINE-1]]:5: warning: 'val' used after it was moved
+// CHECK-NOTES: [[@LINE-3]]:30: note: move occurred here
+  }
+
+private:
+  std::string val_;
+};
Index: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -129,8 +129,12 @@
   Visited.clear();
 
   const CFGBlock *Block = BlockMap->blockContainingStmt(MovingCall);
-  if (!Block)
-return false;
+  if (!Block) {
+// This can happen if MovingCall is in a constructor initializer, which is
+// not included in the CFG because the CFG is built only from the function
+// body.
+Block = &TheCFG->getEntry();
+  }
 
   return findInternal(Block, MovingCall, MovedVariable, TheUseAfterMove);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] c3e3c76 - [clang-tidy] Fix `bugprone-use-after-move` check to also consider moves in constructor initializers

[Carlos Galvez via cfe-commits]

Author: Fabian Wolff
Date: 2021-11-15T07:41:35Z
New Revision: c3e3c762098e8d425731bb40f3b8b04dac1013f3

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

LOG: [clang-tidy] Fix `bugprone-use-after-move` check to also consider moves in 
constructor initializers

Fixes PR#38187. Constructors are actually already checked,
but only as functions, i.e. the check only looks at the
constructor body and not at the initializers, which misses
the (common) case where constructor parameters are moved
as part of an initializer expression.

One remaining false negative is when both the move //and//
the use-after-move occur in constructor initializers.
This is a lot more difficult to handle, though, because
the `bugprone-use-after-move` check is currently based on
a CFG that only takes the body into account, not the
initializers, so e.g. initialization order would have to
manually be considered. I will file a follow-up issue for
this once PR#38187 is closed.

Reviewed By: carlosgalvezp

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
index 136d8f862b956..064b6ae19784e 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -129,8 +129,12 @@ bool UseAfterMoveFinder::find(Stmt *FunctionBody, const 
Expr *MovingCall,
   Visited.clear();
 
   const CFGBlock *Block = BlockMap->blockContainingStmt(MovingCall);
-  if (!Block)
-return false;
+  if (!Block) {
+// This can happen if MovingCall is in a constructor initializer, which is
+// not included in the CFG because the CFG is built only from the function
+// body.
+Block = &TheCFG->getEntry();
+  }
 
   return findInternal(Block, MovingCall, MovedVariable, TheUseAfterMove);
 }

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
index 73ca59ccc91bf..e26db0f6793e0 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -1338,3 +1338,15 @@ void typeId() {
   Foo Other{std::move(Bar)};
 }
 } // namespace UnevalContext
+
+class PR38187 {
+public:
+  PR38187(std::string val) : val_(std::move(val)) {
+val.empty();
+// CHECK-NOTES: [[@LINE-1]]:5: warning: 'val' used after it was moved
+// CHECK-NOTES: [[@LINE-3]]:30: note: move occurred here
+  }
+
+private:
+  std::string val_;
+};



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

[PATCH] D113848: [clang-tidy][NFC] Refactor ClangTidyDiagnosticConsumer files

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

I agree with the comments, but I didn't want to touch any code other than 
moving things around, since it's hard to see the changes in the diff otherwise. 
I was also unsure if this was "LLVM convention" or a mistake. I'm happy to fix 
in a separate patch if that's OK?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113848

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

[PATCH] D113847: [clang-tidy][NFC] Simplify ClangTidyStats

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:50
 struct ClangTidyStats {
-  ClangTidyStats()
-  : ErrorsDisplayed(0), ErrorsIgnoredCheckFilter(0), 
ErrorsIgnoredNOLINT(0),
-ErrorsIgnoredNonUserCode(0), ErrorsIgnoredLineFilter(0) {}
+  int ErrorsDisplayed{0};
+  int ErrorsIgnoredCheckFilter{0};

salman-javed-nz wrote:
> What's the prevalent style for class member initialization? `=` or `{}`?
> cppcoreguidelines-prefer-member-initializer defaults to `{}` but I have seen 
> both types in the code.
I tried to find this info in the LLVM coding guidelines but didn't find 
anything, so I assume it's maybe up to developers' discretion.

I prefer using braced initialization, since it prevents implicit conversions:
https://godbolt.org/z/4sP4rGsrY

More strict guidelines like Autosar enforce this. Also CppCoreGuidelines prefer 
that style as you point out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113847

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

[PATCH] D112777: [X86][FP16] add alias for f*mul_*ch intrinsics

[Phoebe Wang via Phabricator via cfe-commits]

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

LGTM. Please wait one or two days for other comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112777

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

[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

[Warren Ristow via Phabricator via cfe-commits]

wristow added a comment.

The Release Note change here says:

  Floating Point Support in Clang
  ---
  - The -ffp-model=precise now implies -ffp-contract=on rather than
-ffp-contract=fast, and the documentation of these features has been
clarified. Previously, the documentation claimed that -ffp-model=precise was
the default, but this was incorrect because the precise model implied
-ffp-contract=fast, whereas the default behavior is -ffp-contract=on.
-ffp-model=precise is now exactly the default mode of the compiler.

Unless I'm missing something, there is a related change here that I think 
should be more overtly noted (given the discussions in this review, I //think// 
this additional change is understood/expected, but I'm surprised it's not 
pointed out explicitly -- so maybe I'm misunderstanding).

Specifically, this commit explicitly sets `-ffp-contract=on` in the default 
mode (which is what the documentation said, and continues to say).  But 
previously, there wasn't //any// explicit setting of `-ffp-contract` by default 
(and I think that lack of an explicit setting, was equivalent to 
`-ffp-contract=off`).

So with this commit, we now enable FMA by default (even at `-O0`). Noting the 
semantic change that FMA is now being enabled by default seems sensible.

Succinctly, in terms of the Release Note, the documentation claims that 
`-ffp-contract=on` is the default, but in fact the behavior //was// as though 
`-ffp-contract=off` was the default.  The default now really is 
`-ffp-contract=on`.

__

Also, I see a relatively minor point about `-ffp-contract` in the Users Manual. 
 It says that setting `-ffast-math` implies `-ffp-contract=fast`, and it says 
that setting `-ffp-model=fast` "Behaves identically to specifying both 
`-ffast-math` and `ffp-contract=fast`".  But that's redundant, since 
`-ffast-math` already implies  `-ffp-contract=fast`.  That is, `-ffast-math` 
and `-ffp-model=fast` are equivalent.


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

https://reviews.llvm.org/D107994

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

[PATCH] D113830: [clang-tidy] Fix false positive in `readability-identifier-naming` check involving `override` attribute

[Salman Javed via Phabricator via cfe-commits]

salman-javed-nz added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp:299
   // CHECK-FIXES: {{^}}  void v_Bad_Base_Method() override {}
+  void BadBaseMethodNoAttr() {}
+  // CHECK-FIXES: {{^}}  void v_Bad_Base_Method_No_Attr() {}

I think this line could spell out more clearly that it is testing the case 
where the `override` keyword is omitted. I don't think the `NoAttr()` suffix 
says enough. `override` is not the only Attr.

Possible solutions:
- You could incorporate the word "override" in the method name e.g. 
`BadBaseMethodNoOverride()`, or
- You could put a `/* override */` where `override` normally would be to bring 
attention to the fact that it is missing, or
- You could add a comment explaining what's going on, like the `// Overriding a 
badly-named base isn't a new violation` a couple of lines above.

Feel free to do what you think is the least janky.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp:299
   // CHECK-FIXES: {{^}}  void v_Bad_Base_Method() override {}
+  void BadBaseMethodNoAttr() {}
+  // CHECK-FIXES: {{^}}  void v_Bad_Base_Method_No_Attr() {}

salman-javed-nz wrote:
> I think this line could spell out more clearly that it is testing the case 
> where the `override` keyword is omitted. I don't think the `NoAttr()` suffix 
> says enough. `override` is not the only Attr.
> 
> Possible solutions:
> - You could incorporate the word "override" in the method name e.g. 
> `BadBaseMethodNoOverride()`, or
> - You could put a `/* override */` where `override` normally would be to 
> bring attention to the fact that it is missing, or
> - You could add a comment explaining what's going on, like the `// Overriding 
> a badly-named base isn't a new violation` a couple of lines above.
> 
> Feel free to do what you think is the least janky.




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp:313
+BadBaseMethodNoAttr();
+// CHECK-FIXES: {{^}}v_Bad_Base_Method_No_Attr();
   }

I would throw in the
```
this->BadBaseMethodNoAttr();
AOverridden::BadBaseMethodNoAttr();
COverriding::BadBaseMethodNoAttr();
```
lines as well.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp:335
+  //(note that there could be specializations of the template base 
class, though)
+  void BadBaseMethod() override{}
+





Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp:365
+  // Overriding a badly-named base isn't a new violation.
+  void BadBaseMethod() override{}
+  // CHECK-FIXES: {{^}}  void v_Bad_Base_Method() override{}





Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp:370
+template class CTIOverriding;
+
 template 

Are you missing another `VirtualCall()` function here? (to test `ATIOverridden` 
and `CTIOverriding`?)


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

https://reviews.llvm.org/D113830

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

[PATCH] D77598: Integral template argument suffix and cast printing

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/AST/DeclPrinter.cpp:1101
   Out << ", ";
-Args[I].print(Policy, Out);
+if (TemplOverloaded || !Params)
+  Args[I].print(Policy, Out, /*IncludeType*/ true);

rsmith wrote:
> dblaikie wrote:
> > dblaikie wrote:
> > > Looks like this (& the `TemplOverloaded` in the other function, below) is 
> > > undertested.
> > > 
> > > Hardcoding:
> > > true in this function results in no failures
> > > false in this function results in 1 failure in 
> > > `SemaTemplate/temp_arg_enum_printing.cpp`
> > >  - this failure, at least to me, seems to not have much to do with 
> > > overloading - at least syntactically foo<2> (where the parameter is an 
> > > enum type) doesn't compile, not due to any overloading ambiguity, but due 
> > > to a lack of implicit conversion from int to the enum type, I think? and 
> > > the example doesn't look like it involves any overloading... 
> > > 
> > > true in the other overload results in no failures
> > > false in the other overload results in no failures
> > > 
> > > I came across this because I was confused by how this feature works - 
> > > when the suffixes are used and when they are not to better understand the 
> > > implications this might have for debug info. (though I'm still generally 
> > > leaning towards just always putting the suffixes on for debug info)
> > > 
> > > @rsmith - could you give an example of what you meant by passing in a 
> > > parameter when the template is overloaded & that should use the suffix? 
> > > My simple examples, like this:
> > > ```
> > > template
> > > void f1() { }
> > > template
> > > void f1() { }
> > > int main() {
> > >   f1<3U>();
> > > }
> > > ```
> > > That's just ambiguous, apparently, and doesn't compile despite the type 
> > > suffix on the literal. If I put a parameter on one of the functions it 
> > > doesn't seem to trigger the "TemplOverloaded" case - both instantiations 
> > > still get un-suffixed names "f1<3>"... 
> > Ping on this (posted https://reviews.llvm.org/D111477 for some further 
> > discussion on the debug info side of things)
> I think the `TemplOverloaded` parameter is unnecessary: passing a null 
> `Params` list will result in `shouldIncludeTypeForArgument` returning `true`, 
> which would have the same effect as passing in `TemplOverloaded == true`. We 
> don't need two different ways to request that fully-unambiguous printing is 
> used for every argument, so removing this flag and passing a null `Params` 
> list instead might be a good idea.
> 
> Here's an unambiguous example where we need suffixes to identify which 
> specialization we're referring to:
> ```
> template void f() {}
> void (*p)() = f<0>;
> template void f() {}
> void (*q)() = f<>;
> ```
> 
> For this, `-ast-print` prints:
> ```
> template  void f() {
> }
> template<> void f<0L>() {
> }
> void (*p)() = f<0>;
> template  void f() {
> }
> template<> void f<0U>() {
> }
> void (*q)() = f<>;
> ```
> ... where the `0U` is intended to indicate that the second specialization is 
> for the second template not the first. (This `-ast-print` output isn't 
> actually valid code because `f<0U>` is still ambiguous, but we've done the 
> best we can.)
Thanks for the details!

Removed `TemplOverloaded` here: 5de369056dee2c4de81625cb05a5c212a0bdc053

(notes: The `DeclPrinter::VisitCXXRecordDecl` support for including/omiting 
types is untested (making it always print suffixes didn't fail any tests) - I 
was confused why that was, expecting some tests to fail even incidentally - but 
seems that is only used for ast-print? Whereas printing of 
`ClassTemplateSpecializationDecl` for diagnostics uses `getNameForDiagnostic` 
which I guess is fair - though could these share more of their implementation? 
Looks like they currently don't share template argument printing, the ast-print 
uses `DeclPrinter::printTemplateArguments` and the diagnostic stuff uses 
`TypePrinter.cpp:printTo` (so I guess function templates have another/third way 
of printing template arguments?)

Did eventually find at least a way to test the 
`DeclPrinter::VisitCXXRecordDecl` case, committed 
in400eb59adf43b29af3117c163cf770e6d6e514f7 (& found some minor ast-print bugs 
to commit as follow-ups in 604446aa6b41461e2691c9f4253e9ef70a5d68e4 and 
b2589e326ba4407d8314938a4f7498086e2203ea) - open to ideas/suggestions if 
there's other testing that might be suitable as well or instead.

Added your (@rsmith's) test as well in 50fdd7df827137c8465abafa82a6bae7c87096c5.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77598

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

[clang] 50fdd7d - Add more test coverage for D77598

[David Blaikie via cfe-commits]

Author: David Blaikie
Date: 2021-11-14T21:09:11-08:00
New Revision: 50fdd7df827137c8465abafa82a6bae7c87096c5

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

LOG: Add more test coverage for D77598

Add coverage to demonstrate why including the type of template
parameters is necessary to disambiguate function template
specializations.

Test courtesy of Richard Smith

Added: 


Modified: 
clang/test/AST/ast-dump-templates.cpp

Removed: 




diff  --git a/clang/test/AST/ast-dump-templates.cpp 
b/clang/test/AST/ast-dump-templates.cpp
index b08bc76ed179..3d26eb917c12 100644
--- a/clang/test/AST/ast-dump-templates.cpp
+++ b/clang/test/AST/ast-dump-templates.cpp
@@ -93,3 +93,14 @@ void test() {
 // CHECK1: {{^}}template<> struct foo<1, 0 + 0L> {
 template struct foo<1, 0 + 0L>;
 }
+
+namespace test5 {
+template void f() {}
+void (*p)() = f<0>;
+template void f() {}
+void (*q)() = f<>;
+// Not perfect - this code in the dump would be ambiguous, but it's the best we
+// can do to 
diff erentiate these two implicit specializations.
+// CHECK1: template<> void f<0L>()
+// CHECK1: template<> void f<0U>()
+}



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

[clang] b2589e3 - ast-print: Avoid extra whitespace before function opening brace

[David Blaikie via cfe-commits]

Author: David Blaikie
Date: 2021-11-14T20:45:16-08:00
New Revision: b2589e326ba4407d8314938a4f7498086e2203ea

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

LOG: ast-print: Avoid extra whitespace before function opening brace

Added: 


Modified: 
clang/include/clang/AST/Stmt.h
clang/lib/AST/DeclPrinter.cpp
clang/lib/AST/StmtPrinter.cpp
clang/test/AST/ast-dump-templates.cpp

Removed: 




diff  --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index 73cbff537847..a32126d23d31 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -1216,6 +1216,11 @@ class alignas(void *) Stmt {
const PrintingPolicy &Policy, unsigned Indentation = 0,
StringRef NewlineSymbol = "\n",
const ASTContext *Context = nullptr) const;
+  void printPrettyControlled(raw_ostream &OS, PrinterHelper *Helper,
+ const PrintingPolicy &Policy,
+ unsigned Indentation = 0,
+ StringRef NewlineSymbol = "\n",
+ const ASTContext *Context = nullptr) const;
 
   /// Pretty-prints in JSON format.
   void printJson(raw_ostream &Out, PrinterHelper *Helper,

diff  --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index 5c6781c26ed7..044eb8f8f8e5 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -782,11 +782,10 @@ void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) {
   Out << ";\n";
 }
 Indentation -= Policy.Indentation;
-  } else
-Out << ' ';
+  }
 
   if (D->getBody())
-D->getBody()->printPretty(Out, nullptr, SubPolicy, Indentation, "\n",
+D->getBody()->printPrettyControlled(Out, nullptr, SubPolicy, 
Indentation, "\n",
   &Context);
 } else {
   if (!Policy.TerseOutput && isa(*D))

diff  --git a/clang/lib/AST/StmtPrinter.cpp b/clang/lib/AST/StmtPrinter.cpp
index 12af5bfa2013..fc267d7006a1 100644
--- a/clang/lib/AST/StmtPrinter.cpp
+++ b/clang/lib/AST/StmtPrinter.cpp
@@ -2595,6 +2595,14 @@ void Stmt::printPretty(raw_ostream &Out, PrinterHelper 
*Helper,
   P.Visit(const_cast(this));
 }
 
+void Stmt::printPrettyControlled(raw_ostream &Out, PrinterHelper *Helper,
+ const PrintingPolicy &Policy,
+ unsigned Indentation, StringRef NL,
+ const ASTContext *Context) const {
+  StmtPrinter P(Out, Helper, Policy, Indentation, NL, Context);
+  P.PrintControlledStmt(const_cast(this));
+}
+
 void Stmt::printJson(raw_ostream &Out, PrinterHelper *Helper,
  const PrintingPolicy &Policy, bool AddQuotes) const {
   std::string Buf;

diff  --git a/clang/test/AST/ast-dump-templates.cpp 
b/clang/test/AST/ast-dump-templates.cpp
index dcecdca58c75..b08bc76ed179 100644
--- a/clang/test/AST/ast-dump-templates.cpp
+++ b/clang/test/AST/ast-dump-templates.cpp
@@ -79,6 +79,8 @@ struct foo {
 // type/unsigned argument (see
 // TemplateParameterList::shouldIncludeTypeForArgument)
 // CHECK1: {{^}}template<> struct foo<0, 0L> {
+// CHECK1: {{^}}void test(){{ }}{
+// CHECK1: {{^}}foo<0, 0 + 0L>::fn();
 void test() {
   foo<0, 0 + 0L>::fn();
 }



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

[clang] 604446a - ast-dump: Add missing identation of class template specializations

[David Blaikie via cfe-commits]

Author: David Blaikie
Date: 2021-11-14T20:45:16-08:00
New Revision: 604446aa6b41461e2691c9f4253e9ef70a5d68e4

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

LOG: ast-dump: Add missing identation of class template specializations

Added: 


Modified: 
clang/lib/AST/DeclPrinter.cpp
clang/test/AST/ast-dump-templates.cpp

Removed: 




diff  --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index 884b8e730905..5c6781c26ed7 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -1185,6 +1185,7 @@ void 
DeclPrinter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
 if (D->isThisDeclarationADefinition())
   Out << ";";
 Out << "\n";
+Indent();
 Visit(I);
   }
   }

diff  --git a/clang/test/AST/ast-dump-templates.cpp 
b/clang/test/AST/ast-dump-templates.cpp
index cb2994edcc67..dcecdca58c75 100644
--- a/clang/test/AST/ast-dump-templates.cpp
+++ b/clang/test/AST/ast-dump-templates.cpp
@@ -78,7 +78,7 @@ struct foo {
 // includes the type for the auto argument and omits it for the fixed
 // type/unsigned argument (see
 // TemplateParameterList::shouldIncludeTypeForArgument)
-// CHECK1: template<> struct foo<0, 0L> {
+// CHECK1: {{^}}template<> struct foo<0, 0L> {
 void test() {
   foo<0, 0 + 0L>::fn();
 }
@@ -88,6 +88,6 @@ void test() {
 // powered by the shouldIncludeTypeForArgument functionality.
 // Not sure if this it's intentional that these two specializations are 
rendered
 // 
diff erently in this way.
-// CHECK1: template<> struct foo<1, 0 + 0L> {
+// CHECK1: {{^}}template<> struct foo<1, 0 + 0L> {
 template struct foo<1, 0 + 0L>;
 }



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

[clang] 400eb59 - Add test for a case in D77598

[David Blaikie via cfe-commits]

Author: David Blaikie
Date: 2021-11-14T20:45:16-08:00
New Revision: 400eb59adf43b29af3117c163cf770e6d6e514f7

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

LOG: Add test for a case in D77598

This covers the DeclPrinter::VisitCXXRecordDecl caller - though also
demonstrates some possible inconsistency in template specialization
printing.

Added: 


Modified: 
clang/test/AST/ast-dump-templates.cpp

Removed: 




diff  --git a/clang/test/AST/ast-dump-templates.cpp 
b/clang/test/AST/ast-dump-templates.cpp
index ab4a7356028e1..cb2994edcc67e 100644
--- a/clang/test/AST/ast-dump-templates.cpp
+++ b/clang/test/AST/ast-dump-templates.cpp
@@ -67,3 +67,27 @@ namespace test3 {
   template A(T) -> A;
   // CHECK1: template  A(T) -> A;
 }
+
+namespace test4 {
+template 
+struct foo {
+  static void fn();
+};
+
+// Prints using an "integral" template argument. Test that this correctly
+// includes the type for the auto argument and omits it for the fixed
+// type/unsigned argument (see
+// TemplateParameterList::shouldIncludeTypeForArgument)
+// CHECK1: template<> struct foo<0, 0L> {
+void test() {
+  foo<0, 0 + 0L>::fn();
+}
+
+// Prints using an "expression" template argument. This renders based on the 
way
+// the user wrote the arguments (including that + expression) - so it's not
+// powered by the shouldIncludeTypeForArgument functionality.
+// Not sure if this it's intentional that these two specializations are 
rendered
+// 
diff erently in this way.
+// CHECK1: template<> struct foo<1, 0 + 0L> {
+template struct foo<1, 0 + 0L>;
+}



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

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3193
+def mskip_rax_setup : Flag<["-"], "mskip-rax-setup">, Group, 
Flags<[NoXarchOption]>,
+  HelpText<"Skip setting up RAX register when passing variable arguments (x86 
only)">;
 def mstackrealign : Flag<["-"], "mstackrealign">, Group, 
Flags<[CC1Option]>,

MaskRay wrote:
> nickdesaulniers wrote:
> > I think GCC support `-mno-skip-rax-setup` as well. Can you please add that 
> > (and tests for it) as well?  We don't need to actually handle it, I think, 
> > but we shouldn't warn about the flag being unsupported, for example.
> Consider `SimpleMFlag`
Thanks for the suggestion. I think adding a `-mno-skip-rax-setup` is simply 
here.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2197
 
+  if (Args.hasArg(options::OPT_mskip_rax_setup)) {
+CmdArgs.push_back("-mllvm");

nickdesaulniers wrote:
> It might be nice to warn the user if this flag depends on `-mno-sse`.
GCC doesn't warn it either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112413

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

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

[Phoebe Wang via Phabricator via cfe-commits]

pengfei updated this revision to Diff 387147.
pengfei marked 3 inline comments as done.
pengfei added a comment.
Herald added a subscriber: dexonsmith.

Address review comments.

1. Add support for `-mno-skip-rax-setup` and its test;
2. Use module flag metadata instead of command line option in backend;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112413

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/pr23258.c
  clang/test/Driver/x86_features.c
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/test/CodeGen/X86/pr23258.ll

Index: llvm/test/CodeGen/X86/pr23258.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/pr23258.ll
@@ -0,0 +1,25 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s --check-prefix=HAS-RAX
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mattr=+sse | FileCheck %s --check-prefix=HAS-RAX
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mattr=-sse | FileCheck %s --check-prefix=NO-RAX
+
+define dso_local void @foo() nounwind {
+; HAS-RAX-LABEL: foo:
+; HAS-RAX:   # %bb.0: # %entry
+; HAS-RAX-NEXT:movl $1, %edi
+; HAS-RAX-NEXT:xorl %eax, %eax
+; HAS-RAX-NEXT:jmp bar # TAILCALL
+;
+; NO-RAX-LABEL: foo:
+; NO-RAX:   # %bb.0: # %entry
+; NO-RAX-NEXT:movl $1, %edi
+; NO-RAX-NEXT:jmp bar # TAILCALL
+entry:
+  tail call void (i32, ...) @bar(i32 1)
+  ret void
+}
+
+declare dso_local void @bar(i32, ...)
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 4, !"SkipRaxSetup", i32 1}
Index: llvm/lib/Target/X86/X86ISelLowering.cpp
===
--- llvm/lib/Target/X86/X86ISelLowering.cpp
+++ llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -4414,7 +4414,8 @@
 }
   }
 
-  if (Is64Bit && isVarArg && !IsWin64 && !IsMustTail) {
+  if (Is64Bit && isVarArg && !IsWin64 && !IsMustTail &&
+  (Subtarget.hasSSE1() || !M->getModuleFlag("SkipRaxSetup"))) {
 // From AMD64 ABI document:
 // For calls that may call functions that use varargs or stdargs
 // (prototype-less calls or calls to functions containing ellipsis (...) in
Index: clang/test/Driver/x86_features.c
===
--- clang/test/Driver/x86_features.c
+++ clang/test/Driver/x86_features.c
@@ -5,3 +5,9 @@
 // Test that we don't produce an error with -mieee-fp.
 // RUN: %clang -### %s -mieee-fp -S 2>&1 | FileCheck --check-prefix=IEEE %s
 // IEEE-NOT: error: unknown argument
+
+// RUN: %clang -### %s -mskip-rax-setup -S 2>&1 | FileCheck --check-prefix=SRS %s
+// SRS: "-mskip-rax-setup"
+
+// RUN: %clang -### %s -mno-skip-rax-setup -S 2>&1 | FileCheck --check-prefix=NO-SRS %s
+// NO-SRS-NOT: "-mskip-rax-setup"
Index: clang/test/CodeGen/pr23258.c
===
--- /dev/null
+++ clang/test/CodeGen/pr23258.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s -check-prefix=NO-SKIP
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -mskip-rax-setup -emit-llvm %s -o - | FileCheck %s -check-prefix=SKIP
+
+void f() {}
+
+// SKIP: !"SkipRaxSetup", i32 1}
+// NO-SKIP-NOT: "SkipRaxSetup"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2194,6 +2194,11 @@
 CmdArgs.push_back("-x86-asm-syntax=intel");
   }
 
+  if (Arg *A = Args.getLastArg(options::OPT_mskip_rax_setup,
+   options::OPT_mno_skip_rax_setup))
+if (A->getOption().matches(options::OPT_mskip_rax_setup))
+  CmdArgs.push_back(Args.MakeArgString("-mskip-rax-setup"));
+
   // Set flags to support MCU ABI.
   if (Args.hasFlag(options::OPT_miamcu, options::OPT_mno_iamcu, false)) {
 CmdArgs.push_back("-mfloat-abi");
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -813,6 +813,8 @@
 getCodeGenOpts().StackProtectorGuardOffset);
   if (getCodeGenOpts().StackAlignment)
 getModule().setOverrideStackAlignment(getCodeGenOpts().StackAlignment);
+  if (getCodeGenOpts().SkipRaxSetup)
+getModule().addModuleFlag(llvm::Module::Override, "SkipRaxSetup", 1);
 
   getTargetCodeGenInfo().emitTargetMetadata(*this, MangledDeclNames);
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3189,6 +3189,10 @@
 Hel

[PATCH] D113664: [cmake] use project relative paths when generating ASTNodeAPI.json

[Stephen Neuendorffer via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113664

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

[PATCH] D112903: [C++20] [Module] Fix bug47116 and implement [module.interface]/p6

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 387144.
ChuanqiXu added a comment.

Fix a mismatch in test


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

https://reviews.llvm.org/D112903

Files:
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclBase.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/CXX/module/module.interface/p2-2.cpp
  clang/test/CXX/module/module.interface/p6.cpp

Index: clang/test/CXX/module/module.interface/p6.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.interface/p6.cpp
@@ -0,0 +1,93 @@
+// The test is check we couldn't export a redeclaration which isn't exported previously and
+// check it is OK to redeclare no matter exported nor not if is the previous declaration is exported.
+// RUN: %clang_cc1 -std=c++20 %s -verify
+
+export module X;
+
+struct S { // expected-note {{previous declaration is here}}
+  int n;
+};
+typedef S S;
+export typedef S S; // OK, does not redeclare an entity
+export struct S;// expected-error {{cannot export redeclaration 'S' here since the previous declaration is not exported.}}
+
+namespace A {
+struct X; // expected-note {{previous declaration is here}}
+export struct Y;
+} // namespace A
+
+namespace A {
+export struct X; // expected-error {{cannot export redeclaration 'X' here since the previous declaration is not exported.}}
+export struct Y; // OK
+struct Z;// expected-note {{previous declaration is here}}
+export struct Z; // expected-error {{cannot export redeclaration 'Z' here since the previous declaration is not exported.}}
+} // namespace A
+
+namespace A {
+struct B;// expected-note {{previous declaration is here}}
+struct C {}; // expected-note {{previous declaration is here}}
+} // namespace A
+
+namespace A {
+export struct B {}; // expected-error {{cannot export redeclaration 'B' here since the previous declaration is not exported.}}
+export struct C;// expected-error {{cannot export redeclaration 'C' here since the previous declaration is not exported.}}
+} // namespace A
+
+template 
+struct TemplS; // expected-note {{previous declaration is here}}
+
+export template 
+struct TemplS {}; // expected-error {{cannot export redeclaration 'TemplS' here since the previous declaration is not exported.}}
+
+template 
+struct TemplS2; // expected-note {{previous declaration is here}}
+
+export template 
+struct TemplS2 {}; // expected-error {{cannot export redeclaration 'TemplS2' here since the previous declaration is not exported.}}
+
+void baz();// expected-note {{previous declaration is here}}
+export void baz(); // expected-error {{cannot export redeclaration 'baz' here since the previous declaration is not exported.}}
+
+namespace A {
+export void foo();
+void bar();// expected-note {{previous declaration is here}}
+export void bar(); // expected-error {{cannot export redeclaration 'bar' here since the previous declaration is not exported.}}
+void f1(); // expected-note {{previous declaration is here}}
+} // namespace A
+
+// OK
+//
+// [module.interface]/p6
+// A redeclaration of an entity X is implicitly exported if X was introduced by an exported declaration
+void A::foo();
+
+// The compiler couldn't export A::f1() here since A::f1() is declared above without exported.
+// See [module.interface]/p6 for details.
+export void A::f1(); // expected-error {{cannot export redeclaration 'f1' here since the previous declaration is not exported.}}
+
+template 
+void TemplFunc(); // expected-note {{previous declaration is here}}
+
+export template 
+void TemplFunc() { // expected-error {{cannot export redeclaration 'TemplFunc' here since the previous declaration is not exported.}}
+}
+
+namespace A {
+template 
+void TemplFunc2(); // expected-note {{previous declaration is here}}
+export template 
+void TemplFunc2() {} // expected-error {{cannot export redeclaration 'TemplFunc2' here since the previous declaration is not exported.}}
+template 
+void TemplFunc3(); // expected-note {{previous declaration is here}}
+} // namespace A
+
+export template 
+void A::TemplFunc3() {} // expected-error {{cannot export redeclaration 'TemplFunc3' here since the previous declaration is not exported.}}
+
+int var;// expected-note {{previous declaration is here}}
+export int var; // expected-error {{cannot export redeclaration 'var' here since the previous declaration is not exported.}}
+
+template 
+T TemplVar; // expected-note {{previous declaration is here}}
+export template 
+T TemplVar; // expected-error {{cannot export redeclaration 'TemplVar' here since the previous declaration is not exported.}}
Index: clang/test/CXX/module/module.interface/p2-2.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.in

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

[Hyeongyu Kim via Phabricator via cfe-commits]

hyeongyukim added a comment.

In D105169#3115814 , @erichkeane 
wrote:

> Either this or D108453  (which were 
> committed together!) caused this assert according to my git-bisect: 
> https://godbolt.org/z/4rqYKfW7K
>
> NOTE that this fails in a lit-test for me, clang CodeGen/ifunc.c (though my 
> downstream seems to run the verifier even with -emit-llvm, so you might need 
> to just swap it to an -emit-obj to get this to repro).

The lit-test failure of CodeGen/ifunc.c was not directly related to this patch.
`emitIFuncDefinition` was creating an incorrect function attribute.
It added the noundef attribute to the function even though there are no 
parameters (`foo_ifunc` function of `ifunc.c`), and it was fixed a few days ago.

The patch that solved this problem is D113352 
.

> The `emitIFuncDefinition` fucntion incorrectly passes the GlobalDecl of the 
> IFunc itself to the call to GetOrCreateLLVMFunction for creating the 
> resolver, which causes it to be created with a wrong attribute list, which 
> fails `Verifier::visitFunction` with "Attribute after last parameter!". 
> You'll note that unlike the relationship between aliases and their aliasees, 
> the resolver and the ifunc have different types - the resolver takes no 
> parameters.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105169

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

[PATCH] D113848: [clang-tidy][NFC] Refactor ClangTidyDiagnosticConsumer files

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyContext.cpp:40
+#include 
+using namespace clang;
+using namespace tidy;

Shouldn't namespaces be used instead?



Comment at: clang-tools-extra/clang-tidy/ClangTidyError.cpp:18
+#include "ClangTidyError.h"
+using namespace clang;
+using namespace tidy;

Shouldn't namespaces be used instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113848

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

[PATCH] D113859: [Sema] Fix consteval function calls with value-dependent args

[Léo Lam via Phabricator via cfe-commits]

leoetlino added a comment.

While this patch doesn't introduce new regressions and does fix compilation of 
the code snippet mentioned earlier, I'm not sure this is the correct approach, 
considering there are a bunch of other similar template-related consteval bugs 
that this patch does *not* fix:

  template 
  struct Foo {
  static consteval void ok(int) {}
  
  static consteval void bad(T) {}
  
  template 
  static consteval void call(T x, U fn) { fn(x); }
  
  static constexpr bool test() {
  ok({});
  bad({});  // error: cannot take address of consteval function 'bad' 
outside of an immediate invocation
  call({}, bad);  // error: cannot take address of consteval function 
'bad' outside of an immediate invocation
  return true;
  }
  };

The problem seems to be that the call/arguments are type dependent so nothing 
is added to `ImmediateInvocationCandidates` -- and so 
`HandleImmediateInvocations` (in SemaExpr.cpp) never removes anything from 
`ReferenceToConsteval` despite the references to the consteval functions being 
used for immediate invocations.

Another variant of the issue:

  template 
  struct Bar {
  static consteval void bad(int) {}
  
  static constexpr bool test() {
  bad(T::value);  // error: cannot take address of consteval function 
'bad' outside of an immediate invocation
  return true;
  }
  };

Not sure what the best way of fixing those issues would be -- any guidance 
would be greatly appreciated!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113859

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

[PATCH] D112777: [X86][FP16] add alias for f*mul_*ch intrinsics

[Freddy, Ye via Phabricator via cfe-commits]

FreddyYe added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112777

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

[PATCH] D93298: [RISCV] add the MC layer support of Zfinx extension

[Shao-Ce SUN via Phabricator via cfe-commits]

achieveartificialintelligence added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93298

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

[PATCH] D113847: [clang-tidy][NFC] Simplify ClangTidyStats

[Salman Javed via Phabricator via cfe-commits]

salman-javed-nz added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:50
 struct ClangTidyStats {
-  ClangTidyStats()
-  : ErrorsDisplayed(0), ErrorsIgnoredCheckFilter(0), 
ErrorsIgnoredNOLINT(0),
-ErrorsIgnoredNonUserCode(0), ErrorsIgnoredLineFilter(0) {}
+  int ErrorsDisplayed{0};
+  int ErrorsIgnoredCheckFilter{0};

What's the prevalent style for class member initialization? `=` or `{}`?
cppcoreguidelines-prefer-member-initializer defaults to `{}` but I have seen 
both types in the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113847

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

[PATCH] D113863: [clang-tidy] Make `readability-container-data-pointer` more robust

[Fabian Wolff via Phabricator via cfe-commits]

fwolff updated this revision to Diff 387133.

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

https://reviews.llvm.org/D113863

Files:
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
@@ -109,3 +109,38 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
   // CHECK-FIXES: {{^  }}return v.data();{{$}}
 }
+
+template 
+struct container_without_data {
+  using size_type = size_t;
+  T &operator[](size_type);
+  const T &operator[](size_type) const;
+};
+
+template 
+const T *n(const container_without_data &c) {
+  // c has no "data" member function, so there should not be a warning here:
+  return &c[0];
+}
+
+const int *o(const std::vector>> &v, const size_t idx1, const size_t idx2) {
+  return &v[idx1][idx2][0];
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}return v[idx1][idx2].data();{{$}}
+}
+
+std::vector &select(std::vector &u, std::vector &v) {
+  return v;
+}
+
+int *p(std::vector &v1, std::vector &v2) {
+  return &select(*&v1, v2)[0];
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}return select(*&v1, v2).data();{{$}}
+}
+
+int *q(std::vector ***v) {
+  return &(***v)[0];
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}return (**v)->data();{{$}}
+}
Index: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
@@ -16,6 +16,12 @@
 namespace clang {
 namespace tidy {
 namespace readability {
+
+static const char ContainerExprName[] = "container-expr";
+static const char DerefContainerExprName[] = "deref-container-expr";
+static const char AddrofContainerExprName[] = "addrof-container-expr";
+static const char AddressOfName[] = "address-of";
+
 ContainerDataPointerCheck::ContainerDataPointerCheck(StringRef Name,
  ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context) {}
@@ -38,69 +44,65 @@
   const auto Container =
   qualType(anyOf(NonTemplateContainerType, TemplateContainerType));
 
+  const auto ContainerExpr = anyOf(
+  unaryOperator(
+  hasOperatorName("*"),
+  hasUnaryOperand(
+  expr(hasType(pointsTo(Container))).bind(DerefContainerExprName)))
+  .bind(ContainerExprName),
+  unaryOperator(hasOperatorName("&"),
+hasUnaryOperand(expr(anyOf(hasType(Container),
+   hasType(references(Container
+.bind(AddrofContainerExprName)))
+  .bind(ContainerExprName),
+  expr(anyOf(hasType(Container), hasType(pointsTo(Container)),
+ hasType(references(Container
+  .bind(ContainerExprName));
+
+  const auto Zero = integerLiteral(equals(0));
+
+  const auto SubscriptOperator = callee(cxxMethodDecl(hasName("operator[]")));
+
   Finder->addMatcher(
   unaryOperator(
   unless(isExpansionInSystemHeader()), hasOperatorName("&"),
-  hasUnaryOperand(anyOf(
-  ignoringParenImpCasts(
-  cxxOperatorCallExpr(
-  callee(cxxMethodDecl(hasName("operator[]"))
- .bind("operator[]")),
-  argumentCountIs(2),
-  hasArgument(
-  0,
-  anyOf(ignoringParenImpCasts(
-declRefExpr(
-to(varDecl(anyOf(
-hasType(Container),
-hasType(references(Container))
-.bind("var")),
-ignoringParenImpCasts(hasDescendant(
-declRefExpr(
-  

[PATCH] D113863: [clang-tidy] Make `readability-container-data-pointer` more robust

[Fabian Wolff via Phabricator via cfe-commits]

fwolff updated this revision to Diff 387132.

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

https://reviews.llvm.org/D113863

Files:
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
@@ -109,3 +109,38 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
   // CHECK-FIXES: {{^  }}return v.data();{{$}}
 }
+
+template 
+struct container_without_data {
+  using size_type = size_t;
+  T &operator[](size_type);
+  const T &operator[](size_type) const;
+};
+
+template 
+const T *n(const container_without_data &c) {
+  // c has no "data" member function, so there should not be a warning here:
+  return &c[0];
+}
+
+const int *o(const std::vector>> &v, const size_t idx1, const size_t idx2) {
+  return &v[idx1][idx2][0];
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}return v[idx1][idx2].data();{{$}}
+}
+
+std::vector &select(std::vector &u, std::vector &v) {
+  return v;
+}
+
+int *p(std::vector &v1, std::vector &v2) {
+  return &select(*&v1, v2)[0];
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}return select(*&v1, v2).data();{{$}}
+}
+
+int *q(std::vector ***v) {
+  return &(***v)[0];
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}return (**v)->data();{{$}}
+}
Index: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
@@ -16,6 +16,12 @@
 namespace clang {
 namespace tidy {
 namespace readability {
+
+static const char ContainerExprName[] = "container-expr";
+static const char DerefContainerExprName[] = "deref-container-expr";
+static const char AddrofContainerExprName[] = "addrof-container-expr";
+static const char AddressOfName[] = "address-of";
+
 ContainerDataPointerCheck::ContainerDataPointerCheck(StringRef Name,
  ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context) {}
@@ -38,69 +44,65 @@
   const auto Container =
   qualType(anyOf(NonTemplateContainerType, TemplateContainerType));
 
+  const auto ContainerExpr = anyOf(
+  unaryOperator(
+  hasOperatorName("*"),
+  hasUnaryOperand(
+  expr(hasType(pointsTo(Container))).bind(DerefContainerExprName)))
+  .bind(ContainerExprName),
+  unaryOperator(hasOperatorName("&"),
+hasUnaryOperand(expr(anyOf(hasType(Container),
+   hasType(references(Container
+.bind(AddrofContainerExprName)))
+  .bind(ContainerExprName),
+  expr(anyOf(hasType(Container), hasType(pointsTo(Container)),
+ hasType(references(Container
+  .bind(ContainerExprName));
+
+  const auto Zero = ignoringParens(integerLiteral(equals(0)));
+
+  const auto SubscriptOperator = callee(cxxMethodDecl(hasName("operator[]")));
+
   Finder->addMatcher(
   unaryOperator(
   unless(isExpansionInSystemHeader()), hasOperatorName("&"),
-  hasUnaryOperand(anyOf(
-  ignoringParenImpCasts(
-  cxxOperatorCallExpr(
-  callee(cxxMethodDecl(hasName("operator[]"))
- .bind("operator[]")),
-  argumentCountIs(2),
-  hasArgument(
-  0,
-  anyOf(ignoringParenImpCasts(
-declRefExpr(
-to(varDecl(anyOf(
-hasType(Container),
-hasType(references(Container))
-.bind("var")),
-ignoringParenImpCasts(hasDescendant(
-decl

[PATCH] D113863: [clang-tidy] Make `readability-container-data-pointer` more robust

[Fabian Wolff via Phabricator via cfe-commits]

fwolff created this revision.
fwolff added reviewers: compnerd, alexfh, aaron.ballman.
fwolff added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, xazax.hun.
fwolff requested review of this revision.
Herald added a subscriber: cfe-commits.

Fixes PR#52245. I've also added a few test cases beyond PR#52245 that would 
also fail with the current implementation, which is quite brittle in many 
respects (e.g. it uses the `hasDescendant()` matcher to find the container that 
is being accessed, which is very easy to trick, as in the example in PR#52245).

I have not been able to reproduce the second issue mentioned in PR#52245 
(namely that using the `data()` member function is suggested even for 
containers that don't have it), but I've added a test case for it to be sure.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113863

Files:
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
@@ -109,3 +109,38 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
   // CHECK-FIXES: {{^  }}return v.data();{{$}}
 }
+
+template 
+struct container_without_data {
+  using size_type = size_t;
+  T &operator[](size_type);
+  const T &operator[](size_type) const;
+};
+
+template 
+const T *n(const container_without_data &c) {
+  // c has no "data" member function, so there should not be a warning here:
+  return &c[0];
+}
+
+const int *o(const std::vector>> &v, const size_t idx1, const size_t idx2) {
+  return &v[idx1][idx2][0];
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}return v[idx1][idx2].data();{{$}}
+}
+
+std::vector &select(std::vector &u, std::vector &v) {
+  return v;
+}
+
+int *p(std::vector &v1, std::vector &v2) {
+  return &select(*&v1, v2)[0];
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}return select(*&v1, v2).data();{{$}}
+}
+
+int *q(std::vector ***v) {
+  return &(***v)[0];
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}return (**v)->data();{{$}}
+}
Index: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
@@ -16,6 +16,12 @@
 namespace clang {
 namespace tidy {
 namespace readability {
+
+static const char ContainerExprName[] = "container-expr";
+static const char DerefContainerExprName[] = "deref-container-expr";
+static const char AddrofContainerExprName[] = "addrof-container-expr";
+static const char AddressOfName[] = "address-of";
+
 ContainerDataPointerCheck::ContainerDataPointerCheck(StringRef Name,
  ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context) {}
@@ -38,69 +44,69 @@
   const auto Container =
   qualType(anyOf(NonTemplateContainerType, TemplateContainerType));
 
+  const auto ContainerExpr = anyOf(
+  declRefExpr(
+  to(varDecl(anyOf(hasType(Container), hasType(pointsTo(Container)),
+   hasType(references(Container))
+  .bind(ContainerExprName),
+  unaryOperator(
+  hasOperatorName("*"),
+  hasUnaryOperand(
+  expr(hasType(pointsTo(Container))).bind(DerefContainerExprName)))
+  .bind(ContainerExprName),
+  unaryOperator(hasOperatorName("&"),
+hasUnaryOperand(expr(anyOf(hasType(Container),
+   hasType(references(Container
+.bind(AddrofContainerExprName)))
+  .bind(ContainerExprName),
+  expr(anyOf(hasType(Container), hasType(pointsTo(Container)),
+ hasType(references(Container
+  .bind(ContainerExprName));
+
+  const auto Zero = ignoringParens(integerLiteral(equals(0)));
+
+  const auto SubscriptOperator = callee(cxxMethodDecl(hasName("operator[]")));
+
   Finde

[PATCH] D113708: [clang-tidy] Fix `bugprone-use-after-move` check to also consider moves in constructor initializers

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

I will push tomorrow so I have time to keep an eye on the bots ;) Btw @fwolff 
could you post your name and email so i add it to  the commit?


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

https://reviews.llvm.org/D113708

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

[clang] 5de3690 - Follow-up to D77598: Simplify API by passing template parameters only when used/to imply "TemplOverloaded/overloadable"

[David Blaikie via cfe-commits]

Author: David Blaikie
Date: 2021-11-14T13:35:22-08:00
New Revision: 5de369056dee2c4de81625cb05a5c212a0bdc053

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

LOG: Follow-up to D77598: Simplify API by passing template parameters only when 
used/to imply "TemplOverloaded/overloadable"

These arguments were redundant, and other parts of D77598 did rely on
the presence/absence of template parameters to imply whether types
should be included for the argument (like
clang::printTemplateArgumentList) so do that here too.

Added: 


Modified: 
clang/lib/AST/DeclPrinter.cpp

Removed: 




diff  --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index 38f2d10cdc90..884b8e730905 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -112,11 +112,9 @@ namespace {
 void printTemplateParameters(const TemplateParameterList *Params,
  bool OmitTemplateKW = false);
 void printTemplateArguments(llvm::ArrayRef Args,
-const TemplateParameterList *Params,
-bool TemplOverloaded);
+const TemplateParameterList *Params);
 void printTemplateArguments(llvm::ArrayRef Args,
-const TemplateParameterList *Params,
-bool TemplOverloaded);
+const TemplateParameterList *Params);
 void prettyPrintAttributes(Decl *D);
 void prettyPrintPragmas(Decl *D);
 void printDeclType(QualType T, StringRef DeclName, bool Pack = false);
@@ -652,16 +650,11 @@ void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) {
 llvm::raw_string_ostream POut(Proto);
 DeclPrinter TArgPrinter(POut, SubPolicy, Context, Indentation);
 const auto *TArgAsWritten = D->getTemplateSpecializationArgsAsWritten();
-const TemplateParameterList *TPL = D->getTemplateSpecializationInfo()
-   ->getTemplate()
-   ->getTemplateParameters();
 if (TArgAsWritten && !Policy.PrintCanonicalTypes)
-  TArgPrinter.printTemplateArguments(TArgAsWritten->arguments(), TPL,
- /*TemplOverloaded*/ true);
+  TArgPrinter.printTemplateArguments(TArgAsWritten->arguments(), nullptr);
 else if (const TemplateArgumentList *TArgs =
  D->getTemplateSpecializationArgs())
-  TArgPrinter.printTemplateArguments(TArgs->asArray(), TPL,
- /*TemplOverloaded*/ true);
+  TArgPrinter.printTemplateArguments(TArgs->asArray(), nullptr);
   }
 
   QualType Ty = D->getType();
@@ -1002,8 +995,7 @@ void DeclPrinter::VisitCXXRecordDecl(CXXRecordDecl *D) {
   dyn_cast(TSI->getType()))
 Args = TST->template_arguments();
   printTemplateArguments(
-  Args, S->getSpecializedTemplate()->getTemplateParameters(),
-  /*TemplOverloaded*/ false);
+  Args, S->getSpecializedTemplate()->getTemplateParameters());
 }
   }
 
@@ -1096,13 +1088,12 @@ void DeclPrinter::printTemplateParameters(const 
TemplateParameterList *Params,
 }
 
 void DeclPrinter::printTemplateArguments(ArrayRef Args,
- const TemplateParameterList *Params,
- bool TemplOverloaded) {
+ const TemplateParameterList *Params) {
   Out << "<";
   for (size_t I = 0, E = Args.size(); I < E; ++I) {
 if (I)
   Out << ", ";
-if (TemplOverloaded || !Params)
+if (!Params)
   Args[I].print(Policy, Out, /*IncludeType*/ true);
 else
   Args[I].print(Policy, Out,
@@ -1113,13 +1104,12 @@ void 
DeclPrinter::printTemplateArguments(ArrayRef Args,
 }
 
 void DeclPrinter::printTemplateArguments(ArrayRef Args,
- const TemplateParameterList *Params,
- bool TemplOverloaded) {
+ const TemplateParameterList *Params) {
   Out << "<";
   for (size_t I = 0, E = Args.size(); I < E; ++I) {
 if (I)
   Out << ", ";
-if (TemplOverloaded)
+if (!Params)
   Args[I].getArgument().print(Policy, Out, /*IncludeType*/ true);
 else
   Args[I].getArgument().print(



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

[PATCH] D113844: [clang-format] [NFC] build clang-format with -Wall

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

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

Is okay, but I think the compiler is a bit overreacting, since there is only 
one call (chain).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113844

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

[PATCH] D113708: [clang-tidy] Fix `bugprone-use-after-move` check to also consider moves in constructor initializers

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp accepted this revision.
carlosgalvezp added a comment.

Awesome, thanks! Sure, I'm quite new here as well so I'll need to take some 
time to figure out how to commit on behalf of someone else, bear with me :)


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

https://reviews.llvm.org/D113708

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

[PATCH] D113859: [Sema] Fix consteval function calls with value-dependent args

[Léo Lam via Phabricator via cfe-commits]

leoetlino created this revision.
leoetlino added a reviewer: rsmith.
Herald added a subscriber: kristof.beyls.
leoetlino requested review of this revision.
Herald added a project: clang.

If any arguments of a consteval function call are value-dependent,
the call cannot be evaluated until instantiation.

This patch fixes Sema::CheckForImmediateInvocation so we don't attempt
to evaluate consteval function calls too early (before instantiation).

This fixes things like:

  consteval int f(int n) { return n; }
  
  template 
  constexpr int broken() {
return f(M);
  }

Without the value-dependency checks, what happens is that the constant
expression evaluation engine is called on the following expression:

  ConstantExpr 'int'
  `-CallExpr 'int'
|-ImplicitCastExpr 'int (*)(int)' 
| `-DeclRefExpr 'int (int)' lvalue Function 'f' 'int (int)'
`-DeclRefExpr 'int' NonTypeTemplateParm 'M' 'int'

which obviously fails when it tries to evaluate M.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113859

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp


Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -61,6 +61,20 @@
 struct E : C {
   consteval ~E() {} // expected-error {{cannot be declared consteval}}
 };
+
+template 
+constexpr int callConstevalWithNameDependentArg() {
+  return f1(X);
+}
+
+template 
+constexpr int callConstevalWithNameDependentArgAsVariable() {
+  constexpr int x = X;
+  return f1(x);
+}
+
+auto foo = callConstevalWithNameDependentArg<42>();
+
 }
 
 consteval int main() { // expected-error {{'main' is not allowed to be 
declared consteval}}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16654,11 +16654,22 @@
   /// It's OK if this fails; we'll also remove this in
   /// HandleImmediateInvocations, but catching it here allows us to avoid
   /// walking the AST looking for it in simple cases.
-  if (auto *Call = dyn_cast(E.get()->IgnoreImplicit()))
+  if (auto *Call = dyn_cast(E.get()->IgnoreImplicit())) {
 if (auto *DeclRef =
 dyn_cast(Call->getCallee()->IgnoreImplicit()))
   ExprEvalContexts.back().ReferenceToConsteval.erase(DeclRef);
 
+// If any arguments are value-dependent, we will not be able to evaluate
+// the function call until instantiation.
+if (Call->isValueDependent())
+  return E;
+
+for (Expr *Arg : Call->arguments()) {
+  if (Arg->isValueDependent())
+return E;
+}
+  }
+
   E = MaybeCreateExprWithCleanups(E);
 
   ConstantExpr *Res = ConstantExpr::Create(


Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -61,6 +61,20 @@
 struct E : C {
   consteval ~E() {} // expected-error {{cannot be declared consteval}}
 };
+
+template 
+constexpr int callConstevalWithNameDependentArg() {
+  return f1(X);
+}
+
+template 
+constexpr int callConstevalWithNameDependentArgAsVariable() {
+  constexpr int x = X;
+  return f1(x);
+}
+
+auto foo = callConstevalWithNameDependentArg<42>();
+
 }
 
 consteval int main() { // expected-error {{'main' is not allowed to be declared consteval}}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16654,11 +16654,22 @@
   /// It's OK if this fails; we'll also remove this in
   /// HandleImmediateInvocations, but catching it here allows us to avoid
   /// walking the AST looking for it in simple cases.
-  if (auto *Call = dyn_cast(E.get()->IgnoreImplicit()))
+  if (auto *Call = dyn_cast(E.get()->IgnoreImplicit())) {
 if (auto *DeclRef =
 dyn_cast(Call->getCallee()->IgnoreImplicit()))
   ExprEvalContexts.back().ReferenceToConsteval.erase(DeclRef);
 
+// If any arguments are value-dependent, we will not be able to evaluate
+// the function call until instantiation.
+if (Call->isValueDependent())
+  return E;
+
+for (Expr *Arg : Call->arguments()) {
+  if (Arg->isValueDependent())
+return E;
+}
+  }
+
   E = MaybeCreateExprWithCleanups(E);
 
   ConstantExpr *Res = ConstantExpr::Create(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113830: [clang-tidy] Fix false positive in `readability-identifier-naming` check involving `override` attribute

[Fabian Wolff via Phabricator via cfe-commits]

fwolff updated this revision to Diff 387121.
fwolff added a comment.

Thanks for your comments @salman-javed-nz! I have expanded the tests now 
according to your suggestions.


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

https://reviews.llvm.org/D113830

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
@@ -285,6 +285,10 @@
   virtual void BadBaseMethod() = 0;
   // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethod'
   // CHECK-FIXES: {{^}}  virtual void v_Bad_Base_Method() = 0;
+
+  virtual void BadBaseMethodNoAttr() = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethodNoAttr'
+  // CHECK-FIXES: {{^}}  virtual void v_Bad_Base_Method_No_Attr() = 0;
 };
 
 class COverriding : public AOverridden {
@@ -292,6 +296,8 @@
   // Overriding a badly-named base isn't a new violation.
   void BadBaseMethod() override {}
   // CHECK-FIXES: {{^}}  void v_Bad_Base_Method() override {}
+  void BadBaseMethodNoAttr() {}
+  // CHECK-FIXES: {{^}}  void v_Bad_Base_Method_No_Attr() {}
 
   void foo() {
 BadBaseMethod();
@@ -302,14 +308,66 @@
 // CHECK-FIXES: {{^}}AOverridden::v_Bad_Base_Method();
 COverriding::BadBaseMethod();
 // CHECK-FIXES: {{^}}COverriding::v_Bad_Base_Method();
+
+BadBaseMethodNoAttr();
+// CHECK-FIXES: {{^}}v_Bad_Base_Method_No_Attr();
   }
 };
 
-void VirtualCall(AOverridden &a_vItem) {
+// Same test as above, now with a dependent base class.
+template
+class ATOverridden {
+public:
+  virtual void BadBaseMethod() = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethod'
+  // CHECK-FIXES: {{^}}  virtual void v_Bad_Base_Method() = 0;
+
+  virtual void BadBaseMethodNoAttr() = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethodNoAttr'
+  // CHECK-FIXES: {{^}}  virtual void v_Bad_Base_Method_No_Attr() = 0;
+};
+
+template
+class CTOverriding : public ATOverridden {
+  // Overriding a badly-named base isn't a new violation.
+  // FIXME: The fixes from the base class should be propagated to the derived class here
+  //(note that there could be specializations of the template base class, though)
+  void BadBaseMethod() override{}
+
+  // Without the "override" attribute, and due to the dependent base class, it is not
+  // known whether this method overrides anything, so we get the warning here.
+  virtual void BadBaseMethodNoAttr() {};
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethodNoAttr'
+  // CHECK-FIXES: {{^}}  virtual void v_Bad_Base_Method_No_Attr() {};
+};
+
+template
+void VirtualCall(AOverridden &a_vItem, ATOverridden &a_vTitem) {
   a_vItem.BadBaseMethod();
   // CHECK-FIXES: {{^}}  a_vItem.v_Bad_Base_Method();
+
+  // FIXME: The fixes from ATOverridden should be propagated to the following call
+  a_vTitem.BadBaseMethod();
 }
 
+// Same test as above, now with a dependent base class that is instantiated below.
+template
+class ATIOverridden {
+public:
+  virtual void BadBaseMethod() = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethod'
+  // CHECK-FIXES: {{^}}  virtual void v_Bad_Base_Method() = 0;
+};
+
+template
+class CTIOverriding : public ATIOverridden {
+  // Overriding a badly-named base isn't a new violation.
+  void BadBaseMethod() override{}
+  // CHECK-FIXES: {{^}}  void v_Bad_Base_Method() override{}
+};
+
+template class CTIOverriding;
+
 template 
 class CRTPBase {
 public:
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -1257,7 +1257,7 @@
 
   if (const auto *Decl = dyn_cast(D)) {
 if (Decl->isMain() || !Decl->isUserProvided() ||
-Decl->size_overridden_methods() > 0)
+Decl->size_overridden_methods() > 0 || Decl->hasAttr())
   return SK_Invalid;
 
 // If this method has the same name as any base method, this is likely
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113708: [clang-tidy] Fix `bugprone-use-after-move` check to also consider moves in constructor initializers

[Fabian Wolff via Phabricator via cfe-commits]

fwolff updated this revision to Diff 387116.
fwolff added a comment.

Thanks for reviewing this @carlosgalvezp! I have removed the "dead" test and 
put it into a follow-up issue (PR#52502).

If you are otherwise happy with the changes, could you commit them for me?


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

https://reviews.llvm.org/D113708

Files:
  clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -1338,3 +1338,15 @@
   Foo Other{std::move(Bar)};
 }
 } // namespace UnevalContext
+
+class PR38187 {
+public:
+  PR38187(std::string val) : val_(std::move(val)) {
+val.empty();
+// CHECK-NOTES: [[@LINE-1]]:5: warning: 'val' used after it was moved
+// CHECK-NOTES: [[@LINE-3]]:30: note: move occurred here
+  }
+
+private:
+  std::string val_;
+};
Index: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -129,8 +129,12 @@
   Visited.clear();
 
   const CFGBlock *Block = BlockMap->blockContainingStmt(MovingCall);
-  if (!Block)
-return false;
+  if (!Block) {
+// This can happen if MovingCall is in a constructor initializer, which is
+// not included in the CFG because the CFG is built only from the function
+// body.
+Block = &TheCFG->getEntry();
+  }
 
   return findInternal(Block, MovingCall, MovedVariable, TheUseAfterMove);
 }


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -1338,3 +1338,15 @@
   Foo Other{std::move(Bar)};
 }
 } // namespace UnevalContext
+
+class PR38187 {
+public:
+  PR38187(std::string val) : val_(std::move(val)) {
+val.empty();
+// CHECK-NOTES: [[@LINE-1]]:5: warning: 'val' used after it was moved
+// CHECK-NOTES: [[@LINE-3]]:30: note: move occurred here
+  }
+
+private:
+  std::string val_;
+};
Index: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -129,8 +129,12 @@
   Visited.clear();
 
   const CFGBlock *Block = BlockMap->blockContainingStmt(MovingCall);
-  if (!Block)
-return false;
+  if (!Block) {
+// This can happen if MovingCall is in a constructor initializer, which is
+// not included in the CFG because the CFG is built only from the function
+// body.
+Block = &TheCFG->getEntry();
+  }
 
   return findInternal(Block, MovingCall, MovedVariable, TheUseAfterMove);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113848: [clang-tidy][NFC] Refactor ClangTidyDiagnosticConsumer files

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp created this revision.
carlosgalvezp added reviewers: aaron.ballman, whisperity.
Herald added subscribers: rnkovacs, xazax.hun, mgorny.
carlosgalvezp requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Currently, ClangTidyDiagnosticConsumer defines 2 additional
classes: ClangTidyContext and ClangTidyError. A reader does
not expect them to be there, and a file needing only one
of them needs to include the heavy ClangTidyDiangosticConsumer.h,
increasing compilation time.

Refactor by moving these classes into their own set of .h/.cpp files.
Besides readability, compilation time is also improved.
Adjust includes accordingly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113848

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/ClangTidyContext.cpp
  clang-tools-extra/clang-tidy/ClangTidyContext.h
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clang-tidy/ClangTidyError.cpp
  clang-tools-extra/clang-tidy/ClangTidyError.h
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
@@ -12,6 +12,7 @@
 #include "ClangTidy.h"
 #include "ClangTidyCheck.h"
 #include "ClangTidyDiagnosticConsumer.h"
+#include "ClangTidyError.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -1,6 +1,7 @@
 #include "ClangTidyOptions.h"
 #include "ClangTidyCheck.h"
 #include "ClangTidyDiagnosticConsumer.h"
+#include "ClangTidyError.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Testing/Support/Annotations.h"
Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -16,6 +16,8 @@
 
 #include "ClangTidyMain.h"
 #include "../ClangTidy.h"
+#include "../ClangTidyContext.h"
+#include "../ClangTidyError.h"
 #include "../ClangTidyForceLinker.h"
 #include "../GlobList.h"
 #include "clang/Tooling/CommonOptionsParser.h"
@@ -128,10 +130,10 @@
cl::init(false), cl::cat(ClangTidyCategory));
 
 static cl::opt FixNotes("fix-notes", cl::desc(R"(
-If a warning has no fix, but a single fix can 
-be found through an associated diagnostic note, 
-apply the fix. 
-Specifying this flag will implicitly enable the 
+If a warning has no fix, but a single fix can
+be found through an associated diagnostic note,
+apply the fix.
+Specifying this flag will implicitly enable the
 '--fix' flag.
 )"),
   cl::init(false), cl::cat(ClangTidyCategory));
Index: clang-tools-extra/clang-tidy/ClangTidyError.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/ClangTidyError.h
@@ -0,0 +1,38 @@
+//===--- ClangTidyError.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_CLANGTIDYERROR_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYERROR_H
+
+#include "clang/Tooling/Core/Diagnostic.h"
+#include "llvm/ADT/StringRef.h"
+#include 
+#include 
+
+namespace clang {
+namespace tidy {
+
+/// A detected error complete with information to display diagnostic and
+/// automatic fix.
+///
+/// This is used as an intermediate format to transport Diagnostics without a
+/// dependency on a SourceManager.
+///
+/// FIXME: Make Diagnostics flexible enough to support this directly.
+struct ClangTidyError : tooling::Diagnostic {
+  ClangTidyError(StringRef CheckName, Level DiagLevel, StringRef BuildDirectory,
+ bool IsWarningAsError);
+
+  bool IsWarningAsError;
+  std::vector EnabledDiagnosticAliases;
+};
+
+} // end namespace tidy
+} // end namespace

[clang] d0ac215 - [clang] Use isa instead of dyn_cast (NFC)

[Kazu Hirata via cfe-commits]

Author: Kazu Hirata
Date: 2021-11-14T09:32:40-08:00
New Revision: d0ac215dd5496a44ce8a6660378ea40a6e1c148d

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

LOG: [clang] Use isa instead of dyn_cast (NFC)

Added: 


Modified: 
clang/include/clang/AST/LambdaCapture.h
clang/include/clang/Sema/Initialization.h
clang/lib/AST/QualTypeNames.cpp
clang/lib/AST/Type.cpp
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/CodeGen/CoverageMappingGen.cpp
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclObjC.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/lib/Sema/SemaType.cpp
clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
clang/lib/StaticAnalyzer/Checkers/WebKit/UncountedCallArgsChecker.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
clang/tools/libclang/CXCursor.cpp

Removed: 




diff  --git a/clang/include/clang/AST/LambdaCapture.h 
b/clang/include/clang/AST/LambdaCapture.h
index 8e2806545dd68..7ad1e2361e427 100644
--- a/clang/include/clang/AST/LambdaCapture.h
+++ b/clang/include/clang/AST/LambdaCapture.h
@@ -86,7 +86,7 @@ class LambdaCapture {
 
   /// Determine whether this capture handles a variable.
   bool capturesVariable() const {
-return dyn_cast_or_null(DeclAndBits.getPointer());
+return isa_and_nonnull(DeclAndBits.getPointer());
   }
 
   /// Determine whether this captures a variable length array bound

diff  --git a/clang/include/clang/Sema/Initialization.h 
b/clang/include/clang/Sema/Initialization.h
index 8c1856f208279..679e12ee22d4a 100644
--- a/clang/include/clang/Sema/Initialization.h
+++ b/clang/include/clang/Sema/Initialization.h
@@ -488,7 +488,7 @@ class alignas(8) InitializedEntity {
 
   /// Determine whether this is an array new with an unknown bound.
   bool isVariableLengthArrayNew() const {
-return getKind() == EK_New && dyn_cast_or_null(
+return getKind() == EK_New && isa_and_nonnull(
   getType()->getAsArrayTypeUnsafe());
   }
 

diff  --git a/clang/lib/AST/QualTypeNames.cpp b/clang/lib/AST/QualTypeNames.cpp
index 9a1b418f5ac1b..6738210783455 100644
--- a/clang/lib/AST/QualTypeNames.cpp
+++ b/clang/lib/AST/QualTypeNames.cpp
@@ -296,7 +296,7 @@ static NestedNameSpecifier 
*createNestedNameSpecifierForScopeOf(
 } else if (const auto *TD = dyn_cast(Outer)) {
   return createNestedNameSpecifier(
   Ctx, TD, FullyQualified, WithGlobalNsPrefix);
-} else if (dyn_cast(Outer)) {
+} else if (isa(Outer)) {
   // Context is the TU. Nothing needs to be done.
   return nullptr;
 } else {

diff  --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 4adf367f2da6d..e0ac3f5b1351d 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -1892,7 +1892,7 @@ DeducedType *Type::getContainedDeducedType() const {
 }
 
 bool Type::hasAutoForTrailingReturnType() const {
-  return dyn_cast_or_null(
+  return isa_and_nonnull(
   GetContainedDeducedTypeVisitor(true).Visit(this));
 }
 

diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 9f4e0f12e0232..8606e80418d7f 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -3913,7 +3913,7 @@ static void emitPrivatesInit(CodeGenFunction &CGF,
   SharedRefLValue.getTBAAInfo());
 } else if (CGF.LambdaCaptureFields.count(
Pair.second.Original->getCanonicalDecl()) > 0 ||
-   dyn_cast_or_null(CGF.CurCodeDecl)) {
+   isa_and_nonnull(CGF.CurCodeDecl)) {
   SharedRefLValue = CGF.EmitLValue(Pair.second.OriginalRef);
 } else {
   // Processing for implicitly captured variables.

diff  --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp 
b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index 2cf01939d9b64..3ca50503c62cb 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -4720,7 +4720,7 @@ void CodeGenFunction::EmitOMPTargetTaskBasedDirective(
[&InputInfo]() { return InputInfo.SizesArray; });
 // If there is no user-defined mapper, the mapper array will be nullptr. In
 // this case, we don't need to privatize it.
-if (!dyn_cast_or_null(
+if (!isa_and_nonnull(
 InputInfo.MappersArray.getPointer())) {
   MVD = createImplicitFirstprivateForType(
   getContext(), Data, BaseAndPointerAndMapperType, CD, 
S.getBeginLoc());

[PATCH] D113847: [clang-tidy][NFC] Simplify ClangTidyStats

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp created this revision.
carlosgalvezp added reviewers: aaron.ballman, whisperity.
Herald added subscribers: rnkovacs, xazax.hun.
carlosgalvezp requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

- Use NSDMI and remove constructor.
- Replace "unsigned" with "int". Unsigned ints should not be used to express 
that "a number cannot be negative". To count things and perform arithmetic, we 
should use regular ints. Unsigned ints are useful for e.g. bitwise operations.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113847

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h


Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -45,20 +45,15 @@
   std::vector EnabledDiagnosticAliases;
 };
 
-/// Contains displayed and ignored diagnostic counters for a ClangTidy
-/// run.
+/// Contains displayed and ignored diagnostic counters for a ClangTidy run.
 struct ClangTidyStats {
-  ClangTidyStats()
-  : ErrorsDisplayed(0), ErrorsIgnoredCheckFilter(0), 
ErrorsIgnoredNOLINT(0),
-ErrorsIgnoredNonUserCode(0), ErrorsIgnoredLineFilter(0) {}
+  int ErrorsDisplayed{0};
+  int ErrorsIgnoredCheckFilter{0};
+  int ErrorsIgnoredNOLINT{0};
+  int ErrorsIgnoredNonUserCode{0};
+  int ErrorsIgnoredLineFilter{0};
 
-  unsigned ErrorsDisplayed;
-  unsigned ErrorsIgnoredCheckFilter;
-  unsigned ErrorsIgnoredNOLINT;
-  unsigned ErrorsIgnoredNonUserCode;
-  unsigned ErrorsIgnoredLineFilter;
-
-  unsigned errorsIgnored() const {
+  int errorsIgnored() const {
 return ErrorsIgnoredNOLINT + ErrorsIgnoredCheckFilter +
ErrorsIgnoredNonUserCode + ErrorsIgnoredLineFilter;
   }


Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -45,20 +45,15 @@
   std::vector EnabledDiagnosticAliases;
 };
 
-/// Contains displayed and ignored diagnostic counters for a ClangTidy
-/// run.
+/// Contains displayed and ignored diagnostic counters for a ClangTidy run.
 struct ClangTidyStats {
-  ClangTidyStats()
-  : ErrorsDisplayed(0), ErrorsIgnoredCheckFilter(0), ErrorsIgnoredNOLINT(0),
-ErrorsIgnoredNonUserCode(0), ErrorsIgnoredLineFilter(0) {}
+  int ErrorsDisplayed{0};
+  int ErrorsIgnoredCheckFilter{0};
+  int ErrorsIgnoredNOLINT{0};
+  int ErrorsIgnoredNonUserCode{0};
+  int ErrorsIgnoredLineFilter{0};
 
-  unsigned ErrorsDisplayed;
-  unsigned ErrorsIgnoredCheckFilter;
-  unsigned ErrorsIgnoredNOLINT;
-  unsigned ErrorsIgnoredNonUserCode;
-  unsigned ErrorsIgnoredLineFilter;
-
-  unsigned errorsIgnored() const {
+  int errorsIgnored() const {
 return ErrorsIgnoredNOLINT + ErrorsIgnoredCheckFilter +
ErrorsIgnoredNonUserCode + ErrorsIgnoredLineFilter;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113837: Sema: Let InitListExpr have dependent type instead of 'void'

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a subscriber: JonasToth.
aaronpuchert added a comment.

We could also introduce a separate (placeholder) type for initializer lists and 
perhaps also `ParenListExpr` if the dependent type seems misleading to you.

CC @JonasToth for the changes to 
`clang-tidy/checkers/cppcoreguidelines-owning-memory.cpp`.




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-owning-memory.cpp:306
   }
   HeapArray(int size, T val, int *problematic) : _data{problematic}, 
size(size) {} // Bad
+  // CHECK-NOTES: [[@LINE-1]]:50: warning: expected initialization of owner 
member variable with value of type 'gsl::owner<>'; got ''

If the warning is looking at this in the original template, we're initializing 
a `gsl::owner` which is type-dependent. So we can't carry out the 
initialization yet.

[Got the impression 
earlier](https://github.com/mgehre/llvm-project/issues/80#issuecomment-567210626)
 that the lifetime checks want to “understand” templates which can of course 
not work because templates can be specialized, overload resolution is only 
performed on  the instantiation, and so on.



Comment at: clang/test/SemaOpenCLCXX/address-space-references.clcpp:25
   c.gen({1, 2});
-  c.glob({1, 2}); //expected-error{{binding reference of type 'const __global 
short2' (vector of 2 'short' values) to value of type 'void' changes address 
space}}
+  c.glob({1, 2}); //expected-error{{binding reference of type 'const __global 
short2' (vector of 2 'short' values) to value of type '' 
changes address space}}
   c.nested_list({{1, 2}, {3, 4}});

Somehow we seem to fail matching the initializer list, but not report that, 
only on type mismatch later. I think we should either go through with the 
initialization, then have a `short __attribute__((ext_vector_type(2)))` and 
complain that we can't bind the reference to that, or complain when processing 
the initializer list.

But that's unrelated to this change, `void` was also not correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113837

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

[PATCH] D113838: Sema: Don't erroneously reject `void{}`

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a subscriber: Tyker.
aaronpuchert added a comment.

CC @Tyker for the changes to `SemaCXX/attr-annotate.cpp`.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5848-5849
 def err_illegal_initializer_type : Error<"illegal initializer type %0">;
+def err_init_list_void_nonempty : Error<
+  "initializer list for 'void' must be empty">;
 def ext_init_list_type_narrowing : ExtWarn<

Might also consider it a narrowing conversion. But I can't find the standard 
specifically calling it that.



Comment at: clang/test/CXX/expr/expr.post/expr.type.conv/p2.cpp:12
+void brace_list_nonempty() {
+  return void{1}; // expected-error {{initializer list for 'void' must be 
empty}}
+}

With parantheses this is (correctly) allowed:

> If the initializer is a parenthesized single expression, the type conversion 
> expression is equivalent to the corresponding cast expression.

[expr.static.cast]p6:

> Any expression can be explicitly converted to type cv `void`, in which case 
> it becomes a discarded-value expression ([expr.prop]).

Not sure if we need a test for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113838

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

[PATCH] D113708: [clang-tidy] Fix `bugprone-use-after-move` check to also consider moves in constructor initializers

[Carlos Galvez via Phabricator via cfe-commits]

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

Looks good! I'm not sure if it's OK to write "dead tests" though (the one with 
the `TODO` comment). Would it make sense to remove it from this patch, and add 
it in the patch where the issue is fixed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113708

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

[PATCH] D113518: [clang] Create delegating constructors even in templates

[Carlos Galvez via Phabricator via cfe-commits]

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

Looks great, thanks! I'm not comfortable reviewing the Sema part, maybe someone 
else can have a look?


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

https://reviews.llvm.org/D113518

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

[PATCH] D95168: [clang-format] Add Insert/Remove Braces option

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/BraceInserter.cpp:21-56
+bool SupportsAnyAutomaticBraces(FormatStyle &Style, bool insertion) {
+  if (insertion) {
+if (Style.AutomaticBraces.AfterIf == FormatStyle::BIS_Always)
+  return true;
+if (Style.AutomaticBraces.AfterIf == FormatStyle::BIS_WrapLikely)
+  return true;
+if (Style.AutomaticBraces.AfterFor == FormatStyle::BIS_Always)

HazardyKnusperkeks wrote:
> MyDeveloperDay wrote:
> > HazardyKnusperkeks wrote:
> > > How about something like that?
> > I think for now
> > 
> > ```
> > bool SupportsAnyAutomaticBraces(FormatStyle &Style, bool insertion) {
> >   if (   Style.AutomaticBraces.AfterIf == FormatStyle::BIS_Leave
> >   && Style.AutomaticBraces.AfterFor == FormatStyle::BIS_Leave
> >   && Style.AutomaticBraces.AfterWhile == FormatStyle::BIS_Leave
> >   && Style.AutomaticBraces.AfterDo == FormatStyle::BIS_Leave
> >   && Style.AutomaticBraces.AfterElse == FormatStyle::BIS_Leave)
> >   return false;
> > 
> >   if (!insertion){
> > if (Style.AutomaticBraces.AfterIf != FormatStyle::BIS_Remove)
> > && Style.AutomaticBraces.AfterFor != FormatStyle::BIS_Remove)
> > && Style.AutomaticBraces.AfterWhile != FormatStyle::BIS_Remove)
> > && Style.AutomaticBraces.AfterDo != FormatStyle::BIS_Remove)
> > && Style.AutomaticBraces.AfterElse != FormatStyle::BIS_Remove)
> >   return false;
> >   }
> >   return true;
> > }
> > ```
> > 
> > Should cover it.
> Yeah, but I like my variant better. :)
but I'm not clever enough to understand it!


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

https://reviews.llvm.org/D95168

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

[PATCH] D113844: [clang-format] [NFC] build clang-format with -Wall

[MyDeveloperDay via Phabricator via cfe-commits]

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

When building clang-format with -Wall on Visual Studio 20119

I see the following, prevent this the only -Wall error

  ..FormatTokenLexer.cpp(45) : warning C4868: compiler may not enforce 
left-to-right evaluation order in braced initializer list


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113844

Files:
  clang/lib/Format/FormatTokenLexer.cpp


Index: clang/lib/Format/FormatTokenLexer.cpp
===
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -37,27 +37,40 @@
   getFormattingLangOpts(Style)));
   Lex->SetKeepWhitespaceMode(true);
 
-  for (const std::string &ForEachMacro : Style.ForEachMacros)
-Macros.insert({&IdentTable.get(ForEachMacro), TT_ForEachMacro});
-  for (const std::string &IfMacro : Style.IfMacros)
-Macros.insert({&IdentTable.get(IfMacro), TT_IfMacro});
-  for (const std::string &AttributeMacro : Style.AttributeMacros)
-Macros.insert({&IdentTable.get(AttributeMacro), TT_AttributeMacro});
-  for (const std::string &StatementMacro : Style.StatementMacros)
-Macros.insert({&IdentTable.get(StatementMacro), TT_StatementMacro});
-  for (const std::string &TypenameMacro : Style.TypenameMacros)
-Macros.insert({&IdentTable.get(TypenameMacro), TT_TypenameMacro});
-  for (const std::string &NamespaceMacro : Style.NamespaceMacros)
-Macros.insert({&IdentTable.get(NamespaceMacro), TT_NamespaceMacro});
+  for (const std::string &ForEachMacro : Style.ForEachMacros) {
+auto Identifier = &IdentTable.get(ForEachMacro);
+Macros.insert({Identifier, TT_ForEachMacro});
+  }
+  for (const std::string &IfMacro : Style.IfMacros) {
+auto Identifier = &IdentTable.get(IfMacro);
+Macros.insert({Identifier, TT_IfMacro});
+  }
+  for (const std::string &AttributeMacro : Style.AttributeMacros) {
+auto Identifier = &IdentTable.get(AttributeMacro);
+Macros.insert({Identifier, TT_AttributeMacro});
+  }
+  for (const std::string &StatementMacro : Style.StatementMacros) {
+auto Identifier = &IdentTable.get(StatementMacro);
+Macros.insert({Identifier, TT_StatementMacro});
+  }
+  for (const std::string &TypenameMacro : Style.TypenameMacros) {
+auto Identifier = &IdentTable.get(TypenameMacro);
+Macros.insert({Identifier, TT_TypenameMacro});
+  }
+  for (const std::string &NamespaceMacro : Style.NamespaceMacros) {
+auto Identifier = &IdentTable.get(NamespaceMacro);
+Macros.insert({Identifier, TT_NamespaceMacro});
+  }
   for (const std::string &WhitespaceSensitiveMacro :
Style.WhitespaceSensitiveMacros) {
-Macros.insert(
-{&IdentTable.get(WhitespaceSensitiveMacro), TT_UntouchableMacroFunc});
+auto Identifier = &IdentTable.get(WhitespaceSensitiveMacro);
+Macros.insert({Identifier, TT_UntouchableMacroFunc});
   }
   for (const std::string &StatementAttributeLikeMacro :
-   Style.StatementAttributeLikeMacros)
-Macros.insert({&IdentTable.get(StatementAttributeLikeMacro),
-   TT_StatementAttributeLikeMacro});
+   Style.StatementAttributeLikeMacros) {
+auto Identifier = &IdentTable.get(StatementAttributeLikeMacro);
+Macros.insert({Identifier, TT_StatementAttributeLikeMacro});
+  }
 }
 
 ArrayRef FormatTokenLexer::lex() {


Index: clang/lib/Format/FormatTokenLexer.cpp
===
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -37,27 +37,40 @@
   getFormattingLangOpts(Style)));
   Lex->SetKeepWhitespaceMode(true);
 
-  for (const std::string &ForEachMacro : Style.ForEachMacros)
-Macros.insert({&IdentTable.get(ForEachMacro), TT_ForEachMacro});
-  for (const std::string &IfMacro : Style.IfMacros)
-Macros.insert({&IdentTable.get(IfMacro), TT_IfMacro});
-  for (const std::string &AttributeMacro : Style.AttributeMacros)
-Macros.insert({&IdentTable.get(AttributeMacro), TT_AttributeMacro});
-  for (const std::string &StatementMacro : Style.StatementMacros)
-Macros.insert({&IdentTable.get(StatementMacro), TT_StatementMacro});
-  for (const std::string &TypenameMacro : Style.TypenameMacros)
-Macros.insert({&IdentTable.get(TypenameMacro), TT_TypenameMacro});
-  for (const std::string &NamespaceMacro : Style.NamespaceMacros)
-Macros.insert({&IdentTable.get(NamespaceMacro), TT_NamespaceMacro});
+  for (const std::string &ForEachMacro : Style.ForEachMacros) {
+auto Identifier = &IdentTable.get(ForEachMacro);
+Macros.insert({Identifier, TT_ForEachMacro});
+  }
+  for (const std::string &IfMacro : Style.IfMacros) {
+auto Identifier = &IdentTable.get(IfMacro);
+Macros.insert({Identifier, TT_IfMacro});
+  }
+  for (const std::string

[PATCH] D113826: [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

[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 rGfce3eed9f93a: [clang-format][c++2b] support removal of the 
space between auto and {} in… (authored by MyDeveloperDay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113826

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
@@ -22566,6 +22566,30 @@
   EXPECT_EQ(Code, format(Code, Style));
 }
 
+TEST_F(FormatTest, FormatDecayCopy) {
+  // error cases from unit tests
+  verifyFormat("foo(auto())");
+  verifyFormat("foo(auto{})");
+  verifyFormat("foo(auto({}))");
+  verifyFormat("foo(auto{{}})");
+
+  verifyFormat("foo(auto(1))");
+  verifyFormat("foo(auto{1})");
+  verifyFormat("foo(new auto(1))");
+  verifyFormat("foo(new auto{1})");
+  verifyFormat("decltype(auto(1)) x;");
+  verifyFormat("decltype(auto{1}) x;");
+  verifyFormat("auto(x);");
+  verifyFormat("auto{x};");
+  verifyFormat("new auto{x};");
+  verifyFormat("auto{x} = y;");
+  verifyFormat("auto(x) = y;"); // actually a declaration, but this is clearly
+// the user's own fault
+  verifyFormat("integral auto(x) = y;"); // actually a declaration, but this is
+ // clearly the user's own fault
+  verifyFormat("auto(*p)() = f;");   // actually a declaration; TODO FIXME
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2940,6 +2940,10 @@
   return true;
   }
 
+  // auto{x} auto(x)
+  if (Left.is(tok::kw_auto) && Right.isOneOf(tok::l_paren, tok::l_brace))
+return false;
+
   // requires clause Concept1 && Concept2
   if (Left.is(TT_ConstraintJunctions) && Right.is(tok::identifier))
 return true;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -22566,6 +22566,30 @@
   EXPECT_EQ(Code, format(Code, Style));
 }
 
+TEST_F(FormatTest, FormatDecayCopy) {
+  // error cases from unit tests
+  verifyFormat("foo(auto())");
+  verifyFormat("foo(auto{})");
+  verifyFormat("foo(auto({}))");
+  verifyFormat("foo(auto{{}})");
+
+  verifyFormat("foo(auto(1))");
+  verifyFormat("foo(auto{1})");
+  verifyFormat("foo(new auto(1))");
+  verifyFormat("foo(new auto{1})");
+  verifyFormat("decltype(auto(1)) x;");
+  verifyFormat("decltype(auto{1}) x;");
+  verifyFormat("auto(x);");
+  verifyFormat("auto{x};");
+  verifyFormat("new auto{x};");
+  verifyFormat("auto{x} = y;");
+  verifyFormat("auto(x) = y;"); // actually a declaration, but this is clearly
+// the user's own fault
+  verifyFormat("integral auto(x) = y;"); // actually a declaration, but this is
+ // clearly the user's own fault
+  verifyFormat("auto(*p)() = f;");   // actually a declaration; TODO FIXME
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2940,6 +2940,10 @@
   return true;
   }
 
+  // auto{x} auto(x)
+  if (Left.is(tok::kw_auto) && Right.isOneOf(tok::l_paren, tok::l_brace))
+return false;
+
   // requires clause Concept1 && Concept2
   if (Left.is(TT_ConstraintJunctions) && Right.is(tok::identifier))
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] fce3eed - [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

[via cfe-commits]

Author: mydeveloperday
Date: 2021-11-14T14:13:44Z
New Revision: fce3eed9f93afac512d809c22234db7be7a9d478

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

LOG: [clang-format][c++2b] support removal of the space between auto and {} in 
P0849R8

Looks like the work of {D113393} requires manual clang-formatting intervention.
Removal of the space between `auto` and `{}`

Reviewed By: HazardyKnusperkeks, Quuxplusone

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

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 73c88201f76d3..ace3d25ca460c 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -2940,6 +2940,10 @@ bool TokenAnnotator::spaceRequiredBetween(const 
AnnotatedLine &Line,
   return true;
   }
 
+  // auto{x} auto(x)
+  if (Left.is(tok::kw_auto) && Right.isOneOf(tok::l_paren, tok::l_brace))
+return false;
+
   // requires clause Concept1 && Concept2
   if (Left.is(TT_ConstraintJunctions) && Right.is(tok::identifier))
 return true;

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index eea1e49da8089..6507cbefc5a90 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -22566,6 +22566,30 @@ TEST_F(FormatTest, LimitlessStringsAndComments) {
   EXPECT_EQ(Code, format(Code, Style));
 }
 
+TEST_F(FormatTest, FormatDecayCopy) {
+  // error cases from unit tests
+  verifyFormat("foo(auto())");
+  verifyFormat("foo(auto{})");
+  verifyFormat("foo(auto({}))");
+  verifyFormat("foo(auto{{}})");
+
+  verifyFormat("foo(auto(1))");
+  verifyFormat("foo(auto{1})");
+  verifyFormat("foo(new auto(1))");
+  verifyFormat("foo(new auto{1})");
+  verifyFormat("decltype(auto(1)) x;");
+  verifyFormat("decltype(auto{1}) x;");
+  verifyFormat("auto(x);");
+  verifyFormat("auto{x};");
+  verifyFormat("new auto{x};");
+  verifyFormat("auto{x} = y;");
+  verifyFormat("auto(x) = y;"); // actually a declaration, but this is clearly
+// the user's own fault
+  verifyFormat("integral auto(x) = y;"); // actually a declaration, but this is
+ // clearly the user's own fault
+  verifyFormat("auto(*p)() = f;");   // actually a declaration; TODO FIXME
+}
+
 } // namespace
 } // namespace format
 } // namespace clang



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

[PATCH] D113826: [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

[MyDeveloperDay via Phabricator via cfe-commits]

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

minor comment and unit test change before committing


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

https://reviews.llvm.org/D113826

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
@@ -22566,6 +22566,30 @@
   EXPECT_EQ(Code, format(Code, Style));
 }
 
+TEST_F(FormatTest, FormatDecayCopy) {
+  // error cases from unit tests
+  verifyFormat("foo(auto())");
+  verifyFormat("foo(auto{})");
+  verifyFormat("foo(auto({}))");
+  verifyFormat("foo(auto{{}})");
+
+  verifyFormat("foo(auto(1))");
+  verifyFormat("foo(auto{1})");
+  verifyFormat("foo(new auto(1))");
+  verifyFormat("foo(new auto{1})");
+  verifyFormat("decltype(auto(1)) x;");
+  verifyFormat("decltype(auto{1}) x;");
+  verifyFormat("auto(x);");
+  verifyFormat("auto{x};");
+  verifyFormat("new auto{x};");
+  verifyFormat("auto{x} = y;");
+  verifyFormat("auto(x) = y;"); // actually a declaration, but this is clearly
+// the user's own fault
+  verifyFormat("integral auto(x) = y;"); // actually a declaration, but this is
+ // clearly the user's own fault
+  verifyFormat("auto(*p)() = f;");   // actually a declaration; TODO FIXME
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2940,6 +2940,10 @@
   return true;
   }
 
+  // auto{x} auto(x)
+  if (Left.is(tok::kw_auto) && Right.isOneOf(tok::l_paren, tok::l_brace))
+return false;
+
   // requires clause Concept1 && Concept2
   if (Left.is(TT_ConstraintJunctions) && Right.is(tok::identifier))
 return true;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -22566,6 +22566,30 @@
   EXPECT_EQ(Code, format(Code, Style));
 }
 
+TEST_F(FormatTest, FormatDecayCopy) {
+  // error cases from unit tests
+  verifyFormat("foo(auto())");
+  verifyFormat("foo(auto{})");
+  verifyFormat("foo(auto({}))");
+  verifyFormat("foo(auto{{}})");
+
+  verifyFormat("foo(auto(1))");
+  verifyFormat("foo(auto{1})");
+  verifyFormat("foo(new auto(1))");
+  verifyFormat("foo(new auto{1})");
+  verifyFormat("decltype(auto(1)) x;");
+  verifyFormat("decltype(auto{1}) x;");
+  verifyFormat("auto(x);");
+  verifyFormat("auto{x};");
+  verifyFormat("new auto{x};");
+  verifyFormat("auto{x} = y;");
+  verifyFormat("auto(x) = y;"); // actually a declaration, but this is clearly
+// the user's own fault
+  verifyFormat("integral auto(x) = y;"); // actually a declaration, but this is
+ // clearly the user's own fault
+  verifyFormat("auto(*p)() = f;");   // actually a declaration; TODO FIXME
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2940,6 +2940,10 @@
   return true;
   }
 
+  // auto{x} auto(x)
+  if (Left.is(tok::kw_auto) && Right.isOneOf(tok::l_paren, tok::l_brace))
+return false;
+
   // requires clause Concept1 && Concept2
   if (Left.is(TT_ConstraintJunctions) && Right.is(tok::identifier))
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110216: [clang] retain type sugar in auto / template argument deduction

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

Okay, libcxx pipeline passes, disregarding the clang-format failure for 
pre-existing badness in those files, which would need to be fixed separately or 
else git loses track of the rename.
The AIX failures also seem completely unrelated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110216

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

[PATCH] D113830: [clang-tidy] Fix false positive in `readability-identifier-naming` check involving `override` attribute

[Salman Javed via Phabricator via cfe-commits]

salman-javed-nz added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp:329
+
+  // FIXME: The fixes from ATOverridden should be propagated to the following 
call
+  a_vTitem.BadBaseMethod();

The fixes /do/ seem to be propagated if there is an explicit template 
instantiation.

```lang=cpp
template 
struct A
{
virtual void method() {}
};

template 
struct B : A
{
void method() override {}
void CallVirtual() { this->method(); }
};

template class B;
```

Running with `clang-tidy -fix` and `readability-identifier-naming.FunctionCase 
= CamelCase` gives:
```lang=cpp
template 
struct A
{
virtual void Method() {}
};

template 
struct B : A
{
void Method() override {}
void CallVirtual() { this->Method(); }
};

template class B;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113830

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

[PATCH] D113830: [clang-tidy] Fix false positive in `readability-identifier-naming` check involving `override` attribute

[Salman Javed via Phabricator via cfe-commits]

salman-javed-nz added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:1260
 if (Decl->isMain() || !Decl->isUserProvided() ||
-Decl->size_overridden_methods() > 0)
+Decl->size_overridden_methods() > 0 || Decl->hasAttr())
   return SK_Invalid;

Adding a call to `hasAttr()` looks OK to me -- this is in line 
with [[ https://clang.llvm.org/doxygen/ASTMatchers_8h_source.html#l06002 | 
AST_MATCHER(CXXMethodDecl, isOverride)]]. Other clang-tidy checks have done the 
same thing.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp:282-294
 class AOverridden {
 public:
   virtual ~AOverridden() = default;
   virtual void BadBaseMethod() = 0;
   // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual 
method 'BadBaseMethod'
   // CHECK-FIXES: {{^}}  virtual void v_Bad_Base_Method() = 0;
 };

How about a test to check that diagnostics are generated even if the `override` 
keyword is omitted.
This challenges the `Decl->size_overridden_methods() > 0` portion of the `if` 
statement.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp:321
+  //(note that there could be specializations of the template base 
class, though)
+  void BadBaseMethod() override{}
+};

What should happen if the base method is overridden without the `override` 
keyword?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113830

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