[PATCH] D66990: [clangd] Add distinct highlightings for declarations of functions and methods

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom added a comment.

Should we have different highlightings for declarations vs usages? (Although I 
guess in the end it will be up to the editor if they highlight them differently 
as the scope is just more specific for declarations)
I guess I personally don't really see the reason as it should be clear from the 
context if it's a declaration or a function call.
But this might actually be nice if you have macros that declare functions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66990



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

[PATCH] D66732: [Clangd] ExtractFunction Added checks for broken control flow

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:433
+// inside Extraction Zone.
+void incrementLoopSwitchCounters(Stmt *S) {
+  if (CurrentLocation != ZoneRelative::Inside)

rather than writing this twice, take an `int Increment` parameter?



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:480
+  if (CurrentLocation == ZoneRelative::Inside &&
+  !(CurNumberOfLoops + CurNumberOfSwitch))
+Info.BrokenControlFlow = true;

`(NumberOfLoops > 0 || NumberOfSwitch = 0)`



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:497
+// stack inside Extraction Zone. Used to check for broken control flow.
+unsigned CurNumberOfLoops = 0;
+unsigned CurNumberOfSwitch = 0;

NestedLoops would be clearer I think



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:523
   EXPECT_THAT(apply(" for([[int i = 0;]];);"), HasSubstr("extracted"));
+  // We should be able to extract break/continue with a parent loop/switch.
+  EXPECT_THAT(apply(" [[for(;;) if(1) break;]] "), HasSubstr("extracted"));

please add these to a new test case e.g. `TEST_F(ExtractFunctionTest, 
ControlFlow)` to avoid each getting too long


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66732



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

[PATCH] D66990: [clangd] Add distinct highlightings for declarations of functions and methods

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D66990#1652172 , @jvikstrom wrote:

> Should we have different highlightings for declarations vs usages? (Although 
> I guess in the end it will be up to the editor if they highlight them 
> differently as the scope is just more specific for declarations)
>  I guess I personally don't really see the reason as it should be clear from 
> the context if it's a declaration or a function call.


I also second the opinion that usages and declarations should be highlighted 
the same.
Both represent the **same** thing, therefore they should have the **same** 
color.

And this highlighting does not seem to add any extra information too - it's 
obvious from the code whether a name is a declaration or a usage.
Even in the macro cases, the users usually know the semantics of the macro or 
can infer from the name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66990



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

[PATCH] D66948: [CodeGen]: fix error message for "=r" asm constraint

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 218037.
glider added a comment.

Minor comment fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66948

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/x86_64-PR42672.c


Index: clang/test/CodeGen/x86_64-PR42672.c
===
--- clang/test/CodeGen/x86_64-PR42672.c
+++ clang/test/CodeGen/x86_64-PR42672.c
@@ -4,6 +4,7 @@
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_BIG 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_BIG
 // RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DPOSSIBLE_X -emit-llvm %s 
-o - 2>&1 | FileCheck %s --check-prefix=CHECK-X
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_X 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_X
+// RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_9BYTES 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_9BYTES
 
 // Make sure Clang doesn't treat |lockval| as asm input.
 void _raw_spin_lock(void) {
@@ -57,7 +58,7 @@
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm
+// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm: can't store value into 
a register
 
 // Check Clang reports an error if attempting to return a big structure via a 
register.
 void big_struct(void) {
@@ -69,7 +70,7 @@
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm
+// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm: can't store value into 
a register
 
 // Clang is able to emit LLVM IR for an 16-byte structure.
 void x_constraint_fit() {
@@ -100,3 +101,17 @@
 }
 
 // CHECK-IMPOSSIBLE_X: invalid output size for constraint
+
+// http://crbug.com/999160
+// Clang used to report the following message:
+//   "impossible constraint in asm: can't store struct into a register"
+// for the assembly directive below, although there's no struct.
+void crbug_999160_regtest() {
+#ifdef IMPOSSIBLE_9BYTES
+  char buf[9];
+  asm(""
+  : "=r"(buf));
+#endif
+}
+
+// CHECK-IMPOSSIBLE_9BYTES: impossible constraint in asm: can't store value 
into a register
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2326,7 +2326,7 @@
 const Expr *OutExpr = S.getOutputExpr(i);
 CGM.Error(
 OutExpr->getExprLoc(),
-"impossible constraint in asm: can't store struct into a 
register");
+"impossible constraint in asm: can't store value into a register");
 return;
   }
   Dest = MakeAddrLValue(A, Ty);


Index: clang/test/CodeGen/x86_64-PR42672.c
===
--- clang/test/CodeGen/x86_64-PR42672.c
+++ clang/test/CodeGen/x86_64-PR42672.c
@@ -4,6 +4,7 @@
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_BIG -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_BIG
 // RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DPOSSIBLE_X -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-X
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_X -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_X
+// RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_9BYTES -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_9BYTES
 
 // Make sure Clang doesn't treat |lockval| as asm input.
 void _raw_spin_lock(void) {
@@ -57,7 +58,7 @@
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm
+// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm: can't store value into a register
 
 // Check Clang reports an error if attempting to return a big structure via a register.
 void big_struct(void) {
@@ -69,7 +70,7 @@
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm
+// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm: can't store value into a register
 
 // Clang is able to emit LLVM IR for an 16-byte structure.
 void x_constraint_fit() {
@@ -100,3 +101,17 @@
 }
 
 // CHECK-IMPOSSIBLE_X: invalid output size for constraint
+
+// http://crbug.com/999160
+// Clang used to report the following message:
+//   "impossible constraint in asm: can't store struct into a register"
+// for the assembly directive below, although there's no struct.
+void crbug_999160_regtest() {
+#ifdef IMPOSSIBLE_9BYTES
+  char buf[9];
+  asm(""
+  : "=r"(buf));
+#endif
+}
+
+// CHECK-IMPOSSIBLE_9BYTES: impossible constraint in asm: can't store value into a register
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2326,7 +2326,7 @@

r370444 - [CodeGen]: fix error message for "=r" asm constraint

[Alexander Potapenko via cfe-commits]

Author: glider
Date: Fri Aug 30 01:58:46 2019
New Revision: 370444

URL: http://llvm.org/viewvc/llvm-project?rev=370444&view=rev
Log:
[CodeGen]: fix error message for "=r" asm constraint

Summary:
Nico Weber reported that the following code:
  char buf[9];
  asm("" : "=r" (buf));

yields the "impossible constraint in asm: can't store struct into a register"
error message, although |buf| is not a struct (see
http://crbug.com/999160).

Make the error message more generic and add a test for it.
Also make sure other tests in x86_64-PR42672.c check for the full error
message.

Reviewers: eli.friedman, thakis

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/CodeGen/CGStmt.cpp
cfe/trunk/test/CodeGen/x86_64-PR42672.c

Modified: cfe/trunk/lib/CodeGen/CGStmt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmt.cpp?rev=370444&r1=370443&r2=370444&view=diff
==
--- cfe/trunk/lib/CodeGen/CGStmt.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGStmt.cpp Fri Aug 30 01:58:46 2019
@@ -2326,7 +2326,7 @@ void CodeGenFunction::EmitAsmStmt(const
 const Expr *OutExpr = S.getOutputExpr(i);
 CGM.Error(
 OutExpr->getExprLoc(),
-"impossible constraint in asm: can't store struct into a 
register");
+"impossible constraint in asm: can't store value into a register");
 return;
   }
   Dest = MakeAddrLValue(A, Ty);

Modified: cfe/trunk/test/CodeGen/x86_64-PR42672.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/x86_64-PR42672.c?rev=370444&r1=370443&r2=370444&view=diff
==
--- cfe/trunk/test/CodeGen/x86_64-PR42672.c (original)
+++ cfe/trunk/test/CodeGen/x86_64-PR42672.c Fri Aug 30 01:58:46 2019
@@ -4,6 +4,7 @@
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_BIG 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_BIG
 // RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DPOSSIBLE_X -emit-llvm %s 
-o - 2>&1 | FileCheck %s --check-prefix=CHECK-X
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_X 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_X
+// RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_9BYTES 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_9BYTES
 
 // Make sure Clang doesn't treat |lockval| as asm input.
 void _raw_spin_lock(void) {
@@ -57,7 +58,7 @@ void odd_struct(void) {
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm
+// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm: can't store value into 
a register
 
 // Check Clang reports an error if attempting to return a big structure via a 
register.
 void big_struct(void) {
@@ -69,7 +70,7 @@ void big_struct(void) {
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm
+// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm: can't store value into 
a register
 
 // Clang is able to emit LLVM IR for an 16-byte structure.
 void x_constraint_fit() {
@@ -100,3 +101,17 @@ void x_constraint_nofit() {
 }
 
 // CHECK-IMPOSSIBLE_X: invalid output size for constraint
+
+// http://crbug.com/999160
+// Clang used to report the following message:
+//   "impossible constraint in asm: can't store struct into a register"
+// for the assembly directive below, although there's no struct.
+void crbug_999160_regtest() {
+#ifdef IMPOSSIBLE_9BYTES
+  char buf[9];
+  asm(""
+  : "=r"(buf));
+#endif
+}
+
+// CHECK-IMPOSSIBLE_9BYTES: impossible constraint in asm: can't store value 
into a register


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

[PATCH] D66933: [ASTImporter] Propagate errors during import of overridden methods.

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 218039.
balazske added a comment.

- Renamed ImportOverrides, additional check in test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66933

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -5183,6 +5183,50 @@
   }
 }
 
+TEST_P(ErrorHandlingTest, ImportOfOverriddenMethods) {
+  auto MatchFooA =
+  functionDecl(hasName("foo"), hasAncestor(cxxRecordDecl(hasName("A";
+  auto MatchFooB =
+  functionDecl(hasName("foo"), hasAncestor(cxxRecordDecl(hasName("B";
+  auto MatchFooC =
+  functionDecl(hasName("foo"), hasAncestor(cxxRecordDecl(hasName("C";
+
+  // Provoke import of a method that has overridden methods with import error.
+  TranslationUnitDecl *FromTU = getTuDecl(std::string(R"(
+struct C;
+struct A {
+  virtual void foo();
+  void f1(C *);
+};
+void A::foo() {
+  )") + ErroneousStmt + R"(
+}
+struct B : public A {
+  void foo() override;
+};
+struct C : public B {
+  void foo() override;
+};
+)",
+  Lang_CXX11);
+  auto *FromFooA = FirstDeclMatcher().match(FromTU, MatchFooA);
+  auto *FromFooB = FirstDeclMatcher().match(FromTU, MatchFooB);
+  auto *FromFooC = FirstDeclMatcher().match(FromTU, MatchFooC);
+
+  EXPECT_FALSE(Import(FromFooA, Lang_CXX11));
+  ASTImporter *Importer = findFromTU(FromFooA)->Importer.get();
+  auto CheckError = [&Importer](Decl *FromD) {
+Optional OptErr = Importer->getImportDeclErrorIfAny(FromD);
+ASSERT_TRUE(OptErr);
+EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+  };
+  CheckError(FromFooA);
+  EXPECT_FALSE(Import(FromFooB, Lang_CXX11));
+  CheckError(FromFooB);
+  EXPECT_FALSE(Import(FromFooC, Lang_CXX11));
+  CheckError(FromFooC);
+}
+
 TEST_P(ASTImporterOptionSpecificTestBase, LambdaInFunctionBody) {
   Decl *FromTU = getTuDecl(
   R"(
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -639,7 +639,8 @@
   return ImportArrayChecked(InContainer.begin(), InContainer.end(), Obegin);
 }
 
-void ImportOverrides(CXXMethodDecl *ToMethod, CXXMethodDecl *FromMethod);
+Error ImportOverriddenMethods(CXXMethodDecl *ToMethod,
+  CXXMethodDecl *FromMethod);
 
 Expected FindFunctionTemplateSpecialization(
 FunctionDecl *FromFD);
@@ -3370,7 +3371,9 @@
   }
 
   if (auto *FromCXXMethod = dyn_cast(D))
-ImportOverrides(cast(ToFunction), FromCXXMethod);
+if (Error Err = ImportOverriddenMethods(cast(ToFunction),
+FromCXXMethod))
+  return std::move(Err);
 
   // Import the rest of the chain. I.e. import all subsequent declarations.
   for (++RedeclIt; RedeclIt != Redecls.end(); ++RedeclIt) {
@@ -7804,15 +7807,18 @@
   *ToTypeOrErr, *ToExprOperandOrErr, *ToSourceRangeOrErr);
 }
 
-void ASTNodeImporter::ImportOverrides(CXXMethodDecl *ToMethod,
-  CXXMethodDecl *FromMethod) {
+Error ASTNodeImporter::ImportOverriddenMethods(CXXMethodDecl *ToMethod,
+   CXXMethodDecl *FromMethod) {
+  Error ImportErrors = Error::success();
   for (auto *FromOverriddenMethod : FromMethod->overridden_methods()) {
 if (auto ImportedOrErr = import(FromOverriddenMethod))
   ToMethod->getCanonicalDecl()->addOverriddenMethod(cast(
   (*ImportedOrErr)->getCanonicalDecl()));
 else
-  consumeError(ImportedOrErr.takeError());
+  ImportErrors =
+  joinErrors(std::move(ImportErrors), ImportedOrErr.takeError());
   }
+  return ImportErrors;
 }
 
 ASTImporter::ASTImporter(ASTContext &ToContext, FileManager &ToFileManager,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66948: [CodeGen]: fix error message for "=r" asm constraint

[Alexander Potapenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370444: [CodeGen]: fix error message for "=r" asm 
constraint (authored by glider, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66948?vs=218037&id=218040#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66948

Files:
  cfe/trunk/lib/CodeGen/CGStmt.cpp
  cfe/trunk/test/CodeGen/x86_64-PR42672.c


Index: cfe/trunk/test/CodeGen/x86_64-PR42672.c
===
--- cfe/trunk/test/CodeGen/x86_64-PR42672.c
+++ cfe/trunk/test/CodeGen/x86_64-PR42672.c
@@ -4,6 +4,7 @@
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_BIG 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_BIG
 // RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DPOSSIBLE_X -emit-llvm %s 
-o - 2>&1 | FileCheck %s --check-prefix=CHECK-X
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_X 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_X
+// RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_9BYTES 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_9BYTES
 
 // Make sure Clang doesn't treat |lockval| as asm input.
 void _raw_spin_lock(void) {
@@ -57,7 +58,7 @@
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm
+// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm: can't store value into 
a register
 
 // Check Clang reports an error if attempting to return a big structure via a 
register.
 void big_struct(void) {
@@ -69,7 +70,7 @@
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm
+// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm: can't store value into 
a register
 
 // Clang is able to emit LLVM IR for an 16-byte structure.
 void x_constraint_fit() {
@@ -100,3 +101,17 @@
 }
 
 // CHECK-IMPOSSIBLE_X: invalid output size for constraint
+
+// http://crbug.com/999160
+// Clang used to report the following message:
+//   "impossible constraint in asm: can't store struct into a register"
+// for the assembly directive below, although there's no struct.
+void crbug_999160_regtest() {
+#ifdef IMPOSSIBLE_9BYTES
+  char buf[9];
+  asm(""
+  : "=r"(buf));
+#endif
+}
+
+// CHECK-IMPOSSIBLE_9BYTES: impossible constraint in asm: can't store value 
into a register
Index: cfe/trunk/lib/CodeGen/CGStmt.cpp
===
--- cfe/trunk/lib/CodeGen/CGStmt.cpp
+++ cfe/trunk/lib/CodeGen/CGStmt.cpp
@@ -2326,7 +2326,7 @@
 const Expr *OutExpr = S.getOutputExpr(i);
 CGM.Error(
 OutExpr->getExprLoc(),
-"impossible constraint in asm: can't store struct into a 
register");
+"impossible constraint in asm: can't store value into a register");
 return;
   }
   Dest = MakeAddrLValue(A, Ty);


Index: cfe/trunk/test/CodeGen/x86_64-PR42672.c
===
--- cfe/trunk/test/CodeGen/x86_64-PR42672.c
+++ cfe/trunk/test/CodeGen/x86_64-PR42672.c
@@ -4,6 +4,7 @@
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_BIG -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_BIG
 // RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DPOSSIBLE_X -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-X
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_X -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_X
+// RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_9BYTES -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_9BYTES
 
 // Make sure Clang doesn't treat |lockval| as asm input.
 void _raw_spin_lock(void) {
@@ -57,7 +58,7 @@
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm
+// CHECK-IMPOSSIBLE_ODD: impossible constraint in asm: can't store value into a register
 
 // Check Clang reports an error if attempting to return a big structure via a register.
 void big_struct(void) {
@@ -69,7 +70,7 @@
   : "=r"(str));
 #endif
 }
-// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm
+// CHECK-IMPOSSIBLE_BIG: impossible constraint in asm: can't store value into a register
 
 // Clang is able to emit LLVM IR for an 16-byte structure.
 void x_constraint_fit() {
@@ -100,3 +101,17 @@
 }
 
 // CHECK-IMPOSSIBLE_X: invalid output size for constraint
+
+// http://crbug.com/999160
+// Clang used to report the following message:
+//   "impossible constraint in asm: can't store struct into a register"
+// for the assembly directive below, although there's no struct.
+void crbug_999160_regtest() {
+#ifdef IMPOSSIBLE_9BYTES
+  char buf[9];
+  asm(""
+  : "=r"(buf));
+#endif
+}
+
+// 

[clang-tools-extra] r370446 - [clangd] Add .vscode-test to .gitignore.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Fri Aug 30 02:06:18 2019
New Revision: 370446

URL: http://llvm.org/viewvc/llvm-project?rev=370446&view=rev
Log:
[clangd] Add .vscode-test to .gitignore.

Reviewers: jvikstrom

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore?rev=370446&r1=370445&r2=370446&view=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore (original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore Fri Aug 30 
02:06:18 2019
@@ -1,2 +1,3 @@
 out
 node_modules
+.vscode-test


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

[PATCH] D66928: [clangd] Collecting main file macro expansion locations in ParsedAST.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!




Comment at: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp:260
+// Macros from token concatenations included.
+#define CONCAT(X) X##1()
+#define MACRO1() 123

jvikstrom wrote:
> ilya-biryukov wrote:
> > Could we also test?
> > ```
> > #define PREPEND(X) Foo##X
> > ```
> > 
> > It'll probably be the same, but still interesting to see whether it's any 
> > differnet.
> So it turns out that the tests weren't actually passing before, must have 
> accidentally forgot to save the file or something before compiling. Anyways 
> tokens from concatenations are not included right now (which for highlighting 
> is probably what we want, we don't highlight types/names that are from macro 
> concatenations either)
> 
> But the reason they are not are because:
> * They do not pass the `isInsideMainFile` check. Their file id is set to 
> something that prints `` when dumped
> * Even if they did pass the check the SourceLocation does not seem to be 
> correct. They return the same SourceLocation as the parent `CONCAT` or 
> `PREPEND`
> 
> Don't know how to fix any of this, and don't know if we actually want to 
> collect these expansions either.
It's definitely ok to not highlight those arguments that get concatenated.
If we wanted to fix this, I think we'd need to detect tokens coming from 
concatenations separately and finds ways to map them back (not sure if it's 
possible at all). But let's not bother with this for now.



Comment at: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp:268
+#include "foo.inc"
+  )cpp");
+  auto TU = TestTU::withCode(TestCase.code());

I've realized there's one last interesting case that we are missing:
```
#define assert(COND) if (!(COND)) { printf("%s", #COND); exit(0); }

void test(int a) {
  assert(0 <= a); // a should be highlighted
}
```

Could you please add this to the tests? If this works - great; if not - feel 
free to add a FIXME and land the patch as is, we can fix this later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66928



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

[PATCH] D66949: [clangd] Add .vscode-test to .gitignore.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370446: [clangd] Add .vscode-test to .gitignore. (authored 
by hokein, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66949?vs=217886&id=218041#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66949

Files:
  clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore


Index: clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore
===
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore
@@ -1,2 +1,3 @@
 out
 node_modules
+.vscode-test


Index: clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore
===
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore
@@ -1,2 +1,3 @@
 out
 node_modules
+.vscode-test
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66960: [Tooling] Migrated APIs that take ownership of objects to unique_ptr

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM from my side.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66960



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

[PATCH] D66960: [Tooling] Migrated APIs that take ownership of objects to unique_ptr

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr updated this revision to Diff 218046.
gribozavr added a comment.

Updated documentation and added release notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66960

Files:
  clang-tools-extra/clangd/unittests/IndexActionTests.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
  clang/docs/LibTooling.rst
  clang/docs/RAVFrontendAction.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Tooling/Tooling.h
  clang/lib/Tooling/Tooling.cpp
  clang/unittests/AST/EvaluateAsRValueTest.cpp
  clang/unittests/AST/RecursiveASTVisitorTest.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
  clang/unittests/Index/IndexTests.cpp
  clang/unittests/Sema/CodeCompleteTest.cpp
  clang/unittests/Sema/ExternalSemaSourceTest.cpp
  clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
  clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
  clang/unittests/StaticAnalyzer/StoreTest.cpp
  clang/unittests/StaticAnalyzer/SymbolReaperTest.cpp
  clang/unittests/Tooling/CommentHandlerTest.cpp
  clang/unittests/Tooling/RefactoringTest.cpp
  clang/unittests/Tooling/TestVisitor.h
  clang/unittests/Tooling/ToolingTest.cpp

Index: clang/unittests/Tooling/ToolingTest.cpp
===
--- clang/unittests/Tooling/ToolingTest.cpp
+++ clang/unittests/Tooling/ToolingTest.cpp
@@ -62,10 +62,10 @@
 
 TEST(runToolOnCode, FindsNoTopLevelDeclOnEmptyCode) {
   bool FoundTopLevelDecl = false;
-  EXPECT_TRUE(
-  runToolOnCode(new TestAction(std::make_unique(
-&FoundTopLevelDecl)),
-""));
+  EXPECT_TRUE(runToolOnCode(
+  std::make_unique(
+  std::make_unique(&FoundTopLevelDecl)),
+  ""));
   EXPECT_FALSE(FoundTopLevelDecl);
 }
 
@@ -102,17 +102,17 @@
 
 TEST(runToolOnCode, FindsClassDecl) {
   bool FoundClassDeclX = false;
-  EXPECT_TRUE(
-  runToolOnCode(new TestAction(std::make_unique(
-&FoundClassDeclX)),
-"class X;"));
+  EXPECT_TRUE(runToolOnCode(
+  std::make_unique(
+  std::make_unique(&FoundClassDeclX)),
+  "class X;"));
   EXPECT_TRUE(FoundClassDeclX);
 
   FoundClassDeclX = false;
-  EXPECT_TRUE(
-  runToolOnCode(new TestAction(std::make_unique(
-&FoundClassDeclX)),
-"class Y;"));
+  EXPECT_TRUE(runToolOnCode(
+  std::make_unique(
+  std::make_unique(&FoundClassDeclX)),
+  "class Y;"));
   EXPECT_FALSE(FoundClassDeclX);
 }
 
@@ -160,8 +160,8 @@
   Args.push_back("-Idef");
   Args.push_back("-fsyntax-only");
   Args.push_back("test.cpp");
-  clang::tooling::ToolInvocation Invocation(Args, new SyntaxOnlyAction,
-Files.get());
+  clang::tooling::ToolInvocation Invocation(
+  Args, std::make_unique(), Files.get());
   InMemoryFileSystem->addFile(
   "test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("#include \n"));
   InMemoryFileSystem->addFile("def/abc", 0,
@@ -186,8 +186,8 @@
   Args.push_back("-Idef");
   Args.push_back("-fsyntax-only");
   Args.push_back("test.cpp");
-  clang::tooling::ToolInvocation Invocation(Args, new SyntaxOnlyAction,
-Files.get());
+  clang::tooling::ToolInvocation Invocation(
+  Args, std::make_unique(), Files.get());
   InMemoryFileSystem->addFile(
   "test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("#include \n"));
   InMemoryFileSystem->addFile("def/abc", 0,
@@ -257,61 +257,61 @@
   std::vector Args = {"-std=c++11"};
   std::vector Args2 = {"-fno-delayed-template-parsing"};
 
-  EXPECT_TRUE(runToolOnCode(new SkipBodyAction,
+  EXPECT_TRUE(runToolOnCode(std::make_unique(),
 "int skipMe() { an_error_here }"));
-  EXPECT_FALSE(runToolOnCode(new SkipBodyAction,
+  EXPECT_FALSE(runToolOnCode(std::make_unique(),
  "int skipMeNot() { an_error_here }"));
 
   // Test constructors with initializers
   EXPECT_TRUE(runToolOnCodeWithArgs(
-  new SkipBodyAction,
+  std::make_unique(),
   "struct skipMe { skipMe() : an_error() { more error } };", Args));
   EXPECT_TRUE(runToolOnCodeWithArgs(
-  new SkipBodyAction, "struct skipMe { skipMe(); };"
+  std::make_unique(), "struct skipMe { skipMe(); };"
   "skipMe::skipMe() : an_error([](){;}) { more error }",
   Args));
   EXPECT_TRUE(runToolOnCodeWithArgs(
-  new SkipBodyAction, "struct skipMe { skipMe(); };"
+  std::make_unique(), "struct skipMe { skipMe(); };"
   "skipMe::skipMe() : an_error{[](){;}} { more error }",
   Args));
   EXPECT_TRUE(runToolOnCodeWithArgs(
-  new SkipBodyAction,
+  std::make_unique(),
   "struct skipMe { skipMe(); };"
   "skipMe::skipMe() : a(e)>>(), f{}, g() { error }",
   Args));
   EXPECT_TRUE(runToolOnCodeWithArgs(
-  new SkipBo

[PATCH] D66960: [Tooling] Migrated APIs that take ownership of objects to unique_ptr

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added a comment.

In D66960#1651258 , @lebedev.ri wrote:

> In D66960#1651243 , @gribozavr wrote:
>
> > In D66960#1651198 , @lebedev.ri 
> > wrote:
> >
> > > This is missing documentation changes.
> >
> >
> > Could you point out such places? I tried to remove "takes ownership" 
> > comments which became redundant.
>
>
> See my two original patches.


Thanks, fixed docs.

>>> And this likely would be good to mention in releasenotes.
>> 
>> Will add.

Added release notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66960



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

r370451 - [Tooling] Migrated APIs that take ownership of objects to unique_ptr

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Fri Aug 30 02:29:34 2019
New Revision: 370451

URL: http://llvm.org/viewvc/llvm-project?rev=370451&view=rev
Log:
[Tooling] Migrated APIs that take ownership of objects to unique_ptr

Subscribers: jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/docs/LibTooling.rst
cfe/trunk/docs/RAVFrontendAction.rst
cfe/trunk/docs/ReleaseNotes.rst
cfe/trunk/include/clang/Tooling/Tooling.h
cfe/trunk/lib/Tooling/Tooling.cpp
cfe/trunk/unittests/AST/EvaluateAsRValueTest.cpp
cfe/trunk/unittests/AST/RecursiveASTVisitorTest.cpp
cfe/trunk/unittests/CrossTU/CrossTranslationUnitTest.cpp
cfe/trunk/unittests/Index/IndexTests.cpp
cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
cfe/trunk/unittests/Sema/ExternalSemaSourceTest.cpp
cfe/trunk/unittests/StaticAnalyzer/CallDescriptionTest.cpp
cfe/trunk/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
cfe/trunk/unittests/StaticAnalyzer/StoreTest.cpp
cfe/trunk/unittests/StaticAnalyzer/SymbolReaperTest.cpp
cfe/trunk/unittests/Tooling/CommentHandlerTest.cpp
cfe/trunk/unittests/Tooling/RefactoringTest.cpp
cfe/trunk/unittests/Tooling/TestVisitor.h
cfe/trunk/unittests/Tooling/ToolingTest.cpp

Modified: cfe/trunk/docs/LibTooling.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibTooling.rst?rev=370451&r1=370450&r2=370451&view=diff
==
--- cfe/trunk/docs/LibTooling.rst (original)
+++ cfe/trunk/docs/LibTooling.rst Fri Aug 30 02:29:34 2019
@@ -34,7 +34,7 @@ looked for.  Let me give you an example:
   TEST(runToolOnCode, CanSyntaxCheckCode) {
 // runToolOnCode returns whether the action was correctly run over the
 // given code.
-EXPECT_TRUE(runToolOnCode(new clang::SyntaxOnlyAction, "class X {};"));
+EXPECT_TRUE(runToolOnCode(std::make_unique(), 
"class X {};"));
   }
 
 Writing a standalone tool

Modified: cfe/trunk/docs/RAVFrontendAction.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/RAVFrontendAction.rst?rev=370451&r1=370450&r2=370451&view=diff
==
--- cfe/trunk/docs/RAVFrontendAction.rst (original)
+++ cfe/trunk/docs/RAVFrontendAction.rst Fri Aug 30 02:29:34 2019
@@ -196,7 +196,7 @@ Now we can combine all of the above into
 
   int main(int argc, char **argv) {
 if (argc > 1) {
-  clang::tooling::runToolOnCode(new FindNamedClassAction, argv[1]);
+  
clang::tooling::runToolOnCode(std::make_unique(), 
argv[1]);
 }
   }
 

Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=370451&r1=370450&r2=370451&view=diff
==
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Fri Aug 30 02:29:34 2019
@@ -148,6 +148,12 @@ These are major API changes that have ha
 Clang. If upgrading an external codebase that uses Clang as a library,
 this section should help get you past the largest hurdles of upgrading.
 
+- libTooling APIs that transfer ownership of `FrontendAction` objects now pass
+  them by `unique_ptr`, making the ownership transfer obvious in the type
+  system. `FrontendActionFactory::create()` now returns a
+  `unique_ptr`. `runToolOnCode`, `runToolOnCodeWithArgs`,
+  `ToolInvocation::ToolInvocation()` now take a `unique_ptr`.
+
 Build System Changes
 
 

Modified: cfe/trunk/include/clang/Tooling/Tooling.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Tooling.h?rev=370451&r1=370450&r2=370451&view=diff
==
--- cfe/trunk/include/clang/Tooling/Tooling.h (original)
+++ cfe/trunk/include/clang/Tooling/Tooling.h Fri Aug 30 02:29:34 2019
@@ -154,19 +154,11 @@ inline std::unique_ptr ToolAction, const Twine 
&Code,
const Twine &FileName = "input.cc",
std::shared_ptr PCHContainerOps =
std::make_shared());
 
-inline bool
-runToolOnCode(std::unique_ptr ToolAction, const Twine &Code,
-  const Twine &FileName = "input.cc",
-  std::shared_ptr PCHContainerOps =
-  std::make_shared()) {
-  return runToolOnCode(ToolAction.release(), Code, FileName, PCHContainerOps);
-}
-
 /// The first part of the pair is the filename, the second part the
 /// file-content.
 using FileContentMappings = std::vector>;
@@ -185,43 +177,21 @@ using FileContentMappings = std::vector<
 ///
 /// \return - True if 'ToolAction' was successfully executed.
 bool runToolOnCodeWithArgs(
-FrontendAction *ToolAction, const Twine &Code,
-const std::vector &Args, const Twine &FileName = "input.cc",
-const Twine &ToolName = "clang-tool",
-

[clang-tools-extra] r370451 - [Tooling] Migrated APIs that take ownership of objects to unique_ptr

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Fri Aug 30 02:29:34 2019
New Revision: 370451

URL: http://llvm.org/viewvc/llvm-project?rev=370451&view=rev
Log:
[Tooling] Migrated APIs that take ownership of objects to unique_ptr

Subscribers: jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/unittests/IndexActionTests.cpp
clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h

Modified: clang-tools-extra/trunk/clangd/unittests/IndexActionTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/IndexActionTests.cpp?rev=370451&r1=370450&r2=370451&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/IndexActionTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/IndexActionTests.cpp Fri Aug 30 
02:29:34 2019
@@ -88,7 +88,7 @@ public:
 Args.push_back(MainFilePath);
 
 tooling::ToolInvocation Invocation(
-Args, Action.release(), Files.get(),
+Args, std::move(Action), Files.get(),
 std::make_shared());
 
 Invocation.run();

Modified: clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h?rev=370451&r1=370450&r2=370451&view=diff
==
--- clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h (original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h Fri Aug 30 
02:29:34 2019
@@ -117,7 +117,9 @@ runCheckOnCode(StringRef Code, std::vect
 
   SmallVector, 1> Checks;
   tooling::ToolInvocation Invocation(
-  Args, new TestClangTidyAction(Checks, Finder, Context),
+  Args,
+  std::make_unique>(Checks, Finder,
+   Context),
   Files.get());
   InMemoryFileSystem->addFile(Filename, 0,
   llvm::MemoryBuffer::getMemBuffer(Code));


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

[PATCH] D66516: [clangd] Added highlighting to types dependant on templates.

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom added a comment.

Pinging about this @ilya-biryukov


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66516



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

[PATCH] D66960: [Tooling] Migrated APIs that take ownership of objects to unique_ptr

[Dmitri Gribenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370451: [Tooling] Migrated APIs that take ownership of 
objects to unique_ptr (authored by gribozavr, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66960?vs=218046&id=218048#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66960

Files:
  cfe/trunk/docs/LibTooling.rst
  cfe/trunk/docs/RAVFrontendAction.rst
  cfe/trunk/docs/ReleaseNotes.rst
  cfe/trunk/include/clang/Tooling/Tooling.h
  cfe/trunk/lib/Tooling/Tooling.cpp
  cfe/trunk/unittests/AST/EvaluateAsRValueTest.cpp
  cfe/trunk/unittests/AST/RecursiveASTVisitorTest.cpp
  cfe/trunk/unittests/CrossTU/CrossTranslationUnitTest.cpp
  cfe/trunk/unittests/Index/IndexTests.cpp
  cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
  cfe/trunk/unittests/Sema/ExternalSemaSourceTest.cpp
  cfe/trunk/unittests/StaticAnalyzer/CallDescriptionTest.cpp
  cfe/trunk/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
  cfe/trunk/unittests/StaticAnalyzer/StoreTest.cpp
  cfe/trunk/unittests/StaticAnalyzer/SymbolReaperTest.cpp
  cfe/trunk/unittests/Tooling/CommentHandlerTest.cpp
  cfe/trunk/unittests/Tooling/RefactoringTest.cpp
  cfe/trunk/unittests/Tooling/TestVisitor.h
  cfe/trunk/unittests/Tooling/ToolingTest.cpp
  clang-tools-extra/trunk/clangd/unittests/IndexActionTests.cpp
  clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h

Index: cfe/trunk/lib/Tooling/Tooling.cpp
===
--- cfe/trunk/lib/Tooling/Tooling.cpp
+++ cfe/trunk/lib/Tooling/Tooling.cpp
@@ -124,12 +124,12 @@
   return Invocation;
 }
 
-bool runToolOnCode(FrontendAction *ToolAction, const Twine &Code,
-   const Twine &FileName,
+bool runToolOnCode(std::unique_ptr ToolAction,
+   const Twine &Code, const Twine &FileName,
std::shared_ptr PCHContainerOps) {
-  return runToolOnCodeWithArgs(ToolAction, Code, std::vector(),
-   FileName, "clang-tool",
-   std::move(PCHContainerOps));
+  return runToolOnCodeWithArgs(std::move(ToolAction), Code,
+   std::vector(), FileName,
+   "clang-tool", std::move(PCHContainerOps));
 }
 
 } // namespace tooling
@@ -151,7 +151,7 @@
 namespace tooling {
 
 bool runToolOnCodeWithArgs(
-FrontendAction *ToolAction, const Twine &Code,
+std::unique_ptr ToolAction, const Twine &Code,
 llvm::IntrusiveRefCntPtr VFS,
 const std::vector &Args, const Twine &FileName,
 const Twine &ToolName,
@@ -164,13 +164,12 @@
   ArgumentsAdjuster Adjuster = getClangStripDependencyFileAdjuster();
   ToolInvocation Invocation(
   getSyntaxOnlyToolArgs(ToolName, Adjuster(Args, FileNameRef), FileNameRef),
-  ToolAction, Files.get(),
-  std::move(PCHContainerOps));
+  std::move(ToolAction), Files.get(), std::move(PCHContainerOps));
   return Invocation.run();
 }
 
 bool runToolOnCodeWithArgs(
-FrontendAction *ToolAction, const Twine &Code,
+std::unique_ptr ToolAction, const Twine &Code,
 const std::vector &Args, const Twine &FileName,
 const Twine &ToolName,
 std::shared_ptr PCHContainerOps,
@@ -192,8 +191,8 @@
 llvm::MemoryBuffer::getMemBuffer(FilenameWithContent.second));
   }
 
-  return runToolOnCodeWithArgs(ToolAction, Code, OverlayFileSystem, Args,
-   FileName, ToolName);
+  return runToolOnCodeWithArgs(std::move(ToolAction), Code, OverlayFileSystem,
+   Args, FileName, ToolName);
 }
 
 llvm::Expected getAbsolutePath(llvm::vfs::FileSystem &FS,
@@ -267,11 +266,11 @@
   Files(Files), PCHContainerOps(std::move(PCHContainerOps)) {}
 
 ToolInvocation::ToolInvocation(
-std::vector CommandLine, FrontendAction *FAction,
-FileManager *Files, std::shared_ptr PCHContainerOps)
+std::vector CommandLine,
+std::unique_ptr FAction, FileManager *Files,
+std::shared_ptr PCHContainerOps)
 : CommandLine(std::move(CommandLine)),
-  Action(new SingleFrontendActionFactory(
-  std::unique_ptr(FAction))),
+  Action(new SingleFrontendActionFactory(std::move(FAction))),
   OwnsAction(true), Files(Files),
   PCHContainerOps(std::move(PCHContainerOps)) {}
 
Index: cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
===
--- cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
+++ cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
@@ -101,10 +101,10 @@
 
 CompletionContext runCompletion(StringRef Code, size_t Offset) {
   CompletionContext ResultCtx;
-  auto Action = std::make_unique(
-  offsetToPosition(Code, Offset), ResultCtx);
-  clang::tooling::runToolOnCodeWithArgs(Action.release(), Code, {"-std=c++11"},
- 

[clang-tools-extra] r370452 - [clangd] Collecting main file macro expansion locations in ParsedAST.

[Johan Vikstrom via cfe-commits]

Author: jvikstrom
Date: Fri Aug 30 02:33:27 2019
New Revision: 370452

URL: http://llvm.org/viewvc/llvm-project?rev=370452&view=rev
Log:
[clangd] Collecting main file macro expansion locations in ParsedAST.

Summary: TokenBuffer does not collect macro expansions inside macro arguments 
which is needed for semantic higlighting. Therefore collects macro expansions 
in the main file in a PPCallback when building the ParsedAST instead.

Reviewers: hokein, ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/clangd/unittests/ClangdUnitTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=370452&r1=370451&r2=370452&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Fri Aug 30 02:33:27 2019
@@ -22,6 +22,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Frontend/CompilerInstance.h"
@@ -32,6 +33,7 @@
 #include "clang/Index/IndexingAction.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/MacroInfo.h"
+#include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Sema/Sema.h"
@@ -103,6 +105,28 @@ private:
   std::vector TopLevelDecls;
 };
 
+// CollectMainFileMacroExpansions and CollectMainFileMacros are two different
+// classes as CollectMainFileMacroExpansions is only used when building the AST
+// for the main file. CollectMainFileMacros is only used when building the
+// preamble.
+class CollectMainFileMacroExpansions : public PPCallbacks {
+  const SourceManager &SM;
+  std::vector &MainFileMacroLocs;
+
+public:
+  CollectMainFileMacroExpansions(const SourceManager &SM,
+ std::vector 
&MainFileMacroLocs)
+  : SM(SM), MainFileMacroLocs(MainFileMacroLocs) {}
+
+  virtual void MacroExpands(const Token &MacroNameTok,
+const MacroDefinition &MD, SourceRange Range,
+const MacroArgs *Args) {
+SourceLocation L = MacroNameTok.getLocation();
+if (!L.isMacroID() && isInsideMainFile(L, SM))
+  MainFileMacroLocs.push_back(L);
+  }
+};
+
 class CollectMainFileMacros : public PPCallbacks {
 public:
   explicit CollectMainFileMacros(const SourceManager &SM,
@@ -417,6 +441,11 @@ ParsedAST::build(std::unique_ptrgetPreprocessor().addPPCallbacks(
   collectIncludeStructureCallback(Clang->getSourceManager(), &Includes));
+  // Collect the macro expansions in the main file.
+  std::vector MainFileMacroExpLocs;
+  Clang->getPreprocessor().addPPCallbacks(
+  std::make_unique(
+  Clang->getSourceManager(), MainFileMacroExpLocs));
 
   // Copy over the includes from the preamble, then combine with the
   // non-preamble includes below.
@@ -470,7 +499,8 @@ ParsedAST::build(std::unique_ptr ParsedAST::getLoc
   return LocalTopLevelDecls;
 }
 
+llvm::ArrayRef ParsedAST::getMainFileExpansions() const {
+  return MainFileMacroExpLocs;
+}
+
 const std::vector &ParsedAST::getDiagnostics() const { return Diags; }
 
 std::size_t ParsedAST::getUsedBytes() const {
@@ -565,11 +599,13 @@ ParsedAST::ParsedAST(std::shared_ptr Clang,
  std::unique_ptr Action,
  syntax::TokenBuffer Tokens,
+ std::vector MainFileMacroExpLocs,
  std::vector LocalTopLevelDecls,
  std::vector Diags, IncludeStructure Includes,
  CanonicalIncludes CanonIncludes)
 : Preamble(std::move(Preamble)), Clang(std::move(Clang)),
   Action(std::move(Action)), Tokens(std::move(Tokens)),
+  MainFileMacroExpLocs(std::move(MainFileMacroExpLocs)),
   Diags(std::move(Diags)),
   LocalTopLevelDecls(std::move(LocalTopLevelDecls)),
   Includes(std::move(Includes)), CanonIncludes(std::move(CanonIncludes)) {

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.h?rev=370452&r1=370451&r2=370452&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.h Fri Aug 30 02:33:27 2019
@@ -118,6 +118,9 @@ public:
   const IncludeStructure &getIncludeStructure() const;
   const CanonicalIncludes &getCanonicalIncludes() const;
 
+  /// The start locations of all macro expansions spelled inside 

[PATCH] D66928: [clangd] Collecting main file macro expansion locations in ParsedAST.

[Johan Vikström via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370452: [clangd] Collecting main file macro expansion 
locations in ParsedAST. (authored by jvikstrom, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66928?vs=217920&id=218049#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66928

Files:
  clang-tools-extra/trunk/clangd/ClangdUnit.cpp
  clang-tools-extra/trunk/clangd/ClangdUnit.h
  clang-tools-extra/trunk/clangd/unittests/ClangdUnitTests.cpp

Index: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp
@@ -22,6 +22,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Frontend/CompilerInstance.h"
@@ -32,6 +33,7 @@
 #include "clang/Index/IndexingAction.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/MacroInfo.h"
+#include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Sema/Sema.h"
@@ -103,6 +105,28 @@
   std::vector TopLevelDecls;
 };
 
+// CollectMainFileMacroExpansions and CollectMainFileMacros are two different
+// classes as CollectMainFileMacroExpansions is only used when building the AST
+// for the main file. CollectMainFileMacros is only used when building the
+// preamble.
+class CollectMainFileMacroExpansions : public PPCallbacks {
+  const SourceManager &SM;
+  std::vector &MainFileMacroLocs;
+
+public:
+  CollectMainFileMacroExpansions(const SourceManager &SM,
+ std::vector &MainFileMacroLocs)
+  : SM(SM), MainFileMacroLocs(MainFileMacroLocs) {}
+
+  virtual void MacroExpands(const Token &MacroNameTok,
+const MacroDefinition &MD, SourceRange Range,
+const MacroArgs *Args) {
+SourceLocation L = MacroNameTok.getLocation();
+if (!L.isMacroID() && isInsideMainFile(L, SM))
+  MainFileMacroLocs.push_back(L);
+  }
+};
+
 class CollectMainFileMacros : public PPCallbacks {
 public:
   explicit CollectMainFileMacros(const SourceManager &SM,
@@ -417,6 +441,11 @@
   // (We can't *just* use the replayed includes, they don't have Resolved path).
   Clang->getPreprocessor().addPPCallbacks(
   collectIncludeStructureCallback(Clang->getSourceManager(), &Includes));
+  // Collect the macro expansions in the main file.
+  std::vector MainFileMacroExpLocs;
+  Clang->getPreprocessor().addPPCallbacks(
+  std::make_unique(
+  Clang->getSourceManager(), MainFileMacroExpLocs));
 
   // Copy over the includes from the preamble, then combine with the
   // non-preamble includes below.
@@ -470,7 +499,8 @@
 Diags.insert(Diags.end(), D.begin(), D.end());
   }
   return ParsedAST(std::move(Preamble), std::move(Clang), std::move(Action),
-   std::move(Tokens), std::move(ParsedDecls), std::move(Diags),
+   std::move(Tokens), std::move(MainFileMacroExpLocs),
+   std::move(ParsedDecls), std::move(Diags),
std::move(Includes), std::move(CanonIncludes));
 }
 
@@ -509,6 +539,10 @@
   return LocalTopLevelDecls;
 }
 
+llvm::ArrayRef ParsedAST::getMainFileExpansions() const {
+  return MainFileMacroExpLocs;
+}
+
 const std::vector &ParsedAST::getDiagnostics() const { return Diags; }
 
 std::size_t ParsedAST::getUsedBytes() const {
@@ -565,11 +599,13 @@
  std::unique_ptr Clang,
  std::unique_ptr Action,
  syntax::TokenBuffer Tokens,
+ std::vector MainFileMacroExpLocs,
  std::vector LocalTopLevelDecls,
  std::vector Diags, IncludeStructure Includes,
  CanonicalIncludes CanonIncludes)
 : Preamble(std::move(Preamble)), Clang(std::move(Clang)),
   Action(std::move(Action)), Tokens(std::move(Tokens)),
+  MainFileMacroExpLocs(std::move(MainFileMacroExpLocs)),
   Diags(std::move(Diags)),
   LocalTopLevelDecls(std::move(LocalTopLevelDecls)),
   Includes(std::move(Includes)), CanonIncludes(std::move(CanonIncludes)) {
Index: clang-tools-extra/trunk/clangd/ClangdUnit.h
===
--- clang-tools-extra/trunk/clangd/ClangdUnit.h
+++ clang-tools-extra/trunk/clangd/ClangdUnit.h
@@ -118,6 +118,9 @@
   const IncludeStructure &getIncludeStructure() const;
   const CanonicalIncludes &getCanonicalIncludes() const;
 
+  /// The start locations of all macro expansions spelled inside the main file.
+  /// Does not include expans

[PATCH] D66738: [clangd] Added highlighting for structured bindings.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

I don't have a strong preference on how to highlight `BindingDecl`, but I think 
highlighting them as `Variable` is better than no highlighting them at all, so 
LGTM from myside. @ilya-biryukov thoughts?

again, please update the patch description in phabricator and your git commit 
message.




Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:443
+$Primitive[[int]] $Variable[[A]][2] = {1,2};
+auto [$Variable[[B1]], $Variable[[B2]]] = $Variable[[A]];
+auto [$Variable[[G1]], $Variable[[G2]]] = $Variable[[Global]];

this is not related to the patch,  but the highlighting behavior for auto here 
is weird, some of them are highlighted while some of them are not. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66738



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

[PATCH] D66732: [Clangd] ExtractFunction Added checks for broken control flow

[Shaurya Gupta via Phabricator via cfe-commits]

SureYeaah updated this revision to Diff 218053.
SureYeaah marked 4 inline comments as done.
SureYeaah added a comment.

Addressed review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66732

Files:
  clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -522,10 +522,7 @@
   EXPECT_THAT(apply(" [[int a = 5;]] a++; "), StartsWith("fail"));
   // Don't extract return
   EXPECT_THAT(apply(" if(true) [[return;]] "), StartsWith("fail"));
-  // Don't extract break and continue.
-  // FIXME: We should be able to extract this since it's non broken.
-  EXPECT_THAT(apply(" [[for(;;) break;]] "), StartsWith("fail"));
-  EXPECT_THAT(apply(" for(;;) [[continue;]] "), StartsWith("fail"));
+  
 }
 
 TEST_F(ExtractFunctionTest, FileTest) {
@@ -604,6 +601,21 @@
   EXPECT_EQ(apply(MacroFailInput), "unavailable");
 }
 
+TEST_F(ExtractFunctionTest, ControlFlow) {
+  Context = Function;
+  // We should be able to extract break/continue with a parent loop/switch.
+  EXPECT_THAT(apply(" [[for(;;) if(1) break;]] "), HasSubstr("extracted"));
+  EXPECT_THAT(apply(" for(;;) [[while(1) break;]] "), HasSubstr("extracted"));
+  EXPECT_THAT(apply(" [[switch(1) { break; }]]"), HasSubstr("extracted"));
+  EXPECT_THAT(apply(" [[while(1) switch(1) { continue; }]]"),
+  HasSubstr("extracted"));
+  // Don't extract break and continue without a loop/switch parent.
+  EXPECT_THAT(apply(" for(;;) [[if(1) continue;]] "), StartsWith("fail"));
+  EXPECT_THAT(apply(" while(1) [[if(1) break;]] "), StartsWith("fail"));
+  EXPECT_THAT(apply(" switch(1) { [[break;]] }"), StartsWith("fail"));
+  EXPECT_THAT(apply(" for(;;) { [[while(1) break; break;]] }"),
+  StartsWith("fail"));
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -29,7 +29,7 @@
 // - Void return type
 // - Cannot extract declarations that will be needed in the original function
 //   after extraction.
-// - Doesn't check for broken control flow (break/continue without loop/switch)
+// - Checks for broken control flow (break/continue without loop/switch)
 //
 // 1. ExtractFunction is the tweak subclass
 //- Prepare does basic analysis of the selection and is therefore fast.
@@ -153,6 +153,7 @@
   // semicolon after the extraction.
   const Node *getLastRootStmt() const { return Parent->Children.back(); }
   void generateRootStmts();
+
 private:
   llvm::DenseSet RootStmts;
 };
@@ -163,7 +164,7 @@
 
 // Generate RootStmts set
 void ExtractionZone::generateRootStmts() {
-  for(const Node *Child : Parent->Children)
+  for (const Node *Child : Parent->Children)
 RootStmts.insert(Child->ASTNode.get());
 }
 
@@ -179,7 +180,7 @@
   if (isa(Func))
 return nullptr;
   // FIXME: Support extraction from templated functions.
-  if(Func->isTemplated())
+  if (Func->isTemplated())
 return nullptr;
   return Func;
 }
@@ -351,8 +352,9 @@
   llvm::DenseMap DeclInfoMap;
   // True if there is a return statement in zone.
   bool HasReturnStmt = false;
-  // For now we just care whether there exists a break/continue in zone.
-  bool HasBreakOrContinue = false;
+  // Control flow is broken if we are extracting a break/continue without a
+  // corresponding parent loop/switch
+  bool BrokenControlFlow = false;
   // FIXME: capture TypeAliasDecl and UsingDirectiveDecl
   // FIXME: Capture type information as well.
   DeclInformation *createDeclInfo(const Decl *D, ZoneRelative RelativeLoc);
@@ -391,6 +393,11 @@
   }
 }
 
+bool isLoop(const Stmt *S) {
+  return isa(S) || isa(S) || isa(S) ||
+ isa(S);
+}
+
 // Captures information from Extraction Zone
 CapturedZoneInfo captureZoneInfo(const ExtractionZone &ExtZone) {
   // We use the ASTVisitor instead of using the selection tree since we need to
@@ -402,24 +409,53 @@
 ExtractionZoneVisitor(const ExtractionZone &ExtZone) : ExtZone(ExtZone) {
   TraverseDecl(const_cast(ExtZone.EnclosingFunction));
 }
+
 bool TraverseStmt(Stmt *S) {
+  if (!S)
+return true;
   bool IsRootStmt = ExtZone.isRootStmt(const_cast(S));
   // If we are starting traversal of a RootStmt, we are somewhere inside
   // ExtractionZone
   if (IsRootStmt)
 CurrentLocation = ZoneRelative::Inside;
+  addToLoopSwitchCounters(S, 1);
   // Traverse using base class's TraverseStmt
   RecursiveASTVisitor

[PATCH] D66995: [clangd] Add highlighting for macro expansions.

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom created this revision.
jvikstrom added reviewers: hokein, ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

https://github.com/clangd/clangd/issues/134


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66995

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/semantic-highlighting.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -47,7 +47,8 @@
   {HighlightingKind::Method, "Method"},
   {HighlightingKind::StaticMethod, "StaticMethod"},
   {HighlightingKind::TemplateParameter, "TemplateParameter"},
-  {HighlightingKind::Primitive, "Primitive"}};
+  {HighlightingKind::Primitive, "Primitive"},
+  {HighlightingKind::MacroExpansion, "MacroExpansion"}};
   std::vector ExpectedTokens;
   for (const auto &KindString : KindToString) {
 std::vector Toks = makeHighlightingTokens(
@@ -391,9 +392,9 @@
   R"cpp(
   #define DEF_MULTIPLE(X) namespace X { class X { int X; }; }
   #define DEF_CLASS(T) class T {};
-  DEF_MULTIPLE(XYZ);
-  DEF_MULTIPLE(XYZW);
-  DEF_CLASS($Class[[A]])
+  $MacroExpansion[[DEF_MULTIPLE]](XYZ);
+  $MacroExpansion[[DEF_MULTIPLE]](XYZW);
+  $MacroExpansion[[DEF_CLASS]]($Class[[A]])
   #define MACRO_CONCAT(X, V, T) T foo##X = V
   #define DEF_VAR(X, V) int X = V
   #define DEF_VAR_T(T, X, V) T X = V
@@ -404,26 +405,27 @@
   #define SOME_NAME_SET variable2 = 123
   #define INC_VAR(X) X += 2
   $Primitive[[void]] $Function[[foo]]() {
-DEF_VAR($LocalVariable[[X]],  123);
-DEF_VAR_REV(908, $LocalVariable[[XY]]);
-$Primitive[[int]] CPY( $LocalVariable[[XX]] );
-DEF_VAR_TYPE($Class[[A]], $LocalVariable[[AA]]);
-$Primitive[[double]] SOME_NAME;
-$Primitive[[int]] SOME_NAME_SET;
+$MacroExpansion[[DEF_VAR]]($LocalVariable[[X]],  123);
+$MacroExpansion[[DEF_VAR_REV]](908, $LocalVariable[[XY]]);
+$Primitive[[int]] $MacroExpansion[[CPY]]( $LocalVariable[[XX]] );
+$MacroExpansion[[DEF_VAR_TYPE]]($Class[[A]], $LocalVariable[[AA]]);
+$Primitive[[double]] $MacroExpansion[[SOME_NAME]];
+$Primitive[[int]] $MacroExpansion[[SOME_NAME_SET]];
 $LocalVariable[[variable]] = 20.1;
-MACRO_CONCAT(var, 2, $Primitive[[float]]);
-DEF_VAR_T($Class[[A]], CPY(CPY($LocalVariable[[Nested]])),
-  CPY($Class[[A]]()));
-INC_VAR($LocalVariable[[variable]]);
+$MacroExpansion[[MACRO_CONCAT]](var, 2, $Primitive[[float]]);
+$MacroExpansion[[DEF_VAR_T]]($Class[[A]], $MacroExpansion[[CPY]](
+  $MacroExpansion[[CPY]]($LocalVariable[[Nested]])),
+$MacroExpansion[[CPY]]($Class[[A]]()));
+$MacroExpansion[[INC_VAR]]($LocalVariable[[variable]]);
   }
-  $Primitive[[void]] SOME_NAME();
-  DEF_VAR($Variable[[XYZ]], 567);
-  DEF_VAR_REV(756, $Variable[[AB]]);
+  $Primitive[[void]] $MacroExpansion[[SOME_NAME]]();
+  $MacroExpansion[[DEF_VAR]]($Variable[[XYZ]], 567);
+  $MacroExpansion[[DEF_VAR_REV]](756, $Variable[[AB]]);
 
   #define CALL_FN(F) F();
   #define DEF_FN(F) void F ()
-  DEF_FN($Function[[g]]) {
-CALL_FN($Function[[foo]]);
+  $MacroExpansion[[DEF_FN]]($Function[[g]]) {
+$MacroExpansion[[CALL_FN]]($Function[[foo]]);
   }
 )cpp",
   R"cpp(
@@ -433,8 +435,8 @@
   $Primitive[[int]] $Variable[[y]];
   $Primitive[[int]] $Function[[f]]();
   $Primitive[[void]] $Function[[foo]]() {
-assert($Variable[[x]] != $Variable[[y]]);
-assert($Variable[[x]] != $Function[[f]]());
+$MacroExpansion[[assert]]($Variable[[x]] != $Variable[[y]]);
+$MacroExpansion[[assert]]($Variable[[x]] != $Function[[f]]());
   }
 )cpp"};
   for (const auto &TestCase : TestCases) {
@@ -455,8 +457,8 @@
   // A separate test for macros in headers.
   checkHighlightings(R"cpp(
 #include "imp.h"
-DEFINE_Y
-DXYZ_Y(A);
+$MacroExpansion[[DEFINE_Y]]
+$MacroExpansion[[DXYZ_Y]](A);
   )cpp",
  {{"imp.h", R"cpp(
 #define DXYZ(X) class X {};
Index: clang-tools-extra/clangd/test/semantic-highlighting.test
===
--- clang-tools-extra/clangd/test/semantic-highlighting.test
+++ clang-tools-extra/clangd/test/semantic-highlighting.test
@@ -45,6 +45,9 @@
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
 # CHECK-NEXT:"storage.type.primitive.cpp"
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  [
+# CH

[clang-tools-extra] r370455 - [Clangd] ExtractFunction Added checks for broken control flow

[Shaurya Gupta via cfe-commits]

Author: sureyeaah
Date: Fri Aug 30 02:57:56 2019
New Revision: 370455

URL: http://llvm.org/viewvc/llvm-project?rev=370455&view=rev
Log:
[Clangd] ExtractFunction Added checks for broken control flow

Summary:
- Added checks for broken control flow
- Added unittests

Reviewers: sammccall, kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractFunction.cpp
clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp

Modified: clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractFunction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractFunction.cpp?rev=370455&r1=370454&r2=370455&view=diff
==
--- clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractFunction.cpp 
(original)
+++ clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractFunction.cpp Fri Aug 
30 02:57:56 2019
@@ -29,7 +29,7 @@
 // - Void return type
 // - Cannot extract declarations that will be needed in the original function
 //   after extraction.
-// - Doesn't check for broken control flow (break/continue without loop/switch)
+// - Checks for broken control flow (break/continue without loop/switch)
 //
 // 1. ExtractFunction is the tweak subclass
 //- Prepare does basic analysis of the selection and is therefore fast.
@@ -153,6 +153,7 @@ struct ExtractionZone {
   // semicolon after the extraction.
   const Node *getLastRootStmt() const { return Parent->Children.back(); }
   void generateRootStmts();
+
 private:
   llvm::DenseSet RootStmts;
 };
@@ -163,7 +164,7 @@ bool ExtractionZone::isRootStmt(const St
 
 // Generate RootStmts set
 void ExtractionZone::generateRootStmts() {
-  for(const Node *Child : Parent->Children)
+  for (const Node *Child : Parent->Children)
 RootStmts.insert(Child->ASTNode.get());
 }
 
@@ -179,7 +180,7 @@ const FunctionDecl *findEnclosingFunctio
   if (isa(Func))
 return nullptr;
   // FIXME: Support extraction from templated functions.
-  if(Func->isTemplated())
+  if (Func->isTemplated())
 return nullptr;
   return Func;
 }
@@ -351,8 +352,9 @@ struct CapturedZoneInfo {
   llvm::DenseMap DeclInfoMap;
   // True if there is a return statement in zone.
   bool HasReturnStmt = false;
-  // For now we just care whether there exists a break/continue in zone.
-  bool HasBreakOrContinue = false;
+  // Control flow is broken if we are extracting a break/continue without a
+  // corresponding parent loop/switch
+  bool BrokenControlFlow = false;
   // FIXME: capture TypeAliasDecl and UsingDirectiveDecl
   // FIXME: Capture type information as well.
   DeclInformation *createDeclInfo(const Decl *D, ZoneRelative RelativeLoc);
@@ -391,6 +393,11 @@ void CapturedZoneInfo::DeclInformation::
   }
 }
 
+bool isLoop(const Stmt *S) {
+  return isa(S) || isa(S) || isa(S) ||
+ isa(S);
+}
+
 // Captures information from Extraction Zone
 CapturedZoneInfo captureZoneInfo(const ExtractionZone &ExtZone) {
   // We use the ASTVisitor instead of using the selection tree since we need to
@@ -402,24 +409,53 @@ CapturedZoneInfo captureZoneInfo(const E
 ExtractionZoneVisitor(const ExtractionZone &ExtZone) : ExtZone(ExtZone) {
   TraverseDecl(const_cast(ExtZone.EnclosingFunction));
 }
+
 bool TraverseStmt(Stmt *S) {
+  if (!S)
+return true;
   bool IsRootStmt = ExtZone.isRootStmt(const_cast(S));
   // If we are starting traversal of a RootStmt, we are somewhere inside
   // ExtractionZone
   if (IsRootStmt)
 CurrentLocation = ZoneRelative::Inside;
+  addToLoopSwitchCounters(S, 1);
   // Traverse using base class's TraverseStmt
   RecursiveASTVisitor::TraverseStmt(S);
+  addToLoopSwitchCounters(S, -1);
   // We set the current location as after since next stmt will either be a
   // RootStmt (handled at the beginning) or after extractionZone
   if (IsRootStmt)
 CurrentLocation = ZoneRelative::After;
   return true;
 }
+
+// Add Increment to CurNumberOf{Loops,Switch} if statement is
+// {Loop,Switch} and inside Extraction Zone.
+void addToLoopSwitchCounters(Stmt *S, int Increment) {
+  if (CurrentLocation != ZoneRelative::Inside)
+return;
+  if (isLoop(S))
+CurNumberOfNestedLoops += Increment;
+  else if (isa(S))
+CurNumberOfSwitch += Increment;
+}
+
+// Decrement CurNumberOf{NestedLoops,Switch} if statement is {Loop,Switch}
+// and inside Extraction Zone.
+void decrementLoopSwitchCounters(Stmt *S) {
+  if (CurrentLocation != ZoneRelative::Inside)
+return;
+  if (isLoop(S))
+CurNumberOfNestedLoops--;
+  else if (isa(S))
+CurNumberOfSwitch--;
+}
+
 bool VisitDecl(Decl *D) {
   Info.createDeclInfo(D, CurrentLocati

[PATCH] D66732: [Clangd] ExtractFunction Added checks for broken control flow

[Shaurya Gupta via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370455: [Clangd] ExtractFunction Added checks for broken 
control flow (authored by SureYeaah, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66732?vs=218053&id=218056#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66732

Files:
  clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractFunction.cpp
  clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractFunction.cpp
===
--- clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractFunction.cpp
+++ clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -29,7 +29,7 @@
 // - Void return type
 // - Cannot extract declarations that will be needed in the original function
 //   after extraction.
-// - Doesn't check for broken control flow (break/continue without loop/switch)
+// - Checks for broken control flow (break/continue without loop/switch)
 //
 // 1. ExtractFunction is the tweak subclass
 //- Prepare does basic analysis of the selection and is therefore fast.
@@ -153,6 +153,7 @@
   // semicolon after the extraction.
   const Node *getLastRootStmt() const { return Parent->Children.back(); }
   void generateRootStmts();
+
 private:
   llvm::DenseSet RootStmts;
 };
@@ -163,7 +164,7 @@
 
 // Generate RootStmts set
 void ExtractionZone::generateRootStmts() {
-  for(const Node *Child : Parent->Children)
+  for (const Node *Child : Parent->Children)
 RootStmts.insert(Child->ASTNode.get());
 }
 
@@ -179,7 +180,7 @@
   if (isa(Func))
 return nullptr;
   // FIXME: Support extraction from templated functions.
-  if(Func->isTemplated())
+  if (Func->isTemplated())
 return nullptr;
   return Func;
 }
@@ -351,8 +352,9 @@
   llvm::DenseMap DeclInfoMap;
   // True if there is a return statement in zone.
   bool HasReturnStmt = false;
-  // For now we just care whether there exists a break/continue in zone.
-  bool HasBreakOrContinue = false;
+  // Control flow is broken if we are extracting a break/continue without a
+  // corresponding parent loop/switch
+  bool BrokenControlFlow = false;
   // FIXME: capture TypeAliasDecl and UsingDirectiveDecl
   // FIXME: Capture type information as well.
   DeclInformation *createDeclInfo(const Decl *D, ZoneRelative RelativeLoc);
@@ -391,6 +393,11 @@
   }
 }
 
+bool isLoop(const Stmt *S) {
+  return isa(S) || isa(S) || isa(S) ||
+ isa(S);
+}
+
 // Captures information from Extraction Zone
 CapturedZoneInfo captureZoneInfo(const ExtractionZone &ExtZone) {
   // We use the ASTVisitor instead of using the selection tree since we need to
@@ -402,24 +409,53 @@
 ExtractionZoneVisitor(const ExtractionZone &ExtZone) : ExtZone(ExtZone) {
   TraverseDecl(const_cast(ExtZone.EnclosingFunction));
 }
+
 bool TraverseStmt(Stmt *S) {
+  if (!S)
+return true;
   bool IsRootStmt = ExtZone.isRootStmt(const_cast(S));
   // If we are starting traversal of a RootStmt, we are somewhere inside
   // ExtractionZone
   if (IsRootStmt)
 CurrentLocation = ZoneRelative::Inside;
+  addToLoopSwitchCounters(S, 1);
   // Traverse using base class's TraverseStmt
   RecursiveASTVisitor::TraverseStmt(S);
+  addToLoopSwitchCounters(S, -1);
   // We set the current location as after since next stmt will either be a
   // RootStmt (handled at the beginning) or after extractionZone
   if (IsRootStmt)
 CurrentLocation = ZoneRelative::After;
   return true;
 }
+
+// Add Increment to CurNumberOf{Loops,Switch} if statement is
+// {Loop,Switch} and inside Extraction Zone.
+void addToLoopSwitchCounters(Stmt *S, int Increment) {
+  if (CurrentLocation != ZoneRelative::Inside)
+return;
+  if (isLoop(S))
+CurNumberOfNestedLoops += Increment;
+  else if (isa(S))
+CurNumberOfSwitch += Increment;
+}
+
+// Decrement CurNumberOf{NestedLoops,Switch} if statement is {Loop,Switch}
+// and inside Extraction Zone.
+void decrementLoopSwitchCounters(Stmt *S) {
+  if (CurrentLocation != ZoneRelative::Inside)
+return;
+  if (isLoop(S))
+CurNumberOfNestedLoops--;
+  else if (isa(S))
+CurNumberOfSwitch--;
+}
+
 bool VisitDecl(Decl *D) {
   Info.createDeclInfo(D, CurrentLocation);
   return true;
 }
+
 bool VisitDeclRefExpr(DeclRefExpr *DRE) {
   // Find the corresponding Decl and mark it's occurence.
   const Decl *D = DRE->getDecl();
@@ -431,26 +467,36 @@
   // FIXME: check if reference mutates the Decl being referred.
   return true;
 }
+
 bool VisitReturnStmt(ReturnStmt *Return) {
   if (CurrentLocation =

[PATCH] D66738: [clangd] Added highlighting for structured bindings.

[Johan Vikström via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:443
+$Primitive[[int]] $Variable[[A]][2] = {1,2};
+auto [$Variable[[B1]], $Variable[[B2]]] = $Variable[[A]];
+auto [$Variable[[G1]], $Variable[[G2]]] = $Variable[[Global]];

hokein wrote:
> this is not related to the patch,  but the highlighting behavior for auto 
> here is weird, some of them are highlighted while some of them are not. 
The reason for that is broken down here: 
https://reviews.llvm.org/D66516#inline-599982

Basically we don't traverse the type hierarchy when trying to add highlightings 
for auto so if we have a pointer/array type it will not find the actual type 
and just silently not add any highlightings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66738



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

r370457 - [ASTImporter] Propagate errors during import of overridden methods.

[Balazs Keri via cfe-commits]

Author: balazske
Date: Fri Aug 30 03:12:14 2019
New Revision: 370457

URL: http://llvm.org/viewvc/llvm-project?rev=370457&view=rev
Log:
[ASTImporter] Propagate errors during import of overridden methods.

Summary:
If importing overridden methods fails for a method it can be seen
incorrectly as non-virtual. To avoid this inconsistency the method
is marked with import error to avoid later use of it.

Reviewers: martong, a.sidorin, shafik, a_sidorin

Reviewed By: martong, shafik

Subscribers: rnkovacs, dkrupp, Szelethus, gamesh411, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/unittests/AST/ASTImporterTest.cpp

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=370457&r1=370456&r2=370457&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Fri Aug 30 03:12:14 2019
@@ -639,7 +639,8 @@ namespace clang {
   return ImportArrayChecked(InContainer.begin(), InContainer.end(), 
Obegin);
 }
 
-void ImportOverrides(CXXMethodDecl *ToMethod, CXXMethodDecl *FromMethod);
+Error ImportOverriddenMethods(CXXMethodDecl *ToMethod,
+  CXXMethodDecl *FromMethod);
 
 Expected FindFunctionTemplateSpecialization(
 FunctionDecl *FromFD);
@@ -3370,7 +3371,9 @@ ExpectedDecl ASTNodeImporter::VisitFunct
   }
 
   if (auto *FromCXXMethod = dyn_cast(D))
-ImportOverrides(cast(ToFunction), FromCXXMethod);
+if (Error Err = ImportOverriddenMethods(cast(ToFunction),
+FromCXXMethod))
+  return std::move(Err);
 
   // Import the rest of the chain. I.e. import all subsequent declarations.
   for (++RedeclIt; RedeclIt != Redecls.end(); ++RedeclIt) {
@@ -7804,15 +7807,18 @@ ExpectedStmt ASTNodeImporter::VisitCXXTy
   *ToTypeOrErr, *ToExprOperandOrErr, *ToSourceRangeOrErr);
 }
 
-void ASTNodeImporter::ImportOverrides(CXXMethodDecl *ToMethod,
-  CXXMethodDecl *FromMethod) {
+Error ASTNodeImporter::ImportOverriddenMethods(CXXMethodDecl *ToMethod,
+   CXXMethodDecl *FromMethod) {
+  Error ImportErrors = Error::success();
   for (auto *FromOverriddenMethod : FromMethod->overridden_methods()) {
 if (auto ImportedOrErr = import(FromOverriddenMethod))
   ToMethod->getCanonicalDecl()->addOverriddenMethod(cast(
   (*ImportedOrErr)->getCanonicalDecl()));
 else
-  consumeError(ImportedOrErr.takeError());
+  ImportErrors =
+  joinErrors(std::move(ImportErrors), ImportedOrErr.takeError());
   }
+  return ImportErrors;
 }
 
 ASTImporter::ASTImporter(ASTContext &ToContext, FileManager &ToFileManager,

Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=370457&r1=370456&r2=370457&view=diff
==
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Fri Aug 30 03:12:14 2019
@@ -5183,6 +5183,50 @@ TEST_P(ErrorHandlingTest,
   }
 }
 
+TEST_P(ErrorHandlingTest, ImportOfOverriddenMethods) {
+  auto MatchFooA =
+  functionDecl(hasName("foo"), hasAncestor(cxxRecordDecl(hasName("A";
+  auto MatchFooB =
+  functionDecl(hasName("foo"), hasAncestor(cxxRecordDecl(hasName("B";
+  auto MatchFooC =
+  functionDecl(hasName("foo"), hasAncestor(cxxRecordDecl(hasName("C";
+
+  // Provoke import of a method that has overridden methods with import error.
+  TranslationUnitDecl *FromTU = getTuDecl(std::string(R"(
+struct C;
+struct A {
+  virtual void foo();
+  void f1(C *);
+};
+void A::foo() {
+  )") + ErroneousStmt + R"(
+}
+struct B : public A {
+  void foo() override;
+};
+struct C : public B {
+  void foo() override;
+};
+)",
+  Lang_CXX11);
+  auto *FromFooA = FirstDeclMatcher().match(FromTU, MatchFooA);
+  auto *FromFooB = FirstDeclMatcher().match(FromTU, MatchFooB);
+  auto *FromFooC = FirstDeclMatcher().match(FromTU, MatchFooC);
+
+  EXPECT_FALSE(Import(FromFooA, Lang_CXX11));
+  ASTImporter *Importer = findFromTU(FromFooA)->Importer.get();
+  auto CheckError = [&Importer](Decl *FromD) {
+Optional OptErr = Importer->getImportDeclErrorIfAny(FromD);
+ASSERT_TRUE(OptErr);
+EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+  };
+  CheckError(FromFooA);
+  EXPECT_FALSE(Import(FromFooB, Lang_CXX11));
+  CheckError(FromFooB);
+  EXPECT_FALSE(Import(FromFooC, Lang_CXX11));
+  CheckError(FromFooC);
+}
+
 TEST_P(ASTImporterOptionSpecificTestBase, La

[PATCH] D66933: [ASTImporter] Propagate errors during import of overridden methods.

[Balázs Kéri via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370457: [ASTImporter] Propagate errors during import of 
overridden methods. (authored by balazske, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66933?vs=218039&id=218060#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66933

Files:
  cfe/trunk/lib/AST/ASTImporter.cpp
  cfe/trunk/unittests/AST/ASTImporterTest.cpp

Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -639,7 +639,8 @@
   return ImportArrayChecked(InContainer.begin(), InContainer.end(), Obegin);
 }
 
-void ImportOverrides(CXXMethodDecl *ToMethod, CXXMethodDecl *FromMethod);
+Error ImportOverriddenMethods(CXXMethodDecl *ToMethod,
+  CXXMethodDecl *FromMethod);
 
 Expected FindFunctionTemplateSpecialization(
 FunctionDecl *FromFD);
@@ -3370,7 +3371,9 @@
   }
 
   if (auto *FromCXXMethod = dyn_cast(D))
-ImportOverrides(cast(ToFunction), FromCXXMethod);
+if (Error Err = ImportOverriddenMethods(cast(ToFunction),
+FromCXXMethod))
+  return std::move(Err);
 
   // Import the rest of the chain. I.e. import all subsequent declarations.
   for (++RedeclIt; RedeclIt != Redecls.end(); ++RedeclIt) {
@@ -7804,15 +7807,18 @@
   *ToTypeOrErr, *ToExprOperandOrErr, *ToSourceRangeOrErr);
 }
 
-void ASTNodeImporter::ImportOverrides(CXXMethodDecl *ToMethod,
-  CXXMethodDecl *FromMethod) {
+Error ASTNodeImporter::ImportOverriddenMethods(CXXMethodDecl *ToMethod,
+   CXXMethodDecl *FromMethod) {
+  Error ImportErrors = Error::success();
   for (auto *FromOverriddenMethod : FromMethod->overridden_methods()) {
 if (auto ImportedOrErr = import(FromOverriddenMethod))
   ToMethod->getCanonicalDecl()->addOverriddenMethod(cast(
   (*ImportedOrErr)->getCanonicalDecl()));
 else
-  consumeError(ImportedOrErr.takeError());
+  ImportErrors =
+  joinErrors(std::move(ImportErrors), ImportedOrErr.takeError());
   }
+  return ImportErrors;
 }
 
 ASTImporter::ASTImporter(ASTContext &ToContext, FileManager &ToFileManager,
Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp
===
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp
@@ -5183,6 +5183,50 @@
   }
 }
 
+TEST_P(ErrorHandlingTest, ImportOfOverriddenMethods) {
+  auto MatchFooA =
+  functionDecl(hasName("foo"), hasAncestor(cxxRecordDecl(hasName("A";
+  auto MatchFooB =
+  functionDecl(hasName("foo"), hasAncestor(cxxRecordDecl(hasName("B";
+  auto MatchFooC =
+  functionDecl(hasName("foo"), hasAncestor(cxxRecordDecl(hasName("C";
+
+  // Provoke import of a method that has overridden methods with import error.
+  TranslationUnitDecl *FromTU = getTuDecl(std::string(R"(
+struct C;
+struct A {
+  virtual void foo();
+  void f1(C *);
+};
+void A::foo() {
+  )") + ErroneousStmt + R"(
+}
+struct B : public A {
+  void foo() override;
+};
+struct C : public B {
+  void foo() override;
+};
+)",
+  Lang_CXX11);
+  auto *FromFooA = FirstDeclMatcher().match(FromTU, MatchFooA);
+  auto *FromFooB = FirstDeclMatcher().match(FromTU, MatchFooB);
+  auto *FromFooC = FirstDeclMatcher().match(FromTU, MatchFooC);
+
+  EXPECT_FALSE(Import(FromFooA, Lang_CXX11));
+  ASTImporter *Importer = findFromTU(FromFooA)->Importer.get();
+  auto CheckError = [&Importer](Decl *FromD) {
+Optional OptErr = Importer->getImportDeclErrorIfAny(FromD);
+ASSERT_TRUE(OptErr);
+EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+  };
+  CheckError(FromFooA);
+  EXPECT_FALSE(Import(FromFooB, Lang_CXX11));
+  CheckError(FromFooB);
+  EXPECT_FALSE(Import(FromFooC, Lang_CXX11));
+  CheckError(FromFooC);
+}
+
 TEST_P(ASTImporterOptionSpecificTestBase, LambdaInFunctionBody) {
   Decl *FromTU = getTuDecl(
   R"(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66516: [clangd] Added highlighting to types dependant on templates.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:177
   return;
+if (TP->isPointerType() || TP->isLValueReferenceType())
+  // When highlighting dependant template types the type can be a pointer 
or

jvikstrom wrote:
> ilya-biryukov wrote:
> > ilya-biryukov wrote:
> > > jvikstrom wrote:
> > > > ilya-biryukov wrote:
> > > > > jvikstrom wrote:
> > > > > > ilya-biryukov wrote:
> > > > > > > jvikstrom wrote:
> > > > > > > > ilya-biryukov wrote:
> > > > > > > > > `RecursiveASTVisitor` also traverses the pointer and 
> > > > > > > > > reference types, why does it not reach the inner 
> > > > > > > > > `TemplateTypeParmType` in the cases you describe?
> > > > > > > > The D in `using D = ...` `typedef ... D` does not have a 
> > > > > > > > TypeLoc (at least not one that is visited). Therefore we use 
> > > > > > > > the VisitTypedefNameDecl (line 121) to get the location of `D` 
> > > > > > > > to be able to highlight it. And we just send the typeLocs 
> > > > > > > > typeptr to addType (which is a Pointer for `using D = T*;`)...
> > > > > > > > 
> > > > > > > > But maybe we should get the underlying type before we call 
> > > > > > > > addType with TypePtr? Just a while loop on line 123 basically 
> > > > > > > > (can we have multiple PointerTypes nested in each other 
> > > > > > > > actually?)
> > > > > > > > 
> > > > > > > > Even if we keep it in addType the comment is actually wrong, 
> > > > > > > > because it obviously works when for the actual "type 
> > > > > > > > occurrences" for `D` (so will fix that no matter what). This 
> > > > > > > > recursion will just make us add more duplicate tokens...
> > > > > > > Could we investigate why `RecursiveASTVisitor` does not visit the 
> > > > > > > `TypeLoc` of a corresponding decl?
> > > > > > > Here's the code from `RecursiveASTVisitor.h` that should do the 
> > > > > > > trick:
> > > > > > > ```
> > > > > > > DEF_TRAVERSE_DECL(TypeAliasDecl, {
> > > > > > >   TRY_TO(TraverseTypeLoc(D->getTypeSourceInfo()->getTypeLoc()));
> > > > > > >   // We shouldn't traverse D->getTypeForDecl(); it's a result of
> > > > > > >   // declaring the type alias, not something that was written in 
> > > > > > > the
> > > > > > >   // source.
> > > > > > > })
> > > > > > > ```
> > > > > > > 
> > > > > > > If it doesn't, we are probably holding it wrong.
> > > > > > There just doesn't seem to be a TypeLoc for the typedef'ed Decl.  
> > > > > > We can get the `T*` TypeLoc (with 
> > > > > > `D->getTypeSourceInfo()->getTypeLoc()`). But there isn't one for 
> > > > > > `D`. Even the `D->getTypeForDecl` returns null.
> > > > > > 
> > > > > > And I have no idea where I'd even start debugging that. Or if it's 
> > > > > > even a bug
> > > > > > 
> > > > > I may have misinterpreted the patch. Are we trying to add 
> > > > > highlightings for the names of using aliases here? E.g. for the 
> > > > > following range:
> > > > > ```
> > > > > template 
> > > > > struct Foo {
> > > > >   using [[D]] = T**;
> > > > > };
> > > > > ```
> > > > > 
> > > > > Why isn't this handled in `VisitNamedDecl`?
> > > > > We don't seem to call this function for `TypedefNameDecl` at all and 
> > > > > it actually weird. Is this because we attempt to highlight typedefs 
> > > > > as their underlying types?
> > > > So currently using aliases and typedefs are highlighted the same as the 
> > > > underlying type (in most cases). One case where they aren't is when the 
> > > > underlying type is a template parameter (which is what this patch is 
> > > > trying to solve).
> > > > 
> > > > 
> > > > > Why isn't this handled in VisitNamedDecl?
> > > > 
> > > > The Decl is actually visited in `VisitNamedDecl`, however as it is a 
> > > > `TypeAliasDecl` which we do not have a check for in the addToken 
> > > > function it will not get highlighted in that visit.
> > > > 
> > > > Actually, could add a check for `TypeAliasDecl` in `addToken` (should 
> > > > probably be a check for `TypedefNameDecl` to cover both `using ...` and 
> > > > `typedef ...`) and move the code from the `VisitTypedefNameDecl` to the 
> > > > `addToken` function inside that check instead.
> > > > 
> > > > 
> > > > 
> > > > > We don't seem to call this function for TypedefNameDecl at all and it 
> > > > > actually weird. Is this because we attempt to highlight typedefs as 
> > > > > their underlying types?
> > > > 
> > > > 
> > > > Don't understand what you mean. What function? 
> > > > So currently using aliases and typedefs are highlighted the same as the 
> > > > underlying type (in most cases). 
> > > Thanks for clarifying this. This is where my confusion is coming from.
> > > A few question to try understanding the approach taken (sorry if that's 
> > > too detailed, I am probably missing the context here)
> > > - What do we fallback to? From my reading of the code, we do not 
> > > highlight them at all if the underlying type is not one of the pred

[PATCH] D66738: [clangd] Added highlighting for structured bindings.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D66738#1652285 , @hokein wrote:

> I don't have a strong preference on how to highlight `BindingDecl`, but I 
> think highlighting them as `Variable` is better than no highlighting them at 
> all, so LGTM from myside. @ilya-biryukov thoughts?


Also think highlighting them as variables is the right thing.
e.g. if I do something like:

  void foo(std::pair p) {
auto [f,s] = p;
return f + s;
  }

this is roughly equivalent to:

  void foo(std::pair p) {
auto f = p.first;
auto s = p.second;
return f + s;
  }

Therefore, it should also be highlighted in the same way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66738



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

[PATCH] D66995: [clangd] Add highlighting for macro expansions.

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom updated this revision to Diff 218061.
jvikstrom added a comment.

Removed SourceManager field from HighlightingTokenCollector.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66995

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/semantic-highlighting.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -47,7 +47,8 @@
   {HighlightingKind::Method, "Method"},
   {HighlightingKind::StaticMethod, "StaticMethod"},
   {HighlightingKind::TemplateParameter, "TemplateParameter"},
-  {HighlightingKind::Primitive, "Primitive"}};
+  {HighlightingKind::Primitive, "Primitive"},
+  {HighlightingKind::MacroExpansion, "MacroExpansion"}};
   std::vector ExpectedTokens;
   for (const auto &KindString : KindToString) {
 std::vector Toks = makeHighlightingTokens(
@@ -391,9 +392,9 @@
   R"cpp(
   #define DEF_MULTIPLE(X) namespace X { class X { int X; }; }
   #define DEF_CLASS(T) class T {};
-  DEF_MULTIPLE(XYZ);
-  DEF_MULTIPLE(XYZW);
-  DEF_CLASS($Class[[A]])
+  $MacroExpansion[[DEF_MULTIPLE]](XYZ);
+  $MacroExpansion[[DEF_MULTIPLE]](XYZW);
+  $MacroExpansion[[DEF_CLASS]]($Class[[A]])
   #define MACRO_CONCAT(X, V, T) T foo##X = V
   #define DEF_VAR(X, V) int X = V
   #define DEF_VAR_T(T, X, V) T X = V
@@ -404,26 +405,27 @@
   #define SOME_NAME_SET variable2 = 123
   #define INC_VAR(X) X += 2
   $Primitive[[void]] $Function[[foo]]() {
-DEF_VAR($LocalVariable[[X]],  123);
-DEF_VAR_REV(908, $LocalVariable[[XY]]);
-$Primitive[[int]] CPY( $LocalVariable[[XX]] );
-DEF_VAR_TYPE($Class[[A]], $LocalVariable[[AA]]);
-$Primitive[[double]] SOME_NAME;
-$Primitive[[int]] SOME_NAME_SET;
+$MacroExpansion[[DEF_VAR]]($LocalVariable[[X]],  123);
+$MacroExpansion[[DEF_VAR_REV]](908, $LocalVariable[[XY]]);
+$Primitive[[int]] $MacroExpansion[[CPY]]( $LocalVariable[[XX]] );
+$MacroExpansion[[DEF_VAR_TYPE]]($Class[[A]], $LocalVariable[[AA]]);
+$Primitive[[double]] $MacroExpansion[[SOME_NAME]];
+$Primitive[[int]] $MacroExpansion[[SOME_NAME_SET]];
 $LocalVariable[[variable]] = 20.1;
-MACRO_CONCAT(var, 2, $Primitive[[float]]);
-DEF_VAR_T($Class[[A]], CPY(CPY($LocalVariable[[Nested]])),
-  CPY($Class[[A]]()));
-INC_VAR($LocalVariable[[variable]]);
+$MacroExpansion[[MACRO_CONCAT]](var, 2, $Primitive[[float]]);
+$MacroExpansion[[DEF_VAR_T]]($Class[[A]], $MacroExpansion[[CPY]](
+  $MacroExpansion[[CPY]]($LocalVariable[[Nested]])),
+$MacroExpansion[[CPY]]($Class[[A]]()));
+$MacroExpansion[[INC_VAR]]($LocalVariable[[variable]]);
   }
-  $Primitive[[void]] SOME_NAME();
-  DEF_VAR($Variable[[XYZ]], 567);
-  DEF_VAR_REV(756, $Variable[[AB]]);
+  $Primitive[[void]] $MacroExpansion[[SOME_NAME]]();
+  $MacroExpansion[[DEF_VAR]]($Variable[[XYZ]], 567);
+  $MacroExpansion[[DEF_VAR_REV]](756, $Variable[[AB]]);
 
   #define CALL_FN(F) F();
   #define DEF_FN(F) void F ()
-  DEF_FN($Function[[g]]) {
-CALL_FN($Function[[foo]]);
+  $MacroExpansion[[DEF_FN]]($Function[[g]]) {
+$MacroExpansion[[CALL_FN]]($Function[[foo]]);
   }
 )cpp",
   R"cpp(
@@ -433,8 +435,8 @@
   $Primitive[[int]] $Variable[[y]];
   $Primitive[[int]] $Function[[f]]();
   $Primitive[[void]] $Function[[foo]]() {
-assert($Variable[[x]] != $Variable[[y]]);
-assert($Variable[[x]] != $Function[[f]]());
+$MacroExpansion[[assert]]($Variable[[x]] != $Variable[[y]]);
+$MacroExpansion[[assert]]($Variable[[x]] != $Function[[f]]());
   }
 )cpp"};
   for (const auto &TestCase : TestCases) {
@@ -455,8 +457,8 @@
   // A separate test for macros in headers.
   checkHighlightings(R"cpp(
 #include "imp.h"
-DEFINE_Y
-DXYZ_Y(A);
+$MacroExpansion[[DEFINE_Y]]
+$MacroExpansion[[DXYZ_Y]](A);
   )cpp",
  {{"imp.h", R"cpp(
 #define DXYZ(X) class X {};
Index: clang-tools-extra/clangd/test/semantic-highlighting.test
===
--- clang-tools-extra/clangd/test/semantic-highlighting.test
+++ clang-tools-extra/clangd/test/semantic-highlighting.test
@@ -45,6 +45,9 @@
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
 # CHECK-NEXT:"storage.type.primitive.cpp"
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  [
+# CHECK-NEXT:"entity.name

[PATCH] D65744: [PR42707][OpenCL] Fix addr space deduction for auto

[Anastasia Stulova via Phabricator via cfe-commits]

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

missing test case

  1 auto* accumulator()
  2 {
  3   __global int * glptr;
  4   return glptr;
  5 }
  6 int i;
  7 auto* ai = &i;
  8 int* ii = &i;

In D65744#1646348 , @rjmccall wrote:

> In D65744#1629055 , @Anastasia wrote:
>
> > In D65744#1627589 , @rjmccall 
> > wrote:
> >
> > > I see.  Is the deduction rule recursive or something, where a pointer to 
> > > pointer is inferred to point to the same address space as the pointee?
> >
> >
> > It is recursive indeed and we currently deduce all pointees to generic AS.
>
>
> Is that likely to change?  If all pointees are inferred to be generic (in the 
> absence of an explicit qualifier) when building a pointer type, this seems 
> very over-thought vs. just adding the generic AS whenever you build a pointer 
> to an unqualified but non-dependent type.


I don't think this is likely to change. Are you suggesting to move the 
deduction logic for pointee of pointers, references and block pointers into 
ASTContext helper that creates a pointer/reference/block pointer type? I guess 
it should work if the only way to construct the pointer is by using this 
helper. I can certainly prototype this change and see.




Comment at: lib/Sema/SemaType.cpp:7441
+  // the initializing expression type during the type deduction.
+  (T->isAutoType() && IsPointee) || (IsAutoPointee) ||
   // OpenCL spec v2.0 s6.9.b:

mantognini wrote:
> mantognini wrote:
> > Shouldn't the parentheses around `IsAutoPointee` be removed for style 
> > consistency?
> With the `if` statement introduced above, `IsAutoPointee` can be true only in 
> C++ mode. Could it be an issue to not guard `(T->isAutoType() && IsPointee)` 
> for non-C++ mode? (I guess not, but I couldn't convince myself.)
I think `TreeTransforms` will only be used in C++ mode. But also `isAutoType` 
should only be true in C++ mode. So I think we should be fine.


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

https://reviews.llvm.org/D65744



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

[PATCH] D66995: [clangd] Add highlighting for macro expansions.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:35
+TraverseAST(AST.getASTContext());
+// Add highlightings for Macro Expansions as they are not traversed by the
+// visitor.

NIT: uncapitalize and use 'macro expansions' instead of `Macro Expansions'



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:469
+  case HighlightingKind::MacroExpansion:
+return "entity.name.function.preprocessor.expansion.cpp";
   case HighlightingKind::NumKinds:

Is there a place on the internet with conventional textmate scope names?
Or do we come up with these on our own?



Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:394
   #define DEF_MULTIPLE(X) namespace X { class X { int X; }; }
   #define DEF_CLASS(T) class T {};
+  $MacroExpansion[[DEF_MULTIPLE]](XYZ);

Similar to how we highlight everything else, we should also highlight macro 
declarations and usages (i.e. expansions in the same way).
Could we add the corresponding highlightings?
```
#define $Macro[[DEF_CLASS]](T) class T T{}
$Macro[[DEF_CLASS]](A);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66995



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

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Trying to start reviewing this.
The llvm side of the patch is self contained; clang patch should be split into 
a dependent review.




Comment at: clang/lib/CodeGen/CodeGenAction.cpp:626
+  unsigned Line, Column;
+  bool BadDebugInfo = false;
+  FullSourceLoc Loc =

This should be `BadDebugInfoLoc` (in `getBestLocationFromDebugLoc()` too)



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:28-1361
 #include "clang/AST/StmtObjC.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "llvm/IR/DataLayout.h"

Dubious changes, drop



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3451-3454
+  const std::vector &warnings = Res.getDiagnosticOpts().Warnings;
+  if (std::find(warnings.begin(), warnings.end(), "misexpect") !=
+  warnings.end())
+Res.FrontendOpts.LLVMArgs.push_back("-pgo-warn-misexpect");

Here you are checking that `-Wmisexpect` is passed to clang?
This doesn't seem idiomatic. Was this copied from somewhere?
Normally this is `Diags.isIgnored(diag::warn_profile_data_misexpect, ???)`



Comment at: llvm/include/llvm/Transforms/Utils/MisExpect.h:1
+//===--- MisExpect.cpp - Check Use of __builtin_expect() with PGO data 
===//
+//

`__builtin_expect()` is a clang-specific thing.
Can all instances of it in all comments be reworded to be more agnostic?



Comment at: llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp:78-87
+  SI.setMetadata(
+  LLVMContext::MD_misexpect,
+  MDBuilder(CI->getContext())
+  .createMisExpect(Index, LikelyBranchWeight, UnlikelyBranchWeight));
+
+  SI.setCondition(ArgValue);
+  misexpect::checkClangInstrumentation(SI);

Why can't `LLVMContext::MD_prof` be reused?



Comment at: llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp:82
 
-  SI.setCondition(ArgValue);
   return true;

Why do you need to move this line?



Comment at: llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp:84
+  SI.setCondition(ArgValue);
+  misexpect::checkClangInstrumentation(SI);
 

clang is not the only llvm frontend, this should not be so specific



Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:30
+#include "llvm/Support/FormatVariadic.h"
+#include 
+#include 

This doesn't look correct.



Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:33
+#include 
+#include 
+





Comment at: llvm/test/Transforms/LowerExpectIntrinsic/basic.ll:288-289
 
 ; CHECK: !0 = !{!"branch_weights", i32 2000, i32 1}
-; CHECK: !1 = !{!"branch_weights", i32 1, i32 2000}
-; CHECK: !2 = !{!"branch_weights", i32 1, i32 2000, i32 1}
-; CHECK: !3 = !{!"branch_weights", i32 2000, i32 1, i32 1}
+; CHECK: !2 = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: !4 = !{!"branch_weights", i32 1, i32 2000, i32 1}

`; CHECK: !1 = ???` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66324



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

r370461 - [ASTImporter] Do not look up lambda classes

[Gabor Marton via cfe-commits]

Author: martong
Date: Fri Aug 30 03:55:41 2019
New Revision: 370461

URL: http://llvm.org/viewvc/llvm-project?rev=370461&view=rev
Log:
[ASTImporter] Do not look up lambda classes

Summary:
Consider this code:
```
  void f() {
auto L0 = [](){};
auto L1 = [](){};
  }

```
First we import `L0` then `L1`. Currently we end up having only one
CXXRecordDecl for the two different lambdas. And that is a problem if
the body of their op() is different. This happens because when we import
`L1` then lookup finds the existing `L0` and since they are structurally
equivalent we just map the imported L0 to be the counterpart of L1.

We have the same problem in this case:
```
  template 
  void f(F0 L0 = [](){}, F1 L1 = [](){}) {}

```

In StructuralEquivalenceContext we could distinquish lambdas only by
their source location in these cases. But we the lambdas are actually
structrually equivalent they differn only by the source location.

Thus, the  solution is to disable lookup completely if the decl in
the "from" context is a lambda.
However, that could have other problems: what if the lambda is defined
in a header file and included in several TUs? I think we'd have as many
duplicates as many includes we have. I think we could live with that,
because the lambda classes are TU local anyway, we cannot just access
them from another TU.

Reviewers: a_sidorin, a.sidorin, shafik

Subscribers: rnkovacs, dkrupp, Szelethus, gamesh411, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/unittests/AST/ASTImporterTest.cpp

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=370461&r1=370460&r2=370461&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Fri Aug 30 03:55:41 2019
@@ -2633,7 +2633,7 @@ ExpectedDecl ASTNodeImporter::VisitRecor
 
   // We may already have a record of the same name; try to find and match it.
   RecordDecl *PrevDecl = nullptr;
-  if (!DC->isFunctionOrMethod()) {
+  if (!DC->isFunctionOrMethod() && !D->isLambda()) {
 SmallVector ConflictingDecls;
 auto FoundDecls =
 Importer.findDeclsInToCtx(DC, SearchName);

Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=370461&r1=370460&r2=370461&view=diff
==
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Fri Aug 30 03:55:41 2019
@@ -5644,6 +5644,117 @@ INSTANTIATE_TEST_CASE_P(ParameterizedTes
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, CanonicalRedeclChain,
 ::testing::Values(ArgVector()), );
 
+TEST_P(ASTImporterOptionSpecificTestBase, LambdasAreDifferentiated) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  void f() {
+auto L0 = [](){};
+auto L1 = [](){};
+  }
+  )",
+  Lang_CXX11, "input0.cc");
+  auto Pattern = lambdaExpr();
+  CXXRecordDecl *FromL0 =
+  FirstDeclMatcher().match(FromTU, Pattern)->getLambdaClass();
+  CXXRecordDecl *FromL1 =
+  LastDeclMatcher().match(FromTU, Pattern)->getLambdaClass();
+  ASSERT_NE(FromL0, FromL1);
+
+  CXXRecordDecl *ToL0 = Import(FromL0, Lang_CXX11);
+  CXXRecordDecl *ToL1 = Import(FromL1, Lang_CXX11);
+  EXPECT_NE(ToL0, ToL1);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase,
+   LambdasInFunctionParamsAreDifferentiated) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  template 
+  void f(F0 L0 = [](){}, F1 L1 = [](){}) {}
+  )",
+  Lang_CXX11, "input0.cc");
+  auto Pattern = cxxRecordDecl(isLambda());
+  CXXRecordDecl *FromL0 =
+  FirstDeclMatcher().match(FromTU, Pattern);
+  CXXRecordDecl *FromL1 =
+  LastDeclMatcher().match(FromTU, Pattern);
+  ASSERT_NE(FromL0, FromL1);
+
+  CXXRecordDecl *ToL0 = Import(FromL0, Lang_CXX11);
+  CXXRecordDecl *ToL1 = Import(FromL1, Lang_CXX11);
+  ASSERT_NE(ToL0, ToL1);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase,
+   LambdasInFunctionParamsAreDifferentiatedWhenMacroIsUsed) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  #define LAMBDA [](){}
+  template 
+  void f(F0 L0 = LAMBDA, F1 L1 = LAMBDA) {}
+  )",
+  Lang_CXX11, "input0.cc");
+  auto Pattern = cxxRecordDecl(isLambda());
+  CXXRecordDecl *FromL0 =
+  FirstDeclMatcher().match(FromTU, Pattern);
+  CXXRecordDecl *FromL1 =
+  LastDeclMatcher().match(FromTU, Pattern);
+  ASSERT_NE(FromL0, FromL1);
+
+  Import(FromL0, Lang_CXX11);
+  Import(FromL1, Lang_CXX11);
+  CXXRecordDecl *ToL0 = Import(FromL0, Lang_CXX11);
+  CXXRecordDecl *ToL1 = Import(FromL1, Lang_CXX11);
+  ASSERT_NE(ToL0, ToL1);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportAssignedLambda) {
+  Decl *Fro

[PATCH] D66348: [ASTImporter] Do not look up lambda classes

[Gabor Marton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370461: [ASTImporter] Do not look up lambda classes 
(authored by martong, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66348?vs=216378&id=218065#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66348

Files:
  cfe/trunk/lib/AST/ASTImporter.cpp
  cfe/trunk/unittests/AST/ASTImporterTest.cpp

Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -2633,7 +2633,7 @@
 
   // We may already have a record of the same name; try to find and match it.
   RecordDecl *PrevDecl = nullptr;
-  if (!DC->isFunctionOrMethod()) {
+  if (!DC->isFunctionOrMethod() && !D->isLambda()) {
 SmallVector ConflictingDecls;
 auto FoundDecls =
 Importer.findDeclsInToCtx(DC, SearchName);
Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp
===
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp
@@ -5644,6 +5644,117 @@
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, CanonicalRedeclChain,
 ::testing::Values(ArgVector()), );
 
+TEST_P(ASTImporterOptionSpecificTestBase, LambdasAreDifferentiated) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  void f() {
+auto L0 = [](){};
+auto L1 = [](){};
+  }
+  )",
+  Lang_CXX11, "input0.cc");
+  auto Pattern = lambdaExpr();
+  CXXRecordDecl *FromL0 =
+  FirstDeclMatcher().match(FromTU, Pattern)->getLambdaClass();
+  CXXRecordDecl *FromL1 =
+  LastDeclMatcher().match(FromTU, Pattern)->getLambdaClass();
+  ASSERT_NE(FromL0, FromL1);
+
+  CXXRecordDecl *ToL0 = Import(FromL0, Lang_CXX11);
+  CXXRecordDecl *ToL1 = Import(FromL1, Lang_CXX11);
+  EXPECT_NE(ToL0, ToL1);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase,
+   LambdasInFunctionParamsAreDifferentiated) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  template 
+  void f(F0 L0 = [](){}, F1 L1 = [](){}) {}
+  )",
+  Lang_CXX11, "input0.cc");
+  auto Pattern = cxxRecordDecl(isLambda());
+  CXXRecordDecl *FromL0 =
+  FirstDeclMatcher().match(FromTU, Pattern);
+  CXXRecordDecl *FromL1 =
+  LastDeclMatcher().match(FromTU, Pattern);
+  ASSERT_NE(FromL0, FromL1);
+
+  CXXRecordDecl *ToL0 = Import(FromL0, Lang_CXX11);
+  CXXRecordDecl *ToL1 = Import(FromL1, Lang_CXX11);
+  ASSERT_NE(ToL0, ToL1);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase,
+   LambdasInFunctionParamsAreDifferentiatedWhenMacroIsUsed) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  #define LAMBDA [](){}
+  template 
+  void f(F0 L0 = LAMBDA, F1 L1 = LAMBDA) {}
+  )",
+  Lang_CXX11, "input0.cc");
+  auto Pattern = cxxRecordDecl(isLambda());
+  CXXRecordDecl *FromL0 =
+  FirstDeclMatcher().match(FromTU, Pattern);
+  CXXRecordDecl *FromL1 =
+  LastDeclMatcher().match(FromTU, Pattern);
+  ASSERT_NE(FromL0, FromL1);
+
+  Import(FromL0, Lang_CXX11);
+  Import(FromL1, Lang_CXX11);
+  CXXRecordDecl *ToL0 = Import(FromL0, Lang_CXX11);
+  CXXRecordDecl *ToL1 = Import(FromL1, Lang_CXX11);
+  ASSERT_NE(ToL0, ToL1);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportAssignedLambda) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  void f() {
+auto x = []{} = {}; auto x2 = x;
+  }
+  )",
+  Lang_CXX2a, "input0.cc");
+  auto FromF = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasName("f")));
+  // We have only one lambda class.
+  ASSERT_EQ(
+  DeclCounter().match(FromTU, cxxRecordDecl(isLambda())),
+  1u);
+
+  FunctionDecl *ToF = Import(FromF, Lang_CXX2a);
+  EXPECT_TRUE(ToF);
+  TranslationUnitDecl *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+  // We have only one lambda class after the import.
+  EXPECT_EQ(DeclCounter().match(ToTU, cxxRecordDecl(isLambda())),
+1u);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportDefaultConstructibleLambdas) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  void f() {
+auto x = []{} = {};
+auto xb = []{} = {};
+  }
+  )",
+  Lang_CXX2a, "input0.cc");
+  auto FromF = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasName("f")));
+  // We have two lambda classes.
+  ASSERT_EQ(
+  DeclCounter().match(FromTU, cxxRecordDecl(isLambda())),
+  2u);
+
+  FunctionDecl *ToF = Import(FromF, Lang_CXX2a);
+  EXPECT_TRUE(ToF);
+  TranslationUnitDecl *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+  // We have two lambda classes after the import.
+  EXPECT_EQ(DeclCounter().match(ToTU, cxxRecordDecl(isLambda())),
+2u);
+}
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest,
 DefaultTestValuesFor

[PATCH] D66538: [AST] AST structural equivalence to work internally with pairs.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

@a_sidorin Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66538



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

[PATCH] D62977: [clang-tidy]: Google: new check 'google-upgrade-googletest-case'

[Sergey Tsatsulin via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp:83
+
+  void Ifndef(SourceLocation Loc, const Token &MacroNameTok,
+  const MacroDefinition &MD) override {

The body of this method is the same as `Ifdef`. Please check.


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

https://reviews.llvm.org/D62977



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

[PATCH] D66738: [clangd] Added highlighting for structured bindings.

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:443
+$Primitive[[int]] $Variable[[A]][2] = {1,2};
+auto [$Variable[[B1]], $Variable[[B2]]] = $Variable[[A]];
+auto [$Variable[[G1]], $Variable[[G2]]] = $Variable[[Global]];

jvikstrom wrote:
> hokein wrote:
> > this is not related to the patch,  but the highlighting behavior for auto 
> > here is weird, some of them are highlighted while some of them are not. 
> The reason for that is broken down here: 
> https://reviews.llvm.org/D66516#inline-599982
> 
> Basically we don't traverse the type hierarchy when trying to add 
> highlightings for auto so if we have a pointer/array type it will not find 
> the actual type and just silently not add any highlightings.
thanks for the explanation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66738



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

[PATCH] D66995: [clangd] Add highlighting for macro expansions.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:469
+  case HighlightingKind::MacroExpansion:
+return "entity.name.function.preprocessor.expansion.cpp";
   case HighlightingKind::NumKinds:

ilya-biryukov wrote:
> Is there a place on the internet with conventional textmate scope names?
> Or do we come up with these on our own?
This is not standard TX scope names, we only have guidelines. In general, we 
use the names using in vscode as references (you could see the TM name via 
vscode's "Insepct TM Scopes" functionality)



Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:394
   #define DEF_MULTIPLE(X) namespace X { class X { int X; }; }
   #define DEF_CLASS(T) class T {};
+  $MacroExpansion[[DEF_MULTIPLE]](XYZ);

ilya-biryukov wrote:
> Similar to how we highlight everything else, we should also highlight macro 
> declarations and usages (i.e. expansions in the same way).
> Could we add the corresponding highlightings?
> ```
> #define $Macro[[DEF_CLASS]](T) class T T{}
> $Macro[[DEF_CLASS]](A);
> ```
+1, I think we could name the HighlightingKind as `Macro` instead of 
`MacroExpansion`  (even through we don't highlighting the macro def yet).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66995



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

[PATCH] D66333: [analyzer] NonNullParamChecker and CStringChecker parameter number in checker message

[Whisperity via Phabricator via cfe-commits]

whisperity accepted this revision.
whisperity added a comment.

@Szelethus Nope.


Repository:
  rC Clang

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

https://reviews.llvm.org/D66333



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

[PATCH] D66505: Make add_new_check.py's insertion of registerCheck<> match the sort order

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Mostly LG, if you've verified that this works. A couple of comments below.




Comment at: clang-tools-extra/clang-tidy/add_new_check.py:192
+  else:
+match = re.search('registerCheck<(.*)> *\( *(?:"([^"]*)")?', line)
+last_line = None

Are there actually any registerCheck calls with spaces between the `>` and the 
`(`? Should we clang-format these instead of supporting them in the script?



Comment at: clang-tools-extra/clang-tidy/add_new_check.py:193
+match = re.search('registerCheck<(.*)> *\( *(?:"([^"]*)")?', line)
+last_line = None
+if match:

I'd call this previous_line or prev_line. "Last" is a bit misleading here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66505



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

[PATCH] D66505: Make add_new_check.py's insertion of registerCheck<> match the sort order

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Generally LG to me as well




Comment at: clang-tools-extra/clang-tidy/add_new_check.py:198
+# If we didn't find the check name on this line, look on the
+# next one
+last_line = line

Missing a full stop at the end of the comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66505



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

[PATCH] D65919: [clang-tidy] Add llvm-prefer-register-over-unsigned check and apply it to LLVM

[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 nit with `auto`.




Comment at: 
clang-tools-extra/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp:47
+for (const auto *UsingDirective : Context->using_directives()) {
+  const auto *Namespace = UsingDirective->getNominatedNamespace();
+  if (isa(Namespace->getDeclContext()) &&

Don't use `const auto *` here as the type is not spelled out in the initializer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65919



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

[libunwind] r370469 - Creating release candidate rc3 from release_900 branch

[Hans Wennborg via cfe-commits]

Author: hans
Date: Fri Aug 30 05:25:14 2019
New Revision: 370469

URL: http://llvm.org/viewvc/llvm-project?rev=370469&view=rev
Log:
Creating release candidate rc3 from release_900 branch

Added:
libunwind/tags/RELEASE_900/rc3/
  - copied from r370468, libunwind/branches/release_90/

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

[libclc] r370469 - Creating release candidate rc3 from release_900 branch

[Hans Wennborg via cfe-commits]

Author: hans
Date: Fri Aug 30 05:25:14 2019
New Revision: 370469

URL: http://llvm.org/viewvc/llvm-project?rev=370469&view=rev
Log:
Creating release candidate rc3 from release_900 branch

Added:
libclc/tags/RELEASE_900/rc3/
  - copied from r370468, libclc/branches/release_90/

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

[PATCH] D66336: [ASTImporter] Add development internals docs

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66336



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

[PATCH] D66999: [ASTImporter][NFC] Add comments to code where we cannot use HandleNameConflict

[Gabor Marton via Phabricator via cfe-commits]

martong created this revision.
martong added reviewers: shafik, a_sidorin.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a project: clang.

This is the continuation of https://reviews.llvm.org/D59692 where we started to
use HandleNameConflict in almost all cases.
The remaining cases are dealt with here.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66999

Files:
  clang/lib/AST/ASTImporter.cpp


Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -3447,12 +3447,15 @@
 return FoundField;
   }
 
-  // FIXME: Why is this case not handled with calling HandleNameConflict?
   Importer.ToDiag(Loc, diag::warn_odr_field_type_inconsistent)
 << Name << D->getType() << FoundField->getType();
   Importer.ToDiag(FoundField->getLocation(), diag::note_odr_value_here)
 << FoundField->getType();
-
+  // This case is not handled with HandleNameConflict, because by the time
+  // when we reach here, the enclosing class is already imported. If there
+  // was any NameConflict during the import of that class it is already
+  // handled. (Otherwise we have a contradiction between the structural
+  // equivalency check of that class and its field [this field].)
   return make_error(ImportError::NameConflict);
 }
   }
@@ -3518,12 +3521,15 @@
   if (!Name && I < N-1)
 continue;
 
-  // FIXME: Why is this case not handled with calling HandleNameConflict?
   Importer.ToDiag(Loc, diag::warn_odr_field_type_inconsistent)
 << Name << D->getType() << FoundField->getType();
   Importer.ToDiag(FoundField->getLocation(), diag::note_odr_value_here)
 << FoundField->getType();
-
+  // This case is not handled with HandleNameConflict, because by the time
+  // when we reach here, the enclosing class is already imported. If there
+  // was any NameConflict during the import of that class it is already
+  // handled. (Otherwise we have a contradiction between the structural
+  // equivalency check of that class and its field [this field].)
   return make_error(ImportError::NameConflict);
 }
   }
@@ -5248,7 +5254,12 @@
 return PrevDecl;
   }
 } else { // ODR violation.
-  // FIXME HandleNameConflict
+  // This is not a name conflict, we have specializations with same
+  // parameters, but with different definitions.
+  // HandleNameConflict cannot be used here and Liberal strategy could not
+  // work because we cannot register two specializations for a primary
+  // template.
+  // FIXME Perhaps return with a different error?
   return make_error(ImportError::NameConflict);
 }
   }


Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -3447,12 +3447,15 @@
 return FoundField;
   }
 
-  // FIXME: Why is this case not handled with calling HandleNameConflict?
   Importer.ToDiag(Loc, diag::warn_odr_field_type_inconsistent)
 << Name << D->getType() << FoundField->getType();
   Importer.ToDiag(FoundField->getLocation(), diag::note_odr_value_here)
 << FoundField->getType();
-
+  // This case is not handled with HandleNameConflict, because by the time
+  // when we reach here, the enclosing class is already imported. If there
+  // was any NameConflict during the import of that class it is already
+  // handled. (Otherwise we have a contradiction between the structural
+  // equivalency check of that class and its field [this field].)
   return make_error(ImportError::NameConflict);
 }
   }
@@ -3518,12 +3521,15 @@
   if (!Name && I < N-1)
 continue;
 
-  // FIXME: Why is this case not handled with calling HandleNameConflict?
   Importer.ToDiag(Loc, diag::warn_odr_field_type_inconsistent)
 << Name << D->getType() << FoundField->getType();
   Importer.ToDiag(FoundField->getLocation(), diag::note_odr_value_here)
 << FoundField->getType();
-
+  // This case is not handled with HandleNameConflict, because by the time
+  // when we reach here, the enclosing class is already imported. If there
+  // was any NameConflict during the import of that class it is already
+  // handled. (Otherwise we have a contradiction between the structural
+  // equivalency check of that class and its field [this field].)
   return make_error(ImportError::NameConflict);
 }
   }
@@ -5248,7 +5254,12 @@
 return PrevDecl;
   }
 } else { // ODR violation.
-  // FIXME HandleNameConflict
+  // This is not a name conflict, we have specializations with same
+  // parameters, but with different definiti

[PATCH] D66524: [SVE][Inline-Asm] Add constraints for SVE predicate registers

[Kerry McLaughlin via Phabricator via cfe-commits]

kmclaughlin updated this revision to Diff 218071.
kmclaughlin added a comment.

- Added isPredicateConstraint function to AArch64ISelLowering.cpp, which 
returns Upl, Upa or Invalid. This is used to replace some repeated checks of 
the predicate type
- Minor changes to InlineAsm.cpp


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

https://reviews.llvm.org/D66524

Files:
  docs/LangRef.rst
  lib/IR/InlineAsm.cpp
  lib/Target/AArch64/AArch64AsmPrinter.cpp
  lib/Target/AArch64/AArch64ISelLowering.cpp
  lib/Target/AArch64/AArch64InstrInfo.cpp
  test/CodeGen/AArch64/aarch64-sve-asm.ll

Index: test/CodeGen/AArch64/aarch64-sve-asm.ll
===
--- test/CodeGen/AArch64/aarch64-sve-asm.ll
+++ test/CodeGen/AArch64/aarch64-sve-asm.ll
@@ -42,3 +42,23 @@
   %1 = tail call  asm "fmul $0.s, $1.s, $2.s", "=w,w,x"( %Zn,  %Zm)
   ret  %1
 }
+
+; Function Attrs: nounwind readnone
+; CHECK: [[ARG1:%[0-9]+]]:zpr = COPY $z1
+; CHECK: [[ARG2:%[0-9]+]]:zpr = COPY $z0
+; CHECK: [[ARG3:%[0-9]+]]:ppr = COPY $p0
+; CHECK: [[ARG4:%[0-9]+]]:ppr_3b = COPY [[ARG3]]
+define  @test_svfadd_f16( %Pg,  %Zn,  %Zm) {
+  %1 = tail call  asm "fadd $0.h, $1/m, $2.h, $3.h", "=w,@3Upl,w,w"( %Pg,  %Zn,  %Zm)
+  ret  %1
+}
+
+; Function Attrs: nounwind readnone
+; CHECK: [[ARG1:%[0-9]+]]:zpr = COPY $z0
+; CHECK: [[ARG2:%[0-9]+]]:ppr = COPY $p0
+; CHECK: [[ARG3:%[0-9]+]]:ppr = COPY [[ARG2]]
+; CHECK: [[ARG4:%[0-9]+]]:zpr = COPY [[ARG1]]
+define  @test_incp( %Pg,  %Zn) {
+  %1 = tail call  asm "incp $0.s, $1", "=w,@3Upa,0"( %Pg,  %Zn)
+  ret  %1
+}
Index: lib/Target/AArch64/AArch64InstrInfo.cpp
===
--- lib/Target/AArch64/AArch64InstrInfo.cpp
+++ lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -2484,6 +2484,17 @@
 return;
   }
 
+  // Copy a Predicate register by ORRing with itself.
+  if (AArch64::PPRRegClass.contains(DestReg) &&
+  AArch64::PPRRegClass.contains(SrcReg)) {
+assert(Subtarget.hasSVE() && "Unexpected SVE register.");
+BuildMI(MBB, I, DL, get(AArch64::ORR_PPzPP), DestReg)
+  .addReg(SrcReg) // Pg
+  .addReg(SrcReg)
+  .addReg(SrcReg, getKillRegState(KillSrc));
+return;
+  }
+
   // Copy a Z register by ORRing with itself.
   if (AArch64::ZPRRegClass.contains(DestReg) &&
   AArch64::ZPRRegClass.contains(SrcReg)) {
Index: lib/Target/AArch64/AArch64ISelLowering.cpp
===
--- lib/Target/AArch64/AArch64ISelLowering.cpp
+++ lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -5738,6 +5738,21 @@
   return "r";
 }
 
+enum PredicateConstraint {
+  Upl,
+  Upa,
+  Invalid
+};
+
+PredicateConstraint isPredicateConstraint(StringRef Constraint) {
+  PredicateConstraint P = PredicateConstraint::Invalid;
+  if (Constraint == "Upa")
+P = PredicateConstraint::Upa;
+  if (Constraint == "Upl")
+P = PredicateConstraint::Upl;
+  return P;
+}
+
 /// getConstraintType - Given a constraint letter, return the type of
 /// constraint it is for this target.
 AArch64TargetLowering::ConstraintType
@@ -5767,7 +5782,8 @@
 case 'S': // A symbolic address
   return C_Other;
 }
-  }
+  } else if (isPredicateConstraint(Constraint) != PredicateConstraint::Invalid)
+  return C_RegisterClass;
   return TargetLowering::getConstraintType(Constraint);
 }
 
@@ -5798,6 +5814,10 @@
   case 'z':
 weight = CW_Constant;
 break;
+  case 'U':
+if (isPredicateConstraint(constraint) != PredicateConstraint::Invalid)
+  weight = CW_Register;
+break;
   }
   return weight;
 }
@@ -5841,6 +5861,14 @@
 return std::make_pair(0U, &AArch64::ZPR_3bRegClass);
   break;
 }
+  } else {
+PredicateConstraint PC = isPredicateConstraint(Constraint);
+if (PC != PredicateConstraint::Invalid) {
+  assert(VT.isScalableVector());
+  bool restricted = (PC == PredicateConstraint::Upl);
+  return restricted ? std::make_pair(0U, &AArch64::PPR_3bRegClass)
+  : std::make_pair(0U, &AArch64::PPRRegClass);
+}
   }
   if (StringRef("{cc}").equals_lower(Constraint))
 return std::make_pair(unsigned(AArch64::NZCV), &AArch64::CCRRegClass);
Index: lib/Target/AArch64/AArch64AsmPrinter.cpp
===
--- lib/Target/AArch64/AArch64AsmPrinter.cpp
+++ lib/Target/AArch64/AArch64AsmPrinter.cpp
@@ -618,6 +618,8 @@
 const TargetRegisterClass *RegClass;
 if (AArch64::ZPRRegClass.contains(Reg)) {
   RegClass = &AArch64::ZPRRegClass;
+} else if (AArch64::PPRRegClass.contains(Reg)) {
+  RegClass = &AArch64::PPRRegClass;
 } else {
   RegClass = &AArch64::FPR128RegClass;
   AltName = AArch64::vreg;
Index: lib/IR/InlineAsm.cpp
===
--- lib/IR/InlineAsm.cpp
+++ lib/IR/InlineAsm.cpp
@@ -181,6 +181,16 @@
   // FIXME: For now assuming these are 2-character con

[PATCH] D66018: [ARM] Take into account -mcpu and -mfpu options while handling 'crypto' feature

[Kristina Bessonova via Phabricator via cfe-commits]

krisb updated this revision to Diff 218076.
krisb edited the summary of this revision.
krisb added a comment.

Added 'CPU.empty()' check back.


Repository:
  rC Clang

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

https://reviews.llvm.org/D66018

Files:
  lib/Driver/ToolChains/Arch/ARM.cpp
  test/Driver/arm-features.c


Index: test/Driver/arm-features.c
===
--- test/Driver/arm-features.c
+++ test/Driver/arm-features.c
@@ -37,7 +37,8 @@
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.2a+crypto -### -c %s 
2>&1 | FileCheck -check-prefix=CHECK-CRYPTO2 %s
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.3a+crypto -### -c %s 
2>&1 | FileCheck -check-prefix=CHECK-CRYPTO2 %s
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.4a+crypto -### -c %s 
2>&1 | FileCheck -check-prefix=CHECK-CRYPTO2 %s
-// CHECK-CRYPTO2: "-cc1"{{.*}} "-target-cpu" "generic"{{.*}} "-target-feature" 
"+crypto" "-target-feature" "+sha2" "-target-feature" "+aes"
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.5a+crypto -### -c %s 
2>&1 | FileCheck -check-prefix=CHECK-CRYPTO2 %s
+// CHECK-CRYPTO2: "-cc1"{{.*}} "-target-cpu" "generic"{{.*}} "-target-feature" 
"+crypto"{{.*}} "-target-feature" "+sha2" "-target-feature" "+aes"
 //
 // Check -crypto:
 //
@@ -45,14 +46,27 @@
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.2a+nocrypto -### -c %s 
2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO2 %s
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.3a+nocrypto -### -c %s 
2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO2 %s
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.4a+nocrypto -### -c %s 
2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO2 %s
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.5a+nocrypto -### -c %s 
2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO2 %s
 // CHECK-NOCRYPTO2-NOT: "-target-feature" "+crypto" "-target-feature" "+sha2" 
"-target-feature" "+aes"
 //
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a57+crypto -### -c %s 
2>&1 | FileCheck -check-prefix=CHECK-CRYPTO2-CPU %s
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a57 
-mfpu=crypto-neon-fp-armv8 -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-CRYPTO2-CPU %s
+// CHECK-CRYPTO2-CPU: "-cc1"{{.*}} "-target-cpu" "cortex-a57"{{.*}} 
"-target-feature" "+crypto"{{.*}} "-target-feature" "+sha2" "-target-feature" 
"+aes"
+//
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a57+norypto -### -c %s 
2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO2-CPU %s
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a57 -mfpu=neon-fp-armv8 
-### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO2-CPU %s
+// CHECK-NOCRYPTO2-CPU-NOT: "-cc1"{{.*}} "-target-cpu" "cortex-a57"{{.*}} 
"-target-feature" "+crypto"{{.*}} "-target-feature" "+sha2" "-target-feature" 
"+aes"
+//
 // Check +crypto -sha2 -aes:
 //
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.1a+crypto+nosha2+noaes 
-### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO3 %s
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a57+crypto+nosha2+noaes 
-### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO3 %s
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a57+nosha2+noaes 
-mfpu=crypto-neon-fp-armv8 -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-CRYPTO3 %s
 // CHECK-CRYPTO3-NOT: "-target-feature" "+sha2" "-target-feature" "+aes"
 //
 // Check -crypto +sha2 +aes:
 //
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.1a+nocrypto+sha2+aes 
-### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO4 %s
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a57+nocrypto+sha2+aes 
-### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO4 %s
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a57+sha2+aes 
-mfpu=neon-fp-armv8 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO4 %s
 // CHECK-CRYPTO4: "-target-feature" "+sha2" "-target-feature" "+aes"
Index: lib/Driver/ToolChains/Arch/ARM.cpp
===
--- lib/Driver/ToolChains/Arch/ARM.cpp
+++ lib/Driver/ToolChains/Arch/ARM.cpp
@@ -479,21 +479,20 @@
 
   // For Arch >= ARMv8.0:  crypto = sha2 + aes
   // FIXME: this needs reimplementation after the TargetParser rewrite
-  if (ArchName.find_lower("armv8a") != StringRef::npos ||
-  ArchName.find_lower("armv8.1a") != StringRef::npos ||
-  ArchName.find_lower("armv8.2a") != StringRef::npos ||
-  ArchName.find_lower("armv8.3a") != StringRef::npos ||
-  ArchName.find_lower("armv8.4a") != StringRef::npos) {
-if (ArchName.find_lower("+crypto") != StringRef::npos) {
-  if (ArchName.find_lower("+nosha2") == StringRef::npos)
+  StringRef ArchSuffix = arm::getLLVMArchSuffixForARM(
+  arm::getARMTargetCPU(CPUName, ArchName, Triple), ArchName, Triple);
+
+  if (llvm::ARM::parseArchVersion(ArchSuffix) >= 8) {
+auto CryptoIt =
+llvm::find_if(llvm::reverse(Features),
+  [](const Strin

[PATCH] D66018: [ARM] Take into account -mcpu and -mfpu options while handling 'crypto' feature

[Kristina Bessonova via Phabricator via cfe-commits]

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

@dnsampaio thanks for reviewing this! Could I ask you to commit the patch 
(since I don't have commit access yet)?


Repository:
  rC Clang

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

https://reviews.llvm.org/D66018



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

[PATCH] D66995: [clangd] Add highlighting for macro expansions.

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom updated this revision to Diff 218087.
jvikstrom marked 3 inline comments as done.
jvikstrom added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66995

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/semantic-highlighting.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -47,7 +47,8 @@
   {HighlightingKind::Method, "Method"},
   {HighlightingKind::StaticMethod, "StaticMethod"},
   {HighlightingKind::TemplateParameter, "TemplateParameter"},
-  {HighlightingKind::Primitive, "Primitive"}};
+  {HighlightingKind::Primitive, "Primitive"},
+  {HighlightingKind::MacroExpansion, "MacroExpansion"}};
   std::vector ExpectedTokens;
   for (const auto &KindString : KindToString) {
 std::vector Toks = makeHighlightingTokens(
@@ -391,9 +392,9 @@
   R"cpp(
   #define DEF_MULTIPLE(X) namespace X { class X { int X; }; }
   #define DEF_CLASS(T) class T {};
-  DEF_MULTIPLE(XYZ);
-  DEF_MULTIPLE(XYZW);
-  DEF_CLASS($Class[[A]])
+  $MacroExpansion[[DEF_MULTIPLE]](XYZ);
+  $MacroExpansion[[DEF_MULTIPLE]](XYZW);
+  $MacroExpansion[[DEF_CLASS]]($Class[[A]])
   #define MACRO_CONCAT(X, V, T) T foo##X = V
   #define DEF_VAR(X, V) int X = V
   #define DEF_VAR_T(T, X, V) T X = V
@@ -404,26 +405,27 @@
   #define SOME_NAME_SET variable2 = 123
   #define INC_VAR(X) X += 2
   $Primitive[[void]] $Function[[foo]]() {
-DEF_VAR($LocalVariable[[X]],  123);
-DEF_VAR_REV(908, $LocalVariable[[XY]]);
-$Primitive[[int]] CPY( $LocalVariable[[XX]] );
-DEF_VAR_TYPE($Class[[A]], $LocalVariable[[AA]]);
-$Primitive[[double]] SOME_NAME;
-$Primitive[[int]] SOME_NAME_SET;
+$MacroExpansion[[DEF_VAR]]($LocalVariable[[X]],  123);
+$MacroExpansion[[DEF_VAR_REV]](908, $LocalVariable[[XY]]);
+$Primitive[[int]] $MacroExpansion[[CPY]]( $LocalVariable[[XX]] );
+$MacroExpansion[[DEF_VAR_TYPE]]($Class[[A]], $LocalVariable[[AA]]);
+$Primitive[[double]] $MacroExpansion[[SOME_NAME]];
+$Primitive[[int]] $MacroExpansion[[SOME_NAME_SET]];
 $LocalVariable[[variable]] = 20.1;
-MACRO_CONCAT(var, 2, $Primitive[[float]]);
-DEF_VAR_T($Class[[A]], CPY(CPY($LocalVariable[[Nested]])),
-  CPY($Class[[A]]()));
-INC_VAR($LocalVariable[[variable]]);
+$MacroExpansion[[MACRO_CONCAT]](var, 2, $Primitive[[float]]);
+$MacroExpansion[[DEF_VAR_T]]($Class[[A]], $MacroExpansion[[CPY]](
+  $MacroExpansion[[CPY]]($LocalVariable[[Nested]])),
+$MacroExpansion[[CPY]]($Class[[A]]()));
+$MacroExpansion[[INC_VAR]]($LocalVariable[[variable]]);
   }
-  $Primitive[[void]] SOME_NAME();
-  DEF_VAR($Variable[[XYZ]], 567);
-  DEF_VAR_REV(756, $Variable[[AB]]);
+  $Primitive[[void]] $MacroExpansion[[SOME_NAME]]();
+  $MacroExpansion[[DEF_VAR]]($Variable[[XYZ]], 567);
+  $MacroExpansion[[DEF_VAR_REV]](756, $Variable[[AB]]);
 
   #define CALL_FN(F) F();
   #define DEF_FN(F) void F ()
-  DEF_FN($Function[[g]]) {
-CALL_FN($Function[[foo]]);
+  $MacroExpansion[[DEF_FN]]($Function[[g]]) {
+$MacroExpansion[[CALL_FN]]($Function[[foo]]);
   }
 )cpp",
   R"cpp(
@@ -433,8 +435,8 @@
   $Primitive[[int]] $Variable[[y]];
   $Primitive[[int]] $Function[[f]]();
   $Primitive[[void]] $Function[[foo]]() {
-assert($Variable[[x]] != $Variable[[y]]);
-assert($Variable[[x]] != $Function[[f]]());
+$MacroExpansion[[assert]]($Variable[[x]] != $Variable[[y]]);
+$MacroExpansion[[assert]]($Variable[[x]] != $Function[[f]]());
   }
 )cpp"};
   for (const auto &TestCase : TestCases) {
@@ -455,8 +457,8 @@
   // A separate test for macros in headers.
   checkHighlightings(R"cpp(
 #include "imp.h"
-DEFINE_Y
-DXYZ_Y(A);
+$MacroExpansion[[DEFINE_Y]]
+$MacroExpansion[[DXYZ_Y]](A);
   )cpp",
  {{"imp.h", R"cpp(
 #define DXYZ(X) class X {};
Index: clang-tools-extra/clangd/test/semantic-highlighting.test
===
--- clang-tools-extra/clangd/test/semantic-highlighting.test
+++ clang-tools-extra/clangd/test/semantic-highlighting.test
@@ -45,6 +45,9 @@
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
 # CHECK-NEXT:"storage.type.primitive.cpp"
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  [
+# CHECK-NEXT:"entity.nam

[PATCH] D66995: [clangd] Add highlighting for macro expansions.

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:469
+  case HighlightingKind::MacroExpansion:
+return "entity.name.function.preprocessor.expansion.cpp";
   case HighlightingKind::NumKinds:

ilya-biryukov wrote:
> Is there a place on the internet with conventional textmate scope names?
> Or do we come up with these on our own?
This is from the vscode c++ extension. (we mostly use the same scopes as in 
that extension, but in a few cases we have our own scopes as there aren't any 
in vscode)



Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:394
   #define DEF_MULTIPLE(X) namespace X { class X { int X; }; }
   #define DEF_CLASS(T) class T {};
+  $MacroExpansion[[DEF_MULTIPLE]](XYZ);

hokein wrote:
> ilya-biryukov wrote:
> > Similar to how we highlight everything else, we should also highlight macro 
> > declarations and usages (i.e. expansions in the same way).
> > Could we add the corresponding highlightings?
> > ```
> > #define $Macro[[DEF_CLASS]](T) class T T{}
> > $Macro[[DEF_CLASS]](A);
> > ```
> +1, I think we could name the HighlightingKind as `Macro` instead of 
> `MacroExpansion`  (even through we don't highlighting the macro def yet).
We don't seem to collect information about where macro declarations are in the 
code in ParsedAST.
Added a fixme to add it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66995



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

[PATCH] D66995: [clangd] Add highlighting for macro expansions.

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom updated this revision to Diff 218089.
jvikstrom added a comment.

Renamed MacroExpansion to Macro.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66995

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/semantic-highlighting.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -47,7 +47,8 @@
   {HighlightingKind::Method, "Method"},
   {HighlightingKind::StaticMethod, "StaticMethod"},
   {HighlightingKind::TemplateParameter, "TemplateParameter"},
-  {HighlightingKind::Primitive, "Primitive"}};
+  {HighlightingKind::Primitive, "Primitive"},
+  {HighlightingKind::Macro, "Macro"}};
   std::vector ExpectedTokens;
   for (const auto &KindString : KindToString) {
 std::vector Toks = makeHighlightingTokens(
@@ -391,9 +392,9 @@
   R"cpp(
   #define DEF_MULTIPLE(X) namespace X { class X { int X; }; }
   #define DEF_CLASS(T) class T {};
-  DEF_MULTIPLE(XYZ);
-  DEF_MULTIPLE(XYZW);
-  DEF_CLASS($Class[[A]])
+  $Macro[[DEF_MULTIPLE]](XYZ);
+  $Macro[[DEF_MULTIPLE]](XYZW);
+  $Macro[[DEF_CLASS]]($Class[[A]])
   #define MACRO_CONCAT(X, V, T) T foo##X = V
   #define DEF_VAR(X, V) int X = V
   #define DEF_VAR_T(T, X, V) T X = V
@@ -404,26 +405,27 @@
   #define SOME_NAME_SET variable2 = 123
   #define INC_VAR(X) X += 2
   $Primitive[[void]] $Function[[foo]]() {
-DEF_VAR($LocalVariable[[X]],  123);
-DEF_VAR_REV(908, $LocalVariable[[XY]]);
-$Primitive[[int]] CPY( $LocalVariable[[XX]] );
-DEF_VAR_TYPE($Class[[A]], $LocalVariable[[AA]]);
-$Primitive[[double]] SOME_NAME;
-$Primitive[[int]] SOME_NAME_SET;
+$Macro[[DEF_VAR]]($LocalVariable[[X]],  123);
+$Macro[[DEF_VAR_REV]](908, $LocalVariable[[XY]]);
+$Primitive[[int]] $Macro[[CPY]]( $LocalVariable[[XX]] );
+$Macro[[DEF_VAR_TYPE]]($Class[[A]], $LocalVariable[[AA]]);
+$Primitive[[double]] $Macro[[SOME_NAME]];
+$Primitive[[int]] $Macro[[SOME_NAME_SET]];
 $LocalVariable[[variable]] = 20.1;
-MACRO_CONCAT(var, 2, $Primitive[[float]]);
-DEF_VAR_T($Class[[A]], CPY(CPY($LocalVariable[[Nested]])),
-  CPY($Class[[A]]()));
-INC_VAR($LocalVariable[[variable]]);
+$Macro[[MACRO_CONCAT]](var, 2, $Primitive[[float]]);
+$Macro[[DEF_VAR_T]]($Class[[A]], $Macro[[CPY]](
+  $Macro[[CPY]]($LocalVariable[[Nested]])),
+$Macro[[CPY]]($Class[[A]]()));
+$Macro[[INC_VAR]]($LocalVariable[[variable]]);
   }
-  $Primitive[[void]] SOME_NAME();
-  DEF_VAR($Variable[[XYZ]], 567);
-  DEF_VAR_REV(756, $Variable[[AB]]);
+  $Primitive[[void]] $Macro[[SOME_NAME]]();
+  $Macro[[DEF_VAR]]($Variable[[XYZ]], 567);
+  $Macro[[DEF_VAR_REV]](756, $Variable[[AB]]);
 
   #define CALL_FN(F) F();
   #define DEF_FN(F) void F ()
-  DEF_FN($Function[[g]]) {
-CALL_FN($Function[[foo]]);
+  $Macro[[DEF_FN]]($Function[[g]]) {
+$Macro[[CALL_FN]]($Function[[foo]]);
   }
 )cpp",
   R"cpp(
@@ -433,8 +435,8 @@
   $Primitive[[int]] $Variable[[y]];
   $Primitive[[int]] $Function[[f]]();
   $Primitive[[void]] $Function[[foo]]() {
-assert($Variable[[x]] != $Variable[[y]]);
-assert($Variable[[x]] != $Function[[f]]());
+$Macro[[assert]]($Variable[[x]] != $Variable[[y]]);
+$Macro[[assert]]($Variable[[x]] != $Function[[f]]());
   }
 )cpp"};
   for (const auto &TestCase : TestCases) {
@@ -455,8 +457,8 @@
   // A separate test for macros in headers.
   checkHighlightings(R"cpp(
 #include "imp.h"
-DEFINE_Y
-DXYZ_Y(A);
+$Macro[[DEFINE_Y]]
+$Macro[[DXYZ_Y]](A);
   )cpp",
  {{"imp.h", R"cpp(
 #define DXYZ(X) class X {};
Index: clang-tools-extra/clangd/test/semantic-highlighting.test
===
--- clang-tools-extra/clangd/test/semantic-highlighting.test
+++ clang-tools-extra/clangd/test/semantic-highlighting.test
@@ -45,6 +45,9 @@
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
 # CHECK-NEXT:"storage.type.primitive.cpp"
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  [
+# CHECK-NEXT:"entity.name.function.preprocessor.expansion.cpp"
 # CHECK-NEXT:  ]
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd

[clang-tools-extra] r370473 - [clangd] Added highlighting for structured bindings.

[Johan Vikstrom via cfe-commits]

Author: jvikstrom
Date: Fri Aug 30 07:07:05 2019
New Revision: 370473

URL: http://llvm.org/viewvc/llvm-project?rev=370473&view=rev
Log:
[clangd] Added highlighting for structured bindings.

Summary: Structured bindings are in a BindingDecl. The decl the declRefExpr 
points to are the BindingDecls. So this adds an additional if statement in the 
addToken function to highlight them.

Reviewers: hokein, ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

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

Modified: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp?rev=370473&r1=370472&r2=370473&view=diff
==
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp (original)
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp Fri Aug 30 07:07:05 
2019
@@ -233,6 +233,10 @@ private:
: HighlightingKind::Variable);
   return;
 }
+if (isa(D)) {
+  addToken(Loc, HighlightingKind::Variable);
+  return;
+}
 if (isa(D)) {
   addToken(Loc, HighlightingKind::Function);
   return;

Modified: clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp?rev=370473&r1=370472&r2=370473&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp 
(original)
+++ clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp Fri 
Aug 30 07:07:05 2019
@@ -436,6 +436,21 @@ TEST(SemanticHighlighting, GetsCorrectTo
 assert($Variable[[x]] != $Variable[[y]]);
 assert($Variable[[x]] != $Function[[f]]());
   }
+)cpp",
+R"cpp(
+  struct $Class[[S]] {
+$Primitive[[float]] $Field[[Value]];
+$Class[[S]] *$Field[[Next]];
+  };
+  $Class[[S]] $Variable[[Global]][2] = {$Class[[S]](), $Class[[S]]()};
+  $Primitive[[void]] $Function[[f]]($Class[[S]] $Parameter[[P]]) {
+$Primitive[[int]] $LocalVariable[[A]][2] = {1,2};
+auto [$Variable[[B1]], $Variable[[B2]]] = $LocalVariable[[A]];
+auto [$Variable[[G1]], $Variable[[G2]]] = $Variable[[Global]];
+$Class[[auto]] [$Variable[[P1]], $Variable[[P2]]] = $Parameter[[P]];
+// Highlights references to BindingDecls.
+$Variable[[B1]]++;
+  }
 )cpp"};
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);


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

[PATCH] D66738: [clangd] Added highlighting for structured bindings.

[Johan Vikström via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370473: [clangd] Added highlighting for structured bindings. 
(authored by jvikstrom, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66738?vs=217652&id=218090#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66738

Files:
  clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
  clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp


Index: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
@@ -233,6 +233,10 @@
: HighlightingKind::Variable);
   return;
 }
+if (isa(D)) {
+  addToken(Loc, HighlightingKind::Variable);
+  return;
+}
 if (isa(D)) {
   addToken(Loc, HighlightingKind::Function);
   return;
Index: clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
@@ -436,6 +436,21 @@
 assert($Variable[[x]] != $Variable[[y]]);
 assert($Variable[[x]] != $Function[[f]]());
   }
+)cpp",
+R"cpp(
+  struct $Class[[S]] {
+$Primitive[[float]] $Field[[Value]];
+$Class[[S]] *$Field[[Next]];
+  };
+  $Class[[S]] $Variable[[Global]][2] = {$Class[[S]](), $Class[[S]]()};
+  $Primitive[[void]] $Function[[f]]($Class[[S]] $Parameter[[P]]) {
+$Primitive[[int]] $LocalVariable[[A]][2] = {1,2};
+auto [$Variable[[B1]], $Variable[[B2]]] = $LocalVariable[[A]];
+auto [$Variable[[G1]], $Variable[[G2]]] = $Variable[[Global]];
+$Class[[auto]] [$Variable[[P1]], $Variable[[P2]]] = $Parameter[[P]];
+// Highlights references to BindingDecls.
+$Variable[[B1]]++;
+  }
 )cpp"};
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);


Index: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
@@ -233,6 +233,10 @@
: HighlightingKind::Variable);
   return;
 }
+if (isa(D)) {
+  addToken(Loc, HighlightingKind::Variable);
+  return;
+}
 if (isa(D)) {
   addToken(Loc, HighlightingKind::Function);
   return;
Index: clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
@@ -436,6 +436,21 @@
 assert($Variable[[x]] != $Variable[[y]]);
 assert($Variable[[x]] != $Function[[f]]());
   }
+)cpp",
+R"cpp(
+  struct $Class[[S]] {
+$Primitive[[float]] $Field[[Value]];
+$Class[[S]] *$Field[[Next]];
+  };
+  $Class[[S]] $Variable[[Global]][2] = {$Class[[S]](), $Class[[S]]()};
+  $Primitive[[void]] $Function[[f]]($Class[[S]] $Parameter[[P]]) {
+$Primitive[[int]] $LocalVariable[[A]][2] = {1,2};
+auto [$Variable[[B1]], $Variable[[B2]]] = $LocalVariable[[A]];
+auto [$Variable[[G1]], $Variable[[G2]]] = $Variable[[Global]];
+$Class[[auto]] [$Variable[[P1]], $Variable[[P2]]] = $Parameter[[P]];
+// Highlights references to BindingDecls.
+$Variable[[B1]]++;
+  }
 )cpp"};
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66995: [clangd] Add highlighting for macro expansions.

[Haojian Wu via Phabricator via cfe-commits]

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

LGTM from my side.




Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:471
+  case HighlightingKind::Macro:
+return "entity.name.function.preprocessor.expansion.cpp";
   case HighlightingKind::NumKinds:

I'd drop the expansion, just use `entity.name.function.preprocessor.cpp`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66995



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

[PATCH] D59922: [Attributor] Deduce "no-capture" argument attribute

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert updated this revision to Diff 218109.
jdoerfert added a comment.

Update tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59922

Files:
  llvm/include/llvm/Transforms/IPO/Attributor.h
  llvm/lib/Transforms/IPO/Attributor.cpp
  llvm/test/Transforms/FunctionAttrs/align.ll
  llvm/test/Transforms/FunctionAttrs/arg_nocapture.ll
  llvm/test/Transforms/FunctionAttrs/arg_returned.ll
  llvm/test/Transforms/FunctionAttrs/dereferenceable.ll
  llvm/test/Transforms/FunctionAttrs/internal-noalias.ll
  llvm/test/Transforms/FunctionAttrs/liveness.ll
  llvm/test/Transforms/FunctionAttrs/noalias_returned.ll
  llvm/test/Transforms/FunctionAttrs/nocapture.ll
  llvm/test/Transforms/FunctionAttrs/nonnull.ll
  llvm/test/Transforms/FunctionAttrs/nosync.ll
  llvm/test/Transforms/FunctionAttrs/read_write_returned_arguments_scc.ll

Index: llvm/test/Transforms/FunctionAttrs/read_write_returned_arguments_scc.ll
===
--- llvm/test/Transforms/FunctionAttrs/read_write_returned_arguments_scc.ll
+++ llvm/test/Transforms/FunctionAttrs/read_write_returned_arguments_scc.ll
@@ -102,7 +102,7 @@
 }
 
 ; CHECK: Function Attrs: nofree norecurse nosync nounwind
-; CHECK-NEXT: define i32* @external_sink_ret2_nrw(i32* readnone %n0, i32* nocapture readonly %r0, i32* returned %w0)
+; CHECK-NEXT: define i32* @external_sink_ret2_nrw(i32* readnone %n0, i32* nocapture readonly %r0, i32* returned "no-capture-maybe-returned" %w0)
 define i32* @external_sink_ret2_nrw(i32* %n0, i32* %r0, i32* %w0) {
 entry:
   %tobool = icmp ne i32* %n0, null
Index: llvm/test/Transforms/FunctionAttrs/nosync.ll
===
--- llvm/test/Transforms/FunctionAttrs/nosync.ll
+++ llvm/test/Transforms/FunctionAttrs/nosync.ll
@@ -28,7 +28,7 @@
 ; FNATTR: Function Attrs: norecurse nounwind optsize readnone ssp uwtable
 ; FNATTR-NEXT: define nonnull i32* @foo(%struct.ST* readnone %s)
 ; ATTRIBUTOR: Function Attrs: nofree nosync nounwind optsize readnone ssp uwtable
-; ATTRIBUTOR-NEXT: define nonnull i32* @foo(%struct.ST* %s)
+; ATTRIBUTOR-NEXT: define nonnull i32* @foo(%struct.ST* "no-capture-maybe-returned" %s)
 define i32* @foo(%struct.ST* %s) nounwind uwtable readnone optsize ssp {
 entry:
   %arrayidx = getelementptr inbounds %struct.ST, %struct.ST* %s, i64 1, i32 2, i32 1, i64 5, i64 13
@@ -186,7 +186,7 @@
 ; FNATTR: Function Attrs: nofree noinline nounwind uwtable
 ; FNATTR-NEXT: define i32 @scc1(i32* %0)
 ; ATTRIBUTOR: Function Attrs: nofree noinline noreturn nosync nounwind uwtable
-; ATTRIBUTOR-NEXT: define i32 @scc1(i32* %0)
+; ATTRIBUTOR-NEXT: define i32 @scc1(i32* nocapture %0)
 define i32 @scc1(i32* %0) noinline nounwind uwtable {
   tail call void @scc2(i32* %0);
   %val = tail call i32 @volatile_load(i32* %0);
@@ -196,7 +196,7 @@
 ; FNATTR: Function Attrs: nofree noinline nounwind uwtable
 ; FNATTR-NEXT: define void @scc2(i32* %0)
 ; ATTRIBUTOR: Function Attrs: nofree noinline noreturn nosync nounwind uwtable
-; ATTRIBUTOR-NEXT: define void @scc2(i32* %0)
+; ATTRIBUTOR-NEXT: define void @scc2(i32* nocapture %0)
 define void @scc2(i32* %0) noinline nounwind uwtable {
   tail call i32 @scc1(i32* %0);
   ret void;
@@ -224,7 +224,7 @@
 ; FNATTR: Function Attrs: nofree norecurse nounwind
 ; FNATTR-NEXT: define void @foo1(i32* nocapture %0, %"struct.std::atomic"* nocapture %1)
 ; ATTRIBUTOR-NOT: nosync
-; ATTRIBUTOR: define void @foo1(i32* %0, %"struct.std::atomic"* %1)
+; ATTRIBUTOR: define void @foo1(i32* nocapture %0, %"struct.std::atomic"* nocapture %1)
 define void @foo1(i32* %0, %"struct.std::atomic"* %1) {
   store i32 100, i32* %0, align 4
   fence release
@@ -236,7 +236,7 @@
 ; FNATTR: Function Attrs: nofree norecurse nounwind
 ; FNATTR-NEXT: define void @bar(i32* nocapture readnone %0, %"struct.std::atomic"* nocapture readonly %1)
 ; ATTRIBUTOR-NOT: nosync
-; ATTRIBUTOR: define void @bar(i32* %0, %"struct.std::atomic"* %1)
+; ATTRIBUTOR: define void @bar(i32* nocapture %0, %"struct.std::atomic"* nocapture %1)
 define void @bar(i32* %0, %"struct.std::atomic"* %1) {
   %3 = getelementptr inbounds %"struct.std::atomic", %"struct.std::atomic"* %1, i64 0, i32 0, i32 0
   br label %4
@@ -256,7 +256,7 @@
 ; FNATTR: Function Attrs: nofree norecurse nounwind
 ; FNATTR-NEXT: define void @foo1_singlethread(i32* nocapture %0, %"struct.std::atomic"* nocapture %1)
 ; ATTRIBUTOR: Function Attrs: nofree nosync
-; ATTRIBUTOR: define void @foo1_singlethread(i32* %0, %"struct.std::atomic"* %1)
+; ATTRIBUTOR: define void @foo1_singlethread(i32* nocapture %0, %"struct.std::atomic"* nocapture %1)
 define void @foo1_singlethread(i32* %0, %"struct.std::atomic"* %1) {
   store i32 100, i32* %0, align 4
   fence syncscope("singlethread") release
@@ -268,7 +268,7 @@
 ; FNATTR: Function Attrs: nofree norecurse nounwind
 ; FNATTR-NEXT: define void @bar_singlethread(i32* nocapture r

r370481 - Revert [Clang Interpreter] Initial patch for the constexpr interpreter

[Nandor Licker via cfe-commits]

Author: nand
Date: Fri Aug 30 08:41:45 2019
New Revision: 370481

URL: http://llvm.org/viewvc/llvm-project?rev=370481&view=rev
Log:
Revert [Clang Interpreter] Initial patch for the constexpr interpreter

This reverts r370476 (git commit a5590950549719d0d9ea69ed164b0c8c0f4e02e6)

Removed:
cfe/trunk/docs/ConstantInterpreter.rst
cfe/trunk/include/clang/Basic/OptionalDiagnostic.h
cfe/trunk/lib/AST/Interp/
cfe/trunk/test/AST/Interp/
cfe/trunk/utils/TableGen/ClangOpcodesEmitter.cpp
Modified:
cfe/trunk/include/clang/AST/ASTContext.h
cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/CMakeLists.txt
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp
cfe/trunk/test/SemaCXX/constexpr-many-arguments.cpp
cfe/trunk/test/SemaCXX/shift.cpp
cfe/trunk/utils/TableGen/CMakeLists.txt
cfe/trunk/utils/TableGen/TableGen.cpp
cfe/trunk/utils/TableGen/TableGenBackends.h

Removed: cfe/trunk/docs/ConstantInterpreter.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ConstantInterpreter.rst?rev=370480&view=auto
==
--- cfe/trunk/docs/ConstantInterpreter.rst (original)
+++ cfe/trunk/docs/ConstantInterpreter.rst (removed)
@@ -1,194 +0,0 @@
-
-Constant Interpreter
-
-
-.. contents::
-   :local:
-
-Introduction
-
-
-The constexpr interpreter aims to replace the existing tree evaluator in 
clang, improving performance on constructs which are executed inefficiently by 
the evaluator. The interpreter is activated using the following flags:
-
-* ``-fexperimental-new-constant-interpreter`` enables the interpreter, falling 
back to the evaluator for unsupported features
-* ``-fforce-experimental-new-constant-interpreter`` forces the use of the 
interpreter, bailing out if an unsupported feature is encountered
-
-Bytecode Compilation
-
-
-Bytecode compilation is handled in ``ByteCodeStmtGen.h`` for statements and 
``ByteCodeExprGen.h`` for expressions. The compiler has two different backends: 
one to generate bytecode for functions (``ByteCodeEmitter``) and one to 
directly evaluate expressions as they are compiled, without generating bytecode 
(``EvalEmitter``). All functions are compiled to bytecode, while toplevel 
expressions used in constant contexts are directly evaluated since the bytecode 
would never be reused. This mechanism aims to pave the way towards replacing 
the evaluator, improving its performance on functions and loops, while being 
just as fast on single-use toplevel expressions.
-
-The interpreter relies on stack-based, strongly-typed opcodes. The glue logic 
between the code generator, along with the enumeration and description of 
opcodes, can be found in ``Opcodes.td``. The opcodes are implemented as generic 
template methods in ``Interp.h`` and instantiated with the relevant primitive 
types by the interpreter loop or by the evaluating emitter.
-
-Primitive Types

-
-* ``PT_{U|S}int{8|16|32|64}``
-
-  Signed or unsigned integers of a specific bit width, implemented using the 
```Integral``` type.
-
-* ``PT_{U|S}intFP``
-
-  Signed or unsigned integers of an arbitrary, but fixed width used to 
implement
-  integral types which are required by the target, but are not supported by 
the host.
-  Under the hood, they rely on APValue. The ``Integral`` specialisation for 
these
-  types is required by opcodes to share an implementation with fixed integrals.
-
-* ``PT_Bool``
-
-  Representation for boolean types, essentially a 1-bit unsigned ``Integral``.
-
-* ``PT_RealFP``
-
-  Arbitrary, but fixed precision floating point numbers. Could be specialised 
in
-  the future similarly to integers in order to improve floating point 
performance.
-
-* ``PT_Ptr``
-
-  Pointer type, defined in ``"Pointer.h"``.
-
-* ``PT_FnPtr``
-
-  Function pointer type, can also be a null function pointer. Defined in 
``"Pointer.h"``.
-
-* ``PT_MemPtr``
-
-  Member pointer type, can also be a null member pointer. Defined in 
``"Pointer.h"``
-
-Composite types

-
-The interpreter distinguishes two kinds of composite types: arrays and 
records. Unions are represented as records, except a single field can be marked 
as active. The contents of inactive fields are kept until they
-are reactivated and overwritten.
-
-
-Bytecode Execution
-==
-
-Bytecode is executed using a stack-based interpreter. The execution context 
consists of an ``InterpStack``, along with a chain of ``InterpFrame`` objects 
storing the call frames. Frames are built by call instructions and destroyed by 
return instructions. They perform one alloc

[clang-tools-extra] r370482 - [clangd] Add highlighting for macro expansions.

[Johan Vikstrom via cfe-commits]

Author: jvikstrom
Date: Fri Aug 30 08:47:27 2019
New Revision: 370482

URL: http://llvm.org/viewvc/llvm-project?rev=370482&view=rev
Log:
[clangd] Add highlighting for macro expansions.

Summary: https://github.com/clangd/clangd/issues/134

Reviewers: hokein, ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
clang-tools-extra/trunk/clangd/SemanticHighlighting.h
clang-tools-extra/trunk/clangd/test/semantic-highlighting.test
clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp

Modified: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp?rev=370482&r1=370481&r2=370482&view=diff
==
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp (original)
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp Fri Aug 30 08:47:27 
2019
@@ -24,16 +24,20 @@ namespace {
 class HighlightingTokenCollector
 : public RecursiveASTVisitor {
   std::vector Tokens;
-  ASTContext &Ctx;
-  const SourceManager &SM;
+  ParsedAST *
 
 public:
-  HighlightingTokenCollector(ParsedAST &AST)
-  : Ctx(AST.getASTContext()), SM(AST.getSourceManager()) {}
+  HighlightingTokenCollector(ParsedAST &AST) : AST(AST) {}
 
   std::vector collectTokens() {
 Tokens.clear();
-TraverseAST(Ctx);
+TraverseAST(AST.getASTContext());
+// Add highlightings for macro expansions as they are not traversed by the
+// visitor.
+// FIXME: Should add highlighting to the macro definitions as well. But 
this
+// information is not collected in ParsedAST right now.
+for (const SourceLocation &L : AST.getMainFileExpansions())
+  addToken(L, HighlightingKind::Macro);
 // Initializer lists can give duplicates of tokens, therefore all tokens
 // must be deduplicated.
 llvm::sort(Tokens);
@@ -264,6 +268,7 @@ private:
   }
 
   void addToken(SourceLocation Loc, HighlightingKind Kind) {
+const auto &SM = AST.getSourceManager();
 if (Loc.isMacroID()) {
   // Only intereseted in highlighting arguments in macros (DEF_X(arg)).
   if (!SM.isMacroArgExpansion(Loc))
@@ -279,7 +284,7 @@ private:
 if (!isInsideMainFile(Loc, SM))
   return;
 
-auto R = getTokenRange(SM, Ctx.getLangOpts(), Loc);
+auto R = getTokenRange(SM, AST.getASTContext().getLangOpts(), Loc);
 if (!R) {
   // R should always have a value, if it doesn't something is very wrong.
   elog("Tried to add semantic token with an invalid range");
@@ -466,6 +471,8 @@ llvm::StringRef toTextMateScope(Highligh
 return "entity.name.type.template.cpp";
   case HighlightingKind::Primitive:
 return "storage.type.primitive.cpp";
+  case HighlightingKind::Macro:
+return "entity.name.function.preprocessor.cpp";
   case HighlightingKind::NumKinds:
 llvm_unreachable("must not pass NumKinds to the function");
   }

Modified: clang-tools-extra/trunk/clangd/SemanticHighlighting.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SemanticHighlighting.h?rev=370482&r1=370481&r2=370482&view=diff
==
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.h (original)
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.h Fri Aug 30 08:47:27 
2019
@@ -38,6 +38,7 @@ enum class HighlightingKind {
   Namespace,
   TemplateParameter,
   Primitive,
+  Macro,
 
   NumKinds,
 };

Modified: clang-tools-extra/trunk/clangd/test/semantic-highlighting.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/semantic-highlighting.test?rev=370482&r1=370481&r2=370482&view=diff
==
--- clang-tools-extra/trunk/clangd/test/semantic-highlighting.test (original)
+++ clang-tools-extra/trunk/clangd/test/semantic-highlighting.test Fri Aug 30 
08:47:27 2019
@@ -45,6 +45,9 @@
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
 # CHECK-NEXT:"storage.type.primitive.cpp"
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  [
+# CHECK-NEXT:"entity.name.function.preprocessor.cpp"
 # CHECK-NEXT:  ]
 # CHECK-NEXT:]
 # CHECK-NEXT:  },

Modified: clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp?rev=370482&r1=370481&r2=370482&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp 
(original)
+++ clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp Fri 
Aug 30 08:47:27 2019
@@ -47,7 +4

[PATCH] D66018: [ARM] Take into account -mcpu and -mfpu options while handling 'crypto' feature

[Diogo N. Sampaio via Phabricator via cfe-commits]

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

`clang -### -target arm-arm-none-eabit -march=armv8-m.main+crypto` did not show 
+sha2 or +aes. After the patch it does.
I believe that is not expected, as in ARM.td `crypto` is not applied for any M 
profile. And Arm®v8-M Architecture Reference Manual does not reference these 
extensions neither.




Comment at: lib/Driver/ToolChains/Arch/ARM.cpp:485
+
+  if (llvm::ARM::parseArchVersion(ArchSuffix) >= 8) {
+auto CryptoIt =

` && (llvm::ARM::parseArchProfile(Arch) == llvm::ARM::ProfileKind::A)`



Repository:
  rC Clang

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

https://reviews.llvm.org/D66018



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

[PATCH] D66995: [clangd] Add highlighting for macro expansions.

[Johan Vikström via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370482: [clangd] Add highlighting for macro expansions. 
(authored by jvikstrom, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66995?vs=218089&id=218112#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66995

Files:
  clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
  clang-tools-extra/trunk/clangd/SemanticHighlighting.h
  clang-tools-extra/trunk/clangd/test/semantic-highlighting.test
  clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
@@ -24,16 +24,20 @@
 class HighlightingTokenCollector
 : public RecursiveASTVisitor {
   std::vector Tokens;
-  ASTContext &Ctx;
-  const SourceManager &SM;
+  ParsedAST *
 
 public:
-  HighlightingTokenCollector(ParsedAST &AST)
-  : Ctx(AST.getASTContext()), SM(AST.getSourceManager()) {}
+  HighlightingTokenCollector(ParsedAST &AST) : AST(AST) {}
 
   std::vector collectTokens() {
 Tokens.clear();
-TraverseAST(Ctx);
+TraverseAST(AST.getASTContext());
+// Add highlightings for macro expansions as they are not traversed by the
+// visitor.
+// FIXME: Should add highlighting to the macro definitions as well. But this
+// information is not collected in ParsedAST right now.
+for (const SourceLocation &L : AST.getMainFileExpansions())
+  addToken(L, HighlightingKind::Macro);
 // Initializer lists can give duplicates of tokens, therefore all tokens
 // must be deduplicated.
 llvm::sort(Tokens);
@@ -264,6 +268,7 @@
   }
 
   void addToken(SourceLocation Loc, HighlightingKind Kind) {
+const auto &SM = AST.getSourceManager();
 if (Loc.isMacroID()) {
   // Only intereseted in highlighting arguments in macros (DEF_X(arg)).
   if (!SM.isMacroArgExpansion(Loc))
@@ -279,7 +284,7 @@
 if (!isInsideMainFile(Loc, SM))
   return;
 
-auto R = getTokenRange(SM, Ctx.getLangOpts(), Loc);
+auto R = getTokenRange(SM, AST.getASTContext().getLangOpts(), Loc);
 if (!R) {
   // R should always have a value, if it doesn't something is very wrong.
   elog("Tried to add semantic token with an invalid range");
@@ -466,6 +471,8 @@
 return "entity.name.type.template.cpp";
   case HighlightingKind::Primitive:
 return "storage.type.primitive.cpp";
+  case HighlightingKind::Macro:
+return "entity.name.function.preprocessor.cpp";
   case HighlightingKind::NumKinds:
 llvm_unreachable("must not pass NumKinds to the function");
   }
Index: clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
@@ -47,7 +47,8 @@
   {HighlightingKind::Method, "Method"},
   {HighlightingKind::StaticMethod, "StaticMethod"},
   {HighlightingKind::TemplateParameter, "TemplateParameter"},
-  {HighlightingKind::Primitive, "Primitive"}};
+  {HighlightingKind::Primitive, "Primitive"},
+  {HighlightingKind::Macro, "Macro"}};
   std::vector ExpectedTokens;
   for (const auto &KindString : KindToString) {
 std::vector Toks = makeHighlightingTokens(
@@ -391,9 +392,9 @@
   R"cpp(
   #define DEF_MULTIPLE(X) namespace X { class X { int X; }; }
   #define DEF_CLASS(T) class T {};
-  DEF_MULTIPLE(XYZ);
-  DEF_MULTIPLE(XYZW);
-  DEF_CLASS($Class[[A]])
+  $Macro[[DEF_MULTIPLE]](XYZ);
+  $Macro[[DEF_MULTIPLE]](XYZW);
+  $Macro[[DEF_CLASS]]($Class[[A]])
   #define MACRO_CONCAT(X, V, T) T foo##X = V
   #define DEF_VAR(X, V) int X = V
   #define DEF_VAR_T(T, X, V) T X = V
@@ -404,26 +405,27 @@
   #define SOME_NAME_SET variable2 = 123
   #define INC_VAR(X) X += 2
   $Primitive[[void]] $Function[[foo]]() {
-DEF_VAR($LocalVariable[[X]],  123);
-DEF_VAR_REV(908, $LocalVariable[[XY]]);
-$Primitive[[int]] CPY( $LocalVariable[[XX]] );
-DEF_VAR_TYPE($Class[[A]], $LocalVariable[[AA]]);
-$Primitive[[double]] SOME_NAME;
-$Primitive[[int]] SOME_NAME_SET;
+$Macro[[DEF_VAR]]($LocalVariable[[X]],  123);
+$Macro[[DEF_VAR_REV]](908, $LocalVariable[[XY]]);
+$Primitive[[int]] $Macro[[CPY]]( $LocalVariable[[XX]] );
+$Macro[[DEF_VAR_TYPE]]($Class[[A]], $LocalVariable[[AA]]);
+$Primitive[[double]] $Macro[[SOME_NAME]];
+$Primitive[[int]] $Macro[[SOME_NAME_SET]];
 $LocalVariable[[variable]] = 20.1;
-MACRO_CO

[PATCH] D66834: Driver tests: set `--sysroot=""` to support clang with `DEFAULT_SYSROOT`

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

I think that this is pretty easy to forget.  Fortunately, last argument wins.  
Why not sink this into the `%clang` substitution in lit?  That ensures that we 
run with an empty sysroot and then when the test needs to adjust the sysroot, 
it can do so explicitly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66834



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

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

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

xbolva00 updated this revision to Diff 218115.
xbolva00 added a comment.

Fixed review comment.


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

https://reviews.llvm.org/D66397

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/SemaCXX/warn-xor-as-pow.cpp

Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -,7 +,7 @@
   "result of '%0' is %1; did you mean '%2'?">,
   InGroup;
 def note_xor_used_as_pow_silence : Note<
-  "replace expression with '%0' to silence this warning">;
+  "replace expression with '%0' %select{|or use 'xor' instead of '^' }1to silence this warning">;
 
 def warn_null_pointer_compare : Warning<
 "comparison of %select{address of|function|array}0 '%1' %select{not |}2"
Index: test/SemaCXX/warn-xor-as-pow.cpp
===
--- test/SemaCXX/warn-xor-as-pow.cpp
+++ test/SemaCXX/warn-xor-as-pow.cpp
@@ -10,8 +10,10 @@
 #define XOR(x, y) (x ^ y)
 #define TWO 2
 #define TEN 10
+#define IOP 64
 #define TWO_ULL 2ULL
 #define EPSILON 10 ^ -300
+#define ALPHA_OFFSET 3
 
 #define flexor 7
 
@@ -21,7 +23,7 @@
 constexpr long long operator"" _0b(unsigned long long v) { return v; }
 constexpr long long operator"" _0X(unsigned long long v) { return v; }
 #else
-#define xor^ // iso646.h
+#define xor ^ // iso646.h
 #endif
 
 void test(unsigned a, unsigned b) {
@@ -32,25 +34,25 @@
   res = 2 ^ -1;
   res = 2 ^ 0; // expected-warning {{result of '2 ^ 0' is 2; did you mean '1 << 0' (1)?}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1"
-   // expected-note@-2 {{replace expression with '0x2 ^ 0' to silence this warning}}
+   // expected-note@-2 {{replace expression with '0x2 ^ 0' or use 'xor' instead of '^' to silence this warning}}
   res = 2 ^ 1; // expected-warning {{result of '2 ^ 1' is 3; did you mean '1 << 1' (2)?}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1 << 1"
-   // expected-note@-2 {{replace expression with '0x2 ^ 1' to silence this warning}}
+   // expected-note@-2 {{replace expression with '0x2 ^ 1' or use 'xor' instead of '^' to silence this warning}}
   res = 2 ^ 2; // expected-warning {{result of '2 ^ 2' is 0; did you mean '1 << 2' (4)?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1 << 2"
-  // expected-note@-2 {{replace expression with '0x2 ^ 2' to silence this warning}}
+  // expected-note@-2 {{replace expression with '0x2 ^ 2' or use 'xor' instead of '^' to silence this warning}}
   res = 2 ^ 8; // expected-warning {{result of '2 ^ 8' is 10; did you mean '1 << 8' (256)?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1 << 8"
-  // expected-note@-2 {{replace expression with '0x2 ^ 8' to silence this warning}}
-  res = TWO ^ 8; // expected-warning {{result of 'TWO ^ 8' is 10; did you mean '1 << 8' (256)?}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1 << 8"
-  // expected-note@-2 {{replace expression with '0x2 ^ 8' to silence this warning}}
+  // expected-note@-2 {{replace expression with '0x2 ^ 8' or use 'xor' instead of '^' to silence this warning}}
+  res = 2 ^ +8; // expected-warning {{result of '2 ^ +8' is 10; did you mean '1 << +8' (256)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1 << +8"
+  // expected-note@-2 {{replace expression with '0x2 ^ +8' or use 'xor' instead of '^' to silence this warning}}
+  res = TWO ^ 8;
   res = 2 ^ 16; // expected-warning {{result of '2 ^ 16' is 18; did you mean '1 << 16' (65536)?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1 << 16"
-  // expected-note@-2 {{replace expression with '0x2 ^ 16' to silence this warning}}
-  res = 2 ^ TEN; // expected-warning {{result of '2 ^ TEN' is 8; did you mean '1 << TEN' (1024)?}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1 << TEN"
-  // expected-note@-2 {{replace expression with '0x2 ^ TEN' to silence this warning}}
+  // expected-note@-2 {{replace expression with '0x2 ^ 16' or use 'xor' instead of '^' to silence this warning}}
+  res = 2 ^ TEN;
+  res = res + (2 ^ ALPHA_OFFSET);
   res = 0x2 ^ 16;
   res = 2 xor 16;
 
@@ -69,31 +71,50 @@
   res = TWO_ULL ^ 16;
   res = 2 ^ 32; // expected-warning {{result of '2 ^ 32' is 34; did you mean '1LL << 32'?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1LL << 32"
-  // expected-note@-2 {{replace expression with '0x2 ^ 32' to silence this warning}}
-  res = 2 ^ 64;
+  // expected-note@-2 {{replace expression with '0x2 ^ 32' or use 'xor' instead of '^' to silence this warning}}
+  res = (2 ^ 64) - 1; // expected-warning {{result of '2 ^ 64' is 66; did you mean '-1LL'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:21}:"-1LL"
+  // expected-note@-2 {{rep

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

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

xbolva00 added a comment.

>> tbh, I would appreciate if you would leave the definition of 
>> diagnoseXorMisusedAsPow() where it is and add a forward declare of the 
>> function earlier in the file.

I uploaded the patch almost 2 weeks ago and nobody complained so I thought it 
is okay. Uploaded, fixed.


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

https://reviews.llvm.org/D66397



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

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

[Alexey Bataev via Phabricator via cfe-commits]

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

LG, thanks for the fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66559



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

[PATCH] D66989: FileManager: Remove ShouldCloseOpenFile argument from getBufferForFile, NFC

[Michael Spencer via Phabricator via cfe-commits]

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

lgtm


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

https://reviews.llvm.org/D66989



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

r370488 - FileManager: Remove ShouldCloseOpenFile argument from getBufferForFile, NFC

[Duncan P. N. Exon Smith via cfe-commits]

Author: dexonsmith
Date: Fri Aug 30 09:56:26 2019
New Revision: 370488

URL: http://llvm.org/viewvc/llvm-project?rev=370488&view=rev
Log:
FileManager: Remove ShouldCloseOpenFile argument from getBufferForFile, NFC

Remove this dead code.  We always close it.

Modified:
cfe/trunk/include/clang/Basic/FileManager.h
cfe/trunk/lib/Basic/FileManager.cpp
cfe/trunk/lib/Serialization/ModuleManager.cpp

Modified: cfe/trunk/include/clang/Basic/FileManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileManager.h?rev=370488&r1=370487&r2=370488&view=diff
==
--- cfe/trunk/include/clang/Basic/FileManager.h (original)
+++ cfe/trunk/include/clang/Basic/FileManager.h Fri Aug 30 09:56:26 2019
@@ -317,8 +317,7 @@ public:
   /// Open the specified file as a MemoryBuffer, returning a new
   /// MemoryBuffer if successful, otherwise returning null.
   llvm::ErrorOr>
-  getBufferForFile(const FileEntry *Entry, bool isVolatile = false,
-   bool ShouldCloseOpenFile = true);
+  getBufferForFile(const FileEntry *Entry, bool isVolatile = false);
   llvm::ErrorOr>
   getBufferForFile(StringRef Filename, bool isVolatile = false) {
 return getBufferForFileImpl(Filename, /*FileSize=*/-1, isVolatile);

Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=370488&r1=370487&r2=370488&view=diff
==
--- cfe/trunk/lib/Basic/FileManager.cpp (original)
+++ cfe/trunk/lib/Basic/FileManager.cpp Fri Aug 30 09:56:26 2019
@@ -426,8 +426,7 @@ void FileManager::fillRealPathName(FileE
 }
 
 llvm::ErrorOr>
-FileManager::getBufferForFile(const FileEntry *Entry, bool isVolatile,
-  bool ShouldCloseOpenFile) {
+FileManager::getBufferForFile(const FileEntry *Entry, bool isVolatile) {
   uint64_t FileSize = Entry->getSize();
   // If there's a high enough chance that the file have changed since we
   // got its size, force a stat before opening it.
@@ -440,10 +439,7 @@ FileManager::getBufferForFile(const File
 auto Result =
 Entry->File->getBuffer(Filename, FileSize,
/*RequiresNullTerminator=*/true, isVolatile);
-// FIXME: we need a set of APIs that can make guarantees about whether a
-// FileEntry is open or not.
-if (ShouldCloseOpenFile)
-  Entry->closeFile();
+Entry->closeFile();
 return Result;
   }
 

Modified: cfe/trunk/lib/Serialization/ModuleManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ModuleManager.cpp?rev=370488&r1=370487&r2=370488&view=diff
==
--- cfe/trunk/lib/Serialization/ModuleManager.cpp (original)
+++ cfe/trunk/lib/Serialization/ModuleManager.cpp Fri Aug 30 09:56:26 2019
@@ -185,9 +185,7 @@ ModuleManager::addModule(StringRef FileN
   Buf = llvm::MemoryBuffer::getSTDIN();
 } else {
   // Get a buffer of the file and close the file descriptor when done.
-  Buf = FileMgr.getBufferForFile(NewModule->File,
- /*isVolatile=*/false,
- /*ShouldClose=*/true);
+  Buf = FileMgr.getBufferForFile(NewModule->File, /*isVolatile=*/false);
 }
 
 if (!Buf) {


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

[PATCH] D66989: FileManager: Remove ShouldCloseOpenFile argument from getBufferForFile, NFC

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith closed this revision.
dexonsmith added a comment.

r370488


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

https://reviews.llvm.org/D66989



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

[PATCH] D66706: [Wdocumentation] fixes an assertion failure with typedefed function and block pointer

[Mark de Wever via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/Comment.cpp:151
+static bool getFunctionTypeLoc(TypeLoc TL, FunctionTypeLoc &ResFTL,
+   bool testTypedefTypeLoc = false) {
   TypeLoc PrevTL;

gribozavr wrote:
> Mordante wrote:
> > gribozavr wrote:
> > > Why is the new functionality turned off sometimes? It seems to me that we 
> > > always want to look through typedefs.
> > Setting `testTypedefTypeLoc` to `true` breaks a unit test in 
> > `test/Sema/warn-documentation.cpp:358`
> > 
> > ```
> > typedef int (*test_not_function_like_typedef1)(int aaa);
> > 
> > // expected-warning@+1 {{'\param' command used in a comment that is not 
> > attached to a function declaration}}
> > /// \param aaa Meow.
> > 
> > typedef test_not_function_like_typedef1 test_not_function_like_typedef2;
> > ```
> > and its corresponding test using a `using` instead of `typedef`. This has 
> > been introduced in:
> > ```
> > commit 49fdf8d3f5e114e6f8b49fde29a30b0eaaa3c5dd
> > Author: Dmitri Gribenko 
> > Date:   Sat Sep 15 21:13:36 2012 +
> > 
> > Comment parsing: don't treat typedef to a typedef to a function as a
> > 'function-like' type that can be annotated with \param.
> > 
> > Thanks to Eli Friedman for noticing!
> > 
> > llvm-svn: 163985
> > 
> > ```
> > I'm not sure whether or not we should allow this typedef documentation. I 
> > just tested with Doxygen. It doesn't complain and shows the parameter 
> > documentation for `test_not_function_like_typedef2`. So on that basis we 
> > could remove this `expected-warning` and `testTypedefTypeLoc`.
> > 
> > What do you think?
> Thanks for the explanation. I can't find the context for that decision, and 
> the commit description does not explain the reason either.
> 
> It is really a policy decision -- when introducing a comment for function 
> return value and arguments, should the declaration include the said return 
> value and arguments, or can they be visible through a typedef?
> 
> Thinking about it, my inclination is to say that comments for the return 
> value and arguments should be present on the declaration that introduces 
> them. Otherwise, it is breaking an abstraction barrier.
> 
> WDYT?
(I just noticed I forgot to submit this comment.)

I tend to agree. However Doxygen accepts the code, but Doxygen doesn't seem to 
validate whether parameters exist.
So I wonder whether we do our users a service by warning about code that 
Doxygen properly handles. On that basis I think we can allow it.

We could add an extra warning in the pedantic group and let the user decide 
whether or not she thinks it warrants a warning.

I think that would be the best solution: accept the comment but allow the user 
to opt-in on a warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66706



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

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

[Chi Chun Chen via Phabricator via cfe-commits]

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

I'm not able to `arc land` my patch since I updated the patch for a minor 
refactor and the state is "not approve" now. Do I wait for the approval for the 
new patch so that I can `arc land` or I don't need to do `arc land` myself?
Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66559



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

[PATCH] D66710: ASTReader: Bypass overridden files when reading PCHs

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno accepted this revision.
bruno added a comment.

LGTM!


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

https://reviews.llvm.org/D66710



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

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:5423
+   SemaRef.Diag(DefaultLoc, diag::err_omp_loop_not_canonical_cond)
+   << IneqCondIsCanonical << LCDecl;
 return true;

I would not suggest to rely on the conversion of bool values to integer, better 
to use `IneqCondIsCanonical ? 0 : 1`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66559



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

r370493 - [clang-scan-deps] NFC, remove outdated implementation comment

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Fri Aug 30 10:34:22 2019
New Revision: 370493

URL: http://llvm.org/viewvc/llvm-project?rev=370493&view=rev
Log:
[clang-scan-deps] NFC, remove outdated implementation comment

There's no need to purge symlinked entries in the FileManager,
as the new FileEntryRef API allows us to compute dependencies more
accurately when the FileManager is reused.

Modified:
cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

Modified: cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp?rev=370493&r1=370492&r2=370493&view=diff
==
--- cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp 
(original)
+++ cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp Fri 
Aug 30 10:34:22 2019
@@ -87,7 +87,6 @@ public:
 
 // Use the dependency scanning optimized file system if we can.
 if (DepFS) {
-  // FIXME: Purge the symlink entries from the stat cache in the FM.
   const CompilerInvocation &CI = Compiler.getInvocation();
   // Add any filenames that were explicity passed in the build settings and
   // that might be opened, as we want to ensure we don't run source


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

[PATCH] D67010: [Modules] Move search paths from control block to unhashed control block

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno created this revision.
bruno added reviewers: dexonsmith, arphaman, rsmith.
Herald added subscribers: cfe-commits, ributzka, jkorous.
Herald added a project: clang.

Header search paths are currently ignored for the purpose of computing
`getModuleHash()` during a module build. This means that it doesn't
matter how much one changes header search paths in different clang
invocations, it will always use the same PCM.

This is not really correct but has been mostly "working" ever since.
However, when clang started signing the CONTROL_BLOCK section of the
PCM, which is where the header search paths live in the PCMs, we started
to get unexpected rebuilds when using implicit modules and `signature
mismatch` errors when those are rebuilt in the context of PCHs.

It's inconsistent that we ignore header search paths when selecting a
PCM to build/reuse but consider it as part of the signature of the PCM.
This patch proposes to move serialization of header search paths to
the UNHASHED_CONTROL_BLOCK instead, as we currently do for diagnostics.

Disconsidering the header search paths as part of the signature
shouldn't make it worse for correctness because if the header search
path would change anything, that would change the INPUT_FILE, which
*still is* part of the CONTROL_BLOCK, and therefore will trigger a
module rebuild.

Follow ups from this that might be interesting:

- Introduce -fmodules-strict-header-search, which does consider search paths in 
`getModuleHash()`. This would provide users a way out when they hit bugs 
related to header search paths, but with the downside of bigger module caches 
(in case those paths change often in their builds).
- Optimize the serialization of such paths to only consider the ones that 
actually contribute to the INPUT_LIST. This has the nice effect that a module 
only knows about the paths that are actually relevant to it, increase 
reusability.
- Add warnings or remarks when the header search paths in the PCM don't match 
the ones in the compiler.

There are no testcases for this change, as it would require using
llvm-bcanalyzer in clang tests, which we don't currently do.

rdar://problem/45567811


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67010

Files:
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1327,6 +1327,7 @@
   BLOCK(UNHASHED_CONTROL_BLOCK);
   RECORD(SIGNATURE);
   RECORD(DIAGNOSTIC_OPTIONS);
+  RECORD(HEADER_SEARCH_PATHS);
   RECORD(DIAG_PRAGMA_MAPPINGS);
 
 #undef RECORD
@@ -1446,6 +1447,29 @@
   // are generally transient files and will almost always be overridden.
   Stream.EmitRecord(DIAGNOSTIC_OPTIONS, Record);
 
+  // Header search paths.
+  Record.clear();
+  const HeaderSearchOptions &HSOpts
+= PP.getHeaderSearchInfo().getHeaderSearchOpts();
+
+  // Include entries.
+  Record.push_back(HSOpts.UserEntries.size());
+  for (unsigned I = 0, N = HSOpts.UserEntries.size(); I != N; ++I) {
+const HeaderSearchOptions::Entry &Entry = HSOpts.UserEntries[I];
+AddString(Entry.Path, Record);
+Record.push_back(static_cast(Entry.Group));
+Record.push_back(Entry.IsFramework);
+Record.push_back(Entry.IgnoreSysRoot);
+  }
+
+  // System header prefixes.
+  Record.push_back(HSOpts.SystemHeaderPrefixes.size());
+  for (unsigned I = 0, N = HSOpts.SystemHeaderPrefixes.size(); I != N; ++I) {
+AddString(HSOpts.SystemHeaderPrefixes[I].Prefix, Record);
+Record.push_back(HSOpts.SystemHeaderPrefixes[I].IsSystemHeader);
+  }
+  Stream.EmitRecord(HEADER_SEARCH_PATHS, Record);
+
   // Write out the diagnostic/pragma mappings.
   WritePragmaDiagnosticMappings(Diags, /* isModule = */ WritingModule);
 
@@ -1649,25 +1673,8 @@
   Record.clear();
   const HeaderSearchOptions &HSOpts
 = PP.getHeaderSearchInfo().getHeaderSearchOpts();
-  AddString(HSOpts.Sysroot, Record);
-
-  // Include entries.
-  Record.push_back(HSOpts.UserEntries.size());
-  for (unsigned I = 0, N = HSOpts.UserEntries.size(); I != N; ++I) {
-const HeaderSearchOptions::Entry &Entry = HSOpts.UserEntries[I];
-AddString(Entry.Path, Record);
-Record.push_back(static_cast(Entry.Group));
-Record.push_back(Entry.IsFramework);
-Record.push_back(Entry.IgnoreSysRoot);
-  }
-
-  // System header prefixes.
-  Record.push_back(HSOpts.SystemHeaderPrefixes.size());
-  for (unsigned I = 0, N = HSOpts.SystemHeaderPrefixes.size(); I != N; ++I) {
-AddString(HSOpts.SystemHeaderPrefixes[I].Prefix, Record);
-Record.push_back(HSOpts.SystemHeaderPrefixes[I].IsSystemHeader);
-  }
 
+  AddString(HSOpts.Sysroot, Record);
   AddString(HSOpts.ResourceDir, Record);
   AddString(HSOpts.ModuleCachePath, Record);
   AddString(HSOpts.ModuleUs

[PATCH] D66808: [ConstExprPreter] Full patch of the interpreter.

[Nandor Licker via Phabricator via cfe-commits]

nand updated this revision to Diff 218131.
nand added a comment.

Added more features


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66808

Files:
  clang/docs/ConstantInterpreter.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/OptionalDiagnostic.h
  clang/include/clang/Driver/Options.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/CMakeLists.txt
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/Interp/Block.cpp
  clang/lib/AST/Interp/Block.h
  clang/lib/AST/Interp/Builtin.cpp
  clang/lib/AST/Interp/Builtin.h
  clang/lib/AST/Interp/ByteCodeEmitter.cpp
  clang/lib/AST/Interp/ByteCodeEmitter.h
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/ByteCodeGenError.cpp
  clang/lib/AST/Interp/ByteCodeGenError.h
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/ByteCodeStmtGen.h
  clang/lib/AST/Interp/CMakeLists.txt
  clang/lib/AST/Interp/Context.cpp
  clang/lib/AST/Interp/Context.h
  clang/lib/AST/Interp/Descriptor.cpp
  clang/lib/AST/Interp/Descriptor.h
  clang/lib/AST/Interp/Disasm.cpp
  clang/lib/AST/Interp/EvalEmitter.cpp
  clang/lib/AST/Interp/EvalEmitter.h
  clang/lib/AST/Interp/Frame.cpp
  clang/lib/AST/Interp/Frame.h
  clang/lib/AST/Interp/Function.cpp
  clang/lib/AST/Interp/Function.h
  clang/lib/AST/Interp/HeapUtils.cpp
  clang/lib/AST/Interp/HeapUtils.h
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/Interp.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpFrame.cpp
  clang/lib/AST/Interp/InterpFrame.h
  clang/lib/AST/Interp/InterpStack.cpp
  clang/lib/AST/Interp/InterpStack.h
  clang/lib/AST/Interp/InterpState.cpp
  clang/lib/AST/Interp/InterpState.h
  clang/lib/AST/Interp/Opcode.h
  clang/lib/AST/Interp/Opcodes.td
  clang/lib/AST/Interp/Pointer.cpp
  clang/lib/AST/Interp/Pointer.h
  clang/lib/AST/Interp/Program.cpp
  clang/lib/AST/Interp/Program.h
  clang/lib/AST/Interp/Real.h
  clang/lib/AST/Interp/Record.cpp
  clang/lib/AST/Interp/Record.h
  clang/lib/AST/Interp/Source.cpp
  clang/lib/AST/Interp/Source.h
  clang/lib/AST/Interp/State.cpp
  clang/lib/AST/Interp/State.h
  clang/lib/AST/Interp/Type.cpp
  clang/lib/AST/Interp/Type.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/AST/Interp/alignof.cpp
  clang/test/AST/Interp/arg_struct.cpp
  clang/test/AST/Interp/array.cpp
  clang/test/AST/Interp/array_in_array.cpp
  clang/test/AST/Interp/array_loop_init.cpp
  clang/test/AST/Interp/bitfield.cpp
  clang/test/AST/Interp/boolean.cpp
  clang/test/AST/Interp/bound_member_ptr.cpp
  clang/test/AST/Interp/call.cpp
  clang/test/AST/Interp/case_range.cpp
  clang/test/AST/Interp/cast.cpp
  clang/test/AST/Interp/compound_assign.cpp
  clang/test/AST/Interp/cond.cpp
  clang/test/AST/Interp/default_cons.cpp
  clang/test/AST/Interp/designated_init.cpp
  clang/test/AST/Interp/double_to_int.cpp
  clang/test/AST/Interp/dummy.cpp
  clang/test/AST/Interp/enum.cpp
  clang/test/AST/Interp/extern.cpp
  clang/test/AST/Interp/float.cpp
  clang/test/AST/Interp/flow.cpp
  clang/test/AST/Interp/for_range.cpp
  clang/test/AST/Interp/funcptr.cpp
  clang/test/AST/Interp/global.cpp
  clang/test/AST/Interp/inheritance.cpp
  clang/test/AST/Interp/init.cpp
  clang/test/AST/Interp/init_list_int.cpp
  clang/test/AST/Interp/init_list_ptr.cpp
  clang/test/AST/Interp/int128.cpp
  clang/test/AST/Interp/int64.cpp
  clang/test/AST/Interp/lifetime.cpp
  clang/test/AST/Interp/locals.cpp
  clang/test/AST/Interp/logical.cpp
  clang/test/AST/Interp/loop.cpp
  clang/test/AST/Interp/member_fn.cpp
  clang/test/AST/Interp/member_fn_virtual.cpp
  clang/test/AST/Interp/member_ptr.cpp
  clang/test/AST/Interp/member_ptr_ambiguity.cpp
  clang/test/AST/Interp/member_ptr_derived.cpp
  clang/test/AST/Interp/member_ptr_explicit_path.cpp
  clang/test/AST/Interp/member_ptr_missing.cpp
  clang/test/AST/Interp/member_ptr_print.cpp
  clang/test/AST/Interp/member_ptr_wrong_path.cpp
  clang/test/AST/Interp/method.cpp
  clang/test/AST/Interp/null_method.cpp
  clang/test/AST/Interp/nullptr.cpp
  clang/test/AST/Interp/nullptr_struct.cpp
  clang/test/AST/Interp/operator.cpp
  clang/test/AST/Interp/overflow.cpp
  clang/test/AST/Interp/past_end.cpp
  clang/test/AST/Interp/ptr.cpp
  clang/test/AST/Interp/ptr_derived_cast.cpp
  clang/test/AST/Interp/ptrdiff.cpp
  clang/test/AST/Interp/ptrdiff_overflow.cpp
  clang/test/AST/Interp/pure_virt_ind.cpp
  clang/test/AST/Interp/read_only.cpp
  clang/test/AST/Interp/ref_field.cpp
  clang/test/AST/Interp/relocation.cpp
  clang/test/AST/Interp/return.cpp
  clang/test/AST/Interp/rvo.cpp
  clang/test/AST/Interp/scoping.cpp
  clang/test/AST/Interp/selfref.cpp
  clang/test/AST/Interp/set_field.cpp
  clang/test/AST/Interp/shift.cpp
  clang/test/AST/Interp/sizeof.cpp
  clang/test/AST/Interp/stack_trace.

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Paul Kirth via Phabricator via cfe-commits]

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

Thanks for the review.

I'm working on splitting up the patch now.

I should be able to address most of your comments, but I left some inline 
comments for clarification/discussion.




Comment at: clang/lib/CodeGen/CodeGenAction.cpp:626
+  unsigned Line, Column;
+  bool BadDebugInfo = false;
+  FullSourceLoc Loc =

lebedev.ri wrote:
> This should be `BadDebugInfoLoc` (in `getBestLocationFromDebugLoc()` too)
That should be in a separate patch, right?



Comment at: llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp:78-87
+  SI.setMetadata(
+  LLVMContext::MD_misexpect,
+  MDBuilder(CI->getContext())
+  .createMisExpect(Index, LikelyBranchWeight, UnlikelyBranchWeight));
+
+  SI.setCondition(ArgValue);
+  misexpect::checkClangInstrumentation(SI);

lebedev.ri wrote:
> Why can't `LLVMContext::MD_prof` be reused?
It didn't seem appropriate to me, but that can be changed if it is the right 
thing to do. 

However, I don't see code elsewhere that loops over `MD_prof` metadata checking 
its tag. I see lots of examples that check if the tag matches, and then exits 
if it doesn't match. 

If I added "misexpect" as a new `MD_prof`tag, then code in places like 
`extractProfTotalWeight(...)` in IR/Metadata.cpp would have to be updated. I'm 
not sure how pervasive that pattern is, but I did  want to avoid introducing 
systemic changes like that.

https://github.com/llvm/llvm-project/blob/9976a5bc1db5a240378a61a68c0e182060bcb554/llvm/lib/IR/Metadata.cpp#L1336



Comment at: llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp:82
 
-  SI.setCondition(ArgValue);
   return true;

lebedev.ri wrote:
> Why do you need to move this line?
I did this so that the condition is available to check when performing 
validation, and the metadata won't be replaced by the likely/unlikely values 
until after I've finished the validation. Having the condition available in 
when performing the check provides for better diagnostics output.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66324



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

[PATCH] D66505: Make add_new_check.py's insertion of registerCheck<> match the sort order

[Daniel Sanders via Phabricator via cfe-commits]

dsanders marked 4 inline comments as done.
dsanders added a comment.

In D66505#1652420 , @alexfh wrote:

> Mostly LG, if you've verified that this works. A couple of comments below.


I verified it using

  ./add_new_check.py llvm prefer-register-over-unsigned
  ./add_new_check.py llvm zzz
  ./add_new_check.py llvm nb
  ./add_new_check.py llvm iz
  ./add_new_check.py llvm a

which is enough to cover every insertion point in the LLVM module




Comment at: clang-tools-extra/clang-tidy/add_new_check.py:192
+  else:
+match = re.search('registerCheck<(.*)> *\( *(?:"([^"]*)")?', line)
+last_line = None

alexfh wrote:
> Are there actually any registerCheck calls with spaces between the `>` and 
> the `(`? Should we clang-format these instead of supporting them in the 
> script?
> Are there actually any registerCheck calls with spaces between the > and the 
> (?

No, it was just trivial to be robust against it in case it does somehow happen. 
Likewise for the whitespace after the '(' but before the '"' or newline.

> Should we clang-format these instead of supporting them in the script?

I think the script should stick to inserting the new code rather than 
correcting existing mistakes that somehow crept in. I do agree that people 
should generally clang-format their patches before commit though which will 
prevent it creeping in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66505



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

[PATCH] D66505: Make add_new_check.py's insertion of registerCheck<> match the sort order

[Daniel Sanders via Phabricator via cfe-commits]

dsanders updated this revision to Diff 218137.
dsanders marked an inline comment as done.
dsanders added a comment.

- Full stop at end of comment
- last_line -> prev_line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66505

Files:
  clang-tools-extra/clang-tidy/add_new_check.py


Index: clang-tools-extra/clang-tidy/add_new_check.py
===
--- clang-tools-extra/clang-tidy/add_new_check.py
+++ clang-tools-extra/clang-tidy/add_new_check.py
@@ -165,31 +165,50 @@
 header_added = False
 header_found = False
 check_added = False
+check_fq_name = module + '-' + check_name
 check_decl = ('CheckFactories.registerCheck<' + check_name_camel +
-  '>(\n"' + module + '-' + check_name + '");\n')
-
-for line in lines:
-  if not header_added:
-match = re.search('#include "(.*)"', line)
-if match:
-  header_found = True
-  if match.group(1) > check_name_camel:
+  '>(\n"' + check_fq_name + '");\n')
+
+lines = iter(lines)
+try:
+  while True:
+line = lines.next()
+if not header_added:
+  match = re.search('#include "(.*)"', line)
+  if match:
+header_found = True
+if match.group(1) > check_name_camel:
+  header_added = True
+  f.write('#include "' + check_name_camel + '.h"\n')
+  elif header_found:
 header_added = True
 f.write('#include "' + check_name_camel + '.h"\n')
-elif header_found:
-  header_added = True
-  f.write('#include "' + check_name_camel + '.h"\n')
-
-  if not check_added:
-if line.strip() == '}':
-  check_added = True
-  f.write(check_decl)
-else:
-  match = re.search('registerCheck<(.*)>', line)
-  if match and match.group(1) > check_name_camel:
+
+if not check_added:
+  if line.strip() == '}':
 check_added = True
 f.write(check_decl)
-  f.write(line)
+  else:
+match = re.search('registerCheck<(.*)> *\( *(?:"([^"]*)")?', line)
+prev_line = None
+if match:
+  current_check_name = match.group(2)
+  if current_check_name is None:
+# If we didn't find the check name on this line, look on the
+# next one.
+prev_line = line
+line = lines.next()
+match = re.search(' *"([^"]*)"', line)
+if match:
+  current_check_name = match.group(1)
+  if current_check_name > check_fq_name:
+check_added = True
+f.write(check_decl)
+  if prev_line:
+f.write(prev_line)
+f.write(line)
+except StopIteration:
+  pass
 
 
 # Adds a release notes entry.


Index: clang-tools-extra/clang-tidy/add_new_check.py
===
--- clang-tools-extra/clang-tidy/add_new_check.py
+++ clang-tools-extra/clang-tidy/add_new_check.py
@@ -165,31 +165,50 @@
 header_added = False
 header_found = False
 check_added = False
+check_fq_name = module + '-' + check_name
 check_decl = ('CheckFactories.registerCheck<' + check_name_camel +
-  '>(\n"' + module + '-' + check_name + '");\n')
-
-for line in lines:
-  if not header_added:
-match = re.search('#include "(.*)"', line)
-if match:
-  header_found = True
-  if match.group(1) > check_name_camel:
+  '>(\n"' + check_fq_name + '");\n')
+
+lines = iter(lines)
+try:
+  while True:
+line = lines.next()
+if not header_added:
+  match = re.search('#include "(.*)"', line)
+  if match:
+header_found = True
+if match.group(1) > check_name_camel:
+  header_added = True
+  f.write('#include "' + check_name_camel + '.h"\n')
+  elif header_found:
 header_added = True
 f.write('#include "' + check_name_camel + '.h"\n')
-elif header_found:
-  header_added = True
-  f.write('#include "' + check_name_camel + '.h"\n')
-
-  if not check_added:
-if line.strip() == '}':
-  check_added = True
-  f.write(check_decl)
-else:
-  match = re.search('registerCheck<(.*)>', line)
-  if match and match.group(1) > check_name_camel:
+
+if not check_added:
+  if line.strip() == '}':
 check_added = True
 f.write(check_decl)
-  f.write(line)
+  else:
+match = re.search('registerCheck<(.*)> *\( *(?:"([^"]*)")?', line)
+prev_line = None
+if match

[PATCH] D66505: Make add_new_check.py's insertion of registerCheck<> match the sort order

[Daniel Sanders via Phabricator via cfe-commits]

dsanders added a comment.

Missed one of the commands from my history that I used to verify it:

  ./add_new_check.py llvm i 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66505



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

[PATCH] D66018: [ARM] Take into account -mcpu and -mfpu options while handling 'crypto' feature

[Kristina Bessonova via Phabricator via cfe-commits]

krisb added a comment.

I see. But this doesn't seem like a valid case, does it? Shouldn't we somehow 
diagnose it to not to silently ignore user-specified options?


Repository:
  rC Clang

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

https://reviews.llvm.org/D66018



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

[PATCH] D66706: [Wdocumentation] fixes an assertion failure with typedefed function and block pointer

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang/lib/AST/Comment.cpp:151
+static bool getFunctionTypeLoc(TypeLoc TL, FunctionTypeLoc &ResFTL,
+   bool testTypedefTypeLoc = false) {
   TypeLoc PrevTL;

Mordante wrote:
> gribozavr wrote:
> > Mordante wrote:
> > > gribozavr wrote:
> > > > Why is the new functionality turned off sometimes? It seems to me that 
> > > > we always want to look through typedefs.
> > > Setting `testTypedefTypeLoc` to `true` breaks a unit test in 
> > > `test/Sema/warn-documentation.cpp:358`
> > > 
> > > ```
> > > typedef int (*test_not_function_like_typedef1)(int aaa);
> > > 
> > > // expected-warning@+1 {{'\param' command used in a comment that is not 
> > > attached to a function declaration}}
> > > /// \param aaa Meow.  
> > >   
> > > typedef test_not_function_like_typedef1 test_not_function_like_typedef2;
> > > ```
> > > and its corresponding test using a `using` instead of `typedef`. This has 
> > > been introduced in:
> > > ```
> > > commit 49fdf8d3f5e114e6f8b49fde29a30b0eaaa3c5dd
> > > Author: Dmitri Gribenko 
> > > Date:   Sat Sep 15 21:13:36 2012 +
> > > 
> > > Comment parsing: don't treat typedef to a typedef to a function as a
> > > 'function-like' type that can be annotated with \param.
> > > 
> > > Thanks to Eli Friedman for noticing!
> > > 
> > > llvm-svn: 163985
> > > 
> > > ```
> > > I'm not sure whether or not we should allow this typedef documentation. I 
> > > just tested with Doxygen. It doesn't complain and shows the parameter 
> > > documentation for `test_not_function_like_typedef2`. So on that basis we 
> > > could remove this `expected-warning` and `testTypedefTypeLoc`.
> > > 
> > > What do you think?
> > Thanks for the explanation. I can't find the context for that decision, and 
> > the commit description does not explain the reason either.
> > 
> > It is really a policy decision -- when introducing a comment for function 
> > return value and arguments, should the declaration include the said return 
> > value and arguments, or can they be visible through a typedef?
> > 
> > Thinking about it, my inclination is to say that comments for the return 
> > value and arguments should be present on the declaration that introduces 
> > them. Otherwise, it is breaking an abstraction barrier.
> > 
> > WDYT?
> (I just noticed I forgot to submit this comment.)
> 
> I tend to agree. However Doxygen accepts the code, but Doxygen doesn't seem 
> to validate whether parameters exist.
> So I wonder whether we do our users a service by warning about code that 
> Doxygen properly handles. On that basis I think we can allow it.
> 
> We could add an extra warning in the pedantic group and let the user decide 
> whether or not she thinks it warrants a warning.
> 
> I think that would be the best solution: accept the comment but allow the 
> user to opt-in on a warning.
Doxygen and C++ both allow a lot of dubious stuff. That can't be a reason to 
avoid producing a warning -- we would have no warnings if that was the bar.

I think what we should discuss is, what is the use case for adding a `\returns` 
command in this position -- and how is documenting parameters not a use case. 
If there is a valid use case, how common it is in real world.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66706



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

[PATCH] D66505: Make add_new_check.py's insertion of registerCheck<> match the sort order

[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, thank you for working on this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66505



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

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

[Alexey Bataev via Phabricator via cfe-commits]

ABataev accepted this revision.
ABataev added a comment.

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66559



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

[PATCH] D66836: [libc++] Add `__truncating_cast` for safely casting float types to integers

[Eric Fiselier via Phabricator via cfe-commits]

EricWF updated this revision to Diff 218149.
EricWF marked an inline comment as done.
EricWF added a comment.

Address review comments.

- Document that NaN isn't a supported input.
- Fix spelling mistake.


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

https://reviews.llvm.org/D66836

Files:
  include/math.h
  test/libcxx/numerics/clamp_to_integral.pass.cpp

Index: test/libcxx/numerics/clamp_to_integral.pass.cpp
===
--- /dev/null
+++ test/libcxx/numerics/clamp_to_integral.pass.cpp
@@ -0,0 +1,90 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+// __clamp_to_integral(RealT)
+
+// Test the conversion function that truncates floating point types to the
+// closest representable value for the specified integer type, or
+// numeric_limits::max()/min() if the value isn't representable.
+
+#include 
+#include 
+#include 
+
+template 
+void test() {
+  typedef std::numeric_limits Lim;
+  const bool MaxIsRepresentable = sizeof(IntT) < 8;
+  const bool IsSigned = std::is_signed::value;
+  struct TestCase {
+double Input;
+IntT Expect;
+bool IsRepresentable;
+  } TestCases[] = {
+  {0, 0, true},
+  {1, 1, true},
+  {IsSigned ? static_cast(-1) : 0,
+   IsSigned ? static_cast(-1) : 0, true},
+  {Lim::lowest(), Lim::lowest(), true},
+  {static_cast(Lim::max()), Lim::max(), MaxIsRepresentable},
+  {static_cast(Lim::max()) + 1, Lim::max(), false},
+  {static_cast(Lim::max()) + 1024, Lim::max(), false},
+  {nextafter(static_cast(Lim::max()), INFINITY), Lim::max(), false},
+  };
+  for (TestCase TC : TestCases) {
+auto res = std::__clamp_to_integral(TC.Input);
+assert(res == TC.Expect);
+if (TC.IsRepresentable) {
+  auto other = static_cast(std::trunc(TC.Input));
+  assert(res == other);
+} else
+  assert(res == Lim::min() || res == Lim::max());
+  }
+}
+
+template 
+void test_float() {
+  typedef std::numeric_limits Lim;
+  const bool MaxIsRepresentable = sizeof(IntT) < 4;
+  ((void)MaxIsRepresentable);
+  const bool IsSigned = std::is_signed::value;
+  struct TestCase {
+float Input;
+IntT Expect;
+bool IsRepresentable;
+  } TestCases[] = {
+  {0, 0, true},
+  {1, 1, true},
+  {IsSigned ? static_cast(-1) : 0,
+   IsSigned ? static_cast(-1) : 0, true},
+  {Lim::lowest(), Lim::lowest(), true},
+  {static_cast(Lim::max()), Lim::max(), MaxIsRepresentable },
+   {nextafter(static_cast(Lim::max()), INFINITY), Lim::max(), false},
+  };
+  for (TestCase TC : TestCases) {
+auto res = std::__clamp_to_integral(TC.Input);
+assert(res == TC.Expect);
+if (TC.IsRepresentable) {
+  auto other = static_cast(std::trunc(TC.Input));
+  assert(res == other);
+} else
+  assert(res == Lim::min() || res == Lim::max());
+  }
+}
+
+int main() {
+  test();
+  test();
+  test();
+  test();
+  test();
+  test();
+  test_float();
+  test_float();
+  test_float();
+}
Index: include/math.h
===
--- include/math.h
+++ include/math.h
@@ -1553,6 +1553,40 @@
 typename std::enable_if::value, double>::type
 trunc(_A1 __lcpp_x) _NOEXCEPT {return ::trunc((double)__lcpp_x);}
 
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+template ::digits > numeric_limits<_IntT>::digits),
+int _Bits = (numeric_limits<_IntT>::digits - numeric_limits<_FloatT>::digits)>
+_LIBCPP_INLINE_VISIBILITY
+_LIBCPP_CONSTEXPR _IntT __max_representable_int_for_float() _NOEXCEPT {
+  static_assert(is_floating_point<_FloatT>::value, "must be a floating point type");
+  static_assert(is_integral<_IntT>::value, "must be an integral type");
+  static_assert(numeric_limits<_FloatT>::radix == 2, "FloatT has incorrect radix");
+  static_assert(_IsSame<_FloatT, float>::value || _IsSame<_FloatT, double>::value
+   || _IsSame<_FloatT,long double>::value, "unsupported floating point type");
+  return _FloatBigger ? numeric_limits<_IntT>::max() :  (numeric_limits<_IntT>::max() >> _Bits << _Bits);
+}
+
+// Convert a floating point number to the specified integral type after
+// clamping to the integral types representable range.
+//
+// The behavior is undefined if `__r` is NaN.
+template 
+_LIBCPP_INLINE_VISIBILITY
+_IntT __clamp_to_integral(_RealT __r) _NOEXCEPT {
+  using _Lim = std::numeric_limits<_IntT>;
+  const _IntT _MaxVal = std::__max_representable_int_for_float<_IntT, _RealT>();
+  if (__r >= ::nextafter(static_cast<_RealT>(_MaxVal), INFINITY)) {
+return _Lim::max();
+  } else if (__r <= _Lim::lowest()) {
+return _Lim::min();
+  }
+  return static_cast<_IntT>(__r);
+}
+
+_L

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:54-55
+const SourceLocation Loc,
+const Expr *SubLHS = nullptr,
+const Expr *SubRHS = nullptr);
+

I'd appreciate some comments on what these arguments should be when non-null.



Comment at: lib/Sema/SemaExpr.cpp:11031-11032
   // Do not diagnose macros.
-  if (Loc.isMacroID())
+  if (Loc.isMacroID() || XorLHS.get()->getBeginLoc().isMacroID() ||
+  XorRHS.get()->getBeginLoc().isMacroID())
 return;

I would appreciate it if this patch didn't also change the behavior for macros. 
That seems like a larger discussion that can happen in a separate patch.



Comment at: lib/Sema/SemaExpr.cpp:11052
+  Negative = (Opc == UO_Minus);
+  ExplicitPlus = (Opc == UO_Plus);
 } else {

`!Negative` (we already verified above that it's either the + or - operator, so 
if it's not one, it's the other.)?



Comment at: lib/Sema/SemaExpr.cpp:11067
+
+  // Do not diagnose 2 ^ 64, but allow special case (2 ^ 64) - 1.
+  if (SubLHS && SubRHS && (LeftSideValue != 2 || RightSideValue != 64))

This comment explains what the code does, but not why it does it. Given that 
we're adding special cases, I think more comments here explaining why this is 
valuable would be appreciated.



Comment at: lib/Sema/SemaExpr.cpp:11073
   LHSInt->getBeginLoc(), S.getLocForEndOfToken(RHSInt->getLocation()));
-  llvm::StringRef ExprStr =
+  std::string ExprStr =
   Lexer::getSourceText(ExprRange, S.getSourceManager(), S.getLangOpts());

Why is this type changing?



Comment at: lib/Sema/SemaExpr.cpp:11078
   CharSourceRange::getCharRange(Loc, S.getLocForEndOfToken(Loc));
-  llvm::StringRef XorStr =
+  std::string XorStr =
   Lexer::getSourceText(XorRange, S.getSourceManager(), S.getLangOpts());

Same question here.



Comment at: lib/Sema/SemaExpr.cpp:11130
+  ? "ULLONG_MAX"
+  : "-1LL";
+  ExprRange = CharSourceRange::getCharRange(

The two branches are not equivalent. What about `~0ULL` instead?


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

https://reviews.llvm.org/D66397



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

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D66397#1652771 , @xbolva00 wrote:

> >> tbh, I would appreciate if you would leave the definition of 
> >> diagnoseXorMisusedAsPow() where it is and add a forward declare of the 
> >> function earlier in the file.
>
> I uploaded the patch almost 2 weeks ago and nobody complained so I thought it 
> is okay. Uploaded, fixed.


I appreciate it -- this made it much easier to spot the actual changes in the 
code.


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

https://reviews.llvm.org/D66397



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

[PATCH] D66919: Warn about zero-parameter K&R definitions in -Wstrict-prototypes

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

> In D66919#1650174 , @dexonsmith 
> wrote:
> 
>> We could carve out a `-W` flag (if it doesn't already exist) that warns if 
>> you incorrectly pass parameters to a function whose definition has no 
>> prototype
> 
> 
> The warning exists, but there is no flag apparently.
> 
>   void f() {}
>   void g() {
>   f(0);
>   }
> 
> 
> spits out
> 
>   test.c:3:8: warning: too many arguments in call to 'f'
>   f(0);
>   ~  ^

Great; then I think we should add a flag.  I would suggest `-Werror`-by-default 
since the compiler knows it's incorrect code.

In D66919#1651473 , @aaronpuchert 
wrote:

> "Meaning" is a difficult term. What we know is that this is a K&R-style 
> definition, and does not define a prototype. So one would expect 
> `-Wstrict-prototypes` to warn about this.


All I'm suggesting is that the compiler knows that the call is wrong, so it 
should error (by-default).

After adding that, my feeling is that diagnosing a missing prototype on a 
defining declaration would be a style nitpick that might fit better in 
clang-tidy.  IIRC, when we rolled out `-Wstrict-prototypes` we explicitly 
excluded this case since it hit a lot of code without finding bugs.

But at the same time, I don't want to block this.  If you do pursue this please 
use a distinct `-W` flag so it can be turned off without losing the benefits of 
`-Wstrict-prototypes`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66919



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

[PATCH] D65744: [PR42707][OpenCL] Fix addr space deduction for auto

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D65744#1652355 , @Anastasia wrote:

> I don't think this is likely to change. Are you suggesting to move the 
> deduction logic for pointee of pointers, references and block pointers into 
> ASTContext helper that creates a pointer/reference/block pointer type?


No.  I'm suggesting that the deduction logic should be much more 
straightforward, just some sort of "is the type non-dependent and lacking a 
qualifier", and it should be applied in the two basic places we build these 
types in Sema, i.e. in the type-from-declarator logic and in the 
`Build{Pointer,Reference}Type` logic.  Instead we have something very elaborate 
that apparently recursively looks through pointer types and is contingent on 
the exact spelling, e.g. trying to find `auto` types, which seems both brittle 
and unnecessary.




Comment at: include/clang/AST/Type.h:6509
+  return isa(CanonicalType);
+}
+

Hmm.  So this method, confusingly, will not return true for a deduced `auto`, 
unless the deduced type is itself an undeduced `auto` (which I'm not sure can 
happen).  I think it at least needs a different name; `isUndeducedAutoType()` 
would be okay if the latter case is not possible.  But it might be better if we 
can just define away the need for the method entirely.



Comment at: lib/Sema/SemaType.cpp:7441
+  // the initializing expression type during the type deduction.
+  (T->isAutoType() && IsPointee) || (IsAutoPointee) ||
   // OpenCL spec v2.0 s6.9.b:

Anastasia wrote:
> mantognini wrote:
> > mantognini wrote:
> > > Shouldn't the parentheses around `IsAutoPointee` be removed for style 
> > > consistency?
> > With the `if` statement introduced above, `IsAutoPointee` can be true only 
> > in C++ mode. Could it be an issue to not guard `(T->isAutoType() && 
> > IsPointee)` for non-C++ mode? (I guess not, but I couldn't convince myself.)
> I think `TreeTransforms` will only be used in C++ mode. But also `isAutoType` 
> should only be true in C++ mode. So I think we should be fine.
I don't think `TreeTransform` is expected to be C++-only, but I agree that 
`isAutoType` is good enough.


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

https://reviews.llvm.org/D65744



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

[PATCH] D64480: [ASTImporter] Added visibility context check for TypedefNameDecl.

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: lib/AST/ASTImporter.cpp:949
+return Importer.GetFromTU(Found) == From->getTranslationUnitDecl();
+  return From->isInAnonymousNamespace() == Found->isInAnonymousNamespace();
+}

I am not sure what case this covers? Can you elaborate? I see the case the 
condition above is catching.



Comment at: unittests/AST/ASTImporterVisibilityTest.cpp:348
+::testing::Values(
+std::make_tuple(ExternTypedef, ExternTypedef, ExpectLink),
+std::make_tuple(ExternTypedef, AnonTypedef, ExpectNotLink),

Perhaps `ExpectLink` would be better as `ExpectLinkedDeclChain` and the same 
for `ExpectNotLink`.

It was actually confusing at first because link is an overload term.



Comment at: unittests/AST/ASTImporterVisibilityTest.cpp:352
+std::make_tuple(AnonTypedef, AnonTypedef, ExpectNotLink))), );
+INSTANTIATE_TEST_CASE_P(
+ParameterizedTests, ImportTypeAliasVisibility,

We need another set of tests for typedefs and using e.g. `ExternTypedef` and 
`ExternUsing` since they are equivalent. 


Repository:
  rC Clang

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

https://reviews.llvm.org/D64480



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

[clang-tools-extra] r370512 - [clang-tidy] Add llvm-prefer-register-over-unsigned to clang-tidy

[Daniel Sanders via cfe-commits]

Author: dsanders
Date: Fri Aug 30 13:01:59 2019
New Revision: 370512

URL: http://llvm.org/viewvc/llvm-project?rev=370512&view=rev
Log:
[clang-tidy] Add llvm-prefer-register-over-unsigned to clang-tidy

Summary:
This clang-tidy check is looking for unsigned integer variables whose 
initializer
starts with an implicit cast from llvm::Register and changes the type of the
variable to llvm::Register (dropping the llvm:: where possible).

Reviewers: arsenm, bogner

Subscribers: jholewinski, MatzeB, qcolombet, dschuff, jyknight, dylanmckay, 
sdardis, nemanjai, jvesely, wdng, nhaehnle, mgorny, sbc100, jgravelle-google, 
kristof.beyls, hiraditya, aheejin, kbarton, fedor.sergeev, javed.absar, asb, 
rbar, johnrusso, simoncook, apazos, sabuasal, niosHD, jrtc27, MaskRay, zzheng, 
edward-jones, atanasyan, rogfer01, MartinMosbeck, brucehoult, the_o, tpr, PkmX, 
jocewei, jsji, Petar.Avramovic, asbirlea, Jim, s.egerton, cfe-commits, 
llvm-commits

Tags: #clang, #llvm

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

Added:
clang-tools-extra/trunk/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp
clang-tools-extra/trunk/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/llvm-prefer-register-over-unsigned.rst

clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-register-over-unsigned.cpp

clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-register-over-unsigned2.cpp

clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-register-over-unsigned3.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/llvm/LLVMTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt?rev=370512&r1=370511&r2=370512&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt Fri Aug 30 13:01:59 
2019
@@ -5,6 +5,7 @@ add_clang_library(clangTidyLLVMModule
   IncludeOrderCheck.cpp
   LLVMTidyModule.cpp
   PreferIsaOrDynCastInConditionalsCheck.cpp
+  PreferRegisterOverUnsignedCheck.cpp
   TwineLocalCheck.cpp
 
   LINK_LIBS

Modified: clang-tools-extra/trunk/clang-tidy/llvm/LLVMTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/llvm/LLVMTidyModule.cpp?rev=370512&r1=370511&r2=370512&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/llvm/LLVMTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/llvm/LLVMTidyModule.cpp Fri Aug 30 
13:01:59 2019
@@ -13,6 +13,7 @@
 #include "HeaderGuardCheck.h"
 #include "IncludeOrderCheck.h"
 #include "PreferIsaOrDynCastInConditionalsCheck.h"
+#include "PreferRegisterOverUnsignedCheck.h"
 #include "TwineLocalCheck.h"
 
 namespace clang {
@@ -28,6 +29,8 @@ public:
 "llvm-namespace-comment");
 CheckFactories.registerCheck(
 "llvm-prefer-isa-or-dyn-cast-in-conditionals");
+CheckFactories.registerCheck(
+"llvm-prefer-register-over-unsigned");
 CheckFactories.registerCheck("llvm-twine-local");
   }
 };

Added: 
clang-tools-extra/trunk/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp?rev=370512&view=auto
==
--- clang-tools-extra/trunk/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp 
(added)
+++ clang-tools-extra/trunk/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp 
Fri Aug 30 13:01:59 2019
@@ -0,0 +1,64 @@
+//===--- PreferRegisterOverUnsignedCheck.cpp - clang-tidy 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "PreferRegisterOverUnsignedCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace llvm_check {
+
+void PreferRegisterOverUnsignedCheck::registerMatchers(MatchFinder *Finder) {
+  auto RegisterClassMatch = hasType(
+  cxxRecordDecl(hasName("::llvm::Register")).bind("registerClassDecl"));
+
+  Finder->addMatcher(
+  valueDecl(allOf(
+  hasType(qualType(isUnsignedInteger()).bind("varType")),
+  varDecl(hasInitializer(exprWithCleanups(has(implicitCastExpr(has(
+  cxxMemberCallExpr(allOf(on(RegisterClas

[PATCH] D65919: [clang-tidy] Add llvm-prefer-register-over-unsigned check and apply it to LLVM

[Daniel Sanders via Phabricator via cfe-commits]

dsanders added a comment.

r370512


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65919



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

[PATCH] D65919: [clang-tidy] Add llvm-prefer-register-over-unsigned check and apply it to LLVM

[Daniel Sanders via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
dsanders marked an inline comment as done.
Closed by commit rL370512: [clang-tidy] Add llvm-prefer-register-over-unsigned 
to clang-tidy (authored by dsanders, committed by ).
Herald added a project: LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D65919?vs=218015&id=218156#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65919

Files:
  clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/trunk/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp
  clang-tools-extra/trunk/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.h
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/llvm-prefer-register-over-unsigned.rst
  clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-register-over-unsigned.cpp
  
clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-register-over-unsigned2.cpp
  
clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-register-over-unsigned3.cpp

Index: clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-register-over-unsigned3.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-register-over-unsigned3.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-register-over-unsigned3.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy %s llvm-prefer-register-over-unsigned %t
+
+namespace llvm { };
+
+// This class shouldn't trigger it despite the similarity as it's not inside the llvm namespace
+class Register {
+public:
+  operator unsigned();
+};
+
+Register getReg();
+
+void do_nothing_1() {
+  unsigned Reg1 = getReg();
+  // CHECK-FIXES: do_nothing_1()
+  // CHECK-FIXES-NEXT: unsigned Reg1 = getReg();
+}
+
+void do_nothing_2() {
+  using namespace llvm;
+  unsigned Reg2 = getReg();
+  // CHECK-FIXES: do_nothing_2()
+  // CHECK-FIXES-NEXT: using namespace llvm;
+  // CHECK-FIXES-NEXT: unsigned Reg2 = getReg();
+}
+
+namespace llvm {
+void do_nothing_3() {
+  unsigned Reg3 = getReg();
+  // CHECK-FIXES: do_nothing_3()
+  // CHECK-FIXES-NEXT: unsigned Reg3 = getReg();
+}
+} // end namespace llvm
Index: clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-register-over-unsigned.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-register-over-unsigned.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-register-over-unsigned.cpp
@@ -0,0 +1,143 @@
+// RUN: %check_clang_tidy %s llvm-prefer-register-over-unsigned %t
+
+namespace llvm {
+class Register {
+public:
+  operator unsigned();
+
+  unsigned Reg;
+};
+
+// This class shouldn't trigger it despite the similarity.
+class RegisterLike {
+public:
+  operator unsigned();
+
+  unsigned Reg;
+};
+} // end namespace llvm
+
+llvm::Register getReg();
+llvm::RegisterLike getRegLike();
+
+void apply_1() {
+  unsigned Reg1 = getReg();
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'Reg1' declared as 'unsigned int'; use 'llvm::Register' instead [llvm-prefer-register-over-unsigned]
+  // CHECK-FIXES: apply_1()
+  // CHECK-FIXES-NEXT: llvm::Register Reg1 = getReg();
+}
+
+void apply_2() {
+  using namespace llvm;
+  unsigned Reg2 = getReg();
+  // FIXME: Function-scoped UsingDirectiveDecl's don't appear to be in the
+  //DeclContext for the function so we can't elide the llvm:: in this
+  //case. Fortunately, it doesn't actually occur in the LLVM code base.
+  // CHECK-MESSAGES: :[[@LINE-4]]:12: warning: variable 'Reg2' declared as 'unsigned int'; use 'llvm::Register' instead [llvm-prefer-register-over-unsigned]
+  // CHECK-FIXES: apply_2()
+  // CHECK-FIXES-NEXT: using namespace llvm;
+  // CHECK-FIXES-NEXT: llvm::Register Reg2 = getReg();
+}
+
+namespace llvm {
+void apply_3() {
+  unsigned Reg3 = getReg();
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'Reg3' declared as 'unsigned int'; use 'Register' instead [llvm-prefer-register-over-unsigned]
+  // CHECK-FIXES: apply_3()
+  // CHECK-FIXES-NEXT: Register Reg3 = getReg();
+}
+} // end namespace llvm
+
+void done_1() {
+  llvm::Register Reg1 = getReg();
+  // CHECK-FIXES: done_1()
+  // CHECK-FIXES-NEXT: llvm::Register Reg1 = getReg();
+}
+
+void done_2() {
+  using namespace llvm;
+  Register Reg2 = getReg();
+  // CHECK-FIXES: done_2()
+  // CHECK-FIXES-NEXT: using namespace llvm;
+  // CHECK-FIXES-NEXT: Register Reg2 = getReg();
+}
+
+namespace llvm {
+void done_3() {
+  Register Reg3 = getReg();
+  // CHECK-FIXES: done_3()
+  // CHECK-FIXES-NEXT: Register Reg3 = getReg();
+}
+} // end namespace llvm
+
+void do_nothing_1() {
+  unsigned Reg1 = getRegLike();
+  // CHECK-FIXES: do_nothing_1()
+  // CHECK-FIXES-NEXT: unsigned Reg1 = getRegLike();
+}
+
+void do_nothing_2() {
+  using 

[PATCH] D67019: [analyzer] pr43179: CallDescription: Check number of parameters as well as number of arguments.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, 
baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, szepet.
Herald added a project: clang.
NoQ retitled this revision from "[analyzer] CallDescription: Check number of 
parameters as well as number of arguments." to "[analyzer] pr43179: 
CallDescription: Check number of parameters as well as number of arguments.".
NoQ edited the summary of this revision.

This is https://bugs.llvm.org/show_bug.cgi?id=43179.

Most functions that our checkers react upon are not C-style variadic functions, 
and therefore they have as many actual arguments as they have formal parameters.

However, it's not impossible to define a variadic function with the same name. 
This will crash any checker that relies on `CallDescription` to check the 
number of arguments but then silently assumes that the number of parameters is 
the same.

Change `CallDescription` to check both the number of arguments and the number 
of parameters by default.

If we're intentionally trying to match variadic functions, allow specifying 
arguments and parameters separately (possibly omitting any of them). For now we 
only have one `CallDescription` which would make use of those, namely 
`__builtin_va_start` itself.


Repository:
  rC Clang

https://reviews.llvm.org/D67019

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/test/Analysis/cast-value-weird.cpp


Index: clang/test/Analysis/cast-value-weird.cpp
===
--- /dev/null
+++ clang/test/Analysis/cast-value-weird.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,apiModeling -verify %s
+
+// expected-no-diagnostics
+
+namespace llvm {
+template 
+void cast(...);
+void a() { cast(int()); } // no-crash
+} // namespace llvm
Index: clang/lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -368,7 +368,8 @@
 
   if (CD.Flags & CDF_MaybeBuiltin) {
 return CheckerContext::isCLibraryFunction(FD, CD.getFunctionName()) &&
-   (!CD.RequiredArgs || CD.RequiredArgs <= getNumArgs());
+   (!CD.RequiredArgs || CD.RequiredArgs <= getNumArgs()) &&
+   (!CD.RequiredParams || CD.RequiredParams <= parameters().size());
   }
 
   if (!CD.IsLookupDone) {
@@ -407,7 +408,8 @@
   return false;
   }
 
-  return (!CD.RequiredArgs || CD.RequiredArgs == getNumArgs());
+  return (!CD.RequiredArgs || CD.RequiredArgs == getNumArgs()) &&
+ (!CD.RequiredParams || CD.RequiredParams == parameters().size());
 }
 
 SVal CallEvent::getArgSVal(unsigned Index) const {
Index: clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
@@ -116,7 +116,9 @@
 // vswprintf is the wide version of vsnprintf,
 // vsprintf has no wide version
 {{"vswscanf", 3}, 2}};
-const CallDescription ValistChecker::VaStart("__builtin_va_start", 2),
+
+const CallDescription
+ValistChecker::VaStart("__builtin_va_start", /*Args=*/2, /*Params=*/1),
 ValistChecker::VaCopy("__builtin_va_copy", 2),
 ValistChecker::VaEnd("__builtin_va_end", 1);
 } // end anonymous namespace
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -1064,8 +1064,19 @@
   // e.g. "{a, b}" represent the qualified names, like "a::b".
   std::vector QualifiedName;
   Optional RequiredArgs;
+  Optional RequiredParams;
   int Flags;
 
+  // A constructor helper.
+  static Optional readRequiredParams(Optional RequiredArgs,
+ Optional RequiredParams) {
+if (RequiredParams)
+  return RequiredParams;
+if (RequiredArgs)
+  return static_cast(*RequiredArgs);
+return None;
+  }
+
 public:
   /// Constructs a CallDescription object.
   ///
@@ -1078,14 +1089,17 @@
   /// call. Omit this parameter to match every occurrence of call with a given
   /// name regardless the number of arguments.
   CallDescription(int Flags, ArrayRef QualifiedName,
-  Optional RequiredArgs = None)
+  Optional RequiredArgs = None,
+  Optional RequiredParams = None)
   : QualifiedName(QualifiedName), RequiredArgs(RequiredArgs),
+RequiredParams(readRequiredParams(RequiredArgs, RequiredParams)),
 Flags(Flags) {}

[clang-tools-extra] r370527 - Make add_new_check.py's insertion of registerCheck<> match the sort order

[Daniel Sanders via cfe-commits]

Author: dsanders
Date: Fri Aug 30 13:47:02 2019
New Revision: 370527

URL: http://llvm.org/viewvc/llvm-project?rev=370527&view=rev
Log:
Make add_new_check.py's insertion of registerCheck<> match the sort order

Summary:
Following on from review comments in D65919 about the ordering
of the registerCheck<> calls. Sort based on the check name which might
be on the line after the registerCheck<>

Reviewers: aaron.ballman

Subscribers: cfe-commits, llvm-commits

Tags: #clang

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

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

Modified: clang-tools-extra/trunk/clang-tidy/add_new_check.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/add_new_check.py?rev=370527&r1=370526&r2=370527&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/add_new_check.py (original)
+++ clang-tools-extra/trunk/clang-tidy/add_new_check.py Fri Aug 30 13:47:02 2019
@@ -165,31 +165,50 @@ def adapt_module(module_path, module, ch
 header_added = False
 header_found = False
 check_added = False
+check_fq_name = module + '-' + check_name
 check_decl = ('CheckFactories.registerCheck<' + check_name_camel +
-  '>(\n"' + module + '-' + check_name + '");\n')
+  '>(\n"' + check_fq_name + '");\n')
 
-for line in lines:
-  if not header_added:
-match = re.search('#include "(.*)"', line)
-if match:
-  header_found = True
-  if match.group(1) > check_name_camel:
+lines = iter(lines)
+try:
+  while True:
+line = lines.next()
+if not header_added:
+  match = re.search('#include "(.*)"', line)
+  if match:
+header_found = True
+if match.group(1) > check_name_camel:
+  header_added = True
+  f.write('#include "' + check_name_camel + '.h"\n')
+  elif header_found:
 header_added = True
 f.write('#include "' + check_name_camel + '.h"\n')
-elif header_found:
-  header_added = True
-  f.write('#include "' + check_name_camel + '.h"\n')
 
-  if not check_added:
-if line.strip() == '}':
-  check_added = True
-  f.write(check_decl)
-else:
-  match = re.search('registerCheck<(.*)>', line)
-  if match and match.group(1) > check_name_camel:
+if not check_added:
+  if line.strip() == '}':
 check_added = True
 f.write(check_decl)
-  f.write(line)
+  else:
+match = re.search('registerCheck<(.*)> *\( *(?:"([^"]*)")?', line)
+prev_line = None
+if match:
+  current_check_name = match.group(2)
+  if current_check_name is None:
+# If we didn't find the check name on this line, look on the
+# next one.
+prev_line = line
+line = lines.next()
+match = re.search(' *"([^"]*)"', line)
+if match:
+  current_check_name = match.group(1)
+  if current_check_name > check_fq_name:
+check_added = True
+f.write(check_decl)
+  if prev_line:
+f.write(prev_line)
+f.write(line)
+except StopIteration:
+  pass
 
 
 # Adds a release notes entry.


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

[PATCH] D67019: [analyzer] pr43179: CallDescription: Check number of parameters as well as number of arguments.

[Csaba Dabis via Phabricator via cfe-commits]

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

I like the isolation of this kind of stuff. Thanks you!


Repository:
  rC Clang

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

https://reviews.llvm.org/D67019



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

[PATCH] D66505: Make add_new_check.py's insertion of registerCheck<> match the sort order

[Daniel Sanders via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370527: Make add_new_check.py's insertion of 
registerCheck<> match the sort order (authored by dsanders, committed by 
).
Herald added a project: LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D66505?vs=218137&id=218163#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66505

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


Index: clang-tools-extra/trunk/clang-tidy/add_new_check.py
===
--- clang-tools-extra/trunk/clang-tidy/add_new_check.py
+++ clang-tools-extra/trunk/clang-tidy/add_new_check.py
@@ -165,31 +165,50 @@
 header_added = False
 header_found = False
 check_added = False
+check_fq_name = module + '-' + check_name
 check_decl = ('CheckFactories.registerCheck<' + check_name_camel +
-  '>(\n"' + module + '-' + check_name + '");\n')
+  '>(\n"' + check_fq_name + '");\n')
 
-for line in lines:
-  if not header_added:
-match = re.search('#include "(.*)"', line)
-if match:
-  header_found = True
-  if match.group(1) > check_name_camel:
+lines = iter(lines)
+try:
+  while True:
+line = lines.next()
+if not header_added:
+  match = re.search('#include "(.*)"', line)
+  if match:
+header_found = True
+if match.group(1) > check_name_camel:
+  header_added = True
+  f.write('#include "' + check_name_camel + '.h"\n')
+  elif header_found:
 header_added = True
 f.write('#include "' + check_name_camel + '.h"\n')
-elif header_found:
-  header_added = True
-  f.write('#include "' + check_name_camel + '.h"\n')
-
-  if not check_added:
-if line.strip() == '}':
-  check_added = True
-  f.write(check_decl)
-else:
-  match = re.search('registerCheck<(.*)>', line)
-  if match and match.group(1) > check_name_camel:
+
+if not check_added:
+  if line.strip() == '}':
 check_added = True
 f.write(check_decl)
-  f.write(line)
+  else:
+match = re.search('registerCheck<(.*)> *\( *(?:"([^"]*)")?', line)
+prev_line = None
+if match:
+  current_check_name = match.group(2)
+  if current_check_name is None:
+# If we didn't find the check name on this line, look on the
+# next one.
+prev_line = line
+line = lines.next()
+match = re.search(' *"([^"]*)"', line)
+if match:
+  current_check_name = match.group(1)
+  if current_check_name > check_fq_name:
+check_added = True
+f.write(check_decl)
+  if prev_line:
+f.write(prev_line)
+f.write(line)
+except StopIteration:
+  pass
 
 
 # Adds a release notes entry.


Index: clang-tools-extra/trunk/clang-tidy/add_new_check.py
===
--- clang-tools-extra/trunk/clang-tidy/add_new_check.py
+++ clang-tools-extra/trunk/clang-tidy/add_new_check.py
@@ -165,31 +165,50 @@
 header_added = False
 header_found = False
 check_added = False
+check_fq_name = module + '-' + check_name
 check_decl = ('CheckFactories.registerCheck<' + check_name_camel +
-  '>(\n"' + module + '-' + check_name + '");\n')
+  '>(\n"' + check_fq_name + '");\n')
 
-for line in lines:
-  if not header_added:
-match = re.search('#include "(.*)"', line)
-if match:
-  header_found = True
-  if match.group(1) > check_name_camel:
+lines = iter(lines)
+try:
+  while True:
+line = lines.next()
+if not header_added:
+  match = re.search('#include "(.*)"', line)
+  if match:
+header_found = True
+if match.group(1) > check_name_camel:
+  header_added = True
+  f.write('#include "' + check_name_camel + '.h"\n')
+  elif header_found:
 header_added = True
 f.write('#include "' + check_name_camel + '.h"\n')
-elif header_found:
-  header_added = True
-  f.write('#include "' + check_name_camel + '.h"\n')
-
-  if not check_added:
-if line.strip() == '}':
-  check_added = True
-  f.write(check_decl)
-else:
-  match = re.search('registerCheck<(.*)>', line)
-  if match and match.group(1) > check_name_camel:
+
+if not check_added:
+  if line.strip() == '}':
 check_added = True
 f.write(ch

[PATCH] D65917: [clang-tidy] Added check for the Google style guide's category method naming rule.

[David Gatwood via Phabricator via cfe-commits]

dgatwood added a comment.

Whoops.  I was expecting to get emailed about review comments and never got 
anything, so I thought it was just not getting reviewed.  I'll work on this 
again.  Sorry about that.  :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65917



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

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 218164.
paulkirth added a comment.

Address Review items

- minimize debug info in llvm tests
- sanitize paths in debug info
- use more complete checks in test  for LowerExpectIntrisic
- remove references to __builtin_expect from backend code
  - update test code for new diagnostic messages
- rename checkClangInstrumentation to checkFrontendInstrumentation
- revert whitespace removal in CodeGenFunction.cpp
- use more idomatic check when adding diagnostic to the backend


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66324

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
  clang/test/Profile/Inputs/misexpect-switch-default.proftext
  clang/test/Profile/Inputs/misexpect-switch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-nonconst-expected-val.c
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/FixedMetadataKinds.def
  llvm/include/llvm/IR/MDBuilder.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/ThinLTO/X86/lazyload_metadata.ll
  llvm/test/Transforms/LowerExpectIntrinsic/basic.ll
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-stripped.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-switch.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/misexpect-switch.ll
@@ -0,0 +1,293 @@
+
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch-correct.proftext -o %t.c.profdata
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; New PM
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -S 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.c.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=CORRECT
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.c.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=CORRECT
+
+; WARNING-DAG: warning: misexpect-switch.c:26:5: 0.00%
+; WARNING-NOT: remark: misexpect-switch.c:26:5: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% of profiled executions.
+
+; REMARK-NOT: warning: misexpect-switch.c:26:5

[PATCH] D66919: Warn about zero-parameter K&R definitions in -Wstrict-prototypes

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

We had a discussion on IRC yesterday and @aaron.ballman pointed out that the 
K&R syntax has been considered "obsolescent" (= deprecated) since C89, 30 years 
ago. He added that there are thoughts within the standards committee about 
removing the syntax entirely from C2x.

In D66919#1653078 , @dexonsmith wrote:

> I would suggest `-Werror`-by-default since the compiler knows it's incorrect 
> code.


Do we have any precedence for a warning that's treated as error by default? I 
haven't seen that before, that's why I'm asking. Even `-Wreturn-type` isn't 
treated as error by default.

> All I'm suggesting is that the compiler knows that the call is wrong, so it 
> should error (by-default).

K&R allows weird stuff, so I'm really not sure it's necessarily wrong. To my 
knowledge the `...` varargs syntax was only introduced with prototypes, and the 
K&R version of printf was "implicitly vararg". Meaning that the additional 
parameters weren't mentioned in the function header at all, and scraped from 
the stack using the `va_*` macros as it's done today.

Also by that argument we shouldn't emit `-Wstrict-prototypes` at all on 
definitions, because clearly the compiler can also do type checking then. In 
fact, we actually do that to some extent: instead of default-promoting the call 
arguments, we default-promote the parameter types and build a 
`FunctionProtoType`, so that calls work as if there had been a proper 
prototype. This has the odd side effect of causing different behavior depending 
on whether the compiler sees the definition of a function, as @aaron.ballman 
noted. The example we looked at was

  int f();
  int g() { return f(1.0); }
  int f(x) int x; {}
  int h() { return f(1.0); }

One might think that we emit the same code for `g` and `h`, but we don't!

> After adding that, my feeling is that diagnosing a missing prototype on a 
> defining declaration would be a style nitpick that might fit better in 
> clang-tidy.

It's not a stylistic issue, prototype and non-prototype declarations are 
semantically very different. C has **no notion of argument checking for 
non-prototypes** at all. The `identifier-list` of a K&R definition is 
essentially private.

Of course we can impose our own semantics for non-prototypes, but we can also 
just nudge people into using standard syntax that has been available for many 
decades that guarantees them proper argument checks, both regarding number and 
types of arguments.

> IIRC, when we rolled out `-Wstrict-prototypes` we explicitly excluded this 
> case since it hit a lot of code without finding bugs.

The GCC people didn't, so it can't be that bad. Also we already warn on block 
definitions with zero parameters for some reason. Lastly, the fix is very easy: 
just add `void`. With some `-fixit` magic it might just be a matter of running 
Clang over the entire code base once and you're done with it.

We don't only warn on actual bugs, but also on using deprecated language 
features and generally about problematic code. As I pointed out, 
**zero-parameter K&R definitions are problematic even if we error out when they 
are called with the wrong number of parameters**, because such definitions 
might be inline parts of an interface that's used with other compilers that 
don't have such checks.

> If you do pursue this please use a distinct `-W` flag so it can be turned off 
> without losing the benefits of `-Wstrict-prototypes`.

The warning is not on by default, and not enabled by `-Wall` or `-Wextra`, so 
this would serve the small sliver of developers who researched long enough to 
find this flag, decided to activate it only with Clang and not GCC, and then 
for some reason don't want it to do what it promises in its name. If the 
warning calls itself "strict", then that's what it should be.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66919



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

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

In D66324#1652364 , @lebedev.ri wrote:

> Trying to start reviewing this.
>  The llvm side of the patch is self contained; clang patch should be split 
> into a dependent review.


Looking at this, is it necessary to split up the patch? That will loose a lot 
of the previous comment/context, and I'd prefer to land this as a single 
change.  Doing so has the benefit that if my patch needs to be reverted, it can 
be done all together.  For example, if only the llvm patch is reverted, then it 
will likely cause other problems and make triage needlessly painful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66324



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