[PATCH] D45815: [libclang] Allow skipping function bodies in preamble only

[Nikolai Kosjar via Phabricator via cfe-commits]

nik added a comment.

Do I miss something? I've uploaded a new diff/version and state is still "(X) 
Requested Changes to Prior Diff".

Waiting for review.


Repository:
  rC Clang

https://reviews.llvm.org/D45815



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

[PATCH] D41537: Optionally add code completion results for arrow instead of dot

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

Ping


https://reviews.llvm.org/D41537



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

[PATCH] D46180: [clang-format] Refactor #include insertion/deletion functionality into a class.

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

Friendly ping.


Repository:
  rC Clang

https://reviews.llvm.org/D46180



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

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added inline comments.



Comment at: lib/Sema/SemaDeclCXX.cpp:8931
+  /*ConstArg*/ true, false, false, false, false);
+  auto *CopyCtor = cast_or_null(SMI.getMethod());
+

rjmccall wrote:
> Sorry, I didn't realize you'd go off and write this code manually.
> 
> The way we generally handle this sort of thing is just to synthesize an 
> expression in Sema that performs the copy-construction.  That way, the stdlib 
> can be as crazy as it wants — default arguments on the copy-constructor or 
> whatever else — and it just works.  The pattern to follow here is the code in 
> Sema::BuildExceptionDeclaration, except that in your case you can completely 
> dispense with faking up an OpaqueValueExpr and instead just build a 
> DeclRefExpr to the actual variable.  (You could even use ActOnIdExpression 
> for this, although just synthesizing the DRE shouldn't be too hard.)  Then 
> the ComparisonCategoryInfo can just store expressions for each of the three 
> (four?) variables, and in IRGen you just evaluate the appropriate one into 
> the destination.
I think this goes against the direction Richard and I decided to go, which was 
to not synthesize any expressions.

The problem is that the synthesized expressions cannot be stored in 
`ComparisonCategoryInfo`, because the data it contains is not serialized. So 
when we read the AST back, the `ComparisonCategoryInfo` will be empty. And for 
obvious reasons we can't cache the data in Sema either. Additionally, we 
probably don't want to waste space building and storing synthesized expressions 
in each AST node which needs them.

Although the checking here leaves something to be desired, I suspect it's more 
than enough to handle any conforming STL implementation.




https://reviews.llvm.org/D45476



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

[PATCH] D46286: [Driver] Don't warn about unused inputs in config files

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC331504: [Driver] Don't warn about unused inputs in 
config files (authored by mstorsjo, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D46286

Files:
  lib/Driver/Driver.cpp
  test/Driver/Inputs/config-4.cfg
  test/Driver/config-file.c


Index: test/Driver/config-file.c
===
--- test/Driver/config-file.c
+++ test/Driver/config-file.c
@@ -63,6 +63,7 @@
 //
 // RUN: %clang --config %S/Inputs/config-4.cfg -S %s -o /dev/null -v 2>&1 | 
FileCheck %s -check-prefix CHECK-UNUSED
 // CHECK-UNUSED-NOT: argument unused during compilation:
+// CHECK-UNUSED-NOT: 'linker' input unused
 
 
 //--- User directory is searched first.
Index: test/Driver/Inputs/config-4.cfg
===
--- test/Driver/Inputs/config-4.cfg
+++ test/Driver/Inputs/config-4.cfg
@@ -1,2 +1,3 @@
 -L/usr/local/lib
--stdlib=libc++
\ No newline at end of file
+-lfoo
+-stdlib=libc++
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -285,11 +285,12 @@
 }
 
 static Arg *MakeInputArg(DerivedArgList &Args, OptTable &Opts,
- StringRef Value) {
+ StringRef Value, bool Claim = true) {
   Arg *A = new Arg(Opts.getOption(options::OPT_INPUT), Value,
Args.getBaseArgs().MakeIndex(Value), Value.data());
   Args.AddSynthesizedArg(A);
-  A->claim();
+  if (Claim)
+A->claim();
   return A;
 }
 
@@ -357,7 +358,7 @@
 if (A->getOption().matches(options::OPT__DASH_DASH)) {
   A->claim();
   for (StringRef Val : A->getValues())
-DAL->append(MakeInputArg(*DAL, *Opts, Val));
+DAL->append(MakeInputArg(*DAL, *Opts, Val, false));
   continue;
 }
 
@@ -2906,6 +2907,9 @@
 // this compilation, warn the user about it.
 phases::ID InitialPhase = PL[0];
 if (InitialPhase > FinalPhase) {
+  if (InputArg->isClaimed())
+continue;
+
   // Claim here to avoid the more general unused warning.
   InputArg->claim();
 


Index: test/Driver/config-file.c
===
--- test/Driver/config-file.c
+++ test/Driver/config-file.c
@@ -63,6 +63,7 @@
 //
 // RUN: %clang --config %S/Inputs/config-4.cfg -S %s -o /dev/null -v 2>&1 | FileCheck %s -check-prefix CHECK-UNUSED
 // CHECK-UNUSED-NOT: argument unused during compilation:
+// CHECK-UNUSED-NOT: 'linker' input unused
 
 
 //--- User directory is searched first.
Index: test/Driver/Inputs/config-4.cfg
===
--- test/Driver/Inputs/config-4.cfg
+++ test/Driver/Inputs/config-4.cfg
@@ -1,2 +1,3 @@
 -L/usr/local/lib
--stdlib=libc++
\ No newline at end of file
+-lfoo
+-stdlib=libc++
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -285,11 +285,12 @@
 }
 
 static Arg *MakeInputArg(DerivedArgList &Args, OptTable &Opts,
- StringRef Value) {
+ StringRef Value, bool Claim = true) {
   Arg *A = new Arg(Opts.getOption(options::OPT_INPUT), Value,
Args.getBaseArgs().MakeIndex(Value), Value.data());
   Args.AddSynthesizedArg(A);
-  A->claim();
+  if (Claim)
+A->claim();
   return A;
 }
 
@@ -357,7 +358,7 @@
 if (A->getOption().matches(options::OPT__DASH_DASH)) {
   A->claim();
   for (StringRef Val : A->getValues())
-DAL->append(MakeInputArg(*DAL, *Opts, Val));
+DAL->append(MakeInputArg(*DAL, *Opts, Val, false));
   continue;
 }
 
@@ -2906,6 +2907,9 @@
 // this compilation, warn the user about it.
 phases::ID InitialPhase = PL[0];
 if (InitialPhase > FinalPhase) {
+  if (InputArg->isClaimed())
+continue;
+
   // Claim here to avoid the more general unused warning.
   InputArg->claim();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r331504 - [Driver] Don't warn about unused inputs in config files

[Martin Storsjo via cfe-commits]

Author: mstorsjo
Date: Thu May  3 23:05:58 2018
New Revision: 331504

URL: http://llvm.org/viewvc/llvm-project?rev=331504&view=rev
Log:
[Driver] Don't warn about unused inputs in config files

This avoids warnings about unused linker parameters, just like
other flags are ignored if they're from config files.

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

Modified:
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/test/Driver/Inputs/config-4.cfg
cfe/trunk/test/Driver/config-file.c

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=331504&r1=331503&r2=331504&view=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Thu May  3 23:05:58 2018
@@ -285,11 +285,12 @@ phases::ID Driver::getFinalPhase(const D
 }
 
 static Arg *MakeInputArg(DerivedArgList &Args, OptTable &Opts,
- StringRef Value) {
+ StringRef Value, bool Claim = true) {
   Arg *A = new Arg(Opts.getOption(options::OPT_INPUT), Value,
Args.getBaseArgs().MakeIndex(Value), Value.data());
   Args.AddSynthesizedArg(A);
-  A->claim();
+  if (Claim)
+A->claim();
   return A;
 }
 
@@ -357,7 +358,7 @@ DerivedArgList *Driver::TranslateInputAr
 if (A->getOption().matches(options::OPT__DASH_DASH)) {
   A->claim();
   for (StringRef Val : A->getValues())
-DAL->append(MakeInputArg(*DAL, *Opts, Val));
+DAL->append(MakeInputArg(*DAL, *Opts, Val, false));
   continue;
 }
 
@@ -2906,6 +2907,9 @@ void Driver::BuildActions(Compilation &C
 // this compilation, warn the user about it.
 phases::ID InitialPhase = PL[0];
 if (InitialPhase > FinalPhase) {
+  if (InputArg->isClaimed())
+continue;
+
   // Claim here to avoid the more general unused warning.
   InputArg->claim();
 

Modified: cfe/trunk/test/Driver/Inputs/config-4.cfg
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/config-4.cfg?rev=331504&r1=331503&r2=331504&view=diff
==
--- cfe/trunk/test/Driver/Inputs/config-4.cfg (original)
+++ cfe/trunk/test/Driver/Inputs/config-4.cfg Thu May  3 23:05:58 2018
@@ -1,2 +1,3 @@
 -L/usr/local/lib
--stdlib=libc++
\ No newline at end of file
+-lfoo
+-stdlib=libc++

Modified: cfe/trunk/test/Driver/config-file.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/config-file.c?rev=331504&r1=331503&r2=331504&view=diff
==
--- cfe/trunk/test/Driver/config-file.c (original)
+++ cfe/trunk/test/Driver/config-file.c Thu May  3 23:05:58 2018
@@ -63,6 +63,7 @@
 //
 // RUN: %clang --config %S/Inputs/config-4.cfg -S %s -o /dev/null -v 2>&1 | 
FileCheck %s -check-prefix CHECK-UNUSED
 // CHECK-UNUSED-NOT: argument unused during compilation:
+// CHECK-UNUSED-NOT: 'linker' input unused
 
 
 //--- User directory is searched first.


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

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaDeclCXX.cpp:8931
+  /*ConstArg*/ true, false, false, false, false);
+  auto *CopyCtor = cast_or_null(SMI.getMethod());
+

Sorry, I didn't realize you'd go off and write this code manually.

The way we generally handle this sort of thing is just to synthesize an 
expression in Sema that performs the copy-construction.  That way, the stdlib 
can be as crazy as it wants — default arguments on the copy-constructor or 
whatever else — and it just works.  The pattern to follow here is the code in 
Sema::BuildExceptionDeclaration, except that in your case you can completely 
dispense with faking up an OpaqueValueExpr and instead just build a DeclRefExpr 
to the actual variable.  (You could even use ActOnIdExpression for this, 
although just synthesizing the DRE shouldn't be too hard.)  Then the 
ComparisonCategoryInfo can just store expressions for each of the three (four?) 
variables, and in IRGen you just evaluate the appropriate one into the 
destination.


https://reviews.llvm.org/D45476



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

[PATCH] D45131: [AST] Refactor UnaryTransformType into TransformTraitType supporting non-unary transforms

[Eric Fiselier via Phabricator via cfe-commits]

EricWF planned changes to this revision.
EricWF marked 7 inline comments as done.
EricWF added inline comments.



Comment at: include/clang/ASTMatchers/ASTMatchers.h:5262-5266
+AST_MATCHER(Type, unaryTransformType) {
+  if (const auto *T = Node.getAs())
+return T->getNumArgs() == 1;
+  return false;
+}

rsmith wrote:
> Generally I think we want the primitive AST matchers to pretty directly 
> correspond to the Clang AST. I think it would make sense to replace this 
> matcher with a generalized `transformTrait` matcher -- but in a separate 
> commit from this one. This is OK for now.
I added a generalized `transformTrait` matcher, but forgot to declare it.

The reason for keeping the `unaryTransformType` was simply to avoid breaking 
existing code which already depends on it.
My plan was to possibly remove this wrapper later, after cleaning up usages in 
the LLVM code base.



Comment at: lib/AST/Type.cpp:3089
+  this->setContainsUnexpandedParameterPack(true);
+  }
+}

rsmith wrote:
> (And just for clarity: instantiation-dependence and 
> contains-unexpanded-parameter-pack are concerned with the syntactic form of 
> the type, not the semantics, so those two *should* be based on the 
> corresponding properties of the arguments.)
Thanks for the clarification. I was about to ask.



Comment at: lib/Sema/SemaType.cpp:1508-1509
+  break;
+default:
+  llvm_unreachable("unhandled case");
+}

rsmith wrote:
> EricWF wrote:
> > rsmith wrote:
> > > Try to avoid `default:` cases like this, as they suppress the warning 
> > > notifying the user to update this switch when new traits are added.
> > Understood, but surely this is a case where `default:` is warranted.  We're 
> > switching over a range of values much larger than 
> > `TransformTraitType::TTKind` in order to transform it into the `TTKind`
> > 
> > Do you have suggestions for improving it?
> I think you should convert the trait to a `QualType` when you parse it, 
> rather than waiting until now, which would make this moot.
Hmm. I'm not quite sure how/where to do that in the parser.

Are you suggesting calling `BuildTransformTrait` from inside 
`ParseTransformTraitTypeSpecifier`, and instead of building the `DeclSpec` 
containing the result of `BuildTransformTrait` instead of the list of argument 
type?



Comment at: lib/Sema/SemaType.cpp:8014-8022
+  if (DiagSelect.hasValue()) {
+auto Info = DiagSelect.getValue();
+PartialDiagnostic PD = PDiag(diag::err_type_trait_arity);
+PD << Info.ReqNumArgs << Info.SelectOne << (Info.ReqNumArgs != 1)
+   << (int)NumArgs << R;
+return PD;
+  }

rsmith wrote:
> Do you really need a `PartialDiagnostic` here? (Could you directly emit the 
> diagnostic instead?)
As it stands now, I believe I do, since the diagnostic needs to be emitted in 
both the parser and sema. If I'm not mistaken it would be incorrect to use 
Sema's diagnostic engine inside the parser and vice-versa.

However, if I delay the arity check until we're in `SemaType` as you suggested 
elsewhere, then the partial diagnostic is unneeded.



Comment at: lib/Sema/SemaType.cpp:8036
 
-DiagnoseUseOfDecl(ED, Loc);
+  // Delay all checking while any of the arguments are instantiation dependent.
+  if (IsInstantDependent)

rsmith wrote:
> This doesn't seem right; you should delay if any of the arguments is 
> dependent, but instantiation-dependent / containing a pack don't seem like 
> they should matter here.
We can't check the arity here as long as any of the arguments contain 
unexpanded parameter packs, so at least in that case we need to delay.

However, I'll change the `isInstantationDependent` check to `isDependent`.


https://reviews.llvm.org/D45131



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

[PATCH] D46286: [Driver] Don't warn about unused inputs in config files

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff accepted this revision.
sepavloff added a comment.

With additional changes the fix is OK.

LGTM.


https://reviews.llvm.org/D46286



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

[PATCH] D45463: [AST] Print correct tag decl for tag specifier

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added inline comments.



Comment at: lib/AST/DeclPrinter.cpp:180-181
 if (isFirst) {
-  if(TD)
-SubPolicy.IncludeTagDefinition = true;
+  if (TD)
+SubPolicy.TagSpecifierAs = TD;
   SubPolicy.SuppressSpecifiers = false;

rsmith wrote:
> rsmith wrote:
> > aaron.ballman wrote:
> > > jdenny wrote:
> > > > rsmith wrote:
> > > > > Instead of the changes in this patch, can you address your use case 
> > > > > by just changing the `if` here to `if (TD && 
> > > > > TD->isThisDeclarationADefinition())`?
> > > > It sounds like that would fix the case of duplicated definitions.
> > > > 
> > > > It doesn't address the case of dropped or relocated attributes.  For 
> > > > example, -ast-print drops the following attribute and thus the 
> > > > deprecation warning:
> > > > 
> > > > ```
> > > > void fn() {
> > > >   struct T *p0;
> > > >   struct __attribute__((deprecated)) T *p1;
> > > > }
> > > > ```
> > > > 
> > > > I don't know how clang *should* accept and interpret attributes in such 
> > > > cases.  However, it seems wrong for -ast-print to produce a program 
> > > > that clang interprets differently than the original program.
> > > > I don't know how clang *should* accept and interpret attributes in such 
> > > > cases. However, it seems wrong for -ast-print to produce a program that 
> > > > clang interprets differently than the original program.
> > > 
> > > This has been a *very* long-standing issue with -ast-print that I'd love 
> > > to see fixed. The AST printer does not print attributes in the correct 
> > > locations except by accident, and it oftentimes prints with the wrong 
> > > syntax, or is missing from places where it should be printed.
> > > 
> > > Long story short, I agree that this behavior is wrong and needs to be 
> > > fixed, but it's also going to take a bit of work to get it right.
> > > it seems wrong for -ast-print to produce a program that clang interprets 
> > > differently than the original program.
> > 
> > Historically we've viewed `-ast-print` as a debugging tool, not as a way of 
> > producing valid source code. If you want to change that, there will be a 
> > lng list of issues to fix. However, one of the important and intended 
> > uses of the `-ast-print` mechanism is producing pretty-printed type names 
> > and the like for diagnostics, and in that context, it *is* important to 
> > print attributes properly.
> > 
> > > It doesn't address the case of dropped or relocated attributes.
> > 
> > Hmm. A big part of the problem here, in my view, is that when we come to 
> > print a `TagType`, we call `TagType::getDecl`, which returns our 
> > "favourite" declaration of that entity, not necessarily the one that was 
> > named (or even created) for that instance of the type. We could easily fix 
> > that (by adding an accessor to reach `TagType::decl` rather than calling 
> > `TagType::getDecl()`), and perhaps *that* plus the current tracking of 
> > whether we're printing the tag declaration that is owned by the current 
> > `DeclGroup` would be enough to get correct displays here.
> > 
> > Another part of the problem is that `TagType` does not track whether it's 
> > referring to its "own" `TagDecl` (ie, one created when parsing that type) 
> > or some pre-existing one, and that affects whether and how attributes 
> > should be printed. That's why we need the `IncludeTagDefinition` flag in 
> > the first place; it's our attempt to recreate knowledge about whether the 
> > `TagType` owns its `TagDecl`. We could consider instead tracking a flag on 
> > `TagType` sugar nodes to model that, which would let us get rid of that 
> > flag.
> > Hmm. A big part of the problem here, in my view, is that when we come to 
> > print a TagType, we call TagType::getDecl, which returns our "favourite" 
> > declaration of that entity, not necessarily the one that was named (or even 
> > created) for that instance of the type.
> 
> Ah, no, the problem is a little deeper than that; we don't even create 
> distinct type sugar for the new-decl vs existing-decl cases. Maybe that'd be 
> somewhere to start -- perhaps the `ElaboratedType` node could track that for 
> us.
> Historically we've viewed -ast-print as a debugging tool, not as a way of 
> producing valid source code. If you want to change that, there will be a 
> lng list of issues to fix. 

I've heard that, but it seemed to me that what's good for the latter is likely 
good for the former.

Still, I don't want to waste time on patches that won't be accepted because 
they're harder to justify purely for debugging purposes.  Is there a better 
place to explore pretty printing valid source code from a clang AST?

> However, one of the important and intended uses of the -ast-print mechanism 
> is producing pretty-printed type names and the like for diagnostics, and in 
> that context, it *is* important to print attributes properly.

I haven't considered cases where diagnost

[PATCH] D46374: Add support for ObjC property name to be a single acronym.

[Yan Zhang via Phabricator via cfe-commits]

Wizard marked 3 inline comments as done.
Wizard added inline comments.



Comment at: test/clang-tidy/objc-property-declaration.m:24
 @property(assign, nonatomic) int enableGLAcceleration;
+@property(assign, nonatomic) int ID;
 @end

benhamilton wrote:
> Please add a test for a built-in regex (4G) as well as a custom regex in the 
> other test file.
Unable to add single property test of 4G because it is illegal to use digit as 
the first character of property name.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46374



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

[PATCH] D46374: Add support for ObjC property name to be a single acronym.

[Yan Zhang via Phabricator via cfe-commits]

Wizard updated this revision to Diff 145145.
Wizard added a comment.

resolve comments


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46374

Files:
  clang-tidy/objc/PropertyDeclarationCheck.cpp
  test/clang-tidy/objc-property-declaration-custom.m
  test/clang-tidy/objc-property-declaration.m


Index: test/clang-tidy/objc-property-declaration.m
===
--- test/clang-tidy/objc-property-declaration.m
+++ test/clang-tidy/objc-property-declaration.m
@@ -21,6 +21,7 @@
 @property(assign, nonatomic) int enable2GBackgroundFetch;
 @property(assign, nonatomic) int shouldUseCFPreferences;
 @property(assign, nonatomic) int enableGLAcceleration;
+@property(assign, nonatomic) int ID;
 @end
 
 @interface Foo (Bar)
Index: test/clang-tidy/objc-property-declaration-custom.m
===
--- test/clang-tidy/objc-property-declaration-custom.m
+++ test/clang-tidy/objc-property-declaration-custom.m
@@ -14,4 +14,5 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' 
not using lowerCamelCase style or not prefixed in a category, according to the 
Apple Coding Guidelines [objc-property-declaration]
 @property(assign, nonatomic) int GIFIgnoreStandardAcronym;
 // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 
'GIFIgnoreStandardAcronym' not using lowerCamelCase style or not prefixed in a 
category, according to the Apple Coding Guidelines [objc-property-declaration]
+@property(strong, nonatomic) NSString *TGIF;
 @end
Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -217,6 +217,13 @@
   assert(MatchedDecl->getName().size() > 0);
   auto *DeclContext = MatchedDecl->getDeclContext();
   auto *CategoryDecl = llvm::dyn_cast(DeclContext);
+  if (std::any_of(EscapedAcronyms.begin(), EscapedAcronyms.end(),
+  [MatchedDecl](std::string const &s) {
+auto Acronym  = llvm::Regex("^" + s + "$");
+return Acronym.match(MatchedDecl->getName());
+  })) {
+return;
+  }
   if (CategoryDecl != nullptr &&
   hasCategoryPropertyPrefix(MatchedDecl->getName())) {
 if (!prefixedPropertyNameValid(MatchedDecl->getName(), EscapedAcronyms) ||


Index: test/clang-tidy/objc-property-declaration.m
===
--- test/clang-tidy/objc-property-declaration.m
+++ test/clang-tidy/objc-property-declaration.m
@@ -21,6 +21,7 @@
 @property(assign, nonatomic) int enable2GBackgroundFetch;
 @property(assign, nonatomic) int shouldUseCFPreferences;
 @property(assign, nonatomic) int enableGLAcceleration;
+@property(assign, nonatomic) int ID;
 @end
 
 @interface Foo (Bar)
Index: test/clang-tidy/objc-property-declaration-custom.m
===
--- test/clang-tidy/objc-property-declaration-custom.m
+++ test/clang-tidy/objc-property-declaration-custom.m
@@ -14,4 +14,5 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 @property(assign, nonatomic) int GIFIgnoreStandardAcronym;
 // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'GIFIgnoreStandardAcronym' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
+@property(strong, nonatomic) NSString *TGIF;
 @end
Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -217,6 +217,13 @@
   assert(MatchedDecl->getName().size() > 0);
   auto *DeclContext = MatchedDecl->getDeclContext();
   auto *CategoryDecl = llvm::dyn_cast(DeclContext);
+  if (std::any_of(EscapedAcronyms.begin(), EscapedAcronyms.end(),
+  [MatchedDecl](std::string const &s) {
+auto Acronym  = llvm::Regex("^" + s + "$");
+return Acronym.match(MatchedDecl->getName());
+  })) {
+return;
+  }
   if (CategoryDecl != nullptr &&
   hasCategoryPropertyPrefix(MatchedDecl->getName())) {
 if (!prefixedPropertyNameValid(MatchedDecl->getName(), EscapedAcronyms) ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45463: [AST] Print correct tag decl for tag specifier

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: lib/AST/DeclPrinter.cpp:180-181
 if (isFirst) {
-  if(TD)
-SubPolicy.IncludeTagDefinition = true;
+  if (TD)
+SubPolicy.TagSpecifierAs = TD;
   SubPolicy.SuppressSpecifiers = false;

rsmith wrote:
> aaron.ballman wrote:
> > jdenny wrote:
> > > rsmith wrote:
> > > > Instead of the changes in this patch, can you address your use case by 
> > > > just changing the `if` here to `if (TD && 
> > > > TD->isThisDeclarationADefinition())`?
> > > It sounds like that would fix the case of duplicated definitions.
> > > 
> > > It doesn't address the case of dropped or relocated attributes.  For 
> > > example, -ast-print drops the following attribute and thus the 
> > > deprecation warning:
> > > 
> > > ```
> > > void fn() {
> > >   struct T *p0;
> > >   struct __attribute__((deprecated)) T *p1;
> > > }
> > > ```
> > > 
> > > I don't know how clang *should* accept and interpret attributes in such 
> > > cases.  However, it seems wrong for -ast-print to produce a program that 
> > > clang interprets differently than the original program.
> > > I don't know how clang *should* accept and interpret attributes in such 
> > > cases. However, it seems wrong for -ast-print to produce a program that 
> > > clang interprets differently than the original program.
> > 
> > This has been a *very* long-standing issue with -ast-print that I'd love to 
> > see fixed. The AST printer does not print attributes in the correct 
> > locations except by accident, and it oftentimes prints with the wrong 
> > syntax, or is missing from places where it should be printed.
> > 
> > Long story short, I agree that this behavior is wrong and needs to be 
> > fixed, but it's also going to take a bit of work to get it right.
> > it seems wrong for -ast-print to produce a program that clang interprets 
> > differently than the original program.
> 
> Historically we've viewed `-ast-print` as a debugging tool, not as a way of 
> producing valid source code. If you want to change that, there will be a 
> lng list of issues to fix. However, one of the important and intended 
> uses of the `-ast-print` mechanism is producing pretty-printed type names and 
> the like for diagnostics, and in that context, it *is* important to print 
> attributes properly.
> 
> > It doesn't address the case of dropped or relocated attributes.
> 
> Hmm. A big part of the problem here, in my view, is that when we come to 
> print a `TagType`, we call `TagType::getDecl`, which returns our "favourite" 
> declaration of that entity, not necessarily the one that was named (or even 
> created) for that instance of the type. We could easily fix that (by adding 
> an accessor to reach `TagType::decl` rather than calling 
> `TagType::getDecl()`), and perhaps *that* plus the current tracking of 
> whether we're printing the tag declaration that is owned by the current 
> `DeclGroup` would be enough to get correct displays here.
> 
> Another part of the problem is that `TagType` does not track whether it's 
> referring to its "own" `TagDecl` (ie, one created when parsing that type) or 
> some pre-existing one, and that affects whether and how attributes should be 
> printed. That's why we need the `IncludeTagDefinition` flag in the first 
> place; it's our attempt to recreate knowledge about whether the `TagType` 
> owns its `TagDecl`. We could consider instead tracking a flag on `TagType` 
> sugar nodes to model that, which would let us get rid of that flag.
> Hmm. A big part of the problem here, in my view, is that when we come to 
> print a TagType, we call TagType::getDecl, which returns our "favourite" 
> declaration of that entity, not necessarily the one that was named (or even 
> created) for that instance of the type.

Ah, no, the problem is a little deeper than that; we don't even create distinct 
type sugar for the new-decl vs existing-decl cases. Maybe that'd be somewhere 
to start -- perhaps the `ElaboratedType` node could track that for us.


https://reviews.llvm.org/D45463



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

[PATCH] D45463: [AST] Print correct tag decl for tag specifier

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: lib/AST/DeclPrinter.cpp:180-181
 if (isFirst) {
-  if(TD)
-SubPolicy.IncludeTagDefinition = true;
+  if (TD)
+SubPolicy.TagSpecifierAs = TD;
   SubPolicy.SuppressSpecifiers = false;

aaron.ballman wrote:
> jdenny wrote:
> > rsmith wrote:
> > > Instead of the changes in this patch, can you address your use case by 
> > > just changing the `if` here to `if (TD && 
> > > TD->isThisDeclarationADefinition())`?
> > It sounds like that would fix the case of duplicated definitions.
> > 
> > It doesn't address the case of dropped or relocated attributes.  For 
> > example, -ast-print drops the following attribute and thus the deprecation 
> > warning:
> > 
> > ```
> > void fn() {
> >   struct T *p0;
> >   struct __attribute__((deprecated)) T *p1;
> > }
> > ```
> > 
> > I don't know how clang *should* accept and interpret attributes in such 
> > cases.  However, it seems wrong for -ast-print to produce a program that 
> > clang interprets differently than the original program.
> > I don't know how clang *should* accept and interpret attributes in such 
> > cases. However, it seems wrong for -ast-print to produce a program that 
> > clang interprets differently than the original program.
> 
> This has been a *very* long-standing issue with -ast-print that I'd love to 
> see fixed. The AST printer does not print attributes in the correct locations 
> except by accident, and it oftentimes prints with the wrong syntax, or is 
> missing from places where it should be printed.
> 
> Long story short, I agree that this behavior is wrong and needs to be fixed, 
> but it's also going to take a bit of work to get it right.
> it seems wrong for -ast-print to produce a program that clang interprets 
> differently than the original program.

Historically we've viewed `-ast-print` as a debugging tool, not as a way of 
producing valid source code. If you want to change that, there will be a 
lng list of issues to fix. However, one of the important and intended uses 
of the `-ast-print` mechanism is producing pretty-printed type names and the 
like for diagnostics, and in that context, it *is* important to print 
attributes properly.

> It doesn't address the case of dropped or relocated attributes.

Hmm. A big part of the problem here, in my view, is that when we come to print 
a `TagType`, we call `TagType::getDecl`, which returns our "favourite" 
declaration of that entity, not necessarily the one that was named (or even 
created) for that instance of the type. We could easily fix that (by adding an 
accessor to reach `TagType::decl` rather than calling `TagType::getDecl()`), 
and perhaps *that* plus the current tracking of whether we're printing the tag 
declaration that is owned by the current `DeclGroup` would be enough to get 
correct displays here.

Another part of the problem is that `TagType` does not track whether it's 
referring to its "own" `TagDecl` (ie, one created when parsing that type) or 
some pre-existing one, and that affects whether and how attributes should be 
printed. That's why we need the `IncludeTagDefinition` flag in the first place; 
it's our attempt to recreate knowledge about whether the `TagType` owns its 
`TagDecl`. We could consider instead tracking a flag on `TagType` sugar nodes 
to model that, which would let us get rid of that flag.


https://reviews.llvm.org/D45463



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

[PATCH] D45463: [AST] Print correct tag decl for tag specifier

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/AST/DeclPrinter.cpp:180-181
 if (isFirst) {
-  if(TD)
-SubPolicy.IncludeTagDefinition = true;
+  if (TD)
+SubPolicy.TagSpecifierAs = TD;
   SubPolicy.SuppressSpecifiers = false;

jdenny wrote:
> rsmith wrote:
> > Instead of the changes in this patch, can you address your use case by just 
> > changing the `if` here to `if (TD && TD->isThisDeclarationADefinition())`?
> It sounds like that would fix the case of duplicated definitions.
> 
> It doesn't address the case of dropped or relocated attributes.  For example, 
> -ast-print drops the following attribute and thus the deprecation warning:
> 
> ```
> void fn() {
>   struct T *p0;
>   struct __attribute__((deprecated)) T *p1;
> }
> ```
> 
> I don't know how clang *should* accept and interpret attributes in such 
> cases.  However, it seems wrong for -ast-print to produce a program that 
> clang interprets differently than the original program.
> I don't know how clang *should* accept and interpret attributes in such 
> cases. However, it seems wrong for -ast-print to produce a program that clang 
> interprets differently than the original program.

This has been a *very* long-standing issue with -ast-print that I'd love to see 
fixed. The AST printer does not print attributes in the correct locations 
except by accident, and it oftentimes prints with the wrong syntax, or is 
missing from places where it should be printed.

Long story short, I agree that this behavior is wrong and needs to be fixed, 
but it's also going to take a bit of work to get it right.


https://reviews.llvm.org/D45463



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

[PATCH] D45463: [AST] Print correct tag decl for tag specifier

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In https://reviews.llvm.org/D45463#1087174, @rsmith wrote:

> > TagSpecifierAs expands sizeof(PrintingPolicy) from 8 to 16 bytes. My 
> > concern is the comments on PrintingPolicy about how PrintingPolicy is 
> > intended to be small. My guess it that 16 bytes is still small enough, but 
> > perhaps Richard Smith, who wrote that comment, knows better.
>
> This seems fine. See r270009 for some background for that comment -- we used 
> to store a copy of the `LangOptions` in the `PrintingPolicy` (among other 
> things), and copying these objects (including the potentially-large vectors 
> within the `LangOptions`) was a double-digit percentage of the compile time 
> of some compilations. That's a very different ball park from a change from 8 
> bytes to 16 bytes.


Makes sense.  Thanks.




Comment at: include/clang-c/Index.h:4116
   CXPrintingPolicy_SuppressTagKeyword,
-  CXPrintingPolicy_IncludeTagDefinition,
   CXPrintingPolicy_SuppressScope,

rsmith wrote:
> This is not OK: this would be an ABI break for the stable libclang ABI.
Understood.  We could keep IncludeTagDefinition for that purpose and just 
replace its current internal use with TagSpecifierAs.

Out of curiosity, do we know how others are using IncludeTagDefinition?



Comment at: lib/AST/DeclPrinter.cpp:180-181
 if (isFirst) {
-  if(TD)
-SubPolicy.IncludeTagDefinition = true;
+  if (TD)
+SubPolicy.TagSpecifierAs = TD;
   SubPolicy.SuppressSpecifiers = false;

rsmith wrote:
> Instead of the changes in this patch, can you address your use case by just 
> changing the `if` here to `if (TD && TD->isThisDeclarationADefinition())`?
It sounds like that would fix the case of duplicated definitions.

It doesn't address the case of dropped or relocated attributes.  For example, 
-ast-print drops the following attribute and thus the deprecation warning:

```
void fn() {
  struct T *p0;
  struct __attribute__((deprecated)) T *p1;
}
```

I don't know how clang *should* accept and interpret attributes in such cases.  
However, it seems wrong for -ast-print to produce a program that clang 
interprets differently than the original program.


https://reviews.llvm.org/D45463



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

[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:258-260
+Report->addNote(NoteBuf,
+PathDiagnosticLocation::create(FieldChain.getEndOfChain(),
+   
Context.getSourceManager()));

Szelethus wrote:
> NoQ wrote:
> > Aha, ok, got it, so you're putting the warning at the end of the 
> > constructor and squeezing all uninitialized fields into a single warning by 
> > presenting them as notes.
> > 
> > Because for now notes aren't supported everywhere (and aren't always 
> > supported really well), i guess it'd be necessary to at least provide an 
> > option to make note-less reports before the checker is enabled by default 
> > everywhere. In this case notes contain critical pieces of information and 
> > as such cannot be really discarded.
> > 
> > I'm not sure that notes are providing a better user experience here than 
> > simply saying that "field x.y->z is never initialized". With notes, the 
> > user will have to scroll around (at least in scan-build) to find which 
> > field is uninitialized.
> > 
> > Notes will probably also not decrease the number of reports too much 
> > because in most cases there will be just one uninitialized field. Though i 
> > agree that when there is more than one report, the user will be likely to 
> > deal with all fields at once rather than separately.
> > 
> > So it's a bit wonky.
> > 
> > We might want to enable one mode or another in the checker depending on the 
> > analyzer output flag. Probably in the driver (`RenderAnalyzerOptions()`).
> > 
> > It'd be a good idea to land the checker into alpha first and fix this in a 
> > follow-up patch because this review is already overweight.
> > [...]i guess it'd be necessary to at least provide an option to make 
> > note-less reports before the checker is enabled by default everywhere. In 
> > this case notes contain critical pieces of information and as such cannot 
> > be really discarded.
> This can already be achieved with `-analyzer-config notes-as-events=true`. 
> There no good reason however not to make an additional flag that would emit 
> warnings instead of notes.
> > I'm not sure that notes are providing a better user experience here than 
> > simply saying that "field x.y->z is never initialized". With notes, the 
> > user will have to scroll around (at least in scan-build) to find which 
> > field is uninitialized.
> This checker had a previous implementation that emitted a warning for each 
> uninitialized field, instead of squeezing them in a single warning per 
> constructor call. One of the major reasons behind rewriting it was that it 
> emitted so many of them that it was difficult to differentiate which warning 
> belonged to which constructor call. Also, in the case of the LLVM/Clang 
> project, the warnings from this checker alone would be in the thousands.
> > Notes will probably also not decrease the number of reports too much 
> > because in most cases there will be just one uninitialized field.
> While this is true to some extent, it's not uncommon that a single 
> constructor call leaves 7 uninitialized -- in the LLVM/Clang project I found 
> one that had over 30!
> Since its practically impossible to determine whether this was a performance 
> enhancement or just poor programming, the few cases where it is intentional, 
> an object would emit many more warnings would make  make majority of the 
> findings (where an object truly had only 1 or 2 fields uninit) a lot harder 
> to spot in my opinion.
> 
> In general, I think one warning per constructor call makes the best user 
> experience, but a temporary solution should be implemented until there's 
> better support for notes.
> 
> I agree, this should be a topic of a follow-up patch.
> 
> This can already be achieved with `-analyzer-config notes-as-events=true`.

Yep. But the resulting user experience seems pretty bad to me. It was a good 
quick proof-of-concept trick to test things, but the fact that notes aren't 
path pieces is way too apparent in case of this checker. So i guess this flag 
was a bad idea.

> Also, in the case of the LLVM/Clang project, the warnings from this checker 
> alone would be in the thousands.

This makes me a bit worried, i wonder what's the reason why does the checker 
shout so loudly on a codebase that doesn't seem to have actual uninitialized 
value bugs all over the place.

Are any of these duplicates found in the same constructor for the same field in 
different translation units? Suppose we discard the duplicates (if any) and 
warn exactly once (across the whole LLVM) for every uninitialized field (which 
is probably more than once per constructor). Then I wonder:
1. How many uninitialize fields would we be able to find this way?
2. How many of such fields were intentionally left uninitialized?
3. How many were found by deep inspection of the heap through field c

Re: [libcxxabi] r331150 - Move _LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS macro to build system

[Maxim Kuvyrkov via cfe-commits]

Yes, it did.  Thanks!

--
Maxim Kuvyrkov
www.linaro.org



> On May 3, 2018, at 3:48 PM, Nico Weber  wrote:
> 
> r331450 should hopefully fix this.
> 
> On Tue, May 1, 2018 at 8:55 AM, Nico Weber  wrote:
> tzik, can you take a look what's up on those bots?
> 
> On Tue, May 1, 2018, 5:48 AM Maxim Kuvyrkov  wrote:
> Hi Nico,
> 
> This broke armv7 and aarch64 bots:
> - 
> http://lab.llvm.org:8011/builders/libcxx-libcxxabi-libunwind-armv7-linux/builds/87
> - 
> http://lab.llvm.org:8011/builders/libcxx-libcxxabi-libunwind-aarch64-linux/builds/1304
> 
> Would you please investigate?
> 
> --
> Maxim Kuvyrkov
> www.linaro.org
> 
> 
> 
> > On Apr 30, 2018, at 2:10 AM, Nico Weber via cfe-commits 
> >  wrote:
> > 
> > This should have said "Patch from Taiju Tsuiki ", 
> > apologies.
> > 
> > On Sun, Apr 29, 2018 at 7:05 PM, Nico Weber via cfe-commits 
> >  wrote:
> > Author: nico
> > Date: Sun Apr 29 16:05:11 2018
> > New Revision: 331150
> > 
> > URL: http://llvm.org/viewvc/llvm-project?rev=331150&view=rev
> > Log:
> > Move _LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS macro to build system
> > 
> > _LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS is currently used to
> > bring back std::unexpected, which is removed in C++17, but still needed
> > for libc++abi for backward compatibility.
> > 
> > This macro used to define in cxa_exception.cpp only, but actually
> > needed for all sources that touches exceptions.
> > So, a build-system-level macro is better fit to define this macro.
> > 
> > https://reviews.llvm.org/D46056
> > Patch from Taiju Tsuiku !
> > 
> > Modified:
> > libcxxabi/trunk/CMakeLists.txt
> > libcxxabi/trunk/src/cxa_exception.cpp
> > libcxxabi/trunk/test/test_exception_storage.pass.cpp
> > 
> > Modified: libcxxabi/trunk/CMakeLists.txt
> > URL: 
> > http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/CMakeLists.txt?rev=331150&r1=331149&r2=331150&view=diff
> > ==
> > --- libcxxabi/trunk/CMakeLists.txt (original)
> > +++ libcxxabi/trunk/CMakeLists.txt Sun Apr 29 16:05:11 2018
> > @@ -387,6 +387,10 @@ endif()
> >  # Prevent libc++abi from having library dependencies on libc++
> >  add_definitions(-D_LIBCPP_DISABLE_EXTERN_TEMPLATE)
> > 
> > +# Bring back `std::unexpected`, which is removed in C++17, to support
> > +# pre-C++17.
> > +add_definitions(-D_LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS)
> > +
> >  if (MSVC)
> >add_definitions(-D_CRT_SECURE_NO_WARNINGS)
> >  endif()
> > 
> > Modified: libcxxabi/trunk/src/cxa_exception.cpp
> > URL: 
> > http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/cxa_exception.cpp?rev=331150&r1=331149&r2=331150&view=diff
> > ==
> > --- libcxxabi/trunk/src/cxa_exception.cpp (original)
> > +++ libcxxabi/trunk/src/cxa_exception.cpp Sun Apr 29 16:05:11 2018
> > @@ -11,8 +11,6 @@
> >  //  
> >  
> > //===--===//
> > 
> > -#define _LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS
> > -
> >  #include "cxxabi.h"
> > 
> >  #include // for std::terminate
> > 
> > Modified: libcxxabi/trunk/test/test_exception_storage.pass.cpp
> > URL: 
> > http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/test/test_exception_storage.pass.cpp?rev=331150&r1=331149&r2=331150&view=diff
> > ==
> > --- libcxxabi/trunk/test/test_exception_storage.pass.cpp (original)
> > +++ libcxxabi/trunk/test/test_exception_storage.pass.cpp Sun Apr 29 
> > 16:05:11 2018
> > @@ -7,11 +7,6 @@
> >  //
> >  
> > //===--===//
> > 
> > -// FIXME: cxa_exception.hpp directly references `std::unexpected` and 
> > friends.
> > -// This breaks this test when compiled in C++17. For now fix this by 
> > manually
> > -// re-enabling the STL functions.
> > -#define _LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS
> > -
> >  #include 
> >  #include 
> >  #include 
> > 
> > 
> > ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> > 
> > ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> 
> 

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

[PATCH] D46415: [analyzer] pr36458: Fix retrieved value cast for symbolic void pointers.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, 
rnkovacs.
Herald added subscribers: cfe-commits, baloghadamsoftware.

Through C cast magic it's possible to put a raw void pointer into a variable of 
non-void pointer type. It is fine - generally, it's possible to put anything 
into anywhere by temporarily re-interpreting the anywhere. We cast it back to 
the correct type during load - with the help of 
`StoreManager::CastRetrievedVal()`.

The attached example demonstrates that we're not casting the retrieved value 
pedantically enough when it comes to retrieving pointer values. In fact, we 
don't cast pointers-to-pointers at all because `SValBuilder` doesn't know how 
to do that or even that it needs to do that at all.

In this patch i perform the pointer cast manually for the situation that causes 
the crash. Performing the cast more carefully in other cases is possible, but i 
didn't manage to produce an observable effect (the second test passes just fine 
without it, and works correctly).

I cannot put the `castRegion()` call into `SValBuilder` because `SValBuilder` 
doesn't have access to `StoreManager` when doing casts. We really need to 
decouple this stuff and make a single good function for handling casts, because 
understanding how current code for pointer casts works is a nightmare.


Repository:
  rC Clang

https://reviews.llvm.org/D46415

Files:
  lib/StaticAnalyzer/Core/Store.cpp
  test/Analysis/casts.c


Index: test/Analysis/casts.c
===
--- test/Analysis/casts.c
+++ test/Analysis/casts.c
@@ -149,3 +149,19 @@
 
   clang_analyzer_eval(*((char *)y1) == *((char *) y3)); // 
expected-warning{{TRUE}}
 }
+
+void *getVoidPtr();
+
+void testCastVoidPtrToIntPtrThroughIntTypedAssignment() {
+  int *x;
+  (*((int *)(&x))) = (int)getVoidPtr();
+  *x = 1; // no-crash
+}
+
+void testCastCharPtrToIntPtrThroughIntTypedAssignment() {
+  unsigned c;
+  int *x;
+  (*((int *)(&x))) = (int)&c;
+  *x = 1;
+  clang_analyzer_eval(c == 1); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Core/Store.cpp
===
--- lib/StaticAnalyzer/Core/Store.cpp
+++ lib/StaticAnalyzer/Core/Store.cpp
@@ -378,6 +378,20 @@
   if (castTy.isNull() || V.isUnknownOrUndef())
 return V;
 
+  // When retrieving symbolic pointer and expecting a non-void pointer,
+  // wrap them into element regions of the expected type if necessary.
+  // SValBuilder::dispatchCast() doesn't do that, but it is necessary to
+  // make sure that the retrieved value makes sense, because there's no other
+  // cast in the AST that would tell us to cast it to the correct pointer type.
+  // We might need to do that for non-void pointers as well.
+  // FIXME: We really need a single good function to perform casts for us
+  // correctly every time we need it.
+  if (castTy->isPointerType() && !castTy->isVoidPointerType())
+if (const auto *SR = dyn_cast_or_null(V.getAsRegion()))
+  if (SR->getSymbol()->getType().getCanonicalType() !=
+  castTy.getCanonicalType())
+return loc::MemRegionVal(castRegion(SR, castTy));
+
   return svalBuilder.dispatchCast(V, castTy);
 }
 


Index: test/Analysis/casts.c
===
--- test/Analysis/casts.c
+++ test/Analysis/casts.c
@@ -149,3 +149,19 @@
 
   clang_analyzer_eval(*((char *)y1) == *((char *) y3)); // expected-warning{{TRUE}}
 }
+
+void *getVoidPtr();
+
+void testCastVoidPtrToIntPtrThroughIntTypedAssignment() {
+  int *x;
+  (*((int *)(&x))) = (int)getVoidPtr();
+  *x = 1; // no-crash
+}
+
+void testCastCharPtrToIntPtrThroughIntTypedAssignment() {
+  unsigned c;
+  int *x;
+  (*((int *)(&x))) = (int)&c;
+  *x = 1;
+  clang_analyzer_eval(c == 1); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Core/Store.cpp
===
--- lib/StaticAnalyzer/Core/Store.cpp
+++ lib/StaticAnalyzer/Core/Store.cpp
@@ -378,6 +378,20 @@
   if (castTy.isNull() || V.isUnknownOrUndef())
 return V;
 
+  // When retrieving symbolic pointer and expecting a non-void pointer,
+  // wrap them into element regions of the expected type if necessary.
+  // SValBuilder::dispatchCast() doesn't do that, but it is necessary to
+  // make sure that the retrieved value makes sense, because there's no other
+  // cast in the AST that would tell us to cast it to the correct pointer type.
+  // We might need to do that for non-void pointers as well.
+  // FIXME: We really need a single good function to perform casts for us
+  // correctly every time we need it.
+  if (castTy->isPointerType() && !castTy->isVoidPointerType())
+if (const auto *SR = dyn_cast_or_null(V.getAsRegion()))
+  if (SR->getSymbol()->getType().getCanonicalType() !=
+  castTy.getCanonicalType())
+return loc::MemRegionVal(castRegion(SR, castTy)

[PATCH] D46410: [Target] Diagnose mismatch in required CPU for always_inline functions

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

From the backend codegen perspective it would be fine if the callees 
instruction features were a subset. So it really depends on why the user wrote 
the different arch on the callee in the first place. If they did it because of 
one of the various tuning flags in the backend that we set based on CPU name 
that information would be lost during the inlining. Maybe a warning would be 
more appropriate. My main goal was to give the user something more to go on 
when the instruction set features weren't a subset so we didn't just give them 
some random feature name that mismatched first.

Experiments with gcc showed that it throws an error for CPU name mismatch. 
Strangely it even gave an error if one function had a target attribute with an 
arch= and the command line had the same -march.


https://reviews.llvm.org/D46410



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

r331496 - [analyzer] NFC: Remove unused parameteer of StoreManager::CastRetrievedVal().

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Thu May  3 17:53:41 2018
New Revision: 331496

URL: http://llvm.org/viewvc/llvm-project?rev=331496&view=rev
Log:
[analyzer] NFC: Remove unused parameteer of StoreManager::CastRetrievedVal().

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h?rev=331496&r1=331495&r2=331496&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h Thu May  
3 17:53:41 2018
@@ -282,8 +282,8 @@ protected:
   /// CastRetrievedVal - Used by subclasses of StoreManager to implement
   ///  implicit casts that arise from loads from regions that are reinterpreted
   ///  as another region.
-  SVal CastRetrievedVal(SVal val, const TypedValueRegion *region, 
-QualType castTy, bool performTestOnly = true);
+  SVal CastRetrievedVal(SVal val, const TypedValueRegion *region,
+QualType castTy);
 
 private:
   SVal getLValueFieldOrIvar(const Decl *decl, SVal base);

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=331496&r1=331495&r2=331496&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Thu May  3 17:53:41 2018
@@ -1446,7 +1446,7 @@ SVal RegionStoreManager::getBinding(Regi
 return UnknownVal();
 
   if (const FieldRegion* FR = dyn_cast(R))
-return CastRetrievedVal(getBindingForField(B, FR), FR, T, false);
+return CastRetrievedVal(getBindingForField(B, FR), FR, T);
 
   if (const ElementRegion* ER = dyn_cast(R)) {
 // FIXME: Here we actually perform an implicit conversion from the loaded
@@ -1454,7 +1454,7 @@ SVal RegionStoreManager::getBinding(Regi
 // more intelligently.  For example, an 'element' can encompass multiple
 // bound regions (e.g., several bound bytes), or could be a subset of
 // a larger value.
-return CastRetrievedVal(getBindingForElement(B, ER), ER, T, false);
+return CastRetrievedVal(getBindingForElement(B, ER), ER, T);
   }
 
   if (const ObjCIvarRegion *IVR = dyn_cast(R)) {
@@ -1464,7 +1464,7 @@ SVal RegionStoreManager::getBinding(Regi
 // reinterpretted, it is possible we stored a different value that could
 // fit within the ivar.  Either we need to cast these when storing them
 // or reinterpret them lazily (as we do here).
-return CastRetrievedVal(getBindingForObjCIvar(B, IVR), IVR, T, false);
+return CastRetrievedVal(getBindingForObjCIvar(B, IVR), IVR, T);
   }
 
   if (const VarRegion *VR = dyn_cast(R)) {
@@ -1474,7 +1474,7 @@ SVal RegionStoreManager::getBinding(Regi
 // variable is reinterpretted, it is possible we stored a different value
 // that could fit within the variable.  Either we need to cast these when
 // storing them or reinterpret them lazily (as we do here).
-return CastRetrievedVal(getBindingForVar(B, VR), VR, T, false);
+return CastRetrievedVal(getBindingForVar(B, VR), VR, T);
   }
 
   const SVal *V = B.lookup(R, BindingKey::Direct);

Modified: cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp?rev=331496&r1=331495&r2=331496&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp Thu May  3 17:53:41 2018
@@ -378,22 +378,10 @@ SVal StoreManager::attemptDownCast(SVal
 ///  implicit casts that arise from loads from regions that are reinterpreted
 ///  as another region.
 SVal StoreManager::CastRetrievedVal(SVal V, const TypedValueRegion *R,
-QualType castTy, bool performTestOnly) {
+QualType castTy) {
   if (castTy.isNull() || V.isUnknownOrUndef())
 return V;
 
-  ASTContext &Ctx = svalBuilder.getContext();
-
-  if (performTestOnly) {
-// Automatically translate references to pointers.
-QualType T = R->getValueType();
-if (const ReferenceType *RT = T->getAs())
-  T = Ctx.getPointerType(RT->getPointeeType());
-
-assert(svalBuilder.getContext().hasSameUnqualifiedType(castTy, T));
-return V;
-  }
-
   return svalBuilder.dispatchCast(V, castTy);
 }
 


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

[PATCH] D46410: [Target] Diagnose mismatch in required CPU for always_inline functions

[Eric Christopher via Phabricator via cfe-commits]

echristo added a comment.

I like the idea of a front end warning, but had believed that a subset of cpu 
features should be allowed to be inlined into something that's a superset and 
it sounds like you don't agree? Your thoughts here?

-eric


https://reviews.llvm.org/D46410



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

[PATCH] D45679: [clang-tidy] Add a helper function isModified, that checks whether an expression is modified within a statement.

[Shuai Wang via Phabricator via cfe-commits]

shuaiwang updated this revision to Diff 145121.
shuaiwang marked 4 inline comments as done.
shuaiwang added a comment.

Addressed review comments, notably added check for DeclRefExpr to volatile 
variables.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45679

Files:
  clang-tidy/utils/CMakeLists.txt
  clang-tidy/utils/ExprMutationAnalyzer.cpp
  clang-tidy/utils/ExprMutationAnalyzer.h
  unittests/clang-tidy/CMakeLists.txt
  unittests/clang-tidy/ExprMutationAnalyzerTest.cpp

Index: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
===
--- /dev/null
+++ unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
@@ -0,0 +1,561 @@
+//===-- ExprMutationAnalyzerTest.cpp - clang-tidy -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "../clang-tidy/utils/ExprMutationAnalyzer.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace tidy {
+namespace test {
+
+using namespace clang::ast_matchers;
+using ::testing::ElementsAre;
+using ::testing::IsEmpty;
+using ::testing::ResultOf;
+using ::testing::StartsWith;
+using ::testing::Values;
+
+namespace {
+
+using ExprMatcher = internal::Matcher;
+using StmtMatcher = internal::Matcher;
+
+ExprMatcher declRefTo(StringRef Name) {
+  return declRefExpr(to(namedDecl(hasName(Name;
+}
+
+StmtMatcher withEnclosingCompound(ExprMatcher Matcher) {
+  return expr(Matcher, hasAncestor(compoundStmt().bind("stmt"))).bind("expr");
+}
+
+bool isMutated(const SmallVectorImpl &Results, ASTUnit *AST) {
+  const auto *const S = selectFirst("stmt", Results);
+  const auto *const E = selectFirst("expr", Results);
+  return utils::ExprMutationAnalyzer(S, &AST->getASTContext()).isMutated(E);
+}
+
+SmallVector
+mutatedBy(const SmallVectorImpl &Results, ASTUnit *AST) {
+  const auto *const S = selectFirst("stmt", Results);
+  SmallVector Chain;
+  utils::ExprMutationAnalyzer Analyzer(S, &AST->getASTContext());
+  for (const auto *E = selectFirst("expr", Results); E != nullptr;) {
+const Stmt *By = Analyzer.findMutation(E);
+std::string buffer;
+llvm::raw_string_ostream stream(buffer);
+By->printPretty(stream, nullptr, AST->getASTContext().getPrintingPolicy());
+Chain.push_back(StringRef(stream.str()).trim().str());
+E = dyn_cast(By);
+  }
+  return Chain;
+}
+
+std::string removeSpace(std::string s) {
+  s.erase(std::remove_if(s.begin(), s.end(),
+ [](char c) { return std::isspace(c); }),
+  s.end());
+  return s;
+}
+
+} // namespace
+
+TEST(ExprMutationAnalyzerTest, Trivial) {
+  const auto AST = tooling::buildASTFromCode("void f() { int x; x; }");
+  const auto Results =
+  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+}
+
+TEST(ExprMutationAnalyzerTest, RefToVolatile) {
+  const auto AST = tooling::buildASTFromCode("void f() { volatile int x; x; }");
+  const auto Results =
+  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_TRUE(isMutated(Results, AST.get()));
+}
+
+class AssignmentTest : public ::testing::TestWithParam {};
+
+TEST_P(AssignmentTest, AssignmentModifies) {
+  const std::string ModExpr = "x " + GetParam() + " 10";
+  const auto AST =
+  tooling::buildASTFromCode("void f() { int x; " + ModExpr + "; }");
+  const auto Results =
+  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+}
+
+INSTANTIATE_TEST_CASE_P(AllAssignmentOperators, AssignmentTest,
+Values("=", "+=", "-=", "*=", "/=", "%=", "&=", "|=",
+   "^=", "<<=", ">>="), );
+
+class IncDecTest : public ::testing::TestWithParam {};
+
+TEST_P(IncDecTest, IncDecModifies) {
+  const std::string ModExpr = GetParam();
+  const auto AST =
+  tooling::buildASTFromCode("void f() { int x; " + ModExpr + "; }");
+  const auto Results =
+  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+}
+
+INSTANTIATE_TEST_CASE_P(AllIncDecOperators, IncDecTest,
+Values("++x", "--x", "x++", "x--"), );
+
+TEST(ExprMutationAnalyzerTest, NonConstMemberFunc) {
+  const auto AST = tooling::buildASTFromCode(
+  "void f() { struct Foo { void mf(); }; Foo x; x.mf(); }");
+  const auto Results =
+  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
+}
+
+TES

[PATCH] D45679: [clang-tidy] Add a helper function isModified, that checks whether an expression is modified within a statement.

[Shuai Wang via Phabricator via cfe-commits]

shuaiwang added inline comments.



Comment at: clang-tidy/utils/ExprMutationAnalyzer.cpp:82-83
+const auto *E = RefNodes.getNodeAs("expr");
+if (findMutation(E))
+  return E;
+  }

aaron.ballman wrote:
> Why does this not return the result of `findMutation()` like the other call 
> above?
find*Mutation is intended to return where the given Expr is mutated.
To make it a bit more useful, for Decl's I'm returning the DeclRefExpr and 
caller can chose to follow it to find where the DeclRefExpr is mutated if 
needed.
As a concrete example:
```
struct A { int x; };
struct B { A a; };
struct C { B b; };
C c;
C& c2 = c;
c2.b.a.x = 10;
```
If we start with DeclRefExpr to `c` we'll find mutation at a DeclRefExpr to 
`c2`, following that we can find the mutation Stmt being `c2.b.a.x = 10`.
Returning `E` here makes it possible to reveal intermediate checkpoint instead 
of directly saying `c` is mutated at `c2.b.a.x = 10` which might be confusing.



Comment at: clang-tidy/utils/ExprMutationAnalyzer.cpp:88
+
+const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) {
+  // LHS of any assignment operators.

aaron.ballman wrote:
> Should this also consider a DeclRefExpr to a volatile-qualified variable as a 
> direct mutation?
> 
> What about using `Expr::HasSideEffect()`?
Good catch about DeclRefExpr to volatile.

`HasSideEffects` means something different. Here find*Mutation means find a 
Stmt **in ancestors** that mutates the given Expr. `HasSideEffects` IIUC means 
whether anything is mutated by the Expr but doesn't care about what exactly is 
mutated.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45679



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

[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: lib/CodeGen/CGExprConstant.cpp:1413
+  } else if (!Init->isEvaluatable(CE.CGM.getContext())) {
+return false;
+  } else if (InitTy->hasPointerRepresentation()) {

sepavloff wrote:
> rjmccall wrote:
> > Aren't the null-pointer and integer-constant-expression checks below 
> > already checking this?  Also, `isEvaluatable` actually computes the full 
> > value internally (as an `APValue`), so if you're worried about the memory 
> > and compile-time effects of producing such a value, you really shouldn't 
> > call it.
> > 
> > You could reasonably move this entire function to be a method on `Expr` 
> > that takes an `ASTContext`.
> Comment for `EvaluateAsRValue` says that it tries calculate expression 
> agressively. Indeed, for the code:
> ```
>   decltype(nullptr) null();
>   int *p = null();
> ```
> compiler ignores potential side effect of `null()` and removes the call, 
> leaving only zero initialization. `isNullPointerConstant` behaves similarly.
Nonetheless, it looks like this function could evaluate `Init` up to three 
times, which seems unreasonable. Instead of the checks based on trying to 
evaluate the initializer (`isNullPointerConstant` + `isIntegerConstantExpr`), 
how about calling `VarDecl::evaluateValue()` (which will return a potentially 
pre-computed and cached initializer value) and checking if the result is a zero 
constant?

In fact, `tryEmitPrivateForVarInit` already does most of that for you, and the 
right place to make this change is probably in `tryEmitPrivateForMemory`, where 
you can test to see if the `APValue` is zero-initialized and produce a 
`zeroinitializer` if so. As a side-benefit, putting the change there will mean 
we'll also start using `zeroinitializer` for zero-initialized subobjects of 
objects that have non-zero pieces.


Repository:
  rC Clang

https://reviews.llvm.org/D46241



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

[PATCH] D45860: [Coroutines] Catch exceptions in await_resume

[Gor Nishanov via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D45860



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

[PATCH] D45463: [AST] Print correct tag decl for tag specifier

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

> TagSpecifierAs expands sizeof(PrintingPolicy) from 8 to 16 bytes. My concern 
> is the comments on PrintingPolicy about how PrintingPolicy is intended to be 
> small. My guess it that 16 bytes is still small enough, but perhaps Richard 
> Smith, who wrote that comment, knows better.

This seems fine. See r270009 for some background for that comment -- we used to 
store a copy of the `LangOptions` in the `PrintingPolicy` (among other things), 
and copying these objects (including the potentially-large vectors within the 
`LangOptions`) was a double-digit percentage of the compile time of some 
compilations. That's a very different ball park from a change from 8 bytes to 
16 bytes.




Comment at: include/clang-c/Index.h:4116
   CXPrintingPolicy_SuppressTagKeyword,
-  CXPrintingPolicy_IncludeTagDefinition,
   CXPrintingPolicy_SuppressScope,

This is not OK: this would be an ABI break for the stable libclang ABI.



Comment at: lib/AST/DeclPrinter.cpp:180-181
 if (isFirst) {
-  if(TD)
-SubPolicy.IncludeTagDefinition = true;
+  if (TD)
+SubPolicy.TagSpecifierAs = TD;
   SubPolicy.SuppressSpecifiers = false;

Instead of the changes in this patch, can you address your use case by just 
changing the `if` here to `if (TD && TD->isThisDeclarationADefinition())`?


https://reviews.llvm.org/D45463



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

[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Given that these are overloaded builtins, why are there separate builtins for 
`min` and `umin`?

If this is a Clang extension, it needs to be documented in our extensions 
documentation.

The documentation should describe the exact ordering semantics, to the extent 
we want to guarantee them.  For example, does `__atomic_fetch_max` order this 
operation with later operations even if the new value is in fact less than the 
current value of the variable, or does it use some treatment more like the 
failure case of a compare-exchange?

The documentation should describe the set of allowed types.  If the builtins 
work on pointer types, the documentation should describe the semantics of the 
pointer comparison; for example, is it undefined behavior if an ordinary 
pointer comparison would be?  Also, your test case should actually check the 
well-formedness conditions more completely, e.g. verifying that the value type 
is convertible to the atomic type.


Repository:
  rC Clang

https://reviews.llvm.org/D46386



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

[PATCH] D45131: [AST] Refactor UnaryTransformType into TransformTraitType supporting non-unary transforms

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: include/clang/AST/TypeLoc.h:1992-1993
+  unsigned getExtraLocalDataAlignment() const {
+static_assert(alignof(TransformTraitTypeLoc) >= alignof(TypeSourceInfo *),
+  "not enough alignment for tail-allocated data");
+return alignof(TypeSourceInfo *);

This assert doesn't make sense: the extra `TypeSourceInfo*`s are not stored 
after a `TransformTraitTypeLoc`. Rather, `*TypeLoc` is a non-owning veneer 
around separately-allocated data. `ConcreteTypeLoc` takes care of adding any 
necessary padding to get to the right alignment for you.



Comment at: include/clang/ASTMatchers/ASTMatchers.h:5262-5266
+AST_MATCHER(Type, unaryTransformType) {
+  if (const auto *T = Node.getAs())
+return T->getNumArgs() == 1;
+  return false;
+}

Generally I think we want the primitive AST matchers to pretty directly 
correspond to the Clang AST. I think it would make sense to replace this 
matcher with a generalized `transformTrait` matcher -- but in a separate commit 
from this one. This is OK for now.



Comment at: lib/AST/ASTContext.cpp:4630
 
-/// getUnaryTransformationType - We don't unique these, since the memory
+/// getTransformTraitType - We don't unique these, since the memory
 /// savings are minimal and these are rare.

You can just drop the function name (and hyphen) here; that's an obsolescent 
style.



Comment at: lib/AST/ItaniumMangle.cpp:3250-3251
 
-  mangleType(T->getBaseType());
+  for (auto Ty : T->getArgs())
+mangleType(Ty);
 }

EricWF wrote:
> EricWF wrote:
> > rsmith wrote:
> > > We need manglings to be self-delimiting, and we can't tell where the end 
> > > of the argument list is with this mangling approach. :(
> > > 
> > > (Eg, `f(__raw_invocation_type(T1, T2, T3))` and 
> > > `f(__raw_invocation_type(T1, T2), T3)` would mangle the same with this 
> > > mangling.)
> > > 
> > > Maybe drop an `E` on the end? (Or maybe do that only for traits that 
> > > don't require exactly one argument, but that would create pain for 
> > > demanglers)
> > Makes sense. Thanks for the explanation. I'll go ahead and drop and E on 
> > the end.
> > 
> > However, will this cause ABI issues? Either with existing code or with GCC? 
> > If so, perhaps we should maintain the current mangling for 
> > `__underlying_type`.
> To answer my own question, GCC doesn't mangle `__underlying_type` yet. And I 
> doubt that people are currently depending on the dependent mangling of 
> `__underlying_type`. Perhaps I'm wrong about that last assumption though.
The existing `U3eut` mangling (with no terminating `E`) was approximately OK, 
following 
http://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangle.qualified-type. It 
would be better to use `U17__underlying_type`, though. This is not exactly 
right, since it treats `__underlying_type` as a type qualifier rather than a 
type function, but that's not too far off.

The new mangling doesn't match the Itanium ABI rule for vendor extensions. We 
basically have a choice between `u`  (which doesn't give us a good 
way to encode the arguments) or `U`(which 
could work, but we'd need to make a somewhat-arbitrary choice of which type we 
consider to be the "main" type to which the qualifier applies).

We could arbitrarily say:

 * the first type in the trait is the "main" type
 * the rest of the trait is mangled as a qualifier
 * the qualifier is given template-args if and only if there is more than one 
argument

So:

 * `__underlying_type(T)` would mangle as `U17__underlying_type1T` (or 
approximately `T __underlying_type`)
 * `__raw_invocation_type(F, A1, A2)` would mangle as 
`U21__raw_invocation_typeI2A12A2E1F` (or approximately `F 
__raw_invocation_type`)
 * `__raw_invocation_type(F)` would mangle as `U21__raw_invocation_type1F` (or 
approximately `F __raw_invocation_type`)

Or we could track here whether the trait can accept >1 argument, and always use 
the template-args formulation if so. I have no strong opinions either way.

Or we could suggest an Itanium ABI extension to permit  for the 
`u...` vendor extension type form, for vendor type traits. That'd lead to:

 * `__underlying_type(T)` would mangle as `u17__underlying_typeI1TE` (or 
approximately `__underlying_type`)
 * `__raw_invocation_type(F, A1, A2)` would mangle as 
`u21__raw_invocation_typeI1F2A12A2E` (or approximately 
`__raw_invocation_type`)
 * `__raw_invocation_type(F)` would mangle as `u21__raw_invocation_typeI1FE` 
(or approximately `__raw_invocation_type`)

... and we could encourage demanglers to use parens instead of angles for those 
arguments.


The final of the above options seems best to me. What do you think?



Comment at: lib/AST/Type.cpp:3081-3082
+new ((void *)(ArgStorage + I)) QualType(T);
+if (T->isDependentType())
+  this->setDependent(true);
+if (T->isInstantiati

[PATCH] D46410: [Target] Diagnose mismatch in required CPU for always_inline functions

[Craig Topper via Phabricator via cfe-commits]

craig.topper created this revision.
craig.topper added reviewers: echristo, erichkeane.

If an always inline function requests a different CPU than its caller we should 
probably error.

If the callee CPU has features that the caller CPU doesn't we would already 
error for the feature mismatch, but it prints a misleading error about the 
first feature that mismatches.
If the callee CPU feature list a subset of the caller features we wouldn't 
error at all.

We also only error right now if the callee as a target attribute, but don't 
check anything if only the caller has a target attribute. This is consistent 
with our previous checking behavior, but we might want to fix that. I've left a 
TODO.

I've simplified some of GetCPUAndFeaturesAttributes since I needed to return 
the CPU string from getFunctionFeatureMap for the other callers anyway. This 
also saves us a second parse of the target attribute when adding attributes.


https://reviews.llvm.org/D46410

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  test/CodeGen/target-cpu-error-2.c
  test/CodeGen/target-cpu-error-3.c
  test/CodeGen/target-cpu-error.c

Index: test/CodeGen/target-cpu-error.c
===
--- /dev/null
+++ test/CodeGen/target-cpu-error.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o -
+int __attribute__((target("arch=silvermont"), always_inline)) foo(int a) {
+  return a + 4;
+}
+int __attribute__((target("arch=nehalem"))) bar() {
+  return foo(4); // expected-error {{always_inline function 'foo' requires target CPU 'silvermont', but would be inlined into function 'bar' that is compiled for 'nehalem'}}
+}
+
Index: test/CodeGen/target-cpu-error-3.c
===
--- /dev/null
+++ test/CodeGen/target-cpu-error-3.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o - -target-cpu silvermont
+int __attribute__((target("sse4.2"), always_inline)) foo(int a) {
+  return a + 4;
+}
+int __attribute__((target("arch=nehalem"))) bar() {
+  return foo(4); // expected-error {{always_inline function 'foo' requires target CPU 'silvermont', but would be inlined into function 'bar' that is compiled for 'nehalem'}}
+}
+
Index: test/CodeGen/target-cpu-error-2.c
===
--- /dev/null
+++ test/CodeGen/target-cpu-error-2.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o - -target-cpu nehalem
+int __attribute__((target("arch=silvermont"), always_inline)) foo(int a) {
+  return a + 4;
+}
+int bar() {
+  return foo(4); // expected-error {{always_inline function 'foo' requires target CPU 'silvermont', but would be inlined into function 'bar' that is compiled for 'nehalem'}}
+}
+
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -1083,9 +1083,10 @@
   void AddDefaultFnAttrs(llvm::Function &F);
 
   // Fills in the supplied string map with the set of target features for the
-  // passed in function.
-  void getFunctionFeatureMap(llvm::StringMap &FeatureMap,
- const FunctionDecl *FD);
+  // passed in function. Also fills in the target CPU.
+  void getFunctionCPUAndFeatureMap(llvm::StringMap &FeatureMap,
+   StringRef &TargetCPU,
+   const FunctionDecl *FD);
 
   StringRef getMangledName(GlobalDecl GD);
   StringRef getBlockMangledName(GlobalDecl GD, const BlockDecl *BD);
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -1282,23 +1282,15 @@
   bool AddedAttr = false;
   if (TD) {
 llvm::StringMap FeatureMap;
-getFunctionFeatureMap(FeatureMap, FD);
+getFunctionCPUAndFeatureMap(FeatureMap, TargetCPU, FD);
 
 // Produce the canonical string for this set of features.
 for (const llvm::StringMap::value_type &Entry : FeatureMap)
   Features.push_back((Entry.getValue() ? "+" : "-") + Entry.getKey().str());
-
-// Now add the target-cpu and target-features to the function.
-// While we populated the feature map above, we still need to
-// get and parse the target attribute so we can get the cpu for
-// the function.
-TargetAttr::ParsedTargetAttr ParsedAttr = TD->parse();
-if (ParsedAttr.Architecture != "" &&
-getTarget().isValidCPUName(ParsedAttr.Architecture))
-  TargetCPU = ParsedAttr.Architecture;
   } else {
 // Otherwise just add the existing target cpu and target features to the
 // function.
+TargetCPU = getTarget().getTargetOpts().CPU;
 Features = getTarget().getTargetOpts().Features;
   }
 
@@ -4988,9 +4980

[PATCH] D45470: Emit an error when mixing and

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

In https://reviews.llvm.org/D45470#1087026, @jfb wrote:

> This isn't bad, so I'd go with it, but separately I imagine that we could 
> implement the suggestion in http://wg21.link/p0943 and expose it even before 
> C++20? Not sure we do this much, but I'd argue that before that fix 
> stdatomic.h is just useless anyways, so it's a fine breakage. I imagine that 
> the stdatomic.h where it's implemented would be the one injected by clang, 
> not the libc++ one?


Change visible here is for the header injected by clang. libc++ change is 
r331379  but that error triggers only when 
you consciously decided to opt in C atomics but included C++ ``.

I am concerned that the change adds the error to the valid code that uses C 
atomics from C++ and doesn't mix them with C++ atomics. It is unfortunate but I 
think it is a right trade-off. Adding `__ALLOW_STDC_ATOMICS_IN_CXX__` to fix 
such code shouldn't take much time. While having an explicit error can save a 
lot of time trying to figure out a broken build. Also I expect that with time 
libc++ will use `` more and mixing currently safe parts of libc++ with 
`` is likely to cause problems at some point. So it is better to 
be prepared and have an explicit error for this case.


Repository:
  rC Clang

https://reviews.llvm.org/D45470



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

[PATCH] D46363: Follow-up to r331378. Update tests to allow to use C atomics in C++.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Thanks for the quick review.


Repository:
  rL LLVM

https://reviews.llvm.org/D46363



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

[PATCH] D46363: Follow-up to r331378. Update tests to allow to use C atomics in C++.

[Volodymyr Sapsai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL331484: Follow-up to r331378. Update tests to allow to use C 
atomics in C++. (authored by vsapsai, committed by ).
Herald added subscribers: llvm-commits, delcypher.

Changed prior to commit:
  https://reviews.llvm.org/D46363?vs=144912&id=145094#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D46363

Files:
  compiler-rt/trunk/test/tsan/Darwin/gcd-groups-destructor.mm
  compiler-rt/trunk/test/tsan/Darwin/libcxx-shared-ptr-stress.mm
  compiler-rt/trunk/test/tsan/Darwin/xpc-race.mm


Index: compiler-rt/trunk/test/tsan/Darwin/xpc-race.mm
===
--- compiler-rt/trunk/test/tsan/Darwin/xpc-race.mm
+++ compiler-rt/trunk/test/tsan/Darwin/xpc-race.mm
@@ -1,4 +1,4 @@
-// RUN: %clangxx_tsan %s -o %t -framework Foundation
+// RUN: %clangxx_tsan %s -o %t -framework Foundation 
-D__ALLOW_STDC_ATOMICS_IN_CXX__
 // RUN: %deflake %run %t 2>&1 | FileCheck %s
 
 // UNSUPPORTED: ios
Index: compiler-rt/trunk/test/tsan/Darwin/libcxx-shared-ptr-stress.mm
===
--- compiler-rt/trunk/test/tsan/Darwin/libcxx-shared-ptr-stress.mm
+++ compiler-rt/trunk/test/tsan/Darwin/libcxx-shared-ptr-stress.mm
@@ -1,4 +1,4 @@
-// RUN: %clangxx_tsan %s -o %t -framework Foundation
+// RUN: %clangxx_tsan %s -o %t -framework Foundation 
-D__ALLOW_STDC_ATOMICS_IN_CXX__
 // RUN: %run %t 2>&1 | FileCheck %s
 
 #import 
Index: compiler-rt/trunk/test/tsan/Darwin/gcd-groups-destructor.mm
===
--- compiler-rt/trunk/test/tsan/Darwin/gcd-groups-destructor.mm
+++ compiler-rt/trunk/test/tsan/Darwin/gcd-groups-destructor.mm
@@ -1,4 +1,4 @@
-// RUN: %clangxx_tsan %s -o %t -framework Foundation
+// RUN: %clangxx_tsan %s -o %t -framework Foundation 
-D__ALLOW_STDC_ATOMICS_IN_CXX__
 // RUN: %run %t 2>&1 | FileCheck %s
 
 #import 


Index: compiler-rt/trunk/test/tsan/Darwin/xpc-race.mm
===
--- compiler-rt/trunk/test/tsan/Darwin/xpc-race.mm
+++ compiler-rt/trunk/test/tsan/Darwin/xpc-race.mm
@@ -1,4 +1,4 @@
-// RUN: %clangxx_tsan %s -o %t -framework Foundation
+// RUN: %clangxx_tsan %s -o %t -framework Foundation -D__ALLOW_STDC_ATOMICS_IN_CXX__
 // RUN: %deflake %run %t 2>&1 | FileCheck %s
 
 // UNSUPPORTED: ios
Index: compiler-rt/trunk/test/tsan/Darwin/libcxx-shared-ptr-stress.mm
===
--- compiler-rt/trunk/test/tsan/Darwin/libcxx-shared-ptr-stress.mm
+++ compiler-rt/trunk/test/tsan/Darwin/libcxx-shared-ptr-stress.mm
@@ -1,4 +1,4 @@
-// RUN: %clangxx_tsan %s -o %t -framework Foundation
+// RUN: %clangxx_tsan %s -o %t -framework Foundation -D__ALLOW_STDC_ATOMICS_IN_CXX__
 // RUN: %run %t 2>&1 | FileCheck %s
 
 #import 
Index: compiler-rt/trunk/test/tsan/Darwin/gcd-groups-destructor.mm
===
--- compiler-rt/trunk/test/tsan/Darwin/gcd-groups-destructor.mm
+++ compiler-rt/trunk/test/tsan/Darwin/gcd-groups-destructor.mm
@@ -1,4 +1,4 @@
-// RUN: %clangxx_tsan %s -o %t -framework Foundation
+// RUN: %clangxx_tsan %s -o %t -framework Foundation -D__ALLOW_STDC_ATOMICS_IN_CXX__
 // RUN: %run %t 2>&1 | FileCheck %s
 
 #import 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46286: [Driver] Don't warn about unused inputs in config files

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

@sepavloff - does the additional change to this one also look fine to you?


https://reviews.llvm.org/D46286



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

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added inline comments.



Comment at: lib/CodeGen/CGExprAgg.cpp:1002
+  return EmitFinalDestCopy(
+  E->getType(), CGF.MakeNaturalAlignAddrLValue(Select, E->getType()));
+}

EricWF wrote:
> rjmccall wrote:
> > Is there something in Sema actually validating that the comparison types is 
> > trivially copyable?  Because EmitFinalDestCopy does not work on non-trivial 
> > types.
> > 
> > Also, are we certain that we're allowed to do a copy from an actual global 
> > variable here instead of potentially a constexpr evaluation of the variable 
> > reference?  And if we are doing a copy, are we registering ODR-uses of all 
> > the variables in Sema?
> > 
> > I don't think you should worry about trying to generate a select between 
> > the "actual" comparison result values.  At least not yet.
> There is nothing is Sema validating that these comparison types are trivially 
> copyable, and according to the standard they don't need to be.
> I assumed we only ended up in `CGExprAgg` if the types were trivially 
> copyable. But I'll look into implementing this for non-trivially copyable 
> comparison types (although we'll probably never actually encounter them).
> 
> > Also, are we certain that we're allowed to do a copy from an actual global 
> > variable here instead of potentially a constexpr evaluation of the variable 
> > reference?
> 
> I'm not sure exactly what this means. How would I observe the difference?
> 
> >And if we are doing a copy, are we registering ODR-uses of all the variables 
> >in Sema?
> 
> Probably not. I'll double check that.
> 
> > I don't think you should worry about trying to generate a select between 
> > the "actual" comparison result values. At least not yet.
> 
> I'm not sure exactly what you mean by this.
To follow up:

>> And if we are doing a copy, are we registering ODR-uses of all the variables 
>> in Sema?
>
> Probably not. I'll double check that.

We do mark the potential result variables as ODR-used when we first check them 
when building builtin and defaulted comparison operators.

>> I don't think you should worry about trying to generate a select between the 
>> "actual" comparison result values. At least not yet.
>
> I'm not sure exactly what you mean by this.

Sorry, I see what you mean. You're talking about the comment. Richard asked me 
to leave that TODO there.


https://reviews.llvm.org/D45476



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

[PATCH] D45470: Emit an error when mixing and

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

This isn't bad, so I'd go with it, but separately I imagine that we could 
implement the suggestion in http://wg21.link/p0943 and expose it even before 
C++20? Not sure we do this much, but I'd argue that before that fix stdatomic.h 
is just useless anyways, so it's a fine breakage. I imagine that the 
stdatomic.h where it's implemented would be the one injected by clang, not the 
libc++ one?


Repository:
  rC Clang

https://reviews.llvm.org/D45470



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

[PATCH] D45131: [AST] Refactor UnaryTransformType into TransformTraitType supporting non-unary transforms

[Eric Fiselier via Phabricator via cfe-commits]

EricWF marked 12 inline comments as done.
EricWF added a comment.

Ping.


https://reviews.llvm.org/D45131



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

[PATCH] D45463: [AST] Print correct tag decl for tag specifier

[Joel E. Denny via Phabricator via cfe-commits]

jdenny updated this revision to Diff 145092.
jdenny removed a reviewer: jbcoe.
jdenny added a comment.

Rebased.  Ping.


https://reviews.llvm.org/D45463

Files:
  include/clang-c/Index.h
  include/clang/AST/PrettyPrinter.h
  lib/AST/DeclPrinter.cpp
  lib/AST/TypePrinter.cpp
  test/Misc/ast-print-enum-decl.c
  test/Misc/ast-print-record-decl.c
  tools/c-index-test/c-index-test.c
  tools/libclang/CIndex.cpp

Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -4743,8 +4743,6 @@
 return P->SuppressSpecifiers;
   case CXPrintingPolicy_SuppressTagKeyword:
 return P->SuppressTagKeyword;
-  case CXPrintingPolicy_IncludeTagDefinition:
-return P->IncludeTagDefinition;
   case CXPrintingPolicy_SuppressScope:
 return P->SuppressScope;
   case CXPrintingPolicy_SuppressUnwrittenScope:
@@ -4812,9 +4810,6 @@
   case CXPrintingPolicy_SuppressTagKeyword:
 P->SuppressTagKeyword = Value;
 return;
-  case CXPrintingPolicy_IncludeTagDefinition:
-P->IncludeTagDefinition = Value;
-return;
   case CXPrintingPolicy_SuppressScope:
 P->SuppressScope = Value;
 return;
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -97,8 +97,6 @@
CXPrintingPolicy_SuppressSpecifiers},
   {"CINDEXTEST_PRINTINGPOLICY_SUPPRESSTAGKEYWORD",
CXPrintingPolicy_SuppressTagKeyword},
-  {"CINDEXTEST_PRINTINGPOLICY_INCLUDETAGDEFINITION",
-   CXPrintingPolicy_IncludeTagDefinition},
   {"CINDEXTEST_PRINTINGPOLICY_SUPPRESSSCOPE",
CXPrintingPolicy_SuppressScope},
   {"CINDEXTEST_PRINTINGPOLICY_SUPPRESSUNWRITTENSCOPE",
Index: test/Misc/ast-print-record-decl.c
===
--- /dev/null
+++ test/Misc/ast-print-record-decl.c
@@ -0,0 +1,251 @@
+// Check struct:
+//
+//   First check compiling and printing of this file.
+//
+//   RUN: %clang -Xclang -verify -S -emit-llvm -DKW=struct -DBASES= -o - %s \
+//   RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
+//
+//   RUN: %clang_cc1 -verify -ast-print -DKW=struct -DBASES= %s > %t.c
+//   RUN: FileCheck --check-prefixes=CHECK,PRINT -DKW=struct -DBASES= %s \
+//   RUN:   --input-file %t.c
+//
+//   Now check compiling and printing of the printed file.
+//
+//   RUN: echo "// expected""-warning@* 10 {{'T' is deprecated}}" >> %t.c
+//   RUN: echo "// expected""-note@* 10 {{'T' has been explicitly marked deprecated here}}" >> %t.c
+//
+//   RUN: %clang -Xclang -verify -S -emit-llvm -o - %t.c \
+//   RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
+//
+//   RUN: %clang_cc1 -verify -ast-print %t.c \
+//   RUN: | FileCheck --check-prefixes=CHECK,PRINT -DKW=struct -DBASES= %s
+
+// Repeat for union:
+//
+//   First check compiling and printing of this file.
+//
+//   RUN: %clang -Xclang -verify -S -emit-llvm -DKW=union -DBASES= -o - %s \
+//   RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
+//
+//   RUN: %clang_cc1 -verify -ast-print -DKW=union -DBASES= %s > %t.c
+//   RUN: FileCheck --check-prefixes=CHECK,PRINT -DKW=union -DBASES= %s \
+//   RUN:   --input-file %t.c
+//
+//   Now check compiling and printing of the printed file.
+//
+//   RUN: echo "// expected""-warning@* 10 {{'T' is deprecated}}" >> %t.c
+//   RUN: echo "// expected""-note@* 10 {{'T' has been explicitly marked deprecated here}}" >> %t.c
+//
+//   RUN: %clang -Xclang -verify -S -emit-llvm -o - %t.c \
+//   RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
+//
+//   RUN: %clang_cc1 -verify -ast-print %t.c \
+//   RUN: | FileCheck --check-prefixes=CHECK,PRINT -DKW=union -DBASES= %s
+
+// Repeat for C++ (BASES helps ensure we're printing as C++ not as C):
+//
+//   First check compiling and printing of this file.
+//
+//   RUN: %clang -Xclang -verify -S -emit-llvm -DKW=struct -DBASES=' : B' -o - \
+//   RUN:-xc++ %s \
+//   RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
+//
+//   RUN: %clang_cc1 -verify -ast-print -DKW=struct -DBASES=' : B' -xc++ %s \
+//   RUN: > %t.cpp
+//   RUN: FileCheck --check-prefixes=CHECK,PRINT,CXX -DKW=struct \
+//   RUN:   -DBASES=' : B' %s --input-file %t.cpp
+//
+//   Now check compiling and printing of the printed file.
+//
+//   RUN: echo "// expected""-warning@* 10 {{'T' is deprecated}}" >> %t.cpp
+//   RUN: echo "// expected""-note@* 10 {{'T' has been explicitly marked deprecated here}}" >> %t.cpp
+//
+//   RUN: %clang -Xclang -verify -S -emit-llvm -o - %t.cpp \
+//   RUN: | FileCheck --check-prefixes=CHECK,LLVM %s
+//
+//   RUN: %clang_cc1 -verify -ast-print %t.cpp \
+//   RUN: | FileCheck --check-prefixes=CHECK,PRINT,CXX -DKW=struct \
+//   RUN: -DBASES=' : B' %s
+
+// END.
+
+#ifndef KW
+# error KW undefined
+# define KW struct // help syntax checkers
+#endif
+
+#ifndef BASES
+# error BASES undefined
+# 

r331483 - [X86] Make __builtin_ia32_directstore_u32 and __builtin_ia32_movdir64b 'nothrow'

[Craig Topper via cfe-commits]

Author: ctopper
Date: Thu May  3 14:01:35 2018
New Revision: 331483

URL: http://llvm.org/viewvc/llvm-project?rev=331483&view=rev
Log:
[X86] Make __builtin_ia32_directstore_u32 and __builtin_ia32_movdir64b 'nothrow'

These builtins snuck in while I was in the middle of adding nothrow to the 
other builtins in my local clone and I guess I missed them.

Modified:
cfe/trunk/include/clang/Basic/BuiltinsX86.def

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=331483&r1=331482&r2=331483&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Thu May  3 14:01:35 2018
@@ -1884,8 +1884,8 @@ TARGET_BUILTIN(__builtin_ia32_clzero, "v
 TARGET_BUILTIN(__builtin_ia32_cldemote, "vvC*", "n", "cldemote")
 
 // Direct Move
-TARGET_BUILTIN(__builtin_ia32_directstore_u32, "vUi*Ui", "", "movdiri")
-TARGET_BUILTIN(__builtin_ia32_movdir64b, "vv*vC*", "", "movdir64b")
+TARGET_BUILTIN(__builtin_ia32_directstore_u32, "vUi*Ui", "n", "movdiri")
+TARGET_BUILTIN(__builtin_ia32_movdir64b, "vv*vC*", "n", "movdir64b")
 
 // MSVC
 TARGET_HEADER_BUILTIN(_BitScanForward, "UcUNi*UNi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")


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

r331482 - [CodeGenFunction] Use the StringRef::split function that takes a char separator instead of StringRef separator. NFC

[Craig Topper via cfe-commits]

Author: ctopper
Date: Thu May  3 14:01:33 2018
New Revision: 331482

URL: http://llvm.org/viewvc/llvm-project?rev=331482&view=rev
Log:
[CodeGenFunction] Use the StringRef::split function that takes a char separator 
instead of StringRef separator. NFC

The char separator version should be a little better optimized.

Modified:
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=331482&r1=331481&r2=331482&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Thu May  3 14:01:33 2018
@@ -2284,7 +2284,7 @@ static bool hasRequiredFeatures(const Sm
   return std::all_of(
   ReqFeatures.begin(), ReqFeatures.end(), [&](StringRef Feature) {
 SmallVector OrFeatures;
-Feature.split(OrFeatures, "|");
+Feature.split(OrFeatures, '|');
 return std::any_of(OrFeatures.begin(), OrFeatures.end(),
[&](StringRef Feature) {
  if (!CallerFeatureMap.lookup(Feature)) {
@@ -2322,7 +2322,7 @@ void CodeGenFunction::checkTargetFeature
 // Return if the builtin doesn't have any required features.
 if (!FeatureList || StringRef(FeatureList) == "")
   return;
-StringRef(FeatureList).split(ReqFeatures, ",");
+StringRef(FeatureList).split(ReqFeatures, ',');
 if (!hasRequiredFeatures(ReqFeatures, CGM, FD, MissingFeature))
   CGM.getDiags().Report(E->getLocStart(), diag::err_builtin_needs_feature)
   << TargetDecl->getDeclName()


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

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added inline comments.



Comment at: lib/CodeGen/CGExprAgg.cpp:1002
+  return EmitFinalDestCopy(
+  E->getType(), CGF.MakeNaturalAlignAddrLValue(Select, E->getType()));
+}

rjmccall wrote:
> Is there something in Sema actually validating that the comparison types is 
> trivially copyable?  Because EmitFinalDestCopy does not work on non-trivial 
> types.
> 
> Also, are we certain that we're allowed to do a copy from an actual global 
> variable here instead of potentially a constexpr evaluation of the variable 
> reference?  And if we are doing a copy, are we registering ODR-uses of all 
> the variables in Sema?
> 
> I don't think you should worry about trying to generate a select between the 
> "actual" comparison result values.  At least not yet.
There is nothing is Sema validating that these comparison types are trivially 
copyable, and according to the standard they don't need to be.
I assumed we only ended up in `CGExprAgg` if the types were trivially copyable. 
But I'll look into implementing this for non-trivially copyable comparison 
types (although we'll probably never actually encounter them).

> Also, are we certain that we're allowed to do a copy from an actual global 
> variable here instead of potentially a constexpr evaluation of the variable 
> reference?

I'm not sure exactly what this means. How would I observe the difference?

>And if we are doing a copy, are we registering ODR-uses of all the variables 
>in Sema?

Probably not. I'll double check that.

> I don't think you should worry about trying to generate a select between the 
> "actual" comparison result values. At least not yet.

I'm not sure exactly what you mean by this.



Comment at: lib/Sema/SemaExpr.cpp:9795
+  if (int Count = LHSType->isBooleanType() + RHSType->isBooleanType()) {
+// TODO: What about bool non-narrowing cases? Like '0' or '1.
+if (Count != 2) {

rsmith wrote:
> Missing `'`. Seems like a question to take to Herb and CWG (and maybe EWG), 
> but I suspect the answer will be "this does what we wanted".
> 
> Looks like P0946R0 missed this case of a difference between `<=>` and other 
> operators... oops.
I'll remove the comment for now then.



Comment at: lib/Sema/SemaExpr.cpp:9906
+  bool IsThreeWay = Opc == BO_Cmp;
+  auto IsPointerType = [](ExprResult E) {
+QualType Ty = E.get()->getType().getNonReferenceType();

rsmith wrote:
> I'd prefer a name that captures that this also recognizes member pointer 
> types.
I'm bad at naming things. Is `IsAnyPointerType` better?


https://reviews.llvm.org/D45476



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

[PATCH] D45931: [ASTMatchers] Don't garble the profiling output when multiple TU's are processed

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D45931#1086665, @alexfh wrote:

> In https://reviews.llvm.org/D45931#1084503, @lebedev.ri wrote:
>
> > In https://reviews.llvm.org/D45931#1083192, @lebedev.ri wrote:
> >
> > > Thank you for looking at this.
> > >
> > > In https://reviews.llvm.org/D45931#1083184, @alexfh wrote:
> > >
> > > > From a user's perspective I'd probably prefer a different behavior of 
> > > > checks profiling with multiple translation units: per-file table after 
> > > > each file and an aggregate table at the end.
> > >
> > >
> > > Is this a review note, or a general observation?
> >
>
>
> Why not both? ;)


BTW, that did not answer the question:

In https://reviews.llvm.org/D45931#1084503, @lebedev.ri wrote:

> In https://reviews.llvm.org/D45931#1083192, @lebedev.ri wrote:
>
> > Thank you for looking at this.
> >
> > In https://reviews.llvm.org/D45931#1083184, @alexfh wrote:
> >
> > > From a user's perspective I'd probably prefer a different behavior of 
> > > checks profiling with multiple translation units: per-file table after 
> > > each file and an aggregate table at the end.
> >
> >
> > Is this a review note, or a general observation?
> >
> > I'm sure it could be done, i'm just not //too// sure how useful it would 
> > be, since it seems no one before now even noticed that timing with multiple 
> > TU's was 'broken'.
>
>
> Hi @alexfh. Do i need to make those changes or not?
>  I'd really prefer to have such kind of high-level feedback the sooner the 
> better, to avoid wasting everyone's time.



Repository:
  rC Clang

https://reviews.llvm.org/D45931



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

[PATCH] D46374: Add support for ObjC property name to be a single acronym.

[Yan Zhang via Phabricator via cfe-commits]

Wizard added inline comments.



Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:222
+  [MatchedDecl](std::string const &s) {
+return s == MatchedDecl->getName();
+  })) {

benhamilton wrote:
> `s` is a regular expression here, so you need to match it using 
> `llvm::Regex`, not `==`.
> 
> Why not just update `validPropertyNameRegex()` to handle this case?
> 
I would rather not to make the regex more complex as long as the change is 
simple and does bring extra cost. If update the regex it will be something like 
'(originalregex | acronyms)', which seems too much to me.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46374



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

[PATCH] D46406: [docs] Fix typos in the Clang User's Manual.

[Joel Galenson via Phabricator via cfe-commits]

jgalenson created this revision.
jgalenson added reviewers: hans, sylvestre.ledru.

Repository:
  rC Clang

https://reviews.llvm.org/D46406

Files:
  docs/UsersManual.rst


Index: docs/UsersManual.rst
===
--- docs/UsersManual.rst
+++ docs/UsersManual.rst
@@ -1735,11 +1735,11 @@
 .. option:: -fprofile-generate[=]
 
   The ``-fprofile-generate`` and ``-fprofile-generate=`` flags will use
-  an alterantive instrumentation method for profile generation. When
+  an alternative instrumentation method for profile generation. When
   given a directory name, it generates the profile file
   ``default_%m.profraw`` in the directory named ``dirname`` if specified.
   If ``dirname`` does not exist, it will be created at runtime. ``%m`` 
specifier
-  will be substibuted with a unique id documented in step 2 above. In other 
words,
+  will be substituted with a unique id documented in step 2 above. In other 
words,
   with ``-fprofile-generate[=]`` option, the "raw" profile data 
automatic
   merging is turned on by default, so there will no longer any risk of profile
   clobbering from different running processes.  For example,


Index: docs/UsersManual.rst
===
--- docs/UsersManual.rst
+++ docs/UsersManual.rst
@@ -1735,11 +1735,11 @@
 .. option:: -fprofile-generate[=]
 
   The ``-fprofile-generate`` and ``-fprofile-generate=`` flags will use
-  an alterantive instrumentation method for profile generation. When
+  an alternative instrumentation method for profile generation. When
   given a directory name, it generates the profile file
   ``default_%m.profraw`` in the directory named ``dirname`` if specified.
   If ``dirname`` does not exist, it will be created at runtime. ``%m`` specifier
-  will be substibuted with a unique id documented in step 2 above. In other words,
+  will be substituted with a unique id documented in step 2 above. In other words,
   with ``-fprofile-generate[=]`` option, the "raw" profile data automatic
   merging is turned on by default, so there will no longer any risk of profile
   clobbering from different running processes.  For example,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45093: [AST] Fix -ast-print for _Bool when have diagnostics

[Joel E. Denny via Phabricator via cfe-commits]

jdenny updated this revision to Diff 145078.
jdenny edited the summary of this revision.
jdenny added a comment.

Rebased.  Added example to summary.  Ping.


https://reviews.llvm.org/D45093

Files:
  include/clang/AST/ASTContext.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseDecl.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaCodeComplete.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExceptionSpec.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaLookup.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Sema/SemaTemplateInstantiate.cpp
  test/Misc/ast-print-bool.c

Index: test/Misc/ast-print-bool.c
===
--- /dev/null
+++ test/Misc/ast-print-bool.c
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_CBOOL \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-CBOOL,CBOOL
+//
+// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_CBOOL -DDIAG \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-CBOOL,CBOOL
+//
+// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_INT \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-INT,CBOOL
+//
+// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_INT -DDIAG \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-INT,CBOOL
+//
+// RUN: %clang_cc1 -verify -ast-print %s -xc++ \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-BOOL
+//
+// RUN: %clang_cc1 -verify -ast-print %s -xc++ -DDIAG \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-BOOL
+
+#if DEF_BOOL_CBOOL
+# define bool _Bool
+#elif DEF_BOOL_INT
+# define bool int
+#endif
+
+// BOOL-AS-CBOOL: _Bool i;
+// BOOL-AS-INT:   int i;
+// BOOL-AS-BOOL:  bool i;
+bool i;
+
+#ifndef __cplusplus
+// CBOOL: _Bool j;
+_Bool j;
+#endif
+
+// Induce a diagnostic (and verify we actually managed to do so), which used to
+// permanently alter the -ast-print printing policy for _Bool.  How bool is
+// defined by the preprocessor is examined only once per compilation, when the
+// diagnostic is emitted, and it used to affect the entirety of -ast-print, so
+// test only one definition of bool per compilation.
+#if DIAG
+void fn() { 1; } // expected-warning {{expression result unused}}
+#else
+// expected-no-diagnostics
+#endif
Index: lib/Sema/SemaTemplateInstantiate.cpp
===
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -505,7 +505,7 @@
   llvm::raw_svector_ostream OS(TemplateArgsStr);
   Template->printName(OS);
   printTemplateArgumentList(OS, Active->template_arguments(),
-getPrintingPolicy());
+getDiagPrintingPolicy());
   Diags.Report(Active->PointOfInstantiation,
diag::note_default_arg_instantiation_here)
 << OS.str()
@@ -571,7 +571,7 @@
   llvm::raw_svector_ostream OS(TemplateArgsStr);
   FD->printName(OS);
   printTemplateArgumentList(OS, Active->template_arguments(),
-getPrintingPolicy());
+getDiagPrintingPolicy());
   Diags.Report(Active->PointOfInstantiation,
diag::note_default_function_arg_instantiation_here)
 << OS.str()
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -3012,7 +3012,7 @@
   std::string Description;
   {
 llvm::raw_string_ostream Out(Description);
-FailedCond->printPretty(Out, nullptr, getPrintingPolicy());
+FailedCond->printPretty(Out, nullptr, getDiagPrintingPolicy());
   }
   return { FailedCond, Description };
 }
@@ -9835,7 +9835,7 @@
 }
 
 Out << " = ";
-Args[I].print(getPrintingPolicy(), Out);
+Args[I].print(getDiagPrintingPolicy(), Out);
   }
 
   Out << ']';
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -5553,7 +5553,7 @@
 // conversion; use it.
 QualType ConvTy = Conversion->getConversionType().getNonReferenceType();
 std::string TypeStr;
-ConvTy.getAsStringInternal(TypeStr, SemaRef.getPrintingPolicy());
+ConvTy.getAsStringInternal(TypeStr, SemaRef.getDiagPrintingPolicy());
 
 Converter.diagnoseExplicitConv(SemaRef, Loc, T, ConvTy)
 << FixItHint::CreateInsertion(From->getLocStart(),
Index: lib/Sema/SemaLookup.cpp
===
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -4202,7 +4202,7 @@
   std::string NewQualified = TC.getAsString(SemaRef.getLangOpts());
   std::string OldQualified;
   llvm::raw_string_ostream OldOStream(OldQualified);
-  SS->getScopeRep()->print(OldOStream, SemaRef.getPrintingPolicy());
+  SS->getScopeRep()->print(OldOStream, SemaRef.ge

[PATCH] D45835: Add new driver mode for dumping compiler options

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 145077.
aaron.ballman added a comment.

Updated to remove the contentious parts, though I still hope to add those back 
in.


https://reviews.llvm.org/D45835

Files:
  include/clang/Basic/Features.def
  include/clang/Driver/CC1Options.td
  include/clang/Frontend/FrontendActions.h
  include/clang/Frontend/FrontendOptions.h
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/FrontendActions.cpp
  lib/FrontendTool/ExecuteCompilerInvocation.cpp
  lib/Lex/PPMacroExpansion.cpp
  test/Frontend/compiler-options-dump.cpp

Index: test/Frontend/compiler-options-dump.cpp
===
--- test/Frontend/compiler-options-dump.cpp
+++ test/Frontend/compiler-options-dump.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -compiler-options-dump -std=c++03 %s -o - | FileCheck %s --check-prefix=CXX03
+// RUN: %clang_cc1 -compiler-options-dump -std=c++17 %s -o - | FileCheck %s --check-prefix=CXX17
+// RUN: %clang_cc1 -compiler-options-dump -std=c99 -ffast-math -x c %s -o - | FileCheck %s --check-prefix=C99
+
+// CXX03: "features"
+// CXX03: "cxx_auto_type" : false
+// CXX03: "cxx_range_for" : false
+// CXX03: "extensions"
+// CXX03: "cxx_range_for" : true
+
+// CXX17: "features"
+// CXX17: "cxx_auto_type" : true
+// CXX17: "cxx_range_for" : true
+// CXX17: "extensions"
+// CXX17: "cxx_range_for" : true
+
+// C99: "features"
+// C99: "c_alignas" : false
+// C99: "c_atomic" : false
+// C99: "cxx_auto_type" : false
+// C99: "cxx_range_for" : false
+// C99: "extensions"
+// C99: "c_alignas" : true
+// C99: "c_atomic" : true
+// C99: "cxx_range_for" : false
Index: lib/Lex/PPMacroExpansion.cpp
===
--- lib/Lex/PPMacroExpansion.cpp
+++ lib/Lex/PPMacroExpansion.cpp
@@ -1099,187 +1099,11 @@
   if (Feature.startswith("__") && Feature.endswith("__") && Feature.size() >= 4)
 Feature = Feature.substr(2, Feature.size() - 4);
 
+#define FEATURE(Name, Predicate) .Case(#Name, Predicate)
   return llvm::StringSwitch(Feature)
-  .Case("address_sanitizer",
-LangOpts.Sanitize.hasOneOf(SanitizerKind::Address |
-   SanitizerKind::KernelAddress))
-  .Case("hwaddress_sanitizer",
-LangOpts.Sanitize.hasOneOf(SanitizerKind::HWAddress |
-   SanitizerKind::KernelHWAddress))
-  .Case("assume_nonnull", true)
-  .Case("attribute_analyzer_noreturn", true)
-  .Case("attribute_availability", true)
-  .Case("attribute_availability_with_message", true)
-  .Case("attribute_availability_app_extension", true)
-  .Case("attribute_availability_with_version_underscores", true)
-  .Case("attribute_availability_tvos", true)
-  .Case("attribute_availability_watchos", true)
-  .Case("attribute_availability_with_strict", true)
-  .Case("attribute_availability_with_replacement", true)
-  .Case("attribute_availability_in_templates", true)
-  .Case("attribute_cf_returns_not_retained", true)
-  .Case("attribute_cf_returns_retained", true)
-  .Case("attribute_cf_returns_on_parameters", true)
-  .Case("attribute_deprecated_with_message", true)
-  .Case("attribute_deprecated_with_replacement", true)
-  .Case("attribute_ext_vector_type", true)
-  .Case("attribute_ns_returns_not_retained", true)
-  .Case("attribute_ns_returns_retained", true)
-  .Case("attribute_ns_consumes_self", true)
-  .Case("attribute_ns_consumed", true)
-  .Case("attribute_cf_consumed", true)
-  .Case("attribute_objc_ivar_unused", true)
-  .Case("attribute_objc_method_family", true)
-  .Case("attribute_overloadable", true)
-  .Case("attribute_unavailable_with_message", true)
-  .Case("attribute_unused_on_fields", true)
-  .Case("attribute_diagnose_if_objc", true)
-  .Case("blocks", LangOpts.Blocks)
-  .Case("c_thread_safety_attributes", true)
-  .Case("cxx_exceptions", LangOpts.CXXExceptions)
-  .Case("cxx_rtti", LangOpts.RTTI && LangOpts.RTTIData)
-  .Case("enumerator_attributes", true)
-  .Case("nullability", true)
-  .Case("nullability_on_arrays", true)
-  .Case("memory_sanitizer", LangOpts.Sanitize.has(SanitizerKind::Memory))
-  .Case("thread_sanitizer", LangOpts.Sanitize.has(SanitizerKind::Thread))
-  .Case("dataflow_sanitizer",
-LangOpts.Sanitize.has(SanitizerKind::DataFlow))
-  .Case("efficiency_sanitizer",
-LangOpts.Sanitize.hasOneOf(SanitizerKind::Efficiency))
-  .Case("scudo", LangOpts.Sanitize.hasOneOf(SanitizerKind::Scudo))
-  // Objective-C features
-  .Case("objc_arr", LangOpts.ObjCAutoRefCount) // FIXME: REMOVE?
-  .Case("objc_arc", LangOpts.ObjCAutoRefCount)
-  .Case("objc_arc_fields", true)
-  .Case("objc_arc_weak", LangOpts.ObjCWeak)
-  .Case("objc_default_synthesize_properties", LangOpts.ObjC2)
-  .Case("objc_fixed_enum", LangOp

[PATCH] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

In https://reviews.llvm.org/D46403#1086923, @cmtice wrote:

> Ok, I'll work on making this CFI-specific and adding a test case.


Sounds good, thanks! I'd suggest modifying test/Driver/fsanitize-blacklist.c 
for your purposes. It should be possible to pair an empty resource directory 
with -fsanitize=cfi, and to check for a diagnostic.


https://reviews.llvm.org/D46403



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

[PATCH] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.

[Caroline Tice via Phabricator via cfe-commits]

cmtice added a comment.

Ok, I'll work on making this CFI-specific and adding a test case.


https://reviews.llvm.org/D46403



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

[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang

[Elena Demikhovsky via Phabricator via cfe-commits]

delena added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:3098
+  case AtomicExpr::AO__atomic_fetch_umax:
+IsMinMax = true;
+Form = Arithmetic;

jfb wrote:
> Should `__sync_fetch_and_min` and others also set `IsMinMax`?
__sync_fetch_and_min is not variadic and not overloaded. The types of arguments 
are defined with the builtin itself in the def file.
BUILTIN(__sync_fetch_and_min, "iiD*i", "n"). So it is checked automatically.

The other __sync_fetch* functions are overloaded and checked in 
SemaBuiltinAtomicOverloaded()


Repository:
  rC Clang

https://reviews.llvm.org/D46386



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

[PATCH] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.

[Caroline Tice via Phabricator via cfe-commits]

cmtice updated this revision to Diff 145073.
cmtice added a comment.

Tried to upload the diff from the correct location.


https://reviews.llvm.org/D46403

Files:
  lib/Driver/SanitizerArgs.cpp


Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -115,6 +115,8 @@
 llvm::sys::path::append(Path, "share", BL.File);
 if (llvm::sys::fs::exists(Path))
   BlacklistFiles.push_back(Path.str());
+else
+  D.Diag(clang::diag::err_drv_no_such_file) << Path;
   }
 }
 


Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -115,6 +115,8 @@
 llvm::sys::path::append(Path, "share", BL.File);
 if (llvm::sys::fs::exists(Path))
   BlacklistFiles.push_back(Path.str());
+else
+  D.Diag(clang::diag::err_drv_no_such_file) << Path;
   }
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46049: [OpenCL] Add constant address space to __func__ in AST

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl accepted this revision.
yaxunl added a comment.

LGTM. Thanks!


https://reviews.llvm.org/D46049



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

[PATCH] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.

[Vedant Kumar via Phabricator via cfe-commits]

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

Toolchain vendors aren't currently required to provide default blacklists for 
every sanitizer clang supports. We don't ship a default ubsan blacklist on 
macOS, so this patch would break ubsan for all our users.

I think we need to find a different solution here. Have you considered making a 
blacklist mandatory just for CFI? Alternatively, if the blacklist is tiny and 
never changes, perhaps we could embed it within the compiler?


Repository:
  rC Clang

https://reviews.llvm.org/D46403



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

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

[Paul Fultz II via Phabricator via cfe-commits]

pfultz2 updated this revision to Diff 145072.
pfultz2 added a comment.

Rename flag to `AllowEnablingAnalyzerAlphaCheckers`.


https://reviews.llvm.org/D46159

Files:
  clang-tidy/ClangTidy.cpp
  clang-tidy/ClangTidyOptions.cpp
  clang-tidy/ClangTidyOptions.h
  clang-tidy/tool/ClangTidyMain.cpp
  test/clang-tidy/enable-alpha-checks.cpp

Index: test/clang-tidy/enable-alpha-checks.cpp
===
--- /dev/null
+++ test/clang-tidy/enable-alpha-checks.cpp
@@ -0,0 +1,6 @@
+// Check if '-allow-enabling-analyzer-alpha-checkers' is visible for users
+// RUN: clang-tidy -help | not grep 'allow-enabling-analyzer-alpha-checkers'
+
+// Check if '-allow-enabling-analyzer-alpha-checkers' enables alpha checks.
+// RUN: clang-tidy -checks=* -list-checks | not grep 'clang-analyzer-alpha'
+// RUN: clang-tidy -checks=* -list-checks -allow-enabling-analyzer-alpha-checkers | grep 'clang-analyzer-alpha'
Index: clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -192,6 +192,14 @@
cl::init(false),
cl::cat(ClangTidyCategory));
 
+/// This option allows enabling alpha checkers from the static analyzer, that
+/// are experimental. This option is set to false and not visible in help,
+/// because it is highly not recommended for users.
+static cl::opt
+AllowEnablingAnalyzerAlphaCheckers("allow-enabling-analyzer-alpha-checkers",
+   cl::init(false), cl::Hidden,
+   cl::cat(ClangTidyCategory));
+
 static cl::opt ExportFixes("export-fixes", cl::desc(R"(
 YAML file to store suggested fixes in. The
 stored fixes can be applied to the input source
@@ -301,6 +309,8 @@
   DefaultOptions.HeaderFilterRegex = HeaderFilter;
   DefaultOptions.SystemHeaders = SystemHeaders;
   DefaultOptions.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors;
+  DefaultOptions.AllowEnablingAnalyzerAlphaCheckers =
+  AllowEnablingAnalyzerAlphaCheckers;
   DefaultOptions.FormatStyle = FormatStyle;
   DefaultOptions.User = llvm::sys::Process::GetEnv("USER");
   // USERNAME is used on Windows.
Index: clang-tidy/ClangTidyOptions.h
===
--- clang-tidy/ClangTidyOptions.h
+++ clang-tidy/ClangTidyOptions.h
@@ -77,6 +77,9 @@
   /// \brief Turns on temporary destructor-based analysis.
   llvm::Optional AnalyzeTemporaryDtors;
 
+  /// \brief Turns on experimental alpha checkers from the static analyzer.
+  llvm::Optional AllowEnablingAnalyzerAlphaCheckers;
+
   /// \brief Format code around applied fixes with clang-format using this
   /// style.
   ///
Index: clang-tidy/ClangTidyOptions.cpp
===
--- clang-tidy/ClangTidyOptions.cpp
+++ clang-tidy/ClangTidyOptions.cpp
@@ -108,6 +108,7 @@
   Options.HeaderFilterRegex = "";
   Options.SystemHeaders = false;
   Options.AnalyzeTemporaryDtors = false;
+  Options.AllowEnablingAnalyzerAlphaCheckers = false;
   Options.FormatStyle = "none";
   Options.User = llvm::None;
   for (ClangTidyModuleRegistry::iterator I = ClangTidyModuleRegistry::begin(),
@@ -148,6 +149,8 @@
   overrideValue(Result.HeaderFilterRegex, Other.HeaderFilterRegex);
   overrideValue(Result.SystemHeaders, Other.SystemHeaders);
   overrideValue(Result.AnalyzeTemporaryDtors, Other.AnalyzeTemporaryDtors);
+  overrideValue(Result.AllowEnablingAnalyzerAlphaCheckers,
+Other.AllowEnablingAnalyzerAlphaCheckers);
   overrideValue(Result.FormatStyle, Other.FormatStyle);
   overrideValue(Result.User, Other.User);
   mergeVectors(Result.ExtraArgs, Other.ExtraArgs);
Index: clang-tidy/ClangTidy.cpp
===
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -303,11 +303,12 @@
 
 typedef std::vector> CheckersList;
 
-static CheckersList getCheckersControlList(ClangTidyContext &Context) {
+static CheckersList getCheckersControlList(ClangTidyContext &Context,
+   bool IncludeExperimental) {
   CheckersList List;
 
   const auto &RegisteredCheckers =
-  AnalyzerOptions::getRegisteredCheckers(/*IncludeExperimental=*/false);
+  AnalyzerOptions::getRegisteredCheckers(IncludeExperimental);
   bool AnalyzerChecksEnabled = false;
   for (StringRef CheckName : RegisteredCheckers) {
 std::string ClangTidyCheckName((AnalyzerCheckNamePrefix + CheckName).str());
@@ -374,7 +375,8 @@
   AnalyzerOptions->Config["cfg-temporary-dtors"] =
   Context.getOptions().AnalyzeTemporaryDtors ? "true" : "false";
 
-  AnalyzerOptions->CheckersControlList = getCheckersControlList(Context);
+  AnalyzerOptions->CheckersControlList = getCheckersControlList(
+  Context, *Context.getOptions().AllowEnablingAna

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

[Paul Fultz II via Phabricator via cfe-commits]

pfultz2 added inline comments.



Comment at: clang-tidy/ClangTidyOptions.h:80-82
+  /// \brief Turns on experimental alpha checkers from the static analyzer.
+  llvm::Optional AllowEnablingAlphaChecks;
+

alexfh wrote:
> Since this will only be configurable via a flag, this option will be global 
> (i.e. not depend on the location of the file being analyzed). I'd suggest to 
> remove this option altogether and use something else to pass it to 
> ClangTidyASTConsumerFactory. It could be stashed into 
> ClangTidyASTConsumerFactory and passed as a parameter of runClangTidy,  or it 
> could live in ClangTidyContext.
But it needs to be passed to `getCheckNames` as well, which doesn't take a 
`ClangTidyContext`.


https://reviews.llvm.org/D46159



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

[PATCH] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Test?


Repository:
  rC Clang

https://reviews.llvm.org/D46403



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

[PATCH] D46050: [Frontend] Avoid running plugins during code completion parse

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: test/Frontend/plugins.c:7
+
+// RUN: c-index-test -code-completion-at=%s:6:1 -load 
%llvmshlibdir/PrintFunctionNames%pluginext -add-plugin print-fns %s | FileCheck 
-check-prefix=CHECK-COMPLETION-WITHOUT-PLUGINS  %s
+// REQUIRES: plugins, examples

nik wrote:
> Should this test rather go into test/Index because of the c-index-test?
Yes, I think so.



Comment at: test/Frontend/plugins.c:8
+// RUN: c-index-test -code-completion-at=%s:6:1 -load 
%llvmshlibdir/PrintFunctionNames%pluginext -add-plugin print-fns %s | FileCheck 
-check-prefix=CHECK-COMPLETION-WITHOUT-PLUGINS  %s
+// REQUIRES: plugins, examples
+

nik wrote:
> Note that I actually have problems with this REQUIRES line. I use 
> -DCLANG_BUILD_EXAMPLES and -DDBUILD_SHARED_LIBS=ON and this test (and the one 
> above too) is skipped/unsupported. What else do I need?
> 
> Note that If I remove this line, the test is run - apparently the 
> requirements are fulfilled, but not properly detected. I guess this is set up 
> issue on my local machine?
Are you on Windows? plugins is set here: 
http://llvm-cs.pcc.me.uk/tools/clang/test/lit.cfg.py#73
enable_shared here: 
http://llvm-cs.pcc.me.uk/tools/clang/test/lit.site.cfg.py.in#25
from here: http://llvm-cs.pcc.me.uk/cmake/modules/AddLLVM.cmake#1193

So my guess is you need to set LLVM_ENABLE_PLUGINS (and that you are on 
Windows), but I haven't tried it.


Repository:
  rC Clang

https://reviews.llvm.org/D46050



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

[PATCH] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Also, please upload the correct patch, from the root of the repo, not from the 
directory of the file.


Repository:
  rC Clang

https://reviews.llvm.org/D46403



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

[PATCH] D46393: Remove explicit cfg-temporary-dtors=true

[Artem Dergachev via Phabricator via cfe-commits]

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

Thanks!

Just curious - did these flags bother you? Cause we never really care about 
cleaning up run lines after flipping the flag, so we have a lot of such stale 
flags in our tests. We could start cleaning them up if they cause problems.


Repository:
  rC Clang

https://reviews.llvm.org/D46393



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

[PATCH] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.

[Caroline Tice via Phabricator via cfe-commits]

cmtice created this revision.
cmtice added a reviewer: pcc.
Herald added a subscriber: cfe-commits.

Currently LLVM CFI tries to use an implicit blacklist file, currently in 
/usr/lib64/clang//share.  If the file is not there, LLVM happily 
continues, which causes CFI to add checks to files/functions that are known to 
fail, generating binaries that fail.  This CL causes LLVM to die (I hope) if it 
can't find these implicit blacklist files.


Repository:
  rC Clang

https://reviews.llvm.org/D46403

Files:
  SanitizerArgs.cpp


Index: SanitizerArgs.cpp
===
--- SanitizerArgs.cpp
+++ SanitizerArgs.cpp
@@ -115,6 +115,8 @@
 llvm::sys::path::append(Path, "share", BL.File);
 if (llvm::sys::fs::exists(Path))
   BlacklistFiles.push_back(Path.str());
+else
+  D.Diag(clang::diag::err_drv_no_such_file) << Path;
   }
 }
 


Index: SanitizerArgs.cpp
===
--- SanitizerArgs.cpp
+++ SanitizerArgs.cpp
@@ -115,6 +115,8 @@
 llvm::sys::path::append(Path, "share", BL.File);
 if (llvm::sys::fs::exists(Path))
   BlacklistFiles.push_back(Path.str());
+else
+  D.Diag(clang::diag::err_drv_no_such_file) << Path;
   }
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46042: Cap vector alignment at 16 for all Darwin platforms

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I think we should seriously consider making alignment attributes on typedefs 
(and maybe some other attributes like may_alias) actual type qualifiers that 
are preserved in the canonical type, mangled, and so on.  It would be an ABI 
break, but it'd also solve a lot of problems.


Repository:
  rC Clang

https://reviews.llvm.org/D46042



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

[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang

[JF Bastien via Phabricator via cfe-commits]

jfb added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:3098
+  case AtomicExpr::AO__atomic_fetch_umax:
+IsMinMax = true;
+Form = Arithmetic;

Should `__sync_fetch_and_min` and others also set `IsMinMax`?


Repository:
  rC Clang

https://reviews.llvm.org/D46386



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

[PATCH] D46042: Cap vector alignment at 16 for all Darwin platforms

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

> Note that this sort of attribute is stripped from template arguments in 
> template substitution, so there's a possibility that code templated over 
> vectors will produce inadequately-aligned objects.

I was wondering whether there is a warning clang issues when the aligned 
attribute is stripped. If it doesn't warn, should it? I recently came across a 
case where a 16-byte vector annotated with a 4-byte alignment was passed to 
std::swap, which caused a crash because the alignment was stripped and the x86 
backend decided to emit an 16-byte aligned load to load an unaligned vector.


Repository:
  rC Clang

https://reviews.llvm.org/D46042



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

[clang-tools-extra] r331475 - Simplify test clang-tidy-__clang_analyzer__macro.cpp

[Zinovy Nis via cfe-commits]

Author: zinovy.nis
Date: Thu May  3 11:31:39 2018
New Revision: 331475

URL: http://llvm.org/viewvc/llvm-project?rev=331475&view=rev
Log:
Simplify test clang-tidy-__clang_analyzer__macro.cpp 


Modified:

clang-tools-extra/trunk/test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp

Modified: 
clang-tools-extra/trunk/test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp?rev=331475&r1=331474&r2=331475&view=diff
==
--- 
clang-tools-extra/trunk/test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp 
(original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp 
Thu May  3 11:31:39 2018
@@ -3,8 +3,3 @@
 #if !defined(__clang_analyzer__)
 #error __clang_analyzer__ is not defined
 #endif
-// RUN: clang-tidy %s -checks=-*,modernize-use-nullptr -- | count 0
-
-#if !defined(__clang_analyzer__)
-#error __clang_analyzer__ is not defined
-#endif


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

[PATCH] D46325: [clang-tidy] Define __clang_analyzer__ macro for clang-tidy for compatibility with clang static analyzer. 2nd try.

[Zinovy Nis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL331474: [clang-tidy] Define __clang_analyzer__ macro for 
clang-tidy for compatibility… (authored by zinovy.nis, committed by ).
Herald added subscribers: llvm-commits, klimek.

Changed prior to commit:
  https://reviews.llvm.org/D46325?vs=145059&id=145060#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D46325

Files:
  clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
  clang-tools-extra/trunk/test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp


Index: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
===
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
@@ -524,6 +524,18 @@
 ActionFactory(ClangTidyContext &Context) : ConsumerFactory(Context) {}
 FrontendAction *create() override { return new Action(&ConsumerFactory); }
 
+bool runInvocation(std::shared_ptr Invocation,
+   FileManager *Files,
+   std::shared_ptr PCHContainerOps,
+   DiagnosticConsumer *DiagConsumer) override {
+  // Explicitly set ProgramAction to RunAnalysis to make the preprocessor
+  // define __clang_analyzer__ macro. The frontend analyzer action will not
+  // be called here.
+  Invocation->getFrontendOpts().ProgramAction = frontend::RunAnalysis;
+  return FrontendActionFactory::runInvocation(
+  Invocation, Files, PCHContainerOps, DiagConsumer);
+}
+
   private:
 class Action : public ASTFrontendAction {
 public:
Index: 
clang-tools-extra/trunk/test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp
===
--- 
clang-tools-extra/trunk/test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp
+++ 
clang-tools-extra/trunk/test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp
@@ -0,0 +1,10 @@
+// RUN: clang-tidy %s -checks=-*,modernize-use-nullptr -- | count 0
+
+#if !defined(__clang_analyzer__)
+#error __clang_analyzer__ is not defined
+#endif
+// RUN: clang-tidy %s -checks=-*,modernize-use-nullptr -- | count 0
+
+#if !defined(__clang_analyzer__)
+#error __clang_analyzer__ is not defined
+#endif


Index: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
===
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
@@ -524,6 +524,18 @@
 ActionFactory(ClangTidyContext &Context) : ConsumerFactory(Context) {}
 FrontendAction *create() override { return new Action(&ConsumerFactory); }
 
+bool runInvocation(std::shared_ptr Invocation,
+   FileManager *Files,
+   std::shared_ptr PCHContainerOps,
+   DiagnosticConsumer *DiagConsumer) override {
+  // Explicitly set ProgramAction to RunAnalysis to make the preprocessor
+  // define __clang_analyzer__ macro. The frontend analyzer action will not
+  // be called here.
+  Invocation->getFrontendOpts().ProgramAction = frontend::RunAnalysis;
+  return FrontendActionFactory::runInvocation(
+  Invocation, Files, PCHContainerOps, DiagConsumer);
+}
+
   private:
 class Action : public ASTFrontendAction {
 public:
Index: clang-tools-extra/trunk/test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp
@@ -0,0 +1,10 @@
+// RUN: clang-tidy %s -checks=-*,modernize-use-nullptr -- | count 0
+
+#if !defined(__clang_analyzer__)
+#error __clang_analyzer__ is not defined
+#endif
+// RUN: clang-tidy %s -checks=-*,modernize-use-nullptr -- | count 0
+
+#if !defined(__clang_analyzer__)
+#error __clang_analyzer__ is not defined
+#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r331474 - [clang-tidy] Define __clang_analyzer__ macro for clang-tidy for compatibility with clang static analyzer

[Zinovy Nis via cfe-commits]

Author: zinovy.nis
Date: Thu May  3 11:26:39 2018
New Revision: 331474

URL: http://llvm.org/viewvc/llvm-project?rev=331474&view=rev
Log:
[clang-tidy] Define __clang_analyzer__ macro for clang-tidy for compatibility 
with clang static analyzer

This macro is widely used in many well-known projects, ex. Chromium.
But it's not set for clang-tidy, so for ex. DCHECK in Chromium is not considered
as [[no-return]], and a lot of false-positive warnings about nullptr
dereferenced are emitted.

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


Added:

clang-tools-extra/trunk/test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp

Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp?rev=331474&r1=331473&r2=331474&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp Thu May  3 11:26:39 2018
@@ -524,6 +524,18 @@ void runClangTidy(clang::tidy::ClangTidy
 ActionFactory(ClangTidyContext &Context) : ConsumerFactory(Context) {}
 FrontendAction *create() override { return new Action(&ConsumerFactory); }
 
+bool runInvocation(std::shared_ptr Invocation,
+   FileManager *Files,
+   std::shared_ptr PCHContainerOps,
+   DiagnosticConsumer *DiagConsumer) override {
+  // Explicitly set ProgramAction to RunAnalysis to make the preprocessor
+  // define __clang_analyzer__ macro. The frontend analyzer action will not
+  // be called here.
+  Invocation->getFrontendOpts().ProgramAction = frontend::RunAnalysis;
+  return FrontendActionFactory::runInvocation(
+  Invocation, Files, PCHContainerOps, DiagConsumer);
+}
+
   private:
 class Action : public ASTFrontendAction {
 public:

Added: 
clang-tools-extra/trunk/test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp?rev=331474&view=auto
==
--- 
clang-tools-extra/trunk/test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp 
(added)
+++ 
clang-tools-extra/trunk/test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp 
Thu May  3 11:26:39 2018
@@ -0,0 +1,10 @@
+// RUN: clang-tidy %s -checks=-*,modernize-use-nullptr -- | count 0
+
+#if !defined(__clang_analyzer__)
+#error __clang_analyzer__ is not defined
+#endif
+// RUN: clang-tidy %s -checks=-*,modernize-use-nullptr -- | count 0
+
+#if !defined(__clang_analyzer__)
+#error __clang_analyzer__ is not defined
+#endif


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

[PATCH] D45702: [clang-tidy] Add a new check, readability-redundant-data-call, that finds and removes redundant calls to .data().

[Shuai Wang via Phabricator via cfe-commits]

shuaiwang added a comment.

In https://reviews.llvm.org/D45702#1086250, @aaron.ballman wrote:

> In https://reviews.llvm.org/D45702#1085890, @shuaiwang wrote:
>
> > In https://reviews.llvm.org/D45702#1085224, @aaron.ballman wrote:
> >
> > > > Have you run this over any large code bases to see whether the check 
> > > > triggers in practice?
> > >
> > > I'm still curious about this, btw.
> >
> >
> > Yes it triggers in Google's code base.
>
>
> Were there any false positives that you saw?


From randomly checking several triggerings no I didn't find any false 
positives. I feel the check should be pretty safe in terms of false positives 
because we only trigger on configured types.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45702



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

[PATCH] D46325: [clang-tidy] Define __clang_analyzer__ macro for clang-tidy for compatibility with clang static analyzer. 2nd try.

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis added a comment.

Thanks Alexander for your feedback!


https://reviews.llvm.org/D46325



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

[PATCH] D46325: [clang-tidy] Define __clang_analyzer__ macro for clang-tidy for compatibility with clang static analyzer. 2nd try.

[Alexander Kornienko via Phabricator via cfe-commits]

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

LG. Thank you!


https://reviews.llvm.org/D46325



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

[PATCH] D46325: [clang-tidy] Define __clang_analyzer__ macro for clang-tidy for compatibility with clang static analyzer. 2nd try.

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis updated this revision to Diff 145059.
zinovy.nis added a comment.

- Added comments on why setting `ProgramAction` explicitly.


https://reviews.llvm.org/D46325

Files:
  clang-tidy/ClangTidy.cpp
  test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp


Index: test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp
===
--- test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp
+++ test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp
@@ -0,0 +1,5 @@
+// RUN: clang-tidy %s -checks=-*,modernize-use-nullptr -- | count 0
+
+#if !defined(__clang_analyzer__)
+#error __clang_analyzer__ is not defined
+#endif
Index: clang-tidy/ClangTidy.cpp
===
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -524,6 +524,18 @@
 ActionFactory(ClangTidyContext &Context) : ConsumerFactory(Context) {}
 FrontendAction *create() override { return new Action(&ConsumerFactory); }
 
+bool runInvocation(std::shared_ptr Invocation,
+   FileManager *Files,
+   std::shared_ptr PCHContainerOps,
+   DiagnosticConsumer *DiagConsumer) override {
+  // Explicitly set ProgramAction to RunAnalysis to make the preprocessor
+  // define __clang_analyzer__ macro. The frontend analyzer action will not
+  // be called here.
+  Invocation->getFrontendOpts().ProgramAction = frontend::RunAnalysis;
+  return FrontendActionFactory::runInvocation(
+  Invocation, Files, PCHContainerOps, DiagConsumer);
+}
+
   private:
 class Action : public ASTFrontendAction {
 public:


Index: test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp
===
--- test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp
+++ test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp
@@ -0,0 +1,5 @@
+// RUN: clang-tidy %s -checks=-*,modernize-use-nullptr -- | count 0
+
+#if !defined(__clang_analyzer__)
+#error __clang_analyzer__ is not defined
+#endif
Index: clang-tidy/ClangTidy.cpp
===
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -524,6 +524,18 @@
 ActionFactory(ClangTidyContext &Context) : ConsumerFactory(Context) {}
 FrontendAction *create() override { return new Action(&ConsumerFactory); }
 
+bool runInvocation(std::shared_ptr Invocation,
+   FileManager *Files,
+   std::shared_ptr PCHContainerOps,
+   DiagnosticConsumer *DiagConsumer) override {
+  // Explicitly set ProgramAction to RunAnalysis to make the preprocessor
+  // define __clang_analyzer__ macro. The frontend analyzer action will not
+  // be called here.
+  Invocation->getFrontendOpts().ProgramAction = frontend::RunAnalysis;
+  return FrontendActionFactory::runInvocation(
+  Invocation, Files, PCHContainerOps, DiagConsumer);
+}
+
   private:
 class Action : public ASTFrontendAction {
 public:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45931: [ASTMatchers] Don't garble the profiling output when multiple TU's are processed

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D45931#1086665, @alexfh wrote:

> In https://reviews.llvm.org/D45931#1084503, @lebedev.ri wrote:
>
> > In https://reviews.llvm.org/D45931#1083192, @lebedev.ri wrote:
> >
> > > Thank you for looking at this.
> > >
> > > In https://reviews.llvm.org/D45931#1083184, @alexfh wrote:
> > >
> > > > From a user's perspective I'd probably prefer a different behavior of 
> > > > checks profiling with multiple translation units: per-file table after 
> > > > each file and an aggregate table at the end.
> > >
> > >
> > > Is this a review note, or a general observation?
> >
>
>
> Why not both? ;)


Review latency, finite time amount, you name it :)

 An independent improvement could be to support TSV/CSV output and/or 
 dumping to a file to spare the user from parsing the tables out of the 
 stdout/stderr.
>>> 
>>> Yes, and a script to merge those CSV's, would be nice.
> 
> I'd probably go with a set of features enough for various use cases:
>  0. don't add any profile merging logic to clang-tidy
> 
> 1. dump profile after each TU to the screen in the current tabulated format
> 2. add a flag to specify a file name prefix to dump profile output for each 
> file as CSV
> 3. (optional) add a script to merge profiles from CSV files and dump as CSV 
> or tabulated (without a script this could be done in a spreadsheet)
> 
>   WDYT?




Repository:
  rC Clang

https://reviews.llvm.org/D45931



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

[PATCH] D46398: [ASTImporterTest] Fix potential use-after-free

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin created this revision.
a.sidorin added reviewers: xazax.hun, martong, szepet, jingham.
Herald added subscribers: cfe-commits, rnkovacs.

`buildASTFromCodeWithArgs()` accepts `llvm::Twine` as `Code` argument. However, 
if the argument is not a C string or std::string, the argument is being copied 
into a temporary buffer in order to get a null-terminated string. This lead to 
a potential UAF. Fixing this via calling `.data()` on StringRef since our 
`Code` is always null-terminated.

The issue was introduced by me in https://reviews.llvm.org/D44079 (sorry) but 
was not noticed.


Repository:
  rC Clang

https://reviews.llvm.org/D46398

Files:
  unittests/AST/ASTImporterTest.cpp


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -213,7 +213,7 @@
 TranslationUnitDecl *TUDecl = nullptr;
 TU(StringRef Code, StringRef FileName, ArgVector Args)
 : Code(Code), FileName(FileName),
-  Unit(tooling::buildASTFromCodeWithArgs(this->Code, Args,
+  Unit(tooling::buildASTFromCodeWithArgs(this->Code.data(), Args,
  this->FileName)),
   TUDecl(Unit->getASTContext().getTranslationUnitDecl()) {}
   };


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -213,7 +213,7 @@
 TranslationUnitDecl *TUDecl = nullptr;
 TU(StringRef Code, StringRef FileName, ArgVector Args)
 : Code(Code), FileName(FileName),
-  Unit(tooling::buildASTFromCodeWithArgs(this->Code, Args,
+  Unit(tooling::buildASTFromCodeWithArgs(this->Code.data(), Args,
  this->FileName)),
   TUDecl(Unit->getASTContext().getTranslationUnitDecl()) {}
   };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization

[Serge Pavlov via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGExprConstant.cpp:1403
+  if (auto *IL = dyn_cast_or_null(Init)) {
+if (InitTy->isConstantArrayType()) {
+  for (auto I : IL->inits())

rjmccall wrote:
> Do you actually care if it's an array initialization instead of a struct/enum 
> initialization?
If this code is enabled for for records too, some tests start to fail. For 
instance, the code:
```
union { int i; double f; } u2 = { };
```
produces output:
```
%union.anon = type { double }
@u2 = global %union.anon zeroinitializer, align 4
```
while previously it produced:
```
@u2 = global { i32, [4 x i8] } { i32 0, [4 x i8] undef }, align 4
```
The latter looks more correct.



Comment at: lib/CodeGen/CGExprConstant.cpp:1413
+  } else if (!Init->isEvaluatable(CE.CGM.getContext())) {
+return false;
+  } else if (InitTy->hasPointerRepresentation()) {

rjmccall wrote:
> Aren't the null-pointer and integer-constant-expression checks below already 
> checking this?  Also, `isEvaluatable` actually computes the full value 
> internally (as an `APValue`), so if you're worried about the memory and 
> compile-time effects of producing such a value, you really shouldn't call it.
> 
> You could reasonably move this entire function to be a method on `Expr` that 
> takes an `ASTContext`.
Comment for `EvaluateAsRValue` says that it tries calculate expression 
agressively. Indeed, for the code:
```
  decltype(nullptr) null();
  int *p = null();
```
compiler ignores potential side effect of `null()` and removes the call, 
leaving only zero initialization. `isNullPointerConstant` behaves similarly.



Comment at: lib/CodeGen/CGExprConstant.cpp:1417
+if (Init->EvaluateAsRValue(ResVal, CE.CGM.getContext()))
+  return ResVal.Val.isLValue() && ResVal.Val.isNullPointer();
+  } else {

rjmccall wrote:
> There's a `isNullPointerConstant` method (you should use 
> `NPC_NeverValueDependent`).
It make code more readable. Thank you!


Repository:
  rC Clang

https://reviews.llvm.org/D46241



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

[PATCH] D46394: [clang-cl] Print /showIncludes to stderr, if used in combination with /E, /EP or /P

[Felix Bruns via Phabricator via cfe-commits]

fxb added a comment.

I'll have a look at these issues tomorrow and submit a new patch then.

Thanks for all your help so far!


https://reviews.llvm.org/D46394



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

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D46159#1086627, @alexfh wrote:

> In https://reviews.llvm.org/D46159#1086493, @aaron.ballman wrote:
>
> > I think the premise is a bit off the mark. It's not that these are not for 
> > the common user -- it's that they're simply not ready for users at all. 
> > Making it easier to expose does not seem like it serves users because those 
> > users expect exposed features to work.
>
>
> That was also the sentiment static analyzer folks were voicing at some point. 
> I also sympathize to the idea of testing checks and contributing fixes to 
> them, but what the CSA maintainers seem to dislike is a stream of bugs for 
> alpha checkers from users expecting of a certain level of support. So it's 
> basically their decision whether they want to expose alpha checkers via clang 
> frontend and/or via clang-tidy. I can only say whether I like the specific 
> way it is done in clang-tidy.


If the static analyzer people desire this feature, that would sway my position 
on it, but it sounds like they're hesitant as well. However, I don't think 
clang-tidy is beholden either -- if we don't think this functionality should be 
exposed and can justify that position, that should carry weight as well. From a 
policy perspective, I would be fine with a policy for clang-tidy where we never 
expose an alpha checker from the static analyzer (or only expose checks on a 
case by case basis) because I don't mind users having to jump through hoops to 
get to experimental, unsupported functionality.

As for the way this is surfaced in clang-tidy, I'm also not keen on it but I 
don't have an improved suggestion to make yet. I primarily don't like the fact 
that, as a user, I enable checks by name but for some kinds of checks I have to 
*also* enable them via a secondary mechanism otherwise the name doesn't even 
exist. This strikes me as being a likely source of confusion where forgetting 
one flag causes behavioral differences the user doesn't expect.

>> Making the flag sound scary doesn't suffice -- many users never see the 
>> flags because they're hidden away in a build script, but they definitely see 
>> the diagnostics and file bug reports.
> 
> "We've fixed the glitch" by making everyone wanting a bugzilla account send 
> an email to a human. So only the users who pass this sort of a Turing test 
> will file bugs ;)

Which is an even worse user experience.


https://reviews.llvm.org/D46159



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

[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 145049.
sepavloff marked an inline comment as done.
sepavloff added a comment.

Small simplification


Repository:
  rC Clang

https://reviews.llvm.org/D46241

Files:
  lib/CodeGen/CGExprConstant.cpp
  test/CodeGenCXX/cxx11-initializer-aggregate.cpp


Index: test/CodeGenCXX/cxx11-initializer-aggregate.cpp
===
--- test/CodeGenCXX/cxx11-initializer-aggregate.cpp
+++ test/CodeGenCXX/cxx11-initializer-aggregate.cpp
@@ -51,3 +51,27 @@
   // meaningful.
   B b[30] = {};
 }
+
+namespace ZeroInit {
+  enum { Zero, One };
+  constexpr int zero() { return 0; }
+  constexpr int *null() { return nullptr; }
+  struct Filler {
+int x;
+Filler();
+  };
+
+  // These declarations, if implemented elementwise, require huge
+  // amout of memory and compiler time.
+  unsigned char data_1[1024 * 1024 * 1024 * 2u] = { 0 };
+  unsigned char data_2[1024 * 1024 * 1024 * 2u] = { Zero };
+  unsigned char data_3[1024][1024][1024] = {{{0}}};
+  unsigned char data_4[1024 * 1024 * 1024 * 2u] = { zero() };
+  int *data_5[1024 * 1024 * 512] = { nullptr };
+  int *data_6[1024 * 1024 * 512] = { null() };
+
+
+  // This variable must be initialized elementwise.
+  Filler data_e1[1024] = {};
+  // CHECK: getelementptr inbounds {{.*}} @_ZN8ZeroInit7data_e1E
+}
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -1392,20 +1392,43 @@
   return type;
 }
 
+/// Checks if the specified initializer is equivalent to zero initialization.
+static bool isZeroInitializer(ConstantEmitter &CE, const Expr *Init) {
+  QualType InitTy = Init->getType().getCanonicalType();
+  if (auto *E = dyn_cast_or_null(Init)) {
+CXXConstructorDecl *CD = E->getConstructor();
+return CD->isDefaultConstructor() && CD->isTrivial();
+  }
+  if (auto *IL = dyn_cast_or_null(Init)) {
+if (InitTy->isConstantArrayType()) {
+  for (auto I : IL->inits())
+if (!isZeroInitializer(CE, I))
+  return false;
+  if (const Expr *Filler = IL->getArrayFiller()) {
+return isZeroInitializer(CE, Filler);
+  }
+  return true;
+}
+  } else if (!Init->isEvaluatable(CE.CGM.getContext())) {
+return false;
+  } else if (Init->isNullPointerConstant(CE.CGM.getContext(),
+ Expr::NPC_NeverValueDependent)) {
+return true;
+  } else {
+llvm::APSInt Value;
+if (Init->isIntegerConstantExpr(Value, CE.CGM.getContext()))
+  return Value.isNullValue();
+  }
+
+  return false;
+}
+
 llvm::Constant *ConstantEmitter::tryEmitPrivateForVarInit(const VarDecl &D) {
   // Make a quick check if variable can be default NULL initialized
   // and avoid going through rest of code which may do, for c++11,
   // initialization of memory to all NULLs.
-  if (!D.hasLocalStorage()) {
-QualType Ty = CGM.getContext().getBaseElementType(D.getType());
-if (Ty->isRecordType())
-  if (const CXXConstructExpr *E =
-  dyn_cast_or_null(D.getInit())) {
-const CXXConstructorDecl *CD = E->getConstructor();
-if (CD->isTrivial() && CD->isDefaultConstructor())
-  return CGM.EmitNullConstant(D.getType());
-  }
-  }
+  if (!D.hasLocalStorage() && isZeroInitializer(*this, D.getInit()))
+return CGM.EmitNullConstant(D.getType());
 
   QualType destType = D.getType();
 


Index: test/CodeGenCXX/cxx11-initializer-aggregate.cpp
===
--- test/CodeGenCXX/cxx11-initializer-aggregate.cpp
+++ test/CodeGenCXX/cxx11-initializer-aggregate.cpp
@@ -51,3 +51,27 @@
   // meaningful.
   B b[30] = {};
 }
+
+namespace ZeroInit {
+  enum { Zero, One };
+  constexpr int zero() { return 0; }
+  constexpr int *null() { return nullptr; }
+  struct Filler {
+int x;
+Filler();
+  };
+
+  // These declarations, if implemented elementwise, require huge
+  // amout of memory and compiler time.
+  unsigned char data_1[1024 * 1024 * 1024 * 2u] = { 0 };
+  unsigned char data_2[1024 * 1024 * 1024 * 2u] = { Zero };
+  unsigned char data_3[1024][1024][1024] = {{{0}}};
+  unsigned char data_4[1024 * 1024 * 1024 * 2u] = { zero() };
+  int *data_5[1024 * 1024 * 512] = { nullptr };
+  int *data_6[1024 * 1024 * 512] = { null() };
+
+
+  // This variable must be initialized elementwise.
+  Filler data_e1[1024] = {};
+  // CHECK: getelementptr inbounds {{.*}} @_ZN8ZeroInit7data_e1E
+}
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -1392,20 +1392,43 @@
   return type;
 }
 
+/// Checks if the specified initializer is equivalent to zero initialization.
+static bool isZeroInitializer(ConstantEmitter &CE, const Expr *Init) {
+  QualType InitTy = Init->getType().getCanonicalType();
+  if (auto *E = 

[PATCH] D46370: [OPENMP] Fix test typos: CHECK-DAG-N -> CHECK-N-DAG

[Joel E. Denny via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC331469: [OPENMP] Fix test typos: CHECK-DAG-N -> 
CHECK-N-DAG (authored by jdenny, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D46370?vs=144949&id=145047#toc

Repository:
  rC Clang

https://reviews.llvm.org/D46370

Files:
  test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp
  test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_firstprivate_codegen.cpp
  test/OpenMP/target_teams_distribute_simd_firstprivate_codegen.cpp
  test/OpenMP/teams_distribute_firstprivate_codegen.cpp
  test/OpenMP/teams_distribute_parallel_for_firstprivate_codegen.cpp
  test/OpenMP/teams_distribute_parallel_for_simd_firstprivate_codegen.cpp
  test/OpenMP/teams_distribute_simd_firstprivate_codegen.cpp

Index: test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp
===
--- test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp
+++ test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp
@@ -266,8 +266,8 @@
 // CHECK: store i{{[0-9]+}} {{.+}}, i{{[0-9]+}}* [[SIVAR_ADDR]],
 
 // T_VAR and SIVAR
-// CHECK-DAG-64: [[CONV_TVAR:%.+]] = bitcast i64* [[T_VAR_ADDR]] to i32*
-// CHECK-DAG-64: [[CONV_SIVAR:%.+]] = bitcast i64* [[SIVAR_ADDR]] to i32*
+// CHECK-64-DAG: [[CONV_TVAR:%.+]] = bitcast i64* [[T_VAR_ADDR]] to i32*
+// CHECK-64-DAG: [[CONV_SIVAR:%.+]] = bitcast i64* [[SIVAR_ADDR]] to i32*
 
 // preparation vars
 // CHECK-DAG: [[VEC_ADDR_VAL:%.+]] = load [2 x i{{[0-9]+}}]*, [2 x i{{[0-9]+}}]** [[VEC_ADDR]],
@@ -332,8 +332,8 @@
 // CHECK: store i{{[0-9]+}} {{.+}}, i{{[0-9]+}}* [[SIVAR_ADDR]],
 
 // T_VAR and SIVAR
-// CHECK-DAG-64: [[CONV_TVAR:%.+]] = bitcast i64* [[T_VAR_ADDR]] to i32*
-// CHECK-DAG-64: [[CONV_SIVAR:%.+]] = bitcast i64* [[SIVAR_ADDR]] to i32*
+// CHECK-64-DAG: [[CONV_TVAR:%.+]] = bitcast i64* [[T_VAR_ADDR]] to i32*
+// CHECK-64-DAG: [[CONV_SIVAR:%.+]] = bitcast i64* [[SIVAR_ADDR]] to i32*
 
 // preparation vars
 // CHECK-DAG: [[VEC_ADDR_VAL:%.+]] = load [2 x i{{[0-9]+}}]*, [2 x i{{[0-9]+}}]** [[VEC_ADDR]],
@@ -363,13 +363,13 @@
 // CHECK-DAG: call void @{{.+}}({{.+}} [[AGG_TMP2]])
 
 // CHECK: call void @__kmpc_for_static_init_4(
-// CHECK-DAG-32: {{.+}} = {{.+}} [[T_VAR_ADDR]]
-// CHECK-DAG-64: {{.+}} = {{.+}} [[CONV_TVAR]]
+// CHECK-32-DAG: {{.+}} = {{.+}} [[T_VAR_ADDR]]
+// CHECK-64-DAG: {{.+}} = {{.+}} [[CONV_TVAR]]
 // CHECK-DAG: {{.+}} = {{.+}} [[VEC_PRIV]]
 // CHECK-DAG: {{.+}} = {{.+}} [[S_ARR_PRIV]]
 // CHECK-DAG: {{.+}} = {{.+}} [[VAR_PRIV]]
-// CHECK-DAG-32: {{.+}} = {{.+}} [[SIVAR_ADDR]]
-// CHECK-DAG-64: {{.+}} = {{.+}} [[CONV_SIVAR]]
+// CHECK-32-DAG: {{.+}} = {{.+}} [[SIVAR_ADDR]]
+// CHECK-64-DAG: {{.+}} = {{.+}} [[CONV_SIVAR]]
 // CHECK: call void @__kmpc_for_static_fini(
 // CHECK: ret void
 
@@ -512,8 +512,8 @@
 // CHECK-DAG: store [[S_INT_TY]]* [[VAR_PRIV]], [[S_INT_TY]]** [[TMP]],
 
 // CHECK: call void @__kmpc_for_static_init_4(
-// CHECK-DAG-32: {{.+}} = {{.+}} [[T_VAR_ADDR]]
-// CHECK-DAG-64: {{.+}} = {{.+}} [[CONV_TVAR]]
+// CHECK-32-DAG: {{.+}} = {{.+}} [[T_VAR_ADDR]]
+// CHECK-64-DAG: {{.+}} = {{.+}} [[CONV_TVAR]]
 // CHECK-DAG: {{.+}} = {{.+}} [[VEC_PRIV]]
 // CHECK-DAG: {{.+}} = {{.+}} [[TMP]]
 // CHECK-DAG: {{.+}} = {{.+}} [[S_ARR_PRIV]]
Index: test/OpenMP/teams_distribute_firstprivate_codegen.cpp
===
--- test/OpenMP/teams_distribute_firstprivate_codegen.cpp
+++ test/OpenMP/teams_distribute_firstprivate_codegen.cpp
@@ -219,8 +219,8 @@
 // CHECK: store i{{[0-9]+}} {{.+}}, i{{[0-9]+}}* [[SIVAR_ADDR]],
 
 // T_VAR and SIVAR
-// CHECK-DAG-64: [[CONV_TVAR:%.+]] = bitcast i64* [[T_VAR_ADDR]] to i32*
-// CHECK-DAG-64: [[CONV_SIVAR:%.+]] = bitcast i64* [[SIVAR_ADDR]] to i32*
+// CHECK-64-DAG: [[CONV_TVAR:%.+]] = bitcast i64* [[T_VAR_ADDR]] to i32*
+// CHECK-64-DAG: [[CONV_SIVAR:%.+]] = bitcast i64* [[SIVAR_ADDR]] to i32*
 
 // preparation vars
 // CHECK-DAG: [[VEC_ADDR_VAL:%.+]] = load [2 x i{{[0-9]+}}]*, [2 x i{{[0-9]+}}]** [[VEC_ADDR]],
@@ -250,13 +250,13 @@
 // CHECK-DAG: call void @{{.+}}({{.+}} [[AGG_TMP2]])
 
 // CHECK: call void @__kmpc_for_static_init_4(
-// CHECK-DAG-32: {{.+}} = {{.+}} [[T_VAR_ADDR]]
-// CHECK-DAG-64: {{.+}} = {{.+}} [[CONV_TVAR]]
+// CHECK-32-DAG: {{.+}} = {{.+}} [[T_VAR_ADDR]]
+// CHECK-64-DAG: {{.+}} = {{.+}} [[CONV_TVAR]]
 // CHECK-DAG: {{.+}} = {{.+}} [[VEC_PRIV]]
 // CHECK-DAG: {{.+}} = {{.+}} [[S_ARR_PRIV]]
 // CHECK-DAG: {{.+}} = {{.+}} [[VAR_PRIV]]
-// CHECK-DAG-32: {{.+}} = {{.+}} [[SIVAR_ADDR]]
-// CHECK-DAG-64: {{.+}} = {{.+}} [[CONV_SIVAR]]
+// CHECK-32-DAG: {{.+}} = {{.+}} [[SIVAR_ADDR]]
+// CHECK-64-DAG: {{.+}} = {{.+}} [[CONV_SIVAR]]
 // CHECK: call void @__kmpc_for_static_fini(
 // CHECK: ret void
 
@@ -338,8 +338,8 @@
 // CHECK-DAG: store [[S_INT_TY]]* [[VAR_PRIV]], [[S_INT_TY]

r331468 - Revert r331466: [OPENMP] Fix test typos: CHECK-DAG-N -> CHECK-N-DAG"

[Joel E. Denny via cfe-commits]

Author: jdenny
Date: Thu May  3 10:22:01 2018
New Revision: 331468

URL: http://llvm.org/viewvc/llvm-project?rev=331468&view=rev
Log:
Revert r331466: [OPENMP] Fix test typos: CHECK-DAG-N -> CHECK-N-DAG"

Sorry, forgot to add commit log attributes.

Modified:
cfe/trunk/test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_simd_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_parallel_for_firstprivate_codegen.cpp

cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_simd_firstprivate_codegen.cpp

Modified: cfe/trunk/test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp?rev=331468&r1=331467&r2=331468&view=diff
==
--- cfe/trunk/test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp 
(original)
+++ cfe/trunk/test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp Thu 
May  3 10:22:01 2018
@@ -216,8 +216,8 @@ int main() {
 // CHECK: store i{{[0-9]+}} {{.+}}, i{{[0-9]+}}* [[SIVAR_ADDR]],
 
 // T_VAR and SIVAR
-// CHECK-64-DAG: [[CONV_TVAR:%.+]] = bitcast i64* [[T_VAR_ADDR]] to i32*
-// CHECK-64-DAG: [[CONV_SIVAR:%.+]] = bitcast i64* [[SIVAR_ADDR]] to i32*
+// CHECK-DAG-64: [[CONV_TVAR:%.+]] = bitcast i64* [[T_VAR_ADDR]] to i32*
+// CHECK-DAG-64: [[CONV_SIVAR:%.+]] = bitcast i64* [[SIVAR_ADDR]] to i32*
 
 // preparation vars
 // CHECK-DAG: [[VEC_ADDR_VAL:%.+]] = load [2 x i{{[0-9]+}}]*, [2 x 
i{{[0-9]+}}]** [[VEC_ADDR]],
@@ -247,13 +247,13 @@ int main() {
 // CHECK-DAG: call void @{{.+}}({{.+}} [[AGG_TMP2]])
 
 // CHECK: call void @__kmpc_for_static_init_4(
-// CHECK-32-DAG: {{.+}} = {{.+}} [[T_VAR_ADDR]]
-// CHECK-64-DAG: {{.+}} = {{.+}} [[CONV_TVAR]]
+// CHECK-DAG-32: {{.+}} = {{.+}} [[T_VAR_ADDR]]
+// CHECK-DAG-64: {{.+}} = {{.+}} [[CONV_TVAR]]
 // CHECK-DAG: {{.+}} = {{.+}} [[VEC_PRIV]]
 // CHECK-DAG: {{.+}} = {{.+}} [[S_ARR_PRIV]]
 // CHECK-DAG: {{.+}} = {{.+}} [[VAR_PRIV]]
-// CHECK-32-DAG: {{.+}} = {{.+}} [[SIVAR_ADDR]]
-// CHECK-64-DAG: {{.+}} = {{.+}} [[CONV_SIVAR]]
+// CHECK-DAG-32: {{.+}} = {{.+}} [[SIVAR_ADDR]]
+// CHECK-DAG-64: {{.+}} = {{.+}} [[CONV_SIVAR]]
 // CHECK: call void @__kmpc_for_static_fini(
 // CHECK: ret void
 
@@ -335,8 +335,8 @@ int main() {
 // CHECK-DAG: store [[S_INT_TY]]* [[VAR_PRIV]], [[S_INT_TY]]** [[TMP]],
 
 // CHECK: call void @__kmpc_for_static_init_4(
-// CHECK-32-DAG: {{.+}} = {{.+}} [[T_VAR_ADDR]]
-// CHECK-64-DAG: {{.+}} = {{.+}} [[CONV_TVAR]]
+// CHECK-DAG-32: {{.+}} = {{.+}} [[T_VAR_ADDR]]
+// CHECK-DAG-64: {{.+}} = {{.+}} [[CONV_TVAR]]
 // CHECK-DAG: {{.+}} = {{.+}} [[VEC_PRIV]]
 // CHECK-DAG: {{.+}} = {{.+}} [[TMP]]
 // CHECK-DAG: {{.+}} = {{.+}} [[S_ARR_PRIV]]

Modified: 
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp?rev=331468&r1=331467&r2=331468&view=diff
==
--- 
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp
 (original)
+++ 
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp
 Thu May  3 10:22:01 2018
@@ -266,8 +266,8 @@ int main() {
 // CHECK: store i{{[0-9]+}} {{.+}}, i{{[0-9]+}}* [[SIVAR_ADDR]],
 
 // T_VAR and SIVAR
-// CHECK-64-DAG: [[CONV_TVAR:%.+]] = bitcast i64* [[T_VAR_ADDR]] to i32*
-// CHECK-64-DAG: [[CONV_SIVAR:%.+]] = bitcast i64* [[SIVAR_ADDR]] to i32*
+// CHECK-DAG-64: [[CONV_TVAR:%.+]] = bitcast i64* [[T_VAR_ADDR]] to i32*
+// CHECK-DAG-64: [[CONV_SIVAR:%.+]] = bitcast i64* [[SIVAR_ADDR]] to i32*
 
 // preparation vars
 // CHECK-DAG: [[VEC_ADDR_VAL:%.+]] = load [2 x i{{[0-9]+}}]*, [2 x 
i{{[0-9]+}}]** [[VEC_ADDR]],
@@ -332,8 +332,8 @@ int main() {
 // CHECK: store i{{[0-9]+}} {{.+}}, i{{[0-9]+}}* [[SIVAR_ADDR]],
 
 // T_VAR and SIVAR
-// CHECK-64-DAG: [[CONV_TVAR:%.+]] = bitcast i64* [[T_VAR_ADDR]] to i32*
-// CHECK-64-DAG: [[CONV_SIVAR:%.+]] = bitcast i64* [[SIVAR_ADDR]] to i32*
+// CHECK-DAG-64: [[CONV_TVAR:%.+]] = bitcast i64* [[T_VAR_ADDR]] to i32*
+// CHECK-DAG-64: [[CONV_SIVAR:%.+]] = bitcast i64* [[SIVAR_ADDR]] to i32*
 
 // preparation vars
 // CHECK-DAG: [[VEC_ADDR_VAL:%.+]] = load [2 x i{{[0-9]+}}]*, [2 x 
i{{[0-9]+}}]** [[VEC_ADDR]],
@@ -363,13 +363,13 @@ int main() {
 // CHECK-DAG: call void @{{.+}}({{.+}} [[AGG_TMP2]])
 
 // CHECK: call void @__kmpc_for_static_init_4(
-// CHECK-32-DAG: {{.+}} = {{.+}} [[T_VAR_ADDR]]
-// CHECK-64-D

r331469 - [OPENMP] Fix test typos: CHECK-DAG-N -> CHECK-N-DAG

[Joel E. Denny via cfe-commits]

Author: jdenny
Date: Thu May  3 10:22:04 2018
New Revision: 331469

URL: http://llvm.org/viewvc/llvm-project?rev=331469&view=rev
Log:
[OPENMP] Fix test typos: CHECK-DAG-N -> CHECK-N-DAG

Reviewed by: ABataev

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

Modified:
cfe/trunk/test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_simd_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_parallel_for_firstprivate_codegen.cpp

cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_simd_firstprivate_codegen.cpp

Modified: cfe/trunk/test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp?rev=331469&r1=331468&r2=331469&view=diff
==
--- cfe/trunk/test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp 
(original)
+++ cfe/trunk/test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp Thu 
May  3 10:22:04 2018
@@ -216,8 +216,8 @@ int main() {
 // CHECK: store i{{[0-9]+}} {{.+}}, i{{[0-9]+}}* [[SIVAR_ADDR]],
 
 // T_VAR and SIVAR
-// CHECK-DAG-64: [[CONV_TVAR:%.+]] = bitcast i64* [[T_VAR_ADDR]] to i32*
-// CHECK-DAG-64: [[CONV_SIVAR:%.+]] = bitcast i64* [[SIVAR_ADDR]] to i32*
+// CHECK-64-DAG: [[CONV_TVAR:%.+]] = bitcast i64* [[T_VAR_ADDR]] to i32*
+// CHECK-64-DAG: [[CONV_SIVAR:%.+]] = bitcast i64* [[SIVAR_ADDR]] to i32*
 
 // preparation vars
 // CHECK-DAG: [[VEC_ADDR_VAL:%.+]] = load [2 x i{{[0-9]+}}]*, [2 x 
i{{[0-9]+}}]** [[VEC_ADDR]],
@@ -247,13 +247,13 @@ int main() {
 // CHECK-DAG: call void @{{.+}}({{.+}} [[AGG_TMP2]])
 
 // CHECK: call void @__kmpc_for_static_init_4(
-// CHECK-DAG-32: {{.+}} = {{.+}} [[T_VAR_ADDR]]
-// CHECK-DAG-64: {{.+}} = {{.+}} [[CONV_TVAR]]
+// CHECK-32-DAG: {{.+}} = {{.+}} [[T_VAR_ADDR]]
+// CHECK-64-DAG: {{.+}} = {{.+}} [[CONV_TVAR]]
 // CHECK-DAG: {{.+}} = {{.+}} [[VEC_PRIV]]
 // CHECK-DAG: {{.+}} = {{.+}} [[S_ARR_PRIV]]
 // CHECK-DAG: {{.+}} = {{.+}} [[VAR_PRIV]]
-// CHECK-DAG-32: {{.+}} = {{.+}} [[SIVAR_ADDR]]
-// CHECK-DAG-64: {{.+}} = {{.+}} [[CONV_SIVAR]]
+// CHECK-32-DAG: {{.+}} = {{.+}} [[SIVAR_ADDR]]
+// CHECK-64-DAG: {{.+}} = {{.+}} [[CONV_SIVAR]]
 // CHECK: call void @__kmpc_for_static_fini(
 // CHECK: ret void
 
@@ -335,8 +335,8 @@ int main() {
 // CHECK-DAG: store [[S_INT_TY]]* [[VAR_PRIV]], [[S_INT_TY]]** [[TMP]],
 
 // CHECK: call void @__kmpc_for_static_init_4(
-// CHECK-DAG-32: {{.+}} = {{.+}} [[T_VAR_ADDR]]
-// CHECK-DAG-64: {{.+}} = {{.+}} [[CONV_TVAR]]
+// CHECK-32-DAG: {{.+}} = {{.+}} [[T_VAR_ADDR]]
+// CHECK-64-DAG: {{.+}} = {{.+}} [[CONV_TVAR]]
 // CHECK-DAG: {{.+}} = {{.+}} [[VEC_PRIV]]
 // CHECK-DAG: {{.+}} = {{.+}} [[TMP]]
 // CHECK-DAG: {{.+}} = {{.+}} [[S_ARR_PRIV]]

Modified: 
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp?rev=331469&r1=331468&r2=331469&view=diff
==
--- 
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp
 (original)
+++ 
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp
 Thu May  3 10:22:04 2018
@@ -266,8 +266,8 @@ int main() {
 // CHECK: store i{{[0-9]+}} {{.+}}, i{{[0-9]+}}* [[SIVAR_ADDR]],
 
 // T_VAR and SIVAR
-// CHECK-DAG-64: [[CONV_TVAR:%.+]] = bitcast i64* [[T_VAR_ADDR]] to i32*
-// CHECK-DAG-64: [[CONV_SIVAR:%.+]] = bitcast i64* [[SIVAR_ADDR]] to i32*
+// CHECK-64-DAG: [[CONV_TVAR:%.+]] = bitcast i64* [[T_VAR_ADDR]] to i32*
+// CHECK-64-DAG: [[CONV_SIVAR:%.+]] = bitcast i64* [[SIVAR_ADDR]] to i32*
 
 // preparation vars
 // CHECK-DAG: [[VEC_ADDR_VAL:%.+]] = load [2 x i{{[0-9]+}}]*, [2 x 
i{{[0-9]+}}]** [[VEC_ADDR]],
@@ -332,8 +332,8 @@ int main() {
 // CHECK: store i{{[0-9]+}} {{.+}}, i{{[0-9]+}}* [[SIVAR_ADDR]],
 
 // T_VAR and SIVAR
-// CHECK-DAG-64: [[CONV_TVAR:%.+]] = bitcast i64* [[T_VAR_ADDR]] to i32*
-// CHECK-DAG-64: [[CONV_SIVAR:%.+]] = bitcast i64* [[SIVAR_ADDR]] to i32*
+// CHECK-64-DAG: [[CONV_TVAR:%.+]] = bitcast i64* [[T_VAR_ADDR]] to i32*
+// CHECK-64-DAG: [[CONV_SIVAR:%.+]] = bitcast i64* [[SIVAR_ADDR]] to i32*
 
 // preparation vars
 // CHECK-DAG: [[VEC_ADDR_VAL:%.+]] = load [2 x i{{[0-9]+}}]*, [2 x 
i{{[0-9]+}}]** [[VEC_ADDR]],
@@ -363,13 +363,13 @@ int main() {
 // CHECK-DAG: call void @{{.+}}({{.+}} [[AGG_TMP2]])
 
 // CHECK: call void @__kmpc_for_static_init_4(
-// CHECK-DAG-32: {{.+}} = {{.+}} [[T_VAR_ADDR]

[PATCH] D46394: [clang-cl] Print /showIncludes to stderr, if used in combination with /E, /EP or /P

[Erich Keane via Phabricator via cfe-commits]

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

When building this, it didn't build since you'd reused the name.  I changed the 
variable to be named "Dest" instead of "Destination"

That said, the test suite now fails on Clang :: 
Frontend/print-header-includes.c'.

Please do a build with 'check-clang' to see why this no longer works.
Thanks!
-Erich


https://reviews.llvm.org/D46394



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

r331466 - [OPENMP] Fix test typos: CHECK-DAG-N -> CHECK-N-DAG

[Joel E. Denny via cfe-commits]

Author: jdenny
Date: Thu May  3 10:15:44 2018
New Revision: 331466

URL: http://llvm.org/viewvc/llvm-project?rev=331466&view=rev
Log:
[OPENMP] Fix test typos: CHECK-DAG-N -> CHECK-N-DAG

Modified:
cfe/trunk/test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_simd_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_parallel_for_firstprivate_codegen.cpp

cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_simd_firstprivate_codegen.cpp

Modified: cfe/trunk/test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp?rev=331466&r1=331465&r2=331466&view=diff
==
--- cfe/trunk/test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp 
(original)
+++ cfe/trunk/test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp Thu 
May  3 10:15:44 2018
@@ -216,8 +216,8 @@ int main() {
 // CHECK: store i{{[0-9]+}} {{.+}}, i{{[0-9]+}}* [[SIVAR_ADDR]],
 
 // T_VAR and SIVAR
-// CHECK-DAG-64: [[CONV_TVAR:%.+]] = bitcast i64* [[T_VAR_ADDR]] to i32*
-// CHECK-DAG-64: [[CONV_SIVAR:%.+]] = bitcast i64* [[SIVAR_ADDR]] to i32*
+// CHECK-64-DAG: [[CONV_TVAR:%.+]] = bitcast i64* [[T_VAR_ADDR]] to i32*
+// CHECK-64-DAG: [[CONV_SIVAR:%.+]] = bitcast i64* [[SIVAR_ADDR]] to i32*
 
 // preparation vars
 // CHECK-DAG: [[VEC_ADDR_VAL:%.+]] = load [2 x i{{[0-9]+}}]*, [2 x 
i{{[0-9]+}}]** [[VEC_ADDR]],
@@ -247,13 +247,13 @@ int main() {
 // CHECK-DAG: call void @{{.+}}({{.+}} [[AGG_TMP2]])
 
 // CHECK: call void @__kmpc_for_static_init_4(
-// CHECK-DAG-32: {{.+}} = {{.+}} [[T_VAR_ADDR]]
-// CHECK-DAG-64: {{.+}} = {{.+}} [[CONV_TVAR]]
+// CHECK-32-DAG: {{.+}} = {{.+}} [[T_VAR_ADDR]]
+// CHECK-64-DAG: {{.+}} = {{.+}} [[CONV_TVAR]]
 // CHECK-DAG: {{.+}} = {{.+}} [[VEC_PRIV]]
 // CHECK-DAG: {{.+}} = {{.+}} [[S_ARR_PRIV]]
 // CHECK-DAG: {{.+}} = {{.+}} [[VAR_PRIV]]
-// CHECK-DAG-32: {{.+}} = {{.+}} [[SIVAR_ADDR]]
-// CHECK-DAG-64: {{.+}} = {{.+}} [[CONV_SIVAR]]
+// CHECK-32-DAG: {{.+}} = {{.+}} [[SIVAR_ADDR]]
+// CHECK-64-DAG: {{.+}} = {{.+}} [[CONV_SIVAR]]
 // CHECK: call void @__kmpc_for_static_fini(
 // CHECK: ret void
 
@@ -335,8 +335,8 @@ int main() {
 // CHECK-DAG: store [[S_INT_TY]]* [[VAR_PRIV]], [[S_INT_TY]]** [[TMP]],
 
 // CHECK: call void @__kmpc_for_static_init_4(
-// CHECK-DAG-32: {{.+}} = {{.+}} [[T_VAR_ADDR]]
-// CHECK-DAG-64: {{.+}} = {{.+}} [[CONV_TVAR]]
+// CHECK-32-DAG: {{.+}} = {{.+}} [[T_VAR_ADDR]]
+// CHECK-64-DAG: {{.+}} = {{.+}} [[CONV_TVAR]]
 // CHECK-DAG: {{.+}} = {{.+}} [[VEC_PRIV]]
 // CHECK-DAG: {{.+}} = {{.+}} [[TMP]]
 // CHECK-DAG: {{.+}} = {{.+}} [[S_ARR_PRIV]]

Modified: 
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp?rev=331466&r1=331465&r2=331466&view=diff
==
--- 
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp
 (original)
+++ 
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp
 Thu May  3 10:15:44 2018
@@ -266,8 +266,8 @@ int main() {
 // CHECK: store i{{[0-9]+}} {{.+}}, i{{[0-9]+}}* [[SIVAR_ADDR]],
 
 // T_VAR and SIVAR
-// CHECK-DAG-64: [[CONV_TVAR:%.+]] = bitcast i64* [[T_VAR_ADDR]] to i32*
-// CHECK-DAG-64: [[CONV_SIVAR:%.+]] = bitcast i64* [[SIVAR_ADDR]] to i32*
+// CHECK-64-DAG: [[CONV_TVAR:%.+]] = bitcast i64* [[T_VAR_ADDR]] to i32*
+// CHECK-64-DAG: [[CONV_SIVAR:%.+]] = bitcast i64* [[SIVAR_ADDR]] to i32*
 
 // preparation vars
 // CHECK-DAG: [[VEC_ADDR_VAL:%.+]] = load [2 x i{{[0-9]+}}]*, [2 x 
i{{[0-9]+}}]** [[VEC_ADDR]],
@@ -332,8 +332,8 @@ int main() {
 // CHECK: store i{{[0-9]+}} {{.+}}, i{{[0-9]+}}* [[SIVAR_ADDR]],
 
 // T_VAR and SIVAR
-// CHECK-DAG-64: [[CONV_TVAR:%.+]] = bitcast i64* [[T_VAR_ADDR]] to i32*
-// CHECK-DAG-64: [[CONV_SIVAR:%.+]] = bitcast i64* [[SIVAR_ADDR]] to i32*
+// CHECK-64-DAG: [[CONV_TVAR:%.+]] = bitcast i64* [[T_VAR_ADDR]] to i32*
+// CHECK-64-DAG: [[CONV_SIVAR:%.+]] = bitcast i64* [[SIVAR_ADDR]] to i32*
 
 // preparation vars
 // CHECK-DAG: [[VEC_ADDR_VAL:%.+]] = load [2 x i{{[0-9]+}}]*, [2 x 
i{{[0-9]+}}]** [[VEC_ADDR]],
@@ -363,13 +363,13 @@ int main() {
 // CHECK-DAG: call void @{{.+}}({{.+}} [[AGG_TMP2]])
 
 // CHECK: call void @__kmpc_for_static_init_4(
-// CHECK-DAG-32: {{.+}} = {{.+}} [[T_VAR_ADDR]]
-// CHECK-DAG-64: {{.+}} = {{.+}} [[CONV_TVAR]]
+// CHECK-32-DAG: {{.+}} = {

[PATCH] D46013: [ARM] Conform to AAPCS when passing overaligned composites as arguments

[Momchil Velikov via Phabricator via cfe-commits]

chill added a comment.

In https://reviews.llvm.org/D46013#1084440, @efriedma wrote:

> I'd like to see some tests for __attribute((packed)).


Thanks, indeed it does not work correctly on packed structures. Back to the 
drawing board ...


Repository:
  rC Clang

https://reviews.llvm.org/D46013



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

[PATCH] D45931: [ASTMatchers] Don't garble the profiling output when multiple TU's are processed

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In https://reviews.llvm.org/D45931#1084503, @lebedev.ri wrote:

> In https://reviews.llvm.org/D45931#1083192, @lebedev.ri wrote:
>
> > Thank you for looking at this.
> >
> > In https://reviews.llvm.org/D45931#1083184, @alexfh wrote:
> >
> > > From a user's perspective I'd probably prefer a different behavior of 
> > > checks profiling with multiple translation units: per-file table after 
> > > each file and an aggregate table at the end.
> >
> >
> > Is this a review note, or a general observation?
>


Why not both? ;)

>>> An independent improvement could be to support TSV/CSV output and/or 
>>> dumping to a file to spare the user from parsing the tables out of the 
>>> stdout/stderr.
>> 
>> Yes, and a script to merge those CSV's, would be nice.

I'd probably go with a set of features enough for various use cases:
0. don't add any profile merging logic to clang-tidy

1. dump profile after each TU to the screen in the current tabulated format
2. add a flag to specify a file name prefix to dump profile output for each 
file as CSV
3. (optional) add a script to merge profiles from CSV files and dump as CSV or 
tabulated (without a script this could be done in a spreadsheet)

WDYT?


Repository:
  rC Clang

https://reviews.llvm.org/D45931



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

[PATCH] D46187: [Analyzer] getRegisteredCheckers(): handle debug checkers too.

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

What's the use case for debug CSA checkers in clang-tidy?


Repository:
  rC Clang

https://reviews.llvm.org/D46187



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

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

[Alexander Kornienko via Phabricator via cfe-commits]

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

In https://reviews.llvm.org/D46159#1086493, @aaron.ballman wrote:

> I think the premise is a bit off the mark. It's not that these are not for 
> the common user -- it's that they're simply not ready for users at all. 
> Making it easier to expose does not seem like it serves users because those 
> users expect exposed features to work.


That was also the sentiment static analyzer folks were voicing at some point. I 
also sympathize to the idea of testing checks and contributing fixes to them, 
but what the CSA maintainers seem to dislike is a stream of bugs for alpha 
checkers from users expecting of a certain level of support. So it's basically 
their decision whether they want to expose alpha checkers via clang frontend 
and/or via clang-tidy. I can only say whether I like the specific way it is 
done in clang-tidy.

> Making the flag sound scary doesn't suffice -- many users never see the flags 
> because they're hidden away in a build script, but they definitely see the 
> diagnostics and file bug reports.

"We've fixed the glitch" by making everyone wanting a bugzilla account send an 
email to a human. So only the users who pass this sort of a Turing test will 
file bugs ;)




Comment at: clang-tidy/ClangTidyOptions.h:80-82
+  /// \brief Turns on experimental alpha checkers from the static analyzer.
+  llvm::Optional AllowEnablingAlphaChecks;
+

Since this will only be configurable via a flag, this option will be global 
(i.e. not depend on the location of the file being analyzed). I'd suggest to 
remove this option altogether and use something else to pass it to 
ClangTidyASTConsumerFactory. It could be stashed into 
ClangTidyASTConsumerFactory and passed as a parameter of runClangTidy,  or it 
could live in ClangTidyContext.



Comment at: clang-tidy/tool/ClangTidyMain.cpp:198
+/// because it is highly not recommended for users.
+static cl::opt AllowEnablingAlphaChecks("allow-enabling-alpha-checks",
+  cl::init(false), cl::Hidden,

s/checks/checkers/ (in the static analyzer's terminology)

I would also make it more explicit that these are static analyzer checkers. 
-allow-enabling-clang-static-analyzer-alpha-checkers or something like that.

The variable name could be AllowEnablingAnalyzerAlphaCheckers, for example.



Comment at: test/clang-tidy/enable-alpha-checks.cpp:2
+// Check if '-allow-enabling-alpha-checks' is visible for users
+// RUN: clang-tidy -help | not grep 'enable-alpha-checks'
+

grep 'allow-enabling-alpha-checks'


https://reviews.llvm.org/D46159



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

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added inline comments.



Comment at: clang-tidy/tool/ClangTidyMain.cpp:195
 
+/// This option Enables alpha checkers from the static analyzer, that are
+/// experimental. This option is set to false and not visible in help, because

lebedev.ri wrote:
> xbolva00 wrote:
> > This option enables...
> > This option enables...
> The current version is correct.
There was "This option Enables.." when I commented it.


https://reviews.llvm.org/D46159



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

[PATCH] D26054: Use `getFileLoc()` instead of `getSpellingLoc()` in the ASTImporter

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

I see,t hank you. Please feel free to submit a patch - it seems like you 
already have a nice test case that shows the difference between two import 
options.


https://reviews.llvm.org/D26054



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

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tidy/tool/ClangTidyMain.cpp:195
 
+/// This option Enables alpha checkers from the static analyzer, that are
+/// experimental. This option is set to false and not visible in help, because

xbolva00 wrote:
> This option enables...
> This option enables...
The current version is correct.


https://reviews.llvm.org/D46159



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

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

[Paul Fultz II via Phabricator via cfe-commits]

pfultz2 added a comment.

> When something is merged into Clang trunk, the expectation is that it will be 
> production quality or will be worked on rapidly to get it to production 
> quality, which is somewhat orthogonal to getting user feedback. I don't know 
> that I have the full history of all of the alpha checks so my generalization 
> may be inaccurate, but it seems like some of these checks were accepted as a 
> WIP and the "progress" stopped for a long time with no one volunteering to 
> remove the features from the analyzer.

Some checkers work better than others, while some just need some simple fixes. 
Of course, no one will volunteer to fix or remove them if they aren't available 
to run.

> That does not require clang-tidy to surface the alpha checks, correct?

A good portion of users are using clang-tidy to run the static analyzer, and I 
would like it to be exposed to them.


https://reviews.llvm.org/D46159



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

[PATCH] D46394: [clang-cl] Print /showIncludes to stderr, if used in combination with /E, /EP or /P

[Felix Bruns via Phabricator via cfe-commits]

fxb added a comment.

@erichkeane That would be great! Thanks!


https://reviews.llvm.org/D46394



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

[PATCH] D46394: [clang-cl] Print /showIncludes to stderr, if used in combination with /E, /EP or /P

[Erich Keane via Phabricator via cfe-commits]

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

This looks fine to me. Let me know if you need me to commit this for you.


https://reviews.llvm.org/D46394



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

[PATCH] D46394: [clang-cl] Print /showIncludes to stderr, if used in combination with /E, /EP or /P

[Felix Bruns via Phabricator via cfe-commits]

fxb updated this revision to Diff 145030.
fxb added a comment.

Updated the code to use an enum called `ShowIncludesDestination`


https://reviews.llvm.org/D46394

Files:
  include/clang/Frontend/DependencyOutputOptions.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInstance.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/HeaderIncludeGen.cpp
  test/Driver/cl-options.c

Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -198,10 +198,8 @@
 // RUN: %clang_cl /E /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E %s
 // RUN: %clang_cl /EP /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E %s
 // RUN: %clang_cl /E /EP /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E %s
-// showIncludes_E: warning: argument unused during compilation: '--show-includes'
-
-// RUN: %clang_cl /EP /P /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E_And_P %s
-// showIncludes_E_And_P-NOT: warning: argument unused during compilation: '--show-includes'
+// RUN: %clang_cl /EP /P /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E %s
+// showIncludes_E-NOT: warning: argument unused during compilation: '--show-includes'
 
 // /source-charset: should warn on everything except UTF-8.
 // RUN: %clang_cl /source-charset:utf-16 -### -- %s 2>&1 | FileCheck -check-prefix=source-charset-utf-16 %s
Index: lib/Frontend/HeaderIncludeGen.cpp
===
--- lib/Frontend/HeaderIncludeGen.cpp
+++ lib/Frontend/HeaderIncludeGen.cpp
@@ -80,9 +80,23 @@
const DependencyOutputOptions &DepOpts,
bool ShowAllHeaders, StringRef OutputPath,
bool ShowDepth, bool MSStyle) {
-  raw_ostream *OutputFile = MSStyle ? &llvm::outs() : &llvm::errs();
+  raw_ostream *OutputFile = &llvm::errs();
   bool OwnsOutputFile = false;
 
+  // Choose output stream, when printing in cl.exe /showIncludes style.
+  if (MSStyle) {
+switch (DepOpts.ShowIncludesDestination) {
+default:
+  llvm_unreachable("Invalid destination for /showIncludes output!");
+case ShowIncludesDestination::Stderr:
+  OutputFile = &llvm::errs();
+  break;
+case ShowIncludesDestination::Stdout:
+  OutputFile = &llvm::outs();
+  break;
+}
+  }
+
   // Open the output file, if used.
   if (!OutputPath.empty()) {
 std::error_code EC;
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1130,7 +1130,17 @@
   Opts.ShowHeaderIncludes = Args.hasArg(OPT_H);
   Opts.HeaderIncludeOutputFile = Args.getLastArgValue(OPT_header_include_file);
   Opts.AddMissingHeaderDeps = Args.hasArg(OPT_MG);
-  Opts.PrintShowIncludes = Args.hasArg(OPT_show_includes);
+  if (Args.hasArg(OPT_show_includes)) {
+// Writing both /showIncludes and preprocessor output to stdout
+// would produce interleaved output, so use stderr for /showIncludes.
+// This behaves the same as cl.exe, when /E, /EP or /P are passed.
+if (Args.hasArg(options::OPT_E) || Args.hasArg(options::OPT_P))
+  Opts.ShowIncludesDestination = ShowIncludesDestination::Stderr;
+else
+  Opts.ShowIncludesDestination = ShowIncludesDestination::Stdout;
+  } else {
+Opts.ShowIncludesDestination = ShowIncludesDestination::None;
+  }
   Opts.DOTOutputFile = Args.getLastArgValue(OPT_dependency_dot);
   Opts.ModuleDependencyOutputDir =
   Args.getLastArgValue(OPT_module_dependency_dir);
Index: lib/Frontend/CompilerInstance.cpp
===
--- lib/Frontend/CompilerInstance.cpp
+++ lib/Frontend/CompilerInstance.cpp
@@ -462,7 +462,7 @@
/*ShowDepth=*/false);
   }
 
-  if (DepOpts.PrintShowIncludes) {
+  if (DepOpts.ShowIncludesDestination != ShowIncludesDestination::None) {
 AttachHeaderIncludeGen(*PP, DepOpts,
/*ShowAllHeaders=*/true, /*OutputPath=*/"",
/*ShowDepth=*/true, /*MSStyle=*/true);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -5077,13 +5077,8 @@
 CmdArgs.push_back("--dependent-lib=oldnames");
   }
 
-  // Both /showIncludes and /E (and /EP) write to stdout. Allowing both
-  // would produce interleaved output, so ignore /showIncludes in such cases.
-  if ((!Args.hasArg(options::OPT_E) && !Args.hasArg(options::OPT__SLASH_EP)) ||
-  (Args.hasArg(options::OPT__SLASH_P) &&
-   Args.hasArg(options::OPT__SLASH_EP) && !Args.hasArg(options::OPT_E)))
-if (Arg *A = Args.getLastArg(options::OPT_sho

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D46159#1086553, @pfultz2 wrote:

> > I think the premise is a bit off the mark. It's not that these are not for 
> > the common user -- it's that they're simply not ready for users at all.
>
> Then why was it merged into clang in the first place? It seems like the whole 
> point of merging it into clang is to get user feedback.


When something is merged into Clang trunk, the expectation is that it will be 
production quality or will be worked on rapidly to get it to production 
quality, which is somewhat orthogonal to getting user feedback. I don't know 
that I have the full history of all of the alpha checks so my generalization 
may be inaccurate, but it seems like some of these checks were accepted as a 
WIP and the "progress" stopped for a long time with no one volunteering to 
remove the features from the analyzer.

> Furthermore, I am trying to update the conversion checker to report more 
> errors, and I would like it to be exposed to users so I can find or fix the 
> FPs.

That does not require clang-tidy to surface the alpha checks, correct?


https://reviews.llvm.org/D46159



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

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Alexander Kornienko via Phabricator via cfe-commits]

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

It looks like you've missed some comments or uploaded a wrong patch.




Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:105
+const TypeVec throwsException(const FunctionDecl *Func) {
+  static thread_local llvm::SmallSet CallStack;
+

alexfh wrote:
> I don't think this is a good use case for a static local variable. If this 
> function needs a state, either pass it from the caller or create a utility 
> class/struct for it. You can leave a non-recursive entry point with the 
> current interface, if you like. For example:
> 
>   const TypeVec throwsException(const FunctionDecl *Func, 
> llvm::SmallSet *CallStack) { ... }
> 
>   const TypeVec throwsException(const FunctionDecl *Func) {
> llvm::SmallSet CallStack;
> return throwsException(Func, &CallStack);
>   }
>   
Still not addressed?



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:110
+
+  if (const auto *Body = Func->getBody()) {
+CallStack.insert(Func);

alexfh wrote:
> Please use the concrete type here, since it's not obvious from the context.
Still not addressed?



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:119
+  if (const auto *FPT = Func->getType()->getAs()) {
+for (const auto Ex : FPT->exceptions()) {
+  Result.push_back(&*Ex);

alexfh wrote:
> Please use the concrete type here.
Still not addressed?



Comment at: clang-tidy/bugprone/ExceptionEscapeCheck.cpp:120
+for (const auto Ex : FPT->exceptions()) {
+  Result.push_back(&*Ex);
+}

alexfh wrote:
> Ex.getTypePtrOrNull() / Ex.getTypePtr() would be easier to understand here.
Still not addressed?


https://reviews.llvm.org/D33537



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

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

[Paul Fultz II via Phabricator via cfe-commits]

pfultz2 updated this revision to Diff 145029.
pfultz2 added a comment.

Rename flag to `allow-enabling-alpha-checks` and removed the option from the 
clang-tidy file.


https://reviews.llvm.org/D46159

Files:
  clang-tidy/ClangTidy.cpp
  clang-tidy/ClangTidyOptions.cpp
  clang-tidy/ClangTidyOptions.h
  clang-tidy/tool/ClangTidyMain.cpp
  test/clang-tidy/enable-alpha-checks.cpp

Index: test/clang-tidy/enable-alpha-checks.cpp
===
--- /dev/null
+++ test/clang-tidy/enable-alpha-checks.cpp
@@ -0,0 +1,6 @@
+// Check if '-allow-enabling-alpha-checks' is visible for users
+// RUN: clang-tidy -help | not grep 'enable-alpha-checks'
+
+// Check if '-allow-enabling-alpha-checks' enables alpha checks.
+// RUN: clang-tidy -checks=* -list-checks | not grep 'clang-analyzer-alpha'
+// RUN: clang-tidy -checks=* -list-checks -allow-enabling-alpha-checks | grep 'clang-analyzer-alpha'
Index: clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -192,6 +192,13 @@
cl::init(false),
cl::cat(ClangTidyCategory));
 
+/// This option allows enabling alpha checkers from the static analyzer, that
+/// are experimental. This option is set to false and not visible in help,
+/// because it is highly not recommended for users.
+static cl::opt AllowEnablingAlphaChecks("allow-enabling-alpha-checks",
+  cl::init(false), cl::Hidden,
+  cl::cat(ClangTidyCategory));
+
 static cl::opt ExportFixes("export-fixes", cl::desc(R"(
 YAML file to store suggested fixes in. The
 stored fixes can be applied to the input source
@@ -301,6 +308,7 @@
   DefaultOptions.HeaderFilterRegex = HeaderFilter;
   DefaultOptions.SystemHeaders = SystemHeaders;
   DefaultOptions.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors;
+  DefaultOptions.AllowEnablingAlphaChecks = AllowEnablingAlphaChecks;
   DefaultOptions.FormatStyle = FormatStyle;
   DefaultOptions.User = llvm::sys::Process::GetEnv("USER");
   // USERNAME is used on Windows.
Index: clang-tidy/ClangTidyOptions.h
===
--- clang-tidy/ClangTidyOptions.h
+++ clang-tidy/ClangTidyOptions.h
@@ -77,6 +77,9 @@
   /// \brief Turns on temporary destructor-based analysis.
   llvm::Optional AnalyzeTemporaryDtors;
 
+  /// \brief Turns on experimental alpha checkers from the static analyzer.
+  llvm::Optional AllowEnablingAlphaChecks;
+
   /// \brief Format code around applied fixes with clang-format using this
   /// style.
   ///
Index: clang-tidy/ClangTidyOptions.cpp
===
--- clang-tidy/ClangTidyOptions.cpp
+++ clang-tidy/ClangTidyOptions.cpp
@@ -108,6 +108,7 @@
   Options.HeaderFilterRegex = "";
   Options.SystemHeaders = false;
   Options.AnalyzeTemporaryDtors = false;
+  Options.AllowEnablingAlphaChecks = false;
   Options.FormatStyle = "none";
   Options.User = llvm::None;
   for (ClangTidyModuleRegistry::iterator I = ClangTidyModuleRegistry::begin(),
@@ -148,6 +149,8 @@
   overrideValue(Result.HeaderFilterRegex, Other.HeaderFilterRegex);
   overrideValue(Result.SystemHeaders, Other.SystemHeaders);
   overrideValue(Result.AnalyzeTemporaryDtors, Other.AnalyzeTemporaryDtors);
+  overrideValue(Result.AllowEnablingAlphaChecks,
+Other.AllowEnablingAlphaChecks);
   overrideValue(Result.FormatStyle, Other.FormatStyle);
   overrideValue(Result.User, Other.User);
   mergeVectors(Result.ExtraArgs, Other.ExtraArgs);
Index: clang-tidy/ClangTidy.cpp
===
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -303,11 +303,12 @@
 
 typedef std::vector> CheckersList;
 
-static CheckersList getCheckersControlList(ClangTidyContext &Context) {
+static CheckersList getCheckersControlList(ClangTidyContext &Context,
+   bool IncludeExperimental) {
   CheckersList List;
 
   const auto &RegisteredCheckers =
-  AnalyzerOptions::getRegisteredCheckers(/*IncludeExperimental=*/false);
+  AnalyzerOptions::getRegisteredCheckers(IncludeExperimental);
   bool AnalyzerChecksEnabled = false;
   for (StringRef CheckName : RegisteredCheckers) {
 std::string ClangTidyCheckName((AnalyzerCheckNamePrefix + CheckName).str());
@@ -374,7 +375,8 @@
   AnalyzerOptions->Config["cfg-temporary-dtors"] =
   Context.getOptions().AnalyzeTemporaryDtors ? "true" : "false";
 
-  AnalyzerOptions->CheckersControlList = getCheckersControlList(Context);
+  AnalyzerOptions->CheckersControlList = getCheckersControlList(
+  Context, *Context.getOptions().AllowEnablingAlphaChecks);
   if (!AnalyzerOptions->CheckersControlList.empty()) {
 setStaticAnalyzer

[PATCH] D33844: [clang-tidy] terminating continue check

[Alexander Kornienko via Phabricator via cfe-commits]

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

There are still outstanding comments.


https://reviews.llvm.org/D33844



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

[PATCH] D36610: [Tooling] Add option to getFullyQualifiedName using a custom PritingPolicy

[Sterling Augustine via Phabricator via cfe-commits]

saugustine added a comment.

In https://reviews.llvm.org/D36610#1083952, @mikhail.ramalho wrote:

> Ping.


Given that richard smith is the only non-approver, and that he hasn't 
responded, and that I contributed this code, I'm going to make an executive 
decision and say that this is OK to submit.

We will roll back if there are problems.


https://reviews.llvm.org/D36610



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

[PATCH] D40937: [clang-tidy] Infinite loop checker

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.



Comment at: clang-tidy/bugprone/InfiniteLoopCheck.h:37
+private:
+  bool updateSequence(Stmt *FunctionBody, ASTContext &ASTCtx);
+  const Stmt *PrevFunctionBody;

Why bool? The return value is not used anywhere.



Comment at: docs/ReleaseNotes.rst:68
+- New :doc:`bugprone-infinite-loop
+  ` 
check
+

There should be a trailing period in each of these `New ... check` items. I've 
updated the script in r331460 and the existing release notes in r331461. Please 
rebase.


https://reviews.llvm.org/D40937



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

[PATCH] D46115: [ASTImporter] properly import SrcLoc of Attr

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

Maybe this is a user error of CrossTU, but it seemed to import a FuncDecl with 
attributes, causing the imported FuncDecl to have all those attributes twice. 
That's why I thought merging would maybe make sense. However I did not 
encounter any issue with the duplicate attributes. Only the wrong source 
locations produced odd crashes.

Thanks for the input, then I will prepare the full patch.


Repository:
  rC Clang

https://reviews.llvm.org/D46115



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

[clang-tools-extra] r331461 - Added trailing periods.

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Thu May  3 09:01:49 2018
New Revision: 331461

URL: http://llvm.org/viewvc/llvm-project?rev=331461&view=rev
Log:
Added trailing periods.

Modified:
clang-tools-extra/trunk/docs/ReleaseNotes.rst

Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=331461&r1=331460&r2=331461&view=diff
==
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Thu May  3 09:01:49 2018
@@ -65,30 +65,30 @@ Improvements to clang-tidy
 - New module ``zircon`` for checks related to Fuchsia's Zircon kernel.
 
 - New :doc:`android-comparison-in-temp-failure-retry
-  ` check
+  ` check.
 
   Diagnoses comparisons that appear to be incorrectly placed in the argument to
   the ``TEMP_FAILURE_RETRY`` macro.
 
 - New :doc:`bugprone-parent-virtual-call
-  ` check
+  ` check.
 
   Detects and fixes calls to grand-...parent virtual methods instead of calls
   to overridden parent's virtual methods.
 
 - New :doc:`bugprone-throw-keyword-missing
-  ` check
+  ` check.
 
   Diagnoses when a temporary object that appears to be an exception is
   constructed but not thrown.
 
 - New :doc:`bugprone-unused-return-value
-  ` check
+  ` check.
 
   Warns on unused function return values.
 
 - New :doc:`cppcoreguidelines-avoid-goto
-  ` check
+  ` check.
 
   The usage of ``goto`` for control flow is error prone and should be replaced
   with looping constructs. Every backward jump is rejected. Forward jumps are
@@ -100,53 +100,53 @@ Improvements to clang-tidy
   added.
 
 - New :doc:`fuchsia-multiple-inheritance
-  ` check
+  ` check.
 
   Warns if a class inherits from multiple classes that are not pure virtual.
 
 - New :doc:`abseil-string-find-startswith
-  ` check
+  ` check.
 
   Checks whether a ``std::string::find()`` result is compared with 0, and
   suggests replacing with ``absl::StartsWith()``.
 
 - New :doc:`fuchsia-statically-constructed-objects
-  ` check
+  ` check.
 
   Warns if global, non-trivial objects with static storage are constructed,
   unless the object is statically initialized with a ``constexpr`` constructor
   or has no explicit constructor.
 
 - New :doc:`fuchsia-trailing-return
-  ` check
+  ` check.
 
   Functions that have trailing returns are disallowed, except for those
   using ``decltype`` specifiers and lambda with otherwise unutterable
   return types.
 
 - New :doc:`hicpp-multiway-paths-covered
-  ` check
+  ` check.
 
   Checks on ``switch`` and ``if`` - ``else if`` constructs that do not cover 
all possible code paths.
 
 - New :doc:`modernize-use-uncaught-exceptions
-  ` check
+  ` check.
 
   Finds and replaces deprecated uses of ``std::uncaught_exception`` to
   ``std::uncaught_exceptions``.
 
 - New :doc:`portability-simd-intrinsics
-  ` check
+  ` check.
 
   Warns or suggests alternatives if SIMD intrinsics are used which can be 
replaced by
   ``std::experimental::simd`` operations.
 
 - New :doc:`zircon-temporary-objects
-  ` check
+  ` check.
 
   Warns on construction of specific temporary objects in the Zircon kernel.
 
-- Adding the missing bitwise assignment operations to 
+- Added the missing bitwise assignment operations to
   :doc:`hicpp-signed-bitwise `.
 
 - New option `MinTypeNameLength` for :doc:`modernize-use-auto
@@ -156,8 +156,8 @@ Improvements to clang-tidy
   replace types with the name length >= 5 letters only (ex. ``double``,
   ``unsigned``).
 
-- Added `VariableThreshold` option to :doc:`readability-function-size
-  ` check
+- Add `VariableThreshold` option to :doc:`readability-function-size
+  ` check.
 
   Flags functions that have more than a specified number of variables declared
   in the body.


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

[clang-tools-extra] r331460 - Add a trailing period in release notes.

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Thu May  3 08:59:39 2018
New Revision: 331460

URL: http://llvm.org/viewvc/llvm-project?rev=331460&view=rev
Log:
Add a trailing period in release notes.

Modified:
clang-tools-extra/trunk/clang-tidy/add_new_check.py

Modified: clang-tools-extra/trunk/clang-tidy/add_new_check.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/add_new_check.py?rev=331460&r1=331459&r2=331460&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/add_new_check.py (original)
+++ clang-tools-extra/trunk/clang-tidy/add_new_check.py Thu May  3 08:59:39 2018
@@ -214,7 +214,7 @@ def add_release_notes(module_path, modul
   if not line.startswith(''):
 f.write("""
 - New :doc:`%s
-  ` check
+  ` check.
 
   FIXME: add release notes.
 """ % (check_name_dashes, check_name_dashes))


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