date:20181109

[PATCH] D54373: [clang]: Correct naming conventions and 80 col rule violation in CGDeclCXX.cpp. NFC.

2018-11-09 Thread Kristina Brooks via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL346582: Correct naming conventions and 80 col rule violation 
in CGDeclCXX.cpp. NFC. (authored by kristina, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D54373?vs=173493=173494#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D54373

Files:
  cfe/trunk/lib/CodeGen/CGDeclCXX.cpp


Index: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
===
--- cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
+++ cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
@@ -68,10 +68,10 @@
 
   // FIXME:  __attribute__((cleanup)) ?
 
-  QualType type = D.getType();
-  QualType::DestructionKind dtorKind = type.isDestructedType();
+  QualType Type = D.getType();
+  QualType::DestructionKind DtorKind = Type.isDestructedType();
 
-  switch (dtorKind) {
+  switch (DtorKind) {
   case QualType::DK_none:
 return;
 
@@ -86,13 +86,14 @@
 return;
   }
 
-  llvm::Constant *function;
-  llvm::Constant *argument;
+  llvm::Constant *Func;
+  llvm::Constant *Argument;
 
   // Special-case non-array C++ destructors, if they have the right signature.
   // Under some ABIs, destructors return this instead of void, and cannot be
-  // passed directly to __cxa_atexit if the target does not allow this 
mismatch.
-  const CXXRecordDecl *Record = type->getAsCXXRecordDecl();
+  // passed directly to __cxa_atexit if the target does not allow this
+  // mismatch.
+  const CXXRecordDecl *Record = Type->getAsCXXRecordDecl();
   bool CanRegisterDestructor =
   Record && (!CGM.getCXXABI().HasThisReturn(
  GlobalDecl(Record->getDestructor(), Dtor_Complete)) ||
@@ -103,21 +104,21 @@
   bool UsingExternalHelper = !CGM.getCodeGenOpts().CXAAtExit;
   if (Record && (CanRegisterDestructor || UsingExternalHelper)) {
 assert(!Record->hasTrivialDestructor());
-CXXDestructorDecl *dtor = Record->getDestructor();
+CXXDestructorDecl *Dtor = Record->getDestructor();
 
-function = CGM.getAddrOfCXXStructor(dtor, StructorType::Complete);
-argument = llvm::ConstantExpr::getBitCast(
-addr.getPointer(), CGF.getTypes().ConvertType(type)->getPointerTo());
+Func = CGM.getAddrOfCXXStructor(Dtor, StructorType::Complete);
+Argument = llvm::ConstantExpr::getBitCast(
+Addr.getPointer(), CGF.getTypes().ConvertType(Type)->getPointerTo());
 
   // Otherwise, the standard logic requires a helper function.
   } else {
-function = CodeGenFunction(CGM)
-.generateDestroyHelper(addr, type, CGF.getDestroyer(dtorKind),
-   CGF.needsEHCleanup(dtorKind), );
-argument = llvm::Constant::getNullValue(CGF.Int8PtrTy);
+Func = CodeGenFunction(CGM)
+   .generateDestroyHelper(Addr, Type, CGF.getDestroyer(DtorKind),
+  CGF.needsEHCleanup(DtorKind), );
+Argument = llvm::Constant::getNullValue(CGF.Int8PtrTy);
   }
 
-  CGM.getCXXABI().registerGlobalDtor(CGF, D, function, argument);
+  CGM.getCXXABI().registerGlobalDtor(CGF, D, Func, Argument);
 }
 
 /// Emit code to cause the variable at the given address to be considered as


Index: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
===
--- cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
+++ cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
@@ -68,10 +68,10 @@
 
   // FIXME:  __attribute__((cleanup)) ?
 
-  QualType type = D.getType();
-  QualType::DestructionKind dtorKind = type.isDestructedType();
+  QualType Type = D.getType();
+  QualType::DestructionKind DtorKind = Type.isDestructedType();
 
-  switch (dtorKind) {
+  switch (DtorKind) {
   case QualType::DK_none:
 return;
 
@@ -86,13 +86,14 @@
 return;
   }
 
-  llvm::Constant *function;
-  llvm::Constant *argument;
+  llvm::Constant *Func;
+  llvm::Constant *Argument;
 
   // Special-case non-array C++ destructors, if they have the right signature.
   // Under some ABIs, destructors return this instead of void, and cannot be
-  // passed directly to __cxa_atexit if the target does not allow this mismatch.
-  const CXXRecordDecl *Record = type->getAsCXXRecordDecl();
+  // passed directly to __cxa_atexit if the target does not allow this
+  // mismatch.
+  const CXXRecordDecl *Record = Type->getAsCXXRecordDecl();
   bool CanRegisterDestructor =
   Record && (!CGM.getCXXABI().HasThisReturn(
  GlobalDecl(Record->getDestructor(), Dtor_Complete)) ||
@@ -103,21 +104,21 @@
   bool UsingExternalHelper = !CGM.getCodeGenOpts().CXAAtExit;
   if (Record && (CanRegisterDestructor || UsingExternalHelper)) {
 assert(!Record->hasTrivialDestructor());
-CXXDestructorDecl *dtor = Record->getDestructor();
+CXXDestructorDecl *Dtor = Record->getDestructor();
 
-function = CGM.getAddrOfCXXStructor(dtor, StructorType::Complete);
-argument = llvm::ConstantExpr::getBitCast(
-addr.getPointer(),

r346582 - Correct naming conventions and 80 col rule violation in CGDeclCXX.cpp. NFC.

2018-11-09 Thread Kristina Brooks via cfe-commits

Author: kristina
Date: Fri Nov  9 23:53:47 2018
New Revision: 346582

URL: http://llvm.org/viewvc/llvm-project?rev=346582=rev
Log:
Correct naming conventions and 80 col rule violation in CGDeclCXX.cpp. NFC.

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


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

Modified: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDeclCXX.cpp?rev=346582=346581=346582=diff
==
--- cfe/trunk/lib/CodeGen/CGDeclCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDeclCXX.cpp Fri Nov  9 23:53:47 2018
@@ -68,10 +68,10 @@ static void EmitDeclDestroy(CodeGenFunct
 
   // FIXME:  __attribute__((cleanup)) ?
 
-  QualType type = D.getType();
-  QualType::DestructionKind dtorKind = type.isDestructedType();
+  QualType Type = D.getType();
+  QualType::DestructionKind DtorKind = Type.isDestructedType();
 
-  switch (dtorKind) {
+  switch (DtorKind) {
   case QualType::DK_none:
 return;
 
@@ -86,13 +86,14 @@ static void EmitDeclDestroy(CodeGenFunct
 return;
   }
 
-  llvm::Constant *function;
-  llvm::Constant *argument;
+  llvm::Constant *Func;
+  llvm::Constant *Argument;
 
   // Special-case non-array C++ destructors, if they have the right signature.
   // Under some ABIs, destructors return this instead of void, and cannot be
-  // passed directly to __cxa_atexit if the target does not allow this 
mismatch.
-  const CXXRecordDecl *Record = type->getAsCXXRecordDecl();
+  // passed directly to __cxa_atexit if the target does not allow this
+  // mismatch.
+  const CXXRecordDecl *Record = Type->getAsCXXRecordDecl();
   bool CanRegisterDestructor =
   Record && (!CGM.getCXXABI().HasThisReturn(
  GlobalDecl(Record->getDestructor(), Dtor_Complete)) ||
@@ -103,21 +104,21 @@ static void EmitDeclDestroy(CodeGenFunct
   bool UsingExternalHelper = !CGM.getCodeGenOpts().CXAAtExit;
   if (Record && (CanRegisterDestructor || UsingExternalHelper)) {
 assert(!Record->hasTrivialDestructor());
-CXXDestructorDecl *dtor = Record->getDestructor();
+CXXDestructorDecl *Dtor = Record->getDestructor();
 
-function = CGM.getAddrOfCXXStructor(dtor, StructorType::Complete);
-argument = llvm::ConstantExpr::getBitCast(
-addr.getPointer(), CGF.getTypes().ConvertType(type)->getPointerTo());
+Func = CGM.getAddrOfCXXStructor(Dtor, StructorType::Complete);
+Argument = llvm::ConstantExpr::getBitCast(
+Addr.getPointer(), CGF.getTypes().ConvertType(Type)->getPointerTo());
 
   // Otherwise, the standard logic requires a helper function.
   } else {
-function = CodeGenFunction(CGM)
-.generateDestroyHelper(addr, type, CGF.getDestroyer(dtorKind),
-   CGF.needsEHCleanup(dtorKind), );
-argument = llvm::Constant::getNullValue(CGF.Int8PtrTy);
+Func = CodeGenFunction(CGM)
+   .generateDestroyHelper(Addr, Type, CGF.getDestroyer(DtorKind),
+  CGF.needsEHCleanup(DtorKind), );
+Argument = llvm::Constant::getNullValue(CGF.Int8PtrTy);
   }
 
-  CGM.getCXXABI().registerGlobalDtor(CGF, D, function, argument);
+  CGM.getCXXABI().registerGlobalDtor(CGF, D, Func, Argument);
 }
 
 /// Emit code to cause the variable at the given address to be considered as


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

[PATCH] D54373: [clang]: Correct naming conventions and 80 col rule violation in CGDeclCXX.cpp. NFC.

2018-11-09 Thread Kristina Brooks via Phabricator via cfe-commits

kristina created this revision.
kristina added a reviewer: clang.
Herald added a subscriber: cfe-commits.

Noticed while working with that area of code, NFC.


Repository:
  rC Clang

https://reviews.llvm.org/D54373

Files:
  lib/CodeGen/CGDeclCXX.cpp


Index: lib/CodeGen/CGDeclCXX.cpp
===
--- lib/CodeGen/CGDeclCXX.cpp
+++ lib/CodeGen/CGDeclCXX.cpp
@@ -68,10 +68,10 @@
 
   // FIXME:  __attribute__((cleanup)) ?
 
-  QualType type = D.getType();
-  QualType::DestructionKind dtorKind = type.isDestructedType();
+  QualType Type = D.getType();
+  QualType::DestructionKind DtorKind = Type.isDestructedType();
 
-  switch (dtorKind) {
+  switch (DtorKind) {
   case QualType::DK_none:
 return;
 
@@ -86,13 +86,14 @@
 return;
   }
 
-  llvm::Constant *function;
-  llvm::Constant *argument;
+  llvm::Constant *Func;
+  llvm::Constant *Argument;
 
   // Special-case non-array C++ destructors, if they have the right signature.
   // Under some ABIs, destructors return this instead of void, and cannot be
-  // passed directly to __cxa_atexit if the target does not allow this 
mismatch.
-  const CXXRecordDecl *Record = type->getAsCXXRecordDecl();
+  // passed directly to __cxa_atexit if the target does not allow this
+  // mismatch.
+  const CXXRecordDecl *Record = Type->getAsCXXRecordDecl();
   bool CanRegisterDestructor =
   Record && (!CGM.getCXXABI().HasThisReturn(
  GlobalDecl(Record->getDestructor(), Dtor_Complete)) ||
@@ -103,21 +104,21 @@
   bool UsingExternalHelper = !CGM.getCodeGenOpts().CXAAtExit;
   if (Record && (CanRegisterDestructor || UsingExternalHelper)) {
 assert(!Record->hasTrivialDestructor());
-CXXDestructorDecl *dtor = Record->getDestructor();
+CXXDestructorDecl *Dtor = Record->getDestructor();
 
-function = CGM.getAddrOfCXXStructor(dtor, StructorType::Complete);
-argument = llvm::ConstantExpr::getBitCast(
-addr.getPointer(), CGF.getTypes().ConvertType(type)->getPointerTo());
+Func = CGM.getAddrOfCXXStructor(Dtor, StructorType::Complete);
+Argument = llvm::ConstantExpr::getBitCast(
+Addr.getPointer(), CGF.getTypes().ConvertType(Type)->getPointerTo());
 
   // Otherwise, the standard logic requires a helper function.
   } else {
-function = CodeGenFunction(CGM)
-.generateDestroyHelper(addr, type, CGF.getDestroyer(dtorKind),
-   CGF.needsEHCleanup(dtorKind), );
-argument = llvm::Constant::getNullValue(CGF.Int8PtrTy);
+Func = CodeGenFunction(CGM)
+   .generateDestroyHelper(Addr, Type, CGF.getDestroyer(DtorKind),
+  CGF.needsEHCleanup(DtorKind), );
+Argument = llvm::Constant::getNullValue(CGF.Int8PtrTy);
   }
 
-  CGM.getCXXABI().registerGlobalDtor(CGF, D, function, argument);
+  CGM.getCXXABI().registerGlobalDtor(CGF, D, Func, Argument);
 }
 
 /// Emit code to cause the variable at the given address to be considered as


Index: lib/CodeGen/CGDeclCXX.cpp
===
--- lib/CodeGen/CGDeclCXX.cpp
+++ lib/CodeGen/CGDeclCXX.cpp
@@ -68,10 +68,10 @@
 
   // FIXME:  __attribute__((cleanup)) ?
 
-  QualType type = D.getType();
-  QualType::DestructionKind dtorKind = type.isDestructedType();
+  QualType Type = D.getType();
+  QualType::DestructionKind DtorKind = Type.isDestructedType();
 
-  switch (dtorKind) {
+  switch (DtorKind) {
   case QualType::DK_none:
 return;
 
@@ -86,13 +86,14 @@
 return;
   }
 
-  llvm::Constant *function;
-  llvm::Constant *argument;
+  llvm::Constant *Func;
+  llvm::Constant *Argument;
 
   // Special-case non-array C++ destructors, if they have the right signature.
   // Under some ABIs, destructors return this instead of void, and cannot be
-  // passed directly to __cxa_atexit if the target does not allow this mismatch.
-  const CXXRecordDecl *Record = type->getAsCXXRecordDecl();
+  // passed directly to __cxa_atexit if the target does not allow this
+  // mismatch.
+  const CXXRecordDecl *Record = Type->getAsCXXRecordDecl();
   bool CanRegisterDestructor =
   Record && (!CGM.getCXXABI().HasThisReturn(
  GlobalDecl(Record->getDestructor(), Dtor_Complete)) ||
@@ -103,21 +104,21 @@
   bool UsingExternalHelper = !CGM.getCodeGenOpts().CXAAtExit;
   if (Record && (CanRegisterDestructor || UsingExternalHelper)) {
 assert(!Record->hasTrivialDestructor());
-CXXDestructorDecl *dtor = Record->getDestructor();
+CXXDestructorDecl *Dtor = Record->getDestructor();
 
-function = CGM.getAddrOfCXXStructor(dtor, StructorType::Complete);
-argument = llvm::ConstantExpr::getBitCast(
-addr.getPointer(), CGF.getTypes().ConvertType(type)->getPointerTo());
+Func = CGM.getAddrOfCXXStructor(Dtor, StructorType::Complete);
+Argument = llvm::ConstantExpr::getBitCast(
+Addr.getPointer(), CGF.getTypes().ConvertType(Type)->getPointerTo());
 
   //

[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

2018-11-09 Thread Kristina Brooks via Phabricator via cfe-commits

kristina added a comment.

This seems to be a peculiar side effect of weird combinations of previous uses 
of attributes `no_destroy`, `used`, and `section`, as well as complex macros 
used to create metaclass-like systems (through the use of said attributes). It 
seems that there is a serious bug somewhere, the repeated use of metaclass-like 
macros seem to break Clang's lexer/parser and leave it in a state where it 
starts behaving erratically, with this being a major symptom of it.

The trace state at the time of the crash usually resembles this:

  1.  /SourceCache/ips-1.0.0/os/credentials.cpp:20:41: current parser token 
';'
  2.  LLVM IR generation of declaration ''
  3.  /SourceCache/ips-1.0.0/os/credentials.cpp:20:1 
: Generating code for 
declaration '(anonymous namespace)::s_log'

When using `static` instead of an anonymous namespace the last token parsed is 
`static` instead of `;`. I also seem to have found a bug in implementation of 
`no_destroy`, fixing which would be considered an actual "fix" instead of a 
"workaround". I gave several other approaches of replicating this state, 
closest one being:

  class global_ns_class {
  public:
global_ns_class(int some_value) { SomeValue[2] = some_value; }
~global_ns_class() { SomeValue[2] = 1; }
  protected:
volatile int SomeValue[10];
  };
  
  #define MAKE_GVAR(_sfx, _init_val) \
  global_ns_class g_var##_sfx(_init_val); \
  __attribute((no_destroy, used, section("namedsect"))) \
  global_ns_class* g_var##_sfx##_ref = _var##_sfx;
  
  MAKE_GVAR(ONE,   1);
  MAKE_GVAR(TWO,   2);
  MAKE_GVAR(THREE, 3);
  
  namespace my_ns {
class with_nontrivial {
public:
with_nontrivial() { SomeValue[2] = 3; }
~with_nontrivial() { SomeValue[2] = 4; }
protected:
int SomeValue[10];
};

class my_class : public with_nontrivial {
public:
my_class(const char* cstr) {}
};
  }
  
  #define MAKE_VAR(_x,_y) \
namespace { constexpr char _$__##_x[] = _y; \
__attribute((no_destroy)) my_ns::my_class _x (_$__##_x); }

  MAKE_VAR(s_var, "test str lit");

Which seems to force Clang to take an incorrect code path but not trip the 
assert, unlike in the project where the assert is reliably tripped likely due 
to a manifestation of another, far more bening bug which is far beyond the 
scope of my understanding of Clang internals (it seems to be rooted in the 
preprocessor<->sema interactions). The 400k line preprocessed file is able to 
trigger it with 100% reliability, the preconditions for triggering the bug 
require the code to be semantically valid, which makes it hard to fuzz.

There seem to be too many variables to take into account to reliably reproduce 
it with the assert firing off, some manifestations just generate 
subtly-incorrect code (by emitting `__cxa_atexit` even with attribute present). 
The libc `atexit` fallback in handling of that attribute also seems like it 
could lead to issue but at this point this is just speculation.

I'm honestly not sure what to make of it at this point. Considering it's an 
immensely useful attribute for eliminating destructors of objects with 
"infinite" lifetime (I first hit this bug when I added it to all top level 
loggers which are created through a macro similar to one above), given that the 
x86_64 test suite does pass with it, I still think some kind of workaround is 
worth it (with reference to this issue)? I'd be happy to revert it if/once a 
concrete cause is found but so far I seem to be hitting dead ends.

I added a logger for when this specific issue is triggered to my downstream 
fork and it seems in most cases it causes "bening" behavior of attribute being 
ineffective. The assert is tripped when the attribute is partially ineffective, 
leading to Clang trying to emit code to register a destructor that doesn't 
exist with cxxabi.

Considering this is something I will likely have to dig through by hand, can 
anyone help me brainstorm how to approach this besides fuzzing it which I don't 
think will work due to the precondition of the code being semantically valid 
(not being diagnosed with an error)?


https://reviews.llvm.org/D54344



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

[PATCH] D53522: [Frontend] Include module map header declaration in dependency file output

2018-11-09 Thread Volodymyr Sapsai via Phabricator via cfe-commits

vsapsai added inline comments.



Comment at: clang/lib/Lex/ModuleMap.cpp:1198-1203
+// If the header in the module map refers to a symlink, Header.Entry
+// refers to the actual file. The callback should be notified of both.
+if (!FullPathAsWritten.empty() &&
+!FullPathAsWritten.equals(Header.Entry->getName())) {
+  Cb->moduleMapAddHeader(FullPathAsWritten, Mod->IsSystem, Imported);
+}

erik.pilkington wrote:
> vsapsai wrote:
> > vsapsai wrote:
> > > It is strange but after removing this part the tests are still passing. I 
> > > suspect the code is correct but the test allows some roundabout way to 
> > > add symlink to dependencies. In my experiments only 
> > > `DFGMMCallback::moduleMapAddHeader` affects the tests. Is it expected?
> > Looks like you have 3 cases:
> > 
> > 1. Add all files in module map to dependencies, even if a file isn't 
> > `#include`d anywhere (this turned out to be `link.h`).
> > 2. For `-fmodule-file` replace header files in dependencies with .pcm file 
> > (that's what `Imported` achieves).
> > 3. Some scenario with symlinks. Here I haven't found a representative test 
> > case.
> 3 and 1 should be the same; the problem is that a `FileEntry`'s name mutates 
> over the course of the compile to refer to the most recent reference to it. 
> (see FileManager::getFile() and the FIXME from 2007). This puts us in a 
> pretty awkward position here: we're trying to recover the set of symlinks 
> that clang used to refer to this file, but I think that that information is 
> lost in the general case. The Right Thing To Do is probably to actually track 
> that somewhere. I think we could also probably work around this by attaching 
> the DependencyFileGenerator callback to the module builder thread, in which 
> case we'd be able to ensure we use the most-recent version of a `FileEntry`.
> 
> I'll keep trying to get a reproducer for this, but FYI you can check it out 
> in action for the file `ncurses.h` in the SDK.
I think your test case reproduces `ncurses.h` situation properly. And now I see 
the problem is with symlinks. It is fixed by 
`DFGMMCallback::moduleMapAddHeader` but `moduleMapAddHeader(FullPathAsWritten` 
doesn't affect that. I suspect it can be useful when in module map you have 
entries both for a real file and its symlink but that's a different case.

I was playing a little bit more with umbrella headers and symlinks and think 
there is a case not covered. I'll have to double check that's not a mistake in 
my testing and will post it as an example. It is fairly convoluted but maybe it 
can be reduced to something more likely to happen in a real codebase.


https://reviews.llvm.org/D53522



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

[PATCH] D52296: [Clang] - Add -fdwarf-fission=split,single option.

2018-11-09 Thread David Blaikie via Phabricator via cfe-commits

dblaikie added a comment.

OK - thanks for that.

I'm going to make an executive decision on the naming. Let's go with 
-gsplit-dwarf[=single] (or explicitly -gsplit-dwarf=split, which is the default 
value when -gsplit-dwarf is specified). Saves adding a new name/flag, avoids 
the use of the word Fission which isn't a standard term & so best avoided as a 
way of talking about configuring DWARF standardized functionality.

Could you make that change? & I'll give it another look over after that, but 
should be fine.


https://reviews.llvm.org/D52296



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

[PATCH] D18860: [analyzer] Fix the "Zombie symbols" issue.

2018-11-09 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added a comment.

The manual region cleanup is removed from `MisusedMovedObjectChecker` in 
https://reviews.llvm.org/D54372.

> Another thing, since we're directly testing the functionality of 
> `SymbolReaper`, maybe it'd be worth to also include unit tests.

Sounds like a great idea, and/or i should add `ExprInspection`-based tests into 
`test/Analysis/symbol-reaper.c`.


https://reviews.llvm.org/D18860



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

[PATCH] D54372: [analyzer] MisusedMovedObject: NFC: Remove dead code after D18860

2018-11-09 Thread 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, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, baloghadamsoftware.

Since https://reviews.llvm.org/D18860 addresses the problem that some regions 
are not properly cleaned up, as explained in D18860#1294011 
, it's no longer necessary to clean 
them up manually. This patch removes the respective code from 
`checkEndFunction` and from `checkPostCall` for destructors. No functional 
change intended.


Repository:
  rC Clang

https://reviews.llvm.org/D54372

Files:
  lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp


Index: lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
@@ -43,10 +43,9 @@
 };
 
 class MisusedMovedObjectChecker
-: public Checker {
 public:
-  void checkEndFunction(const ReturnStmt *RS, CheckerContext ) const;
   void checkPreCall(const CallEvent , CheckerContext ) const;
   void checkPostCall(const CallEvent , CheckerContext ) const;
   void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
@@ -218,42 +217,6 @@
   return nullptr;
 }
 
-// Removing the function parameters' MemRegion from the state. This is needed
-// for PODs where the trivial destructor does not even created nor executed.
-void MisusedMovedObjectChecker::checkEndFunction(const ReturnStmt *RS,
- CheckerContext ) const {
-  auto State = C.getState();
-  TrackedRegionMapTy Objects = State->get();
-  if (Objects.isEmpty())
-return;
-
-  auto LC = C.getLocationContext();
-
-  const auto LD = dyn_cast_or_null(LC->getDecl());
-  if (!LD)
-return;
-  llvm::SmallSet InvalidRegions;
-
-  for (auto Param : LD->parameters()) {
-auto Type = Param->getType().getTypePtrOrNull();
-if (!Type)
-  continue;
-if (!Type->isPointerType() && !Type->isReferenceType()) {
-  InvalidRegions.insert(State->getLValue(Param, LC).getAsRegion());
-}
-  }
-
-  if (InvalidRegions.empty())
-return;
-
-  for (const auto  : State->get()) {
-if (InvalidRegions.count(E.first->getBaseRegion()))
-  State = State->remove(E.first);
-  }
-
-  C.addTransition(State);
-}
-
 void MisusedMovedObjectChecker::checkPostCall(const CallEvent ,
   CheckerContext ) const {
   const auto *AFC = dyn_cast();
@@ -392,12 +355,6 @@
   if (!ThisRegion)
 return;
 
-  if (dyn_cast_or_null(Call.getDecl())) {
-State = removeFromState(State, ThisRegion);
-C.addTransition(State);
-return;
-  }
-
   const auto MethodDecl = dyn_cast_or_null(IC->getDecl());
   if (!MethodDecl)
 return;


Index: lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
@@ -43,10 +43,9 @@
 };
 
 class MisusedMovedObjectChecker
-: public Checker {
 public:
-  void checkEndFunction(const ReturnStmt *RS, CheckerContext ) const;
   void checkPreCall(const CallEvent , CheckerContext ) const;
   void checkPostCall(const CallEvent , CheckerContext ) const;
   void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
@@ -218,42 +217,6 @@
   return nullptr;
 }
 
-// Removing the function parameters' MemRegion from the state. This is needed
-// for PODs where the trivial destructor does not even created nor executed.
-void MisusedMovedObjectChecker::checkEndFunction(const ReturnStmt *RS,
- CheckerContext ) const {
-  auto State = C.getState();
-  TrackedRegionMapTy Objects = State->get();
-  if (Objects.isEmpty())
-return;
-
-  auto LC = C.getLocationContext();
-
-  const auto LD = dyn_cast_or_null(LC->getDecl());
-  if (!LD)
-return;
-  llvm::SmallSet InvalidRegions;
-
-  for (auto Param : LD->parameters()) {
-auto Type = Param->getType().getTypePtrOrNull();
-if (!Type)
-  continue;
-if (!Type->isPointerType() && !Type->isReferenceType()) {
-  InvalidRegions.insert(State->getLValue(Param, LC).getAsRegion());
-}
-  }
-
-  if (InvalidRegions.empty())
-return;
-
-  for (const auto  : State->get()) {
-if (InvalidRegions.count(E.first->getBaseRegion()))
-  State = State->remove(E.first);
-  }
-
-  C.addTransition(State);
-}
-
 void MisusedMovedObjectChecker::checkPostCall(const CallEvent ,
   CheckerContext ) const {
   const auto *AFC = dyn_cast();
@@ -392,12 +355,6 @@
   if (!ThisRegion)
 return;
 
-  if (dyn_cast_or_null(Call.getDecl())) {
-State = removeFromState(State, ThisRegion);
-C.addTransition(State);
-

[PATCH] D18860: [analyzer] Fix the "Zombie symbols" issue.

2018-11-09 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ updated this revision to Diff 173487.
NoQ added a comment.

Add an interesting test for the `MisusedMovedObject` checker that demonstrates 
one more potential source of false positives caused by the zombie symbol 
problem. In this test there are, well, //no symbols//. Therefore, there are no 
dead symbols or zombie symbols. Therefore `SymReaper.hasDeadSymbols()` is 
always `false`. Therefore `checkDeadSymbols()` is never called at all. However, 
`MisusedMovedObject` checker is not interested in symbols; it is only 
interested in regions, including concrete regions that aren't based on symbols. 
So it was missing the `checkDeadSymbols()` callback that would have unmarked 
the region for variable `e` (in inlined function or not in inlined function - 
doesn't matter). And next time it sees variable `e` in that function within the 
same stack frame, it thinks it's the same variable that has just been moved.

This problem was already discussed in D24246?id=82469#inline-249803 
.

Add tests in `loop-block-counts.c` that demonstrate the other source of the 
problem in `MisusedMovedObject`: in fact, variable `e` should not be the same 
variable on different iterations of the loop. In case of the inlined function, 
the problem is caused by how our `StackFrameContext` doesn't contain "block 
count" for the entrance - which is a hack to discriminate between different 
iterations of the loop that is used for, eg., conjured symbols, but, 
unfortunately, not for addresses of variables / temporaries. In case of 
non-inlined functions, the problem is deeper: we simply don't have a 
`LocationContext` for a single loop iteration, so there's no way we can 
discriminate between loop locals on different loop iterations by their memory 
spaces.


https://reviews.llvm.org/D18860

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
  lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp
  lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
  lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
  lib/StaticAnalyzer/Checkers/StreamChecker.cpp
  lib/StaticAnalyzer/Core/Environment.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  lib/StaticAnalyzer/Core/SymbolManager.cpp
  test/Analysis/MisusedMovedObject.cpp
  test/Analysis/keychainAPI.m
  test/Analysis/loop-block-counts.c
  test/Analysis/pr22954.c
  test/Analysis/retain-release-cpp-classes.cpp
  test/Analysis/self-assign.cpp
  test/Analysis/simple-stream-checks.c
  test/Analysis/unions.cpp

Index: test/Analysis/unions.cpp
===
--- test/Analysis/unions.cpp
+++ test/Analysis/unions.cpp
@@ -90,9 +90,8 @@
 char str[] = "abc";
 vv.s = str;
 
-// FIXME: This is a leak of uu.s.
 uu = vv;
-  }
+  } // expected-warning{{leak}}
 
   void testIndirectInvalidation() {
 IntOrString uu;
Index: test/Analysis/simple-stream-checks.c
===
--- test/Analysis/simple-stream-checks.c
+++ test/Analysis/simple-stream-checks.c
@@ -89,3 +89,8 @@
   fs.p = fopen("myfile.txt", "w");
   fakeSystemHeaderCall(); // invalidates fs, making fs.p unreachable
 }  // no-warning
+
+void testOverwrite() {
+  FILE *fp = fopen("myfile.txt", "w");
+  fp = 0;
+} // expected-warning {{Opened file is never closed; potential resource leak}}
Index: test/Analysis/self-assign.cpp
===
--- test/Analysis/self-assign.cpp
+++ test/Analysis/self-assign.cpp
@@ -32,13 +32,14 @@
   clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}}
   free(str); // expected-note{{Memory is released}}
   str = strdup(rhs.str); // expected-warning{{Use of memory after it is freed}}  expected-note{{Use of memory after it is freed}}
+// expected-note@-1{{Memory is allocated}}
   return *this;
 }
 
 StringUsed& StringUsed::operator=(StringUsed &) { // expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs != *this}}
   clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}}
   str = rhs.str;
-  rhs.str = nullptr; // FIXME: An improved leak checker should warn here
+  rhs.str = nullptr; // expected-warning{{Potential memory leak}} expected-note{{Potential memory leak}}
   return *this;
 }
 
@@ -83,7 +84,7 @@
 
 int main() {
   StringUsed s1 ("test"), s2;
-  s2 = s1;
-  s2 = std::move(s1);
+  s2 = s1; //

[PATCH] D54222: [clang-tidy] Add a check to detect returning static locals in public headers

2018-11-09 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added a comment.

Thanks! I don't have much to add to the review, but i very much approve this 
check.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54222



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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

2018-11-09 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

What do you think about code like:

  #if FOO == 4
  #if FOO == 4
  #endif
  #endif
  
  #if defined(FOO)
  #if defined(FOO)
  #endif
  #endif
  
  #if !defined(FOO)
  #if !defined(FOO)
  #endif
  #endif
  
  #if defined(FOO)
  #if !defined(FOO)
  #endif
  #endif
  
  #if !defined(FOO)
  #if defined(FOO)
  #endif
  #endif




Comment at: clang-tidy/readability/CMakeLists.txt:23
   ReadabilityTidyModule.cpp
+  RedundantPreprocessorCheck.cpp
   RedundantControlFlowCheck.cpp

Please keep this list sorted alphabetically.



Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:52
+private:
+  void Startif(SourceLocation Loc, const Token ,
+   SmallVector , StringRef Warning,

This name is not particularly descriptive. This seems to be more like 
`CheckMacroRedundancy` or something like that?



Comment at: clang-tidy/readability/RedundantPreprocessorCheck.h:1-2
+//===--- RedundantPreprocessorCheck.h - clang-tidy ---*- 
C++
+//-*-===//
+//

This comment should be re-flowed to fit the column width.



Comment at: clang-tidy/readability/RedundantPreprocessorCheck.h:20
+
+/// This check flags redundant preprocessor usage.
+///

What constitutes "redundancy"? A bit more exposition here would be useful.


https://reviews.llvm.org/D54349



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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

2018-11-09 Thread Miklos Vajna via Phabricator via cfe-commits

vmiklos updated this revision to Diff 173468.
vmiklos edited the summary of this revision.

https://reviews.llvm.org/D54349

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/readability/RedundantPreprocessorCheck.cpp
  clang-tidy/readability/RedundantPreprocessorCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-redundant-preprocessor.rst
  test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
  test/clang-tidy/readability-redundant-preprocessor.cpp
  test/clang-tidy/readability-redundant-preprocessor.h

Index: test/clang-tidy/readability-redundant-preprocessor.h
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor.h
@@ -0,0 +1,5 @@
+#ifndef FOO
+#ifndef FOO // this would warn, but not in a header
+void f();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-preprocessor.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor.cpp
@@ -0,0 +1,23 @@
+// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -I %S
+
+// Positive testing.
+#ifndef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor]
+#ifndef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here
+void f();
+#endif
+#endif
+
+// Negative testing.
+#include "readability-redundant-preprocessor.h"
+
+#ifndef BAR
+void g();
+#endif
+
+#ifndef FOO
+#ifndef BAR
+void h();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -DFOO
+
+// Positive testing.
+#ifdef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor]
+#ifdef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here
+void f();
+#endif
+#endif
+
+// Negative testing.
+#ifdef BAR
+void g();
+#endif
+
+#ifdef FOO
+#ifdef BAR
+void h();
+#endif
+#endif
Index: docs/clang-tidy/checks/readability-redundant-preprocessor.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-redundant-preprocessor.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - readability-redundant-preprocessor
+
+readability-redundant-preprocessor
+==
+
+Finds potentially redundant preprocessor directives. At the moment the
+following cases are detected:
+
+* `#ifdef` .. `#endif` pairs which are nested inside an outer pair with the
+  same condition. For example:
+
+.. code-block:: c++
+
+  #ifdef FOO
+  #ifdef FOO // inner ifdef is considered redundant
+  void f();
+  #endif
+  #endif
+
+* Same for `#ifndef` .. `#endif` pairs. For example:
+
+.. code-block:: c++
+
+  #ifndef FOO
+  #ifndef FOO // inner ifndef is considered redundant
+  void f();
+  #endif
+  #endif
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -244,6 +244,7 @@
readability-redundant-declaration
readability-redundant-function-ptr-dereference
readability-redundant-member-init
+   readability-redundant-preprocessor
readability-redundant-smartptr-get
readability-redundant-string-cstr
readability-redundant-string-init
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -154,6 +154,11 @@
   Detects usage of magic numbers, numbers that are used as literals instead of
   introduced via constants or symbols.
 
+- New :doc:`readability-redundant-preprocessor
+  ` check.
+
+  Finds potentially redundant preprocessor directives.
+
 - New :doc:`readability-uppercase-literal-suffix
   ` check.
 
Index: clang-tidy/readability/RedundantPreprocessorCheck.h
===
--- /dev/null
+++ clang-tidy/readability/RedundantPreprocessorCheck.h
@@ -0,0 +1,35 @@
+//===--- RedundantPreprocessorCheck.h - clang-tidy ---*- C++
+//-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTPREPROCESSORCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTPREPROCESSORCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

2018-11-09 Thread Miklos Vajna via Phabricator via cfe-commits

vmiklos added a comment.

> preprocessor directives? Same in documentation.

Done.


https://reviews.llvm.org/D54349



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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

2018-11-09 Thread Miklos Vajna via Phabricator via cfe-commits

vmiklos added a comment.

> I think that name is not very descriptive for the user of clang-tidy. `pp`
>  should be `preprocessor` or some other constellation that makes it very clear
>  its about the preprocessor.

Done, renamed to readability-redundant-preprocessor.

> you are in namespace `clang`, you can ellide `clang::`

Done.

> Maybe `SmallVector`? Would be better for performance.

Done.

> I think it would be better to have these methods defined inline, as the
>  splitting does not achieve its original goal (declaration in header,
>  definition in impl).

Done.

> The two functions are copied, please remove this duplicatoin and refactor it
>  to a general parametrized function.

Done.

> Please order the checks alphabetically in the release notes, and one empty
>  line at the end is enough.

Done.

> This needs more explanation, please add `.. code-block:: c++` sections for
>  the cases that demonstrate the issue.

Done.

> Please add a test where the redundancy comes from including other headers. If
>  the headers are nested this case might still occur, but its not safe to
>  diagnose/remove these checks as other include-places might not have the same
>  constellation.
> 
> `ifdef` and `ifndef` are used for the include-guards and therefore
>  particularly necessary to test.

Done. I've added a test to make sure we don't warn in headers, the code for
this was already there, just had no coverage. (Exactly for the reason you
mention, the possibility of include-guards generating false positives.)


https://reviews.llvm.org/D54349



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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'

2018-11-09 Thread Miklos Vajna via Phabricator via cfe-commits

vmiklos updated this revision to Diff 173465.
vmiklos retitled this revision from "[clang-tidy] new check 
'readability-redundant-pp'" to "[clang-tidy] new check 
'readability-redundant-preprocessor'".

https://reviews.llvm.org/D54349

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/readability/RedundantPreprocessorCheck.cpp
  clang-tidy/readability/RedundantPreprocessorCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-redundant-preprocessor.rst
  test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
  test/clang-tidy/readability-redundant-preprocessor.cpp
  test/clang-tidy/readability-redundant-preprocessor.h

Index: test/clang-tidy/readability-redundant-preprocessor.h
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor.h
@@ -0,0 +1,5 @@
+#ifndef FOO
+#ifndef FOO // this would warn, but not in a header
+void f();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-preprocessor.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor.cpp
@@ -0,0 +1,23 @@
+// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -I %S
+
+// Positive testing.
+#ifndef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor]
+#ifndef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here
+void f();
+#endif
+#endif
+
+// Negative testing.
+#include "readability-redundant-preprocessor.h"
+
+#ifndef BAR
+void g();
+#endif
+
+#ifndef FOO
+#ifndef BAR
+void h();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -DFOO
+
+// Positive testing.
+#ifdef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor]
+#ifdef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here
+void f();
+#endif
+#endif
+
+// Negative testing.
+#ifdef BAR
+void g();
+#endif
+
+#ifdef FOO
+#ifdef BAR
+void h();
+#endif
+#endif
Index: docs/clang-tidy/checks/readability-redundant-preprocessor.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-redundant-preprocessor.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - readability-redundant-preprocessor
+
+readability-redundant-preprocessor
+==
+
+Finds potentially redundant preprocessor usage. At the moment the following
+cases are detected:
+
+* `#ifdef` .. `#endif` pairs which are nested inside an outer pair with the
+  same condition. For example:
+
+.. code-block:: c++
+
+  #ifdef FOO
+  #ifdef FOO // inner ifdef is considered redundant
+  void f();
+  #endif
+  #endif
+
+* Same for `#ifndef` .. `#endif` pairs. For example:
+
+.. code-block:: c++
+
+  #ifndef FOO
+  #ifndef FOO // inner ifndef is considered redundant
+  void f();
+  #endif
+  #endif
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -244,6 +244,7 @@
readability-redundant-declaration
readability-redundant-function-ptr-dereference
readability-redundant-member-init
+   readability-redundant-preprocessor
readability-redundant-smartptr-get
readability-redundant-string-cstr
readability-redundant-string-init
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -154,6 +154,11 @@
   Detects usage of magic numbers, numbers that are used as literals instead of
   introduced via constants or symbols.
 
+- New :doc:`readability-redundant-preprocessor
+  ` check.
+
+  Finds potentially redundant preprocessor usage.
+
 - New :doc:`readability-uppercase-literal-suffix
   ` check.
 
Index: clang-tidy/readability/RedundantPreprocessorCheck.h
===
--- /dev/null
+++ clang-tidy/readability/RedundantPreprocessorCheck.h
@@ -0,0 +1,35 @@
+//===--- RedundantPreprocessorCheck.h - clang-tidy ---*- C++
+//-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTPREPROCESSORCHECK_H
+#define

[PATCH] D53524: [ThinLTO] Enable LTOUnit only when it is needed

2018-11-09 Thread Peter Collingbourne via Phabricator via cfe-commits

pcc added inline comments.



Comment at: docs/LTOVisibility.rst:9
 unit's *LTO unit* is the subset of the linkage unit that is linked together
-using link-time optimization; in the case where LTO is not being used, the
-linkage unit's LTO unit is empty. Each linkage unit has only a single LTO unit.
+using link-time optimization; in the case where LTO units are not being used,
+the linkage unit's LTO unit is empty. Each linkage unit has only a single LTO

It's a little confusing to talk about "LTO units" as a property of a 
translation unit when there is only one LTO unit per linkage unit. I think this 
should say that an LTO unit is the subset of the linkage unit compiled with 
certain flags. Then in the rest of the document you can talk about translation 
units that are either part of or not part of the LTO unit.


Repository:
  rC Clang

https://reviews.llvm.org/D53524



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

[PATCH] D53522: [Frontend] Include module map header declaration in dependency file output

2018-11-09 Thread Erik Pilkington via Phabricator via cfe-commits

erik.pilkington added inline comments.



Comment at: clang/lib/Lex/ModuleMap.cpp:1198-1203
+// If the header in the module map refers to a symlink, Header.Entry
+// refers to the actual file. The callback should be notified of both.
+if (!FullPathAsWritten.empty() &&
+!FullPathAsWritten.equals(Header.Entry->getName())) {
+  Cb->moduleMapAddHeader(FullPathAsWritten, Mod->IsSystem, Imported);
+}

vsapsai wrote:
> vsapsai wrote:
> > It is strange but after removing this part the tests are still passing. I 
> > suspect the code is correct but the test allows some roundabout way to add 
> > symlink to dependencies. In my experiments only 
> > `DFGMMCallback::moduleMapAddHeader` affects the tests. Is it expected?
> Looks like you have 3 cases:
> 
> 1. Add all files in module map to dependencies, even if a file isn't 
> `#include`d anywhere (this turned out to be `link.h`).
> 2. For `-fmodule-file` replace header files in dependencies with .pcm file 
> (that's what `Imported` achieves).
> 3. Some scenario with symlinks. Here I haven't found a representative test 
> case.
3 and 1 should be the same; the problem is that a `FileEntry`'s name mutates 
over the course of the compile to refer to the most recent reference to it. 
(see FileManager::getFile() and the FIXME from 2007). This puts us in a pretty 
awkward position here: we're trying to recover the set of symlinks that clang 
used to refer to this file, but I think that that information is lost in the 
general case. The Right Thing To Do is probably to actually track that 
somewhere. I think we could also probably work around this by attaching the 
DependencyFileGenerator callback to the module builder thread, in which case 
we'd be able to ensure we use the most-recent version of a `FileEntry`.

I'll keep trying to get a reproducer for this, but FYI you can check it out in 
action for the file `ncurses.h` in the SDK.


https://reviews.llvm.org/D53522



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

[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

2018-11-09 Thread Kristina Brooks via Phabricator via cfe-commits

kristina added a comment.

In https://reviews.llvm.org/D54344#1293643, @aaron.ballman wrote:

> In https://reviews.llvm.org/D54344#1293468, @erik.pilkington wrote:
>
> > Have you tried running creduce on the preprocessed source? We should really 
> > have a real reproducer for this, otherwise its really hard to tell what the 
> > underlying problem is.
>
>
> I'm going to echo this request -- without a test case, it's hard to know 
> whether this actually fixes anything or just moves the issue elsewhere.

Running the creduce on it now, it's slowly chugging along, will see what 
happens, only `15112502 bytes` to go, I'll get back to this in a few days and 
see what/if anything it comes up with. I do hope I launched it right.

https://reviews.llvm.org/D54344

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

[PATCH] D53995: [analyzer] Drastically simplify the tblgen files used for checkers

2018-11-09 Thread Umann Kristóf via Phabricator via cfe-commits

Szelethus added a comment.

> I agree that it's aesthetically satisfying, but it is (1) inconvenient to 
> write because that's the whole new syntax you need to memorize, i.e. all 
> those <> and semicolons and (2) not cooperating when i want to look up 
> checker name by source file (have to scan a few pages up to see the alias for 
> the package and then jump multiple times to see what it expands to). So i'm 
> still for ditching tblgen eventually :) But i definitely don't insist.

It is also a very powerful tool, and once you figure out how it works (which 
obviously can't be expected every time it has to be touched), it does its job 
wonderfully. The issue with recreating this by abusing the preprocessor is that 
it leads to

- Very cryptic errors messages while developing
- A **ton** of warnings if by accident it's commited with a flaw that a 
platform will pick up (I managed to generate 976! And that's not counting 
non-windows platforms)
- Arguably harder to comprehend code, that either "just works", or will be the 
nightmare of the ages to fix

Like many other things around here, tblgen usage is severely underdocumented, 
and if the greatest drawback of not understanding how it works could be fixed 
(via even more documentation), I believe we'd appreciate this tool a lot more. 
I'm more than willing to work on this a bit more, as I already chew through 
most of the related files in the last couple weeks.


Repository:
  rC Clang

https://reviews.llvm.org/D53995



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

[PATCH] D54288: Fix ClangFormat issue of recognizing ObjC subscript as C++ attributes when message target is a result of a C-style method.

2018-11-09 Thread Yan Zhang via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rC346566: Fix ClangFormat issue of recognizing ObjC subscript 
as C++ attributes when… (authored by Wizard, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D54288?vs=173258=173457#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D54288

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


Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -366,7 +366,8 @@
   // specifier parameter, although this is technically valid:
   // [[foo(:)]]
   if (AttrTok->is(tok::colon) ||
-  AttrTok->startsSequence(tok::identifier, tok::identifier))
+  AttrTok->startsSequence(tok::identifier, tok::identifier) || 
+  AttrTok->startsSequence(tok::r_paren, tok::identifier))
 return false;
   if (AttrTok->is(tok::ellipsis))
 return true;
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6472,6 +6472,8 @@
   // Make sure we do not mistake attributes for array subscripts.
   verifyFormat("int a() {}\n"
"[[unused]] int b() {}\n");
+  verifyFormat("NSArray *arr;\n"
+   "arr[[Foo() bar]];");
 
   // On the other hand, we still need to correctly find array subscripts.
   verifyFormat("int a = std::vector{1, 2, 3}[0];");


Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -366,7 +366,8 @@
   // specifier parameter, although this is technically valid:
   // [[foo(:)]]
   if (AttrTok->is(tok::colon) ||
-  AttrTok->startsSequence(tok::identifier, tok::identifier))
+  AttrTok->startsSequence(tok::identifier, tok::identifier) || 
+  AttrTok->startsSequence(tok::r_paren, tok::identifier))
 return false;
   if (AttrTok->is(tok::ellipsis))
 return true;
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6472,6 +6472,8 @@
   // Make sure we do not mistake attributes for array subscripts.
   verifyFormat("int a() {}\n"
"[[unused]] int b() {}\n");
+  verifyFormat("NSArray *arr;\n"
+   "arr[[Foo() bar]];");
 
   // On the other hand, we still need to correctly find array subscripts.
   verifyFormat("int a = std::vector{1, 2, 3}[0];");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54288: Fix ClangFormat issue of recognizing ObjC subscript as C++ attributes when message target is a result of a C-style method.

2018-11-09 Thread Yan Zhang via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL346566: Fix ClangFormat issue of recognizing ObjC subscript 
as C++ attributes when… (authored by Wizard, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D54288

Files:
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -366,7 +366,8 @@
   // specifier parameter, although this is technically valid:
   // [[foo(:)]]
   if (AttrTok->is(tok::colon) ||
-  AttrTok->startsSequence(tok::identifier, tok::identifier))
+  AttrTok->startsSequence(tok::identifier, tok::identifier) || 
+  AttrTok->startsSequence(tok::r_paren, tok::identifier))
 return false;
   if (AttrTok->is(tok::ellipsis))
 return true;
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -6472,6 +6472,8 @@
   // Make sure we do not mistake attributes for array subscripts.
   verifyFormat("int a() {}\n"
"[[unused]] int b() {}\n");
+  verifyFormat("NSArray *arr;\n"
+   "arr[[Foo() bar]];");
 
   // On the other hand, we still need to correctly find array subscripts.
   verifyFormat("int a = std::vector{1, 2, 3}[0];");


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -366,7 +366,8 @@
   // specifier parameter, although this is technically valid:
   // [[foo(:)]]
   if (AttrTok->is(tok::colon) ||
-  AttrTok->startsSequence(tok::identifier, tok::identifier))
+  AttrTok->startsSequence(tok::identifier, tok::identifier) || 
+  AttrTok->startsSequence(tok::r_paren, tok::identifier))
 return false;
   if (AttrTok->is(tok::ellipsis))
 return true;
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -6472,6 +6472,8 @@
   // Make sure we do not mistake attributes for array subscripts.
   verifyFormat("int a() {}\n"
"[[unused]] int b() {}\n");
+  verifyFormat("NSArray *arr;\n"
+   "arr[[Foo() bar]];");
 
   // On the other hand, we still need to correctly find array subscripts.
   verifyFormat("int a = std::vector{1, 2, 3}[0];");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r346566 - Fix ClangFormat issue of recognizing ObjC subscript as C++ attributes when message target is a result of a C-style method.

2018-11-09 Thread Yan Zhang via cfe-commits

Author: wizard
Date: Fri Nov  9 15:19:14 2018
New Revision: 346566

URL: http://llvm.org/viewvc/llvm-project?rev=346566=rev
Log:
Fix ClangFormat issue of recognizing ObjC subscript as C++ attributes when 
message target is a result of a C-style method.

Summary:
The issue is that for array subscript like:

```
arr[[Foo() bar]];
```
ClangFormat will recognize it as C++11 attribute syntax and put a space between 
'arr' and first '[', like:

```
arr [[Foo() bar]];
```

Now it is fixed. Tested with:
```
ninja FormatTests
```

Reviewers: benhamilton

Reviewed By: benhamilton

Subscribers: cfe-commits

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

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=346566=346565=346566=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Fri Nov  9 15:19:14 2018
@@ -366,7 +366,8 @@ private:
   // specifier parameter, although this is technically valid:
   // [[foo(:)]]
   if (AttrTok->is(tok::colon) ||
-  AttrTok->startsSequence(tok::identifier, tok::identifier))
+  AttrTok->startsSequence(tok::identifier, tok::identifier) || 
+  AttrTok->startsSequence(tok::r_paren, tok::identifier))
 return false;
   if (AttrTok->is(tok::ellipsis))
 return true;

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=346566=346565=346566=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Fri Nov  9 15:19:14 2018
@@ -6472,6 +6472,8 @@ TEST_F(FormatTest, UnderstandsSquareAttr
   // Make sure we do not mistake attributes for array subscripts.
   verifyFormat("int a() {}\n"
"[[unused]] int b() {}\n");
+  verifyFormat("NSArray *arr;\n"
+   "arr[[Foo() bar]];");
 
   // On the other hand, we still need to correctly find array subscripts.
   verifyFormat("int a = std::vector{1, 2, 3}[0];");


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

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

2018-11-09 Thread Tamás Zolnai via Phabricator via cfe-commits

ztamas added a comment.

In https://reviews.llvm.org/D53974#1293268, @JonasToth wrote:

> LGTM.
>  Did you run this check in its final form against a bigger project? These 
> results would be interesting.

I'll run it on LibreOffice code again and we'll see.

> Do you have commit access?

Commit access? This is my first patch.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53974

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

[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

2018-11-09 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added a comment.

In https://reviews.llvm.org/D53692#1293778, @Szelethus wrote:

> Did you know that clang unit tests do not run with check-clang-analysis?


Well, *some* unit tests definitely do run under check-clang-analysis.


Repository:
  rC Clang

https://reviews.llvm.org/D53692



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

[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

2018-11-09 Thread Umann Kristóf via Phabricator via cfe-commits

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

Thanks! This patch was the last for a while directly related to non-checker 
config options, I'm currently stuck on them as I unearthed countless bugs that 
I have to fix beforehand. (Did you know that clang unit tests do not run with 
check-clang-analysis?)

Comment at: test/Analysis/analyzer-config.cpp:1
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 %s -o /dev/null 
-analyzer-checker=core,osx.cocoa,debug.ConfigDumper -analyzer-max-loop 34 > %t 
2>&1
 // RUN: FileCheck --input-file=%t %s --match-full-lines

NoQ wrote:
> NoQ wrote:
> > Aaand this test should probably be removed entirely.
> I think this one's not actually done^^
Uh, right, I misunderstood what you meant :D

Repository:
  rC Clang

https://reviews.llvm.org/D53692

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

[PATCH] D53995: [analyzer] Drastically simplify the tblgen files used for checkers

2018-11-09 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ accepted this revision.
NoQ added a comment.

Thx!! Burn it.

> tblgen files look awesome. I get that this isn't a very strong point, but 
> it's a pretty sight compared to a .def file.

I agree that it's aesthetically satisfying, but it is (1) inconvenient to write 
because that's the whole new syntax you need to memorize, i.e. all those <> and 
semicolons and (2) not cooperating when i want to look up checker name by 
source file (have to scan a few pages up to see the alias for the package and 
then jump multiple times to see what it expands to). So i'm still for ditching 
tblgen eventually :) But i definitely don't insist.


Repository:
  rC Clang

https://reviews.llvm.org/D53995



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

[PATCH] D53692: [analyzer] Evaluate all non-checker config options before analysis

2018-11-09 Thread 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, this is really wonderful.




Comment at: test/Analysis/analyzer-config.cpp:1
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 %s -o /dev/null 
-analyzer-checker=core,osx.cocoa,debug.ConfigDumper -analyzer-max-loop 34 > %t 
2>&1
 // RUN: FileCheck --input-file=%t %s --match-full-lines

NoQ wrote:
> Aaand this test should probably be removed entirely.
I think this one's not actually done^^


Repository:
  rC Clang

https://reviews.llvm.org/D53692



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

[PATCH] D52984: [analyzer] Checker reviewer's checklist

2018-11-09 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added a comment.

Thx!! Tiny nits.




Comment at: www/analyzer/checker_dev_manual.html:720
+User facing documentation is important for adoption! Make sure the check 
list updated
+at the homepage of the analyzer. Also ensure that the description is good 
quality in
+Checkers.td.

Szelethus wrote:
> ensure the description is clear even to non-developers
> to non-developers

Good luck explaining use-after-move to my grandma :)

Like, i mean, probably to non-analyzer-developers?



Comment at: www/analyzer/checker_dev_manual.html:722-723
+Checkers.td.
+Introduce BugReporterVisitors to emit additional notes that 
better explain the found 
+bug to the user. There are some existing visitors that might be useful for 
your check,
+e.g. trackNullOrUndefValue. For example, SimpleStreamChecker 
should highlight

...that explain the warning to the user better.



Comment at: www/analyzer/checker_dev_manual.html:728
+checkDeadSymbolscallback to clean the state up.
+The check should handle correctly if a tracked symbol is passed to a 
function that is
+unknown to the analyzer. checkPointerEscape callback could help 
you handle

...should correctly handle...
Or: ...should conservatively assume that the program is correct when...



Comment at: www/analyzer/checker_dev_manual.html:744
+  CallEvent::getOriginExpr is nullable - for example, it returns 
null for an
+automatic destructor of a variable. The same applies to all values 
generated while the
+call was modeled, eg. SymbolConjured::getStmt is nullable.

Not necessarily all values (if the call is inlined, it may actually happen that 
all values are concrete), but definitely some values.



Comment at: www/analyzer/checker_dev_manual.html:758
+e.g. for destructors. You could use NamedDecl::getNameAsString 
for those cases.
+Note that, this method is much slower and should be used sparringly, e.g. 
only when generating reports
+but not during analysis.

I suspect that the comma after 'that' is unnecessary.



Comment at: www/analyzer/checker_dev_manual.html:763-764
+
+  A BugReporterVisitor that matches the AST to decide when to 
emit note instead of
+  examining/diffing the states.
+  In State->getSVal(Region), Region is not necessarily a 
TypedValueRegion

Hmm. Because we're actively bashing the developer in this section, i think we 
should be careful about being clear what's wrong and what's right and why, 
while keeping every bullet self-contained (so that it wasn't taken out of the 
context, i.e. "Bad things: 1. X, 2. ..." shouldn't be accidentally understood 
as "X").

Eg.,
```
Patterns that you should most likely avoid even if they're not technically 
wrong:
* `BugReporterVisitor` should most likely not match the AST of the current 
program point to decide when to emit a note. It is much easier to determine 
that by observing changes in the program state.
```



Comment at: www/analyzer/checker_dev_manual.html:777
+For modeling arithmetic/bitwise/comparison operations, 
SValBuilder should be used.
+  Custom ProgramPointTags are created within the checker. There 
is usually
+no good reason for a checker to chain multiple nodes together, because 
checkers aren't worklists.

`ProgramPointTags` (?)



Comment at: www/analyzer/checker_dev_manual.html:781
+Checkers are encouraged to actively participate in the analysis by sharing
+  its knowledge about the program state with the rest of the analyzer,
+  but they should not be disrupting the analysis unnecessarily:

their knowledge



Comment at: www/analyzer/checker_dev_manual.html:789
+paths needs to be dropped entirely. For example, it is fine to eagerly 
split
+paths while modeling isalpha(x) as long as xi is 
constrained accordingly on
+each path. At the same time, it is not a good idea to split paths over the

`x`



Comment at: www/analyzer/checker_dev_manual.html:809-812
+Is -analyzer-checker=core included in all test RUN: 
lines? It was never supported
+  to run the analyzer with the core checks disabled. It might cause unexpected 
behavior and
+  crashes. You should do all your testing with the core checks enabled.
+

Let's move this bullet to the top? It's kinda lonely here after all those huge 
sections with tons of sub-bullets.


https://reviews.llvm.org/D52984



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

2018-11-09 Thread Bill Wendling via Phabricator via cfe-commits

void added a comment.

This has https://reviews.llvm.org/D54356 integrated into it. 
https://reviews.llvm.org/D54356 should be reviewed and submitted first, even 
though it's out of order.


Repository:
  rC Clang

https://reviews.llvm.org/D54355



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

2018-11-09 Thread Bill Wendling via Phabricator via cfe-commits

void updated this revision to Diff 173445.
void added a comment.
Herald added a subscriber: jfb.

Adding ConstantExpr visitor.


Repository:
  rC Clang

https://reviews.llvm.org/D54355

Files:
  include/clang/AST/Expr.h
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaType.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/builtin-functions.cpp
  test/SemaCXX/compound-literal.cpp

Index: test/SemaCXX/compound-literal.cpp
===
--- test/SemaCXX/compound-literal.cpp
+++ test/SemaCXX/compound-literal.cpp
@@ -36,8 +36,8 @@
 
   POD p = (POD){1, 2};
   // CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::POD'
-  // CHECK: ConstantExpr {{.*}} 'brace_initializers::POD'
-  // CHECK-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK-NEXT: ConstantExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
   // CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -70,14 +70,14 @@
   const int j = 2;
   constexpr int k = 3;
   clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning {{FALSE}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}}
 }
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1283,9 +1283,6 @@
   break;
 
 case Expr::ConstantExprClass:
-  // Handled due to it being a wrapper class.
-  break;
-
 case Stmt::ExprWithCleanupsClass:
   // Handled due to fully linearised CFG.
   break;
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -2233,10 +2233,6 @@
 T = Context.getConstantArrayType(T, ConstVal, ASM, Quals);
   }
 
-  if (ArraySize && !CurContext->isFunctionOrMethod())
-// A file-scoped array must have a constant array size.
-ArraySize = new (Context) ConstantExpr(ArraySize);
-
   // OpenCL v1.2 s6.9.d: variable length arrays are not supported.
   if (getLangOpts().OpenCL && T->isVariableArrayType()) {
 Diag(Loc, diag::err_opencl_vla);
Index: lib/Sema/SemaTemplateDeduction.cpp
===
--- lib/Sema/SemaTemplateDeduction.cpp
+++ lib/Sema/SemaTemplateDeduction.cpp
@@ -178,6 +178,8 @@
   while (true) {
 if (ImplicitCastExpr *IC = dyn_cast(E))
   E = IC->getSubExpr();
+else if (ConstantExpr *CE = dyn_cast(E))
+  E = CE->getSubExpr();
 else if (SubstNonTypeTemplateParmExpr *Subst =
dyn_cast(E))
   E = Subst->getReplacement();
@@ -5225,6 +5227,8 @@
   while (true) {
 if (const ImplicitCastExpr *ICE = dyn_cast(E))
   E = ICE->getSubExpr();
+else if (const ConstantExpr *CE = dyn_cast(E))
+  E = CE->getSubExpr();
 else if (const SubstNonTypeTemplateParmExpr *Subst =
dyn_cast(E))
   E = Subst->getReplacement();
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -5444,7 +5444,7 @@
 
 if (Notes.empty()) {
   // It's a constant expression.
-  return new (S.Context) ConstantExpr(Result.get());
+  return ConstantExpr::Create(S.Context, Result.get());
 }

[PATCH] D54356: Convert CheckICE into a statment visitor

2018-11-09 Thread Bill Wendling via Phabricator via cfe-commits

void created this revision.
void added reviewers: rsmith, shafik.
Herald added a subscriber: jfb.

This cleans up the code somewhat and allows us conditionally to act on
different types of nodes depending on their context. E.g., if we're
checking for an ICE in a constant context.


Repository:
  rC Clang

https://reviews.llvm.org/D54356

Files:
  lib/AST/ExprConstant.cpp

Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -10956,401 +10956,372 @@
 
 static ICEDiag Worst(ICEDiag A, ICEDiag B) { return A.Kind >= B.Kind ? A : B; }
 
-static ICEDiag CheckEvalInICE(const Expr* E, const ASTContext ) {
+static ICEDiag CheckEvalInICE(const Expr *E, const ASTContext ,
+  EvalInfo ) {
   Expr::EvalResult EVResult;
-  if (!E->EvaluateAsRValue(EVResult, Ctx) || EVResult.HasSideEffects ||
+  if (!::EvaluateAsRValue(Info, E, EVResult.Val) || EVResult.HasSideEffects ||
   !EVResult.Val.isInt())
 return ICEDiag(IK_NotICE, E->getBeginLoc());
 
   return NoDiag();
 }
 
+class ICEChecker
+  : public ConstStmtVisitor {
+  const ASTContext 
+  EvalInfo 
+
+  typedef ConstStmtVisitor StmtVisitorTy;
+
+public:
+  ICEChecker(const ASTContext , EvalInfo )
+  : Ctx(Ctx), Info(Info) {}
+
+  ICEDiag VisitStmt(const Stmt *S) {
+return ICEDiag(IK_NotICE, S->getBeginLoc());
+  }
+
+  ICEDiag VisitExpr(const Expr *E);
+  ICEDiag VisitInitListExpr(const InitListExpr *E);
+  ICEDiag VisitSubstNonTypeTemplateParmExpr(
+  const SubstNonTypeTemplateParmExpr *E);
+  ICEDiag VisitParenExpr(const ParenExpr *E);
+  ICEDiag VisitGenericSelectionExpr(const GenericSelectionExpr *E);
+  ICEDiag VisitCallExpr(const CallExpr *E);
+  ICEDiag VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *E);
+  ICEDiag VisitDeclRefExpr(const DeclRefExpr *E);
+  ICEDiag VisitBinaryOperator(const BinaryOperator *E);
+  ICEDiag VisitUnaryOperator(const UnaryOperator *E);
+  ICEDiag VisitOffsetOfExpr(const OffsetOfExpr *E);
+  ICEDiag VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *E);
+  ICEDiag VisitCastExpr(const CastExpr *E);
+  ICEDiag VisitBinaryConditionalOperator(const BinaryConditionalOperator *E);
+  ICEDiag VisitConditionalOperator(const ConditionalOperator *E);
+  ICEDiag VisitCXXDefaultArgExpr(const CXXDefaultArgExpr *E);
+  ICEDiag VisitCXXDefaultInitExpr(const CXXDefaultInitExpr *E);
+  ICEDiag VisitChooseExpr(const ChooseExpr *E);
+};
+
+ICEDiag ICEChecker::VisitExpr(const Expr *E) {
+  switch (E->getStmtClass()) {
+  default:
+return ICEDiag(IK_NotICE, E->getBeginLoc());
+  case Expr::ArrayTypeTraitExprClass:
+  case Expr::CharacterLiteralClass:
+  case Expr::CXXBoolLiteralExprClass:
+  case Expr::CXXNoexceptExprClass:
+  case Expr::CXXScalarValueInitExprClass:
+  case Expr::ExpressionTraitExprClass:
+  case Expr::FixedPointLiteralClass:
+  case Expr::GNUNullExprClass:
+// GCC considers the GNU __null value to be an integral constant expression.
+  case Expr::IntegerLiteralClass:
+  case Expr::ObjCBoolLiteralExprClass:
+  case Expr::SizeOfPackExprClass:
+  case Expr::TypeTraitExprClass:
+return NoDiag();
+  }
+}
+
+ICEDiag ICEChecker::VisitInitListExpr(const InitListExpr *E) {
+  // C++03 [dcl.init]p13: If T is a scalar type, then a declaration of the form
+  // "T x = { a };" is equivalent to "T x = a;".
+  // Unless we're initializing a reference, T is a scalar as it is known to be
+  // of integral or enumeration type.
+  if (E->isRValue())
+if (E->getNumInits() == 1)
+  return StmtVisitorTy::Visit(E->getInit(0));
+  return ICEDiag(IK_NotICE, E->getBeginLoc());
+}
+
+ICEDiag ICEChecker::VisitSubstNonTypeTemplateParmExpr(
+const SubstNonTypeTemplateParmExpr *E) {
+  return StmtVisitorTy::Visit(E->getReplacement());
+}
+
+ICEDiag ICEChecker::VisitParenExpr(const ParenExpr *E) {
+  return StmtVisitorTy::Visit(E->getSubExpr());
+}
+
+ICEDiag ICEChecker::VisitGenericSelectionExpr(const GenericSelectionExpr *E) {
+  return StmtVisitorTy::Visit(E->getResultExpr());
+}
+
+ICEDiag ICEChecker::VisitCallExpr(const CallExpr *E) {
+  // C99 6.6/3 allows function calls within unevaluated subexpressions of
+  // constant expressions, but they can never be ICEs because an ICE cannot
+  // contain an operand of (pointer to) function type.
+  if (E->getBuiltinCallee())
+return CheckEvalInICE(E, Ctx, Info);
+  return ICEDiag(IK_NotICE, E->getBeginLoc());
+}
+
+ICEDiag ICEChecker::VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *E) {
+  return VisitCallExpr(E);
+}
+
+ICEDiag ICEChecker::VisitDeclRefExpr(const DeclRefExpr *E) {
+  if (isa(E->getDecl()))
+return NoDiag();
+
+  const ValueDecl *D = E->getDecl();
+  if (Ctx.getLangOpts().CPlusPlus &&
+  D && IsConstNonVolatile(D->getType())) {
+// Parameter variables are never constants.  Without this check,
+// getAnyInitializer() can find a default argument, which leads
+// to chaos.
+if (isa(D))

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

2018-11-09 Thread Bill Wendling via Phabricator via cfe-commits

void created this revision.
void added a reviewer: rsmith.
Herald added subscribers: cfe-commits, kristina.
Herald added a reviewer: shafik.

A __builtin_constant_p may end up with a constant after inlining. Use
the is.constant intrinsic if it's a variable that's in a context where
it may resolve to a constant, e.g., an argument to a function after
inlining.


Repository:
  rC Clang

https://reviews.llvm.org/D54355

Files:
  include/clang/AST/Expr.h
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaType.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/builtin-functions.cpp
  test/SemaCXX/compound-literal.cpp

Index: test/SemaCXX/compound-literal.cpp
===
--- test/SemaCXX/compound-literal.cpp
+++ test/SemaCXX/compound-literal.cpp
@@ -36,8 +36,8 @@
 
   POD p = (POD){1, 2};
   // CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::POD'
-  // CHECK: ConstantExpr {{.*}} 'brace_initializers::POD'
-  // CHECK-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK-NEXT: ConstantExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
   // CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -70,14 +70,14 @@
   const int j = 2;
   constexpr int k = 3;
   clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning {{FALSE}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}}
 }
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1283,9 +1283,6 @@
   break;
 
 case Expr::ConstantExprClass:
-  // Handled due to it being a wrapper class.
-  break;
-
 case Stmt::ExprWithCleanupsClass:
   // Handled due to fully linearised CFG.
   break;
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -2233,10 +2233,6 @@
 T = Context.getConstantArrayType(T, ConstVal, ASM, Quals);
   }
 
-  if (ArraySize && !CurContext->isFunctionOrMethod())
-// A file-scoped array must have a constant array size.
-ArraySize = new (Context) ConstantExpr(ArraySize);
-
   // OpenCL v1.2 s6.9.d: variable length arrays are not supported.
   if (getLangOpts().OpenCL && T->isVariableArrayType()) {
 Diag(Loc, diag::err_opencl_vla);
Index: lib/Sema/SemaTemplateDeduction.cpp
===
--- lib/Sema/SemaTemplateDeduction.cpp
+++ lib/Sema/SemaTemplateDeduction.cpp
@@ -178,6 +178,8 @@
   while (true) {
 if (ImplicitCastExpr *IC = dyn_cast(E))
   E = IC->getSubExpr();
+else if (ConstantExpr *CE = dyn_cast(E))
+  E = CE->getSubExpr();
 else if (SubstNonTypeTemplateParmExpr *Subst =
dyn_cast(E))
   E = Subst->getReplacement();
@@ -5225,6 +5227,8 @@
   while (true) {
 if (const ImplicitCastExpr *ICE = dyn_cast(E))
   E = ICE->getSubExpr();
+else if (const ConstantExpr *CE = dyn_cast(E))
+  E = CE->getSubExpr();
 else if (const SubstNonTypeTemplateParmExpr *Subst =
dyn_cast(E))
   E = Subst->getReplacement();
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-pp'

2018-11-09 Thread Eugene Zelenko via Phabricator via cfe-commits

Eugene.Zelenko added inline comments.



Comment at: docs/ReleaseNotes.rst:73
+
+  Finds potentially redundant preprocessor usage.
+

preprocessor directives? Same in documentation.



Comment at: docs/clang-tidy/checks/readability-redundant-pp.rst:8
+
+* `#ifdef` .. `#endif` pairs which are nested inside an outer pair with the
+  same condition.

JonasToth wrote:
> This needs more explanation, please add `.. code-block:: c++` sections for 
> the cases that demonstrate the issue.
Please use ``. Same below.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54349



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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-pp'

2018-11-09 Thread Miklos Vajna via Phabricator via cfe-commits

vmiklos added a comment.

In https://reviews.llvm.org/D54349#1293622, @lebedev.ri wrote:

> No one will know for sure what "pp" in "readability-redundant-pp" means.
>  I'd highly recommend to fully spell it out.


Will do.

> Also, i'd like to see some analysis of the false-positives.

Things I considered:

- header guards would easily generate "nested ifndef" false positives, so I 
limited the check to the main file only

- if there are nested `#ifdef FOO` .. `#endif` blocks, but FOO is not defined, 
we fail to detect the redundancy.

- I read that in general checks should be careful around templates and macros, 
but given this deals with the preprocessor, I don't expect issues there.

This implicitly means that I'm not aware of actual false positives of the check 
in its current form.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54349



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

[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

2018-11-09 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In https://reviews.llvm.org/D54344#1293468, @erik.pilkington wrote:

> Have you tried running creduce on the preprocessed source? We should really 
> have a real reproducer for this, otherwise its really hard to tell what the 
> underlying problem is.

I'm going to echo this request -- without a test case, it's hard to know 
whether this actually fixes anything or just moves the issue elsewhere.

https://reviews.llvm.org/D54344

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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-pp'

2018-11-09 Thread Jonas Toth via Phabricator via cfe-commits

JonasToth added a comment.

Hi vmiklos,

thank you for working on this patch!
I added a few comments.




Comment at: clang-tidy/readability/ReadabilityTidyModule.cpp:83
 "readability-misplaced-array-index");
+CheckFactories.registerCheck("readability-redundant-pp");
 CheckFactories.registerCheck(

I think that name is not very descriptive for the user of clang-tidy. `pp` 
should be `preprocessor` or some other constellation that makes it very clear 
its about the preprocessor.



Comment at: clang-tidy/readability/RedundantPpCheck.cpp:28
+
+  void Ifdef(clang::SourceLocation aLoc, const clang::Token ,
+ const clang::MacroDefinition ) override;

you are in namespace `clang`, you can ellide `clang::`



Comment at: clang-tidy/readability/RedundantPpCheck.cpp:37
+  Preprocessor 
+  std::vector IfdefStack;
+  std::vector IfndefStack;

Maybe `SmallVector`? Would be better for performance.



Comment at: clang-tidy/readability/RedundantPpCheck.cpp:48
+
+RedundantPPCallbacks::RedundantPPCallbacks(ClangTidyCheck ,
+   Preprocessor )

I think it would be better to have these methods defined inline, as the 
splitting does not achieve its original goal (declaration in header, definition 
in impl).



Comment at: clang-tidy/readability/RedundantPpCheck.cpp:52
+
+void RedundantPPCallbacks::Ifdef(
+clang::SourceLocation Loc, const clang::Token ,

The two functions are copied, please remove this duplicatoin and refactor it to 
a general parametrized function.



Comment at: docs/ReleaseNotes.rst:70
 
+- New :doc:`readability-redundant-pp
+  ` check.

Please order the checks alphabetically in the release notes, and one empty line 
at the end is enough.



Comment at: docs/clang-tidy/checks/readability-redundant-pp.rst:8
+
+* `#ifdef` .. `#endif` pairs which are nested inside an outer pair with the
+  same condition.

This needs more explanation, please add `.. code-block:: c++` sections for the 
cases that demonstrate the issue.



Comment at: test/clang-tidy/readability-redundant-pp-ifdef.cpp:6
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider 
removing it [readability-redundant-pp]
+#ifdef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here

Please add a test where the redundancy comes from including other headers. If 
the headers are nested this case might still occur, but its not safe to 
diagnose/remove these checks as other include-places might not have the same 
constellation.

`ifdef` and `ifndef` are used for the include-guards and therefore particularly 
necessary to test.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54349



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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-pp'

2018-11-09 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

No one will know for sure what "pp" in "readability-redundant-pp" means.
I'd highly recommend to fully spell it out.

Also, i'd like to see some analysis of the false-positives.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54349



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

[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

2018-11-09 Thread Kristina Brooks via Phabricator via cfe-commits

kristina added a comment.

In https://reviews.llvm.org/D54344#1293468, @erik.pilkington wrote:

> Have you tried running creduce on the preprocessed source? We should really 
> have a real reproducer for this, otherwise its really hard to tell what the 
> underlying problem is.

Unable to build it with current Clang/LLVM headers, attribute is a very recent 
addition as well as general changes. I just get a flurry of errors like this:

  /src/clwn/creduce/clang_delta/InstantiateTemplateTypeParamToInt.cpp:187:19: 
error: no member named 'getLocStart' in 'clang::TemplateTypeParmTypeLoc'
void *Ptr = Loc.getLocStart().getPtrEncoding();

https://reviews.llvm.org/D54344

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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-pp'

2018-11-09 Thread Miklos Vajna via Phabricator via cfe-commits

vmiklos added a comment.

I've used this check originally on LibreOffice (core, online) code and it found 
a handful of not necessary ifdef / ifndef lines. It seemed it's simple and 
generic enough that it would make sense to upstream this.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54349



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

r346556 - Revert "Revert rL346454: Fix a use-after-free introduced by r344915."

2018-11-09 Thread Adrian Prantl via cfe-commits

Author: adrian
Date: Fri Nov  9 13:17:38 2018
New Revision: 346556

URL: http://llvm.org/viewvc/llvm-project?rev=346556=rev
Log:
Revert "Revert rL346454: Fix a use-after-free introduced by r344915."

This un-reverts commit 346454 with a relaxed CHECK for Windows.

Added:
cfe/trunk/test/CodeGen/ubsan-debuglog-return.c
Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=346556=346555=346556=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Fri Nov  9 13:17:38 2018
@@ -3538,7 +3538,7 @@ void CGDebugInfo::EmitLocation(CGBuilder
   // Update our current location
   setLocation(Loc);
 
-  if (CurLoc.isInvalid() || CurLoc.isMacroID())
+  if (CurLoc.isInvalid() || CurLoc.isMacroID() || LexicalBlockStack.empty())
 return;
 
   llvm::MDNode *Scope = LexicalBlockStack.back();

Added: cfe/trunk/test/CodeGen/ubsan-debuglog-return.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ubsan-debuglog-return.c?rev=346556=auto
==
--- cfe/trunk/test/CodeGen/ubsan-debuglog-return.c (added)
+++ cfe/trunk/test/CodeGen/ubsan-debuglog-return.c Fri Nov  9 13:17:38 2018
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -x c -debug-info-kind=line-tables-only -emit-llvm 
-fsanitize=returns-nonnull-attribute -o - %s | FileCheck %s
+// The UBSAN function call in the epilogue needs to have a debug location.
+
+__attribute__((returns_nonnull)) void *allocate() {}
+
+// CHECK: define {{.*}}nonnull i8* @allocate(){{.*}} !dbg
+// CHECK: call void @__ubsan_handle_nonnull_return_v1_abort
+// CHECK-SAME:  !dbg ![[LOC:[0-9]+]]
+// CHECK: ret i8*
+// CHECK-SAME:  !dbg ![[LOC]]


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

[PATCH] D54349: [clang-tidy] new check 'readability-redundant-pp'

2018-11-09 Thread Miklos Vajna via Phabricator via cfe-commits

vmiklos created this revision.
vmiklos added reviewers: JonasToth, alexfh.
Herald added subscribers: cfe-commits, kbarton, xazax.hun, mgorny, nemanjai.

Finds potentially redundant preprocessor usage.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54349

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/readability/RedundantPpCheck.cpp
  clang-tidy/readability/RedundantPpCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-redundant-pp.rst
  test/clang-tidy/readability-redundant-pp-ifdef.cpp
  test/clang-tidy/readability-redundant-pp.cpp

Index: test/clang-tidy/readability-redundant-pp.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-pp.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s readability-redundant-pp %t
+
+// Positive testing.
+#ifndef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-pp]
+#ifndef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here
+void f();
+#endif
+#endif
+
+// Negative testing.
+#ifndef BAR
+void g();
+#endif
+
+#ifndef FOO
+#ifndef BAR
+void h();
+#endif
+#endif
Index: test/clang-tidy/readability-redundant-pp-ifdef.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-pp-ifdef.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s readability-redundant-pp %t -- -- -DFOO
+
+// Positive testing.
+#ifdef FOO
+// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-pp]
+#ifdef FOO
+// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here
+void f();
+#endif
+#endif
+
+// Negative testing.
+#ifdef BAR
+void g();
+#endif
+
+#ifdef FOO
+#ifdef BAR
+void h();
+#endif
+#endif
Index: docs/clang-tidy/checks/readability-redundant-pp.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-redundant-pp.rst
@@ -0,0 +1,11 @@
+.. title:: clang-tidy - readability-redundant-pp
+
+readability-redundant-pp
+=
+
+Finds potentially redundant preprocessor usage. At the moment the following cases are detected:
+
+* `#ifdef` .. `#endif` pairs which are nested inside an outer pair with the
+  same condition.
+
+* Same for `#ifndef` .. `#endif` pairs.
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -240,6 +240,7 @@
readability-misplaced-array-index
readability-named-parameter
readability-non-const-parameter
+   readability-redundant-pp
readability-redundant-control-flow
readability-redundant-declaration
readability-redundant-function-ptr-dereference
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -67,6 +67,12 @@
 Improvements to clang-tidy
 --
 
+- New :doc:`readability-redundant-pp
+  ` check.
+
+  Finds potentially redundant preprocessor usage.
+
+
 - New :doc:`abseil-duration-division
   ` check.
 
Index: clang-tidy/readability/RedundantPpCheck.h
===
--- /dev/null
+++ clang-tidy/readability/RedundantPpCheck.h
@@ -0,0 +1,34 @@
+//===--- RedundantPpCheck.h - clang-tidy ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTPPCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTPPCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// This check flags redundant preprocessor usage.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-pp.html
+class RedundantPpCheck : public ClangTidyCheck {
+public:
+  RedundantPpCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerPPCallbacks(CompilerInstance ) override;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTPPCHECK_H
Index: clang-tidy/readability/RedundantPpCheck.cpp
===
--- /dev/null
+++ clang-tidy/readability/RedundantPpCheck.cpp
@@ -0,0 +1,94 @@
+//===--- RedundantPpCheck.cpp - clang-tidy ---===//
+//
+//

[PATCH] D51575: [clang-tidy/checks] Implement a clang-tidy check to verify Google Objective-C function naming conventions 

2018-11-09 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang-tidy/google/FunctionNamingCheck.cpp:55
+  // to use.
+  if (Decl->getStorageClass() != SC_Static) {
+return FixItHint();

Elide braces.



Comment at: clang-tidy/google/FunctionNamingCheck.cpp:59-60
+
+  auto Name = Decl->getName();
+  auto NewName = Decl->getName().str();
+

Do not use `auto` as the type is not mentioned in the initialization.



Comment at: clang-tidy/google/FunctionNamingCheck.cpp:66
+// Exit the loop once we reach the end of the string.
+if (Index >= NewName.size()) break;
+

Please make this the while loop condition rather than an explicit break.



Comment at: clang-tidy/google/FunctionNamingCheck.cpp:86
+  // Generate a fixit hint if the new name is different.
+  if (NewName != Name) {
+return FixItHint::CreateReplacement(

Elide braces.



Comment at: clang-tidy/google/FunctionNamingCheck.cpp:98
+  // This check should only be applied to Objective-C sources.
+  if (!getLangOpts().ObjC1 && !getLangOpts().ObjC2) {
+return;

Elide braces. Also, didn't we get rid of the distinction between ObjcC1 and 
ObjC2?



Comment at: clang-tidy/google/FunctionNamingCheck.cpp:116
+  const auto *MatchedDecl = Result.Nodes.getNodeAs("function");
+  assert(MatchedDecl != nullptr);
+

No need for this assertion. The `check()` function cannot be called without 
this being nonnull.



Comment at: clang-tidy/google/FunctionNamingCheck.cpp:119
+  // Only functions other than main should be matched.
+  assert(!MatchedDecl->isMain());
+

I don't think this assert adds value.



Comment at: clang-tidy/google/FunctionNamingCheck.cpp:122-124
+   "function name '%0' not using function naming conventions described by "
+   "Google Objective-C style guide")
+  << MatchedDecl->getName() << generateFixItHint(MatchedDecl);

You can drop the explicit quotes around %0 and instead pass in `MatchedDecl` 
rather than `MatchedDecl->getName()`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575



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

[PATCH] D54281: [clang-tidy] fix PR39583 - ignoring ParenCast for string-literals in pro-bounds-array-to-pointer-decay

2018-11-09 Thread Jonas Toth via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL346555: [clang-tidy] fix PR39583 - ignoring ParenCast for 
string-literals in pro-bounds… (authored by JonasToth, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D54281

Files:
  
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
  
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp


Index: 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
===
--- 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
+++ 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
@@ -58,10 +58,11 @@
   // 2) inside a range-for over an array
   // 3) if it converts a string literal to a pointer
   Finder->addMatcher(
-  implicitCastExpr(unless(hasParent(arraySubscriptExpr())),
-   unless(hasParentIgnoringImpCasts(explicitCastExpr())),
-   unless(isInsideOfRangeBeginEndStmt()),
-   unless(hasSourceExpression(stringLiteral(
+  implicitCastExpr(
+  unless(hasParent(arraySubscriptExpr())),
+  unless(hasParentIgnoringImpCasts(explicitCastExpr())),
+  unless(isInsideOfRangeBeginEndStmt()),
+  unless(hasSourceExpression(ignoringParens(stringLiteral()
   .bind("cast"),
   this);
 }
Index: 
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
===
--- 
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
+++ 
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
@@ -30,6 +30,7 @@
   arrayviewfun(av); // OK
 
   int i = a[0];  // OK
+  int j = a[(1 + 2)];// OK
   pointerfun([0]); // OK
 
   for (auto  : a) // OK, iteration internally decays array to pointer
@@ -39,6 +40,9 @@
 const char *g() {
   return "clang"; // OK, decay string literal to pointer
 }
+const char *g2() {
+return ("clang"); // OK, ParenExpr hides the literal-pointer decay
+}
 
 void f2(void *const *);
 void bug25362() {


Index: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
@@ -58,10 +58,11 @@
   // 2) inside a range-for over an array
   // 3) if it converts a string literal to a pointer
   Finder->addMatcher(
-  implicitCastExpr(unless(hasParent(arraySubscriptExpr())),
-   unless(hasParentIgnoringImpCasts(explicitCastExpr())),
-   unless(isInsideOfRangeBeginEndStmt()),
-   unless(hasSourceExpression(stringLiteral(
+  implicitCastExpr(
+  unless(hasParent(arraySubscriptExpr())),
+  unless(hasParentIgnoringImpCasts(explicitCastExpr())),
+  unless(isInsideOfRangeBeginEndStmt()),
+  unless(hasSourceExpression(ignoringParens(stringLiteral()
   .bind("cast"),
   this);
 }
Index: clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
@@ -30,6 +30,7 @@
   arrayviewfun(av); // OK
 
   int i = a[0];  // OK
+  int j = a[(1 + 2)];// OK
   pointerfun([0]); // OK
 
   for (auto  : a) // OK, iteration internally decays array to pointer
@@ -39,6 +40,9 @@
 const char *g() {
   return "clang"; // OK, decay string literal to pointer
 }
+const char *g2() {
+return ("clang"); // OK, ParenExpr hides the literal-pointer decay
+}
 
 void f2(void *const *);
 void bug25362() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54281: [clang-tidy] fix PR39583 - ignoring ParenCast for string-literals in pro-bounds-array-to-pointer-decay

2018-11-09 Thread Jonas Toth via Phabricator via cfe-commits

JonasToth updated this revision to Diff 173425.
JonasToth added a comment.

- fix comment


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54281

Files:
  clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
  test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp


Index: test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
===
--- test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
+++ test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
@@ -30,6 +30,7 @@
   arrayviewfun(av); // OK
 
   int i = a[0];  // OK
+  int j = a[(1 + 2)];// OK
   pointerfun([0]); // OK
 
   for (auto  : a) // OK, iteration internally decays array to pointer
@@ -39,6 +40,9 @@
 const char *g() {
   return "clang"; // OK, decay string literal to pointer
 }
+const char *g2() {
+return ("clang"); // OK, ParenExpr hides the literal-pointer decay
+}
 
 void f2(void *const *);
 void bug25362() {
Index: clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
===
--- clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
+++ clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
@@ -58,10 +58,11 @@
   // 2) inside a range-for over an array
   // 3) if it converts a string literal to a pointer
   Finder->addMatcher(
-  implicitCastExpr(unless(hasParent(arraySubscriptExpr())),
-   unless(hasParentIgnoringImpCasts(explicitCastExpr())),
-   unless(isInsideOfRangeBeginEndStmt()),
-   unless(hasSourceExpression(stringLiteral(
+  implicitCastExpr(
+  unless(hasParent(arraySubscriptExpr())),
+  unless(hasParentIgnoringImpCasts(explicitCastExpr())),
+  unless(isInsideOfRangeBeginEndStmt()),
+  unless(hasSourceExpression(ignoringParens(stringLiteral()
   .bind("cast"),
   this);
 }


Index: test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
===
--- test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
+++ test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
@@ -30,6 +30,7 @@
   arrayviewfun(av); // OK
 
   int i = a[0];  // OK
+  int j = a[(1 + 2)];// OK
   pointerfun([0]); // OK
 
   for (auto  : a) // OK, iteration internally decays array to pointer
@@ -39,6 +40,9 @@
 const char *g() {
   return "clang"; // OK, decay string literal to pointer
 }
+const char *g2() {
+return ("clang"); // OK, ParenExpr hides the literal-pointer decay
+}
 
 void f2(void *const *);
 void bug25362() {
Index: clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
===
--- clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
+++ clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
@@ -58,10 +58,11 @@
   // 2) inside a range-for over an array
   // 3) if it converts a string literal to a pointer
   Finder->addMatcher(
-  implicitCastExpr(unless(hasParent(arraySubscriptExpr())),
-   unless(hasParentIgnoringImpCasts(explicitCastExpr())),
-   unless(isInsideOfRangeBeginEndStmt()),
-   unless(hasSourceExpression(stringLiteral(
+  implicitCastExpr(
+  unless(hasParent(arraySubscriptExpr())),
+  unless(hasParentIgnoringImpCasts(explicitCastExpr())),
+  unless(isInsideOfRangeBeginEndStmt()),
+  unless(hasSourceExpression(ignoringParens(stringLiteral()
   .bind("cast"),
   this);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r346555 - [clang-tidy] fix PR39583 - ignoring ParenCast for string-literals in pro-bounds-array-to-pointer-decay

2018-11-09 Thread Jonas Toth via cfe-commits

Author: jonastoth
Date: Fri Nov  9 12:57:28 2018
New Revision: 346555

URL: http://llvm.org/viewvc/llvm-project?rev=346555=rev
Log:
[clang-tidy] fix PR39583 - ignoring ParenCast for string-literals in 
pro-bounds-array-to-pointer-decay

Summary:
The fix to the issue that `const char* p = ("foo")` is diagnosed as decay
is to ignored the ParenCast.
Resolves PR39583

Reviewers: aaron.ballman, alexfh, hokein

Reviewed By: aaron.ballman

Subscribers: nemanjai, xazax.hun, kbarton, cfe-commits

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

Modified:

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp

clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp?rev=346555=346554=346555=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
 (original)
+++ 
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
 Fri Nov  9 12:57:28 2018
@@ -58,10 +58,11 @@ void ProBoundsArrayToPointerDecayCheck::
   // 2) inside a range-for over an array
   // 3) if it converts a string literal to a pointer
   Finder->addMatcher(
-  implicitCastExpr(unless(hasParent(arraySubscriptExpr())),
-   unless(hasParentIgnoringImpCasts(explicitCastExpr())),
-   unless(isInsideOfRangeBeginEndStmt()),
-   unless(hasSourceExpression(stringLiteral(
+  implicitCastExpr(
+  unless(hasParent(arraySubscriptExpr())),
+  unless(hasParentIgnoringImpCasts(explicitCastExpr())),
+  unless(isInsideOfRangeBeginEndStmt()),
+  unless(hasSourceExpression(ignoringParens(stringLiteral()
   .bind("cast"),
   this);
 }

Modified: 
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp?rev=346555=346554=346555=diff
==
--- 
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
 (original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
 Fri Nov  9 12:57:28 2018
@@ -30,6 +30,7 @@ void f() {
   arrayviewfun(av); // OK
 
   int i = a[0];  // OK
+  int j = a[(1 + 2)];// OK
   pointerfun([0]); // OK
 
   for (auto  : a) // OK, iteration internally decays array to pointer
@@ -39,6 +40,9 @@ void f() {
 const char *g() {
   return "clang"; // OK, decay string literal to pointer
 }
+const char *g2() {
+return ("clang"); // OK, ParenExpr hides the literal-pointer decay
+}
 
 void f2(void *const *);
 void bug25362() {


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

[PATCH] D54307: [ASTMatchers] overload ignoringParens for Expr

2018-11-09 Thread Jonas Toth via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rC346554: [ASTMatchers] overload ignoringParens for Expr 
(authored by JonasToth, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D54307?vs=173422=173424#toc

Repository:
  rC Clang

https://reviews.llvm.org/D54307

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/ASTMatchersNodeTest.cpp


Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -106,6 +106,7 @@
   REGISTER_OVERLOADED_2(callee);
   REGISTER_OVERLOADED_2(hasPrefix);
   REGISTER_OVERLOADED_2(hasType);
+  REGISTER_OVERLOADED_2(ignoringParens);
   REGISTER_OVERLOADED_2(isDerivedFrom);
   REGISTER_OVERLOADED_2(isSameOrDerivedFrom);
   REGISTER_OVERLOADED_2(loc);
@@ -318,7 +319,6 @@
   REGISTER_MATCHER(ignoringImplicit);
   REGISTER_MATCHER(ignoringParenCasts);
   REGISTER_MATCHER(ignoringParenImpCasts);
-  REGISTER_MATCHER(ignoringParens);
   REGISTER_MATCHER(imaginaryLiteral);
   REGISTER_MATCHER(implicitCastExpr);
   REGISTER_MATCHER(implicitValueInitExpr);
Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1147,6 +1147,14 @@
  parenExpr()));
 }
 
+TEST(ParenExpression, IgnoringParens) {
+  EXPECT_FALSE(matches("const char* str = (\"my-string\");",
+   
implicitCastExpr(hasSourceExpression(stringLiteral();
+  EXPECT_TRUE(matches(
+  "const char* str = (\"my-string\");",
+  implicitCastExpr(hasSourceExpression(ignoringParens(stringLiteral());
+}
+
 TEST(TypeMatching, MatchesTypes) {
   EXPECT_TRUE(matches("struct S {};", qualType().bind("loc")));
 }
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -5552,6 +5552,17 @@
 
 
 
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1Expr.html;>ExprignoringParensMatcherhttps://clang.llvm.org/doxygen/classclang_1_1Expr.html;>Expr 
InnerMatcher
+Overload 
ignoringParens for Expr.
+
+Given
+  const char* str = ("my-string");
+The matcher
+  implicitCastExpr(hasSourceExpression(ignoringParens(stringLiteral(
+would match the implicit cast resulting from the assignment.
+
+
+
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1FieldDecl.html;>FieldDeclhasInClassInitializerMatcherhttps://clang.llvm.org/doxygen/classclang_1_1Expr.html;>Expr 
InnerMatcher
 Matches 
non-static data members that have an in-class initializer.
 
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -811,11 +811,28 @@
 ///   varDecl(hasType(pointerType(pointee(ignoringParens(functionType())
 /// \endcode
 /// would match the declaration for fp.
-AST_MATCHER_P(QualType, ignoringParens,
-  internal::Matcher, InnerMatcher) {
+AST_MATCHER_P_OVERLOAD(QualType, ignoringParens, internal::Matcher,
+   InnerMatcher, 0) {
   return InnerMatcher.matches(Node.IgnoreParens(), Finder, Builder);
 }
 
+/// Overload \c ignoringParens for \c Expr.
+///
+/// Given
+/// \code
+///   const char* str = ("my-string");
+/// \endcode
+/// The matcher
+/// \code
+///   implicitCastExpr(hasSourceExpression(ignoringParens(stringLiteral(
+/// \endcode
+/// would match the implicit cast resulting from the assignment.
+AST_MATCHER_P_OVERLOAD(Expr, ignoringParens, internal::Matcher,
+   InnerMatcher, 1) {
+  const Expr *E = Node.IgnoreParens();
+  return InnerMatcher.matches(*E, Finder, Builder);
+}
+
 /// Matches expressions that are instantiation-dependent even if it is
 /// neither type- nor value-dependent.
 ///


Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -106,6 +106,7 @@
   REGISTER_OVERLOADED_2(callee);
   REGISTER_OVERLOADED_2(hasPrefix);
   REGISTER_OVERLOADED_2(hasType);
+  REGISTER_OVERLOADED_2(ignoringParens);
   REGISTER_OVERLOADED_2(isDerivedFrom);
   REGISTER_OVERLOADED_2(isSameOrDerivedFrom);
   REGISTER_OVERLOADED_2(loc);
@@ -318,7 +319,6 @@
   REGISTER_MATCHER(ignoringImplicit);
   REGISTER_MATCHER(ignoringParenCasts);
   REGISTER_MATCHER(ignoringParenImpCasts);
-  REGISTER_MATCHER(ignoringParens);
   REGISTER_MATCHER(imaginaryLiteral);
   REGISTER_MATCHER(implicitCastExpr);
   REGISTER_MATCHER(implicitValueInitExpr);
Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp

[PATCH] D54281: [clang-tidy] fix PR39583 - ignoring ParenCast for string-literals in pro-bounds-array-to-pointer-decay

2018-11-09 Thread Jonas Toth via Phabricator via cfe-commits

JonasToth updated this revision to Diff 173423.
JonasToth added a comment.

- use ignoringParens instead


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54281

Files:
  clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
  test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp


Index: test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
===
--- test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
+++ test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
@@ -30,6 +30,7 @@
   arrayviewfun(av); // OK
 
   int i = a[0];  // OK
+  int j = a[(1 + 2)];// OK
   pointerfun([0]); // OK
 
   for (auto  : a) // OK, iteration internally decays array to pointer
@@ -39,6 +40,9 @@
 const char *g() {
   return "clang"; // OK, decay string literal to pointer
 }
+const char *g2() {
+return ("clang"); // OK, ParenCast hides the literal-pointer decay
+}
 
 void f2(void *const *);
 void bug25362() {
Index: clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
===
--- clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
+++ clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
@@ -58,10 +58,11 @@
   // 2) inside a range-for over an array
   // 3) if it converts a string literal to a pointer
   Finder->addMatcher(
-  implicitCastExpr(unless(hasParent(arraySubscriptExpr())),
-   unless(hasParentIgnoringImpCasts(explicitCastExpr())),
-   unless(isInsideOfRangeBeginEndStmt()),
-   unless(hasSourceExpression(stringLiteral(
+  implicitCastExpr(
+  unless(hasParent(arraySubscriptExpr())),
+  unless(hasParentIgnoringImpCasts(explicitCastExpr())),
+  unless(isInsideOfRangeBeginEndStmt()),
+  unless(hasSourceExpression(ignoringParens(stringLiteral()
   .bind("cast"),
   this);
 }


Index: test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
===
--- test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
+++ test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp
@@ -30,6 +30,7 @@
   arrayviewfun(av); // OK
 
   int i = a[0];  // OK
+  int j = a[(1 + 2)];// OK
   pointerfun([0]); // OK
 
   for (auto  : a) // OK, iteration internally decays array to pointer
@@ -39,6 +40,9 @@
 const char *g() {
   return "clang"; // OK, decay string literal to pointer
 }
+const char *g2() {
+return ("clang"); // OK, ParenCast hides the literal-pointer decay
+}
 
 void f2(void *const *);
 void bug25362() {
Index: clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
===
--- clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
+++ clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
@@ -58,10 +58,11 @@
   // 2) inside a range-for over an array
   // 3) if it converts a string literal to a pointer
   Finder->addMatcher(
-  implicitCastExpr(unless(hasParent(arraySubscriptExpr())),
-   unless(hasParentIgnoringImpCasts(explicitCastExpr())),
-   unless(isInsideOfRangeBeginEndStmt()),
-   unless(hasSourceExpression(stringLiteral(
+  implicitCastExpr(
+  unless(hasParent(arraySubscriptExpr())),
+  unless(hasParentIgnoringImpCasts(explicitCastExpr())),
+  unless(isInsideOfRangeBeginEndStmt()),
+  unless(hasSourceExpression(ignoringParens(stringLiteral()
   .bind("cast"),
   this);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r346554 - [ASTMatchers] overload ignoringParens for Expr

2018-11-09 Thread Jonas Toth via cfe-commits

Author: jonastoth
Date: Fri Nov  9 12:54:06 2018
New Revision: 346554

URL: http://llvm.org/viewvc/llvm-project?rev=346554=rev
Log:
[ASTMatchers] overload ignoringParens for Expr

Summary: This patch allows fixing PR39583.

Reviewers: aaron.ballman, sbenza, klimek

Reviewed By: aaron.ballman

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/docs/LibASTMatchersReference.html
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
cfe/trunk/unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Modified: cfe/trunk/docs/LibASTMatchersReference.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=346554=346553=346554=diff
==
--- cfe/trunk/docs/LibASTMatchersReference.html (original)
+++ cfe/trunk/docs/LibASTMatchersReference.html Fri Nov  9 12:54:06 2018
@@ -5552,6 +5552,17 @@ would only match the declaration for a.
 
 
 
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1Expr.html;>ExprignoringParensMatcherhttps://clang.llvm.org/doxygen/classclang_1_1Expr.html;>Expr 
InnerMatcher
+Overload 
ignoringParens for Expr.
+
+Given
+  const char* str = ("my-string");
+The matcher
+  implicitCastExpr(hasSourceExpression(ignoringParens(stringLiteral(
+would match the implicit cast resulting from the assignment.
+
+
+
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1FieldDecl.html;>FieldDeclhasInClassInitializerMatcherhttps://clang.llvm.org/doxygen/classclang_1_1Expr.html;>Expr 
InnerMatcher
 Matches 
non-static data members that have an in-class initializer.
 

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=346554=346553=346554=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Fri Nov  9 12:54:06 2018
@@ -811,11 +811,28 @@ AST_MATCHER_P(Expr, ignoringParenImpCast
 ///   varDecl(hasType(pointerType(pointee(ignoringParens(functionType())
 /// \endcode
 /// would match the declaration for fp.
-AST_MATCHER_P(QualType, ignoringParens,
-  internal::Matcher, InnerMatcher) {
+AST_MATCHER_P_OVERLOAD(QualType, ignoringParens, internal::Matcher,
+   InnerMatcher, 0) {
   return InnerMatcher.matches(Node.IgnoreParens(), Finder, Builder);
 }
 
+/// Overload \c ignoringParens for \c Expr.
+///
+/// Given
+/// \code
+///   const char* str = ("my-string");
+/// \endcode
+/// The matcher
+/// \code
+///   implicitCastExpr(hasSourceExpression(ignoringParens(stringLiteral(
+/// \endcode
+/// would match the implicit cast resulting from the assignment.
+AST_MATCHER_P_OVERLOAD(Expr, ignoringParens, internal::Matcher,
+   InnerMatcher, 1) {
+  const Expr *E = Node.IgnoreParens();
+  return InnerMatcher.matches(*E, Finder, Builder);
+}
+
 /// Matches expressions that are instantiation-dependent even if it is
 /// neither type- nor value-dependent.
 ///

Modified: cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp?rev=346554=346553=346554=diff
==
--- cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp Fri Nov  9 12:54:06 2018
@@ -106,6 +106,7 @@ RegistryMaps::RegistryMaps() {
   REGISTER_OVERLOADED_2(callee);
   REGISTER_OVERLOADED_2(hasPrefix);
   REGISTER_OVERLOADED_2(hasType);
+  REGISTER_OVERLOADED_2(ignoringParens);
   REGISTER_OVERLOADED_2(isDerivedFrom);
   REGISTER_OVERLOADED_2(isSameOrDerivedFrom);
   REGISTER_OVERLOADED_2(loc);
@@ -318,7 +319,6 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(ignoringImplicit);
   REGISTER_MATCHER(ignoringParenCasts);
   REGISTER_MATCHER(ignoringParenImpCasts);
-  REGISTER_MATCHER(ignoringParens);
   REGISTER_MATCHER(imaginaryLiteral);
   REGISTER_MATCHER(implicitCastExpr);
   REGISTER_MATCHER(implicitValueInitExpr);

Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersNodeTest.cpp?rev=346554=346553=346554=diff
==
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersNodeTest.cpp (original)
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersNodeTest.cpp Fri Nov  9 12:54:06 
2018
@@ -1147,6 +1147,14 @@ TEST(ParenExpression, SimpleCases) {
  parenExpr()));
 }
 
+TEST(ParenExpression, IgnoringParens) {
+  EXPECT_FALSE(matches("const char* str = (\"my-string\");",
+

[PATCH] D54307: [ASTMatchers] overload ignoringParens for Expr

2018-11-09 Thread Jonas Toth via Phabricator via cfe-commits

JonasToth updated this revision to Diff 173422.
JonasToth added a comment.

- add unit test


Repository:
  rC Clang

https://reviews.llvm.org/D54307

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/ASTMatchersNodeTest.cpp


Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1147,6 +1147,14 @@
  parenExpr()));
 }
 
+TEST(ParenExpression, IgnoringParens) {
+  EXPECT_FALSE(matches("const char* str = (\"my-string\");",
+   
implicitCastExpr(hasSourceExpression(stringLiteral();
+  EXPECT_TRUE(matches(
+  "const char* str = (\"my-string\");",
+  implicitCastExpr(hasSourceExpression(ignoringParens(stringLiteral());
+}
+
 TEST(TypeMatching, MatchesTypes) {
   EXPECT_TRUE(matches("struct S {};", qualType().bind("loc")));
 }
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -106,6 +106,7 @@
   REGISTER_OVERLOADED_2(callee);
   REGISTER_OVERLOADED_2(hasPrefix);
   REGISTER_OVERLOADED_2(hasType);
+  REGISTER_OVERLOADED_2(ignoringParens);
   REGISTER_OVERLOADED_2(isDerivedFrom);
   REGISTER_OVERLOADED_2(isSameOrDerivedFrom);
   REGISTER_OVERLOADED_2(loc);
@@ -318,7 +319,6 @@
   REGISTER_MATCHER(ignoringImplicit);
   REGISTER_MATCHER(ignoringParenCasts);
   REGISTER_MATCHER(ignoringParenImpCasts);
-  REGISTER_MATCHER(ignoringParens);
   REGISTER_MATCHER(imaginaryLiteral);
   REGISTER_MATCHER(implicitCastExpr);
   REGISTER_MATCHER(implicitValueInitExpr);
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -811,11 +811,28 @@
 ///   varDecl(hasType(pointerType(pointee(ignoringParens(functionType())
 /// \endcode
 /// would match the declaration for fp.
-AST_MATCHER_P(QualType, ignoringParens,
-  internal::Matcher, InnerMatcher) {
+AST_MATCHER_P_OVERLOAD(QualType, ignoringParens, internal::Matcher,
+   InnerMatcher, 0) {
   return InnerMatcher.matches(Node.IgnoreParens(), Finder, Builder);
 }
 
+/// Overload \c ignoringParens for \c Expr.
+///
+/// Given
+/// \code
+///   const char* str = ("my-string");
+/// \endcode
+/// The matcher
+/// \code
+///   implicitCastExpr(hasSourceExpression(ignoringParens(stringLiteral(
+/// \endcode
+/// would match the implicit cast resulting from the assignment.
+AST_MATCHER_P_OVERLOAD(Expr, ignoringParens, internal::Matcher,
+   InnerMatcher, 1) {
+  const Expr *E = Node.IgnoreParens();
+  return InnerMatcher.matches(*E, Finder, Builder);
+}
+
 /// Matches expressions that are instantiation-dependent even if it is
 /// neither type- nor value-dependent.
 ///
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -5552,6 +5552,17 @@
 
 
 
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1Expr.html;>ExprignoringParensMatcherhttps://clang.llvm.org/doxygen/classclang_1_1Expr.html;>Expr 
InnerMatcher
+Overload 
ignoringParens for Expr.
+
+Given
+  const char* str = ("my-string");
+The matcher
+  implicitCastExpr(hasSourceExpression(ignoringParens(stringLiteral(
+would match the implicit cast resulting from the assignment.
+
+
+
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1FieldDecl.html;>FieldDeclhasInClassInitializerMatcherhttps://clang.llvm.org/doxygen/classclang_1_1Expr.html;>Expr 
InnerMatcher
 Matches 
non-static data members that have an in-class initializer.
 


Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1147,6 +1147,14 @@
  parenExpr()));
 }
 
+TEST(ParenExpression, IgnoringParens) {
+  EXPECT_FALSE(matches("const char* str = (\"my-string\");",
+   implicitCastExpr(hasSourceExpression(stringLiteral();
+  EXPECT_TRUE(matches(
+  "const char* str = (\"my-string\");",
+  implicitCastExpr(hasSourceExpression(ignoringParens(stringLiteral());
+}
+
 TEST(TypeMatching, MatchesTypes) {
   EXPECT_TRUE(matches("struct S {};", qualType().bind("loc")));
 }
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -106,6 +106,7 @@
   REGISTER_OVERLOADED_2(callee);

Re: r346491 - [clang-cl] Add warning for /Zc:dllexportInlines- when the flag is used with /fallback

2018-11-09 Thread Nico Weber via cfe-commits

Ah cool. But the diagnostic is "option /dllexportInlines- is ignored when
/fallback happens" – shouldn't it be ignored regardless of if fallback
happens? Does that happen and the warning text is wrong?

On Fri, Nov 9, 2018 at 11:20 AM Hans Wennborg  wrote:

> On Fri, Nov 9, 2018 at 4:53 PM, Nico Weber  wrote:
> > This only prints the warning when /fallback actually happens, right?
>
> No, it prints it when the fallback job is created, not when (or if) it
> runs. I.e. it prints if the /fallback flag is used, regardless of
> whether it actually falls back or not. This is reflected by the test
> which uses -###, i.e. nothing is getting run.
>
> So I think we're good :-)
>
> > I don't
> > think that's good enough: If we build a few TUs with /dllexportInlines-
> and
> > don't fall back, those .obj files are not abi compatible with the
> cl-built
> > ones. (Consider all dllexport TUs of a header being built with clang-cl
> but
> > all dllimport versions of the same header being built by cl – I think
> this
> > will cause link errors). SO I think we should error out if
> > /dllexportIlnlines- /fallback is on the same line, even if the fallback
> > doesn't actually happen.
> >
> > On Fri, Nov 9, 2018 at 8:28 AM Takuto Ikuta via cfe-commits
> >  wrote:
> >>
> >> Author: tikuta
> >> Date: Fri Nov  9 05:25:45 2018
> >> New Revision: 346491
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=346491=rev
> >> Log:
> >> [clang-cl] Add warning for /Zc:dllexportInlines- when the flag is used
> >> with /fallback
> >>
> >> Summary:
> >> This is followup of
> >> https://reviews.llvm.org/D51340
> >>
> >> Reviewers: hans, thakis
> >>
> >> Reviewed By: hans
> >>
> >> Subscribers: cfe-commits, llvm-commits
> >>
> >> Differential Revision: https://reviews.llvm.org/D54298
> >>
> >> Modified:
> >> cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
> >> cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
> >> cfe/trunk/test/Driver/cl-options.c
> >>
> >> Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=346491=346490=346491=diff
> >>
> >>
> ==
> >> --- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
> >> +++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Fri Nov  9
> >> 05:25:45 2018
> >> @@ -161,6 +161,10 @@ def warn_drv_yc_multiple_inputs_clang_cl
> >>"support for '/Yc' with more than one source file not implemented
> yet;
> >> flag ignored">,
> >>InGroup;
> >>
> >> +def warn_drv_non_fallback_argument_clang_cl : Warning<
> >> +  "option '%0' is ignored when /fallback happens">,
> >> +  InGroup;
> >> +
> >>  def err_drv_invalid_value : Error<"invalid value '%1' in '%0'">;
> >>  def err_drv_invalid_int_value : Error<"invalid integral value '%1' in
> >> '%0'">;
> >>  def err_drv_invalid_remap_file : Error<
> >>
> >> Modified: cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/MSVC.cpp?rev=346491=346490=346491=diff
> >>
> >>
> ==
> >> --- cfe/trunk/lib/Driver/ToolChains/MSVC.cpp (original)
> >> +++ cfe/trunk/lib/Driver/ToolChains/MSVC.cpp Fri Nov  9 05:25:45 2018
> >> @@ -669,6 +669,12 @@ std::unique_ptr visualstudio::C
> >>// them too.
> >>Args.AddAllArgs(CmdArgs, options::OPT_UNKNOWN);
> >>
> >> +  // Warning for ignored flag.
> >> +  if (const Arg *dllexportInlines =
> >> +  Args.getLastArg(options::OPT__SLASH_Zc_dllexportInlines_))
> >> +
> >> C.getDriver().Diag(clang::diag::warn_drv_non_fallback_argument_clang_cl)
> >> +  << dllexportInlines->getAsString(Args);
> >> +
> >>// Input filename.
> >>assert(Inputs.size() == 1);
> >>const InputInfo  = Inputs[0];
> >>
> >> Modified: cfe/trunk/test/Driver/cl-options.c
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=346491=346490=346491=diff
> >>
> >>
> ==
> >> --- cfe/trunk/test/Driver/cl-options.c (original)
> >> +++ cfe/trunk/test/Driver/cl-options.c Fri Nov  9 05:25:45 2018
> >> @@ -494,6 +494,8 @@
> >>  // NoDllExportInlines: "-fno-dllexport-inlines"
> >>  // RUN: %clang_cl /Zc:dllexportInlines /c -### -- %s 2>&1 | FileCheck
> >> -check-prefix=DllExportInlines %s
> >>  // DllExportInlines-NOT: "-fno-dllexport-inlines"
> >> +// RUN: %clang_cl /fallback /Zc:dllexportInlines- /c -### -- %s 2>&1 |
> >> FileCheck -check-prefix=DllExportInlinesFallback %s
> >> +// DllExportInlinesFallback: warning: option '/Zc:dllexportInlines-' is
> >> ignored when /fallback happens [-Woption-ignored]
> >>
> >>  // RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi
> %s
> >>  // Zi: "-gcodeview"
> >>
> >>
> >> ___

Re: [clang-tools-extra] r346461 - Ignore implicit things like ConstantExpr.

2018-11-09 Thread Bill Wendling via cfe-commits

This fix was submitted because a test was failing without it. :-)

On Thu, Nov 8, 2018 at 10:08 PM Roman Lebedev  wrote:

> Test?
>
> On Fri, Nov 9, 2018 at 4:34 AM Bill Wendling via cfe-commits
>  wrote:
> >
> > Author: void
> > Date: Thu Nov  8 17:32:30 2018
> > New Revision: 346461
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=346461=rev
> > Log:
> > Ignore implicit things like ConstantExpr.
> >
> > Modified:
> >
>  
> clang-tools-extra/trunk/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
> >
> > Modified:
> clang-tools-extra/trunk/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp?rev=346461=346460=346461=diff
> >
> ==
> > ---
> clang-tools-extra/trunk/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
> (original)
> > +++
> clang-tools-extra/trunk/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
> Thu Nov  8 17:32:30 2018
> > @@ -58,7 +58,8 @@ void NoexceptMoveConstructorCheck::check
> >  // where expr evaluates to false.
> >  if (ProtoType->canThrow() == CT_Can) {
> >Expr *E = ProtoType->getNoexceptExpr();
> > -  if (!isa(ProtoType->getNoexceptExpr())) {
> > +  E = E->IgnoreImplicit();
> > +  if (!isa(E)) {
> >  diag(E->getExprLoc(),
> >   "noexcept specifier on the move %0 evaluates to 'false'")
> >  << MethodType;
> >
> >
> > ___
> > 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] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

2018-11-09 Thread Erik Pilkington via Phabricator via cfe-commits

erik.pilkington added a reviewer: erik.pilkington.
erik.pilkington added a comment.

Have you tried running creduce on the preprocessed source? We should really 
have a real reproducer for this, otherwise its really hard to tell what the 
underlying problem is.


https://reviews.llvm.org/D54344



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

[PATCH] D54307: [clang] overload ignoringParens for Expr

2018-11-09 Thread Jonas Toth via Phabricator via cfe-commits

JonasToth added inline comments.



Comment at: include/clang/ASTMatchers/ASTMatchers.h:814
 /// would match the declaration for fp.
-AST_MATCHER_P(QualType, ignoringParens,
-  internal::Matcher, InnerMatcher) {
+AST_MATCHER_P_OVERLOAD(QualType, ignoringParens, internal::Matcher,
+   InnerMatcher, 0) {

aaron.ballman wrote:
> sbenza wrote:
> > JonasToth wrote:
> > > aaron.ballman wrote:
> > > > JonasToth wrote:
> > > > > aaron.ballman wrote:
> > > > > > Can you do this via `AST_POLYMORPHIC_MATCHER_P` instead, given that 
> > > > > > the implementation is the same?
> > > > > Do you want me to add more types? e.g. `TypeLoc` has 
> > > > > `IgnoreParens()`, too. 
> > > > I'd not be opposed, given that we already expose the `typeLoc()` 
> > > > matcher. I'll leave that to your discretion.
> > > as discussed on IRC making it an `AST_POLYMORPHIC_MATCHER_P` does not 
> > > seem to work, as the polymorphism is only in the return type. We do need 
> > > the `Node` itself to be polymorphic (same type as returntype). The only 
> > > working version I got was using the overloads.
> > You can't use AST_POLYMORPHIC_MATCHER_P to overload on input types.
> > You could try using templates, but that will make registering the matcher 
> > harder.
> > Another one that does input+output polymorphism, `id`, is simply not 
> > supported dynamically.
> Good to know, thank you!
thanks for clarifying.


Repository:
  rC Clang

https://reviews.llvm.org/D54307



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

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

2018-11-09 Thread Reid Kleckner via Phabricator via cfe-commits

rnk added a comment.

Threading a new options argument through mangleType that includes 
QualifierMangleMode as well as these obj-c options seems reasonable.


Repository:
  rC Clang

https://reviews.llvm.org/D52674



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

[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

2018-11-09 Thread Kristina Brooks via Phabricator via cfe-commits

kristina updated this revision to Diff 173411.
kristina added a comment.

Revised (style/ordering).


https://reviews.llvm.org/D54344

Files:
  lib/CodeGen/CGDeclCXX.cpp


Index: lib/CodeGen/CGDeclCXX.cpp
===
--- lib/CodeGen/CGDeclCXX.cpp
+++ lib/CodeGen/CGDeclCXX.cpp
@@ -64,10 +64,19 @@
 /// static storage duration.
 static void EmitDeclDestroy(CodeGenFunction , const VarDecl ,
 ConstantAddress addr) {
-  CodeGenModule  = CGF.CGM;
+  // Workaround for a bug that causes a reference to a nonexistent
+  // destructor under odd circumstances, when attribute no_destroy
+  // is used. This code should not be reachable under normal 
+  // circumstances, this workaround simply checks for the attribute
+  // again and bails if it's present instead of following a path
+  // that's either going to assert or emit incorrect code if reached.
+  if (D.hasAttr())
+return;
 
   // FIXME:  __attribute__((cleanup)) ?
 
+  CodeGenModule  = CGF.CGM;
+
   QualType type = D.getType();
   QualType::DestructionKind dtorKind = type.isDestructedType();
 


Index: lib/CodeGen/CGDeclCXX.cpp
===
--- lib/CodeGen/CGDeclCXX.cpp
+++ lib/CodeGen/CGDeclCXX.cpp
@@ -64,10 +64,19 @@
 /// static storage duration.
 static void EmitDeclDestroy(CodeGenFunction , const VarDecl ,
 ConstantAddress addr) {
-  CodeGenModule  = CGF.CGM;
+  // Workaround for a bug that causes a reference to a nonexistent
+  // destructor under odd circumstances, when attribute no_destroy
+  // is used. This code should not be reachable under normal 
+  // circumstances, this workaround simply checks for the attribute
+  // again and bails if it's present instead of following a path
+  // that's either going to assert or emit incorrect code if reached.
+  if (D.hasAttr())
+return;
 
   // FIXME:  __attribute__((cleanup)) ?
 
+  CodeGenModule  = CGF.CGM;
+
   QualType type = D.getType();
   QualType::DestructionKind dtorKind = type.isDestructedType();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33029: [clang-format] add option for dangling parenthesis

2018-11-09 Thread Adam Van Prooyen via Phabricator via cfe-commits

sciencemanx added a comment.

@djasper bump -- this feature is also really important to our team.


https://reviews.llvm.org/D33029



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

[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

2018-11-09 Thread Kristina Brooks via Phabricator via cfe-commits

kristina created this revision.
kristina added reviewers: rsmith, clang, JonasToth, EricWF, aaron.ballman.
Herald added subscribers: dexonsmith, mehdi_amini.
Herald added a reviewer: jfb.

Difficult to reproduce crash, attempted it in the test, it appears to not 
trigger it. I can consistently reproduce it when building a large project with 
this attribute added and can consistently crash it without the fix in place, 
however.

This avoids the codepath that should not be executed in the first place by 
rechecking `NoDestroyAttr` and bailing out instead of emitting incorrect code 
or causing an assert in assert enabled builds. This does not appear to cause 
any regressions for x86_64 test suite and my test is useless since it does not 
offer the needed coverage.

  clang-8: /SourceCache/llvm-trunk-8.0/include/llvm/Support/Casting.h:106: 
static bool llvm::isa_impl_cl::doit(const From *) [To = clang::CXXConstructorDecl, 
From = const clang::CXXMethodDecl *]: Assertion `Val && "isa<> used on a null 
pointer"' failed.
  
  -- asserts here, rest is signal handler/stack trace --
  
  #9 0x00d5a35d toCXXCtorType 
/SourceCache/llvm-trunk-8.0/tools/clang/lib/CodeGen/CodeGenTypes.h:66:3
  #10 0x00d5a35d 
clang::CodeGen::CodeGenModule::getAddrOfCXXStructor(clang::CXXMethodDecl 
const*, clang::CodeGen::StructorType, clang::CodeGen::CGFunctionInfo const*, 
llvm::FunctionType*, bool, clang::CodeGen::ForDefinition_t) 
/SourceCache/llvm-trunk-8.0/tools/clang/lib/CodeGen/CGCXX.cpp:237:0
  #11 0x00e05c9b Address 
/SourceCache/llvm-trunk-8.0/tools/clang/lib/CodeGen/Address.h:31:5
  #12 0x00e05c9b ConstantAddress 
/SourceCache/llvm-trunk-8.0/tools/clang/lib/CodeGen/Address.h:78:0
  #13 0x00e05c9b 
clang::CodeGen::CodeGenFunction::EmitCXXGlobalVarDeclInit(clang::VarDecl 
const&, llvm::Constant*, bool) 
/SourceCache/llvm-trunk-8.0/tools/clang/lib/CodeGen/CGDeclCXX.cpp:179:0
  #14 0x00e07b7a 
clang::CodeGen::CodeGenFunction::GenerateCXXGlobalVarDeclInitFunc(llvm::Function*,
 clang::VarDecl const*, llvm::GlobalVariable*, bool) 
/SourceCache/llvm-trunk-8.0/tools/clang/lib/CodeGen/CGDeclCXX.cpp:608:3
  #15 0x00e06fc8 
clang::CodeGen::CodeGenModule::EmitCXXGlobalVarDeclInitFunc(clang::VarDecl 
const*, llvm::GlobalVariable*, bool) 
/SourceCache/llvm-trunk-8.0/tools/clang/lib/CodeGen/CGDeclCXX.cpp:441:3
  #16 0x00b6596f 
clang::CodeGen::CodeGenModule::EmitGlobalVarDefinition(clang::VarDecl const*, 
bool) 
/SourceCache/llvm-trunk-8.0/tools/clang/lib/CodeGen/CodeGenModule.cpp:3650:5
  #17 0x00b5c66b 
clang::CodeGen::CodeGenModule::EmitGlobalDefinition(clang::GlobalDecl, 
llvm::GlobalValue*) 
/SourceCache/llvm-trunk-8.0/tools/clang/lib/CodeGen/CodeGenModule.cpp:0:12
  #18 0x00b67dfb getKind 
/SourceCache/llvm-trunk-8.0/tools/clang/include/clang/AST/DeclBase.h:421:51
  #19 0x00b67dfb classof 
/SourceCache/llvm-trunk-8.0/tools/clang/include/clang/AST/DeclCXX.h:3875:0
  #20 0x00b67dfb doit 
/SourceCache/llvm-trunk-8.0/include/llvm/Support/Casting.h:59:0
  #21 0x00b67dfb doit 
/SourceCache/llvm-trunk-8.0/include/llvm/Support/Casting.h:107:0
  #22 0x00b67dfb doit 
/SourceCache/llvm-trunk-8.0/include/llvm/Support/Casting.h:133:0
  #23 0x00b67dfb doit 
/SourceCache/llvm-trunk-8.0/include/llvm/Support/Casting.h:123:0
  #24 0x00b67dfb isa 
/SourceCache/llvm-trunk-8.0/include/llvm/Support/Casting.h:143:0
  #25 0x00b67dfb dyn_cast 
/SourceCache/llvm-trunk-8.0/include/llvm/Support/Casting.h:334:0
  #26 0x00b67dfb 
clang::CodeGen::CodeGenModule::EmitTopLevelDecl(clang::Decl*) 
/SourceCache/llvm-trunk-8.0/tools/clang/lib/CodeGen/CodeGenModule.cpp:4783:0
  #27 0x00b6bf7b getPointer 
/SourceCache/llvm-trunk-8.0/include/llvm/ADT/PointerIntPair.h:56:58
  #28 0x00b6bf7b getNextDeclInContext 
/SourceCache/llvm-trunk-8.0/tools/clang/include/clang/AST/DeclBase.h:424:0
  #29 0x00b6bf7b operator++ 
/SourceCache/llvm-trunk-8.0/tools/clang/include/clang/AST/DeclBase.h:1973:0
  #30 0x00b6bf7b 
clang::CodeGen::CodeGenModule::EmitDeclContext(clang::DeclContext const*) 
/SourceCache/llvm-trunk-8.0/tools/clang/lib/CodeGen/CodeGenModule.cpp:4744:0
  #31 0x0115a470 
_ZN12_GLOBAL__N_117CodeGeneratorImpl18HandleTopLevelDeclEN5clang12DeclGroupRefE$7cf5ba2c3e38da85712ae9dd9c2a6e32
 /SourceCache/llvm-trunk-8.0/tools/clang/lib/CodeGen/ModuleBuilder.cpp:159:73
  #32 0x0115833d 
clang::BackendConsumer::HandleTopLevelDecl(clang::DeclGroupRef) 
/SourceCache/llvm-trunk-8.0/tools/clang/lib/CodeGen/CodeGenAction.cpp:171:11
  #33 0x011642f4 clang::ParseAST(clang::Sema&, bool, bool) 
/SourceCache/llvm-trunk-8.0/tools/clang/lib/Parse/ParseAST.cpp:161:11
  #34 0x010c9d83 shouldBuildGlobalModuleIndex 
/SourceCache/llvm-trunk-8.0/tools/clang/lib/Frontend/CompilerInstance.cpp:81:11
  #35 0x010c9d83 clang::FrontendAction::Execute()

r346551 - [OPENMP][NVPTX]Extend number of constructs executed in SPMD mode.

2018-11-09 Thread Alexey Bataev via cfe-commits

Author: abataev
Date: Fri Nov  9 12:03:19 2018
New Revision: 346551

URL: http://llvm.org/viewvc/llvm-project?rev=346551=rev
Log:
[OPENMP][NVPTX]Extend number of constructs executed in SPMD mode.

If the statements between target|teams|distribute directives does not
require execution in master thread, like constant expressions, null
statements, simple declarations, etc., such construct can be xecuted in
SPMD mode.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/test/OpenMP/nvptx_SPMD_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_target_parallel_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_target_parallel_proc_bind_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_target_teams_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp

cfe/trunk/test/OpenMP/nvptx_target_teams_distribute_parallel_for_simd_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=346551=346550=346551=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Fri Nov  9 12:03:19 2018
@@ -698,12 +698,58 @@ getDataSharingMode(CodeGenModule ) {
   : CGOpenMPRuntimeNVPTX::Generic;
 }
 
+// Checks if the expression is constant or does not have non-trivial function
+// calls.
+static bool isTrivial(ASTContext , const Expr * E) {
+  // We can skip constant expressions.
+  // We can skip expressions with trivial calls or simple expressions.
+  return (E->isEvaluatable(Ctx, Expr::SE_AllowUndefinedBehavior) ||
+  !E->hasNonTrivialCall(Ctx)) &&
+ !E->HasSideEffects(Ctx, /*IncludePossibleEffects=*/true);
+}
+
 /// Checks if the \p Body is the \a CompoundStmt and returns its child 
statement
-/// iff there is only one.
-static const Stmt *getSingleCompoundChild(const Stmt *Body) {
-  if (const auto *C = dyn_cast(Body))
-if (C->size() == 1)
-  return C->body_front();
+/// iff there is only one that is not evaluatable at the compile time.
+static const Stmt *getSingleCompoundChild(ASTContext , const Stmt *Body) {
+  if (const auto *C = dyn_cast(Body)) {
+const Stmt *Child = nullptr;
+for (const Stmt *S : C->body()) {
+  if (const auto *E = dyn_cast(S)) {
+if (isTrivial(Ctx, E))
+  continue;
+  }
+  // Some of the statements can be ignored.
+  if (isa(S) || isa(S) || isa(S) ||
+  isa(S) || isa(S))
+continue;
+  // Analyze declarations.
+  if (const auto *DS = dyn_cast(S)) {
+if (llvm::all_of(DS->decls(), [](const Decl *D) {
+  if (isa(D) || isa(D) ||
+  isa(D) || isa(D) ||
+  isa(D) || isa(D) ||
+  isa(D) ||
+  isa(D) ||
+  isa(D))
+return true;
+  const auto *VD = dyn_cast(D);
+  if (!VD)
+return false;
+  return VD->isConstexpr() ||
+ ((VD->getType().isTrivialType(Ctx) ||
+   VD->getType()->isReferenceType()) &&
+  (!VD->hasInit() || isTrivial(Ctx, VD->getInit(;
+}))
+  continue;
+  }
+  // Found multiple children - cannot get the one child only.
+  if (Child)
+return Body;
+  Child = S;
+}
+if (Child)
+  return Child;
+  }
   return Body;
 }
 
@@ -732,7 +778,7 @@ static bool hasNestedSPMDDirective(ASTCo
   const auto *CS = D.getInnermostCapturedStmt();
   const auto *Body =
   CS->getCapturedStmt()->IgnoreContainers(/*IgnoreCaptured=*/true);
-  const Stmt *ChildStmt = getSingleCompoundChild(Body);
+  const Stmt *ChildStmt = getSingleCompoundChild(Ctx, Body);
 
   if (const auto *NestedDir = dyn_cast(ChildStmt)) {
 OpenMPDirectiveKind DKind = NestedDir->getDirectiveKind();
@@ -746,7 +792,7 @@ static bool hasNestedSPMDDirective(ASTCo
 /*IgnoreCaptured=*/true);
 if (!Body)
   return false;
-ChildStmt = getSingleCompoundChild(Body);
+ChildStmt = getSingleCompoundChild(Ctx, Body);
 if (const auto *NND = dyn_cast(ChildStmt)) {
   DKind = NND->getDirectiveKind();
   if (isOpenMPParallelDirective(DKind) &&
@@ -905,7 +951,7 @@ static bool hasNestedLightweightDirectiv
   const auto *CS = D.getInnermostCapturedStmt();
   const auto *Body =
   CS->getCapturedStmt()->IgnoreContainers(/*IgnoreCaptured=*/true);
-  const Stmt *ChildStmt = getSingleCompoundChild(Body);
+  const Stmt *ChildStmt = getSingleCompoundChild(Ctx, Body);
 
   if (const auto *NestedDir = dyn_cast(ChildStmt)) {
 OpenMPDirectiveKind DKind = NestedDir->getDirectiveKind();
@@ -920,7 +966,7 @@ static bool

[PATCH] D54307: [clang] overload ignoringParens for Expr

2018-11-09 Thread Aaron Ballman via Phabricator via cfe-commits

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

LGTM!




Comment at: include/clang/ASTMatchers/ASTMatchers.h:814
 /// would match the declaration for fp.
-AST_MATCHER_P(QualType, ignoringParens,
-  internal::Matcher, InnerMatcher) {
+AST_MATCHER_P_OVERLOAD(QualType, ignoringParens, internal::Matcher,
+   InnerMatcher, 0) {

sbenza wrote:
> JonasToth wrote:
> > aaron.ballman wrote:
> > > JonasToth wrote:
> > > > aaron.ballman wrote:
> > > > > Can you do this via `AST_POLYMORPHIC_MATCHER_P` instead, given that 
> > > > > the implementation is the same?
> > > > Do you want me to add more types? e.g. `TypeLoc` has `IgnoreParens()`, 
> > > > too. 
> > > I'd not be opposed, given that we already expose the `typeLoc()` matcher. 
> > > I'll leave that to your discretion.
> > as discussed on IRC making it an `AST_POLYMORPHIC_MATCHER_P` does not seem 
> > to work, as the polymorphism is only in the return type. We do need the 
> > `Node` itself to be polymorphic (same type as returntype). The only working 
> > version I got was using the overloads.
> You can't use AST_POLYMORPHIC_MATCHER_P to overload on input types.
> You could try using templates, but that will make registering the matcher 
> harder.
> Another one that does input+output polymorphism, `id`, is simply not 
> supported dynamically.
Good to know, thank you!


Repository:
  rC Clang

https://reviews.llvm.org/D54307



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

[PATCH] D54307: [clang] overload ignoringParens for Expr

2018-11-09 Thread Samuel Benzaquen via Phabricator via cfe-commits

sbenza added inline comments.



Comment at: include/clang/ASTMatchers/ASTMatchers.h:814
 /// would match the declaration for fp.
-AST_MATCHER_P(QualType, ignoringParens,
-  internal::Matcher, InnerMatcher) {
+AST_MATCHER_P_OVERLOAD(QualType, ignoringParens, internal::Matcher,
+   InnerMatcher, 0) {

JonasToth wrote:
> aaron.ballman wrote:
> > JonasToth wrote:
> > > aaron.ballman wrote:
> > > > Can you do this via `AST_POLYMORPHIC_MATCHER_P` instead, given that the 
> > > > implementation is the same?
> > > Do you want me to add more types? e.g. `TypeLoc` has `IgnoreParens()`, 
> > > too. 
> > I'd not be opposed, given that we already expose the `typeLoc()` matcher. 
> > I'll leave that to your discretion.
> as discussed on IRC making it an `AST_POLYMORPHIC_MATCHER_P` does not seem to 
> work, as the polymorphism is only in the return type. We do need the `Node` 
> itself to be polymorphic (same type as returntype). The only working version 
> I got was using the overloads.
You can't use AST_POLYMORPHIC_MATCHER_P to overload on input types.
You could try using templates, but that will make registering the matcher 
harder.
Another one that does input+output polymorphism, `id`, is simply not supported 
dynamically.


Repository:
  rC Clang

https://reviews.llvm.org/D54307



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

Re: r346548 - Use the correct address space when bitcasting func pointer to int pointer

2018-11-09 Thread Roman Lebedev via cfe-commits

The codegen change can't be tested?
On Fri, Nov 9, 2018 at 10:44 PM Dylan McKay via cfe-commits
 wrote:
>
> Author: dylanmckay
> Date: Fri Nov  9 11:42:05 2018
> New Revision: 346548
>
> URL: http://llvm.org/viewvc/llvm-project?rev=346548=rev
> Log:
> Use the correct address space when bitcasting func pointer to int pointer
>
> When we cast a function pointer to an int pointer, at some pointer later
> it gets bitcasted back to a function and called.
>
> In backends that have a nonzero program memory address space specified
> in the data layout, the old code would lose the address space data. When
> LLVM later attempted to generate the bitcast from i8* to i8(..)*
> addrspace(1), it would fail because the pointers are not in the same
> address space.
>
> With this patch, the address space of the function will carry on to the
> address space of the i8* pointer. This is because all function pointers
> in Harvard architectures need to be assigned to the correct address
> space.
>
> This has no effect to any in-tree backends except AVR.
>
> Modified:
> cfe/trunk/lib/CodeGen/CGException.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGException.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGException.cpp?rev=346548=346547=346548=diff
> ==
> --- cfe/trunk/lib/CodeGen/CGException.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGException.cpp Fri Nov  9 11:42:05 2018
> @@ -250,7 +250,11 @@ static llvm::Constant *getPersonalityFn(
>  static llvm::Constant *getOpaquePersonalityFn(CodeGenModule ,
>  const EHPersonality ) {
>llvm::Constant *Fn = getPersonalityFn(CGM, Personality);
> -  return llvm::ConstantExpr::getBitCast(Fn, CGM.Int8PtrTy);
> +  llvm::PointerType* Int8PtrTy = llvm::PointerType::get(
> +  llvm::Type::getInt8Ty(CGM.getLLVMContext()),
> +  CGM.getDataLayout().getProgramAddressSpace());
> +
> +  return llvm::ConstantExpr::getBitCast(Fn, Int8PtrTy);
>  }
>
>  /// Check whether a landingpad instruction only uses C++ features.
>
>
> ___
> 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

r346549 - Revert rL346454: Fix a use-after-free introduced by r344915.

2018-11-09 Thread Simon Pilgrim via cfe-commits

Author: rksimon
Date: Fri Nov  9 11:42:53 2018
New Revision: 346549

URL: http://llvm.org/viewvc/llvm-project?rev=346549=rev
Log:
Revert rL346454: Fix a use-after-free introduced by r344915.

r344915 added a call to ApplyDebugLocation to the sanitizer check
function emitter. Some of the sanitizers are emitted in the function
epilogue though and the LexicalScopeStack is emptied out before. By
detecting this situation and early-exiting from ApplyDebugLocation the
fallback location is used, which is equivalent to the return location.

rdar://problem/45859802

Causes EXPENSIVE_CHECKS build bot failures: 
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win

Removed:
cfe/trunk/test/CodeGen/ubsan-debuglog-return.c
Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=346549=346548=346549=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Fri Nov  9 11:42:53 2018
@@ -3538,7 +3538,7 @@ void CGDebugInfo::EmitLocation(CGBuilder
   // Update our current location
   setLocation(Loc);
 
-  if (CurLoc.isInvalid() || CurLoc.isMacroID() || LexicalBlockStack.empty())
+  if (CurLoc.isInvalid() || CurLoc.isMacroID())
 return;
 
   llvm::MDNode *Scope = LexicalBlockStack.back();

Removed: cfe/trunk/test/CodeGen/ubsan-debuglog-return.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ubsan-debuglog-return.c?rev=346548=auto
==
--- cfe/trunk/test/CodeGen/ubsan-debuglog-return.c (original)
+++ cfe/trunk/test/CodeGen/ubsan-debuglog-return.c (removed)
@@ -1,10 +0,0 @@
-// RUN: %clang_cc1 -x c -debug-info-kind=line-tables-only -emit-llvm 
-fsanitize=returns-nonnull-attribute -o - %s | FileCheck %s
-// The UBSAN function call in the epilogue needs to have a debug location.
-
-__attribute__((returns_nonnull)) void *allocate() {}
-
-// CHECK: define nonnull i8* @allocate(){{.*}} !dbg
-// CHECK: call void @__ubsan_handle_nonnull_return_v1_abort
-// CHECK-SAME:  !dbg ![[LOC:[0-9]+]]
-// CHECK: ret i8*
-// CHECK-SAME:  !dbg ![[LOC]]


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

r346548 - Use the correct address space when bitcasting func pointer to int pointer

2018-11-09 Thread Dylan McKay via cfe-commits

Author: dylanmckay
Date: Fri Nov  9 11:42:05 2018
New Revision: 346548

URL: http://llvm.org/viewvc/llvm-project?rev=346548=rev
Log:
Use the correct address space when bitcasting func pointer to int pointer

When we cast a function pointer to an int pointer, at some pointer later
it gets bitcasted back to a function and called.

In backends that have a nonzero program memory address space specified
in the data layout, the old code would lose the address space data. When
LLVM later attempted to generate the bitcast from i8* to i8(..)*
addrspace(1), it would fail because the pointers are not in the same
address space.

With this patch, the address space of the function will carry on to the
address space of the i8* pointer. This is because all function pointers
in Harvard architectures need to be assigned to the correct address
space.

This has no effect to any in-tree backends except AVR.

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

Modified: cfe/trunk/lib/CodeGen/CGException.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGException.cpp?rev=346548=346547=346548=diff
==
--- cfe/trunk/lib/CodeGen/CGException.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGException.cpp Fri Nov  9 11:42:05 2018
@@ -250,7 +250,11 @@ static llvm::Constant *getPersonalityFn(
 static llvm::Constant *getOpaquePersonalityFn(CodeGenModule ,
 const EHPersonality ) {
   llvm::Constant *Fn = getPersonalityFn(CGM, Personality);
-  return llvm::ConstantExpr::getBitCast(Fn, CGM.Int8PtrTy);
+  llvm::PointerType* Int8PtrTy = llvm::PointerType::get(
+  llvm::Type::getInt8Ty(CGM.getLLVMContext()),
+  CGM.getDataLayout().getProgramAddressSpace());
+
+  return llvm::ConstantExpr::getBitCast(Fn, Int8PtrTy);
 }
 
 /// Check whether a landingpad instruction only uses C++ features.


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

r346547 - Allow a double-underscore spelling of Clang attributes using double square bracket syntax.

2018-11-09 Thread Aaron Ballman via cfe-commits

Author: aaronballman
Date: Fri Nov  9 11:37:18 2018
New Revision: 346547

URL: http://llvm.org/viewvc/llvm-project?rev=346547=rev
Log:
Allow a double-underscore spelling of Clang attributes using double square 
bracket syntax.

This matches a similar behavior with GCC accepting [[gnu::__attr__]] as a alias 
for [[gnu::attr]] in that clang attributes can now be spelled with two leading 
and trailing underscores.

I had always intended for this to work, but missed the critical bit. We already 
had an existing test in test/Preprocessor/has_attribute.cpp for 
[[clang::__fallthrough__]] but using that spelling would still give an "unknown 
attribute" diagnostic.

Modified:
cfe/trunk/lib/Sema/ParsedAttr.cpp
cfe/trunk/test/SemaCXX/attr-optnone.cpp
cfe/trunk/test/SemaCXX/switch-implicit-fallthrough.cpp

Modified: cfe/trunk/lib/Sema/ParsedAttr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/ParsedAttr.cpp?rev=346547=346546=346547=diff
==
--- cfe/trunk/lib/Sema/ParsedAttr.cpp (original)
+++ cfe/trunk/lib/Sema/ParsedAttr.cpp Fri Nov  9 11:37:18 2018
@@ -122,11 +122,12 @@ static StringRef normalizeAttrName(Strin
ParsedAttr::Syntax SyntaxUsed) {
   // Normalize the attribute name, __foo__ becomes foo. This is only allowable
   // for GNU attributes, and attributes using the double square bracket syntax.
-  bool IsGNU = SyntaxUsed == ParsedAttr::AS_GNU ||
-   ((SyntaxUsed == ParsedAttr::AS_CXX11 ||
- SyntaxUsed == ParsedAttr::AS_C2x) &&
-NormalizedScopeName == "gnu");
-  if (IsGNU && AttrName.size() >= 4 && AttrName.startswith("__") &&
+  bool ShouldNormalize =
+  SyntaxUsed == ParsedAttr::AS_GNU ||
+  ((SyntaxUsed == ParsedAttr::AS_CXX11 ||
+SyntaxUsed == ParsedAttr::AS_C2x) &&
+   (NormalizedScopeName == "gnu" || NormalizedScopeName == "clang"));
+  if (ShouldNormalize && AttrName.size() >= 4 && AttrName.startswith("__") &&
   AttrName.endswith("__"))
 AttrName = AttrName.slice(2, AttrName.size() - 2);
 

Modified: cfe/trunk/test/SemaCXX/attr-optnone.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/attr-optnone.cpp?rev=346547=346546=346547=diff
==
--- cfe/trunk/test/SemaCXX/attr-optnone.cpp (original)
+++ cfe/trunk/test/SemaCXX/attr-optnone.cpp Fri Nov  9 11:37:18 2018
@@ -72,8 +72,10 @@ struct B2 {
 };
 
 // Verify that we can handle the [[_Clang::optnone]] and
-// [[__clang__::optnone]] spellings.
+// [[__clang__::optnone]] spellings, as well as [[clang::__optnone__]].
 [[_Clang::optnone]] int foo3();
 [[__clang__::optnone]] int foo4(); // expected-warning {{'__clang__' is a 
predefined macro name, not an attribute scope specifier; did you mean '_Clang' 
instead?}}
+[[clang::__optnone__]] int foo5();
+[[_Clang::__optnone__]] int foo6();
 
-[[_Clang::optnone]] int foo5; // expected-warning {{'optnone' attribute only 
applies to functions}}
+[[_Clang::optnone]] int foo7; // expected-warning {{'optnone' attribute only 
applies to functions}}

Modified: cfe/trunk/test/SemaCXX/switch-implicit-fallthrough.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/switch-implicit-fallthrough.cpp?rev=346547=346546=346547=diff
==
--- cfe/trunk/test/SemaCXX/switch-implicit-fallthrough.cpp (original)
+++ cfe/trunk/test/SemaCXX/switch-implicit-fallthrough.cpp Fri Nov  9 11:37:18 
2018
@@ -314,3 +314,18 @@ int fallthrough_targets(int n) {
   }
   return n;
 }
+
+int fallthrough_alt_spelling(int n) {
+  switch (n) {
+  case 0:
+n++;
+[[clang::fallthrough]];
+  case 1:
+n++;
+[[clang::__fallthrough__]];
+  case 2:
+n++;
+break;
+  }
+  return n;
+}


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

[PATCH] D54307: [clang] overload ignoringParens for Expr

2018-11-09 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added reviewers: sbenza, klimek.
aaron.ballman added a comment.

Adding a few other reviewers in case they have ideas on how to use the 
polymorphic matcher rather than the overload. If they don't have a better idea, 
then I think the overload approach is fine.


Repository:
  rC Clang

https://reviews.llvm.org/D54307



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

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

2018-11-09 Thread Shoaib Meenai via Phabricator via cfe-commits

smeenai added a comment.

In https://reviews.llvm.org/D52674#1293180, @rjmccall wrote:

> In https://reviews.llvm.org/D52674#1291284, @smeenai wrote:
>
> > In https://reviews.llvm.org/D52674#1281747, @rjmccall wrote:
> >
> > > In https://reviews.llvm.org/D52674#1271814, @smeenai wrote:
> > >
> > > > @rjmccall I prototyped the ForRTTI parameter approach in 
> > > > https://reviews.llvm.org/D53546. It could definitely be cleaned up a 
> > > > bit, but it demonstrates the problems I saw with the added parameter. 
> > > > Namely, `mangleType(QualType, SourceRange, QualifierMangleMode)` has a 
> > > > bunch of additional logic which is needed for correctness, so we need 
> > > > to thread the parameter through the entire chain, and the only way I 
> > > > could think of doing that without adding the parameter to every single 
> > > > `mangleType` overload was to have an additional switch to dispatch to 
> > > > the overloads with the added ForRTTI parameter, which is pretty ugly.
> > > >
> > > > As I see it, there are a few ways to proceed here:
> > > >
> > > > - The approach in https://reviews.llvm.org/D53546 (cleaned up a bit). I 
> > > > think it's pretty ugly, but you may have suggestions on how to do it 
> > > > better.
> > > > - In the `mangleType` overload for `ObjCObjectPointerType`, skipping 
> > > > the generic `mangleType` dispatch and going directly to either the 
> > > > `ObjCObjectType` or `ObjCInterfaceType` overloads. That avoids the 
> > > > ugliness in the generic `mangleType`, but it also requires us to 
> > > > duplicate some logic from it in the `ObjCObjectPointerType` overload, 
> > > > which doesn't seem great either.
> > > > - Maintaining `ForRTTI` state in the mangler instead of threading a 
> > > > parameter through (I'm generally not a fan of state like that, but it 
> > > > might be cleaner than the threading in this case?)
> > > > - Just sticking with what I'm doing in this patch.
> > > >
> > > >   What do you think?
> > >
> > >
> > > Sorry for the delay.  I think duplicating the dispatch logic for 
> > > `ObjCObjectPointerType` is probably the best approach; the pointee type 
> > > will always an `ObjCObjectType` of some sort, and there are only two 
> > > kinds of those.
> >
> >
> > Sorry, I'm still not sure how this will work.
> >
> > Duplicating the dispatch logic for `ObjCObjectPointerType` ends up looking 
> > like https://reviews.llvm.org/P8114, which is fine. However, when we're 
> > actually mangling RTTI or RTTI names, we're still going through the main 
> > `mangleType(QualType, SourceRange, QualifierMangleMode)` overload, which 
> > still requires us to thread `ForRTTI` through that function, which still 
> > requires us to duplicate the switch in that function (to handle the 
> > `ForRTTI` case, since the other switch is generated via TypeNodes.def and 
> > not easily modifiable), which is the main ugliness in my opinion. Do you 
> > also want me to add special dispatching to `mangleCXXRTTI` and 
> > `mangleCXXRTTIName` to just call the `ObjCObjectPointerType` function 
> > directly when they're dealing with that type? That's certainly doable, but 
> > at that point just keeping some state around in the demangler starts to 
> > feel cleaner, at least to me.
>
>
> Well, you could check for an `ObjCObjectPointerType` in the top-level routine.
>
> But yeah, probably the cleanest thing to do would to be push the state 
> through the main dispatch.  Don't push a single `bool` through, though; make 
> a `TypeManglingOptions` struct to encapsulate it, in case we have a similar 
> problem elsewhere.  It should have a method for getting options that should 
> be propagated down to component type manglings, and that method can drop the 
> "for RTTI" bit; that's the part that `ObjCObjectPointerType` can handle 
> differently.


All right, the struct is a good idea. It would require adding the 
`TypeManglingOptions` parameter to all the `mangleType` overloads (unless we 
want to stick with the second switch in the main `mangleType` dispatch 
function, but that seems super ugly, especially if we're doing a more general 
solution), so I wanted to confirm @rnk is okay with adding a parameter to all 
those overloads as well before proceeding.


Repository:
  rC Clang

https://reviews.llvm.org/D52674



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

r346542 - Fix a nondeterminism in the debug info for VLA size expressions.

2018-11-09 Thread Adrian Prantl via cfe-commits

Author: adrian
Date: Fri Nov  9 11:17:56 2018
New Revision: 346542

URL: http://llvm.org/viewvc/llvm-project?rev=346542=rev
Log:
Fix a nondeterminism in the debug info for VLA size expressions.

The artificial variable describing the array size is supposed to be
called "__vla_expr", but this was implemented by retrieving the name
of the associated alloca, which isn't a reliable source for the name,
since nonassert compilers may drop names from LLVM IR.

rdar://problem/45924808

Modified:
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/test/CodeGen/debug-info-vla.c
cfe/trunk/test/CodeGenCXX/debug-info-vla.cpp

Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=346542=346541=346542=diff
==
--- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp Fri Nov  9 11:17:56 2018
@@ -1066,6 +1066,7 @@ void CodeGenFunction::EmitAndRegisterVar
   // For each dimension stores its QualType and corresponding
   // size-expression Value.
   SmallVector Dimensions;
+  SmallVector VLAExprNames;
 
   // Break down the array into individual dimensions.
   QualType Type1D = D.getType();
@@ -1074,8 +1075,14 @@ void CodeGenFunction::EmitAndRegisterVar
 if (auto *C = dyn_cast(VlaSize.NumElts))
   Dimensions.emplace_back(C, Type1D.getUnqualifiedType());
 else {
-  auto SizeExprAddr = CreateDefaultAlignTempAlloca(
-  VlaSize.NumElts->getType(), "__vla_expr");
+  // Generate a locally unique name for the size expression.
+  Twine Name = Twine("__vla_expr") + Twine(VLAExprCounter++);
+  SmallString<12> Buffer;
+  StringRef NameRef = Name.toStringRef(Buffer);
+  auto  = getContext().Idents.getOwn(NameRef);
+  VLAExprNames.push_back();
+  auto SizeExprAddr =
+  CreateDefaultAlignTempAlloca(VlaSize.NumElts->getType(), NameRef);
   Builder.CreateStore(VlaSize.NumElts, SizeExprAddr);
   Dimensions.emplace_back(SizeExprAddr.getPointer(),
   Type1D.getUnqualifiedType());
@@ -1089,20 +1096,20 @@ void CodeGenFunction::EmitAndRegisterVar
   // Register each dimension's size-expression with a DILocalVariable,
   // so that it can be used by CGDebugInfo when instantiating a DISubrange
   // to describe this array.
+  unsigned NameIdx = 0;
   for (auto  : Dimensions) {
 llvm::Metadata *MD;
 if (auto *C = dyn_cast(VlaSize.NumElts))
   MD = llvm::ConstantAsMetadata::get(C);
 else {
   // Create an artificial VarDecl to generate debug info for.
-  IdentifierInfo  = getContext().Idents.getOwn(
-  cast(VlaSize.NumElts)->getName());
+  IdentifierInfo *NameIdent = VLAExprNames[NameIdx++];
   auto VlaExprTy = VlaSize.NumElts->getType()->getPointerElementType();
   auto QT = getContext().getIntTypeForBitwidth(
   VlaExprTy->getScalarSizeInBits(), false);
   auto *ArtificialDecl = VarDecl::Create(
   getContext(), const_cast(D.getDeclContext()),
-  D.getLocation(), D.getLocation(), , QT,
+  D.getLocation(), D.getLocation(), NameIdent, QT,
   getContext().CreateTypeSourceInfo(QT), SC_Auto);
   ArtificialDecl->setImplicit();
 

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=346542=346541=346542=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Fri Nov  9 11:17:56 2018
@@ -1197,6 +1197,8 @@ public:
 
 private:
   CGDebugInfo *DebugInfo;
+  /// Used to create unique names for artificial VLA size debug info variables.
+  unsigned VLAExprCounter = 0;
   bool DisableDebugInfo = false;
 
   /// DidCallStackSave - Whether llvm.stacksave has been called. Used to avoid

Modified: cfe/trunk/test/CodeGen/debug-info-vla.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/debug-info-vla.c?rev=346542=346541=346542=diff
==
--- cfe/trunk/test/CodeGen/debug-info-vla.c (original)
+++ cfe/trunk/test/CodeGen/debug-info-vla.c Fri Nov  9 11:17:56 2018
@@ -2,9 +2,9 @@
 
 void testVLAwithSize(int s)
 {
-// CHECK-DAG: dbg.declare({{.*}} %__vla_expr, metadata ![[VLAEXPR:[0-9]+]]
+// CHECK-DAG: dbg.declare({{.*}} %__vla_expr0, metadata ![[VLAEXPR:[0-9]+]]
 // CHECK-DAG: dbg.declare({{.*}} %vla, metadata ![[VAR:[0-9]+]]
-// CHECK-DAG: ![[VLAEXPR]] = !DILocalVariable(name: "__vla_expr", {{.*}} 
flags: DIFlagArtificial
+// CHECK-DAG: ![[VLAEXPR]] = !DILocalVariable(name: "__vla_expr0", {{.*}} 
flags: DIFlagArtificial
 // CHECK-DAG: ![[VAR]] = !DILocalVariable(name: "vla",{{.*}} line: [[@LINE+2]]
 // CHECK-DAG: !DISubrange(count: ![[VLAEXPR]])
   int vla[s];

Modified:

[PATCH] D54307: [clang] overload ignoringParens for Expr

2018-11-09 Thread Jonas Toth via Phabricator via cfe-commits

JonasToth updated this revision to Diff 173391.
JonasToth marked 3 inline comments as done.
JonasToth added a comment.

- add unit test


Repository:
  rC Clang

https://reviews.llvm.org/D54307

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/ASTMatchersNodeTest.cpp


Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1147,6 +1147,14 @@
  parenExpr()));
 }
 
+TEST(ParenExpression, IgnoringParens) {
+  EXPECT_FALSE(matches("const char* str = (\"my-string\");",
+   
implicitCastExpr(hasSourceExpression(stringLiteral();
+  EXPECT_TRUE(matches(
+  "const char* str = (\"my-string\");",
+  implicitCastExpr(hasSourceExpression(ignoringParens(stringLiteral());
+}
+
 TEST(TypeMatching, MatchesTypes) {
   EXPECT_TRUE(matches("struct S {};", qualType().bind("loc")));
 }
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -106,6 +106,7 @@
   REGISTER_OVERLOADED_2(callee);
   REGISTER_OVERLOADED_2(hasPrefix);
   REGISTER_OVERLOADED_2(hasType);
+  REGISTER_OVERLOADED_2(ignoringParens);
   REGISTER_OVERLOADED_2(isDerivedFrom);
   REGISTER_OVERLOADED_2(isSameOrDerivedFrom);
   REGISTER_OVERLOADED_2(loc);
@@ -318,7 +319,6 @@
   REGISTER_MATCHER(ignoringImplicit);
   REGISTER_MATCHER(ignoringParenCasts);
   REGISTER_MATCHER(ignoringParenImpCasts);
-  REGISTER_MATCHER(ignoringParens);
   REGISTER_MATCHER(imaginaryLiteral);
   REGISTER_MATCHER(implicitCastExpr);
   REGISTER_MATCHER(implicitValueInitExpr);
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -811,11 +811,28 @@
 ///   varDecl(hasType(pointerType(pointee(ignoringParens(functionType())
 /// \endcode
 /// would match the declaration for fp.
-AST_MATCHER_P(QualType, ignoringParens,
-  internal::Matcher, InnerMatcher) {
+AST_MATCHER_P_OVERLOAD(QualType, ignoringParens, internal::Matcher,
+   InnerMatcher, 0) {
   return InnerMatcher.matches(Node.IgnoreParens(), Finder, Builder);
 }
 
+/// Overload \c ignoringParens for \c Expr.
+///
+/// Given
+/// \code
+///   const char* str = ("my-string");
+/// \endcode
+/// The matcher
+/// \code
+///   implicitCastExpr(hasSourceExpression(ignoringParens(stringLiteral(
+/// \endcode
+/// would match the implicit cast resulting from the assignment.
+AST_MATCHER_P_OVERLOAD(Expr, ignoringParens, internal::Matcher,
+   InnerMatcher, 1) {
+  const Expr *E = Node.IgnoreParens();
+  return InnerMatcher.matches(*E, Finder, Builder);
+}
+
 /// Matches expressions that are instantiation-dependent even if it is
 /// neither type- nor value-dependent.
 ///
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -5552,6 +5552,17 @@
 
 
 
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1Expr.html;>ExprignoringParensMatcherhttps://clang.llvm.org/doxygen/classclang_1_1Expr.html;>Expr 
InnerMatcher
+Overload 
ignoringParens for Expr.
+
+Given
+  const char* str = ("my-string");
+The matcher
+  implicitCastExpr(hasSourceExpression(ignoringParens(stringLiteral(
+would match the implicit cast resulting from the assignment.
+
+
+
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1FieldDecl.html;>FieldDeclhasInClassInitializerMatcherhttps://clang.llvm.org/doxygen/classclang_1_1Expr.html;>Expr 
InnerMatcher
 Matches 
non-static data members that have an in-class initializer.
 


Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1147,6 +1147,14 @@
  parenExpr()));
 }
 
+TEST(ParenExpression, IgnoringParens) {
+  EXPECT_FALSE(matches("const char* str = (\"my-string\");",
+   implicitCastExpr(hasSourceExpression(stringLiteral();
+  EXPECT_TRUE(matches(
+  "const char* str = (\"my-string\");",
+  implicitCastExpr(hasSourceExpression(ignoringParens(stringLiteral());
+}
+
 TEST(TypeMatching, MatchesTypes) {
   EXPECT_TRUE(matches("struct S {};", qualType().bind("loc")));
 }
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -106,6 +106,7 @@

[PATCH] D54307: [clang] overload ignoringParens for Expr

2018-11-09 Thread Jonas Toth via Phabricator via cfe-commits

JonasToth added inline comments.



Comment at: include/clang/ASTMatchers/ASTMatchers.h:814
 /// would match the declaration for fp.
-AST_MATCHER_P(QualType, ignoringParens,
-  internal::Matcher, InnerMatcher) {
+AST_MATCHER_P_OVERLOAD(QualType, ignoringParens, internal::Matcher,
+   InnerMatcher, 0) {

aaron.ballman wrote:
> JonasToth wrote:
> > aaron.ballman wrote:
> > > Can you do this via `AST_POLYMORPHIC_MATCHER_P` instead, given that the 
> > > implementation is the same?
> > Do you want me to add more types? e.g. `TypeLoc` has `IgnoreParens()`, too. 
> I'd not be opposed, given that we already expose the `typeLoc()` matcher. 
> I'll leave that to your discretion.
as discussed on IRC making it an `AST_POLYMORPHIC_MATCHER_P` does not seem to 
work, as the polymorphism is only in the return type. We do need the `Node` 
itself to be polymorphic (same type as returntype). The only working version I 
got was using the overloads.


Repository:
  rC Clang

https://reviews.llvm.org/D54307



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

[PATCH] D54334: [AVR] Automatically link CRT and libgcc from the system avr-gcc

2018-11-09 Thread Dylan McKay via Phabricator via cfe-commits

dylanmckay created this revision.
dylanmckay added reviewers: aaron.ballman, kparzysz, asb, hfinkel.

This patch modifies the AVR toolchain so that if avr-gcc and avr-libc
are detected during compilation, the CRT, libgcc, libm, and libc anre
linked.

This matches avr-gcc's default behaviour, and the expected behaviour of
all C compilers - including the C runtime.

avr-gcc also needs a -mmcu specified in order to link runtime libraries.

The difference betwen this patch and avr-gcc is that this patch will
warn users whenever they compile without a runtime, as opposed to GCC,
which silently trims the runtime libs from the linker arguments when no
-mmcu is specified.


Repository:
  rC Clang

https://reviews.llvm.org/D54334

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Basic/DiagnosticGroups.td
  lib/Driver/ToolChains/AVR.cpp
  lib/Driver/ToolChains/AVR.h
  lib/Driver/ToolChains/Gnu.cpp
  test/Driver/avr-link-mcu-family-unimplemented.c
  test/Driver/avr-link-no-mcu-specified.c
  test/Driver/avr-link-nostdlib-nodefaultlibs.c

Index: test/Driver/avr-link-nostdlib-nodefaultlibs.c
===
--- /dev/null
+++ test/Driver/avr-link-nostdlib-nodefaultlibs.c
@@ -0,0 +1,8 @@
+// RUN: %clang -### -target avr -no-canonical-prefixes -save-temps -mmcu=atmega328 -nostdlib %s 2>&1 | FileCheck %s
+// RUN: %clang -### -target avr -no-canonical-prefixes -save-temps -mmcu=atmega328 -nodefaultlibs %s 2>&1 | FileCheck %s
+
+// nostdlib and nodefaultlibs programs should compile fine.
+
+// CHECK: main
+int main() { return 0; }
+
Index: test/Driver/avr-link-no-mcu-specified.c
===
--- /dev/null
+++ test/Driver/avr-link-no-mcu-specified.c
@@ -0,0 +1,10 @@
+// RUN: %clang -### -target avr -no-canonical-prefixes -save-temps %s 2>&1 | FileCheck --check-prefix=WARN %s
+// RUN: %clang -### -target avr -no-canonical-prefixes -save-temps -mmcu=atmega328 %s 2>&1 | FileCheck --check-prefix=NOWARN %s
+
+// WARN: warning: no target microcontroller specified on command line, cannot link standard libraries, please pass -mmcu=
+// WARN: warning: standard library not linked and so no interrupt vector table or compiler runtime routines will be linked
+
+// NOWARN: main
+
+int main() { return 0; }
+
Index: test/Driver/avr-link-mcu-family-unimplemented.c
===
--- /dev/null
+++ test/Driver/avr-link-mcu-family-unimplemented.c
@@ -0,0 +1,7 @@
+// RUN: %clang -### -target avr -no-canonical-prefixes -save-temps -mmcu=attiny13a %s 2>&1 | FileCheck --check-prefix=WARN %s
+
+// WARN: warning: support for linking stdlibs for microcontroller 'attiny13a' is not implemented, please file an AVR backend bug at http://bugs.llvm.org/ with your mcu name
+// WARN: warning: standard library not linked and so no interrupt vector table or compiler runtime routines will be linked
+
+int main() { return 0; }
+
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -1865,6 +1865,9 @@
   static const char *const ARMebHFTriples[] = {
   "armeb-linux-gnueabihf", "armebv7hl-redhat-linux-gnueabi"};
 
+  static const char *const AVRLibDirs[] = {"/lib"};
+  static const char *const AVRTriples[] = {"avr"};
+
   static const char *const X86_64LibDirs[] = {"/lib64", "/lib"};
   static const char *const X86_64Triples[] = {
   "x86_64-linux-gnu",   "x86_64-unknown-linux-gnu",
@@ -2077,6 +2080,10 @@
   TripleAliases.append(begin(ARMebTriples), end(ARMebTriples));
 }
 break;
+  case llvm::Triple::avr:
+LibDirs.append(begin(AVRLibDirs), end(AVRLibDirs));
+TripleAliases.append(begin(AVRTriples), end(AVRTriples));
+break;
   case llvm::Triple::x86_64:
 LibDirs.append(begin(X86_64LibDirs), end(X86_64LibDirs));
 TripleAliases.append(begin(X86_64Triples), end(X86_64Triples));
@@ -2205,6 +2212,8 @@
   return false;
   } else if (isRISCV(TargetArch)) {
 findRISCVMultilibs(D, TargetTriple, Path, Args, Detected);
+  } else if (TargetArch == llvm::Triple::avr) {
+// AVR has no multilibs.
   } else if (!findBiarchMultilibs(D, TargetTriple, Path, Args,
   NeedsBiarchSuffix, Detected)) {
 return false;
Index: lib/Driver/ToolChains/AVR.h
===
--- lib/Driver/ToolChains/AVR.h
+++ lib/Driver/ToolChains/AVR.h
@@ -20,26 +20,45 @@
 namespace toolchains {
 
 class LLVM_LIBRARY_VISIBILITY AVRToolChain : public Generic_ELF {
-protected:
-  Tool *buildLinker() const override;
 public:
   AVRToolChain(const Driver , const llvm::Triple ,
const llvm::opt::ArgList );
+protected:
+  Tool *buildLinker() const override;
+
+private:
+  /// Whether libgcc, libct, and friends should be linked.
+  ///
+  /// This is not done if the

[PATCH] D54325: [AST] Pack CXXBoolLiteralExpr, CXXNullPtrLiteralExpr and CXXThisExpr

2018-11-09 Thread Bruno Ricci via Phabricator via cfe-commits

riccibruno updated this revision to Diff 173383.
riccibruno added a comment.

Fix some typos in the added comments in `Stmt.h`


Repository:
  rC Clang

https://reviews.llvm.org/D54325

Files:
  include/clang/AST/ExprCXX.h
  include/clang/AST/Stmt.h

Index: include/clang/AST/Stmt.h
===
--- include/clang/AST/Stmt.h
+++ include/clang/AST/Stmt.h
@@ -516,18 +516,6 @@
 SourceLocation OpLoc;
   };
 
-  class ExprWithCleanupsBitfields {
-friend class ASTStmtReader; // deserialization
-friend class ExprWithCleanups;
-
-unsigned : NumExprBits;
-
-// When false, it must not have side effects.
-unsigned CleanupsHaveSideEffects : 1;
-
-unsigned NumObjects : 32 - 1 - NumExprBits;
-  };
-
   class ParenListExprBitfields {
 friend class ASTStmtReader;
 friend class ParenListExpr;
@@ -560,14 +548,6 @@
 unsigned IsUnique : 1;
   };
 
-  class ObjCIndirectCopyRestoreExprBitfields {
-friend class ObjCIndirectCopyRestoreExpr;
-
-unsigned : NumExprBits;
-
-unsigned ShouldCopy : 1;
-  };
-
   class InitListExprBitfields {
 friend class InitListExpr;
 
@@ -578,6 +558,44 @@
 unsigned HadArrayRangeDesignator : 1;
   };
 
+  //===--- C++ expression bitfields classes ---===//
+
+  class CXXBoolLiteralExprBitfields {
+friend class ASTStmtReader;
+friend class CXXBoolLiteralExpr;
+
+unsigned : NumExprBits;
+
+/// The value of the boolean literal.
+unsigned Value : 1;
+
+/// The location of the boolean literal.
+SourceLocation Loc;
+  };
+
+  class CXXNullPtrLiteralExprBitfields {
+friend class ASTStmtReader;
+friend class CXXNullPtrLiteralExpr;
+
+unsigned : NumExprBits;
+
+/// The location of the null pointer literal.
+SourceLocation Loc;
+  };
+
+  class CXXThisExprBitfields {
+friend class ASTStmtReader;
+friend class CXXThisExpr;
+
+unsigned : NumExprBits;
+
+/// Whether this is an implicit "this".
+unsigned IsImplicit : 1;
+
+/// The location of the "this".
+SourceLocation Loc;
+  };
+
   class TypeTraitExprBitfields {
 friend class ASTStmtReader;
 friend class ASTStmtWriter;
@@ -596,14 +614,38 @@
 unsigned NumArgs : 32 - 8 - 1 - NumExprBits;
   };
 
+  class ExprWithCleanupsBitfields {
+friend class ASTStmtReader;
+friend class ExprWithCleanups;
+
+unsigned : NumExprBits;
+
+/// When false, it must not have side effects.
+unsigned CleanupsHaveSideEffects : 1;
+
+unsigned NumObjects : 32 - 1 - NumExprBits;
+  };
+
+  //===--- C++ Coroutines TS expression bitfields classes ---===//
+
   class CoawaitExprBitfields {
 friend class CoawaitExpr;
 
 unsigned : NumExprBits;
 
 unsigned IsImplicit : 1;
   };
 
+  //===--- Obj-C expression bitfields classes ---===//
+
+  class ObjCIndirectCopyRestoreExprBitfields {
+friend class ObjCIndirectCopyRestoreExpr;
+
+unsigned : NumExprBits;
+
+unsigned ShouldCopy : 1;
+  };
+
   union {
 // Statements
 StmtBitfields StmtBits;
@@ -636,14 +678,23 @@
 MemberExprBitfields MemberExprBits;
 CastExprBitfields CastExprBits;
 BinaryOperatorBitfields BinaryOperatorBits;
-ExprWithCleanupsBitfields ExprWithCleanupsBits;
 ParenListExprBitfields ParenListExprBits;
 PseudoObjectExprBitfields PseudoObjectExprBits;
 OpaqueValueExprBitfields OpaqueValueExprBits;
-ObjCIndirectCopyRestoreExprBitfields ObjCIndirectCopyRestoreExprBits;
 InitListExprBitfields InitListExprBits;
+
+// C++ Expressions
+CXXBoolLiteralExprBitfields CXXBoolLiteralExprBits;
+CXXNullPtrLiteralExprBitfields CXXNullPtrLiteralExprBits;
+CXXThisExprBitfields CXXThisExprBits;
 TypeTraitExprBitfields TypeTraitExprBits;
+ExprWithCleanupsBitfields ExprWithCleanupsBits;
+
+// C++ Coroutines TS expressions
 CoawaitExprBitfields CoawaitBits;
+
+// Obj-C Expressions
+ObjCIndirectCopyRestoreExprBitfields ObjCIndirectCopyRestoreExprBits;
   };
 
 public:
Index: include/clang/AST/ExprCXX.h
===
--- include/clang/AST/ExprCXX.h
+++ include/clang/AST/ExprCXX.h
@@ -548,26 +548,25 @@
 
 /// A boolean literal, per ([C++ lex.bool] Boolean literals).
 class CXXBoolLiteralExpr : public Expr {
-  bool Value;
-  SourceLocation Loc;
-
 public:
-  CXXBoolLiteralExpr(bool val, QualType Ty, SourceLocation l)
+  CXXBoolLiteralExpr(bool Val, QualType Ty, SourceLocation Loc)
   : Expr(CXXBoolLiteralExprClass, Ty, VK_RValue, OK_Ordinary, false, false,
- false, false),
-Value(val), Loc(l) {}
+ false, false) {
+CXXBoolLiteralExprBits.Value = Val;
+CXXBoolLiteralExprBits.Loc = Loc;
+  }
 
   explicit CXXBoolLiteralExpr(EmptyShell Empty)
   : Expr(CXXBoolLiteralExprClass, Empty) {}
 
-  bool getValue() const { return Value; }
-  void setValue(bool V) { Value = V; }
+  bool getValue() const { return CXXBoolLiteralExprBits.Value; }
+

[PATCH] D54275: [HIP] Remove useless sections in linked files

2018-11-09 Thread Yaxun Liu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rC346536: [HIP] Remove useless sections in linked files 
(authored by yaxunl, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D54275

Files:
  lib/Driver/ToolChains/CommonArgs.cpp


Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -1436,6 +1436,10 @@
   LksStream << "PROVIDE_HIDDEN(__hip_fatbin = .);\n";
   LksStream << "" << BundleFileName << "\n";
   LksStream << "  }\n";
+  LksStream << "  /DISCARD/ :\n";
+  LksStream << "  {\n";
+  LksStream << "* ( __CLANG_OFFLOAD_BUNDLE__* )\n";
+  LksStream << "  }\n";
   LksStream << "}\n";
   LksStream << "INSERT BEFORE .data\n";
   LksStream.flush();


Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -1436,6 +1436,10 @@
   LksStream << "PROVIDE_HIDDEN(__hip_fatbin = .);\n";
   LksStream << "" << BundleFileName << "\n";
   LksStream << "  }\n";
+  LksStream << "  /DISCARD/ :\n";
+  LksStream << "  {\n";
+  LksStream << "* ( __CLANG_OFFLOAD_BUNDLE__* )\n";
+  LksStream << "  }\n";
   LksStream << "}\n";
   LksStream << "INSERT BEFORE .data\n";
   LksStream.flush();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r346536 - [HIP] Remove useless sections in linked files

2018-11-09 Thread Yaxun Liu via cfe-commits

Author: yaxunl
Date: Fri Nov  9 10:52:05 2018
New Revision: 346536

URL: http://llvm.org/viewvc/llvm-project?rev=346536=rev
Log:
[HIP] Remove useless sections in linked files

clang-offload-bundler creates __CLANG_OFFLOAD_BUNDLE__* sections in the bundles,
which get into the linked files. These sections are useless after linking. They 
waste disk
space and cause confusion for clang when directly linked with other object 
files, therefore
should be removed.

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

Modified:
cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp?rev=346536=346535=346536=diff
==
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp Fri Nov  9 10:52:05 2018
@@ -1436,6 +1436,10 @@ void tools::AddHIPLinkerScript(const Too
   LksStream << "PROVIDE_HIDDEN(__hip_fatbin = .);\n";
   LksStream << "" << BundleFileName << "\n";
   LksStream << "  }\n";
+  LksStream << "  /DISCARD/ :\n";
+  LksStream << "  {\n";
+  LksStream << "* ( __CLANG_OFFLOAD_BUNDLE__* )\n";
+  LksStream << "  }\n";
   LksStream << "}\n";
   LksStream << "INSERT BEFORE .data\n";
   LksStream.flush();


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

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

2018-11-09 Thread Jonas Toth via Phabricator via cfe-commits

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

LGTM.
Did you run this check in its final form against a bigger project? These 
results would be interesting.
Do you have commit access?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53974



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

[PATCH] D54246: [clang-tidy] Add the abseil-duration-factory-scale check

2018-11-09 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:36
+GetScaleForFactory(llvm::StringRef FactoryName) {
+  static const auto *ScaleMap =
+  new std::unordered_map(

hokein wrote:
> aaron.ballman wrote:
> > hwright wrote:
> > > Eugene.Zelenko wrote:
> > > > This will cause memory leaks, so may be unique_ptr should be used to 
> > > > hold pointer? May be LLVM ADT has better container?
> > > This is a tradeoff between leaking a small amount of known memory for the 
> > > duration of the program, and constructing this map every time this 
> > > function is invoked.  Since I expect the latter to occur frequently, 
> > > that's a tradeoff I think is acceptable.   (Ideally, this would be a 
> > > compile-time constant, but sadly we don't yet have a `constexpr` 
> > > dictionary type.)
> > > 
> > > Of course, if there is a more typical way of doing that here, I'm happy 
> > > to use it.
> > Why did you not use a static value type?
> I think main reason is lazy initialization.
Static locals are lazily initialized, so I'm still not certain why this 
shouldn't use `static const std::unorderd_map 
ScaleMap{...};` instead.



Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:46
+  const auto ScaleIter = ScaleMap->find(FactoryName);
+  if (ScaleIter == ScaleMap->end()) {
+return llvm::None;

Elide braces.



Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:57
+   const FloatingLiteral *FloatLit) {
+  if (IntLit) {
+return IntLit->getValue().getLimitedValue();

Elide braces.



Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:60
+  }
+  assert(FloatLit != nullptr);
+  return FloatLit->getValueAsApproximateDouble();

Please add a message to the assertion. e.g., `assert(foo && "bar");`



Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:179-181
+  functionDecl(hasAnyName("absl::Nanoseconds", 
"absl::Microseconds",
+  "absl::Milliseconds", "absl::Seconds",
+  "absl::Minutes", "absl::Hours"))

`::absl` instead -- otherwise this can trigger in unintended places.


https://reviews.llvm.org/D54246



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

[PATCH] D54307: [clang] overload ignoringParens for Expr

2018-11-09 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: include/clang/ASTMatchers/ASTMatchers.h:814
 /// would match the declaration for fp.
-AST_MATCHER_P(QualType, ignoringParens,
-  internal::Matcher, InnerMatcher) {
+AST_MATCHER_P_OVERLOAD(QualType, ignoringParens, internal::Matcher,
+   InnerMatcher, 0) {

JonasToth wrote:
> aaron.ballman wrote:
> > Can you do this via `AST_POLYMORPHIC_MATCHER_P` instead, given that the 
> > implementation is the same?
> Do you want me to add more types? e.g. `TypeLoc` has `IgnoreParens()`, too. 
I'd not be opposed, given that we already expose the `typeLoc()` matcher. I'll 
leave that to your discretion.


Repository:
  rC Clang

https://reviews.llvm.org/D54307



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

[PATCH] D54307: [clang] overload ignoringParens for Expr

2018-11-09 Thread Jonas Toth via Phabricator via cfe-commits

JonasToth added inline comments.



Comment at: include/clang/ASTMatchers/ASTMatchers.h:814
 /// would match the declaration for fp.
-AST_MATCHER_P(QualType, ignoringParens,
-  internal::Matcher, InnerMatcher) {
+AST_MATCHER_P_OVERLOAD(QualType, ignoringParens, internal::Matcher,
+   InnerMatcher, 0) {

aaron.ballman wrote:
> Can you do this via `AST_POLYMORPHIC_MATCHER_P` instead, given that the 
> implementation is the same?
Do you want me to add more types? e.g. `TypeLoc` has `IgnoreParens()`, too. 


Repository:
  rC Clang

https://reviews.llvm.org/D54307



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

[PATCH] D53995: [analyzer] Drastically simplify the tblgen files used for checkers

2018-11-09 Thread Umann Kristóf via Phabricator via cfe-commits

Szelethus added a comment.

Ping


Repository:
  rC Clang

https://reviews.llvm.org/D53995



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

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

2018-11-09 Thread Tamás Zolnai via Phabricator via cfe-commits

ztamas updated this revision to Diff 173372.
ztamas added a comment.

- Make local functions `static`


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53974

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
  clang-tidy/bugprone/TooSmallLoopVariableCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-too-small-loop-variable.cpp

Index: test/clang-tidy/bugprone-too-small-loop-variable.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-too-small-loop-variable.cpp
@@ -0,0 +1,227 @@
+// RUN: %check_clang_tidy %s bugprone-too-small-loop-variable %t
+
+long size() { return 294967296l; }
+
+
+/// Test cases correctly caught by bugprone-too-small-loop-variable.
+
+void voidBadForLoop() {
+  for (int i = 0; i < size(); ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+  }
+}
+
+void voidBadForLoop2() {
+  for (int i = 0; i < size() + 10; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+  }
+}
+
+void voidBadForLoop3() {
+  for (int i = 0; i <= size() - 1; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+  }
+}
+
+void voidBadForLoop4() {
+  for (int i = 0; size() > i; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+  }
+}
+
+void voidBadForLoop5() {
+  for (int i = 0; size() - 1 >= i; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+  }
+}
+
+void voidBadForLoop6() {
+  int i = 0;
+  for (; i < size(); ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+  }
+}
+
+void voidForLoopUnsignedBound() {
+  unsigned size = 3147483647;
+  for (int i = 0; i < size; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has narrower type 'int' than iteration's upper bound 'unsigned int' [bugprone-too-small-loop-variable]
+  }
+}
+
+// The iteration's upper bound has a template dependent value.
+template 
+void doSomething() {
+  for (short i = 0; i < size; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: loop variable has narrower type 'short' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+  }
+}
+
+// The iteration's upper bound has a template dependent type.
+template 
+void doSomething() {
+  for (T i = 0; i < size(); ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: loop variable has narrower type 'short' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+  }
+}
+
+void voidForLoopInstantiation() {
+  // This line does not trigger the warning.
+  doSomething();
+  // This one triggers the warning.
+  doSomething();
+}
+
+
+/// Correct loops: we should not warn here.
+
+// A simple use case when both expressions have the same type.
+void voidGoodForLoop() {
+  for (long i = 0; i < size(); ++i) { // no warning
+  }
+}
+
+// Other use case where both expressions have the same type,
+// but short expressions are converted to int by the compare operator.
+void voidGoodForLoop2() {
+  short loopCond = 10;
+  for (short i = 0; i < loopCond; ++i) { // no warning
+  }
+}
+
+// Because of the integer literal, the iteration's upper bound is int, but we suppress the warning here.
+void voidForLoopShortPlusLiteral() {
+  short size = 3;
+  for (short i = 0; i <= (size - 1); ++i) { // no warning
+  }
+}
+
+// Addition of two short variables results in an int value, but we suppress this to avoid false positives.
+void voidForLoopShortPlusShort() {
+  short size = 256;
+  short increment = 14;
+  for (short i = 0; i < size + increment; ++i) { // no warning
+  }
+}
+
+// In this test case we have different integer types, but here the loop variable has the bigger type.
+// The iteration's bound is cast implicitly, not the loop variable.
+void voidForLoopBoundImplicitCast() {
+  short start = 256;
+  short end = 14;
+  for (int i = start; i >= end; --i) { // no warning
+  }
+}
+
+// Range based loop and other iterator based loops are ignored by this check.
+void voidRangeBasedForLoop() {
+  int array[] = {1, 2, 3, 4, 5};
+  for (const int  :

[PATCH] D53488: [clang-tidy] Improving narrowing conversions

2018-11-09 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:108-113
+  if (T.isSignedInteger())
+return {llvm::APSInt::getMinValue(Context.getTypeSize(), false),
+llvm::APSInt::getMaxValue(Context.getTypeSize(), false)};
+  assert(T.isUnsignedInteger() && "Unhandled BuiltinType");
+  return {llvm::APSInt::getMinValue(Context.getTypeSize(), true),
+  llvm::APSInt::getMaxValue(Context.getTypeSize(), true)};

This can be further simplified into:
```
  assert(T.isInteger() && "Unexpected builtin type");
  return {llvm::APSInt::getMinValue(Context.getTypeSize(), 
T.isUnsignedInteger()),
   llvm::APSInt::getMaxValue(Context.getTypeSize(), 
T.isUnsignedInteger())};
```



Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:313
+// We create variables so we make sure both sides of the
+// ConditionalOperator are evaluated, the || operator would shortciruit
+// the second call otherwise.

shortciruit -> short circuit


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53488



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

[PATCH] D54330: Driver: Make -fsanitize=shadow-call-stack compatible with -fsanitize-minimal-runtime.

2018-11-09 Thread Peter Collingbourne via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL346526: Driver: Make -fsanitize=shadow-call-stack compatible 
with -fsanitize-minimal… (authored by pcc, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D54330?vs=173367=173368#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D54330

Files:
  cfe/trunk/lib/Driver/SanitizerArgs.cpp
  cfe/trunk/test/Driver/fsanitize.c


Index: cfe/trunk/lib/Driver/SanitizerArgs.cpp
===
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp
@@ -47,7 +47,7 @@
   TrappingDefault = CFI,
   CFIClasses =
   CFIVCall | CFINVCall | CFIMFCall | CFIDerivedCast | CFIUnrelatedCast,
-  CompatibleWithMinimalRuntime = TrappingSupported | Scudo,
+  CompatibleWithMinimalRuntime = TrappingSupported | Scudo | ShadowCallStack,
 };
 
 enum CoverageFeature {
Index: cfe/trunk/test/Driver/fsanitize.c
===
--- cfe/trunk/test/Driver/fsanitize.c
+++ cfe/trunk/test/Driver/fsanitize.c
@@ -767,6 +767,10 @@
 // CHECK-CFI-NOICALL-MINIMAL: 
"-fsanitize-trap=cfi-derived-cast,cfi-mfcall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall"
 // CHECK-CFI-NOICALL-MINIMAL: "-fsanitize-minimal-runtime"
 
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=shadow-call-stack 
-fsanitize-minimal-runtime %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-SCS-MINIMAL
+// CHECK-SCS-MINIMAL: "-fsanitize=shadow-call-stack"
+// CHECK-SCS-MINIMAL: "-fsanitize-minimal-runtime"
+
 // RUN: %clang -target aarch64-linux-gnu -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target arm-linux-androideabi -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SCUDO


Index: cfe/trunk/lib/Driver/SanitizerArgs.cpp
===
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp
@@ -47,7 +47,7 @@
   TrappingDefault = CFI,
   CFIClasses =
   CFIVCall | CFINVCall | CFIMFCall | CFIDerivedCast | CFIUnrelatedCast,
-  CompatibleWithMinimalRuntime = TrappingSupported | Scudo,
+  CompatibleWithMinimalRuntime = TrappingSupported | Scudo | ShadowCallStack,
 };
 
 enum CoverageFeature {
Index: cfe/trunk/test/Driver/fsanitize.c
===
--- cfe/trunk/test/Driver/fsanitize.c
+++ cfe/trunk/test/Driver/fsanitize.c
@@ -767,6 +767,10 @@
 // CHECK-CFI-NOICALL-MINIMAL: "-fsanitize-trap=cfi-derived-cast,cfi-mfcall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall"
 // CHECK-CFI-NOICALL-MINIMAL: "-fsanitize-minimal-runtime"
 
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=shadow-call-stack -fsanitize-minimal-runtime %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCS-MINIMAL
+// CHECK-SCS-MINIMAL: "-fsanitize=shadow-call-stack"
+// CHECK-SCS-MINIMAL: "-fsanitize-minimal-runtime"
+
 // RUN: %clang -target aarch64-linux-gnu -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target arm-linux-androideabi -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r346526 - Driver: Make -fsanitize=shadow-call-stack compatible with -fsanitize-minimal-runtime.

2018-11-09 Thread Peter Collingbourne via cfe-commits

Author: pcc
Date: Fri Nov  9 09:54:49 2018
New Revision: 346526

URL: http://llvm.org/viewvc/llvm-project?rev=346526=rev
Log:
Driver: Make -fsanitize=shadow-call-stack compatible with 
-fsanitize-minimal-runtime.

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

Modified:
cfe/trunk/lib/Driver/SanitizerArgs.cpp
cfe/trunk/test/Driver/fsanitize.c

Modified: cfe/trunk/lib/Driver/SanitizerArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/SanitizerArgs.cpp?rev=346526=346525=346526=diff
==
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp (original)
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp Fri Nov  9 09:54:49 2018
@@ -47,7 +47,7 @@ enum : SanitizerMask {
   TrappingDefault = CFI,
   CFIClasses =
   CFIVCall | CFINVCall | CFIMFCall | CFIDerivedCast | CFIUnrelatedCast,
-  CompatibleWithMinimalRuntime = TrappingSupported | Scudo,
+  CompatibleWithMinimalRuntime = TrappingSupported | Scudo | ShadowCallStack,
 };
 
 enum CoverageFeature {

Modified: cfe/trunk/test/Driver/fsanitize.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fsanitize.c?rev=346526=346525=346526=diff
==
--- cfe/trunk/test/Driver/fsanitize.c (original)
+++ cfe/trunk/test/Driver/fsanitize.c Fri Nov  9 09:54:49 2018
@@ -767,6 +767,10 @@
 // CHECK-CFI-NOICALL-MINIMAL: 
"-fsanitize-trap=cfi-derived-cast,cfi-mfcall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall"
 // CHECK-CFI-NOICALL-MINIMAL: "-fsanitize-minimal-runtime"
 
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=shadow-call-stack 
-fsanitize-minimal-runtime %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-SCS-MINIMAL
+// CHECK-SCS-MINIMAL: "-fsanitize=shadow-call-stack"
+// CHECK-SCS-MINIMAL: "-fsanitize-minimal-runtime"
+
 // RUN: %clang -target aarch64-linux-gnu -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target arm-linux-androideabi -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SCUDO


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

[PATCH] D54330: Driver: Make -fsanitize=shadow-call-stack compatible with -fsanitize-minimal-runtime.

2018-11-09 Thread Evgenii Stepanov via Phabricator via cfe-commits

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

LGTM


Repository:
  rL LLVM

https://reviews.llvm.org/D54330



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

[PATCH] D54258: [Clang] Fix pretty printing of CUDA address spaces

2018-11-09 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In https://reviews.llvm.org/D54258#1292129, @richardmembarth wrote:

> I think it's not so easy to provide such tests for CUDA.
>  CUDA memory space specifiers are implemented via attributes, e.g. `#define 
> __shared__ __attribute__((shared))`.
>  As a result of this, they are pretty-printed via a different code path.
>  In my example, I call `Ctx.getAddrSpaceQualType(QT, LangAS::cuda_shared)`, 
> which is then pretty-printed via the code above.
>  Any hints how to provide tests for this one?


If there's no way to trigger a different spelling that a user would see, why is 
this change needed?


Repository:
  rC Clang

https://reviews.llvm.org/D54258



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

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

2018-11-09 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

In https://reviews.llvm.org/D52674#1291284, @smeenai wrote:

> In https://reviews.llvm.org/D52674#1281747, @rjmccall wrote:
>
> > In https://reviews.llvm.org/D52674#1271814, @smeenai wrote:
> >
> > > @rjmccall I prototyped the ForRTTI parameter approach in 
> > > https://reviews.llvm.org/D53546. It could definitely be cleaned up a bit, 
> > > but it demonstrates the problems I saw with the added parameter. Namely, 
> > > `mangleType(QualType, SourceRange, QualifierMangleMode)` has a bunch of 
> > > additional logic which is needed for correctness, so we need to thread 
> > > the parameter through the entire chain, and the only way I could think of 
> > > doing that without adding the parameter to every single `mangleType` 
> > > overload was to have an additional switch to dispatch to the overloads 
> > > with the added ForRTTI parameter, which is pretty ugly.
> > >
> > > As I see it, there are a few ways to proceed here:
> > >
> > > - The approach in https://reviews.llvm.org/D53546 (cleaned up a bit). I 
> > > think it's pretty ugly, but you may have suggestions on how to do it 
> > > better.
> > > - In the `mangleType` overload for `ObjCObjectPointerType`, skipping the 
> > > generic `mangleType` dispatch and going directly to either the 
> > > `ObjCObjectType` or `ObjCInterfaceType` overloads. That avoids the 
> > > ugliness in the generic `mangleType`, but it also requires us to 
> > > duplicate some logic from it in the `ObjCObjectPointerType` overload, 
> > > which doesn't seem great either.
> > > - Maintaining `ForRTTI` state in the mangler instead of threading a 
> > > parameter through (I'm generally not a fan of state like that, but it 
> > > might be cleaner than the threading in this case?)
> > > - Just sticking with what I'm doing in this patch.
> > >
> > >   What do you think?
> >
> >
> > Sorry for the delay.  I think duplicating the dispatch logic for 
> > `ObjCObjectPointerType` is probably the best approach; the pointee type 
> > will always an `ObjCObjectType` of some sort, and there are only two kinds 
> > of those.
>
>
> Sorry, I'm still not sure how this will work.
>
> Duplicating the dispatch logic for `ObjCObjectPointerType` ends up looking 
> like https://reviews.llvm.org/P8114, which is fine. However, when we're 
> actually mangling RTTI or RTTI names, we're still going through the main 
> `mangleType(QualType, SourceRange, QualifierMangleMode)` overload, which 
> still requires us to thread `ForRTTI` through that function, which still 
> requires us to duplicate the switch in that function (to handle the `ForRTTI` 
> case, since the other switch is generated via TypeNodes.def and not easily 
> modifiable), which is the main ugliness in my opinion. Do you also want me to 
> add special dispatching to `mangleCXXRTTI` and `mangleCXXRTTIName` to just 
> call the `ObjCObjectPointerType` function directly when they're dealing with 
> that type? That's certainly doable, but at that point just keeping some state 
> around in the demangler starts to feel cleaner, at least to me.


Well, you could check for an `ObjCObjectPointerType` in the top-level routine.

But yeah, probably the cleanest thing to do would to be push the state through 
the main dispatch.  Don't push a single `bool` through, though; make a 
`TypeManglingOptions` struct to encapsulate it, in case we have a similar 
problem elsewhere.  It should have a method for getting options that should be 
propagated down to component type manglings, and that method can drop the "for 
RTTI" bit; that's the part that `ObjCObjectPointerType` can handle differently.


Repository:
  rC Clang

https://reviews.llvm.org/D52674



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

[PATCH] D54330: Driver: Make -fsanitize=shadow-call-stack compatible with -fsanitize-minimal-runtime.

2018-11-09 Thread Peter Collingbourne via Phabricator via cfe-commits

pcc created this revision.
pcc added a reviewer: eugenis.
Herald added subscribers: llvm-commits, cryptoad.

Repository:
  rL LLVM

https://reviews.llvm.org/D54330

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/Driver/fsanitize.c


Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -767,6 +767,10 @@
 // CHECK-CFI-NOICALL-MINIMAL: 
"-fsanitize-trap=cfi-derived-cast,cfi-mfcall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall"
 // CHECK-CFI-NOICALL-MINIMAL: "-fsanitize-minimal-runtime"
 
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=shadow-call-stack 
-fsanitize-minimal-runtime %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-SCS-MINIMAL
+// CHECK-SCS-MINIMAL: "-fsanitize=shadow-call-stack"
+// CHECK-SCS-MINIMAL: "-fsanitize-minimal-runtime"
+
 // RUN: %clang -target aarch64-linux-gnu -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target arm-linux-androideabi -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=scudo %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-SCUDO
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -47,7 +47,7 @@
   TrappingDefault = CFI,
   CFIClasses =
   CFIVCall | CFINVCall | CFIMFCall | CFIDerivedCast | CFIUnrelatedCast,
-  CompatibleWithMinimalRuntime = TrappingSupported | Scudo,
+  CompatibleWithMinimalRuntime = TrappingSupported | Scudo | ShadowCallStack,
 };
 
 enum CoverageFeature {


Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -767,6 +767,10 @@
 // CHECK-CFI-NOICALL-MINIMAL: "-fsanitize-trap=cfi-derived-cast,cfi-mfcall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall"
 // CHECK-CFI-NOICALL-MINIMAL: "-fsanitize-minimal-runtime"
 
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=shadow-call-stack -fsanitize-minimal-runtime %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCS-MINIMAL
+// CHECK-SCS-MINIMAL: "-fsanitize=shadow-call-stack"
+// CHECK-SCS-MINIMAL: "-fsanitize-minimal-runtime"
+
 // RUN: %clang -target aarch64-linux-gnu -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target arm-linux-androideabi -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -47,7 +47,7 @@
   TrappingDefault = CFI,
   CFIClasses =
   CFIVCall | CFINVCall | CFIMFCall | CFIDerivedCast | CFIUnrelatedCast,
-  CompatibleWithMinimalRuntime = TrappingSupported | Scudo,
+  CompatibleWithMinimalRuntime = TrappingSupported | Scudo | ShadowCallStack,
 };
 
 enum CoverageFeature {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53764: [OpenCL] Enable address spaces for references in C++

2018-11-09 Thread John McCall via Phabricator via cfe-commits

rjmccall added inline comments.



Comment at: lib/AST/Expr.cpp:1609
   case CK_AddressSpaceConversion:
-assert(getType()->isPointerType() || getType()->isBlockPointerType());
-assert(getSubExpr()->getType()->isPointerType() ||
-   getSubExpr()->getType()->isBlockPointerType());
-assert(getType()->getPointeeType().getAddressSpace() !=
-   getSubExpr()->getType()->getPointeeType().getAddressSpace());
-LLVM_FALLTHROUGH;
+assert(/*If pointer type then addr spaces for pointees must differ*/
+   (((getType()->isPointerType() &&

rjmccall wrote:
> rjmccall wrote:
> > Anastasia wrote:
> > > I don't like this assert now. Would adding extra variable be cleaner here?
> > Yeah, this assertion doesn't make any sense like this.  It should be 
> > checking whether the cast is a gl-value and, if so, requiring the 
> > subexpression to also be a gl-value and then asserting the difference 
> > between the type.  But you can certainly do an address-space conversion on 
> > l-values that just happen to be of pointer or block-pointer type.
> No, if this is a gl-value cast, the assertion must ignore whether there's a 
> pointee type, or it will be messed up on gl-values of pointer types.
> 
> That is, if I have a gl-value of type `char * __private`, I should be able to 
> do an address-space promotion to get a gl-value of type `char * __generic`.  
> It's okay that the pointers are into the same address space here — in fact, 
> it's more than okay, it's necessary.
Thanks, that's right now.  Although please assert that the base has the same 
value kind; I've seen bugs before where ICEs tried to implicitly materialize 
their arguments, and it's really frustrating to root out.



Comment at: lib/Sema/SemaExprCXX.cpp:4285
+? CK_AddressSpaceConversion
+: CK_NoOp;
+

If `ToType` is a reference type, the address space will be on its pointee type.


https://reviews.llvm.org/D53764



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

[clang-tools-extra] r346524 - [clangd] Fix clang-tidy warnings.

2018-11-09 Thread Kadir Cetinkaya via cfe-commits

Author: kadircet
Date: Fri Nov  9 09:33:48 2018
New Revision: 346524

URL: http://llvm.org/viewvc/llvm-project?rev=346524=rev
Log:
[clangd] Fix clang-tidy warnings.

Modified:
clang-tools-extra/trunk/unittests/clangd/TestTU.cpp

Modified: clang-tools-extra/trunk/unittests/clangd/TestTU.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TestTU.cpp?rev=346524=346523=346524=diff
==
--- clang-tools-extra/trunk/unittests/clangd/TestTU.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/TestTU.cpp Fri Nov  9 09:33:48 2018
@@ -41,7 +41,7 @@ ParsedAST TestTU::build() const {
   auto Preamble =
   buildPreamble(FullFilename, *createInvocationFromCommandLine(Cmd),
 /*OldPreamble=*/nullptr,
-/*OldCommand=*/Inputs.CompileCommand, Inputs, PCHs,
+/*OldCompileCommand=*/Inputs.CompileCommand, Inputs, PCHs,
 /*StoreInMemory=*/true, /*PreambleCallback=*/nullptr);
   auto AST = buildAST(FullFilename, createInvocationFromCommandLine(Cmd),
   Inputs, Preamble, PCHs);
@@ -109,17 +109,17 @@ const NamedDecl (ParsedAST 
 }
 
 const NamedDecl (ParsedAST ,
-  std::function Callback) {
+  std::function Filter) {
   struct Visitor : RecursiveASTVisitor {
-decltype(Callback) CB;
+decltype(Filter) F;
 SmallVector Decls;
 bool VisitNamedDecl(const NamedDecl *ND) {
-  if (CB(*ND))
+  if (F(*ND))
 Decls.push_back(ND);
   return true;
 }
   } Visitor;
-  Visitor.CB = Callback;
+  Visitor.F = Filter;
   Visitor.TraverseDecl(AST.getASTContext().getTranslationUnitDecl());
   if (Visitor.Decls.size() != 1) {
 ADD_FAILURE() << Visitor.Decls.size() << " symbols matched.";


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

[PATCH] D54310: Make clang-based tools find libc++ on MacOS

2018-11-09 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

In https://reviews.llvm.org/D54310#1292924, @sammccall wrote:

> If you know of a good reviewer for this, you may want a second opinion!


This would definitely be nice, but I also don't know who'd be the proper person 
to review this.
I'll take a pause until Monday, maybe @arphaman or someone else from the 
community can suggest a better way of fixing this or proper reviewers for this 
change.




Comment at: include/clang/Lex/HeaderSearchOptions.h:111
 
+  /// Compiler install dir as detected by the Driver.
+  /// Only used to add include dirs for libc++ on Darwin. Please avoid relying

sammccall wrote:
> Could you add "typically contains bin/clang, lib/libclang_rt, and 
> include/"?
This is actually the 'bin' subdirectory, I've added a comment about that but 
you see a better way to communicate this, please let me know! E.g. actually 
mentioning the parallel lib/ and include/ subdirs. 



Comment at: lib/Frontend/CompilerInvocation.cpp:1776
   Opts.ResourceDir = Args.getLastArgValue(OPT_resource_dir);
+  Opts.InstallDir = Opts.ResourceDir;
 

sammccall wrote:
> (is this needed? I guess you fall back to this if you don't create the CI 
> from the command line?)
Thanks for noticing this!
This is a leftover from a previous iteration, it's actually incorrect now since 
we rely on `InstallDir` pointing to a different place from the resource dir.
Removed it.


Repository:
  rC Clang

https://reviews.llvm.org/D54310



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

[PATCH] D54310: Make clang-based tools find libc++ on MacOS

2018-11-09 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov updated this revision to Diff 173364.
ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added a comment.

- Update the comment.


Repository:
  rC Clang

https://reviews.llvm.org/D54310

Files:
  include/clang/Lex/HeaderSearchOptions.h
  lib/Frontend/CreateInvocationFromCommandLine.cpp
  lib/Frontend/InitHeaderSearch.cpp
  lib/Tooling/Tooling.cpp
  test/Tooling/Inputs/mock-libcxx/include/c++/v1/mock_vector
  test/Tooling/clang-check-mac-libcxx.cpp

Index: test/Tooling/clang-check-mac-libcxx.cpp
===
--- /dev/null
+++ test/Tooling/clang-check-mac-libcxx.cpp
@@ -0,0 +1,16 @@
+// Clang on MacOS can find libc++ living beside the installed compiler.
+// This test makes sure our libTooling-based tools emulate this properly.
+//
+// RUN: rm -rf %t
+// RUN: mkdir %t
+//
+// Install the mock libc++ (simulates the libc++ directory structure).
+// RUN: cp -r %S/Inputs/mock-libcxx %t/
+//
+// Pretend clang is installed beside the mock library that we provided.
+// RUN: echo '[{"directory":"%t","command":"%t/mock-libcxx/bin/clang++ -stdlib=libc++ -target x86_64-apple-darwin -c test.cpp","file":"test.cpp"}]' | sed -e 's/\\/\//g' > %t/compile_commands.json
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-check -p "%t" "%t/test.cpp"
+
+#include 
+vector v;
Index: test/Tooling/Inputs/mock-libcxx/include/c++/v1/mock_vector
===
--- /dev/null
+++ test/Tooling/Inputs/mock-libcxx/include/c++/v1/mock_vector
@@ -0,0 +1 @@
+class vector {};
Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -323,6 +323,9 @@
 Invocation->getPreprocessorOpts().addRemappedFile(It.getKey(),
   Input.release());
   }
+  // Patch up the install dir, so we find the same standard library as the
+  // original compiler on MacOS.
+  Invocation->getHeaderSearchOpts().InstallDir = Driver->getInstalledDir();
   return runInvocation(BinaryName, Compilation.get(), std::move(Invocation),
std::move(PCHContainerOps));
 }
Index: lib/Frontend/InitHeaderSearch.cpp
===
--- lib/Frontend/InitHeaderSearch.cpp
+++ lib/Frontend/InitHeaderSearch.cpp
@@ -476,14 +476,9 @@
   if (triple.isOSDarwin()) {
 // On Darwin, libc++ may be installed alongside the compiler in
 // include/c++/v1.
-if (!HSOpts.ResourceDir.empty()) {
-  // Remove version from foo/lib/clang/version
-  StringRef NoVer = llvm::sys::path::parent_path(HSOpts.ResourceDir);
-  // Remove clang from foo/lib/clang
-  StringRef Lib = llvm::sys::path::parent_path(NoVer);
-  // Remove lib from foo/lib
-  SmallString<128> P = llvm::sys::path::parent_path(Lib);
-
+if (!HSOpts.InstallDir.empty()) {
+  // Get from foo/bin to foo.
+  SmallString<128> P(llvm::sys::path::parent_path(HSOpts.InstallDir));
   // Get foo/include/c++/v1
   llvm::sys::path::append(P, "include", "c++", "v1");
   AddUnmappedPath(P, CXXSystem, false);
Index: lib/Frontend/CreateInvocationFromCommandLine.cpp
===
--- lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -11,17 +11,18 @@
 //
 //===--===//
 
-#include "clang/Frontend/Utils.h"
 #include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Driver/Action.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
-#include "clang/Driver/Action.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/Tool.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
+#include "clang/Frontend/Utils.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/Host.h"
+#include "llvm/Support/Path.h"
 using namespace clang;
 using namespace llvm::opt;
 
@@ -102,5 +103,8 @@
  CCArgs.size(),
  *Diags))
 return nullptr;
+  // Patch up the install dir, so we find the same standard library as the
+  // original compiler on MacOS.
+  CI->getHeaderSearchOpts().InstallDir = TheDriver.getInstalledDir();
   return CI;
 }
Index: include/clang/Lex/HeaderSearchOptions.h
===
--- include/clang/Lex/HeaderSearchOptions.h
+++ include/clang/Lex/HeaderSearchOptions.h
@@ -108,6 +108,13 @@
   /// etc.).
   std::string ResourceDir;
 
+  /// Compiler install dir as detected by the Driver.
+  /// This is typically the directory that contains the clang executable, i.e.
+  /// the 'bin/' subdir of a clang distribution.
+  /// Only used to

[PATCH] D53700: Support _Clang as a scoped attribute identifier

2018-11-09 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman closed this revision.
aaron.ballman added a comment.

Thanks for the reviews -- I've commit in r346521.


https://reviews.llvm.org/D53700



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

r346521 - Introduce the _Clang scoped attribute token.

2018-11-09 Thread Aaron Ballman via cfe-commits

Author: aaronballman
Date: Fri Nov  9 09:19:45 2018
New Revision: 346521

URL: http://llvm.org/viewvc/llvm-project?rev=346521=rev
Log:
Introduce the _Clang scoped attribute token.

Currently, we only accept clang as the scoped attribute identifier for double 
square bracket attributes provided by Clang, but this has the potential to 
conflict with user-defined macros. To help alleviate these concerns, this 
introduces the _Clang scoped attribute identifier as an alias for clang. It 
also introduces a warning with a fixit on the off chance someone attempts to 
use __clang__ as the scoped attribute (which is a predefined compiler 
identification macro).

Modified:
cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
cfe/trunk/lib/Basic/Attributes.cpp
cfe/trunk/lib/Parse/ParseDeclCXX.cpp
cfe/trunk/lib/Sema/ParsedAttr.cpp
cfe/trunk/test/FixIt/fixit-cxx11-attributes.cpp
cfe/trunk/test/Parser/cxx0x-attributes.cpp
cfe/trunk/test/Preprocessor/has_attribute.cpp
cfe/trunk/test/SemaCXX/attr-optnone.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=346521=346520=346521=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Fri Nov  9 09:19:45 
2018
@@ -575,6 +575,9 @@ def warn_cxx98_compat_noexcept_expr : Wa
 def warn_cxx98_compat_nullptr : Warning<
   "'nullptr' is incompatible with C++98">, InGroup, DefaultIgnore;
 
+def warn_wrong_clang_attr_namespace : Warning<
+  "'__clang__' is a predefined macro name, not an attribute scope specifier; "
+  "did you mean '_Clang' instead?">, InGroup;
 def ext_ns_enum_attribute : Extension<
   "attributes on %select{a namespace|an enumerator}0 declaration are "
   "a C++17 extension">, InGroup;

Modified: cfe/trunk/lib/Basic/Attributes.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Attributes.cpp?rev=346521=346520=346521=diff
==
--- cfe/trunk/lib/Basic/Attributes.cpp (original)
+++ cfe/trunk/lib/Basic/Attributes.cpp Fri Nov  9 09:19:45 2018
@@ -9,16 +9,18 @@ int clang::hasAttribute(AttrSyntax Synta
 const LangOptions ) {
   StringRef Name = Attr->getName();
   // Normalize the attribute name, __foo__ becomes foo.
-  if (Name.size() >= 4 && Name.startswith("__") && Name.endswith("__"))
-Name = Name.substr(2, Name.size() - 4);
-
-  // Normalize the scope name, but only for gnu attributes.
-  StringRef ScopeName = Scope ? Scope->getName() : "";
-  if (ScopeName == "__gnu__")
-ScopeName = ScopeName.slice(2, ScopeName.size() - 2);
-
-#include "clang/Basic/AttrHasAttributeImpl.inc"
-
+  if (Name.size() >= 4 && Name.startswith("__") && Name.endswith("__"))
+Name = Name.substr(2, Name.size() - 4);
+
+  // Normalize the scope name, but only for gnu and clang attributes.
+  StringRef ScopeName = Scope ? Scope->getName() : "";
+  if (ScopeName == "__gnu__")
+ScopeName = "gnu";
+  else if (ScopeName == "_Clang")
+ScopeName = "clang";
+
+#include "clang/Basic/AttrHasAttributeImpl.inc"
+
   return 0;
 }
 

Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=346521=346520=346521=diff
==
--- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Fri Nov  9 09:19:45 2018
@@ -3790,6 +3790,28 @@ IdentifierInfo *Parser::TryParseCXX11Att
 }
 return nullptr;
 
+  case tok::numeric_constant: {
+// If we got a numeric constant, check to see if it comes from a macro that
+// corresponds to the predefined __clang__ macro. If it does, warn the user
+// and recover by pretending they said _Clang instead.
+if (Tok.getLocation().isMacroID()) {
+  SmallString<8> ExpansionBuf;
+  SourceLocation ExpansionLoc =
+  PP.getSourceManager().getExpansionLoc(Tok.getLocation());
+  StringRef Spelling = PP.getSpelling(ExpansionLoc, ExpansionBuf);
+  if (Spelling == "__clang__") {
+SourceRange TokRange(
+ExpansionLoc,
+PP.getSourceManager().getExpansionLoc(Tok.getEndLoc()));
+Diag(Tok, diag::warn_wrong_clang_attr_namespace)
+<< FixItHint::CreateReplacement(TokRange, "_Clang");
+Loc = ConsumeToken();
+return ().get("_Clang");
+  }
+}
+return nullptr;
+  }
+
   case tok::ampamp:   // 'and'
   case tok::pipe: // 'bitor'
   case tok::pipepipe: // 'or'
@@ -3865,24 +3887,22 @@ bool Parser::ParseCXX11AttributeArgs(Ide
 // Eat the left paren, then skip to the ending right paren.
 ConsumeParen();
 SkipUntil(tok::r_paren);
-return false;
-  }
-
-  if

r346520 - Use the correct address space when emitting the ctor function list

2018-11-09 Thread Dylan McKay via cfe-commits

Author: dylanmckay
Date: Fri Nov  9 09:15:06 2018
New Revision: 346520

URL: http://llvm.org/viewvc/llvm-project?rev=346520=rev
Log:
Use the correct address space when emitting the ctor function list

This patch modifies clang so that, if compiling for a target that
explicitly specifies a nonzero program memory address space, the
constructor list global will have the same address space as the
functions it contains.

AVR is the only in-tree backend which has a nonzero program memory
address space.

Without this, the IR verifier would always fail if a constructor
was used on a Harvard architecture backend.

This has no functional change to any in-tree backends except AVR.

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

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=346520=346519=346520=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Fri Nov  9 09:15:06 2018
@@ -1105,11 +1105,12 @@ void CodeGenModule::EmitCtorList(CtorLis
 
   // Ctor function type is void()*.
   llvm::FunctionType* CtorFTy = llvm::FunctionType::get(VoidTy, false);
-  llvm::Type *CtorPFTy = llvm::PointerType::getUnqual(CtorFTy);
+  llvm::Type *CtorPFTy = llvm::PointerType::get(CtorFTy,
+  TheModule.getDataLayout().getProgramAddressSpace());
 
   // Get the type of a ctor entry, { i32, void ()*, i8* }.
   llvm::StructType *CtorStructTy = llvm::StructType::get(
-  Int32Ty, llvm::PointerType::getUnqual(CtorFTy), VoidPtrTy);
+  Int32Ty, CtorPFTy, VoidPtrTy);
 
   // Construct the constructor and destructor arrays.
   ConstantInitBuilder builder(*this);


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

[PATCH] D54326: [AST] Pack CXXThrowExpr, CXXDefaultArgExpr and CXXDefaultInitExpr

2018-11-09 Thread Bruno Ricci via Phabricator via cfe-commits

riccibruno created this revision.
riccibruno added a reviewer: rsmith.
riccibruno added a project: clang.
Herald added a subscriber: cfe-commits.
riccibruno added a dependency: D54325: [AST] Pack CXXBoolLiteralExpr, 
CXXNullPtrLiteralExpr and CXXThisExpr.

Use the newly available space in the bit-fields of `Stmt` to store
some data from `CXXThrowExpr`, `CXXDefaultArgExpr`
and `CXXDefaultInitExpr`. This saves 1 pointer for each of these classes.


Repository:
  rC Clang

https://reviews.llvm.org/D54326

Files:
  include/clang/AST/ExprCXX.h
  include/clang/AST/Stmt.h
  lib/AST/ExprCXX.cpp
  lib/Serialization/ASTReaderStmt.cpp

Index: lib/Serialization/ASTReaderStmt.cpp
===
--- lib/Serialization/ASTReaderStmt.cpp
+++ lib/Serialization/ASTReaderStmt.cpp
@@ -1480,21 +1480,21 @@
 
 void ASTStmtReader::VisitCXXThrowExpr(CXXThrowExpr *E) {
   VisitExpr(E);
-  E->ThrowLoc = ReadSourceLocation();
-  E->Op = Record.readSubExpr();
-  E->IsThrownVariableInScope = Record.readInt();
+  E->CXXThrowExprBits.ThrowLoc = ReadSourceLocation();
+  E->ThrowExpr = Record.readSubExpr();
+  E->CXXThrowExprBits.IsThrownVariableInScope = Record.readInt();
 }
 
 void ASTStmtReader::VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
   VisitExpr(E);
   E->Param = ReadDeclAs();
-  E->Loc = ReadSourceLocation();
+  E->CXXDefaultArgExprBits.Loc = ReadSourceLocation();
 }
 
 void ASTStmtReader::VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E) {
   VisitExpr(E);
   E->Field = ReadDeclAs();
-  E->Loc = ReadSourceLocation();
+  E->CXXDefaultInitExprBits.Loc = ReadSourceLocation();
 }
 
 void ASTStmtReader::VisitCXXBindTemporaryExpr(CXXBindTemporaryExpr *E) {
Index: lib/AST/ExprCXX.cpp
===
--- lib/AST/ExprCXX.cpp
+++ lib/AST/ExprCXX.cpp
@@ -749,14 +749,15 @@
   return cast(getCalleeDecl())->getLiteralIdentifier();
 }
 
-CXXDefaultInitExpr::CXXDefaultInitExpr(const ASTContext , SourceLocation Loc,
-   FieldDecl *Field, QualType T)
-: Expr(CXXDefaultInitExprClass, T.getNonLValueExprType(C),
-   T->isLValueReferenceType() ? VK_LValue : T->isRValueReferenceType()
+CXXDefaultInitExpr::CXXDefaultInitExpr(const ASTContext , SourceLocation Loc,
+   FieldDecl *Field, QualType Ty)
+: Expr(CXXDefaultInitExprClass, Ty.getNonLValueExprType(Ctx),
+   Ty->isLValueReferenceType() ? VK_LValue : Ty->isRValueReferenceType()
 ? VK_XValue
 : VK_RValue,
/*FIXME*/ OK_Ordinary, false, false, false, false),
-  Field(Field), Loc(Loc) {
+  Field(Field) {
+  CXXDefaultInitExprBits.Loc = Loc;
   assert(Field->hasInClassInitializer());
 }
 
Index: include/clang/AST/Stmt.h
===
--- include/clang/AST/Stmt.h
+++ include/clang/AST/Stmt.h
@@ -596,6 +596,39 @@
 SourceLocation Loc;
   };
 
+  class CXXThrowExprBitfields {
+friend class ASTStmtReader;
+friend class CXXThrowExpr;
+
+unsigned : NumExprBits;
+
+/// Whether the thrown variable (if any) is in scope.
+unsigned IsThrownVariableInScope : 1;
+
+/// The location of the "throw".
+SourceLocation ThrowLoc;
+  };
+
+  class CXXDefaultArgExprBitfields {
+friend class ASTStmtReader;
+friend class CXXDefaultArgExpr;
+
+unsigned : NumExprBits;
+
+/// The location where the default argument expression was used.
+SourceLocation Loc;
+  };
+
+  class CXXDefaultInitExprBitfields {
+friend class ASTStmtReader;
+friend class CXXDefaultInitExpr;
+
+unsigned : NumExprBits;
+
+/// The location where the default initializer expression was used.
+SourceLocation Loc;
+  };
+
   class TypeTraitExprBitfields {
 friend class ASTStmtReader;
 friend class ASTStmtWriter;
@@ -687,6 +720,9 @@
 CXXBoolLiteralExprBitfields CXXBoolLiteralExprBits;
 CXXNullPtrLiteralExprBitfields CXXNullPtrLiteralExprBits;
 CXXThisExprBitfields CXXThisExprBits;
+CXXThrowExprBitfields CXXThrowExprBits;
+CXXDefaultArgExprBitfields CXXDefaultArgExprBits;
+CXXDefaultInitExprBitfields CXXDefaultInitExprBits;
 TypeTraitExprBitfields TypeTraitExprBits;
 ExprWithCleanupsBitfields ExprWithCleanupsBits;
 
Index: include/clang/AST/ExprCXX.h
===
--- include/clang/AST/ExprCXX.h
+++ include/clang/AST/ExprCXX.h
@@ -1003,42 +1003,43 @@
 class CXXThrowExpr : public Expr {
   friend class ASTStmtReader;
 
-  Stmt *Op;
-  SourceLocation ThrowLoc;
-
-  /// Whether the thrown variable (if any) is in scope.
-  unsigned IsThrownVariableInScope : 1;
+  /// The optional expression in the throw statement.
+  Stmt *ThrowExpr;
 
 public:
   // \p Ty is the void type which is used as the result type of the
-  //

[PATCH] D51554: [CUDA][OPENMP][NVPTX]Improve logic of the debug info support.

2018-11-09 Thread Alexey Bataev via Phabricator via cfe-commits

ABataev added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:292
+  bool IsDebugEnabled = !A || A->getOption().matches(options::OPT_O0) ||
+Args.hasFlag(options::OPT_cuda_noopt_device_debug,
+ options::OPT_no_cuda_noopt_device_debug,

echristo wrote:
> Is this an nvcc compatibility flag?
No, nvcc uses different set of flags. It uses `-g` for the debug info for the 
host code and `-G` for the device code. I'm not the original author of this 
option. clang uses it to control emission of the debug info for the device.
The bad thing about nvcc that it disables optimizations when `-G` is used. 
Using this option we can use LLVM optimizations and disable the optimizations 
only when we call `ptxas` tool.


Repository:
  rC Clang

https://reviews.llvm.org/D51554



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

[PATCH] D54204: [clangd] Initial clang-tidy diagnostics support.

2018-11-09 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: clangd/ClangdUnit.cpp:175
+CTFactories.createChecks(CTContext.getPointer(), CTChecks);
+for (const auto  : CTChecks) {
+  Check->registerPPCallbacks(*Clang);

hokein wrote:
> Maybe add the check names to the `Trace`?
that would be nice, but there's no API to get that info :-(



Comment at: clangd/ClangdUnit.cpp:468
+  X##ModuleAnchorSource
+LINK_TIDY_MODULE(CERT);
+LINK_TIDY_MODULE(Abseil);

hokein wrote:
> I'm curious how much does clangd binary size get increased.
It's now 21M stripped vs 18M before this patch.
The different with debug info is *much* larger for some reason... sadly this 
will hurt link times.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54204



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

[PATCH] D54325: [AST] Pack CXXBoolLiteralExpr, CXXNullPtrLiteralExpr and CXXThisExpr

2018-11-09 Thread Bruno Ricci via Phabricator via cfe-commits

riccibruno created this revision.
riccibruno added a reviewer: rsmith.
riccibruno added a project: clang.
Herald added a subscriber: cfe-commits.

Use the newly available space in the bit-fields of `Stmt` to
store some data from `CXXBoolLiteralExpr`, `CXXNullPtrLiteralExpr`
and `CXXThisExpr`.

This cuts the size of each of these classes by 1 pointer.


Repository:
  rC Clang

https://reviews.llvm.org/D54325

Files:
  include/clang/AST/ExprCXX.h
  include/clang/AST/Stmt.h

Index: include/clang/AST/Stmt.h
===
--- include/clang/AST/Stmt.h
+++ include/clang/AST/Stmt.h
@@ -516,18 +516,6 @@
 SourceLocation OpLoc;
   };
 
-  class ExprWithCleanupsBitfields {
-friend class ASTStmtReader; // deserialization
-friend class ExprWithCleanups;
-
-unsigned : NumExprBits;
-
-// When false, it must not have side effects.
-unsigned CleanupsHaveSideEffects : 1;
-
-unsigned NumObjects : 32 - 1 - NumExprBits;
-  };
-
   class ParenListExprBitfields {
 friend class ASTStmtReader;
 friend class ParenListExpr;
@@ -560,14 +548,6 @@
 unsigned IsUnique : 1;
   };
 
-  class ObjCIndirectCopyRestoreExprBitfields {
-friend class ObjCIndirectCopyRestoreExpr;
-
-unsigned : NumExprBits;
-
-unsigned ShouldCopy : 1;
-  };
-
   class InitListExprBitfields {
 friend class InitListExpr;
 
@@ -578,6 +558,44 @@
 unsigned HadArrayRangeDesignator : 1;
   };
 
+  //===--- C++ expression bitfields classes ---===//
+
+  class CXXBoolLiteralExprBitfields {
+friend class ASTStmtReader;
+friend class CXXBoolLiteralExpr;
+
+unsigned : NumExprBits;
+
+/// The value of the boolean literal.
+unsigned Value : 1;
+
+/// The location of the boolean literal.
+SourceLocation Loc;
+  };
+
+  class CXXNullPtrLiteralExprBitfields {
+friend class ASTStmtReader;
+friend class CXXNullPtrLiteralExpr;
+
+unsigned : NumExprBits;
+
+/// The location of the null pointer literal.
+SourceLocation Loc;
+  };
+
+  class CXXThisExprBitfields {
+friend class ASTStmtReader;
+friend class CXXThisExpr;
+
+unsigned : NumExprBits;
+
+/// Whether this is an implicit "this".
+unsigned IsImplicit : 1;
+
+/// The location of the "this".
+SourceLocation Loc;
+  };
+
   class TypeTraitExprBitfields {
 friend class ASTStmtReader;
 friend class ASTStmtWriter;
@@ -596,14 +614,38 @@
 unsigned NumArgs : 32 - 8 - 1 - NumExprBits;
   };
 
+  class ExprWithCleanupsBitfields {
+friend class ASTStmtReader;
+friend class ExprWithCleanups;
+
+unsigned : NumExprBits;
+
+/// When false, it must not have side effects.
+unsigned CleanupsHaveSideEffects : 1;
+
+unsigned NumObjects : 32 - 1 - NumExprBits;
+  };
+
+  //===--- C++ Coroutines TS expressions expression bitfields classes ---===//
+
   class CoawaitExprBitfields {
 friend class CoawaitExpr;
 
 unsigned : NumExprBits;
 
 unsigned IsImplicit : 1;
   };
 
+  //===--- Obj-C Expressions expression bitfields classes ---===//
+
+  class ObjCIndirectCopyRestoreExprBitfields {
+friend class ObjCIndirectCopyRestoreExpr;
+
+unsigned : NumExprBits;
+
+unsigned ShouldCopy : 1;
+  };
+
   union {
 // Statements
 StmtBitfields StmtBits;
@@ -636,14 +678,23 @@
 MemberExprBitfields MemberExprBits;
 CastExprBitfields CastExprBits;
 BinaryOperatorBitfields BinaryOperatorBits;
-ExprWithCleanupsBitfields ExprWithCleanupsBits;
 ParenListExprBitfields ParenListExprBits;
 PseudoObjectExprBitfields PseudoObjectExprBits;
 OpaqueValueExprBitfields OpaqueValueExprBits;
-ObjCIndirectCopyRestoreExprBitfields ObjCIndirectCopyRestoreExprBits;
 InitListExprBitfields InitListExprBits;
+
+// C++ Expressions
+CXXBoolLiteralExprBitfields CXXBoolLiteralExprBits;
+CXXNullPtrLiteralExprBitfields CXXNullPtrLiteralExprBits;
+CXXThisExprBitfields CXXThisExprBits;
 TypeTraitExprBitfields TypeTraitExprBits;
+ExprWithCleanupsBitfields ExprWithCleanupsBits;
+
+// C++ Coroutines TS expressions
 CoawaitExprBitfields CoawaitBits;
+
+// Obj-C Expressions
+ObjCIndirectCopyRestoreExprBitfields ObjCIndirectCopyRestoreExprBits;
   };
 
 public:
Index: include/clang/AST/ExprCXX.h
===
--- include/clang/AST/ExprCXX.h
+++ include/clang/AST/ExprCXX.h
@@ -548,26 +548,25 @@
 
 /// A boolean literal, per ([C++ lex.bool] Boolean literals).
 class CXXBoolLiteralExpr : public Expr {
-  bool Value;
-  SourceLocation Loc;
-
 public:
-  CXXBoolLiteralExpr(bool val, QualType Ty, SourceLocation l)
+  CXXBoolLiteralExpr(bool Val, QualType Ty, SourceLocation Loc)
   : Expr(CXXBoolLiteralExprClass, Ty, VK_RValue, OK_Ordinary, false, false,
- false, false),
-Value(val), Loc(l) {}
+ false, false) {
+CXXBoolLiteralExprBits.Value = Val;
+CXXBoolLiteralExprBits.Loc = Loc;
+  }

[PATCH] D54281: [clang-tidy] fix PR39583 - ignoring ParenCast for string-literals in pro-bounds-array-to-pointer-decay

2018-11-09 Thread Aaron Ballman via Phabricator via cfe-commits

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

LGTM aside from a minor commenting nit.




Comment at: 
test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp:44
+const char *g2() {
+return ("clang"); // OK, ParenCast hides the literal-pointer decay
+}

Remove "Cast" as this is not a cast. It's a ParenExpr.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54281



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

[PATCH] D54204: [clangd] Initial clang-tidy diagnostics support.

2018-11-09 Thread Sam McCall via Phabricator via cfe-commits

sammccall updated this revision to Diff 173357.
sammccall marked an inline comment as done.
sammccall added a comment.

Address comments and rebase on https://reviews.llvm.org/D54309, addressing 
performance issues.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54204

Files:
  clang-tidy/misc/UnusedParametersCheck.cpp
  clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tidy/modernize/LoopConvertUtils.h
  clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tidy/readability/SimplifyBooleanExprCheck.h
  clangd/CMakeLists.txt
  clangd/ClangdUnit.cpp
  clangd/XRefs.cpp
  unittests/clangd/ClangdUnitTests.cpp

Index: unittests/clangd/ClangdUnitTests.cpp
===
--- unittests/clangd/ClangdUnitTests.cpp
+++ unittests/clangd/ClangdUnitTests.cpp
@@ -24,6 +24,7 @@
 using testing::Field;
 using testing::IsEmpty;
 using testing::Pair;
+using testing::UnorderedElementsAre;
 
 testing::Matcher WithFix(testing::Matcher FixMatcher) {
   return Field(::Fixes, ElementsAre(FixMatcher));
@@ -128,6 +129,30 @@
   WithFix(Fix(Test.range(), "int", "change return type to 'int'");
 }
 
+TEST(DiagnosticsTest, ClangTidy) {
+  Annotations Test(R"cpp(
+#define $macrodef[[SQUARE]](X) (X)*(X)
+int main() {
+  return [[sizeof]](sizeof(int));
+  int y = 4;
+  return SQUARE($macroarg[[++]]y);
+}
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Test.range(), "suspicious usage of 'sizeof(sizeof(...))' "
+ "[bugprone-sizeof-expression]"),
+  AllOf(
+  Diag(Test.range("macroarg"),
+   "side effects in the 1st macro argument 'X' are repeated in "
+   "macro expansion [bugprone-macro-repeated-side-effects]"),
+  WithNote(Diag(Test.range("macrodef"),
+"macro 'SQUARE' defined here "
+"[bugprone-macro-repeated-side-effects]");
+}
+
 TEST(DiagnosticsTest, Preprocessor) {
   // This looks like a preamble, but there's an #else in the middle!
   // Check that:
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -672,8 +672,7 @@
 return {};
 
   DeducedTypeVisitor V(SourceLocationBeg);
-  for (Decl *D : AST.getLocalTopLevelDecls())
-V.TraverseDecl(D);
+  V.TraverseAST(AST.getASTContext());
   return V.getDeducedType();
 }
 
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -8,6 +8,8 @@
 //===--===//
 
 #include "ClangdUnit.h"
+#include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
+#include "../clang-tidy/ClangTidyModuleRegistry.h"
 #include "Compiler.h"
 #include "Diagnostics.h"
 #include "Logger.h"
@@ -151,6 +153,38 @@
 return None;
   }
 
+  // Set up ClangTidy. Must happen after BeginSourceFile() so ASTContext exists.
+  // Clang-tidy has some limitiations to ensure reasonable performance:
+  //  - checks don't see all preprocessor events in the preamble
+  //  - matchers run only over the main-file top-level decls (and can't see
+  //ancestors outside this scope).
+  // In practice almost all checks work well without modifications.
+  std::vector> CTChecks;
+  ast_matchers::MatchFinder CTFinder;
+  llvm::Optional CTContext;
+  {
+trace::Span Tracer("ClangTidyInit");
+tidy::ClangTidyCheckFactories CTFactories;
+for (const auto  : tidy::ClangTidyModuleRegistry::entries())
+  E.instantiate()->addCheckFactories(CTFactories);
+auto CTOpts = tidy::ClangTidyOptions::getDefaults();
+// FIXME: this needs to be configurable, and we need to support .clang-tidy
+// files and other options providers.
+// These checks exercise the matcher- and preprocessor-based hooks.
+CTOpts.Checks =
+"bugprone-sizeof-expression,bugprone-macro-repeated-side-effects";
+CTContext.emplace(llvm::make_unique(
+tidy::ClangTidyGlobalOptions(), CTOpts));
+CTContext->setDiagnosticsEngine(>getDiagnostics());
+CTContext->setASTContext(>getASTContext());
+CTContext->setCurrentFile(MainInput.getFile());
+CTFactories.createChecks(CTContext.getPointer(), CTChecks);
+for (const auto  : CTChecks) {
+  Check->registerPPCallbacks(*Clang);
+  Check->registerMatchers();
+}
+  }
+
   // Copy over the includes from the preamble, then combine with the
   // non-preamble includes below.
   auto Includes = Preamble ? Preamble->Includes : IncludeStructure{};
@@ -160,13 +194,26 @@
   if (!Action->Execute())
 log("Execute() failed when building AST for {0}", MainInput.getFile());
 
+  std::vector ParsedDecls = Action->takeTopLevelDecls();
+  // AST traversals should exclude the

[PATCH] D54324: [AST] Store the value of CharacterLiteral inline if possible

2018-11-09 Thread Bruno Ricci via Phabricator via cfe-commits

riccibruno created this revision.
riccibruno added a reviewer: rsmith.
riccibruno added a project: clang.
Herald added a reviewer: shafik.
Herald added a subscriber: cfe-commits.

The vast majority of `CharacterLiteral`s have a value which fits into 8 bits
(in the 2666 `CharacterLiteral`s in all of Boost, only 2 don't).
When possible, use the space in the bit-fields of `Stmt` to store the value
and otherwise store it in a trailing object.

This saves 1 pointer per `CharacterLiteral` when the value fits into 8 bits.

Note that in itself this do not save that much space, but this is part of
a larger effort to pack the statements/expressions classes.


Repository:
  rC Clang

https://reviews.llvm.org/D54324

Files:
  include/clang/AST/Expr.h
  include/clang/AST/Stmt.h
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Serialization/ASTReaderStmt.cpp

Index: lib/Serialization/ASTReaderStmt.cpp
===
--- lib/Serialization/ASTReaderStmt.cpp
+++ lib/Serialization/ASTReaderStmt.cpp
@@ -636,8 +636,8 @@
 void ASTStmtReader::VisitCharacterLiteral(CharacterLiteral *E) {
   VisitExpr(E);
   E->setValue(Record.readInt());
-  E->setLocation(ReadSourceLocation());
-  E->setKind(static_cast(Record.readInt()));
+  E->CharacterLiteralBits.Loc = ReadSourceLocation();
+  E->CharacterLiteralBits.Kind = Record.readInt();
 }
 
 void ASTStmtReader::VisitParenExpr(ParenExpr *E) {
@@ -2454,7 +2454,10 @@
   break;
 
 case EXPR_CHARACTER_LITERAL:
-  S = new (Context) CharacterLiteral(Empty);
+  S = CharacterLiteral::CreateEmpty(
+  Context,
+  /* IsSmallValue=*/CharacterLiteral::isSmallValue(
+  Record[ASTStmtReader::NumExprFields + 0]));
   break;
 
 case EXPR_PAREN:
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -6865,8 +6865,9 @@
 else
   Kind = CharacterLiteral::Ascii;
 
-E = new (Context) CharacterLiteral(Arg.getAsIntegral().getZExtValue(),
-   Kind, T, Loc);
+E = CharacterLiteral::Create(Context, Arg.getAsIntegral().getZExtValue(),
+ Kind, T, Loc);
+
   } else if (T->isBooleanType()) {
 E = new (Context) CXXBoolLiteralExpr(Arg.getAsIntegral().getBoolValue(),
  T, Loc);
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -3139,8 +3139,8 @@
   else if (Literal.isUTF8())
 Kind = CharacterLiteral::UTF8;
 
-  Expr *Lit = new (Context) CharacterLiteral(Literal.getValue(), Kind, Ty,
- Tok.getLocation());
+  Expr *Lit = CharacterLiteral::Create(Context, Literal.getValue(), Kind, Ty,
+   Tok.getLocation());
 
   if (Literal.getUDSuffix().empty())
 return Lit;
Index: lib/AST/Expr.cpp
===
--- lib/AST/Expr.cpp
+++ lib/AST/Expr.cpp
@@ -815,6 +815,37 @@
   return S.str();
 }
 
+CharacterLiteral::CharacterLiteral(unsigned Value, CharacterKind Kind,
+   QualType Type, SourceLocation Loc)
+: Expr(CharacterLiteralClass, Type, VK_RValue, OK_Ordinary, false, false,
+   false, false) {
+  CharacterLiteralBits.Kind = Kind;
+  CharacterLiteralBits.Loc = Loc;
+  CharacterLiteralBits.IsSmallValue = isSmallValue(Value);
+  setValue(Value);
+}
+
+CharacterLiteral::CharacterLiteral(EmptyShell Empty, bool IsSmallValue)
+: Expr(CharacterLiteralClass, Empty) {
+  CharacterLiteralBits.IsSmallValue = IsSmallValue;
+}
+
+CharacterLiteral *CharacterLiteral::Create(const ASTContext ,
+   unsigned Value, CharacterKind Kind,
+   QualType Type, SourceLocation Loc) {
+  bool IsSmallValue = isSmallValue(Value);
+  void *Mem = Ctx.Allocate(totalSizeToAlloc(!IsSmallValue),
+   alignof(CharacterLiteral));
+  return new (Mem) CharacterLiteral(Value, Kind, Type, Loc);
+}
+
+CharacterLiteral *CharacterLiteral::CreateEmpty(const ASTContext ,
+bool IsSmallValue) {
+  void *Mem = Ctx.Allocate(totalSizeToAlloc(!IsSmallValue),
+   alignof(CharacterLiteral));
+  return new (Mem) CharacterLiteral(EmptyShell(), IsSmallValue);
+}
+
 FloatingLiteral::FloatingLiteral(const ASTContext , const llvm::APFloat ,
  bool isexact, QualType Type, SourceLocation L)
   : Expr(FloatingLiteralClass, Type, VK_RValue, OK_Ordinary, false, false,
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp

1 2 >

1 - 100 of 148 matches

Mail list logo