[PATCH] D36397: [clangd] Fixed a data race.

2017-08-09 Thread Ilya Biryukov via Phabricator via cfe-commits 
ilya-biryukov added a comment.

In https://reviews.llvm.org/D36397#833912, @klimek wrote:

> Yea, we'll probably want to add a "smash it hard with parallel" requests kind 
> of test to catch these things. You're right, there is probably not a better 
> solution for this specific bug.


Added a relevant test to https://reviews.llvm.org/D36261. It actually found 
this problem (after repeatedly running it for some time).


https://reviews.llvm.org/D36397



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

[PATCH] D36261: [clangd] Use multiple working threads in clangd.

2017-08-09 Thread Ilya Biryukov via Phabricator via cfe-commits 
ilya-biryukov added a comment.

Added a test that exercises some multi-threading behaviour. 
It really finds bugs, at least it found a problem that was fixed by 
https://reviews.llvm.org/D36397. (Will make sure to submit that other change 
before this one).

As discussed with @klimek, will move to `llvm::ThreadPool` after submitting a 
fix for it to allow functions with moveable-only arguments to be used there 
(that's what clangd uses).


https://reviews.llvm.org/D36261



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

[PATCH] D36261: [clangd] Use multiple working threads in clangd.

2017-08-09 Thread Ilya Biryukov via Phabricator via cfe-commits 
ilya-biryukov updated this revision to Diff 110381.
ilya-biryukov added a comment.

- Added a stress test for multithreading.


https://reviews.llvm.org/D36261

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/tool/ClangdMain.cpp
  unittests/clangd/ClangdTests.cpp

Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -136,18 +137,23 @@
 static const std::chrono::seconds DefaultFutureTimeout =
 std::chrono::seconds(10);
 
+static bool diagsContainErrors(ArrayRef Diagnostics) {
+  for (const auto  : Diagnostics) {
+// FIXME: severities returned by clangd should have a descriptive
+// diagnostic severity enum
+const int ErrorSeverity = 1;
+if (DiagAndFixIts.Diag.severity == ErrorSeverity)
+  return true;
+  }
+  return false;
+}
+
 class ErrorCheckingDiagConsumer : public DiagnosticsConsumer {
 public:
   void
   onDiagnosticsReady(PathRef File,
  Tagged Diagnostics) override {
-bool HadError = false;
-for (const auto  : Diagnostics.Value) {
-  // FIXME: severities returned by clangd should have a descriptive
-  // diagnostic severity enum
-  const int ErrorSeverity = 1;
-  HadError = DiagAndFixIts.Diag.severity == ErrorSeverity;
-}
+bool HadError = diagsContainErrors(Diagnostics.Value);
 
 std::lock_guard Lock(Mutex);
 HadErrorInLastDiags = HadError;
@@ -196,24 +202,46 @@
   std::vector ExtraClangFlags;
 };
 
+IntrusiveRefCntPtr
+buildTestFS(llvm::StringMap const ) {
+  IntrusiveRefCntPtr MemFS(
+  new vfs::InMemoryFileSystem);
+  for (auto  : Files)
+MemFS->addFile(FileAndContents.first(), time_t(),
+   llvm::MemoryBuffer::getMemBuffer(FileAndContents.second,
+FileAndContents.first()));
+
+  auto OverlayFS = IntrusiveRefCntPtr(
+  new vfs::OverlayFileSystem(vfs::getTempOnlyFS()));
+  OverlayFS->pushOverlay(std::move(MemFS));
+  return OverlayFS;
+}
+
+class ConstantFSProvider : public FileSystemProvider {
+public:
+  ConstantFSProvider(IntrusiveRefCntPtr FS,
+ VFSTag Tag = VFSTag())
+  : FS(std::move(FS)), Tag(std::move(Tag)) {}
+
+  Tagged
+  getTaggedFileSystem(PathRef File) override {
+return make_tagged(FS, Tag);
+  }
+
+private:
+  IntrusiveRefCntPtr FS;
+  VFSTag Tag;
+};
+
 class MockFSProvider : public FileSystemProvider {
 public:
   Tagged
   getTaggedFileSystem(PathRef File) override {
-IntrusiveRefCntPtr MemFS(
-new vfs::InMemoryFileSystem);
 if (ExpectedFile)
   EXPECT_EQ(*ExpectedFile, File);
 
-for (auto  : Files)
-  MemFS->addFile(FileAndContents.first(), time_t(),
- llvm::MemoryBuffer::getMemBuffer(FileAndContents.second,
-  FileAndContents.first()));
-
-auto OverlayFS = IntrusiveRefCntPtr(
-new vfs::OverlayFileSystem(vfs::getTempOnlyFS()));
-OverlayFS->pushOverlay(std::move(MemFS));
-return make_tagged(OverlayFS, Tag);
+auto FS = buildTestFS(Files);
+return make_tagged(FS, Tag);
   }
 
   llvm::Optional> ExpectedFile;
@@ -272,17 +300,17 @@
 MockFSProvider FS;
 ErrorCheckingDiagConsumer DiagConsumer;
 MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
-ClangdServer Server(CDB, DiagConsumer, FS,
-/*RunSynchronously=*/false);
+ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount());
 for (const auto  : ExtraFiles)
   FS.Files[getVirtualTestFilePath(FileWithContents.first)] =
   FileWithContents.second;
 
 auto SourceFilename = getVirtualTestFilePath(SourceFileRelPath);
 
 FS.ExpectedFile = SourceFilename;
 
-// Have to sync reparses because RunSynchronously is false.
+// Have to sync reparses because requests are processed on the calling
+// thread.
 auto AddDocFuture = Server.addDocument(SourceFilename, SourceContents);
 
 auto Result = dumpASTWithoutMemoryLocs(Server, SourceFilename);
@@ -334,8 +362,7 @@
   MockFSProvider FS;
   ErrorCheckingDiagConsumer DiagConsumer;
   MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
-  ClangdServer Server(CDB, DiagConsumer, FS,
-  /*RunSynchronously=*/false);
+  ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount());
 
   const auto SourceContents = R"cpp(
 #include "foo.h"
@@ -379,8 +406,7 @@
   ErrorCheckingDiagConsumer DiagConsumer;
   MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
 
-  ClangdServer Server(CDB, DiagConsumer, FS,
-  /*RunSynchronously=*/false);
+  ClangdServer Server(CDB,

[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values

2017-08-09 Thread Daniel Marjamäki via Phabricator via cfe-commits 
danielmarjamaki added a comment.

Should evalAPSInt() have machinery to do standard sign/type promotions? I 
suggest that I add one more argument `bool promote = false`, do you think that 
sounds good?


Repository:
  rL LLVM

https://reviews.llvm.org/D36471



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

[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values

2017-08-09 Thread Daniel Marjamäki via Phabricator via cfe-commits 
danielmarjamaki updated this revision to Diff 110378.
danielmarjamaki added a comment.

Refactoring, use BasicValueFactory::evalAPSInt


Repository:
  rL LLVM

https://reviews.llvm.org/D36471

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  test/Analysis/eval-range.c

Index: test/Analysis/eval-range.c
===
--- test/Analysis/eval-range.c
+++ test/Analysis/eval-range.c
@@ -0,0 +1,18 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
+
+void test1(int a) {
+  if (a >= 10 && a <= 50) {
+int b;
+
+b = a + 2;
+clang_analyzer_eval(b >= 12 && b <= 52); // expected-warning{{TRUE}}
+
+b = a - 2;
+clang_analyzer_eval(b >= 8 && b <= 48); // expected-warning{{TRUE}}
+
+b = a / 2;
+clang_analyzer_eval(b >= 5 && b <= 25); // expected-warning{{TRUE}}
+  }
+}
Index: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -304,6 +304,8 @@
   void print(ProgramStateRef State, raw_ostream , const char *nl,
  const char *sep) override;
 
+  ProgramStateRef evalRangeOp(ProgramStateRef state, SVal V) override;
+
   //===--===//
   // Implementation for interface from RangedConstraintManager.
   //===--===//
@@ -741,3 +743,56 @@
   }
   Out << nl;
 }
+
+ProgramStateRef RangeConstraintManager::evalRangeOp(ProgramStateRef St,
+SVal V) {
+  const SymExpr *SE = V.getAsSymExpr();
+  if (!SE)
+return nullptr;
+
+  const SymIntExpr *SIE = dyn_cast(SE);
+  if (!SIE)
+return nullptr;
+
+  const clang::BinaryOperatorKind Opc = SIE->getOpcode();
+
+  if (Opc != BO_Add && Opc != BO_Sub && Opc != BO_Div)
+return nullptr;
+
+  const SymExpr *LHS = SIE->getLHS();
+  const llvm::APSInt  = SIE->getRHS();
+
+  ConstraintRangeTy Ranges = St->get();
+  for (ConstraintRangeTy::iterator I = Ranges.begin(), E = Ranges.end(); I != E;
+   ++I) {
+if (LHS == I.getKey()) {
+  const auto D = I.getData();
+  for (auto I = D.begin(); I != D.end(); ++I) {
+if (I->From().isUnsigned() != RHS.isUnsigned())
+  // TODO: Handle sign conversions.
+  return nullptr;
+if (I->From().getBitWidth() != RHS.getBitWidth())
+  // TODO: Promote values.
+  return nullptr;
+if (I->From().isNegative())
+  // TODO: Handle negative range values
+  return nullptr;
+
+BasicValueFactory  = getBasicVals();
+const llvm::APSInt *Lower = BVF.evalAPSInt(Opc, I->From(), RHS);
+if (!Lower)
+  return nullptr;
+const llvm::APSInt *Upper = BVF.evalAPSInt(Opc, I->To(), RHS);
+if (!Upper)
+  return nullptr;
+
+SymbolRef Sym = V.getAsSymbol();
+RangeSet RS =
+getRange(St, Sym).Intersect(getBasicVals(), F, *Lower, *Upper);
+// TODO: This only evaluates the first range. Evaluate all ranges.
+return RS.isEmpty() ? nullptr : St->set(Sym, RS);
+  }
+}
+  }
+  return nullptr;
+}
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -98,7 +98,9 @@
   }
 
   state = state->BindExpr(B, LCtx, Result);
-  Bldr.generateNode(B, *it, state);
+  ProgramStateRef state2 =
+  getConstraintManager().evalRangeOp(state, Result);
+  Bldr.generateNode(B, *it, state2 ? state2 : state);
   continue;
 }
 
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
@@ -142,13 +142,15 @@
   /// Scan all symbols referenced by the constraints. If the symbol is not
   /// alive, remove it.
   virtual ProgramStateRef removeDeadBindings(ProgramStateRef state,
- SymbolReaper& SymReaper) = 0;
+ SymbolReaper ) = 0;
 
-  virtual void print(ProgramStateRef state,
- raw_ostream ,
- const char* nl,
+  virtual void print(ProgramStateRef state, raw_ostream , const char *nl,
  const char *sep) = 0;
 
+  virtual ProgramStateRef evalRangeOp(ProgramStateRef state, SVal V) {
+return nullptr;
+  }
+
   virtual void EndPath(ProgramStateRef

[PATCH] D36450: [X86][Ms-InlineAsm] Extend MS Dot operator to accept "this" + struct/class pointers aliases

2017-08-09 Thread coby via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL310472: [X86][Ms-InlineAsm] Extend MS Dot operator to accept 
"this" + struct/class… (authored by coby).

Changed prior to commit:
  https://reviews.llvm.org/D36450?vs=110145=110377#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D36450

Files:
  cfe/trunk/lib/Sema/SemaStmtAsm.cpp
  cfe/trunk/test/CodeGen/ms-inline-asm.c
  cfe/trunk/test/CodeGen/ms-inline-asm.cpp

Index: cfe/trunk/test/CodeGen/ms-inline-asm.cpp
===
--- cfe/trunk/test/CodeGen/ms-inline-asm.cpp
+++ cfe/trunk/test/CodeGen/ms-inline-asm.cpp
@@ -180,3 +180,19 @@
   A::g();
 }
 
+void t9() {
+  // CHECK-LABEL: define void @_Z2t9v()
+  struct A {
+int a;
+int b;
+void g() {
+  __asm mov eax, dword ptr [eax]this.b
+  // CHECK: call void asm sideeffect inteldialect
+  // CHECK-SAME: mov eax, dword ptr [eax].4
+  // CHECK-SAME: "~{eax},~{dirflag},~{fpsr},~{flags}"()
+}
+  };
+  A AA;
+  AA.g();
+}
+
Index: cfe/trunk/test/CodeGen/ms-inline-asm.c
===
--- cfe/trunk/test/CodeGen/ms-inline-asm.c
+++ cfe/trunk/test/CodeGen/ms-inline-asm.c
@@ -527,7 +527,7 @@
 typedef struct {
   int a;
   int b;
-} A;
+} A, *pA;
 
 typedef struct {
   int b1;
@@ -539,14 +539,16 @@
   A   c2;
   int c3;
   B   c4;
-} C;
+} C, *pC;
 
 void t39() {
 // CHECK-LABEL: define void @t39
   __asm mov eax, [eax].A.b
 // CHECK: mov eax, [eax].4
   __asm mov eax, [eax] A.b
 // CHECK: mov eax, [eax] .4
+  __asm mov eax, [eax] pA.b
+// CHECK: mov eax, [eax] .4
   __asm mov eax, fs:[0] A.b
 // CHECK: mov eax, fs:[$$0] .4
   __asm mov eax, [eax].B.b2.a
@@ -557,6 +559,8 @@
 // CHECK: mov eax, fs:[$$0] .8
   __asm mov eax, [eax]C.c4.b2.b
 // CHECK: mov eax, [eax].24
+  __asm mov eax, [eax]pC.c4.b2.b
+// CHECK: mov eax, [eax].24
 // CHECK: "~{eax},~{dirflag},~{fpsr},~{flags}"()
 }
 
Index: cfe/trunk/lib/Sema/SemaStmtAsm.cpp
===
--- cfe/trunk/lib/Sema/SemaStmtAsm.cpp
+++ cfe/trunk/lib/Sema/SemaStmtAsm.cpp
@@ -677,22 +677,33 @@
   SmallVector Members;
   Member.split(Members, ".");
 
-  LookupResult BaseResult(*this, (Base), SourceLocation(),
-  LookupOrdinaryName);
+  NamedDecl *FoundDecl = nullptr;
 
-  if (!LookupName(BaseResult, getCurScope()))
-return true;
-  
-  if(!BaseResult.isSingleResult())
+  // MS InlineAsm uses 'this' as a base
+  if (getLangOpts().CPlusPlus && Base.equals("this")) {
+if (const Type *PT = getCurrentThisType().getTypePtrOrNull())
+  FoundDecl = PT->getPointeeType()->getAsTagDecl();
+  } else {
+LookupResult BaseResult(*this, (Base), SourceLocation(),
+LookupOrdinaryName);
+if (LookupName(BaseResult, getCurScope()) && BaseResult.isSingleResult())
+  FoundDecl = BaseResult.getFoundDecl();
+  }
+
+  if (!FoundDecl)
 return true;
-  NamedDecl *FoundDecl = BaseResult.getFoundDecl();
+
   for (StringRef NextMember : Members) {
 const RecordType *RT = nullptr;
 if (VarDecl *VD = dyn_cast(FoundDecl))
   RT = VD->getType()->getAs();
 else if (TypedefNameDecl *TD = dyn_cast(FoundDecl)) {
   MarkAnyDeclReferenced(TD->getLocation(), TD, /*OdrUse=*/false);
-  RT = TD->getUnderlyingType()->getAs();
+  // MS InlineAsm often uses struct pointer aliases as a base
+  QualType QT = TD->getUnderlyingType();
+  if (const auto *PT = QT->getAs())
+QT = PT->getPointeeType();
+  RT = QT->getAs();
 } else if (TypeDecl *TD = dyn_cast(FoundDecl))
   RT = TD->getTypeForDecl()->getAs();
 else if (FieldDecl *TD = dyn_cast(FoundDecl))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r310472 - [X86][Ms-InlineAsm] Extend MS Dot operator to accept "this" + struct/class pointers aliases

2017-08-09 Thread Coby Tayree via cfe-commits 
Author: coby
Date: Wed Aug  9 06:31:41 2017
New Revision: 310472

URL: http://llvm.org/viewvc/llvm-project?rev=310472=rev
Log:
[X86][Ms-InlineAsm] Extend MS Dot operator to accept "this" + struct/class 
pointers aliases

MS InlineAsm Dot operator accepts "Bases" such as "this" (cpp) and class/struct 
pointer typedef.
This patch enhance its implementation with this behavior.

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

Modified:
cfe/trunk/lib/Sema/SemaStmtAsm.cpp
cfe/trunk/test/CodeGen/ms-inline-asm.c
cfe/trunk/test/CodeGen/ms-inline-asm.cpp

Modified: cfe/trunk/lib/Sema/SemaStmtAsm.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmtAsm.cpp?rev=310472=310471=310472=diff
==
--- cfe/trunk/lib/Sema/SemaStmtAsm.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmtAsm.cpp Wed Aug  9 06:31:41 2017
@@ -677,22 +677,33 @@ bool Sema::LookupInlineAsmField(StringRe
   SmallVector Members;
   Member.split(Members, ".");
 
-  LookupResult BaseResult(*this, (Base), SourceLocation(),
-  LookupOrdinaryName);
+  NamedDecl *FoundDecl = nullptr;
 
-  if (!LookupName(BaseResult, getCurScope()))
-return true;
-  
-  if(!BaseResult.isSingleResult())
+  // MS InlineAsm uses 'this' as a base
+  if (getLangOpts().CPlusPlus && Base.equals("this")) {
+if (const Type *PT = getCurrentThisType().getTypePtrOrNull())
+  FoundDecl = PT->getPointeeType()->getAsTagDecl();
+  } else {
+LookupResult BaseResult(*this, (Base), SourceLocation(),
+LookupOrdinaryName);
+if (LookupName(BaseResult, getCurScope()) && BaseResult.isSingleResult())
+  FoundDecl = BaseResult.getFoundDecl();
+  }
+
+  if (!FoundDecl)
 return true;
-  NamedDecl *FoundDecl = BaseResult.getFoundDecl();
+
   for (StringRef NextMember : Members) {
 const RecordType *RT = nullptr;
 if (VarDecl *VD = dyn_cast(FoundDecl))
   RT = VD->getType()->getAs();
 else if (TypedefNameDecl *TD = dyn_cast(FoundDecl)) {
   MarkAnyDeclReferenced(TD->getLocation(), TD, /*OdrUse=*/false);
-  RT = TD->getUnderlyingType()->getAs();
+  // MS InlineAsm often uses struct pointer aliases as a base
+  QualType QT = TD->getUnderlyingType();
+  if (const auto *PT = QT->getAs())
+QT = PT->getPointeeType();
+  RT = QT->getAs();
 } else if (TypeDecl *TD = dyn_cast(FoundDecl))
   RT = TD->getTypeForDecl()->getAs();
 else if (FieldDecl *TD = dyn_cast(FoundDecl))

Modified: cfe/trunk/test/CodeGen/ms-inline-asm.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-inline-asm.c?rev=310472=310471=310472=diff
==
--- cfe/trunk/test/CodeGen/ms-inline-asm.c (original)
+++ cfe/trunk/test/CodeGen/ms-inline-asm.c Wed Aug  9 06:31:41 2017
@@ -527,7 +527,7 @@ void cpuid() {
 typedef struct {
   int a;
   int b;
-} A;
+} A, *pA;
 
 typedef struct {
   int b1;
@@ -539,7 +539,7 @@ typedef struct {
   A   c2;
   int c3;
   B   c4;
-} C;
+} C, *pC;
 
 void t39() {
 // CHECK-LABEL: define void @t39
@@ -547,6 +547,8 @@ void t39() {
 // CHECK: mov eax, [eax].4
   __asm mov eax, [eax] A.b
 // CHECK: mov eax, [eax] .4
+  __asm mov eax, [eax] pA.b
+// CHECK: mov eax, [eax] .4
   __asm mov eax, fs:[0] A.b
 // CHECK: mov eax, fs:[$$0] .4
   __asm mov eax, [eax].B.b2.a
@@ -557,6 +559,8 @@ void t39() {
 // CHECK: mov eax, fs:[$$0] .8
   __asm mov eax, [eax]C.c4.b2.b
 // CHECK: mov eax, [eax].24
+  __asm mov eax, [eax]pC.c4.b2.b
+// CHECK: mov eax, [eax].24
 // CHECK: "~{eax},~{dirflag},~{fpsr},~{flags}"()
 }
 

Modified: cfe/trunk/test/CodeGen/ms-inline-asm.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-inline-asm.cpp?rev=310472=310471=310472=diff
==
--- cfe/trunk/test/CodeGen/ms-inline-asm.cpp (original)
+++ cfe/trunk/test/CodeGen/ms-inline-asm.cpp Wed Aug  9 06:31:41 2017
@@ -180,3 +180,19 @@ void t8() {
   A::g();
 }
 
+void t9() {
+  // CHECK-LABEL: define void @_Z2t9v()
+  struct A {
+int a;
+int b;
+void g() {
+  __asm mov eax, dword ptr [eax]this.b
+  // CHECK: call void asm sideeffect inteldialect
+  // CHECK-SAME: mov eax, dword ptr [eax].4
+  // CHECK-SAME: "~{eax},~{dirflag},~{fpsr},~{flags}"()
+}
+  };
+  A AA;
+  AA.g();
+}
+


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

[PATCH] D36473: Fix broken getAttributeSpellingListIndex for pragma attributes

2017-08-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, good catch!


https://reviews.llvm.org/D36473



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

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

2017-08-09 Thread Vassil Vassilev via cfe-commits 

Hi Diana,

  It seems the service is down. Could you send us the details of the 
failures (incl stack traces if any)


Many thanks,
Vassil
On 09/08/17 15:27, Diana Picus via cfe-commits wrote:

Hi Richard,

I'm sorry but I've reverted this in r310464 because it was breaking
some ASAN tests on this bot:
http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-full/builds/9452

Please let me know if I can help debug this.

Cheers,
Diana

On 8 August 2017 at 21:14, Richard Smith via cfe-commits
 wrote:

I forgot to say:

Based on a patch by Vassil Vassilev, which was based on a patch by Bernd
Schmidt, which was based on a patch by Reid Kleckner.

On 8 August 2017 at 12:12, Richard Smith via cfe-commits
 wrote:

Author: rsmith
Date: Tue Aug  8 12:12:28 2017
New Revision: 310401

URL: http://llvm.org/viewvc/llvm-project?rev=310401=rev
Log:
PR19668, PR23034: Fix handling of move constructors and deleted copy
constructors when deciding whether classes should be passed indirectly.

This fixes ABI differences between Clang and GCC:

  * Previously, Clang ignored the move constructor when making this
determination. It now takes the move constructor into account, per
https://github.com/itanium-cxx-abi/cxx-abi/pull/17 (this change may
seem recent, but the ABI change was agreed on the Itanium C++ ABI
list a long time ago).

  * Previously, Clang's behavior when the copy constructor was deleted
was unstable -- depending on whether the lazy declaration of the
copy constructor had been triggered, you might get different behavior.
We now eagerly declare the copy constructor whenever its deletedness
is unclear, and ignore deleted copy/move constructors when looking for
a trivial such constructor.

This also fixes an ABI difference between Clang and MSVC:

  * If the copy constructor would be implicitly deleted (but has not been
lazily declared yet), for instance because the class has an rvalue
reference member, we would pass it directly. We now pass such a class
indirectly, matching MSVC.

Modified:
 cfe/trunk/include/clang/AST/DeclCXX.h
 cfe/trunk/lib/AST/ASTImporter.cpp
 cfe/trunk/lib/AST/DeclCXX.cpp
 cfe/trunk/lib/CodeGen/CGCXXABI.cpp
 cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
 cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
 cfe/trunk/lib/Sema/SemaDeclCXX.cpp
 cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
 cfe/trunk/lib/Serialization/ASTWriter.cpp
 cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp
 cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=310401=310400=310401=diff

==
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Tue Aug  8 12:12:28 2017
@@ -375,6 +375,7 @@ class CXXRecordDecl : public RecordDecl
  /// \brief These flags are \c true if a defaulted corresponding
special
  /// member can't be fully analyzed without performing overload
resolution.
  /// @{
+unsigned NeedOverloadResolutionForCopyConstructor : 1;
  unsigned NeedOverloadResolutionForMoveConstructor : 1;
  unsigned NeedOverloadResolutionForMoveAssignment : 1;
  unsigned NeedOverloadResolutionForDestructor : 1;
@@ -383,6 +384,7 @@ class CXXRecordDecl : public RecordDecl
  /// \brief These flags are \c true if an implicit defaulted
corresponding
  /// special member would be defined as deleted.
  /// @{
+unsigned DefaultedCopyConstructorIsDeleted : 1;
  unsigned DefaultedMoveConstructorIsDeleted : 1;
  unsigned DefaultedMoveAssignmentIsDeleted : 1;
  unsigned DefaultedDestructorIsDeleted : 1;
@@ -415,6 +417,12 @@ class CXXRecordDecl : public RecordDecl
  /// constructor.
  unsigned HasDefaultedDefaultConstructor : 1;

+/// \brief True if this class can be passed in a
non-address-preserving
+/// fashion (such as in registers) according to the C++ language
rules.
+/// This does not imply anything about how the ABI in use will
actually
+/// pass an object of this class.
+unsigned CanPassInRegisters : 1;
+
  /// \brief True if a defaulted default constructor for this class
would
  /// be constexpr.
  unsigned DefaultedDefaultConstructorIsConstexpr : 1;
@@ -811,18 +819,50 @@ public:
  return data().FirstFriend.isValid();
}

+  /// \brief \c true if a defaulted copy constructor for this class would
be
+  /// deleted.
+  bool defaultedCopyConstructorIsDeleted() const {
+assert((!needsOverloadResolutionForCopyConstructor() ||
+(data().DeclaredSpecialMembers & SMF_CopyConstructor)) &&
+   "this property has not yet been computed by Sema");
+return data().DefaultedCopyConstructorIsDeleted;
+  }
+
+  /// \brief \c true if a defaulted

[PATCH] D36458: Fix crash when current lexer is nullptr

2017-08-09 Thread Ivan Donchevskii via Phabricator via cfe-commits 
yvvan added a comment.

But I don't know the source of that issue, I only know how to fix it because I 
caught that one in debugger and added a check to prevent it.
How can I write testcase for such fix?


https://reviews.llvm.org/D36458



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

[clang-tools-extra] r310470 - [clangd] Fixed a bug in make_tagged.

2017-08-09 Thread Ilya Biryukov via cfe-commits 
Author: ibiryukov
Date: Wed Aug  9 05:55:13 2017
New Revision: 310470

URL: http://llvm.org/viewvc/llvm-project?rev=310470=rev
Log:
[clangd] Fixed a bug in make_tagged.

It accidentally std::move'd from Value parameter if it deduced to an
l-value ref.

Modified:
clang-tools-extra/trunk/clangd/ClangdServer.h

Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=310470=310469=310470=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Wed Aug  9 05:55:13 2017
@@ -66,7 +66,7 @@ public:
 
 template 
 Tagged::type> make_tagged(T &, VFSTag Tag) {
-  return Tagged(std::forward(Value), Tag);
+  return Tagged::type>(std::forward(Value), Tag);
 }
 
 class DiagnosticsConsumer {


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

[PATCH] D36171: AMDGPU: Use direct struct returns

2017-08-09 Thread Yaxun Liu via Phabricator via cfe-commits 
yaxunl accepted this revision.
yaxunl added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks!


https://reviews.llvm.org/D36171



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

[PATCH] D35020: [Modules] Add ability to specify module name to module file mapping

2017-08-09 Thread Boris Kolpackov via Phabricator via cfe-commits 
boris added a comment.

Ping.


https://reviews.llvm.org/D35020



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

[PATCH] D35678: Omit sumbodule semantics for TS modules

2017-08-09 Thread Boris Kolpackov via Phabricator via cfe-commits 
boris added a comment.

Ping.


https://reviews.llvm.org/D35678



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

r310468 - [Sema] -Wenum-compare no longer warn on anonymous enums in switch statements

2017-08-09 Thread Gabor Horvath via cfe-commits 
Author: xazax
Date: Wed Aug  9 05:34:58 2017
New Revision: 310468

URL: http://llvm.org/viewvc/llvm-project?rev=310468=rev
Log:
[Sema] -Wenum-compare no longer warn on anonymous enums in switch statements

Patch by: Reka Nikolett Kovacs

Modified:
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/test/SemaCXX/warn-enum-compare.cpp

Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=310468=310467=310468=diff
==
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Wed Aug  9 05:34:58 2017
@@ -753,6 +753,12 @@ static void checkEnumTypesInSwitchStmt(S
   if (!CondEnumType || !CaseEnumType)
 return;
 
+  // Ignore anonymous enums.
+  if (!CondEnumType->getDecl()->getIdentifier())
+return;
+  if (!CaseEnumType->getDecl()->getIdentifier())
+return;
+
   if (S.Context.hasSameUnqualifiedType(CondType, CaseType))
 return;
 

Modified: cfe/trunk/test/SemaCXX/warn-enum-compare.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-enum-compare.cpp?rev=310468=310467=310468=diff
==
--- cfe/trunk/test/SemaCXX/warn-enum-compare.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-enum-compare.cpp Wed Aug  9 05:34:58 2017
@@ -226,4 +226,11 @@ void test () {
 case BarF: break;
 case FooA: break; // expected-warning {{comparison of two values with 
different enumeration types ('Bar' and 'Foo')}}
   }
+
+  switch(x) {
+case AnonAA: break; // expected-warning {{case value not in enumerated 
type 'Foo'}}
+case FooA: break;
+case FooB: break;
+case FooC: break;
+  }
 }


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

Re: r310401 - PR19668, PR23034: Fix handling of move constructors and deleted copy

2017-08-09 Thread Diana Picus via cfe-commits 
Hi Richard,

I'm sorry but I've reverted this in r310464 because it was breaking
some ASAN tests on this bot:
http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-full/builds/9452

Please let me know if I can help debug this.

Cheers,
Diana

On 8 August 2017 at 21:14, Richard Smith via cfe-commits
 wrote:
> I forgot to say:
>
> Based on a patch by Vassil Vassilev, which was based on a patch by Bernd
> Schmidt, which was based on a patch by Reid Kleckner.
>
> On 8 August 2017 at 12:12, Richard Smith via cfe-commits
>  wrote:
>>
>> Author: rsmith
>> Date: Tue Aug  8 12:12:28 2017
>> New Revision: 310401
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=310401=rev
>> Log:
>> PR19668, PR23034: Fix handling of move constructors and deleted copy
>> constructors when deciding whether classes should be passed indirectly.
>>
>> This fixes ABI differences between Clang and GCC:
>>
>>  * Previously, Clang ignored the move constructor when making this
>>determination. It now takes the move constructor into account, per
>>https://github.com/itanium-cxx-abi/cxx-abi/pull/17 (this change may
>>seem recent, but the ABI change was agreed on the Itanium C++ ABI
>>list a long time ago).
>>
>>  * Previously, Clang's behavior when the copy constructor was deleted
>>was unstable -- depending on whether the lazy declaration of the
>>copy constructor had been triggered, you might get different behavior.
>>We now eagerly declare the copy constructor whenever its deletedness
>>is unclear, and ignore deleted copy/move constructors when looking for
>>a trivial such constructor.
>>
>> This also fixes an ABI difference between Clang and MSVC:
>>
>>  * If the copy constructor would be implicitly deleted (but has not been
>>lazily declared yet), for instance because the class has an rvalue
>>reference member, we would pass it directly. We now pass such a class
>>indirectly, matching MSVC.
>>
>> Modified:
>> cfe/trunk/include/clang/AST/DeclCXX.h
>> cfe/trunk/lib/AST/ASTImporter.cpp
>> cfe/trunk/lib/AST/DeclCXX.cpp
>> cfe/trunk/lib/CodeGen/CGCXXABI.cpp
>> cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
>> cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
>> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>> cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
>> cfe/trunk/lib/Serialization/ASTWriter.cpp
>> cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp
>> cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
>>
>> Modified: cfe/trunk/include/clang/AST/DeclCXX.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=310401=310400=310401=diff
>>
>> ==
>> --- cfe/trunk/include/clang/AST/DeclCXX.h (original)
>> +++ cfe/trunk/include/clang/AST/DeclCXX.h Tue Aug  8 12:12:28 2017
>> @@ -375,6 +375,7 @@ class CXXRecordDecl : public RecordDecl
>>  /// \brief These flags are \c true if a defaulted corresponding
>> special
>>  /// member can't be fully analyzed without performing overload
>> resolution.
>>  /// @{
>> +unsigned NeedOverloadResolutionForCopyConstructor : 1;
>>  unsigned NeedOverloadResolutionForMoveConstructor : 1;
>>  unsigned NeedOverloadResolutionForMoveAssignment : 1;
>>  unsigned NeedOverloadResolutionForDestructor : 1;
>> @@ -383,6 +384,7 @@ class CXXRecordDecl : public RecordDecl
>>  /// \brief These flags are \c true if an implicit defaulted
>> corresponding
>>  /// special member would be defined as deleted.
>>  /// @{
>> +unsigned DefaultedCopyConstructorIsDeleted : 1;
>>  unsigned DefaultedMoveConstructorIsDeleted : 1;
>>  unsigned DefaultedMoveAssignmentIsDeleted : 1;
>>  unsigned DefaultedDestructorIsDeleted : 1;
>> @@ -415,6 +417,12 @@ class CXXRecordDecl : public RecordDecl
>>  /// constructor.
>>  unsigned HasDefaultedDefaultConstructor : 1;
>>
>> +/// \brief True if this class can be passed in a
>> non-address-preserving
>> +/// fashion (such as in registers) according to the C++ language
>> rules.
>> +/// This does not imply anything about how the ABI in use will
>> actually
>> +/// pass an object of this class.
>> +unsigned CanPassInRegisters : 1;
>> +
>>  /// \brief True if a defaulted default constructor for this class
>> would
>>  /// be constexpr.
>>  unsigned DefaultedDefaultConstructorIsConstexpr : 1;
>> @@ -811,18 +819,50 @@ public:
>>  return data().FirstFriend.isValid();
>>}
>>
>> +  /// \brief \c true if a defaulted copy constructor for this class would
>> be
>> +  /// deleted.
>> +  bool defaultedCopyConstructorIsDeleted() const {
>> +assert((!needsOverloadResolutionForCopyConstructor() ||
>> +(data().DeclaredSpecialMembers & SMF_CopyConstructor)) &&
>> +   "this property has not yet been computed by Sema");
>> +return

r310464 - Revert "PR19668, PR23034: Fix handling of move constructors and deleted copy constructors when deciding whether classes should be passed indirectly."

2017-08-09 Thread Diana Picus via cfe-commits 
Author: rovka
Date: Wed Aug  9 05:22:25 2017
New Revision: 310464

URL: http://llvm.org/viewvc/llvm-project?rev=310464=rev
Log:
Revert "PR19668, PR23034: Fix handling of move constructors and deleted copy 
constructors when deciding whether classes should be passed indirectly."

This reverts commit r310401 because it seems to have broken some ARM
bot(s).

Modified:
cfe/trunk/include/clang/AST/DeclCXX.h
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/lib/AST/DeclCXX.cpp
cfe/trunk/lib/CodeGen/CGCXXABI.cpp
cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
cfe/trunk/test/CodeGenCXX/uncopyable-args.cpp
cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=310464=310463=310464=diff
==
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Wed Aug  9 05:22:25 2017
@@ -375,7 +375,6 @@ class CXXRecordDecl : public RecordDecl
 /// \brief These flags are \c true if a defaulted corresponding special
 /// member can't be fully analyzed without performing overload resolution.
 /// @{
-unsigned NeedOverloadResolutionForCopyConstructor : 1;
 unsigned NeedOverloadResolutionForMoveConstructor : 1;
 unsigned NeedOverloadResolutionForMoveAssignment : 1;
 unsigned NeedOverloadResolutionForDestructor : 1;
@@ -384,7 +383,6 @@ class CXXRecordDecl : public RecordDecl
 /// \brief These flags are \c true if an implicit defaulted corresponding
 /// special member would be defined as deleted.
 /// @{
-unsigned DefaultedCopyConstructorIsDeleted : 1;
 unsigned DefaultedMoveConstructorIsDeleted : 1;
 unsigned DefaultedMoveAssignmentIsDeleted : 1;
 unsigned DefaultedDestructorIsDeleted : 1;
@@ -417,12 +415,6 @@ class CXXRecordDecl : public RecordDecl
 /// constructor.
 unsigned HasDefaultedDefaultConstructor : 1;
 
-/// \brief True if this class can be passed in a non-address-preserving
-/// fashion (such as in registers) according to the C++ language rules.
-/// This does not imply anything about how the ABI in use will actually
-/// pass an object of this class.
-unsigned CanPassInRegisters : 1;
-
 /// \brief True if a defaulted default constructor for this class would
 /// be constexpr.
 unsigned DefaultedDefaultConstructorIsConstexpr : 1;
@@ -819,50 +811,18 @@ public:
 return data().FirstFriend.isValid();
   }
 
-  /// \brief \c true if a defaulted copy constructor for this class would be
-  /// deleted.
-  bool defaultedCopyConstructorIsDeleted() const {
-assert((!needsOverloadResolutionForCopyConstructor() ||
-(data().DeclaredSpecialMembers & SMF_CopyConstructor)) &&
-   "this property has not yet been computed by Sema");
-return data().DefaultedCopyConstructorIsDeleted;
-  }
-
-  /// \brief \c true if a defaulted move constructor for this class would be
-  /// deleted.
-  bool defaultedMoveConstructorIsDeleted() const {
-assert((!needsOverloadResolutionForMoveConstructor() ||
-(data().DeclaredSpecialMembers & SMF_MoveConstructor)) &&
-   "this property has not yet been computed by Sema");
-return data().DefaultedMoveConstructorIsDeleted;
-  }
-
-  /// \brief \c true if a defaulted destructor for this class would be deleted.
-  bool defaultedDestructorIsDeleted() const {
-return !data().DefaultedDestructorIsDeleted;
-  }
-
-  /// \brief \c true if we know for sure that this class has a single,
-  /// accessible, unambiguous copy constructor that is not deleted.
-  bool hasSimpleCopyConstructor() const {
-return !hasUserDeclaredCopyConstructor() &&
-   !data().DefaultedCopyConstructorIsDeleted;
-  }
-
   /// \brief \c true if we know for sure that this class has a single,
   /// accessible, unambiguous move constructor that is not deleted.
   bool hasSimpleMoveConstructor() const {
 return !hasUserDeclaredMoveConstructor() && hasMoveConstructor() &&
!data().DefaultedMoveConstructorIsDeleted;
   }
-
   /// \brief \c true if we know for sure that this class has a single,
   /// accessible, unambiguous move assignment operator that is not deleted.
   bool hasSimpleMoveAssignment() const {
 return !hasUserDeclaredMoveAssignment() && hasMoveAssignment() &&
!data().DefaultedMoveAssignmentIsDeleted;
   }
-
   /// \brief \c true if we know for sure that this class has an accessible
   /// destructor that is not deleted.
   bool hasSimpleDestructor() const {
@@ -918,16 +878,7 @@ public:
   /// \brief Determine whether we need to eagerly declare a defaulted copy
   /// constructor for this class.

[PATCH] D36458: Fix crash when current lexer is nullptr

2017-08-09 Thread Benjamin Kramer via Phabricator via cfe-commits 
bkramer added a comment.

Changes like this should come with a small, c-index-test based, test case so we 
don't reintroduce the same bug in the future.


https://reviews.llvm.org/D36458



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

[PATCH] D36450: [X86][Ms-InlineAsm] Extend MS Dot operator to accept "this" + struct/class pointers aliases

2017-08-09 Thread coby via Phabricator via cfe-commits 
coby added inline comments.



Comment at: lib/Sema/SemaStmtAsm.cpp:702-705
+  // MS InlineAsm often uses struct pointer aliases as a base
+  const QualType QT = TD->getUnderlyingType();
+  RT = isa(QT) ? QT->getPointeeType()->getAs() :
+  QT->getAs();

rnk wrote:
> This would probably be simpler as:
>   QualType Ty = TD->getUnderlyingType();
>   if (const auto *PT = Ty->getAs())
> Ty = PT->getPointeeType();
>   RT = Ty->getAsRecordType();
> ... to avoid repeating getAs().
Indeed. thx for pointing it out!


Repository:
  rL LLVM

https://reviews.llvm.org/D36450



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

[PATCH] D36184: [clang-diff] Filter AST nodes

2017-08-09 Thread Alex Lorenz via Phabricator via cfe-commits 
arphaman added inline comments.



Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:177
+static bool isDeclExcluded(const Decl *D) { return D->isImplicit(); }
+static bool isStmtExcluded(const Stmt *S) { return false; }
+

You should just use one call  `isNodeExcluded` that will redirect to 
node-specific overload, and avoid the multiple clauses in conditions, i.e..:

```
static bool isSpecializedNodeExcluded(const Decl *D) { }
static bool isSpecializedNodeExcluded(const Stmt *S) {}

template 
static bool isNodeExcluded(const SourceManager , T *N) {
    current code ...
   return isSpecializedNodeExcluded(N);
}
```

Then you can use `if (isNodeExcluded(Tree.AST.getSourceManager(), D))` 
everywhere.


https://reviews.llvm.org/D36184



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

[PATCH] D36183: [clang-diff] Simplify mapping

2017-08-09 Thread Alex Lorenz via Phabricator via cfe-commits 
arphaman added inline comments.



Comment at: include/clang/Tooling/ASTDiff/ASTDiff.h:114
 
-  /// If this is set to true, nodes that have parents that must not be matched
-  /// (see NodeComparison) will be allowed to be matched.
-  bool EnableMatchingWithUnmatchableParents = false;
+  bool StopAfterTopDown = false;
 

It doesn't look like this field is used in this patch, please remove it or move 
it to a different patch.


https://reviews.llvm.org/D36183



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

[PATCH] D36182: [clang-diff] Add HTML side-by-side diff output

2017-08-09 Thread Alex Lorenz via Phabricator via cfe-commits 
arphaman added inline comments.



Comment at: test/Tooling/clang-diff-html.py:1
+# RUN: clang-diff %S/Inputs/clang-diff-basic-src.cpp %S/clang-diff-basic.cpp 
-html -- | %python %s > %t.filecheck
+# RUN: clang-diff %S/Inputs/clang-diff-basic-src.cpp %S/clang-diff-basic.cpp 
-dump-matches -- | FileCheck %t.filecheck

Why did you decide to use a python script for this? Can you just file check the 
HTML output directly, or do you think this is better?


https://reviews.llvm.org/D36182



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

[PATCH] D36364: [AArch64] Add support for a MinGW AArch64 target

2017-08-09 Thread Martell Malone via Phabricator via cfe-commits 
martell accepted this revision.
martell added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D36364



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

[PATCH] D36180: [clang-diff] Add option to dump the AST, one node per line

2017-08-09 Thread Alex Lorenz via Phabricator via cfe-commits 
arphaman accepted this revision.
arphaman added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D36180



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

[PATCH] D36179: [clang-diff] Move printing of matches and changes to clang-diff

2017-08-09 Thread Alex Lorenz via Phabricator via cfe-commits 
arphaman added a comment.

Previous `Move` resulted in `llvm_unreachable("TODO");`, but now it seems to be 
fixed. The new output for `Move` and `UpdateMove` should be tested in a test.


https://reviews.llvm.org/D36179



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

[PATCH] D34331: func.wrap.func.con: Unset function before destroying anything

2017-08-09 Thread Alex Lorenz via Phabricator via cfe-commits 
arphaman added a comment.

Ah, I got it. There's a `__functional_03` header that seems to implement 
`function` for C++03 because of manual variadic template expansions.
You have to update the operators in the header as well.


https://reviews.llvm.org/D34331



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

[PATCH] D34331: func.wrap.func.con: Unset function before destroying anything

2017-08-09 Thread Alex Lorenz via Phabricator via cfe-commits 
arphaman added a comment.

If you look at the AST diff between C++11 and C++03 this error still happens 
because of how `nullptr` is processed:

C++11:

  | |-CXXDestructorDecl 0x7fdab27a2498  line:24:3 used ~A 
'void (void) noexcept'
  | | `-CompoundStmt 0x7fdab27bcfb8 
  | |   |-GCCAsmStmt 0x7fdab27a2670 
  | |   `-IfStmt 0x7fdab27bcf88 
  | | |-<<>>
  | | |-ImplicitCastExpr 0x7fdab27a26e0  '_Bool' 
  | | | `-DeclRefExpr 0x7fdab27a26b8  '_Bool' lvalue Var 
0x7fdab27a23b8 'cancel' '_Bool'
  | | |-CXXOperatorCallExpr 0x7fdab27bcf40  'class 
std::__1::function' lvalue
  | | | |-ImplicitCastExpr 0x7fdab27bcf28  'class 
std::__1::function &(*)(nullptr_t) noexcept' 

  | | | | `-DeclRefExpr 0x7fdab27bcea0  'class 
std::__1::function &(nullptr_t) noexcept' lvalue CXXMethod 
0x7fdab27ab010 'operator=' 'class std::__1::function &(nullptr_t) 
noexcept'
  | | | |-DeclRefExpr 0x7fdab27a26f8  'std::function':'class std::__1::function' lvalue Var 0x7fdab27a2348 
'global' 'std::function':'class std::__1::function'
  | | | `-CXXNullPtrLiteralExpr 0x7fdab27a2720  'nullptr_t'
  | | `-<<>>

vs C++03:

  | |-CXXDestructorDecl 0x7fc72b6d80d8  line:24:3 used ~A 
'void (void)'
  | | `-CompoundStmt 0x7fc72b6dfd88 
  | |   |-GCCAsmStmt 0x7fc72b6d82a0 
  | |   `-IfStmt 0x7fc72b6dfd58 
  | | |-<<>>
  | | |-ImplicitCastExpr 0x7fc72b6d8310 

 '_Bool' 
  | | | `-DeclRefExpr 0x7fc72b6d82e8  '_Bool' lvalue Var 
0x7fc72b6d8028 'cancel' '_Bool'
  | | |-CXXOperatorCallExpr 0x7fc72b6dfd10  
'class std::__1::function' lvalue
  | | | |-ImplicitCastExpr 0x7fc72b6dfcf8 

 'class std::__1::function &(*)(struct std::__1::nullptr_t)' 

  | | | | `-DeclRefExpr 0x7fc72b6dfca0  'class 
std::__1::function &(struct std::__1::nullptr_t)' lvalue CXXMethod 
0x7fc72b6daea0 'operator=' 'class std::__1::function &(struct 
std::__1::nullptr_t)'
  | | | |-DeclRefExpr 0x7fc72b6d8328  'std::function':'class std::__1::function' lvalue Var 0x7fc72b6d7fb8 
'global' 'std::function':'class std::__1::function'
  | | | `-CXXConstructExpr 0x7fc72b6dfc68 
 
'struct std::__1::nullptr_t' 'void (const struct std::__1::nullptr_t &) 
throw()' elidable
  | | |   `-MaterializeTemporaryExpr 0x7fc72b6dfc50 
 
'const struct std::__1::nullptr_t' lvalue
  | | | `-ImplicitCastExpr 0x7fc72b6dfc38 
 
'const struct std::__1::nullptr_t' 
  | | |   `-CallExpr 0x7fc72b6d83d0 
 
'struct std::__1::nullptr_t'
  | | | `-ImplicitCastExpr 0x7fc72b6d83b8 
 
'struct std::__1::nullptr_t (*)(void)' 
  | | |   `-DeclRefExpr 0x7fc72b6d8380 
 
'struct std::__1::nullptr_t (void)' lvalue Function 0x7fc72b08d340 
'__get_nullptr_t' 'struct std::__1::nullptr_t (void)'
  | | `-<<>>


https://reviews.llvm.org/D34331



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

[PATCH] D34331: func.wrap.func.con: Unset function before destroying anything

2017-08-09 Thread Alex Lorenz via Phabricator via cfe-commits 
arphaman added a comment.

The tests actually do compile in C++03, but they still fail because of infinite 
recursion:

  frame #196562: 0x0001155c 
nullptr_t_assign_reentrant.pass.cpp.exe`A::~A() + 92
  frame #196563: 0x00011405 
nullptr_t_assign_reentrant.pass.cpp.exe`std::__1::__compressed_pair_elem::~__compressed_pair_elem() + 21
  frame #196564: 0x00012685 
nullptr_t_assign_reentrant.pass.cpp.exe`std::__1::__compressed_pair::~__compressed_pair() + 21
  frame #196565: 0x00012665 
nullptr_t_assign_reentrant.pass.cpp.exe`std::__1::__compressed_pair::~__compressed_pair() + 21
  frame #196566: 0x00012389 
nullptr_t_assign_reentrant.pass.cpp.exe`std::__1::__function::__func::destroy() + 25
  frame #196567: 0x000114b9 
nullptr_t_assign_reentrant.pass.cpp.exe`std::__1::function::operator=(std::__1::nullptr_t) + 57
  frame #196568: 0x0001155c 
nullptr_t_assign_reentrant.pass.cpp.exe`A::~A() + 92

Looks like prior to C++11 some different destruction behaviour is triggered 
which isn't fixed by this patch. So the tests should be either guarded by 
`UNSUPPORTED/XFAIL` or the patch should support C++03.


https://reviews.llvm.org/D34331



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

[PATCH] D35355: Fix templated type alias completion when using global completion cache

2017-08-09 Thread Manuel Klimek via Phabricator via cfe-commits 
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.

LG


https://reviews.llvm.org/D35355



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

[PATCH] D36359: [clang-format] Put '/**' and '*/' on own lines in jsdocs ending in comment pragmas

2017-08-09 Thread Krasimir Georgiev via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL310458: [clang-format] Put '/**' and '*/' on own lines in 
jsdocs ending in comment… (authored by krasimir).

Repository:
  rL LLVM

https://reviews.llvm.org/D36359

Files:
  cfe/trunk/lib/Format/BreakableToken.cpp
  cfe/trunk/lib/Format/BreakableToken.h
  cfe/trunk/lib/Format/ContinuationIndenter.cpp
  cfe/trunk/unittests/Format/FormatTestJS.cpp

Index: cfe/trunk/lib/Format/BreakableToken.h
===
--- cfe/trunk/lib/Format/BreakableToken.h
+++ cfe/trunk/lib/Format/BreakableToken.h
@@ -161,8 +161,8 @@
   ///
   /// A result having offset == StringRef::npos means that no reformat is
   /// necessary.
-  virtual Split getSplitAfterLastLine(unsigned TailOffset, unsigned ColumnLimit,
-  llvm::Regex ) const {
+  virtual Split getSplitAfterLastLine(unsigned TailOffset,
+  unsigned ColumnLimit) const {
 return Split(StringRef::npos, 0);
   }
 
@@ -347,8 +347,8 @@
   void replaceWhitespaceBefore(unsigned LineIndex, unsigned PreviousEndColumn,
unsigned ColumnLimit, Split SplitBefore,
WhitespaceManager ) override;
-  Split getSplitAfterLastLine(unsigned TailOffset, unsigned ColumnLimit,
-  llvm::Regex ) const override;
+  Split getSplitAfterLastLine(unsigned TailOffset,
+  unsigned ColumnLimit) const override;
 
   bool mayReflow(unsigned LineIndex,
  llvm::Regex ) const override;
Index: cfe/trunk/lib/Format/BreakableToken.cpp
===
--- cfe/trunk/lib/Format/BreakableToken.cpp
+++ cfe/trunk/lib/Format/BreakableToken.cpp
@@ -681,12 +681,18 @@
   InPPDirective, /*Newlines=*/1, ContentColumn[LineIndex] - Prefix.size());
 }
 
-BreakableToken::Split BreakableBlockComment::getSplitAfterLastLine(
-unsigned TailOffset, unsigned ColumnLimit,
-llvm::Regex ) const {
-  if (DelimitersOnNewline)
-return getSplit(Lines.size() - 1, TailOffset, ColumnLimit,
-CommentPragmasRegex);
+BreakableToken::Split
+BreakableBlockComment::getSplitAfterLastLine(unsigned TailOffset,
+ unsigned ColumnLimit) const {
+  if (DelimitersOnNewline) {
+// Replace the trailing whitespace of the last line with a newline.
+// In case the last line is empty, the ending '*/' is already on its own
+// line.
+StringRef Line = Content.back().substr(TailOffset);
+StringRef TrimmedLine = Line.rtrim(Blanks);
+if (!TrimmedLine.empty())
+  return Split(TrimmedLine.size(), Line.size() - TrimmedLine.size());
+  }
   return Split(StringRef::npos, 0);
 }
 
Index: cfe/trunk/lib/Format/ContinuationIndenter.cpp
===
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp
@@ -1383,8 +1383,8 @@
 }
   }
 
-  BreakableToken::Split SplitAfterLastLine = Token->getSplitAfterLastLine(
-  TailOffset, ColumnLimit, CommentPragmasRegex);
+  BreakableToken::Split SplitAfterLastLine =
+  Token->getSplitAfterLastLine(TailOffset, ColumnLimit);
   if (SplitAfterLastLine.first != StringRef::npos) {
 if (!DryRun)
   Token->replaceWhitespaceAfterLastLine(TailOffset, SplitAfterLastLine,
Index: cfe/trunk/unittests/Format/FormatTestJS.cpp
===
--- cfe/trunk/unittests/Format/FormatTestJS.cpp
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp
@@ -158,6 +158,53 @@
"var x = 1;\n"
"}",
getGoogleJSStyleWithColumns(20)));
+
+  // Don't break the first line of a single line short jsdoc comment pragma.
+  EXPECT_EQ("/** @returns j */",
+format("/** @returns j */",
+   getGoogleJSStyleWithColumns(20)));
+
+  // Break a single line long jsdoc comment pragma.
+  EXPECT_EQ("/**\n"
+" * @returns {string} jsdoc line 12\n"
+" */",
+format("/** @returns {string} jsdoc line 12 */",
+   getGoogleJSStyleWithColumns(20)));
+
+  EXPECT_EQ("/**\n"
+" * @returns {string} jsdoc line 12\n"
+" */",
+format("/** @returns {string} jsdoc line 12  */",
+   getGoogleJSStyleWithColumns(20)));
+
+  EXPECT_EQ("/**\n"
+" * @returns {string} jsdoc line 12\n"
+" */",
+format("/** @returns {string} jsdoc line 12*/",
+   getGoogleJSStyleWithColumns(20)));
+
+  // Fix a multiline jsdoc comment ending in a comment pragma.
+  EXPECT_EQ("/**\n"
+" * line 1\n"
+" * line 2\n"
+" * @returns {string} jsdoc line 12\n"
+" */",
+format("/**

r310458 - [clang-format] Put '/**' and '*/' on own lines in jsdocs ending in comment pragmas

2017-08-09 Thread Krasimir Georgiev via cfe-commits 
Author: krasimir
Date: Wed Aug  9 02:42:32 2017
New Revision: 310458

URL: http://llvm.org/viewvc/llvm-project?rev=310458=rev
Log:
[clang-format] Put '/**' and '*/' on own lines in jsdocs ending in comment 
pragmas

Summary:
This handles a case where the trailing '*/' of a multiline jsdoc eding in a
comment pragma wouldn't be put on a new line.

Reviewers: mprobst

Reviewed By: mprobst

Subscribers: cfe-commits, klimek

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

Modified:
cfe/trunk/lib/Format/BreakableToken.cpp
cfe/trunk/lib/Format/BreakableToken.h
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp

Modified: cfe/trunk/lib/Format/BreakableToken.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.cpp?rev=310458=310457=310458=diff
==
--- cfe/trunk/lib/Format/BreakableToken.cpp (original)
+++ cfe/trunk/lib/Format/BreakableToken.cpp Wed Aug  9 02:42:32 2017
@@ -681,12 +681,18 @@ void BreakableBlockComment::replaceWhite
   InPPDirective, /*Newlines=*/1, ContentColumn[LineIndex] - Prefix.size());
 }
 
-BreakableToken::Split BreakableBlockComment::getSplitAfterLastLine(
-unsigned TailOffset, unsigned ColumnLimit,
-llvm::Regex ) const {
-  if (DelimitersOnNewline)
-return getSplit(Lines.size() - 1, TailOffset, ColumnLimit,
-CommentPragmasRegex);
+BreakableToken::Split
+BreakableBlockComment::getSplitAfterLastLine(unsigned TailOffset,
+ unsigned ColumnLimit) const {
+  if (DelimitersOnNewline) {
+// Replace the trailing whitespace of the last line with a newline.
+// In case the last line is empty, the ending '*/' is already on its own
+// line.
+StringRef Line = Content.back().substr(TailOffset);
+StringRef TrimmedLine = Line.rtrim(Blanks);
+if (!TrimmedLine.empty())
+  return Split(TrimmedLine.size(), Line.size() - TrimmedLine.size());
+  }
   return Split(StringRef::npos, 0);
 }
 

Modified: cfe/trunk/lib/Format/BreakableToken.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.h?rev=310458=310457=310458=diff
==
--- cfe/trunk/lib/Format/BreakableToken.h (original)
+++ cfe/trunk/lib/Format/BreakableToken.h Wed Aug  9 02:42:32 2017
@@ -161,8 +161,8 @@ public:
   ///
   /// A result having offset == StringRef::npos means that no reformat is
   /// necessary.
-  virtual Split getSplitAfterLastLine(unsigned TailOffset, unsigned 
ColumnLimit,
-  llvm::Regex ) const {
+  virtual Split getSplitAfterLastLine(unsigned TailOffset,
+  unsigned ColumnLimit) const {
 return Split(StringRef::npos, 0);
   }
 
@@ -347,8 +347,8 @@ public:
   void replaceWhitespaceBefore(unsigned LineIndex, unsigned PreviousEndColumn,
unsigned ColumnLimit, Split SplitBefore,
WhitespaceManager ) override;
-  Split getSplitAfterLastLine(unsigned TailOffset, unsigned ColumnLimit,
-  llvm::Regex ) const override;
+  Split getSplitAfterLastLine(unsigned TailOffset,
+  unsigned ColumnLimit) const override;
 
   bool mayReflow(unsigned LineIndex,
  llvm::Regex ) const override;

Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=310458=310457=310458=diff
==
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Wed Aug  9 02:42:32 2017
@@ -1383,8 +1383,8 @@ unsigned ContinuationIndenter::breakProt
 }
   }
 
-  BreakableToken::Split SplitAfterLastLine = Token->getSplitAfterLastLine(
-  TailOffset, ColumnLimit, CommentPragmasRegex);
+  BreakableToken::Split SplitAfterLastLine =
+  Token->getSplitAfterLastLine(TailOffset, ColumnLimit);
   if (SplitAfterLastLine.first != StringRef::npos) {
 if (!DryRun)
   Token->replaceWhitespaceAfterLastLine(TailOffset, SplitAfterLastLine,

Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=310458=310457=310458=diff
==
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Wed Aug  9 02:42:32 2017
@@ -158,6 +158,53 @@ TEST_F(FormatTestJS, JSDocComments) {
"var x = 1;\n"
"}",
getGoogleJSStyleWithColumns(20)));
+
+  // Don't break the first line of a single line short jsdoc comment pragma.
+  EXPECT_EQ("/** @returns j

[PATCH] D34324: [clang-format] let PointerAlignment dictate spacing of function ref qualifiers

2017-08-09 Thread Daniel Jasper via Phabricator via cfe-commits 
djasper accepted this revision.
djasper added a comment.
This revision is now accepted and ready to land.

Looks good. Thank you!


https://reviews.llvm.org/D34324



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

[PATCH] D36407: [Sema] Extend -Wenum-compare to handle mixed enum comparisons in switch statements

2017-08-09 Thread Phabricator via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL310449: [Sema] Extend -Wenum-compare to handle mixed enum 
comparisons in switch… (authored by xazax).

Changed prior to commit:
  https://reviews.llvm.org/D36407?vs=110219=110333#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D36407

Files:
  cfe/trunk/lib/Sema/SemaStmt.cpp
  cfe/trunk/test/Sema/switch.c
  cfe/trunk/test/SemaCXX/warn-enum-compare.cpp

Index: cfe/trunk/test/SemaCXX/warn-enum-compare.cpp
===
--- cfe/trunk/test/SemaCXX/warn-enum-compare.cpp
+++ cfe/trunk/test/SemaCXX/warn-enum-compare.cpp
@@ -209,4 +209,21 @@
   while (getBar() > x); // expected-warning  {{comparison of two values with different enumeration types ('Bar' and 'Foo')}}
   while (getBar() < x); // expected-warning  {{comparison of two values with different enumeration types ('Bar' and 'Foo')}}
 
+  switch (a) {
+case name1::F1: break;
+case name1::F3: break;
+case name2::B2: break; // expected-warning {{comparison of two values with different enumeration types ('name1::Foo' and 'name2::Baz')}}
+  }
+
+  switch (x) {
+case FooB: break;
+case FooC: break;
+case BarD: break; // expected-warning {{comparison of two values with different enumeration types ('Foo' and 'Bar')}}
+  }
+
+  switch(getBar()) {
+case BarE: break;
+case BarF: break;
+case FooA: break; // expected-warning {{comparison of two values with different enumeration types ('Bar' and 'Foo')}}
+  }
 }
Index: cfe/trunk/test/Sema/switch.c
===
--- cfe/trunk/test/Sema/switch.c
+++ cfe/trunk/test/Sema/switch.c
@@ -372,6 +372,7 @@
   case EE1_b: break;
   case EE1_c: break; // no-warning
   case EE1_d: break; // expected-warning {{case value not in enumerated type 'enum ExtendedEnum1'}}
+  // expected-warning@-1 {{comparison of two values with different enumeration types ('enum ExtendedEnum1' and 'const enum ExtendedEnum1_unrelated')}}
   }
 }
 
Index: cfe/trunk/lib/Sema/SemaStmt.cpp
===
--- cfe/trunk/lib/Sema/SemaStmt.cpp
+++ cfe/trunk/lib/Sema/SemaStmt.cpp
@@ -602,14 +602,14 @@
 
 /// GetTypeBeforeIntegralPromotion - Returns the pre-promotion type of
 /// potentially integral-promoted expression @p expr.
-static QualType GetTypeBeforeIntegralPromotion(Expr *) {
-  if (ExprWithCleanups *cleanups = dyn_cast(expr))
-expr = cleanups->getSubExpr();
-  while (ImplicitCastExpr *impcast = dyn_cast(expr)) {
-if (impcast->getCastKind() != CK_IntegralCast) break;
-expr = impcast->getSubExpr();
+static QualType GetTypeBeforeIntegralPromotion(const Expr *) {
+  if (const auto *CleanUps = dyn_cast(E))
+E = CleanUps->getSubExpr();
+  while (const auto *ImpCast = dyn_cast(E)) {
+if (ImpCast->getCastKind() != CK_IntegralCast) break;
+E = ImpCast->getSubExpr();
   }
-  return expr->getType();
+  return E->getType();
 }
 
 ExprResult Sema::CheckSwitchCondition(SourceLocation SwitchLoc, Expr *Cond) {
@@ -743,6 +743,24 @@
   return true;
 }
 
+static void checkEnumTypesInSwitchStmt(Sema , const Expr *Cond,
+   const Expr *Case) {
+  QualType CondType = GetTypeBeforeIntegralPromotion(Cond);
+  QualType CaseType = Case->getType();
+
+  const EnumType *CondEnumType = CondType->getAs();
+  const EnumType *CaseEnumType = CaseType->getAs();
+  if (!CondEnumType || !CaseEnumType)
+return;
+
+  if (S.Context.hasSameUnqualifiedType(CondType, CaseType))
+return;
+
+  S.Diag(Case->getExprLoc(), diag::warn_comparison_of_mixed_enum_types)
+  << CondType << CaseType << Cond->getSourceRange()
+  << Case->getSourceRange();
+}
+
 StmtResult
 Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Switch,
 Stmt *BodyStmt) {
@@ -760,7 +778,7 @@
 
   QualType CondType = CondExpr->getType();
 
-  Expr *CondExprBeforePromotion = CondExpr;
+  const Expr *CondExprBeforePromotion = CondExpr;
   QualType CondTypeBeforePromotion =
   GetTypeBeforeIntegralPromotion(CondExprBeforePromotion);
 
@@ -843,6 +861,8 @@
 break;
   }
 
+  checkEnumTypesInSwitchStmt(*this, CondExpr, Lo);
+
   llvm::APSInt LoVal;
 
   if (getLangOpts().CPlusPlus11) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r310449 - [Sema] Extend -Wenum-compare to handle mixed enum comparisons in switch statements

2017-08-09 Thread Gabor Horvath via cfe-commits 
Author: xazax
Date: Wed Aug  9 01:57:09 2017
New Revision: 310449

URL: http://llvm.org/viewvc/llvm-project?rev=310449=rev
Log:
[Sema] Extend -Wenum-compare to handle mixed enum comparisons in switch 
statements

Patch by: Reka Nikolett Kovacs

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

Modified:
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/test/Sema/switch.c
cfe/trunk/test/SemaCXX/warn-enum-compare.cpp

Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=310449=310448=310449=diff
==
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Wed Aug  9 01:57:09 2017
@@ -602,14 +602,14 @@ static bool EqEnumVals(const std::pair(expr))
-expr = cleanups->getSubExpr();
-  while (ImplicitCastExpr *impcast = dyn_cast(expr)) {
-if (impcast->getCastKind() != CK_IntegralCast) break;
-expr = impcast->getSubExpr();
+static QualType GetTypeBeforeIntegralPromotion(const Expr *) {
+  if (const auto *CleanUps = dyn_cast(E))
+E = CleanUps->getSubExpr();
+  while (const auto *ImpCast = dyn_cast(E)) {
+if (ImpCast->getCastKind() != CK_IntegralCast) break;
+E = ImpCast->getSubExpr();
   }
-  return expr->getType();
+  return E->getType();
 }
 
 ExprResult Sema::CheckSwitchCondition(SourceLocation SwitchLoc, Expr *Cond) {
@@ -743,6 +743,24 @@ static bool ShouldDiagnoseSwitchCaseNotI
   return true;
 }
 
+static void checkEnumTypesInSwitchStmt(Sema , const Expr *Cond,
+   const Expr *Case) {
+  QualType CondType = GetTypeBeforeIntegralPromotion(Cond);
+  QualType CaseType = Case->getType();
+
+  const EnumType *CondEnumType = CondType->getAs();
+  const EnumType *CaseEnumType = CaseType->getAs();
+  if (!CondEnumType || !CaseEnumType)
+return;
+
+  if (S.Context.hasSameUnqualifiedType(CondType, CaseType))
+return;
+
+  S.Diag(Case->getExprLoc(), diag::warn_comparison_of_mixed_enum_types)
+  << CondType << CaseType << Cond->getSourceRange()
+  << Case->getSourceRange();
+}
+
 StmtResult
 Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Switch,
 Stmt *BodyStmt) {
@@ -760,7 +778,7 @@ Sema::ActOnFinishSwitchStmt(SourceLocati
 
   QualType CondType = CondExpr->getType();
 
-  Expr *CondExprBeforePromotion = CondExpr;
+  const Expr *CondExprBeforePromotion = CondExpr;
   QualType CondTypeBeforePromotion =
   GetTypeBeforeIntegralPromotion(CondExprBeforePromotion);
 
@@ -843,6 +861,8 @@ Sema::ActOnFinishSwitchStmt(SourceLocati
 break;
   }
 
+  checkEnumTypesInSwitchStmt(*this, CondExpr, Lo);
+
   llvm::APSInt LoVal;
 
   if (getLangOpts().CPlusPlus11) {

Modified: cfe/trunk/test/Sema/switch.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/switch.c?rev=310449=310448=310449=diff
==
--- cfe/trunk/test/Sema/switch.c (original)
+++ cfe/trunk/test/Sema/switch.c Wed Aug  9 01:57:09 2017
@@ -372,6 +372,7 @@ void switch_on_ExtendedEnum1(enum Extend
   case EE1_b: break;
   case EE1_c: break; // no-warning
   case EE1_d: break; // expected-warning {{case value not in enumerated type 
'enum ExtendedEnum1'}}
+  // expected-warning@-1 {{comparison of two values with different enumeration 
types ('enum ExtendedEnum1' and 'const enum ExtendedEnum1_unrelated')}}
   }
 }
 

Modified: cfe/trunk/test/SemaCXX/warn-enum-compare.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-enum-compare.cpp?rev=310449=310448=310449=diff
==
--- cfe/trunk/test/SemaCXX/warn-enum-compare.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-enum-compare.cpp Wed Aug  9 01:57:09 2017
@@ -209,4 +209,21 @@ void test () {
   while (getBar() > x); // expected-warning  {{comparison of two values with 
different enumeration types ('Bar' and 'Foo')}}
   while (getBar() < x); // expected-warning  {{comparison of two values with 
different enumeration types ('Bar' and 'Foo')}}
 
+  switch (a) {
+case name1::F1: break;
+case name1::F3: break;
+case name2::B2: break; // expected-warning {{comparison of two values with 
different enumeration types ('name1::Foo' and 'name2::Baz')}}
+  }
+
+  switch (x) {
+case FooB: break;
+case FooC: break;
+case BarD: break; // expected-warning {{comparison of two values with 
different enumeration types ('Foo' and 'Bar')}}
+  }
+
+  switch(getBar()) {
+case BarE: break;
+case BarF: break;
+case FooA: break; // expected-warning {{comparison of two values with 
different enumeration types ('Bar' and 'Foo')}}
+  }
 }


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

[PATCH] D35955: clang-format: Add preprocessor directive indentation

2017-08-09 Thread Manuel Klimek via Phabricator via cfe-commits 
klimek added a comment.

In https://reviews.llvm.org/D35955#835994, @euhlmann wrote:

> In https://reviews.llvm.org/D35955#835439, @klimek wrote:
>
> > I think if we need this info, we can just make it count down to -1 again 
> > (or, but that's isomorphic, let it run from 0 and make sure we never index 
> > into the data structures at 0 :)
>
>
> Should I do one of these things? Let me know how you'd like me to implement 
> this.


So my suggestion would be to let it count down to -1 again and put a check 
around indexing into the data structures.


https://reviews.llvm.org/D35955



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

Re: [PATCH] D36386: [clang] Remove unit test which uses reverse-iterate and fix a PointerLikeTypeTrait specialization

2017-08-09 Thread Grang, Mandeep Singh via cfe-commits 
In D35043 I have removed the llvm tests which use -reverse-iterate. This 
patch removes the clang tests.


Should I post a later patch to change all "class PointerLikeTypeTraits" 
to "struct PointerLikeTypeTraits"?


On 8/7/2017 2:50 PM, David Blaikie wrote:



On Mon, Aug 7, 2017 at 12:08 PM Mandeep Singh Grang via Phabricator 
> wrote:


mgrang added a comment.

This patch does 3 things:

1. Get rid of the unit test objc-modern-metadata-visibility2.mm
 because this test
check uses flag -reverse-iterate. This flag will be removed in
https://reviews.llvm.org/D35043.


Sure - please commit that separately (probably once D35043 is approved 
- probably best to include that removal in D35043, or a separate patch 
that /only/ removes the -reverse-iterate flag (& any tests that use 
it) as a standalone change?).


Does this test need a replacement? If this test is removed and the 
underlying issue it was testing regresses, will one of the buildbots 
(reverse or normal) catch the problem?


2. https://reviews.llvm.org/D35043 gets rid of the empty base
definition for PointerLikeTypeTraits. This results in a compiler
warning because PointerLikeTypeTrait has been defined as struct
here while in the header it is a class. So I have changed struct
to class.


I'd probably go the other way - traits classes like this make more 
sense as structs, I think - it only has public members & no 
implementation really has any need for supporting private members.


3. Since I changed struct PointerLikeTypeTrait to class
PointerLikeTypeTrait here, the member functions are no longer
public now. This results in a compiler error. So I explicitly
marked them as public here.


https://reviews.llvm.org/D36386





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

[PATCH] D16403: Add scope information to CFG for If/While/For/Do/Compound/CXXRangeFor statements

2017-08-09 Thread Maxim Ostapenko via Phabricator via cfe-commits 
m.ostapenko updated this revision to Diff 110329.
m.ostapenko added a comment.

Rebased and ping.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403

Files:
  include/clang/Analysis/AnalysisContext.h
  include/clang/Analysis/CFG.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/CFG.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/scopes-cfg-output.cpp

Index: test/Analysis/scopes-cfg-output.cpp
===
--- /dev/null
+++ test/Analysis/scopes-cfg-output.cpp
@@ -0,0 +1,1098 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-scopes=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+class A {
+public:
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  A() {}
+
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  ~A() {}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2: 1
+// CHECK-NEXT:   3: return [B1.2];
+// CHECK-NEXT:   4: CFGScopeEnd(ReturnStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  operator int() const { return 1; }
+};
+
+int getX();
+extern const bool UV;
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4: a
+// CHECK-NEXT:   5: [B1.4] (ImplicitCastExpr, NoOp, const class A)
+// CHECK-NEXT:   6: const A  = a;
+// CHECK-NEXT:   7: A() (CXXConstructExpr, class A)
+// CHECK-NEXT:   8: [B1.7] (BindTemporary)
+// CHECK-NEXT:   9: [B1.8] (ImplicitCastExpr, NoOp, const class A)
+// CHECK-NEXT:  10: [B1.9]
+// CHECK-NEXT:  11: const A  = A();
+// CHECK-NEXT:  12: [B1.11].~A() (Implicit destructor)
+// CHECK-NEXT:  13: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:  14: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+void test_const_ref() {
+  A a;
+  const A& b = a;
+  const A& c = A();
+}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A [2])
+// CHECK-NEXT:   3: A a[2];
+// CHECK-NEXT:   4:  (CXXConstructExpr, class A [0])
+// CHECK-NEXT:   5: A b[0];
+// CHECK-NEXT:   6: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:   7: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_array() {
+  A a[2];
+  A b[0];
+}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   5:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   6: A c;
+// CHECK-NEXT:   7:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   8: A d;
+// CHECK-NEXT:   9: [B1.8].~A() (Implicit destructor)
+// CHECK-NEXT:  10: [B1.6].~A() (Implicit destructor)
+// CHECK-NEXT:  11: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:  12:  (CXXConstructExpr, class A)
+// CHECK-NEXT:  13: A b;
+// CHECK-NEXT:  14: [B1.13].~A() (Implicit destructor)
+// CHECK-NEXT:  15: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:  16: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_scope() {
+  A a;
+  { A c;
+A d;
+  }
+  A b;
+}
+
+// CHECK:  [B4 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B3
+// CHECK:  [B1]
+// CHECK-NEXT:   1:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   2: A c;
+// CHECK-NEXT:   3: [B1.2].~A() (Implicit destructor)
+// CHECK-NEXT:   4: [B3.5].~A() (Implicit destructor)
+// CHECK-NEXT:   5: [B3.3].~A() (Implicit destructor)
+// CHECK-NEXT:   6: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B2]
+// CHECK-NEXT:   1: CFGScopeBegin(IfStmt)
+// CHECK-NEXT:   2: return;
+// CHECK-NEXT:   3: [B3.5].~A() (Implicit destructor)
+// CHECK-NEXT:   4: [B3.3].~A() (Implicit destructor)
+// CHECK-NEXT:   5: CFGScopeEnd(ReturnStmt)
+// CHECK-NEXT:   Preds (1): B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B3]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)

r310445 - Attempt #2 to appease buildbots

2017-08-09 Thread George Burgess IV via cfe-commits 
Author: gbiv
Date: Tue Aug  8 23:07:08 2017
New Revision: 310445

URL: http://llvm.org/viewvc/llvm-project?rev=310445=rev
Log:
Attempt #2 to appease buildbots

"error: unable to create target: 'No available targets are compatible
with this triple.'"

Modified:
cfe/trunk/test/CodeGenCXX/pr29160.cpp

Modified: cfe/trunk/test/CodeGenCXX/pr29160.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/pr29160.cpp?rev=310445=310444=310445=diff
==
--- cfe/trunk/test/CodeGenCXX/pr29160.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/pr29160.cpp Tue Aug  8 23:07:08 2017
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++11 -triple i686-linux %s -o /dev/null -S
+// RUN: %clang_cc1 -std=c++11 -triple i686-linux-gnu %s -o /dev/null -S 
-emit-llvm
 //
 // This test's failure mode is running ~forever. (For some value of "forever"
 // that's greater than 25 minutes on my machine)


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

<    1   2