[PATCH] D86720: [clang][aarch64] Drop experimental from __ARM_FEATURE_SVE_BITS macro

2020-09-02 Thread David Sherwood via Phabricator via cfe-commits
david-arm accepted this revision.
david-arm added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: clang/lib/Basic/Targets/AArch64.cpp:381
   if (Opts.ArmSveVectorBits)
-Builder.defineMacro("__ARM_FEATURE_SVE_BITS_EXPERIMENTAL",
+Builder.defineMacro("__ARM_FEATURE_SVE_BITS",
 Twine(Opts.ArmSveVectorBits));

nit: Perhaps just reformat the code before submitting?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86720

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


[PATCH] D86936: [clang] Limit the maximum level of fold-expr expansion.

2020-09-02 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 289372.
hokein marked 4 inline comments as done.
hokein added a comment.

address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86936

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/fold_expr_expansion_limit.cpp


Index: clang/test/SemaCXX/fold_expr_expansion_limit.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/fold_expr_expansion_limit.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -fbracket-depth 2 -verify -std=c++17 %s
+
+template  struct seq {
+  constexpr bool zero() { return (true && ... && (V == 0)); }; // 
expected-error {{instantiating fold expression with 3 arguements exceeded 
expression nesting limit of 2}} \
+  
expected-note {{use -fbracket-depth}}
+};
+constexpr unsigned N = 3;
+auto x = __make_integer_seq{};
+static_assert(!x.zero(), ""); // expected-note {{in instantiation of member 
function}}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -28,6 +28,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/Basic/DiagnosticParse.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Lookup.h"
@@ -13193,6 +13194,18 @@
 E->getEllipsisLoc(), RHS.get(), E->getEndLoc(), NumExpansions);
   }
 
+  // Formally a fold expression expands to nested parenthesized expressions.
+  // Enforce this limit to avoid creating trees so deep we can't safely 
traverse
+  // them.
+  if (NumExpansions && SemaRef.getLangOpts().BracketDepth < NumExpansions) {
+SemaRef.Diag(E->getEllipsisLoc(),
+ clang::diag::err_fold_expression_limit_exceeded)
+<< *NumExpansions << SemaRef.getLangOpts().BracketDepth
+<< E->getSourceRange();
+SemaRef.Diag(E->getEllipsisLoc(), diag::note_bracket_depth);
+return ExprError();
+  }
+
   // The transform has determined that we should perform an elementwise
   // expansion of the pattern. Do so.
   ExprResult Result = getDerived().TransformExpr(E->getInit());
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5092,6 +5092,9 @@
   "with no fallback value">;
 def err_fold_expression_bad_operand : Error<
   "expression not permitted as operand of fold expression">;
+def err_fold_expression_limit_exceeded: Error<
+  "instantiating fold expression with %0 arguements exceeded expression 
nesting "
+  "limit of %1">, DefaultFatal, NoSFINAE;
 
 def err_unexpected_typedef : Error<
   "unexpected type name %0: expected expression">;


Index: clang/test/SemaCXX/fold_expr_expansion_limit.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/fold_expr_expansion_limit.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -fbracket-depth 2 -verify -std=c++17 %s
+
+template  struct seq {
+  constexpr bool zero() { return (true && ... && (V == 0)); }; // expected-error {{instantiating fold expression with 3 arguements exceeded expression nesting limit of 2}} \
+  expected-note {{use -fbracket-depth}}
+};
+constexpr unsigned N = 3;
+auto x = __make_integer_seq{};
+static_assert(!x.zero(), ""); // expected-note {{in instantiation of member function}}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -28,6 +28,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/Basic/DiagnosticParse.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Lookup.h"
@@ -13193,6 +13194,18 @@
 E->getEllipsisLoc(), RHS.get(), E->getEndLoc(), NumExpansions);
   }
 
+  // Formally a fold expression expands to nested parenthesized expressions.
+  // Enforce this limit to avoid creating trees so deep we can't safely traverse
+  // them.
+  if (NumExpansions && SemaRef.getLangOpts().BracketDepth < NumExpansions) {
+SemaRef.Diag(E->getEllipsisLoc(),
+ clang::diag::err_fold_expression_limit_exceeded)
+<< *NumExpansions << SemaRef.getLangOpts().BracketDepth
+<< E->getSourceRange();
+SemaRef.Diag(E->getEllipsisLoc(), diag::note_bracket_depth);
+return ExprError();
+  }
+
   // The transform has determined that we should perform an elem

[PATCH] D86936: [clang] Limit the maximum level of fold-expr expansion.

2020-09-02 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: clang/lib/Sema/TreeTransform.h:13196
+  if (NumExpansions && SemaRef.getLangOpts().BracketDepth < NumExpansions) {
+SemaRef.Diag(E->getEllipsisLoc(),
+ clang::diag::err_fold_expression_expansion_exceeded)

sammccall wrote:
> you might also want to emit note_bracket_depth (I don't think you need to 
> clone it if we get the wording right)
note_bracket_depth is from parse diagnostic, introducing it in Sema feels like 
a slight layer violation, but the compile should be fine, because all 
diagnostic headers in the support library.



Comment at: clang/test/SemaCXX/fold_expr_expansion_limit.cpp:8
+auto x = __make_integer_seq{};
+static_assert(!x.zero(), ""); // expected-error {{static_assert expression is 
not an integral constant expression}} \
+ expected-note {{in instantiation of member 
function}}

sammccall wrote:
> this diagnostic is bogus :-(
> default-fatal would fix this, I guess.
oh, yeah, if a fatal error is emitted, all subsequent diagnostics will be 
silenced.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86936

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


[PATCH] D86958: [Docs] Add/update release notes for D71913 (LTO WPD changes)

2020-09-02 Thread Hans Wennborg via Phabricator via cfe-commits
hans accepted this revision.
hans added a comment.

Looks great, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86958

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


[PATCH] D86820: [X86] Add a /tune: option for clang-cl

2020-09-02 Thread Hans Wennborg via Phabricator via cfe-commits
hans accepted this revision.
hans added a comment.
This revision is now accepted and ready to land.

lgtm


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

https://reviews.llvm.org/D86820

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


[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

2020-09-02 Thread Balázs Benics via Phabricator via cfe-commits
steakhal created this revision.
steakhal added reviewers: NoQ, Szelethus, vsavchenko, xazax.hun.
Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, 
dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, 
baloghadamsoftware, whisperity.
Herald added a project: clang.
steakhal requested review of this revision.

We did not evaluate such expressions, just returned `unknown` for such cases.
After this patch, we will be able to access a unique value identifying a 
template instantiation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87004

Files:
  clang/lib/StaticAnalyzer/Core/Environment.cpp
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
  clang/test/Analysis/eval-predefined-exprs.cpp


Index: clang/test/Analysis/eval-predefined-exprs.cpp
===
--- /dev/null
+++ clang/test/Analysis/eval-predefined-exprs.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_analyze_cc1 --std=c++17 
-analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_dump(const char *);
+
+template 
+void func(U param) {
+  clang_analyzer_dump(__func__);
+  clang_analyzer_dump(__FUNCTION__);
+  clang_analyzer_dump(__PRETTY_FUNCTION__);
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"void func(U) [T = Class, Value = 42, U = 
char]",0 S64b,char}}}
+}
+
+void foo() {
+  clang_analyzer_dump(__func__);
+  clang_analyzer_dump(__FUNCTION__);
+  clang_analyzer_dump(__PRETTY_FUNCTION__);
+  // expected-warning@-3 {{&Element{"foo",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"foo",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"void foo()",0 S64b,char}}}
+
+  func('b'); // instantiate template
+}
Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -306,6 +306,11 @@
 return makeLoc(getRegionManager().getStringRegion(SL));
   }
 
+  case Stmt::PredefinedExprClass: {
+const auto *PE = cast(E);
+return makeLoc(getRegionManager().getStringRegion(PE->getFunctionName()));
+  }
+
   // Fast-path some expressions to avoid the overhead of going through the 
AST's
   // constant evaluator
   case Stmt::CharacterLiteralClass: {
Index: clang/lib/StaticAnalyzer/Core/Environment.cpp
===
--- clang/lib/StaticAnalyzer/Core/Environment.cpp
+++ clang/lib/StaticAnalyzer/Core/Environment.cpp
@@ -109,6 +109,7 @@
   case Stmt::StringLiteralClass:
   case Stmt::TypeTraitExprClass:
   case Stmt::SizeOfPackExprClass:
+  case Stmt::PredefinedExprClass:
 // Known constants; defer to SValBuilder.
 return svalBuilder.getConstantVal(cast(S)).getValue();
 


Index: clang/test/Analysis/eval-predefined-exprs.cpp
===
--- /dev/null
+++ clang/test/Analysis/eval-predefined-exprs.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_analyze_cc1 --std=c++17 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_dump(const char *);
+
+template 
+void func(U param) {
+  clang_analyzer_dump(__func__);
+  clang_analyzer_dump(__FUNCTION__);
+  clang_analyzer_dump(__PRETTY_FUNCTION__);
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"void func(U) [T = Class, Value = 42, U = char]",0 S64b,char}}}
+}
+
+void foo() {
+  clang_analyzer_dump(__func__);
+  clang_analyzer_dump(__FUNCTION__);
+  clang_analyzer_dump(__PRETTY_FUNCTION__);
+  // expected-warning@-3 {{&Element{"foo",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"foo",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"void foo()",0 S64b,char}}}
+
+  func('b'); // instantiate template
+}
Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -306,6 +306,11 @@
 return makeLoc(getRegionManager().getStringRegion(SL));
   }
 
+  case Stmt::PredefinedExprClass: {
+const auto *PE = cast(E);
+return makeLoc(getRegionManager().getStringRegion(PE->getFunctionName()));
+  }
+
   // Fast-path some expressions to avoid the overhead of going through the AST's
   // constant evaluator
   case Stmt::CharacterLiteralClass: {
Index: clang/lib/StaticAnalyzer/Core/Environment.cpp
===
--- clang/lib/StaticAnalyzer/Core/Environment.cpp
+++ clang/lib/StaticAnalyzer/Core/Environment.cpp
@@ -109,6 +109,7 @@
   case Stmt::StringLiteralClass:
   case Stmt::TypeTraitExprClass:
   case Stmt::SizeOfPackExprClass:
+  case Stmt::PredefinedExprClass:
 // Known 

[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

2020-09-02 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

In D72705#2241542 , @balazske wrote:

> The summary of this last discussion is that it is not acceptable to have only 
> the simple check for the explicit comparison with a fixed constant. At least 
> not for return types where the "implicit" check (a check that is always true 
> or false for the error return value) is possible, for example the `char*` 
> case.

Indeed, though I think it is okay to keep them in the checker while this is 
being developed, because it still tests the infrastructure well enough. We 
should just remember to fine tune the set of functions we're checking before 
moving out of alpha.

> For functions that return a sort of "handle" (mainly a pointer to a struct 
> that is not normally used with pointer arithmetic) the checker can still be 
> useful.

Agreed. This is true for //some// out-parameters as well.

> Another solution for the problem is if the system calls are modeled in a way 
> that there is always a state split between error end non-error (we will have 
> a path where it is known that the specific variable can be only (for example) 
> `NULL` and this can be detected by other checkers).

@NoQ had a great comment about the dangers of this: D79432#inline-757474 
. If we still end up wanting to 
do this, then I agree with @martong, they should be placed in 
`StdLibraryFunctionsChecker`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72705

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


[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

2020-09-02 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision.
Szelethus added a comment.
This revision is now accepted and ready to land.

LGTM! Nice!




Comment at: clang/test/Analysis/eval-predefined-exprs.cpp:1
+// RUN: %clang_analyze_cc1 --std=c++17 
-analyzer-checker=core,debug.ExprInspection -verify %s
+

Isn't it `-std=c++17`?



Comment at: clang/test/Analysis/eval-predefined-exprs.cpp:7-21
+  clang_analyzer_dump(__func__);
+  clang_analyzer_dump(__FUNCTION__);
+  clang_analyzer_dump(__PRETTY_FUNCTION__);
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"void func(U) [T = Class, Value = 42, U = 
char]",0 S64b,char}}}
+}

Why not put the expected warning right below the function call?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87004

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


[PATCH] D86621: [clang][Sparc] Default to -mcpu=v9 for SparcV8 on Solaris

2020-09-02 Thread Rainer Orth via Phabricator via cfe-commits
ro added inline comments.



Comment at: clang/lib/Basic/Targets/Sparc.cpp:224
+Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_8");
+  }
 }

efriedma wrote:
> ro wrote:
> > efriedma wrote:
> > > ro wrote:
> > > > efriedma wrote:
> > > > > This probably should be refactored so the target-independent code 
> > > > > generates it based on MaxAtomicInlineWidth, instead of duplicating it 
> > > > > for each target.  But I guess you don't need to do that here.
> > > > > 
> > > > > From the other code, the `getCPUGeneration(CPU) == CG_V9` check 
> > > > > should only guard the definition of 
> > > > > __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8?
> > > > > This probably should be refactored so the target-independent code 
> > > > > generates it based on MaxAtomicInlineWidth, instead of duplicating it 
> > > > > for each target.  But I guess you don't need to do that here.
> > > > 
> > > > Good: one issue at a time ;-)
> > > > 
> > > > > From the other code, the `getCPUGeneration(CPU) == CG_V9` check 
> > > > > should only guard the definition of 
> > > > > __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8?
> > > > 
> > > > I don't think so: at least `gcc` defines none of the four with `-m32 
> > > > -mcpu=v8` and all with `-m32 -mcpu=v9`.
> > > This code, the code that sets MaxAtomicInlineWidth, and the code 
> > > inSPARCISelLowering.cpp that calls setMaxAtomicSizeInBitsSupported() all 
> > > need to agree about the supported atomic operations.
> > > 
> > > I guess the current setting of MaxAtomicInlineWidth is wrong?
> > I'd say so, yes: gcc -m32 inlines ops on `_Atomic long long` while 
> > `clang-11 -m32 -mcpu=v9` doesn't.
> Oh, hmm, it looks like the backend's support for 32-bit v9 is really limited; 
> we basically generate v8 code, with a couple limited exceptions.  Probably 
> okay to make clang assume 64-bit atomics are actually supported, even if we 
> don't inline the implementation at the moment; they should still behave 
> correctly.
> 
> I was more wondering about what we do for v8: we set MaxAtomicInlineWidth to 
> 32, but I don't think it supports atomic cmpxchg at all.
I've now found [[http://temlib.org/pub/SparcStation/Standards/V8plus.pdf | The 
V8+ Technical Specification ]].  I've not checked how far LLVM makes use of 
that, though (gcc seems to be pretty extensive, and it certainly makes use of 
`casx` for v8plus).

I'm not really clear on the semantics of `MaxAtomicInlineWidth`: 
`TargetInfo.h`'s description of `getMaxAtomicInlineWidth` only states
```
  /// Return the maximum width lock-free atomic operation which can be
  /// inlined given the supported features of the given target.
```
which would be satisfied given that **some** 32-bit atomic ops are inlined for 
v8.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86621

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


[PATCH] D86930: [clang-format] Handle typename macros inside cast expressions

2020-09-02 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson added a comment.

In D86930#2251144 , @dougpuob wrote:

> I am a beginner to compiler, interesting in how to write Unit Test case for 
> change so I ran it, but found difference with my expection.
>
> You mentioned 
> Before: x = (STACK_OF(uint64_t)) & a;
> After: x = (STACK_OF(uint64_t))&a;
>
> Your input test data is identical with the expected data. Does this exactly 
> you need? Attach with a screenshot for your reference.
> F12833965: 2020-09-02_081105.png 

It previously would have reformatted that input to add the spaces, so using the 
expected value as an input works fine. This is what almost all tests in this 
file do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86930

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


[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

2020-09-02 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko requested changes to this revision.
vsavchenko added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:311
+const auto *PE = cast(E);
+return makeLoc(getRegionManager().getStringRegion(PE->getFunctionName()));
+  }

I'm not super familiar with predefined expressions, but it looks like it this 
commit doesn't cover all kinds of those.  And for some this can return 
`nullptr`.

Can you, please, add tests for other kinds as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87004

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


[PATCH] D86960: [clang-format] Parse __underlying_type(T) as a type

2020-09-02 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision.
MyDeveloperDay 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/D86960/new/

https://reviews.llvm.org/D86960

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


[PATCH] D71760: [POC][SVE] Allow code generation for fixed length vectorised loops [Patch 1/2].

2020-09-02 Thread Paul Walker via Phabricator via cfe-commits
paulwalker-arm abandoned this revision.
paulwalker-arm added a comment.

The intention of this patch is now complete.  All work is available in master 
with the exception of the hook into -msve-vector-bits which is not necessarily 
the direction we'll use once function attributes are available.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71760

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


[PATCH] D71767: [POC][SVE] Allow code generation for fixed length vectorised loops [Patch 2/2].

2020-09-02 Thread Paul Walker via Phabricator via cfe-commits
paulwalker-arm abandoned this revision.
paulwalker-arm added a comment.
Herald added a subscriber: ecnelises.

With the exception of VSELECT lowering, which is being worked under D85364 
, everything else is available in master.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71767

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


[PATCH] D85128: [Prototype][SVE] Support arm_sve_vector_bits attribute

2020-09-02 Thread Cullen Rhodes via Phabricator via cfe-commits
c-rhodes abandoned this revision.
c-rhodes added a comment.

Closing this now the prototype has been split into separate patches that have 
landed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85128

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


[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

2020-09-02 Thread Balázs Benics via Phabricator via cfe-commits
steakhal updated this revision to Diff 289395.
steakhal added subscribers: riccibruno, rjmccall.
steakhal added a comment.

- Added tests for Microsoft extensions.
- Added an `assert` requiring the `PredefinedExpression` to have a function 
name.



I don't know how could a `PredefinedExpression` lack the function name, 
probably @riccibruno or @rjmccall can help with this - according to D53605 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87004

Files:
  clang/lib/StaticAnalyzer/Core/Environment.cpp
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
  clang/test/Analysis/eval-predefined-exprs.cpp

Index: clang/test/Analysis/eval-predefined-exprs.cpp
===
--- /dev/null
+++ clang/test/Analysis/eval-predefined-exprs.cpp
@@ -0,0 +1,96 @@
+// RUN: %clang_analyze_cc1 -std=c++17 -analyzer-checker=core,debug.ExprInspection -verify %s
+//
+// RUN: %clang_analyze_cc1 -std=c++17 -analyzer-checker=core,debug.ExprInspection -verify \
+// RUN:   -triple i386-pc-win32 -fms-compatibility -fms-extensions -DANALYZER_MS %s
+
+template 
+void clang_analyzer_dump(const T *);
+
+template 
+void func(U param) {
+  clang_analyzer_dump(__func__);
+  clang_analyzer_dump(__FUNCTION__);
+  clang_analyzer_dump(__PRETTY_FUNCTION__);
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"void func(U) [T = Class, Value = 42, U = char]",0 S64b,char}}}
+
+#ifdef ANALYZER_MS
+  clang_analyzer_dump(__FUNCDNAME__);
+  clang_analyzer_dump(L__FUNCTION__);
+  clang_analyzer_dump(__FUNCSIG__);
+  clang_analyzer_dump(L__FUNCSIG__);
+  // expected-warning@-4 {{&Element{"??$func@UClass@?1??foo@@YAXXZ@$0CK@D@@YAXD@Z",0 S64b,char}}}
+  // expected-warning@-4 {{&Element{L"func",0 S64b,wchar_t}}}
+  // expected-warning@-4 {{&Element{"void __cdecl func(U) [T = Class, Value = 42, U = char]",0 S64b,char}}}
+  // expected-warning@-4 {{&Element{L"void __cdecl func(U) [T = Class, Value = 42, U = char]",0 S64b,wchar_t}}}
+#endif
+}
+
+void foo() {
+  clang_analyzer_dump(__func__);
+  clang_analyzer_dump(__FUNCTION__);
+  clang_analyzer_dump(__PRETTY_FUNCTION__);
+  // expected-warning@-3 {{&Element{"foo",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"foo",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"void foo()",0 S64b,char}}}
+
+#ifdef ANALYZER_MS
+  clang_analyzer_dump(__FUNCDNAME__);
+  clang_analyzer_dump(L__FUNCTION__);
+  clang_analyzer_dump(__FUNCSIG__);
+  clang_analyzer_dump(L__FUNCSIG__);
+  // expected-warning@-4 {{&Element{"?foo@@YAXXZ",0 S64b,char}}}
+  // expected-warning@-4 {{&Element{L"foo",0 S64b,wchar_t}}}
+  // expected-warning@-4 {{&Element{"void __cdecl foo(void)",0 S64b,char}}}
+  // expected-warning@-4 {{&Element{L"void __cdecl foo(void)",0 S64b,wchar_t}}}
+#endif
+
+  func('b'); // instantiate template
+}
+
+void test_builtin_unique_stable_name(int a) {
+  clang_analyzer_dump(__builtin_unique_stable_name(a));
+  // expected-warning@-1 {{&Element{"_ZTSi",0 S64b,char}}}
+}
+
+struct A {
+  A() {
+clang_analyzer_dump(__func__);
+clang_analyzer_dump(__FUNCTION__);
+clang_analyzer_dump(__PRETTY_FUNCTION__);
+// expected-warning@-3 {{&Element{"A",0 S64b,char}}}
+// expected-warning@-3 {{&Element{"A",0 S64b,char}}}
+// expected-warning@-3 {{&Element{"A::A()",0 S64b,char}}}
+
+#ifdef ANALYZER_MS
+clang_analyzer_dump(__FUNCDNAME__);
+clang_analyzer_dump(L__FUNCTION__);
+clang_analyzer_dump(__FUNCSIG__);
+clang_analyzer_dump(L__FUNCSIG__);
+// expected-warning@-4 {{&Element{"??0A@@QAE@XZ",0 S64b,char}}}
+// expected-warning@-4 {{&Element{L"A",0 S64b,wchar_t}}}
+// expected-warning@-4 {{&Element{"__thiscall A::A(void)",0 S64b,char}}}
+// expected-warning@-4 {{&Element{L"__thiscall A::A(void)",0 S64b,wchar_t}}}
+#endif
+  }
+  ~A() {
+clang_analyzer_dump(__func__);
+clang_analyzer_dump(__FUNCTION__);
+clang_analyzer_dump(__PRETTY_FUNCTION__);
+// expected-warning@-3 {{&Element{"~A",0 S64b,char}}}
+// expected-warning@-3 {{&Element{"~A",0 S64b,char}}}
+// expected-warning@-3 {{&Element{"A::~A()",0 S64b,char}}}
+
+#ifdef ANALYZER_MS
+clang_analyzer_dump(__FUNCDNAME__);
+clang_analyzer_dump(L__FUNCTION__);
+clang_analyzer_dump(__FUNCSIG__);
+clang_analyzer_dump(L__FUNCSIG__);
+// expected-warning@-4 {{&Element{"??1A@@QAE@XZ",0 S64b,char}}}
+// expected-warning@-4 {{&Element{L"~A",0 S64b,wchar_t}}}
+// expected-warning@-4 {{&Element{"__thiscall A::~A(void)",0 S64b,char}}}
+// expected-warning@-4 {{&Element{L"__thiscall A::~A(void)",0 S64b,wchar_t}}}
+#endif
+  }
+};
Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/li

[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

2020-09-02 Thread Balázs Benics via Phabricator via cfe-commits
steakhal marked 2 inline comments as done.
steakhal added inline comments.



Comment at: clang/test/Analysis/eval-predefined-exprs.cpp:1
+// RUN: %clang_analyze_cc1 --std=c++17 
-analyzer-checker=core,debug.ExprInspection -verify %s
+

Szelethus wrote:
> Isn't it `-std=c++17`?
Thanks, typo.



Comment at: clang/test/Analysis/eval-predefined-exprs.cpp:7-21
+  clang_analyzer_dump(__func__);
+  clang_analyzer_dump(__FUNCTION__);
+  clang_analyzer_dump(__PRETTY_FUNCTION__);
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"void func(U) [T = Class, Value = 42, U = 
char]",0 S64b,char}}}
+}

Szelethus wrote:
> Why not put the expected warning right below the function call?
That would seriously violate the column limit.
This way it is easier to see and validate the pattern IMO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87004

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


[PATCH] D87006: [clang-format][NFC] Store FormatToken::Type as an enum instead of bitfield

2020-09-02 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson created this revision.
arichardson added reviewers: MyDeveloperDay, JakeMerdichAMD, sammccall, 
curdeius.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
arichardson requested review of this revision.

This improves the debugging experience since LLDB will print the enumerator
name instead of a decimal number. This changes TokenType to have uint8_t
as the underlying type and moves it after the remaining bitfields to avoid
increasing the size of FormatToken.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87006

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineParser.cpp


Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2753,7 +2753,7 @@
 E = Line.Tokens.end();
I != E; ++I) {
 llvm::dbgs() << I->Tok->Tok.getName() << "["
- << "T=" << I->Tok->getType()
+ << "T=" << (unsigned)I->Tok->getType()
  << ", OC=" << I->Tok->OriginalColumn << "] ";
   }
   for (std::list::const_iterator I = Line.Tokens.begin(),
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -118,7 +118,7 @@
 
 /// Determines the semantic type of a syntactic token, e.g. whether "<" is a
 /// template opener or binary operator.
-enum TokenType {
+enum TokenType : uint8_t {
 #define TYPE(X) TT_##X,
   LIST_TOKEN_TYPES
 #undef TYPE
@@ -149,8 +149,8 @@
 ClosesTemplateDeclaration(false), StartsBinaryExpression(false),
 EndsBinaryExpression(false), PartOfMultiVariableDeclStmt(false),
 ContinuesLineCommentSection(false), Finalized(false),
-BlockKind(BK_Unknown), Type(TT_Unknown), Decision(FD_Unformatted),
-PackingKind(PPK_Inconclusive) {}
+BlockKind(BK_Unknown), Decision(FD_Unformatted),
+PackingKind(PPK_Inconclusive), Type(TT_Unknown) {}
 
   /// The \c Token.
   Token Tok;
@@ -233,18 +233,6 @@
 assert(getBlockKind() == BBK && "BraceBlockKind overflow!");
   }
 
-private:
-  unsigned Type : 8;
-
-public:
-  /// Returns the token's type, e.g. whether "<" is a template opener or
-  /// binary operator.
-  TokenType getType() const { return static_cast(Type); }
-  void setType(TokenType T) {
-Type = T;
-assert(getType() == T && "TokenType overflow!");
-  }
-
 private:
   /// Stores the formatting decision for the token once it was made.
   unsigned Decision : 2;
@@ -271,6 +259,18 @@
 assert(getPackingKind() == K && "ParameterPackingKind overflow!");
   }
 
+private:
+  TokenType Type;
+
+public:
+  /// Returns the token's type, e.g. whether "<" is a template opener or
+  /// binary operator.
+  TokenType getType() const { return static_cast(Type); }
+  void setType(TokenType T) {
+Type = T;
+assert(getType() == T && "TokenType overflow!");
+  }
+
   /// The number of newlines immediately before the \c Token.
   ///
   /// This can be used to determine what the user wrote in the original code


Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2753,7 +2753,7 @@
 E = Line.Tokens.end();
I != E; ++I) {
 llvm::dbgs() << I->Tok->Tok.getName() << "["
- << "T=" << I->Tok->getType()
+ << "T=" << (unsigned)I->Tok->getType()
  << ", OC=" << I->Tok->OriginalColumn << "] ";
   }
   for (std::list::const_iterator I = Line.Tokens.begin(),
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -118,7 +118,7 @@
 
 /// Determines the semantic type of a syntactic token, e.g. whether "<" is a
 /// template opener or binary operator.
-enum TokenType {
+enum TokenType : uint8_t {
 #define TYPE(X) TT_##X,
   LIST_TOKEN_TYPES
 #undef TYPE
@@ -149,8 +149,8 @@
 ClosesTemplateDeclaration(false), StartsBinaryExpression(false),
 EndsBinaryExpression(false), PartOfMultiVariableDeclStmt(false),
 ContinuesLineCommentSection(false), Finalized(false),
-BlockKind(BK_Unknown), Type(TT_Unknown), Decision(FD_Unformatted),
-PackingKind(PPK_Inconclusive) {}
+BlockKind(BK_Unknown), Decision(FD_Unformatted),
+PackingKind(PPK_Inconclusive), Type(TT_Unknown) {}
 
   /// The \c Token.
   Token Tok;
@@ -233,18 +233,6 @@
 assert(getBlockKind() == BBK && "BraceBlockKind overflow!");
   }
 
-private:
-  unsigned Type : 8;
-
-public:
-  /// Returns the token's type, e.g. whether "<" is a template opener or
-  

[PATCH] D87007: [clang-format] Correctly parse function declarations with TypenameMacros

2020-09-02 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson created this revision.
arichardson added reviewers: MyDeveloperDay, JakeMerdichAMD, sammccall, 
curdeius.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
arichardson requested review of this revision.

When using the always break after return type setting:
Before:
SomeType funcdecl(LIST(uint64_t));
After:
SomeType
funcdecl(LIST(uint64_t));"


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87007

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
@@ -6681,9 +6681,11 @@
Style);
 
   // All declarations and definitions should have the return type moved to its
-  // own
-  // line.
+  // own line.
   Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_All;
+  Style.TypenameMacros = {"LIST"};
+  verifyFormat("SomeType\n"
+   "funcdecl(LIST(uint64_t));", Style);
   verifyFormat("class E {\n"
"  int\n"
"  f() {\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2400,6 +2400,8 @@
 return true;
   for (const FormatToken *Tok = Next->Next; Tok && Tok != Next->MatchingParen;
Tok = Tok->Next) {
+if (Tok->is(TT_TypeDeclarationParen))
+  return true;
 if (Tok->isOneOf(tok::l_paren, TT_TemplateOpener) && Tok->MatchingParen) {
   Tok = Tok->MatchingParen;
   continue;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6681,9 +6681,11 @@
Style);
 
   // All declarations and definitions should have the return type moved to its
-  // own
-  // line.
+  // own line.
   Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_All;
+  Style.TypenameMacros = {"LIST"};
+  verifyFormat("SomeType\n"
+   "funcdecl(LIST(uint64_t));", Style);
   verifyFormat("class E {\n"
"  int\n"
"  f() {\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2400,6 +2400,8 @@
 return true;
   for (const FormatToken *Tok = Next->Next; Tok && Tok != Next->MatchingParen;
Tok = Tok->Next) {
+if (Tok->is(TT_TypeDeclarationParen))
+  return true;
 if (Tok->isOneOf(tok::l_paren, TT_TemplateOpener) && Tok->MatchingParen) {
   Tok = Tok->MatchingParen;
   continue;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D62574: Add support for target-configurable address spaces.

2020-09-02 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:10963
+bool
+ASTContext::isExplicitAddrSpaceConversionLegal(LangAS From, LangAS To) const {
+  // If From and To overlap, the cast is legal.

ebevhan wrote:
> Anastasia wrote:
> > Btw I assume that explicit cast can't reject what is not rejected by 
> > implicit cast?
> > 
> > I am not sure if we need to enforce or document this somehow considering 
> > that we provide full configurability now?
> It shouldn't do that, no. I don't think there's any way to guarantee this, 
> though.
> 
> I could add something to the target methods about it.
Wait, no. This is already guaranteed, because the method here in ASTContext 
will check for overlap first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62574

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


[PATCH] D87006: [clang-format][NFC] Store FormatToken::Type as an enum instead of bitfield

2020-09-02 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson updated this revision to Diff 289403.
arichardson added a comment.

- remove unncessary cast+check


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87006

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineParser.cpp


Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2753,7 +2753,7 @@
 E = Line.Tokens.end();
I != E; ++I) {
 llvm::dbgs() << I->Tok->Tok.getName() << "["
- << "T=" << I->Tok->getType()
+ << "T=" << (unsigned)I->Tok->getType()
  << ", OC=" << I->Tok->OriginalColumn << "] ";
   }
   for (std::list::const_iterator I = Line.Tokens.begin(),
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -118,7 +118,7 @@
 
 /// Determines the semantic type of a syntactic token, e.g. whether "<" is a
 /// template opener or binary operator.
-enum TokenType {
+enum TokenType : uint8_t {
 #define TYPE(X) TT_##X,
   LIST_TOKEN_TYPES
 #undef TYPE
@@ -149,8 +149,8 @@
 ClosesTemplateDeclaration(false), StartsBinaryExpression(false),
 EndsBinaryExpression(false), PartOfMultiVariableDeclStmt(false),
 ContinuesLineCommentSection(false), Finalized(false),
-BlockKind(BK_Unknown), Type(TT_Unknown), Decision(FD_Unformatted),
-PackingKind(PPK_Inconclusive) {}
+BlockKind(BK_Unknown), Decision(FD_Unformatted),
+PackingKind(PPK_Inconclusive), Type(TT_Unknown) {}
 
   /// The \c Token.
   Token Tok;
@@ -233,18 +233,6 @@
 assert(getBlockKind() == BBK && "BraceBlockKind overflow!");
   }
 
-private:
-  unsigned Type : 8;
-
-public:
-  /// Returns the token's type, e.g. whether "<" is a template opener or
-  /// binary operator.
-  TokenType getType() const { return static_cast(Type); }
-  void setType(TokenType T) {
-Type = T;
-assert(getType() == T && "TokenType overflow!");
-  }
-
 private:
   /// Stores the formatting decision for the token once it was made.
   unsigned Decision : 2;
@@ -271,6 +259,15 @@
 assert(getPackingKind() == K && "ParameterPackingKind overflow!");
   }
 
+private:
+  TokenType Type;
+
+public:
+  /// Returns the token's type, e.g. whether "<" is a template opener or
+  /// binary operator.
+  TokenType getType() const { return Type; }
+  void setType(TokenType T) { Type = T; }
+
   /// The number of newlines immediately before the \c Token.
   ///
   /// This can be used to determine what the user wrote in the original code


Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2753,7 +2753,7 @@
 E = Line.Tokens.end();
I != E; ++I) {
 llvm::dbgs() << I->Tok->Tok.getName() << "["
- << "T=" << I->Tok->getType()
+ << "T=" << (unsigned)I->Tok->getType()
  << ", OC=" << I->Tok->OriginalColumn << "] ";
   }
   for (std::list::const_iterator I = Line.Tokens.begin(),
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -118,7 +118,7 @@
 
 /// Determines the semantic type of a syntactic token, e.g. whether "<" is a
 /// template opener or binary operator.
-enum TokenType {
+enum TokenType : uint8_t {
 #define TYPE(X) TT_##X,
   LIST_TOKEN_TYPES
 #undef TYPE
@@ -149,8 +149,8 @@
 ClosesTemplateDeclaration(false), StartsBinaryExpression(false),
 EndsBinaryExpression(false), PartOfMultiVariableDeclStmt(false),
 ContinuesLineCommentSection(false), Finalized(false),
-BlockKind(BK_Unknown), Type(TT_Unknown), Decision(FD_Unformatted),
-PackingKind(PPK_Inconclusive) {}
+BlockKind(BK_Unknown), Decision(FD_Unformatted),
+PackingKind(PPK_Inconclusive), Type(TT_Unknown) {}
 
   /// The \c Token.
   Token Tok;
@@ -233,18 +233,6 @@
 assert(getBlockKind() == BBK && "BraceBlockKind overflow!");
   }
 
-private:
-  unsigned Type : 8;
-
-public:
-  /// Returns the token's type, e.g. whether "<" is a template opener or
-  /// binary operator.
-  TokenType getType() const { return static_cast(Type); }
-  void setType(TokenType T) {
-Type = T;
-assert(getType() == T && "TokenType overflow!");
-  }
-
 private:
   /// Stores the formatting decision for the token once it was made.
   unsigned Decision : 2;
@@ -271,6 +259,15 @@
 assert(getPackingKind() == K && "ParameterPackingKind overflow!");
   }
 
+p

[PATCH] D87007: [clang-format] Correctly parse function declarations with TypenameMacros

2020-09-02 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson updated this revision to Diff 289404.
arichardson added a comment.

- fix formatting of test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87007

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
@@ -6681,9 +6681,12 @@
Style);
 
   // All declarations and definitions should have the return type moved to its
-  // own
-  // line.
+  // own line.
   Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_All;
+  Style.TypenameMacros = {"LIST"};
+  verifyFormat("SomeType\n"
+   "funcdecl(LIST(uint64_t));",
+   Style);
   verifyFormat("class E {\n"
"  int\n"
"  f() {\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2400,6 +2400,8 @@
 return true;
   for (const FormatToken *Tok = Next->Next; Tok && Tok != Next->MatchingParen;
Tok = Tok->Next) {
+if (Tok->is(TT_TypeDeclarationParen))
+  return true;
 if (Tok->isOneOf(tok::l_paren, TT_TemplateOpener) && Tok->MatchingParen) {
   Tok = Tok->MatchingParen;
   continue;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6681,9 +6681,12 @@
Style);
 
   // All declarations and definitions should have the return type moved to its
-  // own
-  // line.
+  // own line.
   Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_All;
+  Style.TypenameMacros = {"LIST"};
+  verifyFormat("SomeType\n"
+   "funcdecl(LIST(uint64_t));",
+   Style);
   verifyFormat("class E {\n"
"  int\n"
"  f() {\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2400,6 +2400,8 @@
 return true;
   for (const FormatToken *Tok = Next->Next; Tok && Tok != Next->MatchingParen;
Tok = Tok->Next) {
+if (Tok->is(TT_TypeDeclarationParen))
+  return true;
 if (Tok->isOneOf(tok::l_paren, TT_TemplateOpener) && Tok->MatchingParen) {
   Tok = Tok->MatchingParen;
   continue;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D86959: [clang-format] Fix formatting of _Atomic() qualifier

2020-09-02 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson updated this revision to Diff 289405.
arichardson added a comment.

- use valid C++ syntax in the test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86959

Files:
  clang/lib/Format/FormatToken.cpp
  clang/lib/Format/FormatToken.h
  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
@@ -168,6 +168,8 @@
   verifyFormat("vector<::Type> v;");
   verifyFormat("::ns::SomeFunction(::ns::SomeOtherFunction())");
   verifyFormat("static constexpr bool Bar = decltype(bar())::value;");
+  verifyFormat("static constexpr bool Bar = typeof(bar())::value;");
+  verifyFormat("static constexpr bool Bar = _Atomic(bar())::value;");
   verifyFormat("bool a = 2 < ::SomeFunction();");
   verifyFormat("ALWAYS_INLINE ::std::string getName();");
   verifyFormat("some::string getName();");
@@ -7904,7 +7906,10 @@
   verifyFormat("auto PointerBinding = [](const char *S) {};");
   verifyFormat("typedef typeof(int(int, int)) *MyFunc;");
   verifyFormat("[](const decltype(*a) &value) {}");
+  verifyFormat("[](const typeof(*a) &value) {}");
+  verifyFormat("[](const _Atomic(a*) &value) {}");
   verifyFormat("decltype(a * b) F();");
+  verifyFormat("typeof(a * b) F();");
   verifyFormat("#define MACRO() [](A *a) { return 1; }");
   verifyFormat("Constructor() : member([](A *a, B *b) {}) {}");
   verifyIndependentOfContext("typedef void (*f)(int *a);");
@@ -7970,6 +7975,8 @@
   verifyFormat("delete *x;", Left);
   verifyFormat("typedef typeof(int(int, int))* MyFuncPtr;", Left);
   verifyFormat("[](const decltype(*a)* ptr) {}", Left);
+  verifyFormat("[](const typeof(*a)* ptr) {}", Left);
+  verifyFormat("[](const _Atomic(a*)* ptr) {}", Left);
   verifyFormat("typedef typeof /*comment*/ (int(int, int))* MyFuncPtr;", Left);
   verifyFormat("auto x(A&&, B&&, C&&) -> D;", Left);
   verifyFormat("auto x = [](A&&, B&&, C&&) -> D {};", Left);
@@ -8066,6 +8073,8 @@
   verifyFormat("foo();");
   verifyFormat("foo();");
   verifyFormat("decltype(*::std::declval()) void F();");
+  verifyFormat("typeof(*::std::declval()) void F();");
+  verifyFormat("_Atomic(*::std::declval()) void F();");
   verifyFormat(
   "template ::value &&\n"
@@ -8089,6 +8098,9 @@
   verifyIndependentOfContext("MACRO(int *i);");
   verifyIndependentOfContext("MACRO(auto *a);");
   verifyIndependentOfContext("MACRO(const A *a);");
+  verifyIndependentOfContext("MACRO(_Atomic(A) *a);");
+  verifyIndependentOfContext("MACRO(decltype(A) *a);");
+  verifyIndependentOfContext("MACRO(typeof(A) *a);");
   verifyIndependentOfContext("MACRO(A *const a);");
   verifyIndependentOfContext("MACRO(A *restrict a);");
   verifyIndependentOfContext("MACRO(A *__restrict__ a);");
@@ -8639,6 +8651,10 @@
"LooongFunctionDefinition() {}");
   verifyFormat("decltype(LngName)\n"
"LooongFunctionDefinition() {}");
+  verifyFormat("typeof(LoongName)\n"
+   "LooongFunctionDefinition() {}");
+  verifyFormat("_Atomic(LongName)\n"
+   "LooongFunctionDefinition() {}");
   verifyFormat("LngReturnType\n"
"LooongFunctionDeclaration(T... t);");
   verifyFormat("LngReturnType\n"
@@ -8988,6 +9004,8 @@
   verifyFormat("int foo(int i) { return fo1{}(i); }");
   verifyFormat("int foo(int i) { return fo1{}(i); }");
   verifyFormat("auto i = decltype(x){};");
+  verifyFormat("auto i = typeof(x){};");
+  verifyFormat("auto i = _Atomic(x){};");
   verifyFormat("std::vector v = {1, 0 /* comment */};");
   verifyFormat("Node n{1, Node{1000}, //\n"
"   2};");
@@ -11580,6 +11598,8 @@
   verifyFormat("auto i = std::make_unique(5);", NoSpace);
   verifyFormat("size_t x = sizeof(x);", NoSpace);
   verifyFormat("auto f(int x) -> decltype(x);", NoSpace);
+  verifyFormat("auto f(int x) -> typeof(x);", NoSpace);
+  verifyFormat("auto f(int x) -> _Atomic(x);", NoSpace);
   verifyFormat("int f(T x) noexcept(x.create());", NoSpace);
   verifyFormat("alignas(128) char a[128];", NoSpace);
   verifyFormat("size_t x = alignof(MyType);", NoSpace);
@@ -11628,6 +11648,8 @@
   verifyFormat("auto i = std::make_unique (5);", Space);
   verifyFormat("size_t x = sizeof (x);", Space);
   verifyFormat("auto f (int x) -> decltype (x);", Space);
+  verifyFormat("auto f (int x) -> typeof (x);", Space);
+  verifyFormat("auto f (int x) -> _Atomic (x);", Space);
   verifyFormat("int f (T x) noexce

[PATCH] D86290: Move all fields of '-cc1' option related classes into def file databases

2020-09-02 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

Looks like this broke tests on windows: http://45.33.8.238/win/23171/step_7.txt

Please take a look, and if it takes a while to fix, please revert for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86290

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


[PATCH] D86960: [clang-format] Parse __underlying_type(T) as a type

2020-09-02 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson updated this revision to Diff 289406.
arichardson added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86960

Files:
  clang/lib/Format/FormatToken.h
  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
@@ -169,6 +169,7 @@
   verifyFormat("::ns::SomeFunction(::ns::SomeOtherFunction())");
   verifyFormat("static constexpr bool Bar = decltype(bar())::value;");
   verifyFormat("static constexpr bool Bar = typeof(bar())::value;");
+  verifyFormat("static constexpr bool Bar = __underlying_type(bar())::value;");
   verifyFormat("static constexpr bool Bar = _Atomic(bar())::value;");
   verifyFormat("bool a = 2 < ::SomeFunction();");
   verifyFormat("ALWAYS_INLINE ::std::string getName();");
@@ -7908,6 +7909,7 @@
   verifyFormat("[](const decltype(*a) &value) {}");
   verifyFormat("[](const typeof(*a) &value) {}");
   verifyFormat("[](const _Atomic(a*) &value) {}");
+  verifyFormat("[](const __underlying_type(a) &value) {}");
   verifyFormat("decltype(a * b) F();");
   verifyFormat("typeof(a * b) F();");
   verifyFormat("#define MACRO() [](A *a) { return 1; }");
@@ -7977,6 +7979,7 @@
   verifyFormat("[](const decltype(*a)* ptr) {}", Left);
   verifyFormat("[](const typeof(*a)* ptr) {}", Left);
   verifyFormat("[](const _Atomic(a*)* ptr) {}", Left);
+  verifyFormat("[](const __underlying_type(a)* ptr) {}", Left);
   verifyFormat("typedef typeof /*comment*/ (int(int, int))* MyFuncPtr;", Left);
   verifyFormat("auto x(A&&, B&&, C&&) -> D;", Left);
   verifyFormat("auto x = [](A&&, B&&, C&&) -> D {};", Left);
@@ -8075,6 +8078,7 @@
   verifyFormat("decltype(*::std::declval()) void F();");
   verifyFormat("typeof(*::std::declval()) void F();");
   verifyFormat("_Atomic(*::std::declval()) void F();");
+  verifyFormat("__underlying_type(*::std::declval()) void F();");
   verifyFormat(
   "template ::value &&\n"
@@ -8101,6 +8105,7 @@
   verifyIndependentOfContext("MACRO(_Atomic(A) *a);");
   verifyIndependentOfContext("MACRO(decltype(A) *a);");
   verifyIndependentOfContext("MACRO(typeof(A) *a);");
+  verifyIndependentOfContext("MACRO(__underlying_type(A) *a);");
   verifyIndependentOfContext("MACRO(A *const a);");
   verifyIndependentOfContext("MACRO(A *restrict a);");
   verifyIndependentOfContext("MACRO(A *__restrict__ a);");
@@ -8655,6 +8660,8 @@
"LooongFunctionDefinition() {}");
   verifyFormat("_Atomic(LongName)\n"
"LooongFunctionDefinition() {}");
+  verifyFormat("__underlying_type(LooongName)\n"
+   "LooongFunctionDefinition() {}");
   verifyFormat("LngReturnType\n"
"LooongFunctionDeclaration(T... t);");
   verifyFormat("LngReturnType\n"
@@ -11600,6 +11607,7 @@
   verifyFormat("auto f(int x) -> decltype(x);", NoSpace);
   verifyFormat("auto f(int x) -> typeof(x);", NoSpace);
   verifyFormat("auto f(int x) -> _Atomic(x);", NoSpace);
+  verifyFormat("auto f(int x) -> __underlying_type(x);", NoSpace);
   verifyFormat("int f(T x) noexcept(x.create());", NoSpace);
   verifyFormat("alignas(128) char a[128];", NoSpace);
   verifyFormat("size_t x = alignof(MyType);", NoSpace);
@@ -11650,6 +11658,7 @@
   verifyFormat("auto f (int x) -> decltype (x);", Space);
   verifyFormat("auto f (int x) -> typeof (x);", Space);
   verifyFormat("auto f (int x) -> _Atomic (x);", Space);
+  verifyFormat("auto f (int x) -> __underlying_type (x);", Space);
   verifyFormat("int f (T x) noexcept (x.create ());", Space);
   verifyFormat("alignas (128) char a[128];", Space);
   verifyFormat("size_t x = alignof (MyType);", Space);
@@ -11704,6 +11713,7 @@
   verifyFormat("auto f (int x) -> decltype (x);", SomeSpace);
   verifyFormat("auto f (int x) -> typeof (x);", SomeSpace);
   verifyFormat("auto f (int x) -> _Atomic (x);", SomeSpace);
+  verifyFormat("auto f (int x) -> __underlying_type (x);", SomeSpace);
   verifyFormat("int f (T x) noexcept (x.create());", SomeSpace);
   verifyFormat("alignas (128) char a[128];", SomeSpace);
   verifyFormat("size_t x = alignof (MyType);", SomeSpace);
@@ -14960,6 +14970,7 @@
"  SomeFunction([](decltype(x), A *a) {});\n"
"  SomeFunction([](typeof(x), A *a) {});\n"
"  SomeFunction([](_Atomic(x), A *a) {});\n"
+   "  SomeFunction([](__underlying_type(x), A *a) {});\n"
"}");
   verifyFormat("aa(\n"
"[](con

[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

2020-09-02 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments.



Comment at: clang/test/Analysis/eval-predefined-exprs.cpp:7-21
+  clang_analyzer_dump(__func__);
+  clang_analyzer_dump(__FUNCTION__);
+  clang_analyzer_dump(__PRETTY_FUNCTION__);
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"void func(U) [T = Class, Value = 42, U = 
char]",0 S64b,char}}}
+}

steakhal wrote:
> Szelethus wrote:
> > Why not put the expected warning right below the function call?
> That would seriously violate the column limit.
> This way it is easier to see and validate the pattern IMO.
This could be like:
```
clang_analyzer_dump(__FUNCDNAME__); // \
// expected-warning@-4 
{{&Element{"??$func@UClass@?1??foo@@YAXXZ@$0CK@D@@YAXD@Z",0 S64b,char}}}
clang_analyzer_dump(L__FUNCTION__); // \
// expected-warning@-4 {{&Element{L"func",0 S64b,wchar_t}}}
...
```
So, this way you can keep the line limit, I think this is what @Szelethus 
refers to.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87004

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


[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

2020-09-02 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments.



Comment at: clang/test/Analysis/eval-predefined-exprs.cpp:7-21
+  clang_analyzer_dump(__func__);
+  clang_analyzer_dump(__FUNCTION__);
+  clang_analyzer_dump(__PRETTY_FUNCTION__);
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"void func(U) [T = Class, Value = 42, U = 
char]",0 S64b,char}}}
+}

martong wrote:
> steakhal wrote:
> > Szelethus wrote:
> > > Why not put the expected warning right below the function call?
> > That would seriously violate the column limit.
> > This way it is easier to see and validate the pattern IMO.
> This could be like:
> ```
> clang_analyzer_dump(__FUNCDNAME__); // \
> // expected-warning@-4 
> {{&Element{"??$func@UClass@?1??foo@@YAXXZ@$0CK@D@@YAXD@Z",0 S64b,char}}}
> clang_analyzer_dump(L__FUNCTION__); // \
> // expected-warning@-4 {{&Element{L"func",0 S64b,wchar_t}}}
> ...
> ```
> So, this way you can keep the line limit, I think this is what @Szelethus 
> refers to.
> This could be like: ...
There is no need for the `@-4` of course with that approach.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87004

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


[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

2020-09-02 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added a comment.

> I don't know how could a `PredefinedExpression` lack the function name, 
> probably @riccibruno or @rjmccall can help with this - according to D53605 
> .

A `PredefinedExpr` whose declaration context is dependent has no name (see 
`Sema::BuildPredefinedExpr`). Example:

  void f0() { __func__; }
  -PredefinedExpr 0x6a76ba8  'const char [3]' lvalue __func__
  `-StringLiteral 0x6a76b88  'const char [3]' lvalue "f0"
  
  template  void f1() { __func__; }
  `-PredefinedExpr 0x6a76e78  '' lvalue __func__
  
  template void f1();
  -PredefinedExpr 0x6a77060  'const char [3]' lvalue __func__
  `-StringLiteral 0x6a77040  'const char [3]' lvalue "f1"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87004

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


[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

2020-09-02 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added inline comments.



Comment at: clang/test/Analysis/eval-predefined-exprs.cpp:7-21
+  clang_analyzer_dump(__func__);
+  clang_analyzer_dump(__FUNCTION__);
+  clang_analyzer_dump(__PRETTY_FUNCTION__);
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"void func(U) [T = Class, Value = 42, U = 
char]",0 S64b,char}}}
+}

martong wrote:
> martong wrote:
> > steakhal wrote:
> > > Szelethus wrote:
> > > > Why not put the expected warning right below the function call?
> > > That would seriously violate the column limit.
> > > This way it is easier to see and validate the pattern IMO.
> > This could be like:
> > ```
> > clang_analyzer_dump(__FUNCDNAME__); // \
> > // expected-warning@-4 
> > {{&Element{"??$func@UClass@?1??foo@@YAXXZ@$0CK@D@@YAXD@Z",0 S64b,char}}}
> > clang_analyzer_dump(L__FUNCTION__); // \
> > // expected-warning@-4 {{&Element{L"func",0 S64b,wchar_t}}}
> > ...
> > ```
> > So, this way you can keep the line limit, I think this is what @Szelethus 
> > refers to.
> > This could be like: ...
> There is no need for the `@-4` of course with that approach.
There is no column limit in `test/`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87004

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


[PATCH] D86207: (De-)serialize BindingDecls before DecompositionDecl

2020-09-02 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added a comment.

Is my comment on the deserialization of `BindingDecl`s in 
https://reviews.llvm.org/D85613?id=284364 related to this change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86207

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


[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

2020-09-02 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added inline comments.



Comment at: clang/test/Analysis/eval-predefined-exprs.cpp:7-21
+  clang_analyzer_dump(__func__);
+  clang_analyzer_dump(__FUNCTION__);
+  clang_analyzer_dump(__PRETTY_FUNCTION__);
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"void func(U) [T = Class, Value = 42, U = 
char]",0 S64b,char}}}
+}

riccibruno wrote:
> martong wrote:
> > martong wrote:
> > > steakhal wrote:
> > > > Szelethus wrote:
> > > > > Why not put the expected warning right below the function call?
> > > > That would seriously violate the column limit.
> > > > This way it is easier to see and validate the pattern IMO.
> > > This could be like:
> > > ```
> > > clang_analyzer_dump(__FUNCDNAME__); // \
> > > // expected-warning@-4 
> > > {{&Element{"??$func@UClass@?1??foo@@YAXXZ@$0CK@D@@YAXD@Z",0 S64b,char}}}
> > > clang_analyzer_dump(L__FUNCTION__); // \
> > > // expected-warning@-4 {{&Element{L"func",0 S64b,wchar_t}}}
> > > ...
> > > ```
> > > So, this way you can keep the line limit, I think this is what @Szelethus 
> > > refers to.
> > > This could be like: ...
> > There is no need for the `@-4` of course with that approach.
> There is no column limit in `test/`.
Do you think it would be more readable that way? I'm still not convinced.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87004

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


[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

2020-09-02 Thread Balázs Benics via Phabricator via cfe-commits
steakhal updated this revision to Diff 289432.
steakhal marked 2 inline comments as done.
steakhal added a comment.

We only analyze instantiated functions, which are not //dependently typed//.
Safe to assume that every encountered `PredefinedExpression` has a defined 
(non-null) function name.

Just to be sure I added a `clang_analyzer_warnIfReached` in a test-case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87004

Files:
  clang/lib/StaticAnalyzer/Core/Environment.cpp
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
  clang/test/Analysis/eval-predefined-exprs.cpp

Index: clang/test/Analysis/eval-predefined-exprs.cpp
===
--- /dev/null
+++ clang/test/Analysis/eval-predefined-exprs.cpp
@@ -0,0 +1,103 @@
+// RUN: %clang_analyze_cc1 -std=c++17 -analyzer-checker=core,debug.ExprInspection -verify %s
+//
+// RUN: %clang_analyze_cc1 -std=c++17 -analyzer-checker=core,debug.ExprInspection -verify \
+// RUN:   -triple i386-pc-win32 -fms-compatibility -fms-extensions -DANALYZER_MS %s
+
+template 
+void clang_analyzer_dump(const T *);
+void clang_analyzer_warnIfReached();
+
+template 
+void func(U param) {
+  clang_analyzer_dump(__func__);
+  clang_analyzer_dump(__FUNCTION__);
+  clang_analyzer_dump(__PRETTY_FUNCTION__);
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"void func(U) [T = Class, Value = 42, U = char]",0 S64b,char}}}
+
+#ifdef ANALYZER_MS
+  clang_analyzer_dump(__FUNCDNAME__);
+  clang_analyzer_dump(L__FUNCTION__);
+  clang_analyzer_dump(__FUNCSIG__);
+  clang_analyzer_dump(L__FUNCSIG__);
+  // expected-warning@-4 {{&Element{"??$func@UClass@?1??foo@@YAXXZ@$0CK@D@@YAXD@Z",0 S64b,char}}}
+  // expected-warning@-4 {{&Element{L"func",0 S64b,wchar_t}}}
+  // expected-warning@-4 {{&Element{"void __cdecl func(U) [T = Class, Value = 42, U = char]",0 S64b,char}}}
+  // expected-warning@-4 {{&Element{L"void __cdecl func(U) [T = Class, Value = 42, U = char]",0 S64b,wchar_t}}}
+#endif
+}
+
+void foo() {
+  clang_analyzer_dump(__func__);
+  clang_analyzer_dump(__FUNCTION__);
+  clang_analyzer_dump(__PRETTY_FUNCTION__);
+  // expected-warning@-3 {{&Element{"foo",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"foo",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"void foo()",0 S64b,char}}}
+
+#ifdef ANALYZER_MS
+  clang_analyzer_dump(__FUNCDNAME__);
+  clang_analyzer_dump(L__FUNCTION__);
+  clang_analyzer_dump(__FUNCSIG__);
+  clang_analyzer_dump(L__FUNCSIG__);
+  // expected-warning@-4 {{&Element{"?foo@@YAXXZ",0 S64b,char}}}
+  // expected-warning@-4 {{&Element{L"foo",0 S64b,wchar_t}}}
+  // expected-warning@-4 {{&Element{"void __cdecl foo(void)",0 S64b,char}}}
+  // expected-warning@-4 {{&Element{L"void __cdecl foo(void)",0 S64b,wchar_t}}}
+#endif
+
+  func('b'); // instantiate template
+}
+
+void test_builtin_unique_stable_name(int a) {
+  clang_analyzer_dump(__builtin_unique_stable_name(a));
+  // expected-warning@-1 {{&Element{"_ZTSi",0 S64b,char}}}
+}
+
+struct A {
+  A() {
+clang_analyzer_dump(__func__);
+clang_analyzer_dump(__FUNCTION__);
+clang_analyzer_dump(__PRETTY_FUNCTION__);
+// expected-warning@-3 {{&Element{"A",0 S64b,char}}}
+// expected-warning@-3 {{&Element{"A",0 S64b,char}}}
+// expected-warning@-3 {{&Element{"A::A()",0 S64b,char}}}
+
+#ifdef ANALYZER_MS
+clang_analyzer_dump(__FUNCDNAME__);
+clang_analyzer_dump(L__FUNCTION__);
+clang_analyzer_dump(__FUNCSIG__);
+clang_analyzer_dump(L__FUNCSIG__);
+// expected-warning@-4 {{&Element{"??0A@@QAE@XZ",0 S64b,char}}}
+// expected-warning@-4 {{&Element{L"A",0 S64b,wchar_t}}}
+// expected-warning@-4 {{&Element{"__thiscall A::A(void)",0 S64b,char}}}
+// expected-warning@-4 {{&Element{L"__thiscall A::A(void)",0 S64b,wchar_t}}}
+#endif
+  }
+  ~A() {
+clang_analyzer_dump(__func__);
+clang_analyzer_dump(__FUNCTION__);
+clang_analyzer_dump(__PRETTY_FUNCTION__);
+// expected-warning@-3 {{&Element{"~A",0 S64b,char}}}
+// expected-warning@-3 {{&Element{"~A",0 S64b,char}}}
+// expected-warning@-3 {{&Element{"A::~A()",0 S64b,char}}}
+
+#ifdef ANALYZER_MS
+clang_analyzer_dump(__FUNCDNAME__);
+clang_analyzer_dump(L__FUNCTION__);
+clang_analyzer_dump(__FUNCSIG__);
+clang_analyzer_dump(L__FUNCSIG__);
+// expected-warning@-4 {{&Element{"??1A@@QAE@XZ",0 S64b,char}}}
+// expected-warning@-4 {{&Element{L"~A",0 S64b,wchar_t}}}
+// expected-warning@-4 {{&Element{"__thiscall A::~A(void)",0 S64b,char}}}
+// expected-warning@-4 {{&Element{L"__thiscall A::~A(void)",0 S64b,wchar_t}}}
+#endif
+  }
+
+  template  int dependent() {
+// We should not analyze dependent functions.
+// Such functions have no function name of predefined expressions such as: '__func__' etc.
+clang_analyzer_warnIfReached();

[PATCH] D84846: [MC] Add support for generating missing GNU build notes

2020-09-02 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added a comment.

Ping


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

https://reviews.llvm.org/D84846

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


[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

2020-09-02 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment.

Awesome!
Can we cover `__builtin_unique_stable_name` as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87004

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


[PATCH] D86936: [clang] Limit the maximum level of fold-expr expansion.

2020-09-02 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5096
+def err_fold_expression_limit_exceeded: Error<
+  "instantiating fold expression with %0 arguements exceeded expression 
nesting "
+  "limit of %1">, DefaultFatal, NoSFINAE;

arguements -> arguments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86936

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


[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

2020-09-02 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments.



Comment at: clang/test/Analysis/eval-predefined-exprs.cpp:101
+// Such functions have no function name of predefined expressions such as: 
'__func__' etc.
+clang_analyzer_warnIfReached();
+  }

Also can you please add `// no-warning` as a matter of conventions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87004

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


[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

2020-09-02 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added inline comments.



Comment at: clang/test/Analysis/eval-predefined-exprs.cpp:7-21
+  clang_analyzer_dump(__func__);
+  clang_analyzer_dump(__FUNCTION__);
+  clang_analyzer_dump(__PRETTY_FUNCTION__);
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"void func(U) [T = Class, Value = 42, U = 
char]",0 S64b,char}}}
+}

steakhal wrote:
> riccibruno wrote:
> > martong wrote:
> > > martong wrote:
> > > > steakhal wrote:
> > > > > Szelethus wrote:
> > > > > > Why not put the expected warning right below the function call?
> > > > > That would seriously violate the column limit.
> > > > > This way it is easier to see and validate the pattern IMO.
> > > > This could be like:
> > > > ```
> > > > clang_analyzer_dump(__FUNCDNAME__); // \
> > > > // expected-warning@-4 
> > > > {{&Element{"??$func@UClass@?1??foo@@YAXXZ@$0CK@D@@YAXD@Z",0 S64b,char}}}
> > > > clang_analyzer_dump(L__FUNCTION__); // \
> > > > // expected-warning@-4 {{&Element{L"func",0 S64b,wchar_t}}}
> > > > ...
> > > > ```
> > > > So, this way you can keep the line limit, I think this is what 
> > > > @Szelethus refers to.
> > > > This could be like: ...
> > > There is no need for the `@-4` of course with that approach.
> > There is no column limit in `test/`.
> Do you think it would be more readable that way? I'm still not convinced.
I have no opinion on whether it is more readable. I was just saying that 
`test/` is an exception to the column limit policy, and indeed that *many* 
tests have lines longer that the limit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87004

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


[PATCH] D80791: [AArch64] Generate .note.gnu.property based on module flags.

2020-09-02 Thread Momchil Velikov via Phabricator via cfe-commits
chill added a comment.

LGTM, as soon as D85649  is accepted (so they 
stay in sync).


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

https://reviews.llvm.org/D80791

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


[PATCH] D85351: [Analyzer] Fix for `ExprEngine::computeObjectUnderConstruction()` for base and delegating consturctor initializers

2020-09-02 Thread Balogh , Ádám via Phabricator via cfe-commits
baloghadamsoftware added a comment.

In D85351#2247095 , @Szelethus wrote:

> I think its a bad experience if you break something while developing. Instead 
> of getting a test failure for "delegating constructor initializers", you'll 
> have to deal with a test that handles a variety of things at once, and are 
> forced to tease it apart to find what just broke. When the introduced assert 
> fires, this wouldn't be an issue, but in any non-crashing case it might be.

This is a crashing case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85351

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


[PATCH] D85351: [Analyzer] Fix for `ExprEngine::computeObjectUnderConstruction()` for base and delegating consturctor initializers

2020-09-02 Thread Balogh , Ádám via Phabricator via cfe-commits
baloghadamsoftware added a comment.

@NoQ could you please take a look on this short fix?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85351

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


[PATCH] D87028: [clang-format] Improve heuristic for detecting function declarations

2020-09-02 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson created this revision.
arichardson added reviewers: MyDeveloperDay, JakeMerdichAMD, sammccall, 
curdeius.
Herald added subscribers: cfe-commits, jfb.
Herald added a project: clang.
arichardson requested review of this revision.

This change also comma-separated identifiers inside parentheses as a
possible function declaration.

When using the always break after return type setting:
Before:
SomeType funcdecl(SomeType);
SomeType funcdecl(SomeType, OtherType);

After:
SomeType
funcdecl(SomeType);
SomeType
funcdecl(SomeType, OtherType);

Depends on D87007  (to apply cleanly)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87028

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
@@ -6683,10 +6683,69 @@
   // All declarations and definitions should have the return type moved to its
   // own line.
   Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_All;
+  Style.NamespaceIndentation = FormatStyle::NI_All;
   Style.TypenameMacros = {"LIST"};
   verifyFormat("SomeType\n"
"funcdecl(LIST(uint64_t));",
Style);
+  verifyFormat("SomeType\n"
+   "funcdecl();\n"
+   "SomeType\n"
+   "funcdecl(SomeType paramname);\n"
+   "SomeType\n"
+   "funcdecl(_Atomic(uint64_t));\n"
+   "SomeType\n"
+   "funcdecl(SomeType param1, OtherType param2);\n"
+   // Also handle parameter lists declaration without names (but
+   // only at the top level, not inside functions
+   "SomeType\n"
+   "funcdecl(SomeType);\n"
+   "SomeType\n"
+   "funcdecl(SomeType, OtherType);\n"
+   // Check that any kind of expression/operator results in parsing
+   // it as a variable declaration and not a function
+   "SomeType vardecl(var + otherVar);\n"
+   "SomeType vardecl(func());\n"
+   "SomeType vardecl(someFunc(arg));\n"
+   "SomeType vardecl(var, var - otherVar);\n"
+   "SomeType x = var * funcdecl();\n"
+   "SomeType x = var * funcdecl(otherVar);\n"
+   "SomeType x = var * funcdecl(var, otherVar);\n"
+   "void\n"
+   "function_scope() {\n"
+   "  SomeType x = var * funcdecl();\n"
+   "  SomeType x = var * funcdecl(otherVar);\n"
+   "  SomeType x = var * funcdecl(var, otherVar);\n"
+   // While clang will parse these as function declarations, at
+   // least for now clang-format assumes they are variables.
+   "  SomeType *funcdecl();\n"
+   "  SomeType *funcdecl(SomeType);\n"
+   "  SomeType *funcdecl(SomeType, OtherType);\n"
+   "}\n"
+   "namespace namspace_scope {\n"
+   "  SomeType\n"
+   "  funcdecl();\n"
+   "  SomeType\n"
+   "  funcdecl(SomeType paramname);\n"
+   "  SomeType\n"
+   "  funcdecl(_Atomic(uint64_t));\n"
+   "  SomeType\n"
+   "  funcdecl(SomeType param1, OtherType param2);\n"
+   // Inside a namespace we should also parse these as a function
+   // declaration and not as a variable.
+   "  SomeType\n"
+   "  funcdecl(SomeType);\n"
+   "  SomeType\n"
+   "  funcdecl(SomeType, OtherType);\n"
+   "  SomeType vardecl(var + otherVar);\n"
+   "  SomeType vardecl(func());\n"
+   "  SomeType vardecl(someFunc(arg));\n"
+   "  SomeType vardecl(var, var - otherVar);\n"
+   "  SomeType x = var * funcdecl();\n"
+   "  SomeType x = var * funcdecl(otherVar);\n"
+   "  SomeType x = var * funcdecl(var, otherVar);\n"
+   "} // namespace namspace_scope\n",
+   Style);
   verifyFormat("class E {\n"
"  int\n"
"  f() {\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2398,12 +2398,17 @@
   if (Next->MatchingParen->Next &&
   Next->MatchingParen->Next->is(TT_PointerOrReference))
 return true;
+  // Treat  cases where the parameter list only contains comma-separated
+  // identifiers as function declarations. For example:
+  // `SomeType funcdecl(OtherType)` or `SomeType funcdecl(Type1, Type2)`
+  bool CouldBeTypeList = true;
   for (const FormatToken *Tok = Next->Next; Tok && Tok != Next->MatchingParen;
Tok = Tok->Next) {
 if (Tok->is(TT_TypeDeclarationParen)

[PATCH] D87029: [AIX] Implement AIX special bitfield related alignment rules

2020-09-02 Thread Xiangling Liao via Phabricator via cfe-commits
Xiangling_L created this revision.
Xiangling_L added reviewers: jasonliu, hubert.reinterpretcast.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Xiangling_L requested review of this revision.

1.[bool, char, short] bitfields have the same alignment as unsigned int
2.Adjust alignment on typedef field decls/honor align attribute
3.Fix alignment for scoped enum class
4.Long long bitfield has 4bytes alignment and StorageUnitSize under 32 bit 
compile mode
5.Emit error for oversized bitfield under 32bit mode


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87029

Files:
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Layout/aix-bitfield-alignment.cpp
  clang/test/Layout/aix-oversized-bitfield.cpp

Index: clang/test/Layout/aix-oversized-bitfield.cpp
===
--- /dev/null
+++ clang/test/Layout/aix-oversized-bitfield.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fsyntax-only -verify %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -fdump-record-layouts \
+// RUN: -fsyntax-only %s | \
+// RUN:   FileCheck --check-prefix=CHECK64 %s
+
+struct A
+{
+long long l : 64; // expected-error{{width of bit-field 'l' (64 bits) exceeds size of its type (32 bits)}}
+};
+
+int a = sizeof(A);
+
+// CHECK64:  *** Dumping AST Record Layout
+// CHECK64-NEXT:  0 | struct A
+// CHECK64-NEXT: 0:0-63 |   long long l
+// CHECK64-NEXT:| [sizeof=8, dsize=8, align=8, preferredalign=8,
+// CHECK64-NEXT:|  nvsize=8, nvalign=8, preferrednvalign=8]
Index: clang/test/Layout/aix-bitfield-alignment.cpp
===
--- /dev/null
+++ clang/test/Layout/aix-bitfield-alignment.cpp
@@ -0,0 +1,94 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
+// RUN: -fsyntax-only  -x c++ %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK32 %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -fdump-record-layouts \
+// RUN: -fsyntax-only -x c++ %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,CHECK64 %s
+
+struct A {
+  bool b : 3;
+  unsigned char c : 2;
+  unsigned short s : 6;
+};
+
+int a = sizeof(A);
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct A
+// CHECK-NEXT:  0:0-2 |   _Bool b
+// CHECK-NEXT:  0:3-4 |   unsigned char c
+// CHECK-NEXT: 0:5-10 |   unsigned short s
+// CHECK-NEXT:| [sizeof=4, dsize=4, align=4, preferredalign=4,
+// CHECK-NEXT:|  nvsize=4, nvalign=4, preferrednvalign=4]
+
+struct B {
+  char c;
+  int : 0;
+};
+
+int b = sizeof(B);
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct B
+// CHECK-NEXT:  0 |   char c
+// CHECK-NEXT:4:- |   int 
+// CHECK-NEXT:| [sizeof=4, dsize=4, align=4, preferredalign=4,
+// CHECK-NEXT:|  nvsize=4, nvalign=4, preferrednvalign=4]
+
+struct C {
+  signed int a1 : 6;
+  signed char a2 : 4;
+  short int a3 : 2;
+  int a4 : 2;
+  signed long a5 : 5;
+  long long int a6 : 6;
+  unsigned long a7 : 8;
+};
+
+int c = sizeof(C);
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct C
+// CHECK-NEXT:  0:0-5 |   int a1
+// CHECK-NEXT:  0:6-9 |   signed char a2
+// CHECK-NEXT:  1:2-3 |   short a3
+// CHECK-NEXT:  1:4-5 |   int a4
+// CHECK-NEXT: 1:6-10 |   long a5
+// CHECK-NEXT:  2:3-8 |   long long a6
+// CHECK32: 4:0-7 |   unsigned long a7
+// CHECK32:   | [sizeof=8, dsize=8, align=4, preferredalign=4,
+// CHECK32:   |  nvsize=8, nvalign=4, preferrednvalign=4]
+// CHECK64: 3:1-8 |   unsigned long a7
+// CHECK64:   | [sizeof=8, dsize=8, align=8, preferredalign=8,
+// CHECK64:   |  nvsize=8, nvalign=8, preferrednvalign=8]
+
+typedef __attribute__((aligned(32))) short mySHORT;
+struct D {
+  char c : 8;
+  mySHORT : 0;
+};
+
+int d = sizeof(D);
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct D
+// CHECK-NEXT:  0:0-7 |   char c
+// CHECK-NEXT:   32:- |   mySHORT
+// CHECK-NEXT:| [sizeof=32, dsize=32, align=32, preferredalign=32,
+// CHECK-NEXT:|  nvsize=32, nvalign=32, preferrednvalign=32]
+
+enum class Bool : bool { False = 0,
+ True = 1 };
+
+struct E {
+  Bool b : 1;
+};
+
+int e = sizeof(E);
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct E
+// CHECK-NEXT:  0:0-0 |   enum Bool b
+// CHECK-NEXT:| [sizeof=4, dsize=4, align=4, preferredalign=4,
+// CHECK-NEXT:|  nvsize=4, nvalign=4, preferrednvalign=4]
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16443,7 +16443,22 @@
 

[PATCH] D87030: Adapt CastExpr::getSubExprAsWritten to ConstantExpr

2020-09-02 Thread Stephan Bergmann via Phabricator via cfe-commits
sberg created this revision.
sberg added a reviewer: rsmith.
Herald added a project: clang.
sberg requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87030

Files:
  clang/lib/AST/Expr.cpp
  clang/unittests/Tooling/CastExprTest.cpp


Index: clang/unittests/Tooling/CastExprTest.cpp
===
--- clang/unittests/Tooling/CastExprTest.cpp
+++ clang/unittests/Tooling/CastExprTest.cpp
@@ -34,4 +34,23 @@
 "S1 f(S2 s) { return static_cast(s); }\n");
 }
 
+// Verify that getSubExprAsWritten looks through a ConstantExpr in a scenario
+// like
+//
+//   CXXFunctionalCastExpr functional cast to struct S 
+//   `-ConstantExpr 'S'
+// |-value: Struct
+// `-CXXConstructExpr 'S' 'void (int)'
+//   `-IntegerLiteral 'int' 0
+TEST(CastExprTest, GetSubExprAsWrittenThroughConstantExpr) {
+  CastExprVisitor Visitor;
+  Visitor.OnExplicitCast = [](ExplicitCastExpr *Expr) {
+auto Sub = Expr->getSubExprAsWritten();
+EXPECT_TRUE(isa(Sub))
+<< "Expected IntegerLiteral, but saw " << Sub->getStmtClassName();
+  };
+  Visitor.runOver("struct S { consteval S(int) {} };\n"
+  "S f() { return S(0); }\n",
+  CastExprVisitor::Lang_CXX2a);
+}
 }
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -1823,8 +1823,8 @@
 // Conversions by constructor and conversion functions have a
 // subexpression describing the call; strip it off.
 if (E->getCastKind() == CK_ConstructorConversion)
-  SubExpr =
-skipImplicitTemporary(cast(SubExpr)->getArg(0));
+  SubExpr = skipImplicitTemporary(
+  cast(SubExpr->IgnoreImplicit())->getArg(0));
 else if (E->getCastKind() == CK_UserDefinedConversion) {
   assert((isa(SubExpr) ||
   isa(SubExpr)) &&


Index: clang/unittests/Tooling/CastExprTest.cpp
===
--- clang/unittests/Tooling/CastExprTest.cpp
+++ clang/unittests/Tooling/CastExprTest.cpp
@@ -34,4 +34,23 @@
 "S1 f(S2 s) { return static_cast(s); }\n");
 }
 
+// Verify that getSubExprAsWritten looks through a ConstantExpr in a scenario
+// like
+//
+//   CXXFunctionalCastExpr functional cast to struct S 
+//   `-ConstantExpr 'S'
+// |-value: Struct
+// `-CXXConstructExpr 'S' 'void (int)'
+//   `-IntegerLiteral 'int' 0
+TEST(CastExprTest, GetSubExprAsWrittenThroughConstantExpr) {
+  CastExprVisitor Visitor;
+  Visitor.OnExplicitCast = [](ExplicitCastExpr *Expr) {
+auto Sub = Expr->getSubExprAsWritten();
+EXPECT_TRUE(isa(Sub))
+<< "Expected IntegerLiteral, but saw " << Sub->getStmtClassName();
+  };
+  Visitor.runOver("struct S { consteval S(int) {} };\n"
+  "S f() { return S(0); }\n",
+  CastExprVisitor::Lang_CXX2a);
+}
 }
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -1823,8 +1823,8 @@
 // Conversions by constructor and conversion functions have a
 // subexpression describing the call; strip it off.
 if (E->getCastKind() == CK_ConstructorConversion)
-  SubExpr =
-skipImplicitTemporary(cast(SubExpr)->getArg(0));
+  SubExpr = skipImplicitTemporary(
+  cast(SubExpr->IgnoreImplicit())->getArg(0));
 else if (E->getCastKind() == CK_UserDefinedConversion) {
   assert((isa(SubExpr) ||
   isa(SubExpr)) &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D81930: [AArch64] Add -mmark-bti-property flag.

2020-09-02 Thread Momchil Velikov via Phabricator via cfe-commits
chill accepted this revision.
chill added a comment.
This revision is now accepted and ready to land.

LGTM. It'd be nice if we could get someone non-Arm to have a look too. though.


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

https://reviews.llvm.org/D81930

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


[PATCH] D87030: Adapt CastExpr::getSubExprAsWritten to ConstantExpr

2020-09-02 Thread Stephan Bergmann via Phabricator via cfe-commits
sberg added a comment.

I hit this with a call to getSubExprAsWriten from the LibreOffice Clang plugin, 
have no idea whether it can be hit from within the compiler itself.  Not sure 
if clang/unittests/Tooling/CastExprTest.cpp is the best place to add a test for 
it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87030

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


[PATCH] D86999: getClangStripDependencyFileAdjuster(): Do not remove -M args when using MSVC cl driver

2020-09-02 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.

Seems reasonable to me. Just some nits.




Comment at: clang/lib/Tooling/ArgumentsAdjusters.cpp:29
+  auto EqualSignIndex = ArgRef.find('=');
+  return StringRef(ArgRef.data() + EqualSignIndex + 1);
+}

Instead of manually indexing into the StringRef, the I think the if-statement 
could be replaced by

```
if (ArgRef.consume_front("--driver-mode="))
  return ArgRef;
```



Comment at: clang/lib/Tooling/ArgumentsAdjusters.cpp:32
+  }
+  return StringRef();
+}

I'd drop the Optional and just let the empty StringRef signal not finding the 
driver mode.



Comment at: clang/lib/Tooling/ArgumentsAdjusters.cpp:108
+auto DriverMode = getDriverMode(Args);
+auto UsingClDriver = (DriverMode.hasValue() && *DriverMode == "cl");
+

If getDriverMode() didn't return Optional, this could be simplified to

```
bool UsingClDriver = (getDriverMode() == "cl");
```



Comment at: clang/lib/Tooling/ArgumentsAdjusters.cpp:115
   // -MF, -MG, -MP, -MT, -MQ, -MD, and -MMD.
-  if (!Arg.startswith("-M") && !Arg.startswith("/showIncludes") &&
-  !Arg.startswith("-showIncludes")) {
+  // Dependency-file options in MSVC do not begin with -M, so we
+  // do not remove those when using the cl driver.

This line and the "All dependency-file options begin with -M" line above seem 
to contradict each other. Perhaps the could be united somehow.



Comment at: clang/lib/Tooling/ArgumentsAdjusters.cpp:117
+  // do not remove those when using the cl driver.
+  bool IsDependencyFileArg;
+  if (Arg.startswith("/showIncludes") || Arg.startswith("-showIncludes"))

Instead of a bool and if below, I'd suggest if-statements and early continues 
instead. Breaking up the old if-statement is nice though, and maybe the comment 
from above about what flags to ignore could be moved down here to those if 
statements. For example:


```
// -M flags blah blah
if (Arg.startswith("-M") && !UsingClDriver)
  continue;
// MSVC flags blah blah
if (Arg.startswith("/showIncludes") || Arg.startswith("-showIncludes"))
  continue;
AdjustedArgs.push_back(Args[i]);
```



Comment at: clang/lib/Tooling/ArgumentsAdjusters.cpp:132
 
   if (Arg == "-MF" || Arg == "-MT" || Arg == "-MQ")
 // These flags take an argument: -MX foo. Skip the next argument also.

Need to handle -MT too I suppose


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86999

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


[PATCH] D87031: [libTooling] Provide overloads of `rewriteDescendants` that operate directly on an AST node.

2020-09-02 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision.
ymandel added a reviewer: gribozavr.
Herald added a project: clang.
ymandel requested review of this revision.

The new overloads apply directly to a node, like the
`clang::ast_matchers::match` functions, Rather than generating an
`EditGenerator` combinator.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87031

Files:
  clang/include/clang/Tooling/Transformer/RewriteRule.h
  clang/lib/Tooling/Transformer/RewriteRule.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -568,6 +568,84 @@
   EXPECT_EQ(ErrorCount, 1);
 }
 
+//
+// We include one test per typed overload. We don't test extensively since that
+// is already covered by the tests above.
+//
+
+TEST_F(TransformerTest, RewriteDescendantsTypedStmt) {
+  // Add an unrelated definition to the header that also has a variable named
+  // "x", to test that the rewrite is limited to the scope we intend.
+  appendToHeader(R"cc(int g(int x) { return x; })cc");
+  std::string Input =
+  "int f(int x) { int y = x; { int z = x * x; } return x; }";
+  std::string Expected =
+  "int f(int x) { int y = 3; { int z = 3 * 3; } return 3; }";
+  auto InlineX =
+  makeRule(declRefExpr(to(varDecl(hasName("x", changeTo(cat("3")));
+  testRule(makeRule(functionDecl(hasName("f"), hasBody(stmt().bind("body"))),
+[&InlineX](const MatchFinder::MatchResult &R) {
+  const auto *Node = R.Nodes.getNodeAs("body");
+  assert(Node != nullptr && "body must be bound");
+  return rewriteDescendants(*Node, InlineX, R);
+}),
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsTypedDecl) {
+  std::string Input =
+  "int f(int x) { int y = x; { int z = x * x; } return x; }";
+  std::string Expected =
+  "int f(int x) { int y = 3; { int z = 3 * 3; } return 3; }";
+  auto InlineX =
+  makeRule(declRefExpr(to(varDecl(hasName("x", changeTo(cat("3")));
+  testRule(makeRule(functionDecl(hasName("f")).bind("fun"),
+[&InlineX](const MatchFinder::MatchResult &R) {
+  const auto *Node = R.Nodes.getNodeAs("fun");
+  assert(Node != nullptr && "fun must be bound");
+  return rewriteDescendants(*Node, InlineX, R);
+}),
+   Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsTypedTypeLoc) {
+  std::string Input = "int f(int *x) { return *x; }";
+  std::string Expected = "int f(char *x) { return *x; }";
+  auto IntToChar =
+  makeRule(typeLoc(loc(qualType(isInteger(), builtinType(.bind("loc"),
+   changeTo(cat("char")));
+  testRule(
+  makeRule(functionDecl(hasName("f"),
+hasParameter(0, varDecl(hasTypeLoc(
+typeLoc().bind("parmType"),
+   [&IntToChar](const MatchFinder::MatchResult &R) {
+ const auto *Node = R.Nodes.getNodeAs("parmType");
+ assert(Node != nullptr && "parmType must be bound");
+ return rewriteDescendants(*Node, IntToChar, R);
+   }),
+  Input, Expected);
+}
+
+TEST_F(TransformerTest, RewriteDescendantsTypedDynTyped) {
+  // Add an unrelated definition to the header that also has a variable named
+  // "x", to test that the rewrite is limited to the scope we intend.
+  appendToHeader(R"cc(int g(int x) { return x; })cc");
+  std::string Input =
+  "int f(int x) { int y = x; { int z = x * x; } return x; }";
+  std::string Expected =
+  "int f(int x) { int y = 3; { int z = 3 * 3; } return 3; }";
+  auto InlineX =
+  makeRule(declRefExpr(to(varDecl(hasName("x", changeTo(cat("3")));
+  testRule(makeRule(functionDecl(hasName("f"), hasBody(stmt().bind("body"))),
+[&InlineX](const MatchFinder::MatchResult &R) {
+  auto It = R.Nodes.getMap().find("body");
+  assert(It != R.Nodes.getMap().end() &&
+ "body must be bound");
+  return rewriteDescendants(It->second, InlineX, R);
+}),
+   Input, Expected);
+}
+
 TEST_F(TransformerTest, InsertBeforeEdit) {
   std::string Input = R"cc(
 int f() {
Index: clang/lib/Tooling/Transformer/RewriteRule.cpp
===
--- clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -242,7 +242,7 @@
 } // namespace
 
 template 
-static llvm::Expected>
+llvm::Expected>
 rewriteDescendantsImpl(const T &Node, RewriteRule Rule,
const MatchResult &Result) {
   ApplyRuleCallback Callback(st

[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

2020-09-02 Thread Balázs Benics via Phabricator via cfe-commits
steakhal updated this revision to Diff 289446.
steakhal added a comment.

- Added `no-warning`.
- Added test-case for `__builtin_unique_stable_name` as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87004

Files:
  clang/lib/StaticAnalyzer/Core/Environment.cpp
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
  clang/test/Analysis/eval-predefined-exprs.cpp

Index: clang/test/Analysis/eval-predefined-exprs.cpp
===
--- /dev/null
+++ clang/test/Analysis/eval-predefined-exprs.cpp
@@ -0,0 +1,109 @@
+// RUN: %clang_analyze_cc1 -std=c++17 -analyzer-checker=core,debug.ExprInspection -verify %s
+//
+// RUN: %clang_analyze_cc1 -std=c++17 -analyzer-checker=core,debug.ExprInspection -verify \
+// RUN:   -triple i386-pc-win32 -fms-compatibility -fms-extensions -DANALYZER_MS %s
+
+template 
+void clang_analyzer_dump(const T *);
+void clang_analyzer_warnIfReached();
+
+void builtin_unique_stable_name_of_lambda() {
+  auto y = [] {};
+  clang_analyzer_dump(__builtin_unique_stable_name(y));
+  // expected-warning@-1 {{&Element{"_ZTSZ36builtin_unique_stable_name_of_lambdavEUlvE11_12",0 S64b,char}}}
+}
+
+template 
+void func(U param) {
+  clang_analyzer_dump(__func__);
+  clang_analyzer_dump(__FUNCTION__);
+  clang_analyzer_dump(__PRETTY_FUNCTION__);
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"func",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"void func(U) [T = Class, Value = 42, U = char]",0 S64b,char}}}
+
+#ifdef ANALYZER_MS
+  clang_analyzer_dump(__FUNCDNAME__);
+  clang_analyzer_dump(L__FUNCTION__);
+  clang_analyzer_dump(__FUNCSIG__);
+  clang_analyzer_dump(L__FUNCSIG__);
+  // expected-warning@-4 {{&Element{"??$func@UClass@?1??foo@@YAXXZ@$0CK@D@@YAXD@Z",0 S64b,char}}}
+  // expected-warning@-4 {{&Element{L"func",0 S64b,wchar_t}}}
+  // expected-warning@-4 {{&Element{"void __cdecl func(U) [T = Class, Value = 42, U = char]",0 S64b,char}}}
+  // expected-warning@-4 {{&Element{L"void __cdecl func(U) [T = Class, Value = 42, U = char]",0 S64b,wchar_t}}}
+#endif
+}
+
+void foo() {
+  clang_analyzer_dump(__func__);
+  clang_analyzer_dump(__FUNCTION__);
+  clang_analyzer_dump(__PRETTY_FUNCTION__);
+  // expected-warning@-3 {{&Element{"foo",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"foo",0 S64b,char}}}
+  // expected-warning@-3 {{&Element{"void foo()",0 S64b,char}}}
+
+#ifdef ANALYZER_MS
+  clang_analyzer_dump(__FUNCDNAME__);
+  clang_analyzer_dump(L__FUNCTION__);
+  clang_analyzer_dump(__FUNCSIG__);
+  clang_analyzer_dump(L__FUNCSIG__);
+  // expected-warning@-4 {{&Element{"?foo@@YAXXZ",0 S64b,char}}}
+  // expected-warning@-4 {{&Element{L"foo",0 S64b,wchar_t}}}
+  // expected-warning@-4 {{&Element{"void __cdecl foo(void)",0 S64b,char}}}
+  // expected-warning@-4 {{&Element{L"void __cdecl foo(void)",0 S64b,wchar_t}}}
+#endif
+
+  func('b'); // instantiate template
+}
+
+void test_builtin_unique_stable_name(int a) {
+  clang_analyzer_dump(__builtin_unique_stable_name(a));
+  // expected-warning@-1 {{&Element{"_ZTSi",0 S64b,char}}}
+}
+
+struct A {
+  A() {
+clang_analyzer_dump(__func__);
+clang_analyzer_dump(__FUNCTION__);
+clang_analyzer_dump(__PRETTY_FUNCTION__);
+// expected-warning@-3 {{&Element{"A",0 S64b,char}}}
+// expected-warning@-3 {{&Element{"A",0 S64b,char}}}
+// expected-warning@-3 {{&Element{"A::A()",0 S64b,char}}}
+
+#ifdef ANALYZER_MS
+clang_analyzer_dump(__FUNCDNAME__);
+clang_analyzer_dump(L__FUNCTION__);
+clang_analyzer_dump(__FUNCSIG__);
+clang_analyzer_dump(L__FUNCSIG__);
+// expected-warning@-4 {{&Element{"??0A@@QAE@XZ",0 S64b,char}}}
+// expected-warning@-4 {{&Element{L"A",0 S64b,wchar_t}}}
+// expected-warning@-4 {{&Element{"__thiscall A::A(void)",0 S64b,char}}}
+// expected-warning@-4 {{&Element{L"__thiscall A::A(void)",0 S64b,wchar_t}}}
+#endif
+  }
+  ~A() {
+clang_analyzer_dump(__func__);
+clang_analyzer_dump(__FUNCTION__);
+clang_analyzer_dump(__PRETTY_FUNCTION__);
+// expected-warning@-3 {{&Element{"~A",0 S64b,char}}}
+// expected-warning@-3 {{&Element{"~A",0 S64b,char}}}
+// expected-warning@-3 {{&Element{"A::~A()",0 S64b,char}}}
+
+#ifdef ANALYZER_MS
+clang_analyzer_dump(__FUNCDNAME__);
+clang_analyzer_dump(L__FUNCTION__);
+clang_analyzer_dump(__FUNCSIG__);
+clang_analyzer_dump(L__FUNCSIG__);
+// expected-warning@-4 {{&Element{"??1A@@QAE@XZ",0 S64b,char}}}
+// expected-warning@-4 {{&Element{L"~A",0 S64b,wchar_t}}}
+// expected-warning@-4 {{&Element{"__thiscall A::~A(void)",0 S64b,char}}}
+// expected-warning@-4 {{&Element{L"__thiscall A::~A(void)",0 S64b,wchar_t}}}
+#endif
+  }
+
+  template  int dependent() {
+// We should not analyze dependent functions.
+// Such functions have no function name of predefined expressions such as: '__func__' etc.
+clang_

[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library

2020-09-02 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment.

Ping! @MaskRay any further comments?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70378

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


[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

2020-09-02 Thread Balázs Benics via Phabricator via cfe-commits
steakhal marked an inline comment as done.
steakhal added a comment.

Thank you all for the comments!




Comment at: clang/test/Analysis/eval-predefined-exprs.cpp:101
+// Such functions have no function name of predefined expressions such as: 
'__func__' etc.
+clang_analyzer_warnIfReached();
+  }

vsavchenko wrote:
> Also can you please add `// no-warning` as a matter of conventions?
Sure, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87004

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


[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

2020-09-02 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko accepted this revision.
vsavchenko added a comment.
This revision is now accepted and ready to land.

Thanks!
Great job!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87004

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


[PATCH] D86743: [analyzer] Ignore VLASizeChecker case that could cause crash

2020-09-02 Thread Balogh , Ádám via Phabricator via cfe-commits
baloghadamsoftware added a comment.

I agree here with @Szelethus. We should investigate first why the assumption 
fails. Then we can decide about the best possible fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86743

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


[PATCH] D86874: [analyzer] Fix ArrayBoundCheckerV2 false positive regarding size_t indexer

2020-09-02 Thread Balogh , Ádám via Phabricator via cfe-commits
baloghadamsoftware added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:226
+  // No unsigned symbolic value can be less then a negative constant.
+  if (const auto SymbolicRoot = RootNonLoc.getAs())
+if (SymbolicRoot->getSymbol()->getType()->isUnsignedIntegerType() &&

martong wrote:
> I really feel that this check would have a better place in the implementation 
> of `eval`. This seems really counter-intuitive to do this stuff at the 
> Checker's level. Is there any reason why we can't do this in `eval`?
> 
> `evalBinOpNN` could return with Unknown, and the state should remain 
> unchanged. Am I missing something?
I agree here. Actually, the //constraint manager// should handle this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86874

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


[PATCH] D86207: (De-)serialize BindingDecls before DecompositionDecl

2020-09-02 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment.

In D86207#2251802 , @riccibruno wrote:

> Is my comment on the deserialization of `BindingDecl`s in 
> https://reviews.llvm.org/D85613?id=284364 related to this change?

Not sure. The `FIXME` comment on the code is correct, and it would be correct 
after this change. But notice that `Decomp` is also not set when constructing a 
`BindingDecl` from the parser: first we build the `BindingDecl`s in 
`Sema::ActOnDecompositionDeclarator` (there `Decomp` remains unset), then build 
the `DecompositionDecl` from that in `Sema::ActOnVariableDeclarator`. The 
constructor of `DecompositionDecl` is then calling `setDecomposedDecl` on all 
bindings, so `Decomp` is set as soon as that's finished. But the `BindingDecl`s 
exist without `Decomp` for a while.

In D85613#2210054 , @riccibruno wrote:

> The expression for the binding (`Binding`) will be deserialized when visiting 
> the `BindingDecl`. This expression when non-null will always (as far as I can 
> tell) contain a reference to the decomposition declaration so the 
> decomposition will be deserialized which will set `Decomp`.

My impression is that `Decomp` is set by 
`ASTDeclReader::VisitDecompositionDecl`, after reading the individual 
`BindingDecl`s, not from deserializing the `Binding` expression. Otherwise the 
`BDs[I]->setDecomposedDecl(DD);` above wouldn't be needed. But maybe I'm 
misunderstanding you.

Maybe you're saying it might be a problem if I read the `BindingDecls` first 
and then they can't reference the `DecompositionDecl`? I'm not sure how I can 
see the deserialized AST, when I use `-ast-dump` with `-include-pch` it doesn't 
show the AST of the included header. But the generated code looks fine for an 
example I tried out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86207

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


[clang] 8ff44e6 - [IRGen] Fix an assert when __attribute__((used)) is used on an ObjC method

2020-09-02 Thread Erik Pilkington via cfe-commits

Author: Erik Pilkington
Date: 2020-09-02T12:19:11-04:00
New Revision: 8ff44e644bb70dfb8decc397a42679df6e6f8ba1

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

LOG: [IRGen] Fix an assert when __attribute__((used)) is used on an ObjC method

This assert doesn't really make sense for functions in general, since they
start life as declarations, and there isn't really any reason to require them
to be defined before attributes are applied to them.

rdar://67895846

Added: 
clang/test/CodeGenObjC/attr-used-on-method.m

Modified: 
clang/lib/CodeGen/CodeGenModule.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 77a5079bd0f1..1f362e2b6b31 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1989,7 +1989,7 @@ void CodeGenModule::SetFunctionAttributes(GlobalDecl GD, 
llvm::Function *F,
 }
 
 void CodeGenModule::addUsedGlobal(llvm::GlobalValue *GV) {
-  assert(!GV->isDeclaration() &&
+  assert(isa(GV) || !GV->isDeclaration() &&
  "Only globals with definition can force usage.");
   LLVMUsed.emplace_back(GV);
 }

diff  --git a/clang/test/CodeGenObjC/attr-used-on-method.m 
b/clang/test/CodeGenObjC/attr-used-on-method.m
new file mode 100644
index ..d8b2a5d29184
--- /dev/null
+++ b/clang/test/CodeGenObjC/attr-used-on-method.m
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.10 %s -S -emit-llvm -o - | 
FileCheck %s
+
+// CHECK: @llvm.used =
+// CHECK-SAME: @"\01-[X m]"
+
+// CHECK: define internal void @"\01-[X m]"(
+
+@interface X @end
+@implementation X
+-(void) m __attribute__((used)) {}
+@end



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


[clang] d46f2c5 - Make -fvisibility-inlines-hidden apply to static local variables in inline functions on Darwin

2020-09-02 Thread Erik Pilkington via cfe-commits

Author: Erik Pilkington
Date: 2020-09-02T12:19:12-04:00
New Revision: d46f2c51e4c849683434bb5a0fb6164957474b8f

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

LOG: Make -fvisibility-inlines-hidden apply to static local variables in inline 
functions on Darwin

This effectively disables r340386 on Darwin, and provides a command line flag
to opt into/out of this behaviour. This change is needed to compile certain
Apple headers correctly.

rdar://47688592

Differential revision: https://reviews.llvm.org/D86881

Added: 
clang/test/CodeGenCXX/visibility-inlines-hidden-static-local-var.cpp

Modified: 
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/Options.td
clang/lib/AST/Decl.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/Darwin.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/Driver/darwin-objc-options.m

Removed: 




diff  --git a/clang/include/clang/Basic/LangOptions.def 
b/clang/include/clang/Basic/LangOptions.def
index d7bba5426c2a..55a784196bb9 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -265,6 +265,9 @@ BENIGN_LANGOPT(DumpRecordLayoutsSimple , 1, 0, "dumping the 
layout of IRgen'd re
 BENIGN_LANGOPT(DumpVTableLayouts , 1, 0, "dumping the layouts of emitted 
vtables")
 LANGOPT(NoConstantCFStrings , 1, 0, "no constant CoreFoundation strings")
 BENIGN_LANGOPT(InlineVisibilityHidden , 1, 0, "hidden visibility for inline 
C++ methods")
+BENIGN_LANGOPT(VisibilityInlinesHiddenStaticLocalVar, 1, 0,
+   "hidden visibility for static local variables in inline C++ "
+   "methods when -fvisibility-inlines hidden is enabled")
 LANGOPT(GlobalAllocationFunctionVisibilityHidden , 1, 0, "hidden visibility 
for global operator new and delete declaration")
 BENIGN_LANGOPT(ParseUnknownAnytype, 1, 0, "__unknown_anytype")
 BENIGN_LANGOPT(DebuggerSupport , 1, 0, "debugger support")

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index ff7b4aa9320c..5a6a196191e7 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1943,6 +1943,17 @@ def fvisibility_EQ : Joined<["-"], "fvisibility=">, 
Group,
 def fvisibility_inlines_hidden : Flag<["-"], "fvisibility-inlines-hidden">, 
Group,
   HelpText<"Give inline C++ member functions hidden visibility by default">,
   Flags<[CC1Option]>;
+def fvisibility_inlines_hidden_static_local_var :
+  Flag<["-"], "fvisibility-inlines-hidden-static-local-var">, Group,
+  HelpText<"When -fvisibility-inlines-hidden is enabled, static variables in "
+   "inline C++ member functions will also be given hidden visibility "
+   "by default">,
+  Flags<[CC1Option]>;
+def fno_visibility_inlines_hidden_static_local_var :
+  Flag<["-"], "fno-visibility-inlines-hidden-static-local-var">, 
Group,
+  HelpText<"Disables -fvisibility-inlines-hidden-static-local-var "
+   "(this is the default on non-darwin targets)">,
+  Flags<[CC1Option]>;
 def fvisibility_ms_compat : Flag<["-"], "fvisibility-ms-compat">, 
Group,
   HelpText<"Give global types 'default' visibility and global functions and "
"variables 'hidden' visibility by default">;

diff  --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 5c0a98815dd7..9815f0648ad7 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -1299,7 +1299,8 @@ LinkageInfo LinkageComputer::getLVForLocalDecl(const 
NamedDecl *D,
 // we should not make static local variables in the function hidden.
 LV = getLVForDecl(FD, computation);
 if (isa(D) && useInlineVisibilityHidden(FD) &&
-!LV.isVisibilityExplicit()) {
+!LV.isVisibilityExplicit() &&
+!Context.getLangOpts().VisibilityInlinesHiddenStaticLocalVar) {
   assert(cast(D)->isStaticLocal());
   // If this was an implicitly hidden inline method, check again for
   // explicit visibility on the parent class, and use that for static 
locals

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 3023c94bf10c..bd5a89c2360c 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5210,6 +5210,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
   }
 
   Args.AddLastArg(CmdArgs, options::OPT_fvisibility_inlines_hidden);
+  Args.AddLastArg(CmdArgs, 
options::OPT_fvisibility_inlines_hidden_static_local_var,
+   
options::OPT_fno_visibility_inlines_hidden_static_local_var);
   Args.AddLastArg(CmdArgs, options::OPT_fvisibility_global_new_delete_hidden);
 
   Args.AddLastArg(CmdArgs, options::OPT_ftlsmodel_EQ);

diff  --git a/clang/lib/Dri

[PATCH] D86881: Make -fvisibility-inlines-hidden apply to static local variables in inline functions on Darwin

2020-09-02 Thread Erik Pilkington via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd46f2c51e4c8: Make -fvisibility-inlines-hidden apply to 
static local variables in inline… (authored by erik.pilkington).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86881

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/AST/Decl.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenCXX/visibility-inlines-hidden-static-local-var.cpp
  clang/test/Driver/darwin-objc-options.m

Index: clang/test/Driver/darwin-objc-options.m
===
--- clang/test/Driver/darwin-objc-options.m
+++ clang/test/Driver/darwin-objc-options.m
@@ -46,3 +46,12 @@
 // RUN: %clang -target x86_64-linux-gnu -### %s 2>&1 | FileCheck --check-prefix=OTHER_COMPATIBILITY %s
 // DARWIN_COMPATIBILITY: -fcompatibility-qualified-id-block-type-checking
 // OTHER_COMPATIBILITY-NOT: -fcompatibility-qualified-id-block-type-checking
+
+// Add -fvisibility-inlines-hidden-static-local-var on Darwin.
+// RUN: %clang -target x86_64-apple-darwin10 -### %s 2>&1 | FileCheck --check-prefix=DARWIN_INLINES_HIDDEN %s
+// RUN: %clang -target x86_64-apple-darwin10 -fno-visibility-inlines-hidden-static-local-var -### %s 2>&1 | FileCheck --check-prefix=DARWIN_INLINES_HIDDEN_EXPLICIT_NO %s
+// RUN: %clang -target x86_64-linux-gnu -### %s 2>&1 | FileCheck --check-prefix=NO_DARWIN_INLINES_HIDDEN %s
+// DARWIN_INLINES_HIDDEN: -fvisibility-inlines-hidden-static-local-var
+// DARWIN_INLINES_HIDDEN_EXPLICIT_NO-NOT: -fvisibility-inlines-hidden-static-local-var
+// DARWIN_INLINES_HIDDEN_EXPLICIT_NO: -fno-visibility-inlines-hidden-static-local-var
+// NO_DARWIN_INLINES_HIDDEN-NOT: -fvisibility-inlines-hidden-static-local-var
Index: clang/test/CodeGenCXX/visibility-inlines-hidden-static-local-var.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/visibility-inlines-hidden-static-local-var.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fvisibility-inlines-hidden -fvisibility-inlines-hidden-static-local-var %s -emit-llvm -o - | FileCheck %s
+
+#define used __attribute__((used))
+
+used inline void f1() {
+  // CHECK: @_ZZ2f1vE6f1_var = linkonce_odr hidden global i32 0
+  static int f1_var = 0;
+}
+
+__attribute__((visibility("default")))
+used inline void f2() {
+  // CHECK: @_ZZ2f2vE6f2_var = linkonce_odr global i32 0
+  static int f2_var = 0;
+}
+
+struct S {
+  used void f3() {
+// CHECK: @_ZZN1S2f3EvE6f3_var = linkonce_odr hidden global i32 0
+static int f3_var = 0;
+  }
+
+  void f6();
+  void f7();
+};
+
+used void f4() {
+  // CHECK: @_ZZ2f4vE6f4_var = internal global i32 0
+  static int f4_var = 0;
+}
+
+__attribute__((visibility("default")))
+used void f5() {
+  // CHECK: @_ZZ2f5vE6f5_var = internal global i32 0
+  static int f5_var = 0;
+}
+
+used void S::f6() {
+  // CHECK: @_ZZN1S2f6EvE6f6_var = internal global i32 0
+  static int f6_var = 0;
+}
+
+used inline void S::f7() {
+  // CHECK: @_ZZN1S2f7EvE6f7_var = linkonce_odr hidden global i32 0
+  static int f7_var = 0;
+}
+
+
+struct __attribute__((visibility("default"))) S2 {
+  used void f8() {
+// CHECK: @_ZZN2S22f8EvE6f8_var = linkonce_odr hidden global i32 0
+static int f8_var = 0;
+  }
+};
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2766,6 +2766,9 @@
   if (Args.hasArg(OPT_fvisibility_inlines_hidden))
 Opts.InlineVisibilityHidden = 1;
 
+  if (Args.hasArg(OPT_fvisibility_inlines_hidden_static_local_var))
+Opts.VisibilityInlinesHiddenStaticLocalVar = 1;
+
   if (Args.hasArg(OPT_fvisibility_global_new_delete_hidden))
 Opts.GlobalAllocationFunctionVisibilityHidden = 1;
 
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -2408,6 +2408,13 @@
   // Enable compatibility mode for NSItemProviderCompletionHandler in
   // Foundation/NSItemProvider.h.
   CC1Args.push_back("-fcompatibility-qualified-id-block-type-checking");
+
+  // Give static local variables in inline functions hidden visibility when
+  // -fvisibility-inlines-hidden is enabled.
+  if (!DriverArgs.getLastArgNoClaim(
+  options::OPT_fvisibility_inlines_hidden_static_local_var,
+  options::OPT_fno_visibility_inlines_hidden_static_local_var))
+CC1Args.push_back("-fvisibility-inlines-hidden-static-local-var");
 }
 
 DerivedArgList *
Index: clang/lib/Driver/ToolChains/Clang.cpp
===

[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.

2020-09-02 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:91
+  auto CheckAndDiagnoseLocalEntity = [&](const VarDecl *VD, unsigned DiagID,
+ const auto &... DiagArgs) -> bool {
+if (VD->isLocalVarDecl() && !DRE->isNonOdrUse()) {

Since you're emitting a specific warning, I think you can just hardcode the 
expected types here instead of using `const auto&...`. Alternatively you could 
guess the integer argument from the (dynamic) type of the `VarDecl` argument.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:147
+//
+const auto *VD = cast_or_null(Binding->getDecomposedDecl());
+if (VD && CheckAndDiagnoseLocalEntity(

Have you seen a case there the `_or_null` is relevant? To me it seems like this 
shouldn't happen, at least not when we get here.



Comment at: clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp:40
   extern void h6(int = i5);
-  // expected-error-re@-1 {{default argument references local variable 
'(unnamed variable of type (anonymous union at {{.*}}:20:3))' of enclosing 
function}}
+  // expected-error-re@-1 {{default argument references local variable 
'(unnamed variable of type (anonymous union at {{.*}}:35:3))' of enclosing 
function}}
+

Better use a relative line number: `[[@LINE-5]]` or something like that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85613

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


[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-02 Thread Rahman Lavaee via Phabricator via cfe-commits
rahmanl updated this revision to Diff 289498.
rahmanl marked 5 inline comments as done.
rahmanl added a comment.

- Address @MaskRay's comments.
- Rebase with upstream.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

Files:
  clang/docs/UsersManual.rst
  clang/test/CodeGen/basic-block-sections.c
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/BasicBlockSections.cpp
  llvm/lib/CodeGen/MIRParser/MIRParser.cpp
  llvm/lib/CodeGen/MachineBasicBlock.cpp
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
  llvm/test/CodeGen/X86/basic-block-sections-labels.ll

Index: llvm/test/CodeGen/X86/basic-block-sections-labels.ll
===
--- llvm/test/CodeGen/X86/basic-block-sections-labels.ll
+++ llvm/test/CodeGen/X86/basic-block-sections-labels.ll
@@ -1,23 +1,24 @@
 ; Check the basic block sections labels option
-; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
 
-define void @_Z3bazb(i1 zeroext) {
-  %2 = alloca i8, align 1
-  %3 = zext i1 %0 to i8
-  store i8 %3, i8* %2, align 1
-  %4 = load i8, i8* %2, align 1
-  %5 = trunc i8 %4 to i1
-  br i1 %5, label %6, label %8
+define void @_Z3bazb(i1 zeroext) personality i32 (...)* @__gxx_personality_v0 {
+  br i1 %0, label %2, label %7
 
-6:; preds = %1
-  %7 = call i32 @_Z3barv()
-  br label %10
+2:
+  %3 = invoke i32 @_Z3barv()
+  to label %7 unwind label %5
+  br label %9
 
-8:; preds = %1
-  %9 = call i32 @_Z3foov()
-  br label %10
+5:
+  landingpad { i8*, i32 }
+  catch i8* null
+  br label %9
 
-10:   ; preds = %8, %6
+7:
+  %8 = call i32 @_Z3foov()
+  br label %9
+
+9:
   ret void
 }
 
@@ -25,9 +26,31 @@
 
 declare i32 @_Z3foov() #1
 
-; LINUX-LABELS: .section
-; LINUX-LABELS: _Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: r.BB._Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: rr.BB._Z3bazb:
+declare i32 @__gxx_personality_v0(...)
+
+; LINUX-LABELS-LABEL:	_Z3bazb:
+; LINUX-LABELS:		.Lfunc_begin0:
+; LINUX-LABELS:		.LBB_END0_[[L1:[0-9]+]]:
+; LINUX-LABELS:		.LBB0_[[L2:[0-9]+]]:
+; LINUX-LABELS:		.LBB_END0_[[L2]]:
+; LINUX-LABELS:		.LBB0_[[L3:[0-9]+]]:
+; LINUX-LABELS:		.LBB_END0_[[L3]]:
+; LINUX-LABELS:		.LBB0_[[L4:[0-9]+]]:
+; LINUX-LABELS:		.LBB_END0_[[L4]]:
+; LINUX-LABELS:		.Lfunc_end0:
+
+; LINUX-LABELS:		.section	.bb_addr_map,"o",@progbits,.text
+; LINUX-LABELS-NEXT:	.quad	.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.byte	4
+; LINUX-LABELS-NEXT:	.uleb128 .Lfunc_begin0-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB_END0_[[L1]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.byte	0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB0_[[L2]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB_END0_[[L2]]-.LBB0_[[L2]]
+; LINUX-LABELS-NEXT:	.byte	0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB0_[[L3]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB_END0_[[L3]]-.LBB0_[[L3]]
+; LINUX-LABELS-NEXT:	.byte	1
+; LINUX-LABELS-NEXT:	.uleb128 .LBB0_[[L4]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB_END0_[[L4]]-.LBB0_[[L4]]
+; LINUX-LABELS-NEXT:	.byte	5
Index: llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
@@ -0,0 +1,35 @@
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -function-sections -basic-block-sections=labels | FileCheck %s
+
+$_Z4fooTIiET_v = comdat any
+
+define dso_local i32 @_Z3barv() {
+  ret i32 0
+}
+;; Check we add SHF_LINK_ORDER for .bb_addr_map and link it with the corresponding .text sections.
+; CHECK:	.section .text._Z3barv,"ax",@progbits
+; CHECK-LABEL:	_Z3barv:
+; CHECK-NEXT:	[[BAR_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:	.section .bb_addr_map,"o",@progbits,.text._Z3barv{{$}}
+; CHECK-NEXT:	.quad [[BAR_BEGIN]]
+
+
+define dso_local i32 @_Z3foov() {
+  %1 = call i32 @_Z4fooTIiET_v()
+  ret i32 %1
+}
+; CHECK:	.section .text._Z3foov,"ax",@progbits
+; CHECK-LABEL:	_Z3foov:
+; CHECK-NEXT:	[[FOO_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:	.section  .bb_addr_map,"o",@progbits,.text._Z3foov{{$}}
+; CHECK-NEXT:	.quad [[FOO_BEGIN]]
+
+
+define linkonce_odr dso_local i32 @_Z4fooTIiET_v() comdat {
+  ret i32 0
+}
+;; Check we add .bb_addr_map section to a COMDAT group with the corresponding .text section if such a COMDAT exists.
+; CHECK:	.section .text._Z4fo

[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library

2020-09-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision.
MaskRay added a comment.

Looks great!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70378

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


[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library

2020-09-02 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment.

@MaskRay Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70378

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


[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-02 Thread Rahman Lavaee via Phabricator via cfe-commits
rahmanl added a comment.

Thanks a lot for the comments @MaskRay.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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


[PATCH] D86207: (De-)serialize BindingDecls before DecompositionDecl

2020-09-02 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added a comment.

In D86207#2252409 , @aaronpuchert 
wrote:

> In D86207#2251802 , @riccibruno 
> wrote:
>
>> Is my comment on the deserialization of `BindingDecl`s in 
>> https://reviews.llvm.org/D85613?id=284364 related to this change?
>
> Not sure. The `FIXME` comment on the code is correct, and it would be correct 
> after this change. But notice that `Decomp` is also not set when constructing 
> a `BindingDecl` from the parser: first we build the `BindingDecl`s in 
> `Sema::ActOnDecompositionDeclarator` (there `Decomp` remains unset), then 
> build the `DecompositionDecl` from that in `Sema::ActOnVariableDeclarator`. 
> The constructor of `DecompositionDecl` is then calling `setDecomposedDecl` on 
> all bindings, so `Decomp` is set as soon as that's finished. But the 
> `BindingDecl`s exist without `Decomp` for a while.

I agree, but I think that the `BindingDecl` should still be considered to be 
under construction until the corresponding `DecompositionDecl` is constructed. 
Any use of the `BindingDecl` in this interval has to be mindful that the 
`BindingDecl` is not fully constructed.

> In D85613#2210054 , @riccibruno 
> wrote:
>
>> The expression for the binding (`Binding`) will be deserialized when 
>> visiting the `BindingDecl`. This expression when non-null will always (as 
>> far as I can tell) contain a reference to the decomposition declaration so 
>> the decomposition will be deserialized which will set `Decomp`.
>
> My impression is that `Decomp` is set by 
> `ASTDeclReader::VisitDecompositionDecl`, after reading the individual 
> `BindingDecl`s, not from deserializing the `Binding` expression. Otherwise 
> the `BDs[I]->setDecomposedDecl(DD);` above wouldn't be needed. But maybe I'm 
> misunderstanding you.
>
> Maybe you're saying it might be a problem if I read the `BindingDecls` first 
> and then they can't reference the `DecompositionDecl`? I'm not sure how I can 
> see the deserialized AST, when I use `-ast-dump` with `-include-pch` it 
> doesn't show the AST of the included header. But the generated code looks 
> fine for an example I tried out.

My concern is:

Is it possible to see a deserialized `BindingDecl` when  the corresponding 
`DecompositionDecl` has not been deserialized (which will as you say set 
`BindingDecl::Decomp`)? I have not been able to construct an example where this 
is the case but I am not at all confident that this is impossible.

(`-ast-dump-all` can be added to force deserialization)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86207

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


[PATCH] D86091: [cmake] Fix build of attribute plugin example on Windows

2020-09-02 Thread John Brawn via Phabricator via cfe-commits
john.brawn accepted this revision.
john.brawn added a comment.
This revision is now accepted and ready to land.

LGTM (looks like the other example plugins just use PRIVATE as well).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86091

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


[PATCH] D85528: [analyzer] Fix cast evaluation on scoped enums in ExprEngine

2020-09-02 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment.

In D85528#2232074 , @xazax.hun wrote:

> I'm not opposed to landing this to master, as we will have more time there to 
> see whether there are any unwanted side effects in practice.

I made some experiments on the following projects:
symengine,oatpp,zstd,simbody,duckdb,drogon,fmt,cppcheck,capnproto

Only these projects were C++ projects using `enum class` constructs under the 
`clang/utils/analyzer/projects/projects.json` enumeration.
According to the results, no reports changed using the `SATest.py` tool for the 
measurement.
I was using this script to collect the log: F12840610: experiment.sh 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85528

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


[PATCH] D87043: [Analyzer] Fix for dereferece of smart pointer after branching on unknown inner pointer

2020-09-02 Thread Nithin VR via Phabricator via cfe-commits
vrnithinkumar created this revision.
Herald added subscribers: cfe-commits, steakhal, ASDenysPetrov, martong, 
Charusso, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, 
baloghadamsoftware, xazax.hun.
Herald added a project: clang.
vrnithinkumar requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87043

Files:
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/test/Analysis/smart-ptr-text-output.cpp
  clang/test/Analysis/smart-ptr.cpp


Index: clang/test/Analysis/smart-ptr.cpp
===
--- clang/test/Analysis/smart-ptr.cpp
+++ clang/test/Analysis/smart-ptr.cpp
@@ -333,7 +333,7 @@
 void drefOnAssignedNullFromMethodPtrValidSmartPtr() {
   std::unique_ptr P(new A());
   P = returnRValRefOfUniquePtr();
-  P->foo(); // No warning. 
+  P->foo(); // No warning.
 }
 
 void derefMoveConstructedWithValidPtr() {
@@ -374,7 +374,7 @@
 
 void derefMoveConstructedWithRValueRefReturn() {
   std::unique_ptr P(functionReturnsRValueRef());
-  P->foo();  // No warning.
+  P->foo(); // No warning.
 }
 
 void derefConditionOnNullPtr() {
@@ -450,3 +450,10 @@
   else
 return *P; // expected-warning {{Dereference of null smart pointer 'P' 
[alpha.cplusplus.SmartPtr]}}
 }
+
+void derefAfterBranchingOnUnknownInnerPtr(std::unique_ptr P) {
+  A *RP = P.get();
+  if (!RP) {
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' 
[alpha.cplusplus.SmartPtr]}}
+  }
+}
Index: clang/test/Analysis/smart-ptr-text-output.cpp
===
--- clang/test/Analysis/smart-ptr-text-output.cpp
+++ clang/test/Analysis/smart-ptr-text-output.cpp
@@ -304,3 +304,12 @@
 // expected-note@-1 {{Division by zero}}
   }
 };
+
+void derefAfterBranchingOnUnknownInnerPtr(std::unique_ptr P) {
+  A *RP = P.get();
+  if (!RP) { // expected-note {{Assuming 'RP' is null}}
+// expected-note@-1 {{Taking true branch}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' 
[alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -103,7 +103,7 @@
 
 bool isNullSmartPtr(const ProgramStateRef State, const MemRegion *ThisRegion) {
   const auto *InnerPointVal = State->get(ThisRegion);
-  return InnerPointVal && InnerPointVal->isZeroConstant();
+  return InnerPointVal && State->isNull(*InnerPointVal).isConstrainedTrue();
 }
 } // namespace smartptr
 } // namespace ento


Index: clang/test/Analysis/smart-ptr.cpp
===
--- clang/test/Analysis/smart-ptr.cpp
+++ clang/test/Analysis/smart-ptr.cpp
@@ -333,7 +333,7 @@
 void drefOnAssignedNullFromMethodPtrValidSmartPtr() {
   std::unique_ptr P(new A());
   P = returnRValRefOfUniquePtr();
-  P->foo(); // No warning. 
+  P->foo(); // No warning.
 }
 
 void derefMoveConstructedWithValidPtr() {
@@ -374,7 +374,7 @@
 
 void derefMoveConstructedWithRValueRefReturn() {
   std::unique_ptr P(functionReturnsRValueRef());
-  P->foo();  // No warning.
+  P->foo(); // No warning.
 }
 
 void derefConditionOnNullPtr() {
@@ -450,3 +450,10 @@
   else
 return *P; // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
+
+void derefAfterBranchingOnUnknownInnerPtr(std::unique_ptr P) {
+  A *RP = P.get();
+  if (!RP) {
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+  }
+}
Index: clang/test/Analysis/smart-ptr-text-output.cpp
===
--- clang/test/Analysis/smart-ptr-text-output.cpp
+++ clang/test/Analysis/smart-ptr-text-output.cpp
@@ -304,3 +304,12 @@
 // expected-note@-1 {{Division by zero}}
   }
 };
+
+void derefAfterBranchingOnUnknownInnerPtr(std::unique_ptr P) {
+  A *RP = P.get();
+  if (!RP) { // expected-note {{Assuming 'RP' is null}}
+// expected-note@-1 {{Taking true branch}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -103,7 +103,7 @@
 
 bool isNullSmartPtr(const ProgramStateRef State, const MemRegion *ThisRegion) {
   const auto *InnerPointVal = State->get(ThisRegion);
-  return InnerPointVal && InnerPointVal->isZeroConstant();
+  return InnerPointVal && State->isNull(*InnerPointVal).isConstrainedTrue();
 }
 } 

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-02 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx updated this revision to Diff 289503.
chrish_ericsson_atx added a comment.

Updating D86796 : [Sema] Address-space 
sensitive index check for unbounded arrays

Refactored math as suggested by Bevin Hansson.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86796

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/const-eval.c
  clang/test/Sema/unbounded-array-bounds.c
  clang/test/SemaCXX/constant-expression-cxx1y.cpp

Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -1018,8 +1018,9 @@
 }
 
 constexpr void PR28739(int n) { // expected-error {{never produces a constant}}
-  int *p = &n;
+  int *p = &n;  // expected-note {{declared here}}
   p += (__int128)(unsigned long)-1; // expected-note {{cannot refer to element 18446744073709551615 of non-array object in a constant expression}}
+  // expected-warning@-1 {{refers past the last possible element}}
 }
 
 constexpr void Void(int n) {
Index: clang/test/Sema/unbounded-array-bounds.c
===
--- /dev/null
+++ clang/test/Sema/unbounded-array-bounds.c
@@ -0,0 +1,70 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-X86-ADDR64 %s  \
+// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple i386-pc-linux-gnu   -fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-I386-ADDR32 %s \
+// RUN:  --implicit-check-not 'past the last possible element'
+// RUN: %clang_cc1 -triple avr-pc-linux-gnu-fsyntax-only %s 2>&1 | FileCheck --check-prefix=CHECK-AVR-ADDR16 %s  \
+// RUN:  --implicit-check-not 'past the last possible element'
+
+struct S {
+  long long a;
+  char b;
+  long long c;
+  short d;
+};
+
+struct S s[];
+
+void f1() {
+  ++s[3].a;
+  ++s[7073650413200313099].b;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  ++s[7073650].c;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+long long ll[];
+
+void f2() {
+  ++ll[3];
+  ++ll[2705843009213693952];
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 2305843009213693952 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+  ++ll[847073650];
+  // CHECK-I386-ADDR32: :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 536870912 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 8192 elements)
+}
+
+void f3(struct S p[]) {
+  ++p[3].a;
+  ++p[7073650413200313099].b;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:5: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  ++p[7073650].c;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:5: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}
+
+void f4(struct S *p) {
+  p += 3;
+  p += 7073650413200313099;
+  // CHECK-X86-ADDR64:  :[[@LINE-1]]:3: warning: {{.*}} past the last possible element {{.*}} in 64-bit {{.*}} (max possible 576460752303423488 elements)
+  // CHECK-I386-ADDR32: :[[@LINE-2]]:3: warning: {{.*}} past the last possible element {{.*}} in 32-bit {{.*}} (max possible 178956970 elements)
+  // CHECK-AVR-ADDR16:  :[[@LINE-3]]:3: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+  p += 7073650;
+  // CHECK-AVR-ADDR16:  :[[@LINE-1]]:3: warning: {{.*}} past the last possible element {{.*}} in 16-bit {{.*}} (max possible 3276 elements)
+}

[PATCH] D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt

2020-09-02 Thread Mark de Wever via Phabricator via cfe-commits
Mordante marked 8 inline comments as done.
Mordante added a comment.

In D85091#2250657 , @rsmith wrote:

> Looking specifically for attributes in the 'then' and 'else' cases of an `if` 
> seems like a fine first pass at this, but I think this is the wrong way to 
> lower these attributes in the longer term: we should have a uniform treatment 
> of them that looks for the most recent prior branch and annotates it with 
> weights. We could do that either by generating LLVM intrinsic calls that an 
> LLVM pass lowers, or by having the frontend look backwards for the most 
> recent branch and annotate it. I suppose it's instructive to consider a case 
> such as:
>
>   void mainloop() noexcept; // probably terminates by calling `exit`
>   void f() {
> mainloop();
> [[unlikely]];
> somethingelse();
>   }
>
> ... where the `[[unlikely]];` should probably cause us to split the 
> `somethingelse()` out into a separate basic block that we mark as cold, but 
> should not cause `f()` itself or its call to `mainloop()` to be considered 
> unlikely -- except in the case where `mainloop()` can be proven to always 
> terminate, in which case the implication is that it's unlikely that `f()` is 
> invoked. Cases like this probably need the LLVM intrinsic approach to model 
> them properly.

We indeed considered similar cases and wondered whether it was really intended 
to work this way. Since this approach seems a bit hard to explain to users, I 
changed the code back to only allow the attribute on the substatement of the 
`if` and `else`. For now I first want to implement the simple approach, which I 
expect will be the most common use case. Once finished we can see what the next 
steps will be.




Comment at: clang/include/clang/AST/Stmt.h:175-178
+/// The likelihood of taking the ThenStmt.
+/// One of the enumeration values in Stmt::Likelihood.
+unsigned ThenLikelihood : 2;
+

rsmith wrote:
> Do we need to store this here? The "then" and "else" cases should be an 
> `AttributedStmt` holding the likelihood attribute, so duplicating that info 
> here seems redundant.
Agreed. I'll remove it again.



Comment at: clang/lib/Sema/SemaStmt.cpp:578
+static std::pair
+getLikelihood(const Stmt *Stmt) {
+  if (const auto *AS = dyn_cast(Stmt))

aaron.ballman wrote:
> rsmith wrote:
> > Sema doesn't care about any of this; can you move this code to CodeGen and 
> > remove the code that stores likelihood data in the AST?
> FWIW, I asked for it to be moved out of CodeGen and into Sema because the 
> original implementation in CodeGen was doing a fair amount of work trying to 
> interrogate the AST for this information. Now that we've switched the design 
> to only be on the substatement of an if/else statement (rather than an 
> arbitrary statement), it may be that CodeGen is a better place for this again 
> (and if so, I'm sorry for the churn).
At the moment the Sema cares about it to generate diagnostics about conflicting 
annotations:
```
if(i) [[ likely]] {}
else [[likely]] {}
```
This diagnostic only happens for an if statement, for a switch multiple values 
can be considered likely.
Do you prefer to have the diagnostic also in the CodeGen?


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

https://reviews.llvm.org/D85091

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


[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-02 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx marked 2 inline comments as done.
chrish_ericsson_atx added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:13989
+  MaxElems <<= AddrBits;
+  MaxElems /= ElemBytes;
+

ebevhan wrote:
> The size calculations here could probably be simplified by doing something 
> like this:
> 
> * If getActiveBits of the index is greater than AddrBits, it's indexing 
> outside
> * Construct an AddrBits-wide APInt containing the index value
> * Use umul_ovf with getTypeSizeInChars(ElementType); if that overflows, it's 
> indexing outside
> 
Refactored as suggested.  I agree -- it's cleaner this way (eliminates the 
loop), and avoids a divide when no diag is required.  But it's still hard to 
read because of all the APInt bitwidth hijinks necessary to do the math without 
tripping asserts. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86796

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


[PATCH] D86853: [modules] Fix crash in call to `FunctionDecl::setPure()`

2020-09-02 Thread Andrew Gallagher via Phabricator via cfe-commits
andrewjcg updated this revision to Diff 289506.
andrewjcg marked 2 inline comments as done.
andrewjcg added a comment.

feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86853

Files:
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/test/Modules/Inputs/set-pure-crash/a.h
  clang/test/Modules/Inputs/set-pure-crash/b.h
  clang/test/Modules/Inputs/set-pure-crash/c.h
  clang/test/Modules/Inputs/set-pure-crash/module.modulemap
  clang/test/Modules/set-pure-crash.cpp

Index: clang/test/Modules/set-pure-crash.cpp
===
--- /dev/null
+++ clang/test/Modules/set-pure-crash.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x c++ -I %S/Inputs/set-pure-crash -verify %s -o %t
+
+// expected-no-diagnostics
+
+#include "b.h"
+#include "c.h"
+
+auto t = simple();
Index: clang/test/Modules/Inputs/set-pure-crash/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/module.modulemap
@@ -0,0 +1,11 @@
+module a {
+  header "a.h"
+}
+
+module b {
+  header "b.h"
+}
+
+module c {
+  header "c.h"
+}
Index: clang/test/Modules/Inputs/set-pure-crash/c.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/c.h
@@ -0,0 +1,5 @@
+#pragma once
+
+template 
+struct simple {
+};
Index: clang/test/Modules/Inputs/set-pure-crash/b.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/b.h
@@ -0,0 +1,14 @@
+#pragma once
+
+#include "a.h"
+#include "c.h"
+
+template >
+void foo(Fun) {}
+
+class Child : public Base {
+public:
+  void func() {
+foo([]() {});
+  }
+};
Index: clang/test/Modules/Inputs/set-pure-crash/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/a.h
@@ -0,0 +1,11 @@
+#pragma once
+
+struct Tag {};
+
+template 
+class Base {
+public:
+  virtual void func() = 0;
+};
+
+Base bar();
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -864,7 +864,10 @@
   FD->setInlineSpecified(Record.readInt());
   FD->setImplicitlyInline(Record.readInt());
   FD->setVirtualAsWritten(Record.readInt());
-  FD->setPure(Record.readInt());
+  // We defer calling `FunctionDecl::setPure()` here as for methods of
+  // `CXXTemplateSpecializationDecl`s, we may not have connected up the
+  // definition (which is required for `setPure`).
+  const bool Pure = Record.readInt();
   FD->setHasInheritedPrototype(Record.readInt());
   FD->setHasWrittenPrototype(Record.readInt());
   FD->setDeletedAsWritten(Record.readInt());
@@ -1012,6 +1015,10 @@
   }
   }
 
+  // Defer calling `setPure` until merging above has guaranteed we've set
+  // `DefinitionData` (as this will need to access it).
+  FD->setPure(Pure);
+
   // Read in the parameters.
   unsigned NumParams = Record.readInt();
   SmallVector Params;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.

2020-09-02 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno marked 2 inline comments as done.
riccibruno added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:91
+  auto CheckAndDiagnoseLocalEntity = [&](const VarDecl *VD, unsigned DiagID,
+ const auto &... DiagArgs) -> bool {
+if (VD->isLocalVarDecl() && !DRE->isNonOdrUse()) {

aaronpuchert wrote:
> Since you're emitting a specific warning, I think you can just hardcode the 
> expected types here instead of using `const auto&...`. Alternatively you 
> could guess the integer argument from the (dynamic) type of the `VarDecl` 
> argument.
I am using a pack here (changed a bit in the latest revision) to make it easy 
to re-use this lambda when the inconsistency between parameters and local 
variables is removed.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:147
+//
+const auto *VD = cast_or_null(Binding->getDecomposedDecl());
+if (VD && CheckAndDiagnoseLocalEntity(

aaronpuchert wrote:
> Have you seen a case there the `_or_null` is relevant? To me it seems like 
> this shouldn't happen, at least not when we get here.
I have removed it in the latest revision since we should fix the 
(de)serialization instead if this is possible.



Comment at: clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp:40
   extern void h6(int = i5);
-  // expected-error-re@-1 {{default argument references local variable 
'(unnamed variable of type (anonymous union at {{.*}}:20:3))' of enclosing 
function}}
+  // expected-error-re@-1 {{default argument references local variable 
'(unnamed variable of type (anonymous union at {{.*}}:35:3))' of enclosing 
function}}
+

aaronpuchert wrote:
> Better use a relative line number: `[[@LINE-5]]` or something like that.
I wish I could. Unfortunately `[[@LINE-5]]` is a `CHECK` line construct. Is it 
possible to refer to a relative line in a regex `verify` line?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85613

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


[PATCH] D82994: [RFC] Instrumenting Clang/LLVM with Perfetto

2020-09-02 Thread Mark de Wever via Phabricator via cfe-commits
Mordante added inline comments.



Comment at: clang/include/clang/Basic/CodeGenOptions.def:244
 VALUE_CODEGENOPT(TimeTraceGranularity, 32, 500) ///< Minimum time granularity 
(in microseconds),
+CODEGENOPT(Perfetto  , 1, 0) ///< Set when -perfetto is enabled.
///< traced by time profiler

Seems this line should be one lower to keep the comment above together.



Comment at: clang/include/clang/Frontend/FrontendOptions.h:251
 
+  /// Output time trace profile.
+  unsigned Perfetto : 1;

Is it intended to have the same comment as `TimeTrace`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82994

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


[PATCH] D87028: [clang-format] Improve heuristic for detecting function declarations

2020-09-02 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson updated this revision to Diff 289508.
arichardson added a comment.

fix tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87028

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
@@ -5482,12 +5482,28 @@
   "   a));");
   verifyFormat("bool aaa\n"
"__attribute__((unused));");
-  verifyGoogleFormat(
+  FormatStyle Google = getGoogleStyle();
+  verifyFormat(
   "bool a\n"
-  "GUARDED_BY();");
-  verifyGoogleFormat(
+  "GUARDED_BY();\n" // parsed as function decl
+  "void f() {\n"
+  "  bool aaa\n"
+  "  GUARDED_BY();\n" // parsed as variable decl
+  "}\n"
+  "class Cls {\n"
+  "  bool a\n"
+  "  GUARDED_BY();\n" // parsed as variable decl
+  "};",
+  Google);
+  verifyFormat(
   "bool a\n"
-  "GUARDED_BY();");
+  "GUARDED_BY();", // parsed as function decl
+  Google);
+  Google.AttributeMacros = {"GUARDED_BY"};
+  verifyFormat(
+  "bool a GUARDED_BY(\n"
+  ");", // parsed as variable decl
+  Google);
   verifyGoogleFormat(
   "bool aa GUARDED_BY() =\n"
   "::aaa;");
@@ -6683,10 +6699,67 @@
   // All declarations and definitions should have the return type moved to its
   // own line.
   Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_All;
+  Style.NamespaceIndentation = FormatStyle::NI_All;
   Style.TypenameMacros = {"LIST"};
   verifyFormat("SomeType\n"
"funcdecl(LIST(uint64_t));",
Style);
+  verifyFormat("SomeType\n"
+   "funcdecl();\n"
+   "SomeType\n"
+   "funcdecl(SomeType paramname);\n"
+   "SomeType\n"
+   "funcdecl(_Atomic(uint64_t));\n"
+   "SomeType\n"
+   "funcdecl(SomeType param1, OtherType param2);\n"
+   // Also handle parameter lists declaration without names (but
+   // only at the top level, not inside functions
+   "SomeType\n"
+   "funcdecl(SomeType);\n"
+   "SomeType\n"
+   "funcdecl(SomeType, OtherType);\n"
+   // Check that any kind of expression/operator results in parsing
+   // it as a variable declaration and not a function
+   "SomeType vardecl(var + otherVar);\n"
+   "SomeType vardecl(func());\n"
+   "SomeType vardecl(someFunc(arg));\n"
+   "SomeType vardecl(var, var - otherVar);\n"
+   "SomeType x = var * funcdecl();\n"
+   "SomeType x = var * funcdecl(otherVar);\n"
+   "SomeType x = var * funcdecl(var, otherVar);\n"
+   "void\n"
+   "function_scope() {\n"
+   "  SomeType x = var * funcdecl();\n"
+   "  SomeType x = var * funcdecl(otherVar);\n"
+   "  SomeType x = var * funcdecl(var, otherVar);\n"
+   // While clang will parse these as function declarations, at
+   // least for now clang-format assumes they are variables.
+   "  SomeType *funcdecl();\n"
+   "  SomeType *funcdecl(SomeType);\n"
+   "  SomeType *funcdecl(SomeType, OtherType);\n"
+   "}\n"
+   "namespace namspace_scope {\n"
+   // TODO: Should we also parse these as a function declaration
+   //  and not as a variable inside namespaces?
+   "  SomeType\n"
+   "  funcdecl();\n"
+   "  SomeType\n"
+   "  funcdecl(SomeType paramname);\n"
+   "  SomeType\n"
+   "  funcdecl(_Atomic(uint64_t));\n"
+   "  SomeType\n"
+   "  funcdecl(SomeType param1, OtherType param2);\n"
+   "  SomeType decl(SomeType);\n"
+   "  SomeType decl(SomeType, OtherType);\n"
+   "  SomeType vardecl(var + otherVar);\n"
+   "  SomeType vardecl(func());\n"
+   "  SomeType vardecl(someFunc(arg));\n"
+   "  SomeType vardecl(var, var - otherVar);\n"
+   "  SomeType x = var * funcdecl();\n"
+   "  SomeType x = var

[PATCH] D86959: [clang-format] Fix formatting of _Atomic() qualifier

2020-09-02 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson updated this revision to Diff 289509.
arichardson added a comment.

- fix test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86959

Files:
  clang/lib/Format/FormatToken.cpp
  clang/lib/Format/FormatToken.h
  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
@@ -168,6 +168,8 @@
   verifyFormat("vector<::Type> v;");
   verifyFormat("::ns::SomeFunction(::ns::SomeOtherFunction())");
   verifyFormat("static constexpr bool Bar = decltype(bar())::value;");
+  verifyFormat("static constexpr bool Bar = typeof(bar())::value;");
+  verifyFormat("static constexpr bool Bar = _Atomic(bar())::value;");
   verifyFormat("bool a = 2 < ::SomeFunction();");
   verifyFormat("ALWAYS_INLINE ::std::string getName();");
   verifyFormat("some::string getName();");
@@ -7904,7 +7906,10 @@
   verifyFormat("auto PointerBinding = [](const char *S) {};");
   verifyFormat("typedef typeof(int(int, int)) *MyFunc;");
   verifyFormat("[](const decltype(*a) &value) {}");
+  verifyFormat("[](const typeof(*a) &value) {}");
+  verifyFormat("[](const _Atomic(a *) &value) {}");
   verifyFormat("decltype(a * b) F();");
+  verifyFormat("typeof(a * b) F();");
   verifyFormat("#define MACRO() [](A *a) { return 1; }");
   verifyFormat("Constructor() : member([](A *a, B *b) {}) {}");
   verifyIndependentOfContext("typedef void (*f)(int *a);");
@@ -7970,6 +7975,8 @@
   verifyFormat("delete *x;", Left);
   verifyFormat("typedef typeof(int(int, int))* MyFuncPtr;", Left);
   verifyFormat("[](const decltype(*a)* ptr) {}", Left);
+  verifyFormat("[](const typeof(*a)* ptr) {}", Left);
+  verifyFormat("[](const _Atomic(a*)* ptr) {}", Left);
   verifyFormat("typedef typeof /*comment*/ (int(int, int))* MyFuncPtr;", Left);
   verifyFormat("auto x(A&&, B&&, C&&) -> D;", Left);
   verifyFormat("auto x = [](A&&, B&&, C&&) -> D {};", Left);
@@ -8066,6 +8073,8 @@
   verifyFormat("foo();");
   verifyFormat("foo();");
   verifyFormat("decltype(*::std::declval()) void F();");
+  verifyFormat("typeof(*::std::declval()) void F();");
+  verifyFormat("_Atomic(*::std::declval()) void F();");
   verifyFormat(
   "template ::value &&\n"
@@ -8089,6 +8098,9 @@
   verifyIndependentOfContext("MACRO(int *i);");
   verifyIndependentOfContext("MACRO(auto *a);");
   verifyIndependentOfContext("MACRO(const A *a);");
+  verifyIndependentOfContext("MACRO(_Atomic(A) *a);");
+  verifyIndependentOfContext("MACRO(decltype(A) *a);");
+  verifyIndependentOfContext("MACRO(typeof(A) *a);");
   verifyIndependentOfContext("MACRO(A *const a);");
   verifyIndependentOfContext("MACRO(A *restrict a);");
   verifyIndependentOfContext("MACRO(A *__restrict__ a);");
@@ -8639,6 +8651,10 @@
"LooongFunctionDefinition() {}");
   verifyFormat("decltype(LngName)\n"
"LooongFunctionDefinition() {}");
+  verifyFormat("typeof(LoongName)\n"
+   "LooongFunctionDefinition() {}");
+  verifyFormat("_Atomic(LongName)\n"
+   "LooongFunctionDefinition() {}");
   verifyFormat("LngReturnType\n"
"LooongFunctionDeclaration(T... t);");
   verifyFormat("LngReturnType\n"
@@ -8988,6 +9004,8 @@
   verifyFormat("int foo(int i) { return fo1{}(i); }");
   verifyFormat("int foo(int i) { return fo1{}(i); }");
   verifyFormat("auto i = decltype(x){};");
+  verifyFormat("auto i = typeof(x){};");
+  verifyFormat("auto i = _Atomic(x){};");
   verifyFormat("std::vector v = {1, 0 /* comment */};");
   verifyFormat("Node n{1, Node{1000}, //\n"
"   2};");
@@ -11580,6 +11598,8 @@
   verifyFormat("auto i = std::make_unique(5);", NoSpace);
   verifyFormat("size_t x = sizeof(x);", NoSpace);
   verifyFormat("auto f(int x) -> decltype(x);", NoSpace);
+  verifyFormat("auto f(int x) -> typeof(x);", NoSpace);
+  verifyFormat("auto f(int x) -> _Atomic(x);", NoSpace);
   verifyFormat("int f(T x) noexcept(x.create());", NoSpace);
   verifyFormat("alignas(128) char a[128];", NoSpace);
   verifyFormat("size_t x = alignof(MyType);", NoSpace);
@@ -11628,6 +11648,8 @@
   verifyFormat("auto i = std::make_unique (5);", Space);
   verifyFormat("size_t x = sizeof (x);", Space);
   verifyFormat("auto f (int x) -> decltype (x);", Space);
+  verifyFormat("auto f (int x) -> typeof (x);", Space);
+  verifyFormat("auto f (int x) -> _Atomic (x);", Space);
   verifyFormat("int f (T x) noexcept (x.create ());", Space);

[PATCH] D86960: [clang-format] Parse __underlying_type(T) as a type

2020-09-02 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson updated this revision to Diff 289510.
arichardson added a comment.

rebase to keep dependent revisions happy


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86960

Files:
  clang/lib/Format/FormatToken.h
  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
@@ -169,6 +169,7 @@
   verifyFormat("::ns::SomeFunction(::ns::SomeOtherFunction())");
   verifyFormat("static constexpr bool Bar = decltype(bar())::value;");
   verifyFormat("static constexpr bool Bar = typeof(bar())::value;");
+  verifyFormat("static constexpr bool Bar = __underlying_type(bar())::value;");
   verifyFormat("static constexpr bool Bar = _Atomic(bar())::value;");
   verifyFormat("bool a = 2 < ::SomeFunction();");
   verifyFormat("ALWAYS_INLINE ::std::string getName();");
@@ -7908,6 +7909,7 @@
   verifyFormat("[](const decltype(*a) &value) {}");
   verifyFormat("[](const typeof(*a) &value) {}");
   verifyFormat("[](const _Atomic(a *) &value) {}");
+  verifyFormat("[](const __underlying_type(a) &value) {}");
   verifyFormat("decltype(a * b) F();");
   verifyFormat("typeof(a * b) F();");
   verifyFormat("#define MACRO() [](A *a) { return 1; }");
@@ -7977,6 +7979,7 @@
   verifyFormat("[](const decltype(*a)* ptr) {}", Left);
   verifyFormat("[](const typeof(*a)* ptr) {}", Left);
   verifyFormat("[](const _Atomic(a*)* ptr) {}", Left);
+  verifyFormat("[](const __underlying_type(a)* ptr) {}", Left);
   verifyFormat("typedef typeof /*comment*/ (int(int, int))* MyFuncPtr;", Left);
   verifyFormat("auto x(A&&, B&&, C&&) -> D;", Left);
   verifyFormat("auto x = [](A&&, B&&, C&&) -> D {};", Left);
@@ -8075,6 +8078,7 @@
   verifyFormat("decltype(*::std::declval()) void F();");
   verifyFormat("typeof(*::std::declval()) void F();");
   verifyFormat("_Atomic(*::std::declval()) void F();");
+  verifyFormat("__underlying_type(*::std::declval()) void F();");
   verifyFormat(
   "template ::value &&\n"
@@ -8101,6 +8105,7 @@
   verifyIndependentOfContext("MACRO(_Atomic(A) *a);");
   verifyIndependentOfContext("MACRO(decltype(A) *a);");
   verifyIndependentOfContext("MACRO(typeof(A) *a);");
+  verifyIndependentOfContext("MACRO(__underlying_type(A) *a);");
   verifyIndependentOfContext("MACRO(A *const a);");
   verifyIndependentOfContext("MACRO(A *restrict a);");
   verifyIndependentOfContext("MACRO(A *__restrict__ a);");
@@ -8655,6 +8660,8 @@
"LooongFunctionDefinition() {}");
   verifyFormat("_Atomic(LongName)\n"
"LooongFunctionDefinition() {}");
+  verifyFormat("__underlying_type(LooongName)\n"
+   "LooongFunctionDefinition() {}");
   verifyFormat("LngReturnType\n"
"LooongFunctionDeclaration(T... t);");
   verifyFormat("LngReturnType\n"
@@ -11600,6 +11607,7 @@
   verifyFormat("auto f(int x) -> decltype(x);", NoSpace);
   verifyFormat("auto f(int x) -> typeof(x);", NoSpace);
   verifyFormat("auto f(int x) -> _Atomic(x);", NoSpace);
+  verifyFormat("auto f(int x) -> __underlying_type(x);", NoSpace);
   verifyFormat("int f(T x) noexcept(x.create());", NoSpace);
   verifyFormat("alignas(128) char a[128];", NoSpace);
   verifyFormat("size_t x = alignof(MyType);", NoSpace);
@@ -11650,6 +11658,7 @@
   verifyFormat("auto f (int x) -> decltype (x);", Space);
   verifyFormat("auto f (int x) -> typeof (x);", Space);
   verifyFormat("auto f (int x) -> _Atomic (x);", Space);
+  verifyFormat("auto f (int x) -> __underlying_type (x);", Space);
   verifyFormat("int f (T x) noexcept (x.create ());", Space);
   verifyFormat("alignas (128) char a[128];", Space);
   verifyFormat("size_t x = alignof (MyType);", Space);
@@ -11704,6 +11713,7 @@
   verifyFormat("auto f (int x) -> decltype (x);", SomeSpace);
   verifyFormat("auto f (int x) -> typeof (x);", SomeSpace);
   verifyFormat("auto f (int x) -> _Atomic (x);", SomeSpace);
+  verifyFormat("auto f (int x) -> __underlying_type (x);", SomeSpace);
   verifyFormat("int f (T x) noexcept (x.create());", SomeSpace);
   verifyFormat("alignas (128) char a[128];", SomeSpace);
   verifyFormat("size_t x = alignof (MyType);", SomeSpace);
@@ -14960,6 +14970,7 @@
"  SomeFunction([](decltype(x), A *a) {});\n"
"  SomeFunction([](typeof(x), A *a) {});\n"
"  SomeFunction([](_Atomic(x), A *a) {});\n"
+   "  SomeFunction([](__underlying_type(x), A *a) {});\n"
"}");
   verifyFormat("aaa

[clang] d70e05c - [clang-format] Parse double-square attributes as pointer qualifiers

2020-09-02 Thread Alex Richardson via cfe-commits

Author: Alex Richardson
Date: 2020-09-02T18:35:21+01:00
New Revision: d70e05c9e36ada3ea6341764a3bc34de7de7d8dd

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

LOG: [clang-format] Parse double-square attributes as pointer qualifiers

Before: x = (foo *[[clang::attr]]) * v;
After:  x = (foo *[[clang::attr]])*v;

Reviewed By: aaron.ballman

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

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 a9077500e041..f04f101f0459 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -1840,6 +1840,12 @@ class AnnotatingParser {
 T = T->MatchingParen->Previous->Previous;
 continue;
   }
+} else if (T->is(TT_AttributeSquare)) {
+  // Handle `x = (foo *[[clang::foo]])&v;`:
+  if (T->MatchingParen && T->MatchingParen->Previous) {
+T = T->MatchingParen->Previous;
+continue;
+  }
 } else if (T->canBePointerOrReferenceQualifier()) {
   T = T->Previous;
   continue;

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index f2978cdbed8d..14c97784b738 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -8068,6 +8068,8 @@ TEST_F(FormatTest, UnderstandsUsesOfStarAndAmp) {
   verifyIndependentOfContext("MACRO(A *_Null_unspecified a);");
   verifyIndependentOfContext("MACRO(A *__attribute__((foo)) a);");
   verifyIndependentOfContext("MACRO(A *__attribute((foo)) a);");
+  verifyIndependentOfContext("MACRO(A *[[clang::attr]] a);");
+  verifyIndependentOfContext("MACRO(A *[[clang::attr(\"foo\")]] a);");
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
   // FIXME: Is there a way to make this work?
@@ -8137,14 +8139,17 @@ TEST_F(FormatTest, UnderstandsPointerQualifiersInCast) {
   verifyFormat("x = (foo *_Nullable)*v;");
   verifyFormat("x = (foo *_Null_unspecified)*v;");
   verifyFormat("x = (foo *_Nonnull)*v;");
+  verifyFormat("x = (foo *[[clang::attr]])*v;");
+  verifyFormat("x = (foo *[[clang::attr(\"foo\")]])*v;");
 
   // Check that we handle multiple trailing qualifiers and skip them all to
   // determine that the expression is a cast to a pointer type.
   FormatStyle LongPointerRight = getLLVMStyleWithColumns(999);
   FormatStyle LongPointerLeft = getLLVMStyleWithColumns(999);
   LongPointerLeft.PointerAlignment = FormatStyle::PAS_Left;
-  StringRef AllQualifiers = "const volatile restrict __attribute__((foo)) "
-"_Nonnull _Null_unspecified _Nonnull";
+  StringRef AllQualifiers =
+  "const volatile restrict __attribute__((foo)) _Nonnull _Null_unspecified 
"
+  "_Nonnull [[clang::attr]]";
   verifyFormat(("x = (foo *" + AllQualifiers + ")*v;").str(), 
LongPointerRight);
   verifyFormat(("x = (foo* " + AllQualifiers + ")*v;").str(), LongPointerLeft);
 



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


[PATCH] D86721: [clang-format] Parse double-square attributes as pointer qualifiers

2020-09-02 Thread Alexander Richardson via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd70e05c9e36a: [clang-format] Parse double-square attributes 
as pointer qualifiers (authored by arichardson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86721

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
@@ -8068,6 +8068,8 @@
   verifyIndependentOfContext("MACRO(A *_Null_unspecified a);");
   verifyIndependentOfContext("MACRO(A *__attribute__((foo)) a);");
   verifyIndependentOfContext("MACRO(A *__attribute((foo)) a);");
+  verifyIndependentOfContext("MACRO(A *[[clang::attr]] a);");
+  verifyIndependentOfContext("MACRO(A *[[clang::attr(\"foo\")]] a);");
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
   // FIXME: Is there a way to make this work?
@@ -8137,14 +8139,17 @@
   verifyFormat("x = (foo *_Nullable)*v;");
   verifyFormat("x = (foo *_Null_unspecified)*v;");
   verifyFormat("x = (foo *_Nonnull)*v;");
+  verifyFormat("x = (foo *[[clang::attr]])*v;");
+  verifyFormat("x = (foo *[[clang::attr(\"foo\")]])*v;");
 
   // Check that we handle multiple trailing qualifiers and skip them all to
   // determine that the expression is a cast to a pointer type.
   FormatStyle LongPointerRight = getLLVMStyleWithColumns(999);
   FormatStyle LongPointerLeft = getLLVMStyleWithColumns(999);
   LongPointerLeft.PointerAlignment = FormatStyle::PAS_Left;
-  StringRef AllQualifiers = "const volatile restrict __attribute__((foo)) "
-"_Nonnull _Null_unspecified _Nonnull";
+  StringRef AllQualifiers =
+  "const volatile restrict __attribute__((foo)) _Nonnull _Null_unspecified 
"
+  "_Nonnull [[clang::attr]]";
   verifyFormat(("x = (foo *" + AllQualifiers + ")*v;").str(), 
LongPointerRight);
   verifyFormat(("x = (foo* " + AllQualifiers + ")*v;").str(), LongPointerLeft);
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1840,6 +1840,12 @@
 T = T->MatchingParen->Previous->Previous;
 continue;
   }
+} else if (T->is(TT_AttributeSquare)) {
+  // Handle `x = (foo *[[clang::foo]])&v;`:
+  if (T->MatchingParen && T->MatchingParen->Previous) {
+T = T->MatchingParen->Previous;
+continue;
+  }
 } else if (T->canBePointerOrReferenceQualifier()) {
   T = T->Previous;
   continue;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8068,6 +8068,8 @@
   verifyIndependentOfContext("MACRO(A *_Null_unspecified a);");
   verifyIndependentOfContext("MACRO(A *__attribute__((foo)) a);");
   verifyIndependentOfContext("MACRO(A *__attribute((foo)) a);");
+  verifyIndependentOfContext("MACRO(A *[[clang::attr]] a);");
+  verifyIndependentOfContext("MACRO(A *[[clang::attr(\"foo\")]] a);");
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
   // FIXME: Is there a way to make this work?
@@ -8137,14 +8139,17 @@
   verifyFormat("x = (foo *_Nullable)*v;");
   verifyFormat("x = (foo *_Null_unspecified)*v;");
   verifyFormat("x = (foo *_Nonnull)*v;");
+  verifyFormat("x = (foo *[[clang::attr]])*v;");
+  verifyFormat("x = (foo *[[clang::attr(\"foo\")]])*v;");
 
   // Check that we handle multiple trailing qualifiers and skip them all to
   // determine that the expression is a cast to a pointer type.
   FormatStyle LongPointerRight = getLLVMStyleWithColumns(999);
   FormatStyle LongPointerLeft = getLLVMStyleWithColumns(999);
   LongPointerLeft.PointerAlignment = FormatStyle::PAS_Left;
-  StringRef AllQualifiers = "const volatile restrict __attribute__((foo)) "
-"_Nonnull _Null_unspecified _Nonnull";
+  StringRef AllQualifiers =
+  "const volatile restrict __attribute__((foo)) _Nonnull _Null_unspecified "
+  "_Nonnull [[clang::attr]]";
   verifyFormat(("x = (foo *" + AllQualifiers + ")*v;").str(), LongPointerRight);
   verifyFormat(("x = (foo* " + AllQualifiers + ")*v;").str(), LongPointerLeft);
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1840,6 +1840,12 @@
 T = T->MatchingParen->Previous->Previous;
 continue;
   }
+} else if (T->is(TT_AttributeSquare))

[PATCH] D86290: Move all fields of '-cc1' option related classes into def file databases

2020-09-02 Thread Douglas Yung via Phabricator via cfe-commits
dyung added a comment.

I have reverted this change in 8d2d0e84857cb1f2d01456eb433b5172d3a0772b 
 to get 
the build bots green again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86290

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


[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given

2020-09-02 Thread Hans Wennborg via Phabricator via cfe-commits
hans added inline comments.



Comment at: clang/lib/Sema/SemaCast.cpp:895
+  if (!Self.getLangOpts().RTTIData) {
+bool isMSVC = Self.getDiagnostics().getDiagnosticOptions().getFormat() ==
+  DiagnosticOptions::MSVC;

zequanwu wrote:
> hans wrote:
> > zequanwu wrote:
> > > hans wrote:
> > > > zequanwu wrote:
> > > > > hans wrote:
> > > > > > I'm not sure isMSVC is the best name (it's not clear what aspect is 
> > > > > > MSVC exactly -- in this case it's the diagnostics format).
> > > > > > 
> > > > > > It's possible to target MSVC both with clang-cl and with regular 
> > > > > > clang.
> > > > > > 
> > > > > > For example, one could use
> > > > > > 
> > > > > >   clang-cl /c /tmp/a.cpp
> > > > > > 
> > > > > > or
> > > > > > 
> > > > > >   clang -c /tmp/a.cpp -target i686-pc-windows-msvc19.11.0 
> > > > > > -fms-extensions
> > > > > > 
> > > > > > 
> > > > > > My understanding is that the purpose of "isMSVC" here is to try and 
> > > > > > detect if we're using clang-cl or clang so that the diagnostic can 
> > > > > > say "/GR-" or "-fno-rtti-data". So maybe it's better to call it 
> > > > > > "isClangCL" or something like that.
> > > > > > 
> > > > > > Also, I don't think we should check "isMSVC" in the if-statement 
> > > > > > below. We want the warning to fire both when using clang and 
> > > > > > clang-cl: as long as -fno-rtti-data or /GR- is used, the warning 
> > > > > > makes sense.
> > > > > > 
> > > > > > So I think the code could be more like:
> > > > > > 
> > > > > > ```
> > > > > > if (!Self.getLangOpts().RTTIData && !DestPointee->isVoidType()) {
> > > > > >   bool isClangCL = ...;
> > > > > >   Self.Diag(...) << isClangCL;
> > > > > > }
> > > > > > ```
> > > > > MSVC will warn even if the DestPointee is void type. What I thought 
> > > > > is if invoked by clang-cl warn regardless of DeskPointee type. If 
> > > > > invoked by clang, warn if it's not void type. 
> > > > > https://godbolt.org/z/475q5v. I noticed MSVC won't warn at typeid if 
> > > > > /GR- is given. Probably I should remove the warning in typeid.
> > > > If it's true the casting to void* doesn't need RTTI data (I think it 
> > > > is, but would be good to verify), then it doesn't make sense to warn. 
> > > > We don't have to follow MSVC's behavior when it doesn't make sense :)
> > > > 
> > > > Similar reasoning for typeid() - I assume it won't work with /GR- also 
> > > > with MSVC, so warning about it probably makes sense.
> > > In clang, I believe that dynamic_cast to void* doesn't use RTTI data: 
> > > https://godbolt.org/z/Kbr7Mq
> > > Looks like MSVC only warns if the operand of typeid is not pointer: 
> > > https://godbolt.org/z/chcMcn
> > > 
> > When targeting Windows, dynamic_cast to void* is implemented with in a 
> > runtime function, RTCastToVoid: https://godbolt.org/z/Kecr7z
> > I wonder if that uses RTTI data internally though...
> > 
> > For typeid() I guess it would also warn on references? Maybe we should do 
> > the same.
> Couldn't find if `__RTCastToVoid` uses RTTI data internally.
> 
> For typeid(), it also warn on references. But the behavior is a bit weird 
> (https://godbolt.org/z/jn4Pjx). Seems like it warns only when dereferencing a 
> pointer or argument is a reference.
Okay, maybe it's safest to warn about dynamic_cast also for void*, like MSVC 
does.

For typeid() maybe we should be conservative like Clang is with -fno-rtti, and 
always warn.

But I'm a little bit surprised about this, because I'd imagine typeid(int) 
could work also with -fno-rtti... if you're curious maybe it would be 
interesting to dig deeper into how this works.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86369

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


[PATCH] D86089: [flang][driver]Add experimental flang driver and frontend with help screen

2020-09-02 Thread Richard Barton via Phabricator via cfe-commits
richard.barton.arm requested changes to this revision.
richard.barton.arm added a comment.
This revision now requires changes to proceed.

Requesting changes mostly because of the exit status issue on the Driver tests.

A few general questions as well:

1. Why not implement `-###` as part of this patch to enable testing more easily?
2. How come there are no regression tests for -fc1 in flang/test/Frontend? I 
suppose these come when the first real FrontendAction is implemented?




Comment at: flang/test/CMakeLists.txt:8
 
+llvm_canonicalize_cmake_booleans(FLANG_BUILD_NEW_DRIVER)
+

So do the other bools like LINK_WITH_FIR also need this treatment and this is a 
latent bug? Seems amazing we've not hit that before now.

From an offline conversation ISTR you saying this was to do with how the 
variable is translated into the lit config? If that is the case then I think 
there is a function you can use called lit.util.pythonize_bool which converts 
various strings that mean "True/False" into a real bool for python. That would 
also let you clean up the lit.cfg.py later on (separate comments).



Comment at: flang/test/Flang-Driver/driver-error-cc1.c:7
+
+// CHECK:error: unknown integrated tool '-cc1'. Valid tools include '-fc1'.

I am surprised that you don't need to prefix this run line with 'not' 
indicating that flang-new returns 0 exit code in this instance, which seems 
wrong to me.



Comment at: flang/test/Flang-Driver/driver-error-cc1.cpp:1
+// RUN: %flang-new %s 2>&1 | FileCheck %s
+

Same comment as above on exit code.



Comment at: flang/test/Flang-Driver/emit-obj.f90:2
+! RUN: %flang-new  %s 2>&1 | FileCheck %s --check-prefix=ERROR
+! RUN: %flang-new  -fc1 -emit-obj %s 2>&1 | FileCheck %s --check-prefix=ERROR
+

Seems like it would be helpful to have also implemented `-###` in this patch so 
that you don't need to write tests like this. You could simply run flang-new 
-### then check the -fc1 line that would have been emitted for the presence of 
-emit-obj.

Same comment above regarding exit code.



Comment at: flang/test/lit.cfg.py:39
 # directories.
+# exclude the tests for flang_new driver while there are two drivers
 config.excludes = ['Inputs', 'CMakeLists.txt', 'README.txt', 'LICENSE.txt']

This comment should be on line 41



Comment at: flang/test/lit.cfg.py:40
+# exclude the tests for flang_new driver while there are two drivers
+if config.include_flang_new_driver_test == "OFF":
+  config.excludes = ['Inputs', 'CMakeLists.txt', 'README.txt', 'LICENSE.txt', 
'Flang-Driver']

awarzynski wrote:
> CarolineConcatto wrote:
> > richard.barton.arm wrote:
> > > richard.barton.arm wrote:
> > > > I think it would be cleaner to define config.excludes unconditionally 
> > > > then append the Flang-Driver dir if our condition passes.
> > > I _think_ the pattern to follow to exclude tests for something you 
> > > haven't built is to use lit features.
> > > 
> > > So you would add a feature like:
> > > `config.available_features.add("new-driver")`
> > > 
> > > then each test that only works on the new driver would be prefixed with a 
> > > statement:
> > > 
> > > `REQUIRES: new-driver`
> > > 
> > > This means that the tests can go in the test/Driver directory and you 
> > > don't need to create a new directory for these tests or this hack. The 
> > > additional benefit would be that all the existing tests for the throwaway 
> > > driver can be re-used on the new Driver to test it. There are not many of 
> > > those though and we are using a different driver name so they can't be 
> > > shared either. Still think it would be a worthwhile thing to do because 
> > > when looking at the test itself it is clear why it is not being run 
> > > whereas with this hack it is hidden away.
> > > 
> > >  Sorry for not thinking this first time around.
> > I like to have the test at a different folder for now so it is clear that 
> > the tests inside this folder all belongs to the new driver. So I don't need 
> > to open the test to know.
> > I can implement the requires, but in the future will not need the REQUIRES 
> > for the driver test.
> Let me clarify a bit. I assume that `FLANG_BUILD_NEW_DRIVER` is Off.
> 
> SCENARIO 1: In order to make sure that `bin/llvm-lit tools/flang/test/` does 
> not _attempt to run_ the new driver tests, add:
>  `config.excludes.append('Flang-Driver')`
> 
> With this, the new driver tests will neither be run nor summarized (e.g. as 
> `UNSUPPORTED`) in the final output.
> 
> SCENARIO 2: `config.excludes.append('Flang-Driver')` will not affect 
> `bin/llvm-lit tools/flang/test/Flang-Driver` (this time I explicitly specify 
> the `Flang-Driver` directory). We need:
> 
> `REQUIERES: new-flang-driver`
> 
> (or similar) for the new Flang driver tests to be reported as `UNSUPPORTED`.
> 
> I agree with 

[PATCH] D86993: Document Clang's expectations of the C standard library.

2020-09-02 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

In D86993#2251198 , @rjmccall wrote:

> Wording looks good.  Should we alsso document our assumptions about what 
> functions exist in the standard library — the functions that we'll always use 
> even in freestanding builds?

Sounds like a good idea to me. Do we have a list?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86993

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


[PATCH] D84886: Create LoopNestPass

2020-09-02 Thread Ta-Wei Tu via Phabricator via cfe-commits
TaWeiTu added a comment.

Hi, thanks for your comments and suggestions!

I've thought about the suggestion by @ychen to extend the existing 
`LoopPassManager` to handle loop-nest passes instead of having a separate 
`LoopNestPassManager`, and I've uploaded a new patch D87045 
 that implements the basic functionalities of 
running loop-nest passes in `LoopPassManager`.
I've also added you as reviewers to the new patch, and I will work on the 
follow-up patches if you think that direction is better.

I'll also be working on evaluating the benefits of using loop-nest passes, as 
@fhahn mentioned. But as far as I understand, most passes that may potentially 
benefit from loop-nest passes require certain algorithmic changes that 
incorporate the information of perfectly-nested loops provided by the 
`LoopNest` object first. So it will probably take more time to investigate 
those passes and convert those passes using the new facilities.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84886

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


[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

2020-09-02 Thread Snehasish Kumar via Phabricator via cfe-commits
snehasish created this revision.
snehasish added reviewers: tmsriram, davidxl.
Herald added subscribers: cfe-commits, dang.
Herald added a project: clang.
snehasish requested review of this revision.

This patch adds a command line flag for the machine function splitter
(added in rG94faadaca4e1 
). The 
command line reference has been updated
with the new flag and tests added to check the pass is run (requires
profile data) and that the driver has registered the correct option.

-fsplit-machine-functions
Split machine functions using profile information (x86-elf only)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87047

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/split-machine-functions.c
  clang/test/Driver/fsplit-machine-functions.c

Index: clang/test/Driver/fsplit-machine-functions.c
===
--- /dev/null
+++ clang/test/Driver/fsplit-machine-functions.c
@@ -0,0 +1,4 @@
+// RUN: %clang -### -fsplit-machine-functions %s -c 2>&1 | FileCheck -check-prefix=CHECK-OPT %s
+// RUN: %clang -### -fsplit-machine-functions -fno-split-machine-functions %s -c 2>&1 | FileCheck -check-prefix=CHECK-NOOPT %s
+// CHECK-OPT: "-fsplit-machine-functions"
+// CHECK-NOOPT-NOT: "-fsplit-machine-functions"
Index: clang/test/CodeGen/split-machine-functions.c
===
--- /dev/null
+++ clang/test/CodeGen/split-machine-functions.c
@@ -0,0 +1,34 @@
+// REQUIRES: x86-registered-target
+
+// RUN: echo "foo"> %t.proftext
+// RUN: echo "# Func Hash:"   >> %t.proftext
+// RUN: echo "11262309905">> %t.proftext
+// RUN: echo "# Num Counters:">> %t.proftext
+// RUN: echo "2"  >> %t.proftext
+// RUN: echo "# Counter Values:"  >> %t.proftext
+// RUN: echo "100">> %t.proftext
+// RUN: echo "0"  >> %t.proftext
+// RUN: llvm-profdata merge -o %t.profdata %t.proftext
+// RUN: %clang_cc1 -triple x86_64 -O3 -S -fprofile-instrument-use-path=%t.profdata -fsplit-machine-functions -o - < %s | FileCheck %s
+
+__attribute__((noinline)) int foo(int argc) {
+  if (argc % 2 == 0) {
+exit(argc);
+  } else {
+return argc + 1;
+  }
+}
+
+int main(int argc, char *argv[]) {
+  int total = 0;
+  for (int i = 0; i < 100; ++i) {
+total += foo(argc);
+  }
+  printf("%d\n", total);
+}
+
+// CHECK: .section .text.hot.,"ax",@progbits
+// CHECK: foo:
+// CHECK: section .text.unlikely.foo,"ax",@progbits
+// CHECK: foo.cold:
+// CHECK: callq exit@PLT
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -998,6 +998,8 @@
   Opts.UniqueInternalLinkageNames =
   Args.hasArg(OPT_funique_internal_linkage_names);
 
+  Opts.SplitMachineFunctions = Args.hasArg(OPT_fsplit_machine_functions);
+
   Opts.MergeFunctions = Args.hasArg(OPT_fmerge_functions);
 
   Opts.NoUseJumpTables = Args.hasArg(OPT_fno_jump_tables);
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4255,6 +4255,8 @@
 options::OPT_fno_unique_section_names,
 options::OPT_funique_basic_block_section_names,
 options::OPT_fno_unique_basic_block_section_names,
+options::OPT_fsplit_machine_functions,
+options::OPT_fno_split_machine_functions,
 options::OPT_mrestrict_it,
 options::OPT_mno_restrict_it,
 options::OPT_mstackrealign,
@@ -4905,6 +4907,10 @@
options::OPT_fno_unique_basic_block_section_names, false))
 CmdArgs.push_back("-funique-basic-block-section-names");
 
+  if (Args.hasFlag(options::OPT_fsplit_machine_functions,
+   options::OPT_fno_split_machine_functions, false))
+CmdArgs.push_back("-fsplit-machine-functions");
+
   Args.AddLastArg(CmdArgs, options::OPT_finstrument_functions,
   options::OPT_finstrument_functions_after_inlining,
   options::OPT_finstrument_function_entry_bare);
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -519,6 +519,7 @@
   Options.UniqueSectionNames = CodeGenOpts.UniqueSectionNames;
   Options.UniqueBasicBlockSectionNames =
   CodeGenOpts.UniqueBasicBlockSectionNames;
+  Options.EnableMachineFunctionSplitter = CodeGenOpts.SplitMachineFunctions;
   Options.TLSSize = CodeGenO

[PATCH] D86621: [clang][Sparc] Default to -mcpu=v9 for SparcV8 on Solaris

2020-09-02 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments.



Comment at: clang/lib/Basic/Targets/Sparc.cpp:224
+Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_8");
+  }
 }

ro wrote:
> efriedma wrote:
> > ro wrote:
> > > efriedma wrote:
> > > > ro wrote:
> > > > > efriedma wrote:
> > > > > > This probably should be refactored so the target-independent code 
> > > > > > generates it based on MaxAtomicInlineWidth, instead of duplicating 
> > > > > > it for each target.  But I guess you don't need to do that here.
> > > > > > 
> > > > > > From the other code, the `getCPUGeneration(CPU) == CG_V9` check 
> > > > > > should only guard the definition of 
> > > > > > __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8?
> > > > > > This probably should be refactored so the target-independent code 
> > > > > > generates it based on MaxAtomicInlineWidth, instead of duplicating 
> > > > > > it for each target.  But I guess you don't need to do that here.
> > > > > 
> > > > > Good: one issue at a time ;-)
> > > > > 
> > > > > > From the other code, the `getCPUGeneration(CPU) == CG_V9` check 
> > > > > > should only guard the definition of 
> > > > > > __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8?
> > > > > 
> > > > > I don't think so: at least `gcc` defines none of the four with `-m32 
> > > > > -mcpu=v8` and all with `-m32 -mcpu=v9`.
> > > > This code, the code that sets MaxAtomicInlineWidth, and the code 
> > > > inSPARCISelLowering.cpp that calls setMaxAtomicSizeInBitsSupported() 
> > > > all need to agree about the supported atomic operations.
> > > > 
> > > > I guess the current setting of MaxAtomicInlineWidth is wrong?
> > > I'd say so, yes: gcc -m32 inlines ops on `_Atomic long long` while 
> > > `clang-11 -m32 -mcpu=v9` doesn't.
> > Oh, hmm, it looks like the backend's support for 32-bit v9 is really 
> > limited; we basically generate v8 code, with a couple limited exceptions.  
> > Probably okay to make clang assume 64-bit atomics are actually supported, 
> > even if we don't inline the implementation at the moment; they should still 
> > behave correctly.
> > 
> > I was more wondering about what we do for v8: we set MaxAtomicInlineWidth 
> > to 32, but I don't think it supports atomic cmpxchg at all.
> I've now found [[http://temlib.org/pub/SparcStation/Standards/V8plus.pdf | 
> The V8+ Technical Specification ]].  I've not checked how far LLVM makes use 
> of that, though (gcc seems to be pretty extensive, and it certainly makes use 
> of `casx` for v8plus).
> 
> I'm not really clear on the semantics of `MaxAtomicInlineWidth`: 
> `TargetInfo.h`'s description of `getMaxAtomicInlineWidth` only states
> ```
>   /// Return the maximum width lock-free atomic operation which can be
>   /// inlined given the supported features of the given target.
> ```
> which would be satisfied given that **some** 32-bit atomic ops are inlined 
> for v8.
> 
> 
The definition of C atomic operations requires that we only expose lock-free 
atomic operations on targets that have a lock-free cmpxchg.  This is necessary 
to allow the implementation in libatomic to work: even if an operation is 
"atomic", it wouldn't correctly honor the libatomic locks.

Note that the dynamic component of libatomic allows using lockfree operations 
in a "v8plus" environment. The libatomic implementation should dynamically 
check whether the CPU supports casx, and use lock-free operations if it does.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86621

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


[clang] e0e7eb2 - [clang] Add missing .def files to Clang's modulemap

2020-09-02 Thread Raphael Isemann via cfe-commits

Author: Raphael Isemann
Date: 2020-09-02T20:42:12+02:00
New Revision: e0e7eb2e2648aee83caf2ecfe2972ce2f653d306

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

LOG: [clang] Add missing .def files to Clang's modulemap

These new .def files weren't marked as textual so they ended up being compiled
into the Clang module (which completely defeats the purpose of .def files).

Added: 


Modified: 
clang/include/clang/module.modulemap

Removed: 




diff  --git a/clang/include/clang/module.modulemap 
b/clang/include/clang/module.modulemap
index 13d4dbf9dc2e..6290548b41f1 100644
--- a/clang/include/clang/module.modulemap
+++ b/clang/include/clang/module.modulemap
@@ -52,8 +52,10 @@ module Clang_Basic {
   textual header "Basic/BuiltinsX86_64.def"
   textual header "Basic/BuiltinsXCore.def"
   textual header "Basic/CodeGenOptions.def"
+  textual header "Basic/CommentOptions.def"
   textual header "Basic/DiagnosticOptions.def"
   textual header "Basic/Features.def"
+  textual header "Basic/FileSystemOptions.def"
   textual header "Basic/FPOptions.def"
   textual header "Basic/MSP430Target.def"
   textual header "Basic/LangOptions.def"
@@ -63,6 +65,7 @@ module Clang_Basic {
   textual header "Basic/OpenMPKinds.def"
   textual header "Basic/OperatorKinds.def"
   textual header "Basic/Sanitizers.def"
+  textual header "Basic/TargetOptions.def"
   textual header "Basic/TokenKinds.def"
   textual header "Basic/X86Target.def"
 
@@ -107,17 +110,35 @@ module Clang_Frontend {
   umbrella "Frontend"
 
   textual header "Basic/LangStandards.def"
+  textual header "Frontend/DependencyOutputOptions.def"
+  textual header "Frontend/FrontendOptions.def"
+  textual header "Frontend/MigratorOptions.def"
+  textual header "Frontend/PreprocessorOutputOptions.def"
 
   module * { export * }
 }
 
 module Clang_FrontendTool { requires cplusplus umbrella "FrontendTool" module 
* { export * } }
 module Clang_Index { requires cplusplus umbrella "Index" module * { export * } 
}
-module Clang_Lex { requires cplusplus umbrella "Lex" module * { export * } }
+module Clang_Lex {
+  requires cplusplus
+  umbrella "Lex"
+  textual header "Lex/HeaderSearchOptions.def"
+  textual header "Lex/PreprocessorOptions.def"
+
+  module * { export * }
+}
 module Clang_Parse { requires cplusplus umbrella "Parse" module * { export * } 
}
 module Clang_Rewrite { requires cplusplus umbrella "Rewrite/Core" module * { 
export * } }
 module Clang_RewriteFrontend { requires cplusplus umbrella "Rewrite/Frontend" 
module * { export * } }
-module Clang_Sema { requires cplusplus umbrella "Sema" module * { export * } }
+module Clang_Sema {
+  requires cplusplus
+  umbrella "Sema"
+
+  textual header "Sema/CodeCompleteOptions.def"
+
+  module * { export * }
+}
 
 module Clang_Serialization {
   requires cplusplus



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


[PATCH] D86207: (De-)serialize BindingDecls before DecompositionDecl

2020-09-02 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:585
   D->setLocation(ThisDeclLoc);
   D->setInvalidDecl(Record.readInt());
   if (Record.readInt()) { // hasAttrs

The bug is here: we should not be calling `Decl::setInvalidDecl` here because 
it has invariants, and the `Decl` is incomplete at this point.



Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:1500
   for (unsigned I = 0; I != DD->NumBindings; ++I) {
 BDs[I] = readDeclAs();
 BDs[I]->setDecomposedDecl(DD);

`BindingDecl` might not be fully initialized here: if we enter deserialization 
to deserialize a `BindingDecl`, and then recurse into reading its binding 
expression, and then deserialize the `DecompositionDecl`, we can land here 
before we finish with the `BindingDecl`. If we called something that looked at 
the `Binding` expression on the `BindingDecl`, that'd be a problem.

But the general idea is that deserialization should never invoke AST functions 
with invariants (and generally should set state directly rather than using AST 
member functions in order to avoid accidentally calling a function with an 
invariant). So it shouldn't matter whether we deserialize the 
`DecompositionDecl` or the `BindingDecl`s first.



Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:1509
   BD->Binding = Record.readExpr();
 }
 

Hm, presumably `BindingDecl::Decomp` gets set here because `readExpr` always 
reads an expression that involves the `DecompositionDecl`, which calls 
`setDecomposedDecl`? That seems ... very subtle. If that's the intended way for 
this to work, we should at least add a comment for that (or better, an assert 
that `Decomp` got set), and `BindingDecl::Decomp` should be a regular pointer 
not a `LazyDeclPtr`. (But even then, is this chain of reasoning guaranteed to 
hold even for invalid declarations? Maybe we should be serializing the 
`DecompositionDecl*` and setting `Decomp` here?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86207

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


[PATCH] D87048: [libTooling] Restore defaults for matchers in makeRule.

2020-09-02 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision.
ymandel added a reviewer: gribozavr.
Herald added a project: clang.
ymandel requested review of this revision.

This patch restores the default traversal for Transformer's `makeRule` to
`TK_AsIs`. The implicit mode has proven problematic.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87048

Files:
  clang/lib/Tooling/Transformer/RewriteRule.cpp
  clang/unittests/Tooling/TransformerTest.cpp


Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -878,10 +878,8 @@
 }
 
 // Verifies that a rule with a top-level matcher for an implicit node (like
-// `implicitCastExpr`) does not change the code, when the AST traversal skips
-// implicit nodes. In this test, only the rule with the explicit-node matcher
-// will fire.
-TEST_F(TransformerTest, OrderedRuleImplicitIgnored) {
+// `implicitCastExpr`) works correctly -- the implicit nodes are not skipped.
+TEST_F(TransformerTest, OrderedRuleImplicitMatched) {
   std::string Input = R"cc(
 void f1();
 int f2();
@@ -892,7 +890,7 @@
 void f1();
 int f2();
 void call_f1() { REPLACE_F1; }
-float call_f2() { return f2(); }
+float call_f2() { return REPLACE_F2; }
   )cc";
 
   RewriteRule ReplaceF1 =
@@ -904,32 +902,6 @@
   testRule(applyFirst({ReplaceF1, ReplaceF2}), Input, Expected);
 }
 
-// Verifies that explicitly setting the traversal kind fixes the problem in the
-// previous test.
-TEST_F(TransformerTest, OrderedRuleImplicitMatched) {
-  std::string Input = R"cc(
-void f1();
-int f2();
-void call_f1() { f1(); }
-float call_f2() { return f2(); }
-  )cc";
-  std::string Expected = R"cc(
-void f1();
-int f2();
-void call_f1() { REPLACE_F1; }
-float call_f2() { return REPLACE_F2; }
-  )cc";
-
-  RewriteRule ReplaceF1 = makeRule(
-  traverse(clang::TK_AsIs, callExpr(callee(functionDecl(hasName("f1"),
-  changeTo(cat("REPLACE_F1")));
-  RewriteRule ReplaceF2 =
-  makeRule(traverse(clang::TK_AsIs,
-implicitCastExpr(hasSourceExpression(callExpr(,
-   changeTo(cat("REPLACE_F2")));
-  testRule(applyFirst({ReplaceF1, ReplaceF2}), Input, Expected);
-}
-
 //
 // Negative tests (where we expect no transformation to occur).
 //
Index: clang/lib/Tooling/Transformer/RewriteRule.cpp
===
--- clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -345,14 +345,13 @@
   // Each anyOf explicitly controls the traversal kind. The anyOf itself is set
   // to `TK_AsIs` to ensure no nodes are skipped, thereby deferring to the kind
   // of the branches. Then, each branch is either left as is, if the kind is
-  // already set, or explicitly set to `TK_IgnoreUnlessSpelledInSource`. We
-  // choose this setting, because we think it is the one most friendly to
-  // beginners, who are (largely) the target audience of Transformer.
+  // already set, or explicitly set to `TK_AsIs`. We choose this setting 
because
+  // it is the default interpretation of matchers.
   std::vector Matchers;
   for (const auto &Bucket : Buckets) {
 DynTypedMatcher M = DynTypedMatcher::constructVariadic(
 DynTypedMatcher::VO_AnyOf, Bucket.first,
-taggedMatchers("Tag", Bucket.second, TK_IgnoreUnlessSpelledInSource));
+taggedMatchers("Tag", Bucket.second, TK_AsIs));
 M.setAllowBind(true);
 // `tryBind` is guaranteed to succeed, because `AllowBind` was set to true.
 Matchers.push_back(M.tryBind(RootID)->withTraversalKind(TK_AsIs));


Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -878,10 +878,8 @@
 }
 
 // Verifies that a rule with a top-level matcher for an implicit node (like
-// `implicitCastExpr`) does not change the code, when the AST traversal skips
-// implicit nodes. In this test, only the rule with the explicit-node matcher
-// will fire.
-TEST_F(TransformerTest, OrderedRuleImplicitIgnored) {
+// `implicitCastExpr`) works correctly -- the implicit nodes are not skipped.
+TEST_F(TransformerTest, OrderedRuleImplicitMatched) {
   std::string Input = R"cc(
 void f1();
 int f2();
@@ -892,7 +890,7 @@
 void f1();
 int f2();
 void call_f1() { REPLACE_F1; }
-float call_f2() { return f2(); }
+float call_f2() { return REPLACE_F2; }
   )cc";
 
   RewriteRule ReplaceF1 =
@@ -904,32 +902,6 @@
   testRule(applyFirst({ReplaceF1, ReplaceF2}), Input, Expected);
 }
 
-// Verifies that explicitly setting the traversal kind fixes the problem in the
-// previous test.
-TEST_F(TransformerTest, OrderedRuleImplicitMatched) {
-  std::string Input = R"cc(
-void f1();
-  

[PATCH] D87049: Exploratory patch - capture DebugInfo for constexpr variables used within lambda

2020-09-02 Thread Melanie Blower via Phabricator via cfe-commits
mibintc created this revision.
mibintc added a reviewer: debug-info.
Herald added a project: clang.
mibintc requested review of this revision.

This is an exploratory patch, an attempt to solve 
bugs.llvm.org/show_bug.cgi?id=47400

That bug report shows a simple test case where the constexpr variable, declared 
outside the scope of the lambda but used within the lambda, is simply unknown 
in the DebugInfo.  This is different from a gcc compilation where gdb knows the 
symbol, and knows that the value is "optimized out".

The problem is constexpr variables like n_steps (see example in the bug report) 
are not captured and so do not appear in the record type. These still appear in 
the AST so if we gather them it is possible to emit static data member. As a 
proof of concept, this is prototyped here. I didn't think about exactly which 
variables should be gathered but this might be a framework.  This is a little 
hacky since the constexpr variable  is not really a static member of the lambda 
but the use is roughly correct.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87049

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp


Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -25,6 +25,7 @@
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/RecordLayout.h"
+#include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
@@ -1358,6 +1359,29 @@
offsetInBits, flags, debugType);
 }
 
+namespace {
+/// Locate the non_odr_use constant variables in a statement.
+struct NonODRUseVarFinder final : public ConstStmtVisitor {
+  llvm::SmallSetVector NonODRUseVars;
+
+  NonODRUseVarFinder(const Stmt *S) {
+Visit(S);
+  }
+
+  void VisitStmt(const Stmt *S) {
+for (const Stmt *Child : S->children())
+  if (Child)
+Visit(Child);
+  }
+
+  void VisitDeclRefExpr(const DeclRefExpr *E) {
+if (E->isNonOdrUse() == NOUR_Constant)
+  if (const auto *VD = dyn_cast(E->getDecl()))
+NonODRUseVars.insert(VD);
+  }
+};
+} // namespace
+
 void CGDebugInfo::CollectRecordLambdaFields(
 const CXXRecordDecl *CXXDecl, SmallVectorImpl &elements,
 llvm::DIType *RecordTy) {
@@ -1397,6 +1421,13 @@
   elements.push_back(fieldType);
 }
   }
+  // constexpr variables do not need to be captured, so to view them
+  // in the debugger pretend they are const static data members.
+  CXXMethodDecl *CallOp = CXXDecl->getLambdaCallOperator();
+  assert(CallOp);
+  NonODRUseVarFinder Finder(CallOp->getBody());
+  for (const auto *VD : Finder.NonODRUseVars)
+elements.push_back(CreateRecordStaticField(VD, RecordTy, CXXDecl));
 }
 
 llvm::DIDerivedType *


Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -25,6 +25,7 @@
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/RecordLayout.h"
+#include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
@@ -1358,6 +1359,29 @@
offsetInBits, flags, debugType);
 }
 
+namespace {
+/// Locate the non_odr_use constant variables in a statement.
+struct NonODRUseVarFinder final : public ConstStmtVisitor {
+  llvm::SmallSetVector NonODRUseVars;
+
+  NonODRUseVarFinder(const Stmt *S) {
+Visit(S);
+  }
+
+  void VisitStmt(const Stmt *S) {
+for (const Stmt *Child : S->children())
+  if (Child)
+Visit(Child);
+  }
+
+  void VisitDeclRefExpr(const DeclRefExpr *E) {
+if (E->isNonOdrUse() == NOUR_Constant)
+  if (const auto *VD = dyn_cast(E->getDecl()))
+NonODRUseVars.insert(VD);
+  }
+};
+} // namespace
+
 void CGDebugInfo::CollectRecordLambdaFields(
 const CXXRecordDecl *CXXDecl, SmallVectorImpl &elements,
 llvm::DIType *RecordTy) {
@@ -1397,6 +1421,13 @@
   elements.push_back(fieldType);
 }
   }
+  // constexpr variables do not need to be captured, so to view them
+  // in the debugger pretend they are const static data members.
+  CXXMethodDecl *CallOp = CXXDecl->getLambdaCallOperator();
+  assert(CallOp);
+  NonODRUseVarFinder Finder(CallOp->getBody());
+  for (const auto *VD : Finder.NonODRUseVars)
+elements.push_back(CreateRecordStaticField(VD, RecordTy, CXXDecl));
 }
 
 llvm::DIDerivedType *
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D86621: [clang][Sparc] Default to -mcpu=v9 for SparcV8 on Solaris

2020-09-02 Thread Rainer Orth via Phabricator via cfe-commits
ro added inline comments.



Comment at: clang/lib/Basic/Targets/Sparc.cpp:224
+Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_8");
+  }
 }

efriedma wrote:
> ro wrote:
> > efriedma wrote:
> > > ro wrote:
> > > > efriedma wrote:
> > > > > ro wrote:
> > > > > > efriedma wrote:
> > > > > > > This probably should be refactored so the target-independent code 
> > > > > > > generates it based on MaxAtomicInlineWidth, instead of 
> > > > > > > duplicating it for each target.  But I guess you don't need to do 
> > > > > > > that here.
> > > > > > > 
> > > > > > > From the other code, the `getCPUGeneration(CPU) == CG_V9` check 
> > > > > > > should only guard the definition of 
> > > > > > > __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8?
> > > > > > > This probably should be refactored so the target-independent code 
> > > > > > > generates it based on MaxAtomicInlineWidth, instead of 
> > > > > > > duplicating it for each target.  But I guess you don't need to do 
> > > > > > > that here.
> > > > > > 
> > > > > > Good: one issue at a time ;-)
> > > > > > 
> > > > > > > From the other code, the `getCPUGeneration(CPU) == CG_V9` check 
> > > > > > > should only guard the definition of 
> > > > > > > __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8?
> > > > > > 
> > > > > > I don't think so: at least `gcc` defines none of the four with 
> > > > > > `-m32 -mcpu=v8` and all with `-m32 -mcpu=v9`.
> > > > > This code, the code that sets MaxAtomicInlineWidth, and the code 
> > > > > inSPARCISelLowering.cpp that calls setMaxAtomicSizeInBitsSupported() 
> > > > > all need to agree about the supported atomic operations.
> > > > > 
> > > > > I guess the current setting of MaxAtomicInlineWidth is wrong?
> > > > I'd say so, yes: gcc -m32 inlines ops on `_Atomic long long` while 
> > > > `clang-11 -m32 -mcpu=v9` doesn't.
> > > Oh, hmm, it looks like the backend's support for 32-bit v9 is really 
> > > limited; we basically generate v8 code, with a couple limited exceptions. 
> > >  Probably okay to make clang assume 64-bit atomics are actually 
> > > supported, even if we don't inline the implementation at the moment; they 
> > > should still behave correctly.
> > > 
> > > I was more wondering about what we do for v8: we set MaxAtomicInlineWidth 
> > > to 32, but I don't think it supports atomic cmpxchg at all.
> > I've now found [[http://temlib.org/pub/SparcStation/Standards/V8plus.pdf | 
> > The V8+ Technical Specification ]].  I've not checked how far LLVM makes 
> > use of that, though (gcc seems to be pretty extensive, and it certainly 
> > makes use of `casx` for v8plus).
> > 
> > I'm not really clear on the semantics of `MaxAtomicInlineWidth`: 
> > `TargetInfo.h`'s description of `getMaxAtomicInlineWidth` only states
> > ```
> >   /// Return the maximum width lock-free atomic operation which can be
> >   /// inlined given the supported features of the given target.
> > ```
> > which would be satisfied given that **some** 32-bit atomic ops are inlined 
> > for v8.
> > 
> > 
> The definition of C atomic operations requires that we only expose lock-free 
> atomic operations on targets that have a lock-free cmpxchg.  This is 
> necessary to allow the implementation in libatomic to work: even if an 
> operation is "atomic", it wouldn't correctly honor the libatomic locks.
> 
> Note that the dynamic component of libatomic allows using lockfree operations 
> in a "v8plus" environment. The libatomic implementation should dynamically 
> check whether the CPU supports casx, and use lock-free operations if it does.
Honestly, I don't see this as my job, though: the issue is pre-existing, it 
doesn't affect any of my targets and I see that GCC's libatomic forces 
`-mcpu=v9` for the 32-bit sparc library.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86621

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


[clang] 2d11ae0 - Fix a -Wparenthesis warning in 8ff44e644bb7, NFC

2020-09-02 Thread Erik Pilkington via cfe-commits

Author: Erik Pilkington
Date: 2020-09-02T15:01:54-04:00
New Revision: 2d11ae0a40e209a7b91aeff0c9cf28fe41dce93c

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

LOG: Fix a -Wparenthesis warning in 8ff44e644bb7, NFC

Added: 


Modified: 
clang/lib/CodeGen/CodeGenModule.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 1f362e2b6b31..3ecc8743265c 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1989,7 +1989,7 @@ void CodeGenModule::SetFunctionAttributes(GlobalDecl GD, 
llvm::Function *F,
 }
 
 void CodeGenModule::addUsedGlobal(llvm::GlobalValue *GV) {
-  assert(isa(GV) || !GV->isDeclaration() &&
+  assert((isa(GV) || !GV->isDeclaration()) &&
  "Only globals with definition can force usage.");
   LLVMUsed.emplace_back(GV);
 }



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


[PATCH] D76323: [AST] Fix handling of long double and bool in __builtin_bit_cast

2020-09-02 Thread Erik Pilkington via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9523cf02c22a: [AST] Fix handling of long double and bool in 
__builtin_bit_cast (authored by erik.pilkington).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76323

Files:
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp

Index: clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp
===
--- clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp
+++ clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp
@@ -23,6 +23,10 @@
 template 
 constexpr To bit_cast(const From &from) {
   static_assert(sizeof(To) == sizeof(From));
+  // expected-note@+9 {{cannot be represented in type 'bool'}}
+#ifdef __x86_64
+  // expected-note@+7 {{or 'std::byte'; '__int128' is invalid}}
+#endif
 #ifdef __CHAR_UNSIGNED__
   // expected-note@+4 2 {{indeterminate value can only initialize an object of type 'unsigned char', 'char', or 'std::byte'; 'signed char' is invalid}}
 #else
@@ -397,3 +401,65 @@
 };
 constexpr IdentityInUnion identity3a = {42};
 constexpr unsigned char identity3b = __builtin_bit_cast(unsigned char, identity3a.n);
+
+namespace test_bool {
+
+constexpr bool test_bad_bool = bit_cast('A'); // expected-error {{must be initialized by a constant expression}} expected-note{{in call}}
+
+static_assert(round_trip(true), "");
+static_assert(round_trip(false), "");
+static_assert(round_trip(false), "");
+
+static_assert(round_trip((char)0), "");
+static_assert(round_trip((char)1), "");
+}
+
+namespace test_long_double {
+#ifdef __x86_64
+constexpr __int128_t test_cast_to_int128 = bit_cast<__int128_t>((long double)0); // expected-error{{must be initialized by a constant expression}} expected-note{{in call}}
+
+constexpr long double ld = 3.1425926539;
+
+struct bytes {
+  unsigned char d[16];
+};
+
+static_assert(round_trip(ld), "");
+
+static_assert(round_trip(10.0L));
+
+constexpr bool f(bool read_uninit) {
+  bytes b = bit_cast(ld);
+  unsigned char ld_bytes[10] = {
+0x0,  0x48, 0x9f, 0x49, 0xf0,
+0x3c, 0x20, 0xc9, 0x0,  0x40,
+  };
+
+  for (int i = 0; i != 10; ++i)
+if (ld_bytes[i] != b.d[i])
+  return false;
+
+  if (read_uninit && b.d[10]) // expected-note{{read of uninitialized object is not allowed in a constant expression}}
+return false;
+
+  return true;
+}
+
+static_assert(f(/*read_uninit=*/false), "");
+static_assert(f(/*read_uninit=*/true), ""); // expected-error{{static_assert expression is not an integral constant expression}} expected-note{{in call to 'f(true)'}}
+
+constexpr bytes ld539 = {
+  0x0, 0x0,  0x0,  0x0,
+  0x0, 0x0,  0xc0, 0x86,
+  0x8, 0x40, 0x0,  0x0,
+  0x0, 0x0,  0x0,  0x0,
+};
+
+constexpr long double fivehundredandthirtynine = 539.0;
+
+static_assert(bit_cast(ld539) == fivehundredandthirtynine, "");
+
+#else
+static_assert(round_trip<__int128_t>(34.0L));
+#endif
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -6627,9 +6627,15 @@
   }
 
   bool visitInt(const APSInt &Val, QualType Ty, CharUnits Offset) {
-CharUnits Width = Info.Ctx.getTypeSizeInChars(Ty);
-SmallVector Bytes(Width.getQuantity());
-llvm::StoreIntToMemory(Val, &*Bytes.begin(), Width.getQuantity());
+APSInt AdjustedVal = Val;
+unsigned Width = AdjustedVal.getBitWidth();
+if (Ty->isBooleanType()) {
+  Width = Info.Ctx.getTypeSize(Ty);
+  AdjustedVal = AdjustedVal.extend(Width);
+}
+
+SmallVector Bytes(Width / 8);
+llvm::StoreIntToMemory(AdjustedVal, &*Bytes.begin(), Width / 8);
 Buffer.writeObject(Offset, Bytes);
 return true;
   }
@@ -6670,6 +6676,13 @@
 return None;
   }
 
+  llvm::NoneType unrepresentableValue(QualType Ty, const APSInt &Val) {
+Info.FFDiag(BCE->getBeginLoc(),
+diag::note_constexpr_bit_cast_unrepresentable_value)
+<< Ty << Val.toString(/*Radix=*/10);
+return None;
+  }
+
   Optional visit(const BuiltinType *T, CharUnits Offset,
   const EnumType *EnumSugar = nullptr) {
 if (T->isNullPtrType()) {
@@ -6680,6 +6693,20 @@
 }
 
 CharUnits SizeOf = Info.Ctx.getTypeSizeInChars(T);
+
+// Work around floating point types that contain unused padding bytes. This
+// is really just `long double` on x86, which is the only fundamental type
+// with padding bytes.
+if (T->isRealFloatingType()) {
+  const llvm::fltSemantics &Semantics =
+  Info.Ctx.getFloatTypeSemantics(QualType(T, 0));
+  unsigned NumBits = llvm::APFloatBase::getSizeInBits(Semantics);
+  assert(NumBits % 8 == 0);
+  CharUnits NumBytes = CharUnits::fromQuantity(NumBits / 8);
+  if (NumBytes != SizeOf)
+   

[clang] 9523cf0 - [AST] Fix handling of long double and bool in __builtin_bit_cast

2020-09-02 Thread Erik Pilkington via cfe-commits

Author: Erik Pilkington
Date: 2020-09-02T15:01:53-04:00
New Revision: 9523cf02c22a83bece8d81080693a0cbf4098bb5

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

LOG: [AST] Fix handling of long double and bool in __builtin_bit_cast

On x86, long double has 6 unused trailing bytes. This patch changes the
constant evaluator to treat them as though they were padding bytes, so reading
from them results in an indeterminate value, and nothing is written for them.
Also, fix a similar bug with bool, but instead of treating the unused bits as
padding, enforce that they're zero.

Differential revision: https://reviews.llvm.org/D76323

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticASTKinds.td
clang/lib/AST/ExprConstant.cpp
clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticASTKinds.td 
b/clang/include/clang/Basic/DiagnosticASTKinds.td
index 9be75f375119..6a9ff309e49c 100644
--- a/clang/include/clang/Basic/DiagnosticASTKinds.td
+++ b/clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -298,6 +298,8 @@ def note_constexpr_bit_cast_invalid_subtype : Note<
 def note_constexpr_bit_cast_indet_dest : Note<
   "indeterminate value can only initialize an object of type 'unsigned char'"
   "%select{, 'char',|}1 or 'std::byte'; %0 is invalid">;
+def note_constexpr_bit_cast_unrepresentable_value : Note<
+  "value %1 cannot be represented in type %0">;
 def note_constexpr_pseudo_destructor : Note<
   "pseudo-destructor call is not permitted in constant expressions "
   "until C++20">;

diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 014c48e6f08f..e8f132dd4803 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -6627,9 +6627,15 @@ class APValueToBufferConverter {
   }
 
   bool visitInt(const APSInt &Val, QualType Ty, CharUnits Offset) {
-CharUnits Width = Info.Ctx.getTypeSizeInChars(Ty);
-SmallVector Bytes(Width.getQuantity());
-llvm::StoreIntToMemory(Val, &*Bytes.begin(), Width.getQuantity());
+APSInt AdjustedVal = Val;
+unsigned Width = AdjustedVal.getBitWidth();
+if (Ty->isBooleanType()) {
+  Width = Info.Ctx.getTypeSize(Ty);
+  AdjustedVal = AdjustedVal.extend(Width);
+}
+
+SmallVector Bytes(Width / 8);
+llvm::StoreIntToMemory(AdjustedVal, &*Bytes.begin(), Width / 8);
 Buffer.writeObject(Offset, Bytes);
 return true;
   }
@@ -6670,6 +6676,13 @@ class BufferToAPValueConverter {
 return None;
   }
 
+  llvm::NoneType unrepresentableValue(QualType Ty, const APSInt &Val) {
+Info.FFDiag(BCE->getBeginLoc(),
+diag::note_constexpr_bit_cast_unrepresentable_value)
+<< Ty << Val.toString(/*Radix=*/10);
+return None;
+  }
+
   Optional visit(const BuiltinType *T, CharUnits Offset,
   const EnumType *EnumSugar = nullptr) {
 if (T->isNullPtrType()) {
@@ -6680,6 +6693,20 @@ class BufferToAPValueConverter {
 }
 
 CharUnits SizeOf = Info.Ctx.getTypeSizeInChars(T);
+
+// Work around floating point types that contain unused padding bytes. This
+// is really just `long double` on x86, which is the only fundamental type
+// with padding bytes.
+if (T->isRealFloatingType()) {
+  const llvm::fltSemantics &Semantics =
+  Info.Ctx.getFloatTypeSemantics(QualType(T, 0));
+  unsigned NumBits = llvm::APFloatBase::getSizeInBits(Semantics);
+  assert(NumBits % 8 == 0);
+  CharUnits NumBytes = CharUnits::fromQuantity(NumBits / 8);
+  if (NumBytes != SizeOf)
+SizeOf = NumBytes;
+}
+
 SmallVector Bytes;
 if (!Buffer.readObject(Offset, SizeOf, Bytes)) {
   // If this is std::byte or unsigned char, then its okay to store an
@@ -6704,6 +6731,15 @@ class BufferToAPValueConverter {
 
 if (T->isIntegralOrEnumerationType()) {
   Val.setIsSigned(T->isSignedIntegerOrEnumerationType());
+
+  unsigned IntWidth = Info.Ctx.getIntWidth(QualType(T, 0));
+  if (IntWidth != Val.getBitWidth()) {
+APSInt Truncated = Val.trunc(IntWidth);
+if (Truncated.extend(Val.getBitWidth()) != Val)
+  return unrepresentableValue(QualType(T, 0), Val);
+Val = Truncated;
+  }
+
   return APValue(Val);
 }
 

diff  --git a/clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp 
b/clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp
index 06771f8f3252..5b5d1cb7bc80 100644
--- a/clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp
+++ b/clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp
@@ -23,6 +23,10 @@ static_assert(sizeof(long long) == 8);
 template 
 constexpr To bit_cast(const From &from) {
   static_assert(sizeof(To) == sizeof(From));
+  // expected-note@+9 {{cannot be represented i

[PATCH] D86290: Move all fields of '-cc1' option related classes into def file databases

2020-09-02 Thread Jordan Rupprecht via Phabricator via cfe-commits
rupprecht added a comment.

Thanks for the revert.

In addition to the one eabi test, we saw widespread msan 
use-of-uninitialized-value errors from here: 
https://github.com/llvm/llvm-project/blob/master/llvm/lib/Target/ARM/ARMTargetMachine.cpp#L229.
 I think it would explain the eabi test failure.

See also: 
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-msan/builds/21663/steps/check-clang%20msan/logs/stdio


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86290

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


[PATCH] D87051: scan-build-py: fix multiprocessing error

2020-09-02 Thread Lawrence D'Anna via Phabricator via cfe-commits
lawrence_danna created this revision.
lawrence_danna added reviewers: ldionne, chandlerc, jasonmolenda, JDevlieghere.
Herald added subscribers: cfe-commits, dexonsmith, whisperity.
Herald added a project: clang.
lawrence_danna requested review of this revision.

Recent versions of python3's multiprocessing module will blow up with
a Runtime error from this code, saying:

  An attempt has been made to start a new process before the
  current process has finished its bootstrapping phase

This is becuae the wrappers in bin/ are not using the __name__

"__main__" idiom correctly.
---


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87051

Files:
  clang/tools/scan-build-py/bin/analyze-build
  clang/tools/scan-build-py/bin/intercept-build
  clang/tools/scan-build-py/bin/scan-build


Index: clang/tools/scan-build-py/bin/scan-build
===
--- clang/tools/scan-build-py/bin/scan-build
+++ clang/tools/scan-build-py/bin/scan-build
@@ -5,12 +5,13 @@
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 import multiprocessing
-multiprocessing.freeze_support()
-
 import sys
 import os.path
 this_dir = os.path.dirname(os.path.realpath(__file__))
 sys.path.append(os.path.dirname(this_dir))
 
 from libscanbuild.analyze import scan_build
-sys.exit(scan_build())
+
+if __name__ == '__main__':
+multiprocessing.freeze_support()
+sys.exit(scan_build())
Index: clang/tools/scan-build-py/bin/intercept-build
===
--- clang/tools/scan-build-py/bin/intercept-build
+++ clang/tools/scan-build-py/bin/intercept-build
@@ -5,12 +5,13 @@
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 import multiprocessing
-multiprocessing.freeze_support()
-
 import sys
 import os.path
 this_dir = os.path.dirname(os.path.realpath(__file__))
 sys.path.append(os.path.dirname(this_dir))
 
 from libscanbuild.intercept import intercept_build
-sys.exit(intercept_build())
+
+if __name__ == '__main__':
+multiprocessing.freeze_support()
+sys.exit(intercept_build())
Index: clang/tools/scan-build-py/bin/analyze-build
===
--- clang/tools/scan-build-py/bin/analyze-build
+++ clang/tools/scan-build-py/bin/analyze-build
@@ -5,12 +5,13 @@
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 import multiprocessing
-multiprocessing.freeze_support()
-
 import sys
 import os.path
 this_dir = os.path.dirname(os.path.realpath(__file__))
 sys.path.append(os.path.dirname(this_dir))
 
 from libscanbuild.analyze import analyze_build
-sys.exit(analyze_build())
+
+if __name__ == '__main__':
+multiprocessing.freeze_support()
+sys.exit(analyze_build())


Index: clang/tools/scan-build-py/bin/scan-build
===
--- clang/tools/scan-build-py/bin/scan-build
+++ clang/tools/scan-build-py/bin/scan-build
@@ -5,12 +5,13 @@
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 import multiprocessing
-multiprocessing.freeze_support()
-
 import sys
 import os.path
 this_dir = os.path.dirname(os.path.realpath(__file__))
 sys.path.append(os.path.dirname(this_dir))
 
 from libscanbuild.analyze import scan_build
-sys.exit(scan_build())
+
+if __name__ == '__main__':
+multiprocessing.freeze_support()
+sys.exit(scan_build())
Index: clang/tools/scan-build-py/bin/intercept-build
===
--- clang/tools/scan-build-py/bin/intercept-build
+++ clang/tools/scan-build-py/bin/intercept-build
@@ -5,12 +5,13 @@
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 import multiprocessing
-multiprocessing.freeze_support()
-
 import sys
 import os.path
 this_dir = os.path.dirname(os.path.realpath(__file__))
 sys.path.append(os.path.dirname(this_dir))
 
 from libscanbuild.intercept import intercept_build
-sys.exit(intercept_build())
+
+if __name__ == '__main__':
+multiprocessing.freeze_support()
+sys.exit(intercept_build())
Index: clang/tools/scan-build-py/bin/analyze-build
===
--- clang/tools/scan-build-py/bin/analyze-build
+++ clang/tools/scan-build-py/bin/analyze-build
@@ -5,12 +5,13 @@
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 import multiprocessing
-multiprocessing.freeze_support()
-
 import sys
 import os.path
 this_dir = os.path.dirname(os.path.realpath(__file__))
 sys.path.append(os.path.dirname(this_dir))
 
 from libscanbuild.analyze import analyze_build
-sys.exit(analyze_build())
+
+if __name__ == '__main__':
+multiprocessing.freeze_support()
+sys.exit(analyze_build())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D83955: [PowerPC][Power10] Implementation of 128-bit Binary Vector Multiply builtins

2020-09-02 Thread Albion Fung 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 rG5d1fe3f903b9: [PowerPC] Implemented Vector Multiply Builtins 
(authored by Conanap).

Changed prior to commit:
  https://reviews.llvm.org/D83955?vs=279637&id=289537#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83955

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/builtins-ppc-p10vector.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCInstrPrefix.td
  llvm/test/CodeGen/PowerPC/p10-vector-multiply.ll

Index: llvm/test/CodeGen/PowerPC/p10-vector-multiply.ll
===
--- llvm/test/CodeGen/PowerPC/p10-vector-multiply.ll
+++ llvm/test/CodeGen/PowerPC/p10-vector-multiply.ll
@@ -10,6 +10,7 @@
 ; This includes the low order and high order versions of vector multiply.
 ; The low order version operates on doublewords, whereas the high order version
 ; operates on signed and unsigned words and doublewords.
+; This file also includes 128 bit vector multiply instructions.
 
 define <2 x i64> @test_vmulld(<2 x i64> %a, <2 x i64> %b) {
 ; CHECK-LABEL: test_vmulld:
@@ -122,3 +123,54 @@
   %mulh = tail call <2 x i64> @llvm.ppc.altivec.vmulhud(<2 x i64> %a, <2 x i64> %b)
   ret <2 x i64> %mulh
 }
+
+declare <1 x i128> @llvm.ppc.altivec.vmuleud(<2 x i64>, <2 x i64>) nounwind readnone
+declare <1 x i128> @llvm.ppc.altivec.vmuloud(<2 x i64>, <2 x i64>) nounwind readnone
+declare <1 x i128> @llvm.ppc.altivec.vmulesd(<2 x i64>, <2 x i64>) nounwind readnone
+declare <1 x i128> @llvm.ppc.altivec.vmulosd(<2 x i64>, <2 x i64>) nounwind readnone
+declare <1 x i128> @llvm.ppc.altivec.vmsumcud(<2 x i64>, <2 x i64>, <1 x i128>) nounwind readnone
+
+define <1 x i128> @test_vmuleud(<2 x i64> %x, <2 x i64> %y) nounwind readnone {
+; CHECK-LABEL: test_vmuleud:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:vmuleud v2, v2, v3
+; CHECK-NEXT:blr
+  %tmp = tail call <1 x i128> @llvm.ppc.altivec.vmuleud(<2 x i64> %x, <2 x i64> %y)
+  ret <1 x i128> %tmp
+}
+
+define <1 x i128> @test_vmuloud(<2 x i64> %x, <2 x i64> %y) nounwind readnone {
+; CHECK-LABEL: test_vmuloud:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:vmuloud v2, v2, v3
+; CHECK-NEXT:blr
+  %tmp = tail call <1 x i128> @llvm.ppc.altivec.vmuloud(<2 x i64> %x, <2 x i64> %y)
+  ret <1 x i128> %tmp
+}
+
+define <1 x i128> @test_vmulesd(<2 x i64> %x, <2 x i64> %y) nounwind readnone {
+; CHECK-LABEL: test_vmulesd:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:vmulesd v2, v2, v3
+; CHECK-NEXT:blr
+  %tmp = tail call <1 x i128> @llvm.ppc.altivec.vmulesd(<2 x i64> %x, <2 x i64> %y)
+  ret <1 x i128> %tmp
+}
+
+define <1 x i128> @test_vmulosd(<2 x i64> %x, <2 x i64> %y) nounwind readnone {
+; CHECK-LABEL: test_vmulosd:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:vmulosd v2, v2, v3
+; CHECK-NEXT:blr
+  %tmp = tail call <1 x i128> @llvm.ppc.altivec.vmulosd(<2 x i64> %x, <2 x i64> %y)
+  ret <1 x i128> %tmp
+}
+
+define <1 x i128> @test_vmsumcud(<2 x i64> %x, <2 x i64> %y, <1 x i128> %z) nounwind readnone {
+; CHECK-LABEL: test_vmsumcud:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:vmsumcud v2, v2, v3, v4
+; CHECK-NEXT:blr
+  %tmp = tail call <1 x i128> @llvm.ppc.altivec.vmsumcud(<2 x i64> %x, <2 x i64> %y, <1 x i128> %z)
+  ret <1 x i128> %tmp
+}
Index: llvm/lib/Target/PowerPC/PPCInstrPrefix.td
===
--- llvm/lib/Target/PowerPC/PPCInstrPrefix.td
+++ llvm/lib/Target/PowerPC/PPCInstrPrefix.td
@@ -1256,16 +1256,25 @@
   }
 
   def VMULESD : VXForm_1<968, (outs vrrc:$vD), (ins vrrc:$vA, vrrc:$vB),
- "vmulesd $vD, $vA, $vB", IIC_VecGeneral, []>;
+ "vmulesd $vD, $vA, $vB", IIC_VecGeneral,
+ [(set v1i128:$vD, (int_ppc_altivec_vmulesd v2i64:$vA,
+   v2i64:$vB))]>;
   def VMULEUD : VXForm_1<712, (outs vrrc:$vD), (ins vrrc:$vA, vrrc:$vB),
- "vmuleud $vD, $vA, $vB", IIC_VecGeneral, []>;
+ "vmuleud $vD, $vA, $vB", IIC_VecGeneral,
+ [(set v1i128:$vD, (int_ppc_altivec_vmuleud v2i64:$vA,
+   v2i64:$vB))]>;
   def VMULOSD : VXForm_1<456, (outs vrrc:$vD), (ins vrrc:$vA, vrrc:$vB),
- "vmulosd $vD, $vA, $vB", IIC_VecGeneral, []>;
+ "vmulosd $vD, $vA, $vB", IIC_VecGeneral,
+ [(set v1i128:$vD, (int_ppc_altivec_vmulosd v2i64:$vA,
+   v2i64:$vB))]>;
   def VMULOUD : VXForm_1<200, (outs vrrc:$vD), (ins vrrc:$vA, vrrc:$vB),
- "vmuloud $vD, $vA, $vB", IIC_VecGeneral, []>;
-  def VMSUMCUD : VAForm_1a<23, (outs vrrc:$vD),
-   (ins vrrc:$vA, vrr

[clang] 5d1fe3f - [PowerPC] Implemented Vector Multiply Builtins

2020-09-02 Thread Albion Fung via cfe-commits

Author: Albion Fung
Date: 2020-09-02T14:16:21-05:00
New Revision: 5d1fe3f903b9f46b994956f3b214305be119c4e2

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

LOG: [PowerPC] Implemented Vector Multiply Builtins

This patch implements the builtins for Vector Multiply Builtins (vmulxxd family 
of instructions), and adds the appropriate test cases for these builtins. The 
builtins utilize the vector multiply instructions itnroduced with ISA 3.1.

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

Added: 


Modified: 
clang/include/clang/Basic/BuiltinsPPC.def
clang/lib/Headers/altivec.h
clang/test/CodeGen/builtins-ppc-p10vector.c
llvm/include/llvm/IR/IntrinsicsPowerPC.td
llvm/lib/Target/PowerPC/PPCInstrPrefix.td
llvm/test/CodeGen/PowerPC/p10-vector-multiply.ll

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsPPC.def 
b/clang/include/clang/Basic/BuiltinsPPC.def
index b9824588939b..57ef39980c9b 100644
--- a/clang/include/clang/Basic/BuiltinsPPC.def
+++ b/clang/include/clang/Basic/BuiltinsPPC.def
@@ -100,6 +100,11 @@ BUILTIN(__builtin_altivec_vmulouh, "V4UiV8UsV8Us", "")
 BUILTIN(__builtin_altivec_vmulosh, "V4SiV8SsV8Ss", "")
 BUILTIN(__builtin_altivec_vmulouw, "V2ULLiV4UiV4Ui", "")
 BUILTIN(__builtin_altivec_vmulosw, "V2SLLiV4SiV4Si", "")
+BUILTIN(__builtin_altivec_vmuleud, "V1ULLLiV2ULLiV2ULLi", "")
+BUILTIN(__builtin_altivec_vmulesd, "V1SLLLiV2SLLiV2SLLi", "")
+BUILTIN(__builtin_altivec_vmuloud, "V1ULLLiV2ULLiV2ULLi", "")
+BUILTIN(__builtin_altivec_vmulosd, "V1SLLLiV2SLLiV2SLLi", "")
+BUILTIN(__builtin_altivec_vmsumcud, "V1ULLLiV2ULLiV2ULLiV1ULLLi", "")
 
 BUILTIN(__builtin_altivec_vnmsubfp, "V4fV4fV4fV4f", "")
 

diff  --git a/clang/lib/Headers/altivec.h b/clang/lib/Headers/altivec.h
index 927f25751664..47119d702683 100644
--- a/clang/lib/Headers/altivec.h
+++ b/clang/lib/Headers/altivec.h
@@ -5487,6 +5487,16 @@ vec_msum(vector unsigned short __a, vector unsigned 
short __b,
   return __builtin_altivec_vmsumuhm(__a, __b, __c);
 }
 
+/* vec_msumc */
+
+#ifdef __POWER10_VECTOR__
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_msumc(vector unsigned long long __a, vector unsigned long long __b,
+  vector unsigned __int128 __c) {
+  return __builtin_altivec_vmsumcud(__a, __b, __c);
+}
+#endif
+
 /* vec_vmsummbm */
 
 static __inline__ vector int __attribute__((__always_inline__))
@@ -5713,6 +5723,26 @@ vec_mule(vector unsigned int __a, vector unsigned int 
__b) {
 }
 #endif
 
+#ifdef __POWER10_VECTOR__
+static __inline__ vector signed __int128 __ATTRS_o_ai
+vec_mule(vector signed long long __a, vector signed long long __b) {
+#ifdef __LITTLE_ENDIAN__
+  return __builtin_altivec_vmulosd(__a, __b);
+#else
+  return __builtin_altivec_vmulesd(__a, __b);
+#endif
+}
+
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_mule(vector unsigned long long __a, vector unsigned long long __b) {
+#ifdef __LITTLE_ENDIAN__
+  return __builtin_altivec_vmuloud(__a, __b);
+#else
+  return __builtin_altivec_vmuleud(__a, __b);
+#endif
+}
+#endif
+
 /* vec_vmulesb */
 
 static __inline__ vector short __attribute__((__always_inline__))
@@ -5839,6 +5869,26 @@ vec_mulo(vector unsigned int __a, vector unsigned int 
__b) {
 }
 #endif
 
+#ifdef __POWER10_VECTOR__
+static __inline__ vector signed __int128 __ATTRS_o_ai
+vec_mulo(vector signed long long __a, vector signed long long __b) {
+#ifdef __LITTLE_ENDIAN__
+  return __builtin_altivec_vmulesd(__a, __b);
+#else
+  return __builtin_altivec_vmulosd(__a, __b);
+#endif
+}
+
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_mulo(vector unsigned long long __a, vector unsigned long long __b) {
+#ifdef __LITTLE_ENDIAN__
+  return __builtin_altivec_vmuleud(__a, __b);
+#else
+  return __builtin_altivec_vmuloud(__a, __b);
+#endif
+}
+#endif
+
 /* vec_vmulosb */
 
 static __inline__ vector short __attribute__((__always_inline__))

diff  --git a/clang/test/CodeGen/builtins-ppc-p10vector.c 
b/clang/test/CodeGen/builtins-ppc-p10vector.c
index 6fe6d9fdf72d..ac766e264b2d 100644
--- a/clang/test/CodeGen/builtins-ppc-p10vector.c
+++ b/clang/test/CodeGen/builtins-ppc-p10vector.c
@@ -928,6 +928,44 @@ int test_vec_test_lsbb_all_zeros(void) {
   return vec_test_lsbb_all_zeros(vuca);
 }
 
+vector unsigned __int128 test_vec_mule_u128(void) {
+  // CHECK-BE: @llvm.ppc.altivec.vmuleud(<2 x i64>
+  // CHECK-BE-NEXT: ret <1 x i128>
+  // CHECK-LE: @llvm.ppc.altivec.vmuloud(<2 x i64>
+  // CHECK-LE-NEXT: ret <1 x i128>
+  return vec_mule(vulla, vullb);
+}
+
+vector signed __int128 test_vec_mule_s128(void) {
+  // CHECK-BE: @llvm.ppc.altivec.vmulesd(<2 x i64>
+  // CHECK-BE-NEXT: ret <1 x i128>
+  // CHECK-LE: @llvm.ppc.altivec.vmulosd(<2 x i64>
+  // CHECK-LE-NEXT: ret <1 x i128>
+  return vec_mule(vslla, vsll

[PATCH] D87047: [clang] Add command line options for the Machine Function Splitter.

2020-09-02 Thread David Li via Phabricator via cfe-commits
davidxl added a comment.

For x86 target, should it be turned on when -fprofile-use= option is specified 
unless -fno-split-machine-function is specified?

Also is it worth to give a warning if the option is specified but PGO is not on?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87047

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


[PATCH] D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt

2020-09-02 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/include/clang/AST/Stmt.h:1105-
+  /// The likelihood of a branch being taken.
+  enum Likelihood {
+LH_None, ///< No attribute set.
+LH_Likely,   ///< Branch has the [[likely]] attribute.
+LH_Unlikely, ///< Branch has the [[unlikely]] attribute.
+LH_Conflict  ///< Both branches of the IfStmt have the same attribute.
+  };

I'd suggest you order these in increasing order of likeliness: unlikely, none, 
likely. Then you can determine what branch weights to use by a three-way 
comparison of the likeliness of the `then` branch and the likeliness of the 
`else` branch.



Comment at: clang/lib/Sema/SemaStmt.cpp:578
+static std::pair
+getLikelihood(const Stmt *Stmt) {
+  if (const auto *AS = dyn_cast(Stmt))

Mordante wrote:
> aaron.ballman wrote:
> > rsmith wrote:
> > > Sema doesn't care about any of this; can you move this code to CodeGen 
> > > and remove the code that stores likelihood data in the AST?
> > FWIW, I asked for it to be moved out of CodeGen and into Sema because the 
> > original implementation in CodeGen was doing a fair amount of work trying 
> > to interrogate the AST for this information. Now that we've switched the 
> > design to only be on the substatement of an if/else statement (rather than 
> > an arbitrary statement), it may be that CodeGen is a better place for this 
> > again (and if so, I'm sorry for the churn).
> At the moment the Sema cares about it to generate diagnostics about 
> conflicting annotations:
> ```
> if(i) [[ likely]] {}
> else [[likely]] {}
> ```
> This diagnostic only happens for an if statement, for a switch multiple 
> values can be considered likely.
> Do you prefer to have the diagnostic also in the CodeGen?
It looked to me like you'd reached agreement to remove that diagnostic. Are you 
intending to keep it?

If so, then I'd suggest you make `getLikelihood` a member of `Stmt` so that 
`CodeGen` and the warning here can both easily call it.


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

https://reviews.llvm.org/D85091

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


[PATCH] D87048: [libTooling] Restore defaults for matchers in makeRule.

2020-09-02 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6f0a3711bc15: [libTooling] Restore defaults for matchers in 
makeRule. (authored by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87048

Files:
  clang/lib/Tooling/Transformer/RewriteRule.cpp
  clang/unittests/Tooling/TransformerTest.cpp


Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -878,10 +878,8 @@
 }
 
 // Verifies that a rule with a top-level matcher for an implicit node (like
-// `implicitCastExpr`) does not change the code, when the AST traversal skips
-// implicit nodes. In this test, only the rule with the explicit-node matcher
-// will fire.
-TEST_F(TransformerTest, OrderedRuleImplicitIgnored) {
+// `implicitCastExpr`) works correctly -- the implicit nodes are not skipped.
+TEST_F(TransformerTest, OrderedRuleImplicitMatched) {
   std::string Input = R"cc(
 void f1();
 int f2();
@@ -892,7 +890,7 @@
 void f1();
 int f2();
 void call_f1() { REPLACE_F1; }
-float call_f2() { return f2(); }
+float call_f2() { return REPLACE_F2; }
   )cc";
 
   RewriteRule ReplaceF1 =
@@ -904,32 +902,6 @@
   testRule(applyFirst({ReplaceF1, ReplaceF2}), Input, Expected);
 }
 
-// Verifies that explicitly setting the traversal kind fixes the problem in the
-// previous test.
-TEST_F(TransformerTest, OrderedRuleImplicitMatched) {
-  std::string Input = R"cc(
-void f1();
-int f2();
-void call_f1() { f1(); }
-float call_f2() { return f2(); }
-  )cc";
-  std::string Expected = R"cc(
-void f1();
-int f2();
-void call_f1() { REPLACE_F1; }
-float call_f2() { return REPLACE_F2; }
-  )cc";
-
-  RewriteRule ReplaceF1 = makeRule(
-  traverse(clang::TK_AsIs, callExpr(callee(functionDecl(hasName("f1"),
-  changeTo(cat("REPLACE_F1")));
-  RewriteRule ReplaceF2 =
-  makeRule(traverse(clang::TK_AsIs,
-implicitCastExpr(hasSourceExpression(callExpr(,
-   changeTo(cat("REPLACE_F2")));
-  testRule(applyFirst({ReplaceF1, ReplaceF2}), Input, Expected);
-}
-
 //
 // Negative tests (where we expect no transformation to occur).
 //
Index: clang/lib/Tooling/Transformer/RewriteRule.cpp
===
--- clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -345,14 +345,13 @@
   // Each anyOf explicitly controls the traversal kind. The anyOf itself is set
   // to `TK_AsIs` to ensure no nodes are skipped, thereby deferring to the kind
   // of the branches. Then, each branch is either left as is, if the kind is
-  // already set, or explicitly set to `TK_IgnoreUnlessSpelledInSource`. We
-  // choose this setting, because we think it is the one most friendly to
-  // beginners, who are (largely) the target audience of Transformer.
+  // already set, or explicitly set to `TK_AsIs`. We choose this setting 
because
+  // it is the default interpretation of matchers.
   std::vector Matchers;
   for (const auto &Bucket : Buckets) {
 DynTypedMatcher M = DynTypedMatcher::constructVariadic(
 DynTypedMatcher::VO_AnyOf, Bucket.first,
-taggedMatchers("Tag", Bucket.second, TK_IgnoreUnlessSpelledInSource));
+taggedMatchers("Tag", Bucket.second, TK_AsIs));
 M.setAllowBind(true);
 // `tryBind` is guaranteed to succeed, because `AllowBind` was set to true.
 Matchers.push_back(M.tryBind(RootID)->withTraversalKind(TK_AsIs));


Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -878,10 +878,8 @@
 }
 
 // Verifies that a rule with a top-level matcher for an implicit node (like
-// `implicitCastExpr`) does not change the code, when the AST traversal skips
-// implicit nodes. In this test, only the rule with the explicit-node matcher
-// will fire.
-TEST_F(TransformerTest, OrderedRuleImplicitIgnored) {
+// `implicitCastExpr`) works correctly -- the implicit nodes are not skipped.
+TEST_F(TransformerTest, OrderedRuleImplicitMatched) {
   std::string Input = R"cc(
 void f1();
 int f2();
@@ -892,7 +890,7 @@
 void f1();
 int f2();
 void call_f1() { REPLACE_F1; }
-float call_f2() { return f2(); }
+float call_f2() { return REPLACE_F2; }
   )cc";
 
   RewriteRule ReplaceF1 =
@@ -904,32 +902,6 @@
   testRule(applyFirst({ReplaceF1, ReplaceF2}), Input, Expected);
 }
 
-// Verifies that explicitly setting the traversal kind fixes the problem in the
-// previous test.
-TEST_F(TransformerTest, OrderedRuleImplicitMatched) {
-  std::string Input = R"cc(
-void f1();
-int f2();
-v

[clang] 6f0a371 - [libTooling] Restore defaults for matchers in makeRule.

2020-09-02 Thread Yitzhak Mandelbaum via cfe-commits

Author: Yitzhak Mandelbaum
Date: 2020-09-02T19:36:14Z
New Revision: 6f0a3711bc15f8b50ad56d64eee70d9ba62f70c6

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

LOG: [libTooling] Restore defaults for matchers in makeRule.

This patch restores the default traversal for Transformer's `makeRule` to
`TK_AsIs`. The implicit mode has proven problematic.

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

Added: 


Modified: 
clang/lib/Tooling/Transformer/RewriteRule.cpp
clang/unittests/Tooling/TransformerTest.cpp

Removed: 




diff  --git a/clang/lib/Tooling/Transformer/RewriteRule.cpp 
b/clang/lib/Tooling/Transformer/RewriteRule.cpp
index fe33f9cf8b0c..594e22f56b87 100644
--- a/clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ b/clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -345,14 +345,13 @@ transformer::detail::buildMatchers(const RewriteRule 
&Rule) {
   // Each anyOf explicitly controls the traversal kind. The anyOf itself is set
   // to `TK_AsIs` to ensure no nodes are skipped, thereby deferring to the kind
   // of the branches. Then, each branch is either left as is, if the kind is
-  // already set, or explicitly set to `TK_IgnoreUnlessSpelledInSource`. We
-  // choose this setting, because we think it is the one most friendly to
-  // beginners, who are (largely) the target audience of Transformer.
+  // already set, or explicitly set to `TK_AsIs`. We choose this setting 
because
+  // it is the default interpretation of matchers.
   std::vector Matchers;
   for (const auto &Bucket : Buckets) {
 DynTypedMatcher M = DynTypedMatcher::constructVariadic(
 DynTypedMatcher::VO_AnyOf, Bucket.first,
-taggedMatchers("Tag", Bucket.second, TK_IgnoreUnlessSpelledInSource));
+taggedMatchers("Tag", Bucket.second, TK_AsIs));
 M.setAllowBind(true);
 // `tryBind` is guaranteed to succeed, because `AllowBind` was set to true.
 Matchers.push_back(M.tryBind(RootID)->withTraversalKind(TK_AsIs));

diff  --git a/clang/unittests/Tooling/TransformerTest.cpp 
b/clang/unittests/Tooling/TransformerTest.cpp
index 26158b1520f9..2c9bd7dfd32d 100644
--- a/clang/unittests/Tooling/TransformerTest.cpp
+++ b/clang/unittests/Tooling/TransformerTest.cpp
@@ -878,10 +878,8 @@ TEST_F(TransformerTest, OrderedRuleMultipleKinds) {
 }
 
 // Verifies that a rule with a top-level matcher for an implicit node (like
-// `implicitCastExpr`) does not change the code, when the AST traversal skips
-// implicit nodes. In this test, only the rule with the explicit-node matcher
-// will fire.
-TEST_F(TransformerTest, OrderedRuleImplicitIgnored) {
+// `implicitCastExpr`) works correctly -- the implicit nodes are not skipped.
+TEST_F(TransformerTest, OrderedRuleImplicitMatched) {
   std::string Input = R"cc(
 void f1();
 int f2();
@@ -892,7 +890,7 @@ TEST_F(TransformerTest, OrderedRuleImplicitIgnored) {
 void f1();
 int f2();
 void call_f1() { REPLACE_F1; }
-float call_f2() { return f2(); }
+float call_f2() { return REPLACE_F2; }
   )cc";
 
   RewriteRule ReplaceF1 =
@@ -904,32 +902,6 @@ TEST_F(TransformerTest, OrderedRuleImplicitIgnored) {
   testRule(applyFirst({ReplaceF1, ReplaceF2}), Input, Expected);
 }
 
-// Verifies that explicitly setting the traversal kind fixes the problem in the
-// previous test.
-TEST_F(TransformerTest, OrderedRuleImplicitMatched) {
-  std::string Input = R"cc(
-void f1();
-int f2();
-void call_f1() { f1(); }
-float call_f2() { return f2(); }
-  )cc";
-  std::string Expected = R"cc(
-void f1();
-int f2();
-void call_f1() { REPLACE_F1; }
-float call_f2() { return REPLACE_F2; }
-  )cc";
-
-  RewriteRule ReplaceF1 = makeRule(
-  traverse(clang::TK_AsIs, callExpr(callee(functionDecl(hasName("f1"),
-  changeTo(cat("REPLACE_F1")));
-  RewriteRule ReplaceF2 =
-  makeRule(traverse(clang::TK_AsIs,
-implicitCastExpr(hasSourceExpression(callExpr(,
-   changeTo(cat("REPLACE_F2")));
-  testRule(applyFirst({ReplaceF1, ReplaceF2}), Input, Expected);
-}
-
 //
 // Negative tests (where we expect no transformation to occur).
 //



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


  1   2   >