[PATCH] D57902: [AST] Fix structural inequivalence of operators

[Gabor Marton via Phabricator via cfe-commits]

martong marked 2 inline comments as done.
martong added inline comments.



Comment at: unittests/AST/StructuralEquivalenceTest.cpp:251
+
+TEST_F(StructuralEquivalenceFunctionTest, CtorVsDtor) {
+  auto t = makeDecls(

shafik wrote:
> Curious, is this test just for completeness or is this somehow related to the 
> overloaded operator check?
This is just for the sake of completeness. Operators and ctor/dtor do not have 
regular names with `IdentifierInfo`.  (See the `DeclarationName` class for 
details.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D57902



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

[PATCH] D45978: dllexport const variables must have external linkage.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

In D45978#1387385 , @aaron.ballman 
wrote:

> Can you add tests for C mode as well, as it seems the behavior differs there.


Aaron,

Let me know if that is enough. Thanks.


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

https://reviews.llvm.org/D45978



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

[PATCH] D45978: dllexport const variables must have external linkage.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 185920.

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

https://reviews.llvm.org/D45978

Files:
  test/Sema/dllexport-1.cpp
  test/Sema/dllexport-2.cpp
  test/Sema/dllexport.c


Index: test/Sema/dllexport.c
===
--- test/Sema/dllexport.c
+++ test/Sema/dllexport.c
@@ -162,3 +162,16 @@
 
 void __declspec(dllexport) precedenceRedecl2();
 void __declspec(dllimport) precedenceRedecl2() {} // 
expected-warning{{'dllimport' attribute ignored}}
+
+// Export const variable.
+
+// CHECK: @y = common dso_local dllexport global i32 0, align 4
+
+__declspec(dllexport) int const x = 3;
+__declspec(dllexport) const int y;
+extern int const z = 4; // expected-warning{{'extern' variable has an 
initializer}}
+
+int main() {
+  int a = x + y + z;
+  return a;
+}
Index: test/Sema/dllexport-2.cpp
===
--- test/Sema/dllexport-2.cpp
+++ test/Sema/dllexport-2.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple i686-win32 -fsyntax-only -fms-extensions -verify %s
-// RUN: %clang_cc1 -triple x86_64-win32   -fsyntax-only -fms-extensions 
-verify -std=c++11 %s
+// RUN: %clang_cc1 -triple x86_64-win32 -fsyntax-only -fms-extensions -verify 
-std=c++11 %s
 
 // Export const variable.
 
Index: test/Sema/dllexport-1.cpp
===
--- test/Sema/dllexport-1.cpp
+++ test/Sema/dllexport-1.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple i686-win32 -fsyntax-only -fms-extensions -verify %s
-// RUN: %clang_cc1 -triple x86_64-win32   -fsyntax-only -fms-extensions 
-verify -std=c++11 %s
+// RUN: %clang_cc1 -triple x86_64-win32 -fsyntax-only -fms-extensions -verify 
-std=c++11 %s
 
 // CHECK: @"?x@@3HB" = dso_local dllexport constant i32 3, align 4
 


Index: test/Sema/dllexport.c
===
--- test/Sema/dllexport.c
+++ test/Sema/dllexport.c
@@ -162,3 +162,16 @@
 
 void __declspec(dllexport) precedenceRedecl2();
 void __declspec(dllimport) precedenceRedecl2() {} // expected-warning{{'dllimport' attribute ignored}}
+
+// Export const variable.
+
+// CHECK: @y = common dso_local dllexport global i32 0, align 4
+
+__declspec(dllexport) int const x = 3;
+__declspec(dllexport) const int y;
+extern int const z = 4; // expected-warning{{'extern' variable has an initializer}}
+
+int main() {
+  int a = x + y + z;
+  return a;
+}
Index: test/Sema/dllexport-2.cpp
===
--- test/Sema/dllexport-2.cpp
+++ test/Sema/dllexport-2.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple i686-win32 -fsyntax-only -fms-extensions -verify %s
-// RUN: %clang_cc1 -triple x86_64-win32   -fsyntax-only -fms-extensions -verify -std=c++11 %s
+// RUN: %clang_cc1 -triple x86_64-win32 -fsyntax-only -fms-extensions -verify -std=c++11 %s
 
 // Export const variable.
 
Index: test/Sema/dllexport-1.cpp
===
--- test/Sema/dllexport-1.cpp
+++ test/Sema/dllexport-1.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple i686-win32 -fsyntax-only -fms-extensions -verify %s
-// RUN: %clang_cc1 -triple x86_64-win32   -fsyntax-only -fms-extensions -verify -std=c++11 %s
+// RUN: %clang_cc1 -triple x86_64-win32 -fsyntax-only -fms-extensions -verify -std=c++11 %s
 
 // CHECK: @"?x@@3HB" = dso_local dllexport constant i32 3, align 4
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r353504 - [AST] Fix structural inequivalence of operators

[Gabor Marton via cfe-commits]

Author: martong
Date: Fri Feb  8 00:55:32 2019
New Revision: 353504

URL: http://llvm.org/viewvc/llvm-project?rev=353504&view=rev
Log:
[AST] Fix structural inequivalence of operators

Summary: Operators kind was not checked, so we reported e.g. op- to be equal 
with op+

Reviewers: shafik, a_sidorin, aaron.ballman

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

Tags: #clang

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

Modified:
cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp
cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp

Modified: cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp?rev=353504&r1=353503&r2=353504&view=diff
==
--- cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp (original)
+++ cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp Fri Feb  8 00:55:32 2019
@@ -1650,6 +1650,12 @@ bool StructuralEquivalenceContext::Check
 }
   } else if (FunctionDecl *FD1 = dyn_cast(D1)) {
 if (FunctionDecl *FD2 = dyn_cast(D2)) {
+  if (FD1->isOverloadedOperator()) {
+if (!FD2->isOverloadedOperator())
+  return false;
+if (FD1->getOverloadedOperator() != FD2->getOverloadedOperator())
+  return false;
+  }
   if (!::IsStructurallyEquivalent(FD1->getIdentifier(),
   FD2->getIdentifier()))
 return false;

Modified: cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp?rev=353504&r1=353503&r2=353504&view=diff
==
--- cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp (original)
+++ cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp Fri Feb  8 00:55:32 
2019
@@ -230,6 +230,33 @@ TEST_F(StructuralEquivalenceFunctionTest
   EXPECT_FALSE(testStructuralMatch(t));
 }
 
+TEST_F(StructuralEquivalenceFunctionTest, DifferentOperators) {
+  auto t = makeDecls(
+  "struct X{}; bool operator<(X, X);",
+  "struct X{}; bool operator==(X, X);", Lang_CXX,
+  functionDecl(hasOverloadedOperatorName("<")),
+  functionDecl(hasOverloadedOperatorName("==")));
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceFunctionTest, SameOperators) {
+  auto t = makeDecls(
+  "struct X{}; bool operator<(X, X);",
+  "struct X{}; bool operator<(X, X);", Lang_CXX,
+  functionDecl(hasOverloadedOperatorName("<")),
+  functionDecl(hasOverloadedOperatorName("<")));
+  EXPECT_TRUE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceFunctionTest, CtorVsDtor) {
+  auto t = makeDecls(
+  "struct X{ X(); };",
+  "struct X{ ~X(); };", Lang_CXX,
+  cxxConstructorDecl(),
+  cxxDestructorDecl());
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
 TEST_F(StructuralEquivalenceFunctionTest, ParamConstWithRef) {
   auto t = makeNamedDecls("void foo(int&);",
   "void foo(const int&);", Lang_CXX);


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

[PATCH] D57902: [AST] Fix structural inequivalence of operators

[Gabor Marton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
martong marked an inline comment as done.
Closed by commit rC353504: [AST] Fix structural inequivalence of operators 
(authored by martong, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57902?vs=185793&id=185922#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D57902

Files:
  lib/AST/ASTStructuralEquivalence.cpp
  unittests/AST/StructuralEquivalenceTest.cpp


Index: lib/AST/ASTStructuralEquivalence.cpp
===
--- lib/AST/ASTStructuralEquivalence.cpp
+++ lib/AST/ASTStructuralEquivalence.cpp
@@ -1650,6 +1650,12 @@
 }
   } else if (FunctionDecl *FD1 = dyn_cast(D1)) {
 if (FunctionDecl *FD2 = dyn_cast(D2)) {
+  if (FD1->isOverloadedOperator()) {
+if (!FD2->isOverloadedOperator())
+  return false;
+if (FD1->getOverloadedOperator() != FD2->getOverloadedOperator())
+  return false;
+  }
   if (!::IsStructurallyEquivalent(FD1->getIdentifier(),
   FD2->getIdentifier()))
 return false;
Index: unittests/AST/StructuralEquivalenceTest.cpp
===
--- unittests/AST/StructuralEquivalenceTest.cpp
+++ unittests/AST/StructuralEquivalenceTest.cpp
@@ -230,6 +230,33 @@
   EXPECT_FALSE(testStructuralMatch(t));
 }
 
+TEST_F(StructuralEquivalenceFunctionTest, DifferentOperators) {
+  auto t = makeDecls(
+  "struct X{}; bool operator<(X, X);",
+  "struct X{}; bool operator==(X, X);", Lang_CXX,
+  functionDecl(hasOverloadedOperatorName("<")),
+  functionDecl(hasOverloadedOperatorName("==")));
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceFunctionTest, SameOperators) {
+  auto t = makeDecls(
+  "struct X{}; bool operator<(X, X);",
+  "struct X{}; bool operator<(X, X);", Lang_CXX,
+  functionDecl(hasOverloadedOperatorName("<")),
+  functionDecl(hasOverloadedOperatorName("<")));
+  EXPECT_TRUE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceFunctionTest, CtorVsDtor) {
+  auto t = makeDecls(
+  "struct X{ X(); };",
+  "struct X{ ~X(); };", Lang_CXX,
+  cxxConstructorDecl(),
+  cxxDestructorDecl());
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
 TEST_F(StructuralEquivalenceFunctionTest, ParamConstWithRef) {
   auto t = makeNamedDecls("void foo(int&);",
   "void foo(const int&);", Lang_CXX);


Index: lib/AST/ASTStructuralEquivalence.cpp
===
--- lib/AST/ASTStructuralEquivalence.cpp
+++ lib/AST/ASTStructuralEquivalence.cpp
@@ -1650,6 +1650,12 @@
 }
   } else if (FunctionDecl *FD1 = dyn_cast(D1)) {
 if (FunctionDecl *FD2 = dyn_cast(D2)) {
+  if (FD1->isOverloadedOperator()) {
+if (!FD2->isOverloadedOperator())
+  return false;
+if (FD1->getOverloadedOperator() != FD2->getOverloadedOperator())
+  return false;
+  }
   if (!::IsStructurallyEquivalent(FD1->getIdentifier(),
   FD2->getIdentifier()))
 return false;
Index: unittests/AST/StructuralEquivalenceTest.cpp
===
--- unittests/AST/StructuralEquivalenceTest.cpp
+++ unittests/AST/StructuralEquivalenceTest.cpp
@@ -230,6 +230,33 @@
   EXPECT_FALSE(testStructuralMatch(t));
 }
 
+TEST_F(StructuralEquivalenceFunctionTest, DifferentOperators) {
+  auto t = makeDecls(
+  "struct X{}; bool operator<(X, X);",
+  "struct X{}; bool operator==(X, X);", Lang_CXX,
+  functionDecl(hasOverloadedOperatorName("<")),
+  functionDecl(hasOverloadedOperatorName("==")));
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceFunctionTest, SameOperators) {
+  auto t = makeDecls(
+  "struct X{}; bool operator<(X, X);",
+  "struct X{}; bool operator<(X, X);", Lang_CXX,
+  functionDecl(hasOverloadedOperatorName("<")),
+  functionDecl(hasOverloadedOperatorName("<")));
+  EXPECT_TRUE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceFunctionTest, CtorVsDtor) {
+  auto t = makeDecls(
+  "struct X{ X(); };",
+  "struct X{ ~X(); };", Lang_CXX,
+  cxxConstructorDecl(),
+  cxxDestructorDecl());
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
 TEST_F(StructuralEquivalenceFunctionTest, ParamConstWithRef) {
   auto t = makeNamedDecls("void foo(int&);",
   "void foo(const int&);", Lang_CXX);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57905: [ASTImporter][ASTImporterSpecificLookup] Add test for different operators

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 185924.
martong marked an inline comment as done.
martong added a comment.

- Add comment above ASSERT_NE


Repository:
  rC Clang

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

https://reviews.llvm.org/D57905

Files:
  unittests/AST/ASTImporterTest.cpp


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -4812,6 +4812,66 @@
   EXPECT_EQ(Res.count(F2), 1u);
 }
 
+TEST_P(ASTImporterLookupTableTest,
+   DifferentOperatorsShouldHaveDifferentResultSet) {
+  TranslationUnitDecl *ToTU = getToTuDecl(
+  R"(
+  struct X{};
+  void operator+(X, X);
+  void operator-(X, X);
+  )",
+  Lang_CXX);
+
+  ASTImporterLookupTable LT(*ToTU);
+  auto *FPlus = FirstDeclMatcher().match(
+  ToTU, functionDecl(hasOverloadedOperatorName("+")));
+  auto *FMinus = FirstDeclMatcher().match(
+  ToTU, functionDecl(hasOverloadedOperatorName("-")));
+  DeclarationName NamePlus = FPlus->getDeclName();
+  auto ResPlus = LT.lookup(ToTU, NamePlus);
+  EXPECT_EQ(ResPlus.size(), 1u);
+  EXPECT_EQ(ResPlus.count(FPlus), 1u);
+  EXPECT_EQ(ResPlus.count(FMinus), 0u);
+  DeclarationName NameMinus = FMinus->getDeclName();
+  auto ResMinus = LT.lookup(ToTU, NameMinus);
+  EXPECT_EQ(ResMinus.size(), 1u);
+  EXPECT_EQ(ResMinus.count(FMinus), 1u);
+  EXPECT_EQ(ResMinus.count(FPlus), 0u);
+  EXPECT_NE(*ResMinus.begin(), *ResPlus.begin());
+}
+
+TEST_P(ASTImporterLookupTableTest, LookupDeclNamesFromDifferentTUs) {
+  TranslationUnitDecl *ToTU = getToTuDecl(
+  R"(
+  struct X {};
+  void operator+(X, X);
+  )",
+  Lang_CXX);
+  auto *ToPlus = FirstDeclMatcher().match(
+  ToTU, functionDecl(hasOverloadedOperatorName("+")));
+
+  Decl *FromTU = getTuDecl(
+  R"(
+  struct X {};
+  void operator+(X, X);
+  )",
+  Lang_CXX);
+  auto *FromPlus = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasOverloadedOperatorName("+")));
+
+  // FromPlus have a different TU, thus its DeclarationName is different too.
+  ASSERT_NE(ToPlus->getDeclName(), FromPlus->getDeclName());
+
+  ASTImporterLookupTable LT(*ToTU);
+  auto Res = LT.lookup(ToTU, ToPlus->getDeclName());
+  ASSERT_EQ(Res.size(), 1u);
+  EXPECT_EQ(*Res.begin(), ToPlus);
+
+  // FromPlus have a different TU, thus its DeclarationName is different too.
+  Res = LT.lookup(ToTU, FromPlus->getDeclName());
+  ASSERT_EQ(Res.size(), 0u);
+}
+
 static const RecordDecl * getRecordDeclOfFriend(FriendDecl *FD) {
   QualType Ty = FD->getFriendType()->getType();
   QualType NamedTy = cast(Ty)->getNamedType();


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -4812,6 +4812,66 @@
   EXPECT_EQ(Res.count(F2), 1u);
 }
 
+TEST_P(ASTImporterLookupTableTest,
+   DifferentOperatorsShouldHaveDifferentResultSet) {
+  TranslationUnitDecl *ToTU = getToTuDecl(
+  R"(
+  struct X{};
+  void operator+(X, X);
+  void operator-(X, X);
+  )",
+  Lang_CXX);
+
+  ASTImporterLookupTable LT(*ToTU);
+  auto *FPlus = FirstDeclMatcher().match(
+  ToTU, functionDecl(hasOverloadedOperatorName("+")));
+  auto *FMinus = FirstDeclMatcher().match(
+  ToTU, functionDecl(hasOverloadedOperatorName("-")));
+  DeclarationName NamePlus = FPlus->getDeclName();
+  auto ResPlus = LT.lookup(ToTU, NamePlus);
+  EXPECT_EQ(ResPlus.size(), 1u);
+  EXPECT_EQ(ResPlus.count(FPlus), 1u);
+  EXPECT_EQ(ResPlus.count(FMinus), 0u);
+  DeclarationName NameMinus = FMinus->getDeclName();
+  auto ResMinus = LT.lookup(ToTU, NameMinus);
+  EXPECT_EQ(ResMinus.size(), 1u);
+  EXPECT_EQ(ResMinus.count(FMinus), 1u);
+  EXPECT_EQ(ResMinus.count(FPlus), 0u);
+  EXPECT_NE(*ResMinus.begin(), *ResPlus.begin());
+}
+
+TEST_P(ASTImporterLookupTableTest, LookupDeclNamesFromDifferentTUs) {
+  TranslationUnitDecl *ToTU = getToTuDecl(
+  R"(
+  struct X {};
+  void operator+(X, X);
+  )",
+  Lang_CXX);
+  auto *ToPlus = FirstDeclMatcher().match(
+  ToTU, functionDecl(hasOverloadedOperatorName("+")));
+
+  Decl *FromTU = getTuDecl(
+  R"(
+  struct X {};
+  void operator+(X, X);
+  )",
+  Lang_CXX);
+  auto *FromPlus = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasOverloadedOperatorName("+")));
+
+  // FromPlus have a different TU, thus its DeclarationName is different too.
+  ASSERT_NE(ToPlus->getDeclName(), FromPlus->getDeclName());
+
+  ASTImporterLookupTable LT(*ToTU);
+  auto Res = LT.lookup(ToTU, ToPlus->getDeclName());
+  ASSERT_EQ(Res.size(), 1u);
+  EXPECT_EQ(*Res.begin(), ToPlus);
+
+  // FromPlus have a different TU, thus its DeclarationName is different too.
+  Res = LT.lookup(ToTU, FromPlus->getDeclName());
+  ASSERT_EQ(Res.size(), 0u);
+}
+
 static const RecordDecl * getRecordDeclO

r353505 - [ASTImporter][ASTImporterSpecificLookup] Add test for different operators

[Gabor Marton via cfe-commits]

Author: martong
Date: Fri Feb  8 01:19:34 2019
New Revision: 353505

URL: http://llvm.org/viewvc/llvm-project?rev=353505&view=rev
Log:
[ASTImporter][ASTImporterSpecificLookup] Add test for different operators

Summary:
This is to check that operators are handled properly in
`ASTImporterSpecificLookup`.  Note, this lookup table is not used in LLDB, only
in CTU.

Reviewers: a_sidorin, shafik, a.sidorin

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

Tags: #clang

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

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

Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=353505&r1=353504&r2=353505&view=diff
==
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Fri Feb  8 01:19:34 2019
@@ -4812,6 +4812,66 @@ TEST_P(ASTImporterLookupTableTest, Looku
   EXPECT_EQ(Res.count(F2), 1u);
 }
 
+TEST_P(ASTImporterLookupTableTest,
+   DifferentOperatorsShouldHaveDifferentResultSet) {
+  TranslationUnitDecl *ToTU = getToTuDecl(
+  R"(
+  struct X{};
+  void operator+(X, X);
+  void operator-(X, X);
+  )",
+  Lang_CXX);
+
+  ASTImporterLookupTable LT(*ToTU);
+  auto *FPlus = FirstDeclMatcher().match(
+  ToTU, functionDecl(hasOverloadedOperatorName("+")));
+  auto *FMinus = FirstDeclMatcher().match(
+  ToTU, functionDecl(hasOverloadedOperatorName("-")));
+  DeclarationName NamePlus = FPlus->getDeclName();
+  auto ResPlus = LT.lookup(ToTU, NamePlus);
+  EXPECT_EQ(ResPlus.size(), 1u);
+  EXPECT_EQ(ResPlus.count(FPlus), 1u);
+  EXPECT_EQ(ResPlus.count(FMinus), 0u);
+  DeclarationName NameMinus = FMinus->getDeclName();
+  auto ResMinus = LT.lookup(ToTU, NameMinus);
+  EXPECT_EQ(ResMinus.size(), 1u);
+  EXPECT_EQ(ResMinus.count(FMinus), 1u);
+  EXPECT_EQ(ResMinus.count(FPlus), 0u);
+  EXPECT_NE(*ResMinus.begin(), *ResPlus.begin());
+}
+
+TEST_P(ASTImporterLookupTableTest, LookupDeclNamesFromDifferentTUs) {
+  TranslationUnitDecl *ToTU = getToTuDecl(
+  R"(
+  struct X {};
+  void operator+(X, X);
+  )",
+  Lang_CXX);
+  auto *ToPlus = FirstDeclMatcher().match(
+  ToTU, functionDecl(hasOverloadedOperatorName("+")));
+
+  Decl *FromTU = getTuDecl(
+  R"(
+  struct X {};
+  void operator+(X, X);
+  )",
+  Lang_CXX);
+  auto *FromPlus = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasOverloadedOperatorName("+")));
+
+  // FromPlus have a different TU, thus its DeclarationName is different too.
+  ASSERT_NE(ToPlus->getDeclName(), FromPlus->getDeclName());
+
+  ASTImporterLookupTable LT(*ToTU);
+  auto Res = LT.lookup(ToTU, ToPlus->getDeclName());
+  ASSERT_EQ(Res.size(), 1u);
+  EXPECT_EQ(*Res.begin(), ToPlus);
+
+  // FromPlus have a different TU, thus its DeclarationName is different too.
+  Res = LT.lookup(ToTU, FromPlus->getDeclName());
+  ASSERT_EQ(Res.size(), 0u);
+}
+
 static const RecordDecl * getRecordDeclOfFriend(FriendDecl *FD) {
   QualType Ty = FD->getFriendType()->getType();
   QualType NamedTy = cast(Ty)->getNamedType();


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

[PATCH] D57905: [ASTImporter][ASTImporterSpecificLookup] Add test for different operators

[Gabor Marton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC353505: [ASTImporter][ASTImporterSpecificLookup] Add test 
for different operators (authored by martong, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57905?vs=185924&id=185925#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D57905

Files:
  unittests/AST/ASTImporterTest.cpp


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -4812,6 +4812,66 @@
   EXPECT_EQ(Res.count(F2), 1u);
 }
 
+TEST_P(ASTImporterLookupTableTest,
+   DifferentOperatorsShouldHaveDifferentResultSet) {
+  TranslationUnitDecl *ToTU = getToTuDecl(
+  R"(
+  struct X{};
+  void operator+(X, X);
+  void operator-(X, X);
+  )",
+  Lang_CXX);
+
+  ASTImporterLookupTable LT(*ToTU);
+  auto *FPlus = FirstDeclMatcher().match(
+  ToTU, functionDecl(hasOverloadedOperatorName("+")));
+  auto *FMinus = FirstDeclMatcher().match(
+  ToTU, functionDecl(hasOverloadedOperatorName("-")));
+  DeclarationName NamePlus = FPlus->getDeclName();
+  auto ResPlus = LT.lookup(ToTU, NamePlus);
+  EXPECT_EQ(ResPlus.size(), 1u);
+  EXPECT_EQ(ResPlus.count(FPlus), 1u);
+  EXPECT_EQ(ResPlus.count(FMinus), 0u);
+  DeclarationName NameMinus = FMinus->getDeclName();
+  auto ResMinus = LT.lookup(ToTU, NameMinus);
+  EXPECT_EQ(ResMinus.size(), 1u);
+  EXPECT_EQ(ResMinus.count(FMinus), 1u);
+  EXPECT_EQ(ResMinus.count(FPlus), 0u);
+  EXPECT_NE(*ResMinus.begin(), *ResPlus.begin());
+}
+
+TEST_P(ASTImporterLookupTableTest, LookupDeclNamesFromDifferentTUs) {
+  TranslationUnitDecl *ToTU = getToTuDecl(
+  R"(
+  struct X {};
+  void operator+(X, X);
+  )",
+  Lang_CXX);
+  auto *ToPlus = FirstDeclMatcher().match(
+  ToTU, functionDecl(hasOverloadedOperatorName("+")));
+
+  Decl *FromTU = getTuDecl(
+  R"(
+  struct X {};
+  void operator+(X, X);
+  )",
+  Lang_CXX);
+  auto *FromPlus = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasOverloadedOperatorName("+")));
+
+  // FromPlus have a different TU, thus its DeclarationName is different too.
+  ASSERT_NE(ToPlus->getDeclName(), FromPlus->getDeclName());
+
+  ASTImporterLookupTable LT(*ToTU);
+  auto Res = LT.lookup(ToTU, ToPlus->getDeclName());
+  ASSERT_EQ(Res.size(), 1u);
+  EXPECT_EQ(*Res.begin(), ToPlus);
+
+  // FromPlus have a different TU, thus its DeclarationName is different too.
+  Res = LT.lookup(ToTU, FromPlus->getDeclName());
+  ASSERT_EQ(Res.size(), 0u);
+}
+
 static const RecordDecl * getRecordDeclOfFriend(FriendDecl *FD) {
   QualType Ty = FD->getFriendType()->getType();
   QualType NamedTy = cast(Ty)->getNamedType();


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -4812,6 +4812,66 @@
   EXPECT_EQ(Res.count(F2), 1u);
 }
 
+TEST_P(ASTImporterLookupTableTest,
+   DifferentOperatorsShouldHaveDifferentResultSet) {
+  TranslationUnitDecl *ToTU = getToTuDecl(
+  R"(
+  struct X{};
+  void operator+(X, X);
+  void operator-(X, X);
+  )",
+  Lang_CXX);
+
+  ASTImporterLookupTable LT(*ToTU);
+  auto *FPlus = FirstDeclMatcher().match(
+  ToTU, functionDecl(hasOverloadedOperatorName("+")));
+  auto *FMinus = FirstDeclMatcher().match(
+  ToTU, functionDecl(hasOverloadedOperatorName("-")));
+  DeclarationName NamePlus = FPlus->getDeclName();
+  auto ResPlus = LT.lookup(ToTU, NamePlus);
+  EXPECT_EQ(ResPlus.size(), 1u);
+  EXPECT_EQ(ResPlus.count(FPlus), 1u);
+  EXPECT_EQ(ResPlus.count(FMinus), 0u);
+  DeclarationName NameMinus = FMinus->getDeclName();
+  auto ResMinus = LT.lookup(ToTU, NameMinus);
+  EXPECT_EQ(ResMinus.size(), 1u);
+  EXPECT_EQ(ResMinus.count(FMinus), 1u);
+  EXPECT_EQ(ResMinus.count(FPlus), 0u);
+  EXPECT_NE(*ResMinus.begin(), *ResPlus.begin());
+}
+
+TEST_P(ASTImporterLookupTableTest, LookupDeclNamesFromDifferentTUs) {
+  TranslationUnitDecl *ToTU = getToTuDecl(
+  R"(
+  struct X {};
+  void operator+(X, X);
+  )",
+  Lang_CXX);
+  auto *ToPlus = FirstDeclMatcher().match(
+  ToTU, functionDecl(hasOverloadedOperatorName("+")));
+
+  Decl *FromTU = getTuDecl(
+  R"(
+  struct X {};
+  void operator+(X, X);
+  )",
+  Lang_CXX);
+  auto *FromPlus = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasOverloadedOperatorName("+")));
+
+  // FromPlus have a different TU, thus its DeclarationName is different too.
+  ASSERT_NE(ToPlus->getDeclName(), FromPlus->getDeclName());
+
+  ASTImporterLookupTable LT(*ToTU);
+  auto Res = LT.lookup(ToTU, ToPlus->getDeclName());
+  ASSERT_EQ(Res.size(), 1u);
+  EXPECT_EQ(*Res.begin(), ToPlus);
+
+  // FromPlus have a different TU, thus its Declarat

[PATCH] D57906: [CTU] Do not allow different CPP dialects in CTU

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 185926.
martong marked 4 inline comments as done.
martong added a comment.

- Revert comment change
- Add lang_dialect_mismatch and stats for that


Repository:
  rC Clang

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

https://reviews.llvm.org/D57906

Files:
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp


Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -42,6 +42,7 @@
   "requested function's body");
 STATISTIC(NumTripleMismatch, "The # of triple mismatches");
 STATISTIC(NumLangMismatch, "The # of language mismatches");
+STATISTIC(NumLangDialectMismatch, "The # of language dialect mismatches");
 
 // Same as Triple's equality operator, but we check a field only if that is
 // known in both instances.
@@ -99,6 +100,8 @@
   return "Triple mismatch";
 case index_error_code::lang_mismatch:
   return "Language mismatch";
+case index_error_code::lang_dialect_mismatch:
+  return "Language dialect mismatch";
 }
 llvm_unreachable("Unrecognized index_error_code.");
   }
@@ -228,6 +231,7 @@
 
   const auto &LangTo = Context.getLangOpts();
   const auto &LangFrom = Unit->getASTContext().getLangOpts();
+
   // FIXME: Currenty we do not support CTU across C++ and C and across
   // different dialects of C++.
   if (LangTo.CPlusPlus != LangFrom.CPlusPlus) {
@@ -235,6 +239,28 @@
 return llvm::make_error(index_error_code::lang_mismatch);
   }
 
+  // If CPP dialects are different then return with error.
+  //
+  // Consider this STL code:
+  //   template
+  // struct __alloc_traits
+  //   #if __cplusplus >= 201103L
+  // : std::allocator_traits<_Alloc>
+  //   #endif
+  // { // ...
+  // };
+  // This class template would create ODR errors during merging the two units,
+  // since in one translation unit the class template has a base class, however
+  // in the other unit it has none.
+  if (LangTo.CPlusPlus11 != LangFrom.CPlusPlus11 ||
+  LangTo.CPlusPlus14 != LangFrom.CPlusPlus14 ||
+  LangTo.CPlusPlus17 != LangFrom.CPlusPlus17 ||
+  LangTo.CPlusPlus2a != LangFrom.CPlusPlus2a) {
+++NumLangDialectMismatch;
+return llvm::make_error(
+index_error_code::lang_dialect_mismatch);
+  }
+
   TranslationUnitDecl *TU = Unit->getASTContext().getTranslationUnitDecl();
   if (const FunctionDecl *ResultDecl =
   findFunctionInDeclContext(TU, LookupFnName))
Index: include/clang/CrossTU/CrossTranslationUnit.h
===
--- include/clang/CrossTU/CrossTranslationUnit.h
+++ include/clang/CrossTU/CrossTranslationUnit.h
@@ -43,7 +43,8 @@
   failed_to_get_external_ast,
   failed_to_generate_usr,
   triple_mismatch,
-  lang_mismatch
+  lang_mismatch,
+  lang_dialect_mismatch
 };
 
 class IndexError : public llvm::ErrorInfo {


Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -42,6 +42,7 @@
   "requested function's body");
 STATISTIC(NumTripleMismatch, "The # of triple mismatches");
 STATISTIC(NumLangMismatch, "The # of language mismatches");
+STATISTIC(NumLangDialectMismatch, "The # of language dialect mismatches");
 
 // Same as Triple's equality operator, but we check a field only if that is
 // known in both instances.
@@ -99,6 +100,8 @@
   return "Triple mismatch";
 case index_error_code::lang_mismatch:
   return "Language mismatch";
+case index_error_code::lang_dialect_mismatch:
+  return "Language dialect mismatch";
 }
 llvm_unreachable("Unrecognized index_error_code.");
   }
@@ -228,6 +231,7 @@
 
   const auto &LangTo = Context.getLangOpts();
   const auto &LangFrom = Unit->getASTContext().getLangOpts();
+
   // FIXME: Currenty we do not support CTU across C++ and C and across
   // different dialects of C++.
   if (LangTo.CPlusPlus != LangFrom.CPlusPlus) {
@@ -235,6 +239,28 @@
 return llvm::make_error(index_error_code::lang_mismatch);
   }
 
+  // If CPP dialects are different then return with error.
+  //
+  // Consider this STL code:
+  //   template
+  // struct __alloc_traits
+  //   #if __cplusplus >= 201103L
+  // : std::allocator_traits<_Alloc>
+  //   #endif
+  // { // ...
+  // };
+  // This class template would create ODR errors during merging the two units,
+  // since in one translation unit the class template has a base class, however
+  // in the other unit it has none.
+  if (LangTo.CPlusPlus11 != LangFrom.CPlusPlus11 ||
+  LangTo.CPlusPlus14 != LangFrom.CPlusPlus14 ||
+  LangTo.CPlusPlus17 != LangFrom.CPlusPlus17 ||
+  LangTo.CPlusPlus2a != LangFrom.CPlusPlus2a) {
+++NumLangDialectMismatch;
+return llvm::make_e

[PATCH] D57906: [CTU] Do not allow different CPP dialects in CTU

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:232
+
+  // We do not support CTU across languages (C vs C++).
   if (LangTo.CPlusPlus != LangFrom.CPlusPlus) {

a_sidorin wrote:
> The comment change looks strange.
Ok, I reverted this hunk.



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:255
+  LangTo.CPlusPlus2a != LangFrom.CPlusPlus2a) {
+++NumLangMismatch;
+return llvm::make_error(index_error_code::lang_mismatch);

a_sidorin wrote:
> Should we create another counter, like NumLangDialectMismatch?
Okay, I added the new error code `lang_dialect_mismatch` too.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57906



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

[PATCH] D57896: Variable names rule

[Michael Platings via Phabricator via cfe-commits]

michaelplatings added a comment.

In D57896#1389067 , @lebedev.ri wrote:

> 1. Does clang-tidy warn on every single existing variable now?
> 2. It might be best to give this more visibility, by submitting a mail to 
> llvm-dev, with a **noticeable** subject, like "RFC: changing variable naming 
> rules in LLVM codebase"




1. Pretty much. Previously clang-tidy gave 56,787 unique warnings. After this 
patch it gives 361,382 unique warnings. I personally would be happy to change 
the settings from `camelBack` to `aNy_CasE`.

2. Done: http://lists.llvm.org/pipermail/llvm-dev/2019-February/130083.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57896



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

[PATCH] D57739: [clangd] Format tweak's replacements.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D57739#1385144 , @hokein wrote:

> In D57739#1384844 , @ilya-biryukov 
> wrote:
>
> > Could we move the code to the `Tweak` itself, so that all the callers would 
> > get the formatting? I.e.
> >
> >   class Tweak {
> >  Replacements apply() {  // This is now non-virtual.
> >auto Replacements = doApply();
> >return cleanupAndFormat(Replacements);
> >  }
> >  
> >   protected:
> >  virtual Replacements doApply() = 0; // inheritors should implement 
> > this now. Note: the name is terrible, suggestions are welcome.
> >   };
> >
> >
> > This would ensure the callers don't need to deal with the formatting on 
> > their own.
>
>
> This seems introduce intrusive changes to the Tweak interface (Tweak will 
> need to know the `FormatStyle` somehow), I think this might be done in 
> another patch.


I agree with this concern, and don't think this is an appropriate layer to be 
aware of styling or failing on format errors. Can we consider moving the 
formatting to clangdserver as originally planned?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57739



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

[PATCH] D57898: [RFC] Split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[Alexander Potapenko via Phabricator via cfe-commits]

glider marked 2 inline comments as done.
glider added inline comments.



Comment at: tools/clang/lib/CodeGen/CGDecl.cpp:1143
+const llvm::StructLayout *Layout =
+CGM.getDataLayout().getStructLayout(cast(Ty));
+for (unsigned i = 0; i != constant->getNumOperands(); i++) {

jfb wrote:
> I think you need a heuristic here, where you don't emit multiple stores if 
> `getNumOperands` is greater than some number. In that case you'd keep doing 
> memcpy instead. Note that a struct containing sub-structs (or arrays) should 
> also count the number of sub-structs (so just checking `getNumOperands` isn't 
> sufficient).
> 
> Other parts of this code chose 6 stores...
Do we ever need to not split these stores? Can't the  MemCpyOpt pass take care 
of them later on?



Comment at: tools/clang/lib/CodeGen/CGDecl.cpp:1765
+  emitStoresForConstant(CGM, D, Loc, isVolatile, Builder, constant,
+/*forInit*/ false);
 }

jfb wrote:
> Can you explain why this case needs to be different? IIUC it's because the 
> types can differ which makes the struct's elements not match up the ones from 
> the constant in the loop above? I think the code above should automatically 
> handle that case instead of passing in a flag here.
> 
> You also definitely need to test the case where that happens :)
I just thought we shouldn't break the constants that we didn't create with 
-ftrivial-var-auto-init.
I'll take a closer look though (indeed, this crashes on one of the structs from 
the Linux kernel)


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

https://reviews.llvm.org/D57898



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

[PATCH] D57898: [RFC] Split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[Alexander Potapenko via Phabricator via cfe-commits]

glider marked 2 inline comments as not done.
glider added a comment.

Sorry, didn't mean to mark these done.


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

https://reviews.llvm.org/D57898



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

[PATCH] D56370: [clangd] Add support for type hierarchy (super types only for now)

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a subscriber: sammccall.
kadircet added a comment.

Implementation LG, but I am not sure about adding a functionality on a proposal 
that might change. WDYT @sammccall ?




Comment at: clang-tools-extra/clangd/FindSymbols.cpp:16
 #include "SourceCode.h"
+#include "XRefs.h"
 #include "index/Index.h"

it seems like there is no change in file requiring this



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:218
   }
+
   return SI;

can you revert this one as well ?



Comment at: clang-tools-extra/clangd/Protocol.cpp:817
+bool fromJSON(const llvm::json::Value &E, TypeHierarchyDirection &Out) {
+  if (auto T = E.getAsInteger()) {
+if (*T < static_cast(TypeHierarchyDirection::Children) ||

nit: get rid of nesting by
```
auto T = E.getas..;
if(!T)
  return false;
if(*T not in rage)
  return false;
Out = cast(*T);
return true;
```



Comment at: clang-tools-extra/clangd/Protocol.h:1026
+  /// When not defined, it is treated as `0`.
+  llvm::Optional resolve;
+

why not just use an int then ?



Comment at: clang-tools-extra/clangd/Protocol.h:1046
+  /// It is `false` by default.
+  llvm::Optional deprecated;
+

again we can just store a bool, and only serialize if it is true



Comment at: clang-tools-extra/clangd/XRefs.cpp:876
+  Optional Result = declToTypeHierarchyItem(ASTCtx, CXXRD);
+  if (Result && Levels > 0) {
+if (Direction == TypeHierarchyDirection::Parents ||

again reduce nesting



Comment at: clang-tools-extra/clangd/XRefs.cpp:891
+if (Optional ParentSym =
+getTypeHierarchy(*ParentDecl, ASTCtx, Levels - 1, Direction)) {
+  Result->parents->emplace_back(std::move(*ParentSym));

A problem that might occur when you add children traversal:
do we really want to include current decl in children of parent when we have 
`BOTH` as direction? If so, maybe we should rather cache the results? Maybe 
leave a todo/fixme ?



Comment at: clang-tools-extra/clangd/XRefs.cpp:917
+  if (Symbols.Decls.empty())
+return {};
+

llvm::None



Comment at: clang-tools-extra/clangd/XRefs.cpp:925
+CXXRD = VD->getType().getTypePtr()->getAsCXXRecordDecl();
+  } else if (const CXXMethodDecl *Method = dyn_cast(D)) {
+// If this is a method, use the type of the class.

what about member fields ?



Comment at: clang-tools-extra/clangd/XRefs.cpp:932
+
+  if (CXXRD) {
+int ResolveLevels = Resolve.getValueOr(0);

reduce nesting



Comment at: clang-tools-extra/clangd/XRefs.cpp:935
+TypeHierarchyDirection ResolveDirection =
+Direction.getValueOr(TypeHierarchyDirection::Parents);
+return getTypeHierarchy(*CXXRD, ASTCtx, ResolveLevels, ResolveDirection);

Is this mentioned in proposal?



Comment at: clang-tools-extra/clangd/XRefs.cpp:939
+
+  return {};
+}

llvm::None



Comment at: clang-tools-extra/unittests/clangd/XRefsTests.cpp:1355
+  Annotations Source(R"cpp(
+struct $ParentDef[[Parent]]
+{

can you format the code?



Comment at: clang-tools-extra/unittests/clangd/XRefsTests.cpp:1387
+
+  for (auto Pt : {"p1", "p2", "p3", "p4"}) {
+llvm::Optional Result = getTypeHierarchy(

You can use `Source.points()` and use `Pt` directly instead of doing 
`Source.point(pt)`. Same for other tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D56370



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

[PATCH] D57936: [CodeGenObjC] When available, emit a direct call to objc_alloc_init(cls) instead of [objc_alloc(cls) init]

[Greg Parker via Phabricator via cfe-commits]

gparker42 added inline comments.



Comment at: clang/lib/CodeGen/CGObjC.cpp:459
+  llvm::Value *Receiver =
+  CGF.CGM.getObjCRuntime().GetClass(CGF, ObjTy->getInterface());
+  return CGF.EmitObjCAllocInit(Receiver, CGF.ConvertType(OME->getType()));

rjmccall wrote:
> You might need to check for a `super` message send, which can be a class 
> message send in a class method.
> 
> Also, I think `getInterface()` can return null here, although it might need 
> to be contrived to fit the other requirements.  `Class`, where 
> that protocol declares `+alloc`?  If there isn't a way to just emit the 
> receiver pointer of a normal message send from an `ObjCMessageExpr`, that's 
> really too bad.
Should this be done inside `tryGenerateSpecializedMessageSend`, or from the 
same place that calls `tryGenerateSpecializedMessageSend`? That position 
already rejects `[super …]` call sites, and might make it more obvious how the 
`objc_alloc` and `objc_alloc_init` optimizations interact. (Currently it takes 
some digging to verify that the `objc_alloc_init` generator does not need to 
look for the pattern `[objc_alloc(cls) init]`).


Repository:
  rC Clang

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

https://reviews.llvm.org/D57936



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

[PATCH] D57739: [clangd] Format tweak's replacements.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D57739#1390252 , @sammccall wrote:

> I agree with this concern, and don't think this is an appropriate layer to be 
> aware of styling or failing on format errors. Can we consider moving the 
> formatting to clangdserver as originally planned?


`clang-format` seems to be somewhat stable, do we actually expect that to be a 
large problem in practice?
On the other side, I can't imagine any clients that don't need formatting? E.g. 
it's nice when tests give the same results as one would see in the final clangd 
and tests don't go through `ClangdServer`.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57739



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

[PATCH] D57936: [CodeGenObjC] When available, emit a direct call to objc_alloc_init(cls) instead of [objc_alloc(cls) init]

[Greg Parker via Phabricator via cfe-commits]

gparker42 added inline comments.



Comment at: clang/lib/CodeGen/CGObjC.cpp:459
+  llvm::Value *Receiver =
+  CGF.CGM.getObjCRuntime().GetClass(CGF, ObjTy->getInterface());
+  return CGF.EmitObjCAllocInit(Receiver, CGF.ConvertType(OME->getType()));

gparker42 wrote:
> rjmccall wrote:
> > You might need to check for a `super` message send, which can be a class 
> > message send in a class method.
> > 
> > Also, I think `getInterface()` can return null here, although it might need 
> > to be contrived to fit the other requirements.  `Class`, 
> > where that protocol declares `+alloc`?  If there isn't a way to just emit 
> > the receiver pointer of a normal message send from an `ObjCMessageExpr`, 
> > that's really too bad.
> Should this be done inside `tryGenerateSpecializedMessageSend`, or from the 
> same place that calls `tryGenerateSpecializedMessageSend`? That position 
> already rejects `[super …]` call sites, and might make it more obvious how 
> the `objc_alloc` and `objc_alloc_init` optimizations interact. (Currently it 
> takes some digging to verify that the `objc_alloc_init` generator does not 
> need to look for the pattern `[objc_alloc(cls) init]`).
(Never mind, that super check wouldn't help catch `[[super alloc] init]`because 
it's checking `init`'s receiver.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D57936



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

[PATCH] D57739: [clangd] Format tweak's replacements.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D57739#1390315 , @ilya-biryukov 
wrote:

> In D57739#1390252 , @sammccall wrote:
>
> > I agree with this concern, and don't think this is an appropriate layer to 
> > be aware of styling or failing on format errors. Can we consider moving the 
> > formatting to clangdserver as originally planned?
>
>
> `clang-format` seems to be somewhat stable, do we actually expect that to be 
> a large problem in practice?
>  On the other side, I can't imagine any clients that don't need formatting? 
> E.g. it's nice when tests give the same results as one would see in the final 
> clangd and tests don't go through `ClangdServer`.


It's not about stability or whether the functionality is desired, but layering.
Unit tests having narrow scope is a good thing - if we want system tests that 
check clangdserver's behavior, they should test clangdserver.
Clients that don't go through clangdserver probably want formatting, but an 
immediate cleanupAndFormat on each generated change isn't necessarily the right 
way to do that.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57739



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

[PATCH] D57883: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer

[Jonas Toth via Phabricator via cfe-commits]

JonasToth marked 2 inline comments as done.
JonasToth added a comment.

Good idea :)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57883



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

[PATCH] D57883: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer

[Jonas Toth via Phabricator via cfe-commits]

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

- [Refactor] move support classes into the analyzer
- [Refactor] move bigger methods into implementation file


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57883

Files:
  clang-tidy/bugprone/ExceptionEscapeCheck.cpp
  clang-tidy/utils/ExceptionAnalyzer.cpp
  clang-tidy/utils/ExceptionAnalyzer.h

Index: clang-tidy/utils/ExceptionAnalyzer.h
===
--- clang-tidy/utils/ExceptionAnalyzer.h
+++ clang-tidy/utils/ExceptionAnalyzer.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_EXCEPTION_ANALYZER_H
 
 #include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringSet.h"
 
@@ -17,29 +18,148 @@
 namespace tidy {
 namespace utils {
 
-/// This class analysis if a `FunctionDecl` can in principle throw an exception,
-/// either directly or indirectly.
-/// It can be configured to ignore custom exception types.
+/// This class analysis if a `FunctionDecl` can in principle throw an
+/// exception, either directly or indirectly. It can be configured to ignore
+/// custom exception types.
 class ExceptionAnalyzer {
 public:
+  enum class State : std::int8_t {
+Throwing,///< The function can definitly throw given an AST.
+NotThrowing, ///< This function can not throw, given an AST.
+Unknown, ///< This can happen for extern functions without available
+ ///< definition.
+  };
+
+  /// Bundle the gathered information about an entity like a function regarding
+  /// it's exception behaviour. The 'NonThrowing'-state can be considered as the
+  /// neutral element in terms of information propagation.
+  /// In the case of 'Throwing' state it is possible that 'getExceptionTypes'
+  /// does not include *ALL* possible types as there is the possibility that
+  /// an 'Unknown' function is called that might throw a previously unknown
+  /// exception at runtime.
+  class ExceptionInfo {
+  public:
+using Throwables = llvm::SmallSet;
+static ExceptionInfo createUnknown() {
+  return ExceptionInfo(State::Unknown);
+}
+static ExceptionInfo createNonThrowing() {
+  return ExceptionInfo(State::Throwing);
+}
+
+/// By default the exception situation is unknown and must be
+/// clarified step-wise.
+ExceptionInfo() : Behaviour(State::NotThrowing), ContainsUnknown(false) {}
+ExceptionInfo(State S)
+: Behaviour(S), ContainsUnknown(S == State::Unknown) {}
+
+ExceptionInfo(const ExceptionInfo &) = default;
+ExceptionInfo &operator=(const ExceptionInfo &) = default;
+ExceptionInfo(ExceptionInfo &&) = default;
+ExceptionInfo &operator=(ExceptionInfo &&) = default;
+
+State getBehaviour() const { return Behaviour; }
+
+void registerException(const Type *ExceptionType) {
+  assert(ExceptionType != nullptr && "Only valid types are accepted");
+  Behaviour = State::Throwing;
+  ThrownExceptions.insert(ExceptionType);
+}
+void registerExceptions(const Throwables &Exceptions) {
+  if (Exceptions.size() == 0)
+return;
+  Behaviour = State::Throwing;
+  ThrownExceptions.insert(Exceptions.begin(), Exceptions.end());
+}
+
+/// Updates the local state according to the other state. That means if
+/// for example a function contains multiple statements the 'ExceptionInfo'
+/// for the final function is the merged result of each statement.
+/// If one of these statements throws the whole function throws and if one
+/// part is unknown and the rest is non-throwing the result will be
+/// unknown.
+ExceptionInfo &merge(const ExceptionInfo &Other);
+
+/// This method is useful in case 'catch' clauses are analyzed as it is
+/// possible to catch multiple exception types by one 'catch' if they
+/// are a subclass of the 'catch'ed exception type.
+/// Returns 'true' if some exceptions were filtered, otherwise 'false'.
+bool filterByCatch(const Type *BaseClass);
+
+/// Filter the set of thrown exception type against a set of ignored
+/// types that shall not be considered in the exception analysis.
+/// This includes explicit `std::bad_alloc` ignoring as separate option.
+ExceptionInfo &
+filterIgnoredExceptions(const llvm::StringSet<> &IgnoredTypes,
+bool IgnoreBadAlloc);
+
+/// Clear the state to 'NonThrowing' to make the corresponding entity
+/// neutral.
+void clear() {
+  Behaviour = State::NotThrowing;
+  ContainsUnknown = false;
+  ThrownExceptions.clear();
+}
+
+/// References the set of known exception types that can escape from the
+/// corresponding entity.
+const Throwables &getExceptionTypes() const { return ThrownExceptions; }
+
+/// Signal if th

[PATCH] D57852: [clang-tidy] Don't use assignment for value-initialized enums

[Jonas Toth via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57852



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

[PATCH] D57943: [clangd] **Prototype**: clang-tidy-based tweaks

[Haojian Wu via Phabricator via cfe-commits]

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

clang-tidy has already provided some simple refactoring-like checks (e.g. 
modernize-use-auto,
modernize-use-using), so we can leverage the power of clang-tidy to implement 
tweaks.

clang-tidy-based tweaks compared to clang-tidy check:

- For refactoring checks, tweaks maybe a better UI compared to showing as 
diagnostics (we only show on the UI when users do it intendedly)
- faster, we run the check on a single AST node rather than the whole AST


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D57943

Files:
  clangd/refactor/tweaks/CMakeLists.txt
  clangd/refactor/tweaks/ClangTidy.cpp
  unittests/clangd/TweakTests.cpp

Index: unittests/clangd/TweakTests.cpp
===
--- unittests/clangd/TweakTests.cpp
+++ unittests/clangd/TweakTests.cpp
@@ -217,6 +217,23 @@
   )cpp");
 }
 
+TEST(TweakTest, ConvertTypedef) {
+  llvm::StringLiteral ID = "ConvertTypedef";
+  checkAvailable(ID, R"cpp(^t^y^p^e^d^e^f^ int ^F^o^o;)cpp");
+  llvm::StringLiteral Input = "^typedef int Foo;";
+  llvm::StringLiteral Output = "using Foo = int;";
+  checkTransform(ID, Input, Output);
+}
+
+TEST(TweakTest, UseAuto) {
+  llvm::StringLiteral ID = "UseAuto";
+  llvm::StringLiteral Input =
+  "void f() { [[unsigned c = static_cast(1);]] }";
+  llvm::StringLiteral Output =
+  "void f() { auto c = static_cast(1); }";
+  checkTransform(ID, Input, Output);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/refactor/tweaks/ClangTidy.cpp
===
--- /dev/null
+++ clangd/refactor/tweaks/ClangTidy.cpp
@@ -0,0 +1,146 @@
+//===--- ClangTidy.cpp ---*- C++-*-===//
+//
+// 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 "ClangdUnit.h"
+#include "refactor/Tweak.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+
+#include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
+#include "../clang-tidy/modernize/UseUsingCheck.h"
+#include "../clang-tidy/modernize/UseAutoCheck.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+bool isInsideMainFile(const SourceLocation Loc, const SourceManager &M) {
+  return Loc.isValid() && M.isWrittenInMainFile(M.getFileLoc(Loc));
+}
+
+struct StoreClangTidyDiags : public DiagnosticConsumer {
+  void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
+const clang::Diagnostic &Info) override {
+assert(Info.hasSourceManager() && "Diagnostic must have SourceManager");
+if (Info.getNumFixItHints() > 0)
+  DiagsWithFixIt.push_back(Info);
+  }
+  std::vector DiagsWithFixIt;
+};
+
+/// An interface for clang-tidy-based Tweaks. The tweak will run a clang-tidy
+/// check over on an AST node under the cursor. It is designed for simple
+/// refactoring-like clang-tidy checks.
+/// Limitations:
+///- checks only match single AST node would work
+///- checks don't see any preprocessor events
+///- no check options are supported yet
+class ClangTidyTweak : public Tweak {
+public:
+  ClangTidyTweak()
+  : DiagsEng(new clang::DiagnosticIDs(), new clang::DiagnosticOptions(),
+ &ClangTidyDiags, /*ShouldOwnClient=*/false) {}
+
+  virtual std::unique_ptr
+  createCheck(tidy::ClangTidyContext *) = 0;
+
+  bool prepare(const Selection &Inputs) override;
+
+protected:
+  Expected execute(const Selection &Inputs) override {
+tooling::Replacements FixReplacements;
+for (const auto &Diag : ClangTidyDiags.DiagsWithFixIt) {
+  for (auto &FixIt : Diag.getFixItHints()) {
+if (!isInsideMainFile(FixIt.RemoveRange.getBegin(),
+  Diag.getSourceManager()))
+  continue;
+tooling::Replacement Fix(Diag.getSourceManager(), FixIt.RemoveRange,
+ FixIt.CodeToInsert);
+
+llvm::Error Err = FixReplacements.add(Fix);
+if (Err)
+  return std::move(Err);
+  }
+}
+return FixReplacements;
+  }
+
+  StoreClangTidyDiags ClangTidyDiags;
+  DiagnosticsEngine DiagsEng; // Extend the lifetime of Diagnostics.
+};
+
+bool ClangTidyTweak::prepare(const Selection &Inputs) {
+  if (!Inputs.ASTSelection.commonAncestor())
+return false;
+  if (Inputs.AST.getASTContext().getDiagnostics().hasErrorOccurred())
+return f

[PATCH] D57943: [clangd] **Prototype**: clang-tidy-based tweaks

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 185941.
hokein added a comment.

Update


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57943

Files:
  clangd/refactor/tweaks/CMakeLists.txt
  clangd/refactor/tweaks/ClangTidy.cpp
  unittests/clangd/TweakTests.cpp

Index: unittests/clangd/TweakTests.cpp
===
--- unittests/clangd/TweakTests.cpp
+++ unittests/clangd/TweakTests.cpp
@@ -217,6 +217,23 @@
   )cpp");
 }
 
+TEST(TweakTest, ConvertTypedef) {
+  llvm::StringLiteral ID = "ConvertTypedef";
+  checkAvailable(ID, R"cpp(^t^y^p^e^d^e^f^ int ^F^o^o;)cpp");
+  llvm::StringLiteral Input = "^typedef int Foo;";
+  llvm::StringLiteral Output = "using Foo = int;";
+  checkTransform(ID, Input, Output);
+}
+
+TEST(TweakTest, UseAuto) {
+  llvm::StringLiteral ID = "UseAuto";
+  llvm::StringLiteral Input =
+  "void f() { [[unsigned c = static_cast(1);]] }";
+  llvm::StringLiteral Output =
+  "void f() { auto c = static_cast(1); }";
+  checkTransform(ID, Input, Output);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/refactor/tweaks/ClangTidy.cpp
===
--- /dev/null
+++ clangd/refactor/tweaks/ClangTidy.cpp
@@ -0,0 +1,149 @@
+//===--- ClangTidy.cpp ---*- C++-*-===//
+//
+// 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 "ClangdUnit.h"
+#include "refactor/Tweak.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+
+#include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
+#include "../clang-tidy/modernize/UseUsingCheck.h"
+#include "../clang-tidy/modernize/UseAutoCheck.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+bool isInsideMainFile(const SourceLocation Loc, const SourceManager &M) {
+  return Loc.isValid() && M.isWrittenInMainFile(M.getFileLoc(Loc));
+}
+
+struct StoreClangTidyDiags : public DiagnosticConsumer {
+  void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
+const clang::Diagnostic &Info) override {
+assert(Info.hasSourceManager() && "Diagnostic must have SourceManager");
+if (Info.getNumFixItHints() > 0)
+  DiagsWithFixIt.push_back(Info);
+  }
+  std::vector DiagsWithFixIt;
+};
+
+/// An interface for clang-tidy-based Tweaks. The tweak will run a clang-tidy
+/// check over on an AST node under the cursor. It is designed for simple
+/// refactoring-like clang-tidy checks.
+/// Limitations:
+///- checks only match single AST node would work
+///- checks don't see any preprocessor events
+///- no check options are supported yet
+class ClangTidyTweak : public Tweak {
+public:
+  ClangTidyTweak()
+  : DiagsEng(new clang::DiagnosticIDs(), new clang::DiagnosticOptions(),
+ &ClangTidyDiags, /*ShouldOwnClient=*/false) {}
+
+  virtual std::unique_ptr
+  createCheck(tidy::ClangTidyContext *) = 0;
+
+  bool prepare(const Selection &Inputs) override;
+
+protected:
+  Expected execute(const Selection &Inputs) override;
+
+  StoreClangTidyDiags ClangTidyDiags;
+  DiagnosticsEngine DiagsEng; // Extend the lifetime of Diagnostics.
+};
+
+bool ClangTidyTweak::prepare(const Selection &Inputs) {
+  if (!Inputs.ASTSelection.commonAncestor())
+return false;
+  if (Inputs.AST.getASTContext().getDiagnostics().hasErrorOccurred())
+return false;
+  DiagsEng.setSourceManager(&Inputs.AST.getASTContext().getSourceManager());
+  tidy::ClangTidyContext CTContext(
+  llvm::make_unique(
+  tidy::ClangTidyGlobalOptions(),
+  tidy::ClangTidyOptions::getDefaults()));
+  CTContext.setDiagnosticsEngine(&DiagsEng);
+  CTContext.setASTContext(&Inputs.AST.getASTContext());
+
+  auto Check = createCheck(&CTContext);
+  ast_matchers::MatchFinder CTFinder;
+  Check->registerMatchers(&CTFinder);
+  // Run the check on the AST node under the cursor.
+  CTFinder.match(Inputs.ASTSelection.commonAncestor()->ASTNode,
+ Inputs.AST.getASTContext());
+  return !ClangTidyDiags.DiagsWithFixIt.empty();
+}
+
+Expected
+ClangTidyTweak::execute(const Selection &Inputs) {
+  tooling::Replacements FixReplacements;
+  for (const auto &Diag : ClangTidyDiags.DiagsWithFixIt) {
+for (auto &FixIt : Diag.getFixItHints()) {
+  if (!isInsideMainFile(FixIt.RemoveRange.getBegin(),
+Diag.getSourceManager()))
+continue;
+  tooling::Replacement Fix(Di

[PATCH] D57943: [clangd] **Prototype**: clang-tidy-based tweaks

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

This is my experiment of playing around tweaks, I think we'll need this.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57943



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

[PATCH] D40988: Clang-format: add finer-grained options for putting all arguments on one line

[Andrey via Phabricator via cfe-commits]

aol-nnov added a comment.

would love such a feature too!

also, there is kind of similar review https://reviews.llvm.org/D14484?id=39647 
(by @djasper , also rejected. which is really sad)


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

https://reviews.llvm.org/D40988



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

[PATCH] D57577: Make predefined FLT16 macros conditional on support for the type

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added a comment.

Does anyone have any further comments or objections to this patch? I would like 
to commit this and close the PR.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57577



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

[PATCH] D56370: [clangd] Add support for type hierarchy (super types only for now)

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D56370#1390283 , @kadircet wrote:

> Implementation LG, but I am not sure about adding a functionality on a 
> proposal that might change. WDYT @sammccall ?


We discussed this a bit on the thread, I think we should follow the proposed 
spec, but with some useful hedges:

- only implement enough bits to be useful
- I'm not sure the internal APIs/implementation should follow the "shape" of 
the spec so deeply.

So on a concrete but still high-level:

- I don't think we should implement the `resolve` operation, or resolution 
bounds, at this point - this patch doesn't do anything slow. Returning complete 
ancestors and never returning any children seems simplest.
- in 'XRefs.h', I think the API to introduce is `findTypeAt(Position) -> Decl*` 
+  `typeAncestors(Decl*)` and later `typeDescendants(Decl*)`, rather than a 
single more complex `typeHierarchy` call. These two operations have little in 
common implementation-wise, and it's easy to imagine editors preferring to 
expose them separately. In clangdserver of course we need to expose a single 
operation because of transactionality. The stitching together could go in 
clangdserver, or a higher-level function exposed by xrefs - but I think the 
separate functions are what we should be unit-testing.




Comment at: clang-tools-extra/unittests/clangd/Matchers.h:130
 
+// Implements the HasValue(m) matcher for matching an Optional whose
+// value matches matcher m.

nridge wrote:
> Should I split this out into a separate patch? It's used by the tests being 
> added for this functionality.
This is nice! I think it should probably go in llvm/Testing/Support/..., but 
it's OK here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D56370



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

[PATCH] D56370: [clangd] Add support for type hierarchy (super types only for now)

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Updated reviewers line to reflect (I think) current reality so this doesn't get 
lost, but feel free to reassign as you'd prefer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D56370



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

[PATCH] D57944: [clangd] Fix an assertion in TypoCorrection.

[Eric Liu via Phabricator via cfe-commits]

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

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


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D57944

Files:
  clangd/IncludeFixer.cpp
  unittests/clangd/DiagnosticsTests.cpp


Index: unittests/clangd/DiagnosticsTests.cpp
===
--- unittests/clangd/DiagnosticsTests.cpp
+++ unittests/clangd/DiagnosticsTests.cpp
@@ -423,6 +423,21 @@
   "Add include \"b.h\" for symbol na::nb::X");
 }
 
+TEST(IncludeFixerTest, NoCrashMemebrAccess) {
+  Annotations Test(R"cpp(
+struct X { int  xyz; };
+void g() { X x; x.$[[xy]] }
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  auto Index = buildIndexWithSymbol(
+  SymbolWithHeader{"na::X", "unittest:///a.h", "\"a.h\""});
+  TU.ExternalIndex = Index.get();
+
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  UnorderedElementsAre(Diag(Test.range(), "no member named 'xy' in 'X'")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/IncludeFixer.cpp
===
--- clangd/IncludeFixer.cpp
+++ clangd/IncludeFixer.cpp
@@ -192,8 +192,6 @@
 if (!SemaPtr->SourceMgr.isWrittenInMainFile(Typo.getLoc()))
   return clang::TypoCorrection();
 
-assert(S && "Enclosing scope must be set.");
-
 UnresolvedName Unresolved;
 Unresolved.Name = Typo.getAsString();
 Unresolved.Loc = Typo.getBeginLoc();
@@ -228,6 +226,8 @@
   return TypoCorrection();
   }
 }
+if (!SpecifiedScope && !S) // Give up if no scope available.
+  return TypoCorrection();
 
 auto *Sem = SemaPtr; // Avoid capturing `this`.
 Unresolved.GetScopes = [Sem, SpecifiedScope, S, LookupKind]() {
@@ -235,6 +235,7 @@
   if (SpecifiedScope) {
 Scopes.push_back(*SpecifiedScope);
   } else {
+assert(S);
 // No scope qualifier is specified. Collect all accessible scopes in 
the
 // context.
 VisitedContextCollector Collector;


Index: unittests/clangd/DiagnosticsTests.cpp
===
--- unittests/clangd/DiagnosticsTests.cpp
+++ unittests/clangd/DiagnosticsTests.cpp
@@ -423,6 +423,21 @@
   "Add include \"b.h\" for symbol na::nb::X");
 }
 
+TEST(IncludeFixerTest, NoCrashMemebrAccess) {
+  Annotations Test(R"cpp(
+struct X { int  xyz; };
+void g() { X x; x.$[[xy]] }
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  auto Index = buildIndexWithSymbol(
+  SymbolWithHeader{"na::X", "unittest:///a.h", "\"a.h\""});
+  TU.ExternalIndex = Index.get();
+
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  UnorderedElementsAre(Diag(Test.range(), "no member named 'xy' in 'X'")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/IncludeFixer.cpp
===
--- clangd/IncludeFixer.cpp
+++ clangd/IncludeFixer.cpp
@@ -192,8 +192,6 @@
 if (!SemaPtr->SourceMgr.isWrittenInMainFile(Typo.getLoc()))
   return clang::TypoCorrection();
 
-assert(S && "Enclosing scope must be set.");
-
 UnresolvedName Unresolved;
 Unresolved.Name = Typo.getAsString();
 Unresolved.Loc = Typo.getBeginLoc();
@@ -228,6 +226,8 @@
   return TypoCorrection();
   }
 }
+if (!SpecifiedScope && !S) // Give up if no scope available.
+  return TypoCorrection();
 
 auto *Sem = SemaPtr; // Avoid capturing `this`.
 Unresolved.GetScopes = [Sem, SpecifiedScope, S, LookupKind]() {
@@ -235,6 +235,7 @@
   if (SpecifiedScope) {
 Scopes.push_back(*SpecifiedScope);
   } else {
+assert(S);
 // No scope qualifier is specified. Collect all accessible scopes in the
 // context.
 VisitedContextCollector Collector;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57739: [clangd] Format tweak's replacements.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D57739#1390321 , @sammccall wrote:

> It's not about stability or whether the functionality is desired, but 
> layering.
>  Unit tests having narrow scope is a good thing - if we want system tests 
> that check clangdserver's behavior, they should test clangdserver.
>  Clients that don't go through clangdserver probably want formatting, but an 
> immediate cleanupAndFormat on each generated change isn't necessarily the 
> right way to do that.


My argument is that it's better to provide formatting by default in the public 
interface for **applying a single tweak**.
I might be missing some use-cases, e.g. one that comes to mind is applying 
multiple tweaks in a row, in which case we'd want to format once and not for 
every tweak.

My concerns are code duplication and ease of use for the clients. Having both 
`apply` and `applyAndFormat` (the latter being a non-virtual or a free-standing 
utility function) in the public interface of the `Tweak` would totally do it 
for me.
However, I also think tests should format by default, not sure we agree on this.
WDYT?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57739



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

[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 185947.
MyDeveloperDay marked 2 inline comments as done.
MyDeveloperDay added a comment.

Pre-Commit Changes

- add FIXME wording to test
- move local header include after libraries
- rebase


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

https://reviews.llvm.org/D57674

Files:
  clang-tidy/bugprone/ArgumentCommentCheck.cpp
  clang-tidy/bugprone/ArgumentCommentCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-argument-comment.rst
  test/clang-tidy/bugprone-argument-comment-literals.cpp

Index: test/clang-tidy/bugprone-argument-comment-literals.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-argument-comment-literals.cpp
@@ -0,0 +1,124 @@
+// RUN: %check_clang_tidy %s bugprone-argument-comment %t -- \
+// RUN:   -config="{CheckOptions: [{key: CommentBoolLiterals, value: 1},{key: CommentIntegerLiterals, value: 1}, {key: CommentFloatLiterals, value: 1}, {key: CommentUserDefinedLiterals, value: 1}, {key: CommentStringLiterals, value: 1}, {key: CommentNullPtrs, value: 1}, {key: CommentCharacterLiterals, value: 1}]}" --
+
+struct A {
+  void foo(bool abc);
+  void foo(bool abc, bool cde);
+  void foo(const char *, bool abc);
+  void foo(int iabc);
+  void foo(float fabc);
+  void foo(double dabc);
+  void foo(const char *strabc);
+  void fooW(const wchar_t *wstrabc);
+  void fooPtr(A *ptrabc);
+  void foo(char chabc);
+};
+
+#define FOO 1
+
+void g(int a);
+void h(double b);
+void i(const char *c);
+
+double operator"" _km(long double);
+
+void test() {
+  A a;
+
+  a.foo(true);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/true);
+
+  a.foo(false);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false);
+
+  a.foo(true, false);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-MESSAGES: [[@LINE-2]]:15: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/true, /*cde=*/false);
+
+  a.foo(false, true);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-MESSAGES: [[@LINE-2]]:16: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+  a.foo(/*abc=*/false, true);
+  // CHECK-MESSAGES: [[@LINE-1]]:24: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+  a.foo(false, /*cde=*/true);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+  bool val1 = true;
+  bool val2 = false;
+  a.foo(val1, val2);
+
+  a.foo("", true);
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo("", /*abc=*/true);
+
+  a.foo(0);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'iabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*iabc=*/0);
+
+  a.foo(1.0f);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'fabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*fabc=*/1.0f);
+
+  a.foo(1.0);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'dabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*dabc=*/1.0);
+
+  int val3 = 10;
+  a.foo(val3);
+
+  float val4 = 10.0;
+  a.foo(val4);
+
+  double val5 = 10.0;
+  a.foo(val5);
+
+  a.foo("Hello World");
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'strabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*strabc=*/"Hello World");
+  //
+  a.fooW(L"Hello World");
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: argument comment missing for literal argument 'wstrabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.fooW(/*wstrabc=*/L"Hello World");
+
+  a.fooPtr(nullptr);
+  // CHECK-MESSAGES: [[@LINE-1]]:12: warning: argument comment missing for literal argument 'ptrabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.fooPtr(/*ptrabc=*/nullptr);
+
+  a.foo(402.0_km);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'dabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*dabc=*/402.0_km);
+
+  a.foo('A');
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'chabc' [bugprone-argument-comment]
+  // CHEC

[PATCH] D57944: [clangd] Fix an assertion in TypoCorrection.

[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: clangd/IncludeFixer.cpp:195
 
-assert(S && "Enclosing scope must be set.");
-
 UnresolvedName Unresolved;
 Unresolved.Name = Typo.getAsString();

nit: move the `Unresolved` below the `if (!SpecifiedScope && !S)`.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57944



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

[clang-tools-extra] r353514 - [clangd] Fix an assertion in TypoCorrection.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Fri Feb  8 05:27:47 2019
New Revision: 353514

URL: http://llvm.org/viewvc/llvm-project?rev=353514&view=rev
Log:
[clangd] Fix an assertion in TypoCorrection.

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

Reviewers: sammccall, hokein

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

Tags: #clang

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

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

Modified: clang-tools-extra/trunk/clangd/IncludeFixer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/IncludeFixer.cpp?rev=353514&r1=353513&r2=353514&view=diff
==
--- clang-tools-extra/trunk/clangd/IncludeFixer.cpp (original)
+++ clang-tools-extra/trunk/clangd/IncludeFixer.cpp Fri Feb  8 05:27:47 2019
@@ -192,12 +192,6 @@ public:
 if (!SemaPtr->SourceMgr.isWrittenInMainFile(Typo.getLoc()))
   return clang::TypoCorrection();
 
-assert(S && "Enclosing scope must be set.");
-
-UnresolvedName Unresolved;
-Unresolved.Name = Typo.getAsString();
-Unresolved.Loc = Typo.getBeginLoc();
-
 // FIXME: support invalid scope before a type name. In the following
 // example, namespace "clang::tidy::" hasn't been declared/imported.
 //namespace clang {
@@ -228,6 +222,12 @@ public:
   return TypoCorrection();
   }
 }
+if (!SpecifiedScope && !S) // Give up if no scope available.
+  return TypoCorrection();
+
+UnresolvedName Unresolved;
+Unresolved.Name = Typo.getAsString();
+Unresolved.Loc = Typo.getBeginLoc();
 
 auto *Sem = SemaPtr; // Avoid capturing `this`.
 Unresolved.GetScopes = [Sem, SpecifiedScope, S, LookupKind]() {
@@ -235,6 +235,7 @@ public:
   if (SpecifiedScope) {
 Scopes.push_back(*SpecifiedScope);
   } else {
+assert(S);
 // No scope qualifier is specified. Collect all accessible scopes in 
the
 // context.
 VisitedContextCollector Collector;

Modified: clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp?rev=353514&r1=353513&r2=353514&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp Fri Feb  8 
05:27:47 2019
@@ -423,6 +423,21 @@ void foo() {
   "Add include \"b.h\" for symbol na::nb::X");
 }
 
+TEST(IncludeFixerTest, NoCrashMemebrAccess) {
+  Annotations Test(R"cpp(
+struct X { int  xyz; };
+void g() { X x; x.$[[xy]] }
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  auto Index = buildIndexWithSymbol(
+  SymbolWithHeader{"na::X", "unittest:///a.h", "\"a.h\""});
+  TU.ExternalIndex = Index.get();
+
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  UnorderedElementsAre(Diag(Test.range(), "no member named 'xy' in 'X'")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang


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

[PATCH] D57944: [clangd] Fix an assertion in TypoCorrection.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
ioeric marked an inline comment as done.
Closed by commit rL353514: [clangd] Fix an assertion in TypoCorrection. 
(authored by ioeric, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57944?vs=185944&id=185950#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57944

Files:
  clang-tools-extra/trunk/clangd/IncludeFixer.cpp
  clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp


Index: clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp
@@ -423,6 +423,21 @@
   "Add include \"b.h\" for symbol na::nb::X");
 }
 
+TEST(IncludeFixerTest, NoCrashMemebrAccess) {
+  Annotations Test(R"cpp(
+struct X { int  xyz; };
+void g() { X x; x.$[[xy]] }
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  auto Index = buildIndexWithSymbol(
+  SymbolWithHeader{"na::X", "unittest:///a.h", "\"a.h\""});
+  TU.ExternalIndex = Index.get();
+
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  UnorderedElementsAre(Diag(Test.range(), "no member named 'xy' in 'X'")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/trunk/clangd/IncludeFixer.cpp
===
--- clang-tools-extra/trunk/clangd/IncludeFixer.cpp
+++ clang-tools-extra/trunk/clangd/IncludeFixer.cpp
@@ -192,12 +192,6 @@
 if (!SemaPtr->SourceMgr.isWrittenInMainFile(Typo.getLoc()))
   return clang::TypoCorrection();
 
-assert(S && "Enclosing scope must be set.");
-
-UnresolvedName Unresolved;
-Unresolved.Name = Typo.getAsString();
-Unresolved.Loc = Typo.getBeginLoc();
-
 // FIXME: support invalid scope before a type name. In the following
 // example, namespace "clang::tidy::" hasn't been declared/imported.
 //namespace clang {
@@ -228,6 +222,12 @@
   return TypoCorrection();
   }
 }
+if (!SpecifiedScope && !S) // Give up if no scope available.
+  return TypoCorrection();
+
+UnresolvedName Unresolved;
+Unresolved.Name = Typo.getAsString();
+Unresolved.Loc = Typo.getBeginLoc();
 
 auto *Sem = SemaPtr; // Avoid capturing `this`.
 Unresolved.GetScopes = [Sem, SpecifiedScope, S, LookupKind]() {
@@ -235,6 +235,7 @@
   if (SpecifiedScope) {
 Scopes.push_back(*SpecifiedScope);
   } else {
+assert(S);
 // No scope qualifier is specified. Collect all accessible scopes in 
the
 // context.
 VisitedContextCollector Collector;


Index: clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp
@@ -423,6 +423,21 @@
   "Add include \"b.h\" for symbol na::nb::X");
 }
 
+TEST(IncludeFixerTest, NoCrashMemebrAccess) {
+  Annotations Test(R"cpp(
+struct X { int  xyz; };
+void g() { X x; x.$[[xy]] }
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  auto Index = buildIndexWithSymbol(
+  SymbolWithHeader{"na::X", "unittest:///a.h", "\"a.h\""});
+  TU.ExternalIndex = Index.get();
+
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  UnorderedElementsAre(Diag(Test.range(), "no member named 'xy' in 'X'")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/trunk/clangd/IncludeFixer.cpp
===
--- clang-tools-extra/trunk/clangd/IncludeFixer.cpp
+++ clang-tools-extra/trunk/clangd/IncludeFixer.cpp
@@ -192,12 +192,6 @@
 if (!SemaPtr->SourceMgr.isWrittenInMainFile(Typo.getLoc()))
   return clang::TypoCorrection();
 
-assert(S && "Enclosing scope must be set.");
-
-UnresolvedName Unresolved;
-Unresolved.Name = Typo.getAsString();
-Unresolved.Loc = Typo.getBeginLoc();
-
 // FIXME: support invalid scope before a type name. In the following
 // example, namespace "clang::tidy::" hasn't been declared/imported.
 //namespace clang {
@@ -228,6 +222,12 @@
   return TypoCorrection();
   }
 }
+if (!SpecifiedScope && !S) // Give up if no scope available.
+  return TypoCorrection();
+
+UnresolvedName Unresolved;
+Unresolved.Name = Typo.getAsString();
+Unresolved.Loc = Typo.getBeginLoc();
 
 auto *Sem = SemaPtr; // Avoid capturing `this`.
 Unresolved.GetScopes = [Sem, SpecifiedScope, S, LookupKind]() {
@@ -235,6 +235,7 @@
   if (SpecifiedScope) {
   

[PATCH] D57577: Make predefined FLT16 macros conditional on support for the type

[Sjoerd Meijer via Phabricator via cfe-commits]

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

Looks okay to me, with one nit inline.




Comment at: test/Preprocessor/init.c:9169
 // WEBASSEMBLY-NEXT:#define __FLOAT128__ 1
-// WEBASSEMBLY-NEXT:#define __FLT16_DECIMAL_DIG__ 5
-// WEBASSEMBLY-NEXT:#define __FLT16_DENORM_MIN__ 5.9604644775390625e-8F16

Perhaps change this in WEBASSEMBLY-NOT so that we also have one negative test 
for this?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57577



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

[PATCH] D57946: [libclang] Add attribute support for 'convergent'.

[Hsin-Hsiao Lin via Phabricator via cfe-commits]

hhlin created this revision.
hhlin added reviewers: yvvan, aaron.ballman, Anastasia.
hhlin created this object with visibility "All Users".
Herald added subscribers: cfe-commits, arphaman.
Herald added a reviewer: serge-sans-paille.
Herald added a project: clang.

This bumps CINDEX_VERSION_MINOR up (to 51).


Repository:
  rC Clang

https://reviews.llvm.org/D57946

Files:
  bindings/python/clang/cindex.py
  include/clang-c/Index.h
  test/Index/attributes.c
  tools/libclang/CIndex.cpp
  tools/libclang/CXCursor.cpp


Index: tools/libclang/CXCursor.cpp
===
--- tools/libclang/CXCursor.cpp
+++ tools/libclang/CXCursor.cpp
@@ -78,6 +78,7 @@
 case attr::ObjCRuntimeVisible: return CXCursor_ObjCRuntimeVisible;
 case attr::ObjCBoxable: return CXCursor_ObjCBoxable;
 case attr::FlagEnum: return CXCursor_FlagEnum;
+case attr::Convergent: return CXCursor_ConvergentAttr;
   }
 
   return CXCursor_UnexposedAttr;
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -5474,7 +5474,9 @@
   case CXCursor_StaticAssert:
   return cxstring::createRef("StaticAssert");
   case CXCursor_FriendDecl:
-return cxstring::createRef("FriendDecl");
+  return cxstring::createRef("FriendDecl");
+  case CXCursor_ConvergentAttr:
+  return cxstring::createRef("attribute(convergent)");
   }
 
   llvm_unreachable("Unhandled CXCursorKind");
Index: test/Index/attributes.c
===
--- test/Index/attributes.c
+++ test/Index/attributes.c
@@ -12,6 +12,8 @@
   Foo
 };
 
+void convergent_fn() __attribute__((convergent));
+
 // CHECK: attributes.c:3:32: StructDecl=Test2:3:32 (Definition) Extent=[3:1 - 
5:2]
 // CHECK: attributes.c:3:23: attribute(packed)=packed Extent=[3:23 - 3:29]
 // CHECK: attributes.c:4:8: FieldDecl=a:4:8 (Definition) Extent=[4:3 - 4:9] 
[access=public]
@@ -24,3 +26,6 @@
 // CHECK: attributes.c:9:38: attribute(noduplicate)= Extent=[9:38 - 9:49]
 // CHECK: attributes.c:11:31: EnumDecl=FlagEnum:11:31 (Definition) 
Extent=[11:1 - 13:2]
 // CHECK: attributes.c:11:19: attribute(flag_enum)= Extent=[11:19 - 11:28]
+// CHECK: attributes.c:12:3: EnumConstantDecl=Foo:12:3 (Definition) 
Extent=[12:3 - 12:6]
+// CHECK: attributes.c:15:6: FunctionDecl=convergent_fn:15:6 Extent=[15:1 - 
15:49]
+// CHECK: attributes.c:15:37: attribute(convergent)= Extent=[15:37 - 15:47]
Index: include/clang-c/Index.h
===
--- include/clang-c/Index.h
+++ include/clang-c/Index.h
@@ -32,7 +32,7 @@
  * compatible, thus CINDEX_VERSION_MAJOR is expected to remain stable.
  */
 #define CINDEX_VERSION_MAJOR 0
-#define CINDEX_VERSION_MINOR 50
+#define CINDEX_VERSION_MINOR 51
 
 #define CINDEX_VERSION_ENCODE(major, minor) ( \
   ((major) * 1)   \
@@ -2586,7 +2586,8 @@
   CXCursor_ObjCRuntimeVisible= 435,
   CXCursor_ObjCBoxable   = 436,
   CXCursor_FlagEnum  = 437,
-  CXCursor_LastAttr  = CXCursor_FlagEnum,
+  CXCursor_ConvergentAttr= 438,
+  CXCursor_LastAttr  = CXCursor_ConvergentAttr,
 
   /* Preprocessing */
   CXCursor_PreprocessingDirective= 500,
Index: bindings/python/clang/cindex.py
===
--- bindings/python/clang/cindex.py
+++ bindings/python/clang/cindex.py
@@ -1342,6 +1342,7 @@
 
 CursorKind.DLLEXPORT_ATTR = CursorKind(418)
 CursorKind.DLLIMPORT_ATTR = CursorKind(419)
+CursorKind.CONVERGENT_ATTR = CursorKind(420)
 
 ###
 # Preprocessing


Index: tools/libclang/CXCursor.cpp
===
--- tools/libclang/CXCursor.cpp
+++ tools/libclang/CXCursor.cpp
@@ -78,6 +78,7 @@
 case attr::ObjCRuntimeVisible: return CXCursor_ObjCRuntimeVisible;
 case attr::ObjCBoxable: return CXCursor_ObjCBoxable;
 case attr::FlagEnum: return CXCursor_FlagEnum;
+case attr::Convergent: return CXCursor_ConvergentAttr;
   }
 
   return CXCursor_UnexposedAttr;
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -5474,7 +5474,9 @@
   case CXCursor_StaticAssert:
   return cxstring::createRef("StaticAssert");
   case CXCursor_FriendDecl:
-return cxstring::createRef("FriendDecl");
+  return cxstring::createRef("FriendDecl");
+  case CXCursor_ConvergentAttr:
+  return cxstring::createRef("attribute(convergent)");
   }
 
   llvm_unreachable("Unhandled CXCursorKind");
Index: test/Index/attributes.c
===
--- test/Index/attributes.c
+++ test/Index/attributes.c
@@ -12,6 +12,8 @@
   Foo
 };
 
+void convergent_fn() __attribute__((co

[PATCH] D57893: [analyzer] Fix function macro crash

[Tibor Brunner via Phabricator via cfe-commits]

bruntib updated this revision to Diff 185957.
bruntib added a comment.

I've added a test case.


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

https://reviews.llvm.org/D57893

Files:
  lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
  test/Analysis/plist-macros-with-expansion.cpp

Index: test/Analysis/plist-macros-with-expansion.cpp
===
--- test/Analysis/plist-macros-with-expansion.cpp
+++ test/Analysis/plist-macros-with-expansion.cpp
@@ -441,3 +441,13 @@
 }
 // CHECK: nameYET_ANOTHER_SET_TO_NULL
 // CHECK-NEXT: expansionprint((void *)5); print((void *)"Remember the Vasa"); ptr = nullptr;
+
+#define FOO(x) int bar() { return x; }
+#define APPLY_ZERO(function) function(0)
+APPLY_ZERO(FOO)
+void useZeroApplier() {
+  (void)(1 / bar()); // expected-warning{{Division by zero}}
+}
+
+// CHECK: nameAPPLY_ZERO
+// CHECK-NEXT: expansionint bar() { return x; }(0)
Index: test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
===
--- test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
+++ test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
@@ -5168,6 +5168,237 @@
file0
   
   
+  
+   path
+   
+
+ kindcontrol
+ edges
+  
+   
+start
+ 
+  
+   line449
+   col10
+   file0
+  
+  
+   line449
+   col10
+   file0
+  
+ 
+end
+ 
+  
+   line449
+   col14
+   file0
+  
+  
+   line449
+   col16
+   file0
+  
+ 
+   
+  
+
+
+ kindevent
+ location
+ 
+  line449
+  col14
+  file0
+ 
+ ranges
+ 
+   
+
+ line449
+ col14
+ file0
+
+
+ line449
+ col18
+ file0
+
+   
+ 
+ depth0
+ extended_message
+ Calling 'bar'
+ message
+ Calling 'bar'
+
+
+ kindevent
+ location
+ 
+  line447
+  col1
+  file0
+ 
+ depth1
+ extended_message
+ Entered call from 'useZeroApplier'
+ message
+ Entered call from 'useZeroApplier'
+
+
+ kindevent
+ location
+ 
+  line447
+  col1
+  file0
+ 
+ ranges
+ 
+   
+
+ line447
+ col1
+ file0
+
+
+ line447
+ col15
+ file0
+
+   
+ 
+ depth1
+ extended_message
+ Returning zero
+ message
+ Returning zero
+
+
+ kindevent
+ location
+ 
+  line449
+  col14
+  file0
+ 
+ ranges
+ 
+   
+
+ line449
+ col14
+ file0
+
+
+ line449
+ col18
+ file0
+
+   
+ 
+ depth0
+ extended_message
+ Returning from 'bar'
+ message
+ Returning from 'bar'
+
+
+ kindcontrol
+ edges
+  
+   
+start
+ 
+  
+   line449
+   col14
+   file0
+  
+  
+   line449
+   col16
+   file0
+  
+ 
+end
+ 
+  
+   line449
+   col12
+   file0
+  
+  
+   line449
+   col12
+   file0
+  
+ 
+   
+  
+
+
+ kindevent
+ location
+ 
+  line449
+  col12
+  file0
+ 
+ ranges
+ 
+   
+
+ line449
+ col10
+ file0
+
+
+ line449
+ col18
+ file0
+
+   
+ 
+ depth0
+ extended_message
+ Division by zero
+ message
+ Division by zero
+
+   
+   macro_expansions
+   
+
+ location
+ 
+  line447
+  col1
+  file0
+ 
+ nameAPPLY_ZERO
+ expansionint bar() { return x; }(0)
+
+   
+   descriptionDivision by zero
+   categoryLogic error
+   typeDivision by zero
+   check_namecore.DivideZero
+   
+   issue_hash_content_of_line_in_contextb41a3835d64fddaac63749c968f17e81
+  issue_context_kindfunction
+  issue_contextuseZeroApplier
+  issue_hash_function_offset1
+  location
+  
+   line449
+   col12
+   file0
+  
+  
  
 
 
Index: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===
--- lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -879,8 +879,18 @@
 
 getMacroNameAndPrintExpansion(Printer, ArgIt->getLocation(), PP,
   Info.Args, AlreadyProcessedTokens);
-if (

[PATCH] D55250: [clangd] Enhance macro hover to see full definition

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle updated this revision to Diff 185959.
malaperle marked an inline comment as not done.
malaperle added a comment.

Remove temporary code.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55250

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/XRefs.cpp
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -648,7 +648,25 @@
 #define MACRO 2
 #undef macro
   )cpp",
-  "#define MACRO",
+  "#define MACRO 1",
+  },
+  {
+  R"cpp(// Macro
+#define MACRO 0
+#define MACRO2 ^MACRO
+  )cpp",
+  "#define MACRO 0",
+  },
+  {
+  R"cpp(// Macro
+#define MACRO {\
+  return 0;\
+}
+int main() ^MACRO
+  )cpp",
+  R"cpp(#define MACRO {\
+  return 0;\
+})cpp",
   },
   {
   R"cpp(// Forward class declaration
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -537,13 +537,30 @@
   return H;
 }
 
-/// Generate a \p Hover object given the macro \p MacroInf.
-static Hover getHoverContents(llvm::StringRef MacroName) {
-  Hover H;
-
-  H.contents.value = "#define ";
-  H.contents.value += MacroName;
+/// Generate a \p Hover object given the macro \p MacroDecl.
+static Hover getHoverContents(MacroDecl Decl, ParsedAST &AST) {
+  SourceManager &SM = AST.getASTContext().getSourceManager();
+  std::string Definition = Decl.Name;
+
+  // Try to get the full definition, not just the name
+  SourceLocation StartLoc = Decl.Info->getDefinitionLoc();
+  SourceLocation EndLoc = Decl.Info->getDefinitionEndLoc();
+  if (EndLoc.isValid()) {
+EndLoc = Lexer::getLocForEndOfToken(EndLoc, 0, SM,
+AST.getASTContext().getLangOpts());
+bool Invalid;
+StringRef Buffer = SM.getBufferData(SM.getFileID(StartLoc), &Invalid);
+if (!Invalid) {
+  unsigned StartOffset = SM.getFileOffset(StartLoc);
+  unsigned EndOffset = SM.getFileOffset(EndLoc);
+  if (EndOffset <= Buffer.size() && StartOffset < EndOffset)
+Definition = Buffer.substr(StartOffset, EndOffset - StartOffset).str();
+}
+  }
 
+  Hover H;
+  H.contents.kind = MarkupKind::PlainText;
+  H.contents.value = "#define " + Definition;
   return H;
 }
 
@@ -667,7 +684,7 @@
   auto Symbols = getSymbolAtPosition(AST, SourceLocationBeg);
 
   if (!Symbols.Macros.empty())
-return getHoverContents(Symbols.Macros[0].Name);
+return getHoverContents(Symbols.Macros[0], AST);
 
   if (!Symbols.Decls.empty())
 return getHoverContents(Symbols.Decls[0].D);
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -331,6 +331,16 @@
 SymbolKind adjustKindToCapability(SymbolKind Kind,
   SymbolKindBitset &supportedSymbolKinds);
 
+enum class MarkupKind {
+  PlainText,
+  Markdown,
+};
+bool fromJSON(const llvm::json::Value &, MarkupKind &);
+constexpr auto MarkupKindMin = static_cast(MarkupKind::PlainText);
+constexpr auto MarkupKindMax = static_cast(MarkupKind::Markdown);
+using MarkupKindBitset = std::bitset;
+bool fromJSON(const llvm::json::Value &, MarkupKindBitset &);
+
 // This struct doesn't mirror LSP!
 // The protocol defines deeply nested structures for client capabilities.
 // Instead of mapping them all, this just parses out the bits we care about.
@@ -362,6 +372,10 @@
   /// Client supports CodeAction return value for textDocument/codeAction.
   /// textDocument.codeAction.codeActionLiteralSupport.
   bool CodeActionStructure = false;
+
+  /// The supported set of MarkupKinds for hover.
+  /// textDocument.hover.contentFormat.
+  llvm::Optional HoverMarkupKinds;
 };
 bool fromJSON(const llvm::json::Value &, ClientCapabilities &);
 
@@ -818,11 +832,6 @@
 };
 bool fromJSON(const llvm::json::Value &, CompletionParams &);
 
-enum class MarkupKind {
-  PlainText,
-  Markdown,
-};
-
 struct MarkupContent {
   MarkupKind kind = MarkupKind::PlainText;
   std::string value;
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -245,6 +245,13 @@
   DocumentSymbol->getBoolean("hierarchicalDocumentSymbolSupport"))
 R.HierarchicalDocumentSymbol = *HierarchicalSupport;
 }
+if (auto *Hover = TextDocument->getObject("hover")) {
+  if (auto HoverMarkupKinds = Hover->get("contentFormat")) {
+R.HoverMarkupKinds.emplace();
+if (!fromJSON(*HoverMarkupKinds, *R.HoverMarkup

[PATCH] D57948: [Sema] Fix a regression introduced in "[AST][Sema] Remove CallExpr::setNumArgs"

[Bruno Ricci via Phabricator via cfe-commits]

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

D54902  removed `CallExpr::setNumArgs` in 
preparation of tail-allocating the arguments of `CallExpr`. It did this by 
allocating storage for `max(number of arguments, number of parameters in the 
prototype)`. The temporarily nulled arguments however causes issues in 
`BuildResolvedCallExpr` when typo correction is done just after the creation of 
the call expression.

This was unfortunately missed by the tests /:

To fix this, delay setting the number of arguments to `max(number of arguments, 
number of parameters in the prototype)` until we are ready for it. It would be 
nice to have this encapsulated in `CallExpr` but this is the best I can come up 
with under the constraint that we cannot add anything the `CallExpr`.

Fixes PR40286. This should probably be cherry-picked into the release branch.


Repository:
  rC Clang

https://reviews.llvm.org/D57948

Files:
  include/clang/AST/Expr.h
  lib/AST/Expr.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOverload.cpp
  test/Sema/typo-correction.c

Index: test/Sema/typo-correction.c
===
--- test/Sema/typo-correction.c
+++ test/Sema/typo-correction.c
@@ -100,3 +100,19 @@
   structVar1.fieldName1.member1, //expected-error{{use of undeclared identifier 'structVar1'}}
   structVar2.fieldName2.member2); //expected-error{{use of undeclared identifier 'structVar2'}}
 }
+
+void PR40286_g(int x, int y);
+void PR40286_h(int x, int y, int z);
+
+void PR40286_1(int the_value) {
+  PR40286_g(the_walue); // expected-error {{use of undeclared identifier 'the_walue'}}
+}
+void PR40286_2(int the_value) {
+  PR40286_h(the_value, the_walue); // expected-error {{use of undeclared identifier 'the_walue'}}
+}
+void PR40286_3(int the_value) {
+  PR40286_h(the_walue); // expected-error {{use of undeclared identifier 'the_walue'}}
+}
+void PR40286_4(int the_value) { // expected-note {{'the_value' declared here}}
+  PR40286_h(the_value, the_value, the_walue); // expected-error {{use of undeclared identifier 'the_walue'; did you mean 'the_value'?}}
+}
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -12906,6 +12906,8 @@
 call, nullptr))
   return ExprError();
 
+call->setNumArgsUnsafe(
+std::max(Args.size(), proto->getNumParams()));
 if (ConvertArgumentsForCall(call, op, nullptr, proto, Args, RParenLoc))
   return ExprError();
 
@@ -13073,6 +13075,8 @@
   }
 
   // Convert the rest of the arguments
+  TheCall->setNumArgsUnsafe(
+  std::max(Args.size(), Proto->getNumParams()));
   if (ConvertArgumentsForCall(TheCall, MemExpr, Method, Proto, Args,
   RParenLoc))
 return ExprError();
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -5799,12 +5799,25 @@
 // dealt with.
 ExprResult Result = CorrectDelayedTyposInExpr(TheCall);
 if (!Result.isUsable()) return ExprError();
+CallExpr *TheOldCall = TheCall;
 TheCall = dyn_cast(Result.get());
+bool CorrectedTypos = TheCall != TheOldCall;
 if (!TheCall) return Result;
-// TheCall at this point has max(Args.size(), NumParams) arguments,
-// with extra arguments nulled. We don't want to introduce nulled
-// arguments in Args and so we only take the first Args.size() arguments.
-Args = llvm::makeArrayRef(TheCall->getArgs(), Args.size());
+Args = llvm::makeArrayRef(TheCall->getArgs(), TheCall->getNumArgs());
+
+// A new call expression node was created if some typos were corrected.
+// However it may not have been constructed with enough storage. In this
+// case rebuild the node with enough storage. The waste of space is
+// immaterial since this only happens when some typos were corrected.
+if (CorrectedTypos && Args.size() < NumParams) {
+  if (Config)
+TheCall = CUDAKernelCallExpr::Create(
+Context, Fn, cast(Config), Args, ResultTy, VK_RValue,
+RParenLoc, NumParams);
+  else
+TheCall = CallExpr::Create(Context, Fn, Args, ResultTy, VK_RValue,
+   RParenLoc, NumParams, UsesADL);
+}
   }
 
   // Bail out early if calling a builtin with custom type checking.
@@ -5840,6 +5853,10 @@
   TheCall->setValueKind(Expr::getValueKindForType(FuncT->getReturnType()));
 
   if (Proto) {
+// We allocated space for max(Args.size(), NumParams) arguments in TheCall.
+// However to avoid exposing null arguments during construction the number
+// of arguments in TheCall was set to Args.size().
+TheCall->setNumArgsUnsafe(std::max(Args.size(), NumParams));
  

[PATCH] D57893: [analyzer] Fix function macro crash

[Kristóf Umann via Phabricator via cfe-commits]

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

Awesome, thanks! Will commit around next Friday.


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

https://reviews.llvm.org/D57893



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

[PATCH] D57267: [AST] Factor out the logic of the various Expr::Ignore*

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added subscribers: rnk, void.
riccibruno added a comment.

@void @rnk Perhaps you can comment on this: currently `Expr::IgnoreImpCasts` 
skips `FullExpr`s, but `Expr::IgnoreParenImpCasts` only skips (via 
`IgnoreParens`) `ConstantExpr`s. Is there any reason for this inconsistency ? I 
would like to add `FullExpr` to the nodes skipped by `IgnoreParenImpCasts` for 
consistency but I am worried about unexpected issues even though all tests pass.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57267



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

[PATCH] D57949: [clang][Index] Add a knob to index function parameters in declarations

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Parameters in declarations are useful for clangd, so that we can
provide symbol information for them as well. It also helps clangd to be
consistent whether a function's definition is accessible or not.


Repository:
  rC Clang

https://reviews.llvm.org/D57949

Files:
  include/clang/Index/IndexingAction.h
  lib/Index/IndexDecl.cpp
  lib/Index/IndexingContext.cpp
  lib/Index/IndexingContext.h
  unittests/Index/IndexTests.cpp


Index: unittests/Index/IndexTests.cpp
===
--- unittests/Index/IndexTests.cpp
+++ unittests/Index/IndexTests.cpp
@@ -119,6 +119,21 @@
   EXPECT_THAT(Index->Symbols, UnorderedElementsAre());
 }
 
+TEST(IndexTest, IndexParametersInDecls) {
+  std::string Code = "void foo(int bar);";
+  auto Index = std::make_shared();
+  IndexingOptions Opts;
+  Opts.IndexFunctionLocals = true;
+  Opts.IndexParametersInDeclarations = true;
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols, Contains(QName("bar")));
+
+  Opts.IndexParametersInDeclarations = false;
+  Index->Symbols.clear();
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols, Not(Contains(QName("bar";
+}
+
 } // namespace
 } // namespace index
 } // namespace clang
Index: lib/Index/IndexingContext.h
===
--- lib/Index/IndexingContext.h
+++ lib/Index/IndexingContext.h
@@ -61,6 +61,8 @@
 
   bool shouldIndexImplicitInstantiation() const;
 
+  bool shouldIndexParametersInDeclarations() const;
+
   static bool isTemplateImplicitInstantiation(const Decl *D);
 
   bool handleDecl(const Decl *D, SymbolRoleSet Roles = SymbolRoleSet(),
Index: lib/Index/IndexingContext.cpp
===
--- lib/Index/IndexingContext.cpp
+++ lib/Index/IndexingContext.cpp
@@ -40,6 +40,10 @@
   return IndexOpts.IndexImplicitInstantiation;
 }
 
+bool IndexingContext::shouldIndexParametersInDeclarations() const {
+  return IndexOpts.IndexParametersInDeclarations;
+}
+
 bool IndexingContext::handleDecl(const Decl *D,
  SymbolRoleSet Roles,
  ArrayRef Relations) {
Index: lib/Index/IndexDecl.cpp
===
--- lib/Index/IndexDecl.cpp
+++ lib/Index/IndexDecl.cpp
@@ -93,7 +93,8 @@
   if (const ParmVarDecl *Parm = dyn_cast(D)) {
 auto *DC = Parm->getDeclContext();
 if (auto *FD = dyn_cast(DC)) {
-  if (FD->isThisDeclarationADefinition())
+  if (IndexCtx.shouldIndexParametersInDeclarations() ||
+  FD->isThisDeclarationADefinition())
 IndexCtx.handleDecl(Parm);
 } else if (auto *MD = dyn_cast(DC)) {
   if (MD->isThisDeclarationADefinition())
@@ -102,7 +103,8 @@
   IndexCtx.handleDecl(Parm);
 }
   } else if (const FunctionDecl *FD = dyn_cast(D)) {
-if (FD->isThisDeclarationADefinition()) {
+if (IndexCtx.shouldIndexParametersInDeclarations() ||
+FD->isThisDeclarationADefinition()) {
   for (auto PI : FD->parameters()) {
 IndexCtx.handleDecl(PI);
   }
Index: include/clang/Index/IndexingAction.h
===
--- include/clang/Index/IndexingAction.h
+++ include/clang/Index/IndexingAction.h
@@ -44,6 +44,8 @@
   // callback is not available (e.g. after parsing has finished). Note that
   // macro references are not available in Proprocessor.
   bool IndexMacrosInPreprocessor = false;
+  // Has no effect if IndexFunctionLocals are false.
+  bool IndexParametersInDeclarations = false;
 };
 
 /// Creates a frontend action that indexes all symbols (macros and AST decls).


Index: unittests/Index/IndexTests.cpp
===
--- unittests/Index/IndexTests.cpp
+++ unittests/Index/IndexTests.cpp
@@ -119,6 +119,21 @@
   EXPECT_THAT(Index->Symbols, UnorderedElementsAre());
 }
 
+TEST(IndexTest, IndexParametersInDecls) {
+  std::string Code = "void foo(int bar);";
+  auto Index = std::make_shared();
+  IndexingOptions Opts;
+  Opts.IndexFunctionLocals = true;
+  Opts.IndexParametersInDeclarations = true;
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols, Contains(QName("bar")));
+
+  Opts.IndexParametersInDeclarations = false;
+  Index->Symbols.clear();
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols, Not(Contains(QName("bar";
+}
+
 } // namespace
 } // namespace index
 } // namespace clang
Index: lib/Index/IndexingContext.h
===

[PATCH] D57950: [clangd] Index parameters in function decls

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D57950

Files:
  clangd/XRefs.cpp
  unittests/clangd/SymbolInfoTests.cpp


Index: unittests/clangd/SymbolInfoTests.cpp
===
--- unittests/clangd/SymbolInfoTests.cpp
+++ unittests/clangd/SymbolInfoTests.cpp
@@ -301,6 +301,12 @@
   }
 )cpp",
   {CreateExpectedSymbolDetails("bar", "foo::", "c:@E@foo@bar")}},
+  {
+  R"cpp( // Parameters in declarations
+  void foo(int ba^r);
+)cpp",
+  {CreateExpectedSymbolDetails("bar", "foo",
+   "c:TestTU.cpp@50@F@foo#I#@bar")}},
   {
   R"cpp( // Type inferrence with auto keyword
   struct foo {};
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -215,6 +215,7 @@
   IndexOpts.SystemSymbolFilter =
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
+  IndexOpts.IndexParametersInDeclarations = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
  AST.getLocalTopLevelDecls(), DeclMacrosFinder, IndexOpts);
 


Index: unittests/clangd/SymbolInfoTests.cpp
===
--- unittests/clangd/SymbolInfoTests.cpp
+++ unittests/clangd/SymbolInfoTests.cpp
@@ -301,6 +301,12 @@
   }
 )cpp",
   {CreateExpectedSymbolDetails("bar", "foo::", "c:@E@foo@bar")}},
+  {
+  R"cpp( // Parameters in declarations
+  void foo(int ba^r);
+)cpp",
+  {CreateExpectedSymbolDetails("bar", "foo",
+   "c:TestTU.cpp@50@F@foo#I#@bar")}},
   {
   R"cpp( // Type inferrence with auto keyword
   struct foo {};
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -215,6 +215,7 @@
   IndexOpts.SystemSymbolFilter =
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
+  IndexOpts.IndexParametersInDeclarations = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
  AST.getLocalTopLevelDecls(), DeclMacrosFinder, IndexOpts);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57950: [clangd] Index parameters in function decls

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet planned changes to this revision.
kadircet added a comment.

Also enable `IndexParametersInDeclarations` in symbolcollector


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57950



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

[PATCH] D54978: Move the SMT API to LLVM

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D54978#1389260 , @shakeel.mahate 
wrote:

> The ninja build also fails


That's not the ninja build, that's the GN build. If you use that, please read 
llvm/utils/gn/README.rst. If something like this happens, it's on you (and 
other people using then gn build) to fix this. If you're not comfortable with 
that, please don't use the GN build. (I just merged this to the GN build in 
r353518, so syncing will make things go again for you.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



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

[PATCH] D57949: [clang][Index] Add a knob to index function parameters in declarations

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 185962.
kadircet added a comment.

- Also delete comment


Repository:
  rC Clang

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

https://reviews.llvm.org/D57949

Files:
  include/clang/Index/IndexingAction.h
  lib/Index/IndexDecl.cpp
  lib/Index/IndexingContext.cpp
  lib/Index/IndexingContext.h
  unittests/Index/IndexTests.cpp


Index: unittests/Index/IndexTests.cpp
===
--- unittests/Index/IndexTests.cpp
+++ unittests/Index/IndexTests.cpp
@@ -119,6 +119,21 @@
   EXPECT_THAT(Index->Symbols, UnorderedElementsAre());
 }
 
+TEST(IndexTest, IndexParametersInDecls) {
+  std::string Code = "void foo(int bar);";
+  auto Index = std::make_shared();
+  IndexingOptions Opts;
+  Opts.IndexFunctionLocals = true;
+  Opts.IndexParametersInDeclarations = true;
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols, Contains(QName("bar")));
+
+  Opts.IndexParametersInDeclarations = false;
+  Index->Symbols.clear();
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols, Not(Contains(QName("bar";
+}
+
 } // namespace
 } // namespace index
 } // namespace clang
Index: lib/Index/IndexingContext.h
===
--- lib/Index/IndexingContext.h
+++ lib/Index/IndexingContext.h
@@ -61,6 +61,8 @@
 
   bool shouldIndexImplicitInstantiation() const;
 
+  bool shouldIndexParametersInDeclarations() const;
+
   static bool isTemplateImplicitInstantiation(const Decl *D);
 
   bool handleDecl(const Decl *D, SymbolRoleSet Roles = SymbolRoleSet(),
Index: lib/Index/IndexingContext.cpp
===
--- lib/Index/IndexingContext.cpp
+++ lib/Index/IndexingContext.cpp
@@ -40,6 +40,10 @@
   return IndexOpts.IndexImplicitInstantiation;
 }
 
+bool IndexingContext::shouldIndexParametersInDeclarations() const {
+  return IndexOpts.IndexParametersInDeclarations;
+}
+
 bool IndexingContext::handleDecl(const Decl *D,
  SymbolRoleSet Roles,
  ArrayRef Relations) {
Index: lib/Index/IndexDecl.cpp
===
--- lib/Index/IndexDecl.cpp
+++ lib/Index/IndexDecl.cpp
@@ -88,12 +88,11 @@
  /*isBase=*/false, isIBType);
 IndexCtx.indexNestedNameSpecifierLoc(D->getQualifierLoc(), Parent);
 if (IndexCtx.shouldIndexFunctionLocalSymbols()) {
-  // Only index parameters in definitions, parameters in declarations are
-  // not useful.
   if (const ParmVarDecl *Parm = dyn_cast(D)) {
 auto *DC = Parm->getDeclContext();
 if (auto *FD = dyn_cast(DC)) {
-  if (FD->isThisDeclarationADefinition())
+  if (IndexCtx.shouldIndexParametersInDeclarations() ||
+  FD->isThisDeclarationADefinition())
 IndexCtx.handleDecl(Parm);
 } else if (auto *MD = dyn_cast(DC)) {
   if (MD->isThisDeclarationADefinition())
@@ -102,7 +101,8 @@
   IndexCtx.handleDecl(Parm);
 }
   } else if (const FunctionDecl *FD = dyn_cast(D)) {
-if (FD->isThisDeclarationADefinition()) {
+if (IndexCtx.shouldIndexParametersInDeclarations() ||
+FD->isThisDeclarationADefinition()) {
   for (auto PI : FD->parameters()) {
 IndexCtx.handleDecl(PI);
   }
Index: include/clang/Index/IndexingAction.h
===
--- include/clang/Index/IndexingAction.h
+++ include/clang/Index/IndexingAction.h
@@ -44,6 +44,8 @@
   // callback is not available (e.g. after parsing has finished). Note that
   // macro references are not available in Proprocessor.
   bool IndexMacrosInPreprocessor = false;
+  // Has no effect if IndexFunctionLocals are false.
+  bool IndexParametersInDeclarations = false;
 };
 
 /// Creates a frontend action that indexes all symbols (macros and AST decls).


Index: unittests/Index/IndexTests.cpp
===
--- unittests/Index/IndexTests.cpp
+++ unittests/Index/IndexTests.cpp
@@ -119,6 +119,21 @@
   EXPECT_THAT(Index->Symbols, UnorderedElementsAre());
 }
 
+TEST(IndexTest, IndexParametersInDecls) {
+  std::string Code = "void foo(int bar);";
+  auto Index = std::make_shared();
+  IndexingOptions Opts;
+  Opts.IndexFunctionLocals = true;
+  Opts.IndexParametersInDeclarations = true;
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols, Contains(QName("bar")));
+
+  Opts.IndexParametersInDeclarations = false;
+  Index->Symbols.clear();
+  tooling::runToolOnCode(new IndexAction(Index, Opts), Code);
+  EXPECT_THAT(Index->Symbols, Not(Contains(QName("bar";
+}
+
 } // namespace
 } // namespace index
 } // namespace clang
Index: lib/Index/IndexingCo

[PATCH] D54978: Move the SMT API to LLVM

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D54978#1389110 , @brzycki wrote:

> This commit is causing a build-break for our nightly cross compilers of arm 
> and aarch64. The immediately preceding commit of D54977 
>  does not break with the exact same 
> invocation.
>
> The problem is our build machine (Ubuntu 18.04 LTS) installs an old version 
> of Z3 that is incompatible with LLVM's tip of tree. To deal with this we add 
> `-D CLANG_ANALYZER_ENABLE_Z3_SOLVER=OFF` to force disabling Z3 even if CMake 
> detects the library is installed.
>
>   With this patch I am unable to disable Z3 because 
> `CLANG_ANALYZER_ENABLE_Z3_SOLVER` is no longer in the codebase and setting 
> `-D LLVM_ENABLE_Z3_SOLVER=OFF` still builds Z3 support (which fails):
>
>   [74/187] Building CXX object 
> lib/Support/CMakeFiles/LLVMSupport.dir/Z3Solver.cpp.o
>   FAILED: lib/Support/CMakeFiles/LLVMSupport.dir/Z3Solver.cpp.o
>   /usr/bin/clang++-6.0  -DGTEST_HAS_RTTI=0 -D_GNU_SOURCE 
> -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
> -Ilib/Support -I/tmp/tmp.UCDOJjmgJ1/src/llvm/lib/Support 
> -I/usr/include/libxml2 -Iinclude -I/tmp/tmp.UCDOJjmgJ1/src/llvm/include -fPIC 
> -fvisibility-inlines-hidden -Werror=date-time 
> -Werror=unguarded-availability-new -std=c++11 -Wall -Wextra 
> -Wno-unused-parameter -Wwrite-strings -Wcast-qual 
> -Wmissing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough 
> -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
> -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color 
> -ffunction-sections -fdata-sections -O3 -DNDEBUG-fno-exceptions -fno-rtti 
> -MD -MT lib/Support/CMakeFiles/LLVMSupport.dir/Z3Solver.cpp.o -MF 
> lib/Support/CMakeFiles/LLVMSupport.dir/Z3Solver.cpp.o.d -o 
> lib/Support/CMakeFiles/LLVMSupport.dir/Z3Solver.cpp.o -c 
> /tmp/tmp.UCDOJjmgJ1/src/llvm/lib/Support/Z3Solver.cpp
>   /tmp/tmp.UCDOJjmgJ1/src/llvm/lib/Support/Z3Solver.cpp:44:40: error: no 
> matching function for call to 'Z3_get_error_msg'
>  llvm::Twine(Z3_get_error_msg(Context, Error)));
>  ^~~~
>


From what I understand, setting `-DLLVM_ENABLE_Z3_SOLVER=OFF` is supposed to 
work – the contents of Z3Solver.cpp are wrapped in a `#if LLVM_WITH_Z3`, and 
the logic in the llvm cmake file looks identical to the logic in the clang 
cmake file previously. Can you maybe debug a bit to see what goes wrong on your 
bot?

lebedev.ri's point about this not being reviewed properly is a good one though.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



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

[PATCH] D57949: [clang][Index] Add a knob to index function parameters in declarations

[Haojian Wu via Phabricator via cfe-commits]

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

The change looks reasonable, I'd wait for a few days before commit in case 
anyone has concerns.




Comment at: include/clang/Index/IndexingAction.h:47
   bool IndexMacrosInPreprocessor = false;
+  // Has no effect if IndexFunctionLocals are false.
+  bool IndexParametersInDeclarations = false;

nit: Only meaningful when IndexFunctionLocals is true?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57949



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

[PATCH] D57896: Variable names rule

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

> I personally would be happy to change the settings from `camelBack` to 
> `aNy_CasE`.

Should we come up with a new style?  UpperOrLowerCamelCase,   I don't mind 
going and doing that in the readability-identifier-naming check, given that I 
just wrote up all the Options for that check 
https://clang.llvm.org/extra/clang-tidy/checks/readability-identifier-naming.html
 in D56563: [clang-tidy]  add options documentation to 
readability-identifier-naming checker 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57896



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

[PATCH] D57127: [analyzer] Port RetainSummaryManager to the new GenericCall interface, decouple ARCMT from the analyzer

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.
Herald added a project: clang.

Did you see the "Should we remove this check that currently requires 
ENABLE_ARCMT => ENABLE_SA? 
http://llvm-cs.pcc.me.uk/tools/clang/CMakeLists.txt#441"; question?

In D57127#1373949 , @george.karpenkov 
wrote:

> @thakis Sorry I don't understand your point. At the end of the day, 
> ProgramPoint and BodyFarm are only used by the static analyzer.


They are used by StaticAnalyzer/Core, which clang does depend on.

> If you disagree, what is your proposed directory structure? What would be the 
> benefits?

I'd put this in a library that isn't linked into clang. The benefit is that 
it's easier to keep clang itself small(er) that way.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57127



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

[PATCH] D57951: [Lex] Allow to set missing include error to not fatal

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan created this revision.
yvvan added reviewers: ilya-biryukov, nik.

For IDE it's a nice feature to not get a fatal error and continue preprocessing 
even if the file is missing.
This allows include further files and get more relevant information about the 
current translation unit.


https://reviews.llvm.org/D57951

Files:
  include/clang/Basic/Diagnostic.h
  include/clang/Basic/DiagnosticLexKinds.td
  include/clang/Lex/PreprocessorOptions.h
  lib/Frontend/ASTUnit.cpp
  lib/Lex/PPDirectives.cpp
  lib/Lex/Pragma.cpp


Index: lib/Lex/Pragma.cpp
===
--- lib/Lex/Pragma.cpp
+++ lib/Lex/Pragma.cpp
@@ -29,6 +29,7 @@
 #include "clang/Lex/ModuleLoader.h"
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Lex/PreprocessorLexer.h"
 #include "clang/Lex/Token.h"
 #include "clang/Lex/TokenLexer.h"
@@ -508,8 +509,12 @@
   LookupFile(FilenameTok.getLocation(), Filename, isAngled, nullptr,
  nullptr, CurDir, nullptr, nullptr, nullptr, nullptr, nullptr);
   if (!File) {
-if (!SuppressIncludeNotFoundError)
-  Diag(FilenameTok, diag::err_pp_file_not_found) << Filename;
+if (!SuppressIncludeNotFoundError) {
+  Diag(FilenameTok, PPOpts->KeepGoingAfterMissingInclude
+? diag::err_pp_file_not_found_keep_going
+: diag::err_pp_file_not_found)
+  << Filename;
+}
 return;
   }
 
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1866,8 +1866,10 @@
 
   // If the file is still not found, just go with the vanilla diagnostic
   if (!File) {
-Diag(FilenameTok, diag::err_pp_file_not_found) << OriginalFilename
-   << FilenameRange;
+Diag(FilenameTok, PPOpts->KeepGoingAfterMissingInclude
+  ? diag::err_pp_file_not_found_keep_going
+  : diag::err_pp_file_not_found)
+<< OriginalFilename << FilenameRange;
 if (IsFrameworkFound) {
   size_t SlashPos = OriginalFilename.find('/');
   assert(SlashPos != StringRef::npos &&
Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -1735,6 +1735,7 @@
   PPOpts.RemappedFilesKeepOriginalName = RemappedFilesKeepOriginalName;
   PPOpts.AllowPCHWithCompilerErrors = AllowPCHWithCompilerErrors;
   PPOpts.SingleFileParseMode = SingleFileParse;
+  PPOpts.KeepGoingAfterMissingInclude = !Diags->getSuppressAfterFatalError();
 
   // Override the resources path.
   CI->getHeaderSearchOpts().ResourceDir = ResourceFilesPath;
Index: include/clang/Lex/PreprocessorOptions.h
===
--- include/clang/Lex/PreprocessorOptions.h
+++ include/clang/Lex/PreprocessorOptions.h
@@ -142,6 +142,12 @@
   /// compiler invocation and its buffers will be reused.
   bool RetainRemappedFileBuffers = false;
 
+  /// Whether to emit normal error instead of fatal in case the include is
+  /// missing. This should help to provide more meaningful information for the
+  /// code that uses data from the following headers which are ignored
+  /// otherwise.
+  bool KeepGoingAfterMissingInclude = false;
+
   /// The Objective-C++ ARC standard library that we should support,
   /// by providing appropriate definitions to retrofit the standard library
   /// with support for lifetime-qualified pointers.
Index: include/clang/Basic/DiagnosticLexKinds.td
===
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -399,6 +399,7 @@
 def err_pp_directive_required : Error<
   "%0 must be used within a preprocessing directive">;
 def err_pp_file_not_found : Error<"'%0' file not found">, DefaultFatal;
+def err_pp_file_not_found_keep_going : Error<"'%0' file not found">;
 def err_pp_through_header_not_found : Error<
   "'%0' required for precompiled header not found">, DefaultFatal;
 def err_pp_through_header_not_seen : Error<
Index: include/clang/Basic/Diagnostic.h
===
--- include/clang/Basic/Diagnostic.h
+++ include/clang/Basic/Diagnostic.h
@@ -618,6 +618,8 @@
   /// a fatal error.
   void setSuppressAfterFatalError(bool Val) { SuppressAfterFatalError = Val; }
 
+  bool getSuppressAfterFatalError() { return SuppressAfterFatalError; }
+
   /// When set to true mask warnings that come from system headers.
   void setSuppressSystemWarnings(bool Val) {
 GetCurDiagState()->SuppressSystemWarnings = Val;


Index: lib/Lex/Pragma.cpp
===
--- lib

[PATCH] D57951: [Lex] Allow to set missing include error to not fatal

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

I've tested it with libclang. If something else is needed for proper clangd 
args forwarding - let me know.


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

https://reviews.llvm.org/D57951



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

[PATCH] D57063: [CMake] Unify scripts for generating VCS headers

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

@phosek, I'm sorry to say but this is apparently breaking stand-alone clang 
builds:

  FAILED: lib/Basic/VCSVersion.inc
  cd 
/var/tmp/portage/sys-devel/clang-/work/x/y/clang--abi_x86_64.amd64/lib/Basic
 && /usr/bin/cmake -DNAMES="LLVM;CLANG" -DLLVM_SOURCE_DIR= 
-DCLANG_SOURCE_DIR=/var/tmp/portage/sys-devel/clang-/work/x/y/clang- 
-DHEADER_FILE=/var/tmp/portage/sys-devel/clang-/work/x/y/clang--abi_x86_64.amd64/lib/Basic/VCSVersion.inc
 -P /usr/lib64/llvm/9/lib64/cmake/llvm/GenerateVersionFromVCS.cmake
  CMake Error at 
/usr/lib64/llvm/9/lib64/cmake/llvm/GenerateVersionFromVCS.cmake:18 (include):
include could not find load file:
  
  VersionFromVCS


Repository:
  rC Clang

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

https://reviews.llvm.org/D57063



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

[PATCH] D57063: [CMake] Unify scripts for generating VCS headers

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

(apparently because the `CMAKE_MODULE_PATH` logic relies on going backwards to 
find `cmake/modules` and does not account that the modules will actually be in 
the same directory)


Repository:
  rC Clang

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

https://reviews.llvm.org/D57063



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

[clang-tools-extra] r353523 - [clang-tidy][NFC] Test commit. Add missing comma.

[Yitzhak Mandelbaum via cfe-commits]

Author: ymandel
Date: Fri Feb  8 06:57:22 2019
New Revision: 353523

URL: http://llvm.org/viewvc/llvm-project?rev=353523&view=rev
Log:
[clang-tidy][NFC] Test commit. Add missing comma.

Modified:
clang-tools-extra/trunk/clang-tidy/readability/ConstReturnTypeCheck.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/readability/ConstReturnTypeCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/ConstReturnTypeCheck.cpp?rev=353523&r1=353522&r2=353523&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/ConstReturnTypeCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/ConstReturnTypeCheck.cpp Fri 
Feb  8 06:57:22 2019
@@ -22,8 +22,8 @@ namespace readability {
 
 // Finds the location of the qualifying `const` token in the `FunctionDecl`'s
 // return type. Returns `None` when the return type is not `const`-qualified or
-// `const` does not appear in `Def`'s source like when the type is an alias or 
a
-// macro.
+// `const` does not appear in `Def`'s source like, when the type is an alias or
+// a macro.
 static llvm::Optional
 findConstToRemove(const FunctionDecl *Def,
   const MatchFinder::MatchResult &Result) {


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

[clang-tools-extra] r353525 - [clang-tidy][NFC] Fix typo.

[Yitzhak Mandelbaum via cfe-commits]

Author: ymandel
Date: Fri Feb  8 07:05:57 2019
New Revision: 353525

URL: http://llvm.org/viewvc/llvm-project?rev=353525&view=rev
Log:
[clang-tidy][NFC] Fix typo.

Fix placement of comma from previous (test) commit.

Modified:
clang-tools-extra/trunk/clang-tidy/readability/ConstReturnTypeCheck.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/readability/ConstReturnTypeCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/ConstReturnTypeCheck.cpp?rev=353525&r1=353524&r2=353525&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/ConstReturnTypeCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/ConstReturnTypeCheck.cpp Fri 
Feb  8 07:05:57 2019
@@ -22,7 +22,7 @@ namespace readability {
 
 // Finds the location of the qualifying `const` token in the `FunctionDecl`'s
 // return type. Returns `None` when the return type is not `const`-qualified or
-// `const` does not appear in `Def`'s source like, when the type is an alias or
+// `const` does not appear in `Def`'s source, like when the type is an alias or
 // a macro.
 static llvm::Optional
 findConstToRemove(const FunctionDecl *Def,


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

[PATCH] D57946: [libclang] Add attribute support for 'convergent'.

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D57946



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

[PATCH] D57950: [clangd] Index parameters in function decls

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 185967.
kadircet added a comment.
This revision is now accepted and ready to land.

Also index function params in findrefs


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57950

Files:
  clangd/XRefs.cpp
  unittests/clangd/SymbolInfoTests.cpp
  unittests/clangd/XRefsTests.cpp


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -86,6 +86,13 @@
   auto *X = &[[foo]];
 }
   )cpp",
+
+  R"cpp(// Function parameter
+void foo(int [[bar]]);
+void foo(int [[bar]])) {
+  [[b^ar]] = 5;
+}
+  )cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);
Index: unittests/clangd/SymbolInfoTests.cpp
===
--- unittests/clangd/SymbolInfoTests.cpp
+++ unittests/clangd/SymbolInfoTests.cpp
@@ -301,6 +301,12 @@
   }
 )cpp",
   {CreateExpectedSymbolDetails("bar", "foo::", "c:@E@foo@bar")}},
+  {
+  R"cpp( // Parameters in declarations
+  void foo(int ba^r);
+)cpp",
+  {CreateExpectedSymbolDetails("bar", "foo",
+   "c:TestTU.cpp@50@F@foo#I#@bar")}},
   {
   R"cpp( // Type inferrence with auto keyword
   struct foo {};
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -215,6 +215,7 @@
   IndexOpts.SystemSymbolFilter =
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
+  IndexOpts.IndexParametersInDeclarations = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
  AST.getLocalTopLevelDecls(), DeclMacrosFinder, IndexOpts);
 
@@ -400,6 +401,7 @@
   IndexOpts.SystemSymbolFilter =
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
+  IndexOpts.IndexParametersInDeclarations = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
  AST.getLocalTopLevelDecls(), RefFinder, IndexOpts);
   return std::move(RefFinder).take();


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -86,6 +86,13 @@
   auto *X = &[[foo]];
 }
   )cpp",
+
+  R"cpp(// Function parameter
+void foo(int [[bar]]);
+void foo(int [[bar]])) {
+  [[b^ar]] = 5;
+}
+  )cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);
Index: unittests/clangd/SymbolInfoTests.cpp
===
--- unittests/clangd/SymbolInfoTests.cpp
+++ unittests/clangd/SymbolInfoTests.cpp
@@ -301,6 +301,12 @@
   }
 )cpp",
   {CreateExpectedSymbolDetails("bar", "foo::", "c:@E@foo@bar")}},
+  {
+  R"cpp( // Parameters in declarations
+  void foo(int ba^r);
+)cpp",
+  {CreateExpectedSymbolDetails("bar", "foo",
+   "c:TestTU.cpp@50@F@foo#I#@bar")}},
   {
   R"cpp( // Type inferrence with auto keyword
   struct foo {};
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -215,6 +215,7 @@
   IndexOpts.SystemSymbolFilter =
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
+  IndexOpts.IndexParametersInDeclarations = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
  AST.getLocalTopLevelDecls(), DeclMacrosFinder, IndexOpts);
 
@@ -400,6 +401,7 @@
   IndexOpts.SystemSymbolFilter =
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
+  IndexOpts.IndexParametersInDeclarations = true;
   indexTopLevelDecls(AST.getASTContext(), AST.getPreprocessor(),
  AST.getLocalTopLevelDecls(), RefFinder, IndexOpts);
   return std::move(RefFinder).take();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57906: [CTU] Do not allow different CPP dialects in CTU

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

> Consider this STL code:
> 
>   template
> struct __alloc_traits
>   #if __cplusplus >= 201103L
> : std::allocator_traits<_Alloc>
>   #endif
> { // ...
> };
> 
> 
> This class template would create ODR errors during merging the two units,
>  since in one translation unit the class template has a base class, however
>  in the other unit it has none.

How is `#if __cplusplus >= 201103L` qualitatively different from `#ifndef 
NDEBUG` or `#if MYLIB_ABI_VERSION==2` or `#if __DATE__ == "2018-04-01"`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57906



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

[PATCH] D57906: [CTU] Do not allow different CPP dialects in CTU

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added inline comments.



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:255
+  // in the other unit it has none.
+  if (LangTo.CPlusPlus11 != LangFrom.CPlusPlus11 ||
+  LangTo.CPlusPlus14 != LangFrom.CPlusPlus14 ||

This is likely to become a bug in the future, but I didn't see a better way to 
compare dialects.

Is there a way to add a test that lights up once there is a new dialect?

Would it be too pessimistic to compare the entire LangOpts? Some stuff in there 
should even still produce errors, right? For example "GnuMode". I skimmed over 
them and didn't find an obvious one that would differ between translation units 
of the same project. Maybe the template depth could be set selectively, but to 
fix that we could mask "COMPATIBLE_" and "BENIGN_" opts.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57906



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

[PATCH] D57893: [analyzer] Fix function macro crash

[Tibor Brunner via Phabricator via cfe-commits]

bruntib updated this revision to Diff 185971.
bruntib added a comment.

There was another place where this crash could have happened.


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

https://reviews.llvm.org/D57893

Files:
  lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
  test/Analysis/plist-macros-with-expansion.cpp

Index: test/Analysis/plist-macros-with-expansion.cpp
===
--- test/Analysis/plist-macros-with-expansion.cpp
+++ test/Analysis/plist-macros-with-expansion.cpp
@@ -441,3 +441,17 @@
 }
 // CHECK: nameYET_ANOTHER_SET_TO_NULL
 // CHECK-NEXT: expansionprint((void *)5); print((void *)"Remember the Vasa"); ptr = nullptr;
+
+#define FOO(x) int foo() { return x; }
+#define APPLY_ZERO1(function) function(0)
+#define BAR(x) int bar() { return x; }
+#define APPLY_ZERO2 BAR(0)
+APPLY_ZERO1(FOO)
+APPLY_ZERO2
+void useZeroApplier1() { (void)(1 / foo()); } // expected-warning{{Division by zero}}
+void useZeroApplier2() { (void)(1 / bar()); } // expected-warning{{Division by zero}}
+
+// CHECK: nameAPPLY_ZERO1
+// CHECK-NEXT: expansionint foo() { return x; }(0)
+// CHECK: nameAPPLY_ZERO2
+// CHECK-NEXT: expansionint bar() { return 0; }
Index: test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
===
--- test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
+++ test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
@@ -5168,6 +5168,468 @@
file0
   
   
+  
+   path
+   
+
+ kindcontrol
+ edges
+  
+   
+start
+ 
+  
+   line451
+   col33
+   file0
+  
+  
+   line451
+   col33
+   file0
+  
+ 
+end
+ 
+  
+   line451
+   col37
+   file0
+  
+  
+   line451
+   col39
+   file0
+  
+ 
+   
+  
+
+
+ kindevent
+ location
+ 
+  line451
+  col37
+  file0
+ 
+ ranges
+ 
+   
+
+ line451
+ col37
+ file0
+
+
+ line451
+ col41
+ file0
+
+   
+ 
+ depth0
+ extended_message
+ Calling 'foo'
+ message
+ Calling 'foo'
+
+
+ kindevent
+ location
+ 
+  line449
+  col1
+  file0
+ 
+ depth1
+ extended_message
+ Entered call from 'useZeroApplier1'
+ message
+ Entered call from 'useZeroApplier1'
+
+
+ kindevent
+ location
+ 
+  line449
+  col1
+  file0
+ 
+ ranges
+ 
+   
+
+ line449
+ col1
+ file0
+
+
+ line449
+ col16
+ file0
+
+   
+ 
+ depth1
+ extended_message
+ Returning zero
+ message
+ Returning zero
+
+
+ kindevent
+ location
+ 
+  line451
+  col37
+  file0
+ 
+ ranges
+ 
+   
+
+ line451
+ col37
+ file0
+
+
+ line451
+ col41
+ file0
+
+   
+ 
+ depth0
+ extended_message
+ Returning from 'foo'
+ message
+ Returning from 'foo'
+
+
+ kindcontrol
+ edges
+  
+   
+start
+ 
+  
+   line451
+   col37
+   file0
+  
+  
+   line451
+   col39
+   file0
+  
+ 
+end
+ 
+  
+   line451
+   col35
+   file0
+  
+  
+   line451
+   col35
+   file0
+  
+ 
+   
+  
+
+
+ kindevent
+ location
+ 
+  line451
+  col35
+  file0
+ 
+ ranges
+ 
+   
+
+ line451
+ col33
+ file0
+
+
+ line451
+ col41
+ file0
+
+   
+ 
+ depth0
+ extended_message
+ Division by zero
+ message
+ Division by zero
+
+   
+   macro_expansions
+   
+
+ location
+ 
+  line449
+  col1
+  file0
+ 
+ nameAPPLY_ZERO1
+ expansionint foo() { return x; }(0)
+
+   
+   descriptionDivision by zero
+   categoryLogic error
+   typeDivision by zero
+   check_namecore.DivideZero
+   
+   issue_hash_content_of_line_in_context7ff82561a6c752746649d05220deeb40
+  issue_context_kindfunction
+  issue_contextuseZeroApplier1
+  issue_hash_function_offset0
+  location
+  
+   line451
+   col35
+   file0
+  
+  
+  
+   path
+   
+
+ kindcontrol
+ edges
+  
+   
+start
+ 
+  
+   lin

[PATCH] D57923: [Format/ObjC] Fix [foo bar]->baz formatting as lambda arrow

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC353531: [Format/ObjC] Fix [foo bar]->baz formatting as 
lambda arrow (authored by benhamilton, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57923?vs=185859&id=185975#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D57923

Files:
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTestObjC.cpp


Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1426,6 +1426,9 @@
   nextToken();
   break;
 case tok::arrow:
+  // This might or might not actually be a lambda arrow (this could be an
+  // ObjC method invocation followed by a dereferencing arrow). We might
+  // reset this back to TT_Unknown in TokenAnnotator.
   FormatTok->Type = TT_LambdaArrow;
   nextToken();
   break;
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -520,6 +520,10 @@
   if (Parent && Parent->is(TT_PointerOrReference))
 Parent->Type = TT_BinaryOperator;
 }
+// An arrow after an ObjC method expression is not a lambda arrow.
+if (CurrentToken->Type == TT_ObjCMethodExpr && CurrentToken->Next &&
+CurrentToken->Next->is(TT_LambdaArrow))
+  CurrentToken->Next->Type = TT_Unknown;
 Left->MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = Left;
 // FirstObjCSelectorName is set when a colon is found. This does
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -611,6 +611,7 @@
 
 TEST_F(FormatTestObjC, FormatObjCMethodExpr) {
   verifyFormat("[foo bar:baz];");
+  verifyFormat("[foo bar]->baz;");
   verifyFormat("return [foo bar:baz];");
   verifyFormat("return (a)[foo bar:baz];");
   verifyFormat("f([foo bar:baz]);");


Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1426,6 +1426,9 @@
   nextToken();
   break;
 case tok::arrow:
+  // This might or might not actually be a lambda arrow (this could be an
+  // ObjC method invocation followed by a dereferencing arrow). We might
+  // reset this back to TT_Unknown in TokenAnnotator.
   FormatTok->Type = TT_LambdaArrow;
   nextToken();
   break;
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -520,6 +520,10 @@
   if (Parent && Parent->is(TT_PointerOrReference))
 Parent->Type = TT_BinaryOperator;
 }
+// An arrow after an ObjC method expression is not a lambda arrow.
+if (CurrentToken->Type == TT_ObjCMethodExpr && CurrentToken->Next &&
+CurrentToken->Next->is(TT_LambdaArrow))
+  CurrentToken->Next->Type = TT_Unknown;
 Left->MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = Left;
 // FirstObjCSelectorName is set when a colon is found. This does
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -611,6 +611,7 @@
 
 TEST_F(FormatTestObjC, FormatObjCMethodExpr) {
   verifyFormat("[foo bar:baz];");
+  verifyFormat("[foo bar]->baz;");
   verifyFormat("return [foo bar:baz];");
   verifyFormat("return (a)[foo bar:baz];");
   verifyFormat("f([foo bar:baz]);");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r353531 - [Format/ObjC] Fix [foo bar]->baz formatting as lambda arrow

[Ben Hamilton via cfe-commits]

Author: benhamilton
Date: Fri Feb  8 07:55:18 2019
New Revision: 353531

URL: http://llvm.org/viewvc/llvm-project?rev=353531&view=rev
Log:
[Format/ObjC] Fix [foo bar]->baz formatting as lambda arrow

Summary:
Currently, `UnwrappedLineParser` thinks an arrow token after
an ObjC method expression is a C++ lambda arrow, so it formats:

```
[foo bar]->baz
```

as:

```
[foo bar] -> baz
```

Because `UnwrappedLineParser` runs before `TokenAnnotator`, it can't
know if the arrow token is after an ObjC method expression or not.

This diff makes `TokenAnnotator` remove the TT_LambdaArrow on
the arrow token if it follows an ObjC method expression.

Test Plan: New test added. Ran test with:
  % ninja FormatTests && ./tools/clang/unittests/Format/FormatTests
  Confirmed test failed before diff and passed after diff.

Reviewers: krasimir, djasper, sammccall

Reviewed By: sammccall

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/unittests/Format/FormatTestObjC.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=353531&r1=353530&r2=353531&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Fri Feb  8 07:55:18 2019
@@ -520,6 +520,10 @@ private:
   if (Parent && Parent->is(TT_PointerOrReference))
 Parent->Type = TT_BinaryOperator;
 }
+// An arrow after an ObjC method expression is not a lambda arrow.
+if (CurrentToken->Type == TT_ObjCMethodExpr && CurrentToken->Next &&
+CurrentToken->Next->is(TT_LambdaArrow))
+  CurrentToken->Next->Type = TT_Unknown;
 Left->MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = Left;
 // FirstObjCSelectorName is set when a colon is found. This does

Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=353531&r1=353530&r2=353531&view=diff
==
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Fri Feb  8 07:55:18 2019
@@ -1426,6 +1426,9 @@ bool UnwrappedLineParser::tryToParseLamb
   nextToken();
   break;
 case tok::arrow:
+  // This might or might not actually be a lambda arrow (this could be an
+  // ObjC method invocation followed by a dereferencing arrow). We might
+  // reset this back to TT_Unknown in TokenAnnotator.
   FormatTok->Type = TT_LambdaArrow;
   nextToken();
   break;

Modified: cfe/trunk/unittests/Format/FormatTestObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestObjC.cpp?rev=353531&r1=353530&r2=353531&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp Fri Feb  8 07:55:18 2019
@@ -611,6 +611,7 @@ TEST_F(FormatTestObjC, FormatObjCMethodD
 
 TEST_F(FormatTestObjC, FormatObjCMethodExpr) {
   verifyFormat("[foo bar:baz];");
+  verifyFormat("[foo bar]->baz;");
   verifyFormat("return [foo bar:baz];");
   verifyFormat("return (a)[foo bar:baz];");
   verifyFormat("f([foo bar:baz]);");


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

[PATCH] D56849: [ASTMatchers][NFC] Update comments on assorted `CXXMemberCallExpr` matchers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 185977.
ymandel added a comment.

Sync with head.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D56849

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h

Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2886,14 +2886,22 @@
  InnerMatcher.matches(*UnderlyingDecl, Finder, Builder);
 }
 
-/// Matches on the implicit object argument of a member call expression.
+/// Matches on the implicit object argument of a member call expression, after
+/// stripping off any parentheses or implicit casts.
 ///
-/// Example matches y.x()
-///   (matcher = cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("Y"))
+/// Given
 /// \code
-///   class Y { public: void x(); };
-///   void z() { Y y; y.x(); }
+///   class Y { public: void m(); };
+///   Y g();
+///   class X : public Y {};
+///   void z(Y y, X x) { y.m(); (g()).m(); x.m(); }
 /// \endcode
+/// cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("Y")
+///   matches `y.m()` and `(g()).m()`.
+/// cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("X")
+///   matches `x.m()`.
+/// cxxMemberCallExpr(on(callExpr()))
+///   matches `(g()).m()`.
 ///
 /// FIXME: Overload to allow directly matching types?
 AST_MATCHER_P(CXXMemberCallExpr, on, internal::Matcher,
@@ -3253,6 +3261,23 @@
   .matches(Node, Finder, Builder);
 }
 
+/// Matches on the implicit object argument of a member call expression. Unlike
+/// `on`, matches the argument directly without stripping away anything.
+///
+/// Given
+/// \code
+///   class Y { public: void m(); };
+///   Y g();
+///   class X : public Y { void g(); };
+///   void z(Y y, X x) { y.m(); x.m(); x.g(); (g()).m(); }
+/// \endcode
+/// cxxMemberCallExpr(onImplicitObjectArgument(hasType(
+/// cxxRecordDecl(hasName("Y")
+///   matches `y.m()`, `x.m()` and (g()).m(), but not `x.g()`.
+/// cxxMemberCallExpr(on(callExpr()))
+///   does not match `(g()).m()`, because the parens are not ignored.
+///
+/// FIXME: Overload to allow directly matching types?
 AST_MATCHER_P(CXXMemberCallExpr, onImplicitObjectArgument,
   internal::Matcher, InnerMatcher) {
   const Expr *ExprNode = Node.getImplicitObjectArgument();
@@ -3260,8 +3285,22 @@
   InnerMatcher.matches(*ExprNode, Finder, Builder));
 }
 
-/// Matches if the expression's type either matches the specified
-/// matcher, or is a pointer to a type that matches the InnerMatcher.
+/// Matches if the type of the expression's implicit object argument either
+/// matches the InnerMatcher, or is a pointer to a type that matches the
+/// InnerMatcher.
+///
+/// Given
+/// \code
+///   class Y { public: void m(); };
+///   class X : public Y { void g(); };
+///   void z() { Y y; y.m(); Y *p; p->m(); X x; x.m(); x.g(); }
+/// \endcode
+/// cxxMemberCallExpr(thisPointerType(hasDeclaration(
+/// cxxRecordDecl(hasName("Y")
+///   matches `y.m()`, `p->m()` and `x.m()`.
+/// cxxMemberCallExpr(thisPointerType(hasDeclaration(
+/// cxxRecordDecl(hasName("X")
+///   matches `x.g()`.
 AST_MATCHER_P_OVERLOAD(CXXMemberCallExpr, thisPointerType,
internal::Matcher, InnerMatcher, 0) {
   return onImplicitObjectArgument(
@@ -4963,18 +5002,22 @@
   return InnerMatcher.matches(*Node.getMemberDecl(), Finder, Builder);
 }
 
-/// Matches a member expression where the object expression is
-/// matched by a given matcher.
+/// Matches a member expression where the object expression is matched by a
+/// given matcher. Implicit object expressions are included; that is, it matches
+/// use of implicit `this`.
 ///
 /// Given
 /// \code
-///   struct X { int m; };
-///   void f(X x) { x.m; m; }
+///   struct X {
+/// int m;
+/// int f(X x) { x.m; return m; }
+///   };
 /// \endcode
-/// memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X")))
-///   matches "x.m" and "m"
-/// with hasObjectExpression(...)
-///   matching "x" and the implicit object expression of "m" which has type X*.
+/// memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X")
+///   matches `x.m`, but not `m`; however,
+/// memberExpr(hasObjectExpression(hasType(pointsTo(
+//  cxxRecordDecl(hasName("X"))
+///   matches `m` (aka. `this->m`), but not `x.m`.
 AST_POLYMORPHIC_MATCHER_P(
 hasObjectExpression,
 AST_POLYMORPHIC_SUPPORTED_TYPES(MemberExpr, UnresolvedMemberExpr,
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -4810,16 +4810,20 @@
 
 
 Matcher

[PATCH] D56849: [ASTMatchers][NFC] Update comments on assorted `CXXMemberCallExpr` matchers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC353532: [ASTMatchers][NFC] Update comments on assorted 
`CXXMemberCallExpr` matchers. (authored by ymandel, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D56849?vs=185977&id=185978#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D56849

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h

Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -4810,16 +4810,20 @@
 
 
 MatcherCXXDependentScopeMemberExpr>hasObjectExpressionMatcherExpr> InnerMatcher
-Matches a member expression where the object expression is
-matched by a given matcher.
+Matches a member expression where the object expression is matched by a
+given matcher. Implicit object expressions are included; that is, it matches
+use of implicit `this`.
 
 Given
-  struct X { int m; };
-  void f(X x) { x.m; m; }
-memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X")))
-  matches "x.m" and "m"
-with hasObjectExpression(...)
-  matching "x" and the implicit object expression of "m" which has type X*.
+  struct X {
+int m;
+int f(X x) { x.m; return m; }
+  };
+memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X")
+  matches `x.m`, but not `m`; however,
+memberExpr(hasObjectExpression(hasType(pointsTo(
+ cxxRecordDecl(hasName("X"))
+  matches `m` (aka. `this->m`), but not `x.m`.
 
 
 
@@ -4857,16 +4861,39 @@
 
 
 MatcherCXXMemberCallExpr>onImplicitObjectArgumentMatcherExpr> InnerMatcher
-
+Matches on the implicit object argument of a member call expression. Unlike
+`on`, matches the argument directly without stripping away anything.
+
+Given
+  class Y { public: void m(); };
+  Y g();
+  class X : public Y { void g(); };
+  void z(Y y, X x) { y.m(); x.m(); x.g(); (g()).m(); }
+cxxMemberCallExpr(onImplicitObjectArgument(hasType(
+cxxRecordDecl(hasName("Y")
+  matches `y.m()`, `x.m()` and (g()).m(), but not `x.g()`.
+cxxMemberCallExpr(on(callExpr()))
+  does not match `(g()).m()`, because the parens are not ignored.
+
+FIXME: Overload to allow directly matching types?
+
 
 
 MatcherCXXMemberCallExpr>onMatcherExpr> InnerMatcher
-Matches on the implicit object argument of a member call expression.
+Matches on the implicit object argument of a member call expression, after
+stripping off any parentheses or implicit casts.
 
-Example matches y.x()
-  (matcher = cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("Y"))
-  class Y { public: void x(); };
-  void z() { Y y; y.x(); }
+Given
+  class Y { public: void m(); };
+  Y g();
+  class X : public Y {};
+  void z(Y y, X x) { y.m(); (g()).m(); x.m(); }
+cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("Y")
+  matches `y.m()` and `(g()).m()`.
+cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("X")
+  matches `x.m()`.
+cxxMemberCallExpr(on(callExpr()))
+  matches `(g()).m()`.
 
 FIXME: Overload to allow directly matching types?
 
@@ -4878,8 +4905,20 @@
 
 
 MatcherCXXMemberCallExpr>thisPointerTypeMatcherQualType> InnerMatcher
-Matches if the expression's type either matches the specified
-matcher, or is a pointer to a type that matches the InnerMatcher.
+Matches if the type of the expression's implicit object argument either
+matches the InnerMatcher, or is a pointer to a type that matches the
+InnerMatcher.
+
+Given
+  class Y { public: void m(); };
+  class X : public Y { void g(); };
+  void z() { Y y; y.m(); Y *p; p->m(); X x; x.m(); x.g(); }
+cxxMemberCallExpr(thisPointerType(hasDeclaration(
+cxxRecordDecl(hasName("Y")
+  matches `y.m()`, `p->m()` and `x.m()`.
+cxxMemberCallExpr(thisPointerType(hasDeclaration(
+cxxRecordDecl(hasName("X")
+  matches `x.g()`.
 
 
 
@@ -5984,16 +6023,20 @@
 
 
 MatcherMemberExpr>hasObjectExpressionMatcherExpr> InnerMatcher
-Matches a member expression where the object expression is
-matched by a given matcher.
+Matches a member expression where the object expression is matched by a
+given matcher. Implicit object expressions are included; that is, it matches
+use of implicit `this`.
 
 Given
-  struct X { int

r353532 - [ASTMatchers][NFC] Update comments on assorted `CXXMemberCallExpr` matchers.

[Yitzhak Mandelbaum via cfe-commits]

Author: ymandel
Date: Fri Feb  8 08:00:44 2019
New Revision: 353532

URL: http://llvm.org/viewvc/llvm-project?rev=353532&view=rev
Log:
[ASTMatchers][NFC] Update comments on assorted `CXXMemberCallExpr` matchers.

Specifically:

* fixes the comments on `hasObjectExpression`,
* clarifies comments on `thisPointerType` and `on`,
* adds comments to `onImplicitObjectArgument`.

It also updates associated reference docs (using the doc tool).

Reviewers: alexfh, steveire, aaron.ballman

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

Modified:
cfe/trunk/docs/LibASTMatchersReference.html
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h

Modified: cfe/trunk/docs/LibASTMatchersReference.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=353532&r1=353531&r2=353532&view=diff
==
--- cfe/trunk/docs/LibASTMatchersReference.html (original)
+++ cfe/trunk/docs/LibASTMatchersReference.html Fri Feb  8 08:00:44 2019
@@ -4810,16 +4810,20 @@ with withInitializer matching (1)
 
 
 MatcherCXXDependentScopeMemberExpr>hasObjectExpressionMatcherExpr> 
InnerMatcher
-Matches a 
member expression where the object expression is
-matched by a given matcher.
+Matches a 
member expression where the object expression is matched by a
+given matcher. Implicit object expressions are included; that is, it matches
+use of implicit `this`.
 
 Given
-  struct X { int m; };
-  void f(X x) { x.m; m; }
-memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X")))
-  matches "x.m" and "m"
-with hasObjectExpression(...)
-  matching "x" and the implicit object expression of "m" which has type X*.
+  struct X {
+int m;
+int f(X x) { x.m; return m; }
+  };
+memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X")
+  matches `x.m`, but not `m`; however,
+memberExpr(hasObjectExpression(hasType(pointsTo(
+ cxxRecordDecl(hasName("X"))
+  matches `m` (aka. `this->m`), but not `x.m`.
 
 
 
@@ -4857,16 +4861,39 @@ matches 'a' in
 
 
 MatcherCXXMemberCallExpr>onImplicitObjectArgumentMatcherExpr> 
InnerMatcher
-
+Matches on 
the implicit object argument of a member call expression. Unlike
+`on`, matches the argument directly without stripping away anything.
+
+Given
+  class Y { public: void m(); };
+  Y g();
+  class X : public Y { void g(); };
+  void z(Y y, X x) { y.m(); x.m(); x.g(); (g()).m(); }
+cxxMemberCallExpr(onImplicitObjectArgument(hasType(
+cxxRecordDecl(hasName("Y")
+  matches `y.m()`, `x.m()` and (g()).m(), but not `x.g()`.
+cxxMemberCallExpr(on(callExpr()))
+  does not match `(g()).m()`, because the parens are not ignored.
+
+FIXME: Overload to allow directly matching types?
+
 
 
 MatcherCXXMemberCallExpr>onMatcherExpr> 
InnerMatcher
-Matches on the implicit object 
argument of a member call expression.
+Matches on the implicit object 
argument of a member call expression, after
+stripping off any parentheses or implicit casts.
 
-Example matches y.x()
-  (matcher = cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("Y"))
-  class Y { public: void x(); };
-  void z() { Y y; y.x(); }
+Given
+  class Y { public: void m(); };
+  Y g();
+  class X : public Y {};
+  void z(Y y, X x) { y.m(); (g()).m(); x.m(); }
+cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("Y")
+  matches `y.m()` and `(g()).m()`.
+cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("X")
+  matches `x.m()`.
+cxxMemberCallExpr(on(callExpr()))
+  matches `(g()).m()`.
 
 FIXME: Overload to allow directly matching types?
 
@@ -4878,8 +4905,20 @@ FIXME: Overload to allow directly matchi
 
 
 MatcherCXXMemberCallExpr>thisPointerTypeMatcherQualType>
 InnerMatcher
-Matches if the 
expression's type either matches the specified
-matcher, or is a pointer to a type that matches the InnerMatcher.
+Matches if the type 
of the expression's implicit object argument either
+matches the InnerMatcher, or is a pointer to a type that matches the
+InnerMatcher.
+
+Given
+  class Y { public: void m(); };
+  class X : public Y { void g(); };
+  void z() { Y y; y.m(); Y *p; p->m(); X x; x.m(); x.g(); }
+cxxMemberCallExpr(thisPointerType(hasDeclaration(
+cxxRecordDecl(hasName("Y")
+  matches `y.m()`, `p->m()` and `x.m()`.
+cxxMemberCallExpr(thisPointerType(hasDeclaration(
+cxxRecordDecl(hasName("X")
+  matches `x.g()`.
 
 
 
@@ -5984,16 +6023,20 @@ Usable as: Matcher

[PATCH] D40720: No -fsanitize=function warning when calling noexcept function through non-noexcept pointer in C++17

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.
Herald added a project: clang.

It seems to be causing https://bugs.llvm.org/show_bug.cgi?id=40658.


Repository:
  rC Clang

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

https://reviews.llvm.org/D40720



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

[PATCH] D54978: Move the SMT API to LLVM

[Brian Rzycki via Phabricator via cfe-commits]

brzycki added a comment.

> From what I understand, setting `-DLLVM_ENABLE_Z3_SOLVER=OFF` is supposed to 
> work

Hello @thakis, I have reduced it down to the minimal required flags on Ubuntu 
18.04. I ran this on llvm-project SHA b0a227049fda9d0d229ea801ae77bf1b812f7328 
 from Feb 
8, 2019.

First, make sure Ubuntu has installed its version of Z3:

  sudo apt install libz3-4 libz3-dev
  dpkg -l | grep libz3
  ii  libz3-4:amd64 4.4.1-0.3build4 
amd64theorem prover from Microsoft Research - runtime libraries
  ii  libz3-dev:amd64   4.4.1-0.3build4 
amd64theorem prover from Microsoft Research - development files

Next, run CMake with the following arguments:

  mkdir build && cd build
  cmake \
-G Ninja \
-D LLVM_OPTIMIZED_TABLEGEN=ON \
-D LLVM_ENABLE_Z3_SOLVER=OFF \
  /path/to/llvm-project/llvm

First, you'll see that CMake detects a version of Z3:

  -- Found Z3: /usr/lib/x86_64-linux-gnu/libz3.so (Required is at least version 
"4.7.1")

At around ninja command 600-700 a second CMake instance is spawned for the 
TableGen optimizations:

  [666/3618] cd /work/brzycki/b/NATIVE && 
/sarc-c/compiler_tmp/tools/build/cmake-3.13.3/bin/cmake -G Ninja 
-DCMAKE_MAKE_PROGRAM="/sarc-c/compiler_tmp/tools/build/ninja-1.8.2/ninja" 
-DCMAKE_C_COMPILER=/usr/bin/cc -DCMAKE_CXX_COMPILER=/usr/bin/c++ 
/work/brzycki/llvm-project/llvm -DLLVM_TARGET_IS_CROSSCOMPILE_HOST=TRUE 
-DLLVM_TARGETS_TO_BUILD="AArch64;AMDGPU;ARM;BPF;Hexagon;Lanai;Mips;MSP430;NVPTX;PowerPC;Sparc;SystemZ;WebAssembly;X86;XCore"
 -DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD="" 
-DLLVM_DEFAULT_TARGET_TRIPLE="x86_64-unknown-linux-gnu" 
-DLLVM_TARGET_ARCH="host" -DLLVM_TEMPORARILY_ALLOW_OLD_TOOLCHAIN="OFF" 
-DCMAKE_BUILD_TYPE=Release
  -- The C compiler identification is GNU 7.3.0
  -- The CXX compiler identification is GNU 7.3.0
  -- The ASM compiler identification is GNU
  -- Found assembler: /usr/bin/cc
  -- Check for working C compiler: /usr/bin/cc
  -- Check for working C compiler: /usr/bin/cc -- works
  ...

And shortly after that we fail:

  -- Build files have been written to: /work/brzycki/b/NATIVE
  [666/3618] cd /work/brzycki/b/NATIVE && 
/sarc-c/compiler_tmp/tools/build/cmake-3.13.3/bin/cmake --build 
/work/brzycki/b/NATIVE --target llvm-tblgen --config Release
  [72/187] Building CXX object 
lib/Support/CMakeFiles/LLVMSupport.dir/Z3Solver.cpp.o
  FAILED: lib/Support/CMakeFiles/LLVMSupport.dir/Z3Solver.cpp.o
  /usr/bin/c++  -DGTEST_HAS_RTTI=0 -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS 
-D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib/Support 
-I/work/brzycki/llvm-project/llvm/lib/Support -I/usr/include/libxml2 -Iinclude 
-I/work/brzycki/llvm-project/llvm/include -fPIC -fvisibility-inlines-hidden 
-Werror=date-time -std=c++11 -Wall -Wextra -Wno-unused-parameter 
-Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic 
-Wno-long-long -Wimplicit-fallthrough -Wno-maybe-uninitialized 
-Wno-noexcept-type -Wdelete-non-virtual-dtor -Wno-comment -fdiagnostics-color 
-ffunction-sections -fdata-sections -O3 -DNDEBUG-fno-exceptions -fno-rtti 
-MD -MT lib/Support/CMakeFiles/LLVMSupport.dir/Z3Solver.cpp.o -MF 
lib/Support/CMakeFiles/LLVMSupport.dir/Z3Solver.cpp.o.d -o 
lib/Support/CMakeFiles/LLVMSupport.dir/Z3Solver.cpp.o -c 
/work/brzycki/llvm-project/llvm/lib/Support/Z3Solver.cpp
  /work/brzycki/llvm-project/llvm/lib/Support/Z3Solver.cpp: In function ‘void 
{anonymous}::Z3ErrorHandler(Z3_context, Z3_error_code)’:
  /work/brzycki/llvm-project/llvm/lib/Support/Z3Solver.cpp:44:71: error: cannot 
convert ‘Z3_context {aka _Z3_context* ’ to ‘Z3_error_code’ for argument ‘1’ to 
‘const char* Z3_get_error_msg(Z3_error_code)’
  llvm::Twine(Z3_get_error_msg(Context, Error)));
 ^
  [183/187] Building CXX object 
utils/TableGen/CMakeFiles/llvm-tblgen.dir/GlobalISelEmitter.cpp.o
  ninja: build stopped: subcommand failed.
  FAILED: NATIVE/bin/llvm-tblgen
  cd /work/brzycki/b/NATIVE && 
/sarc-c/compiler_tmp/tools/build/cmake-3.13.3/bin/cmake --build 
/work/brzycki/b/NATIVE --target llvm-tblgen --config Release
  ninja: build stopped: subcommand failed.

I consider this to be 2 bugs:

1. CMake should not set Z3_FOUND when the library is too old. The 
llvm/CMakeLists.txt file correctly states `find_package(Z3 4.7.1)` but it 
doesn't work right and consideres the 4.4.1 one valid. It's why I'm in this 
mess in the first place.
2. There's some sort of interaction bug between the top-level 
llvm/CMakeLists.txt and the tablegen one when optimizations are turned on. It 
doesn't seem to respect the `LLVM_ENABLE_Z3_SOLVER` directive.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



__

[clang-tools-extra] r353535 - [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

[Paul Hoad via cfe-commits]

Author: paulhoad
Date: Fri Feb  8 09:00:01 2019
New Revision: 353535

URL: http://llvm.org/viewvc/llvm-project?rev=353535&view=rev
Log:
[clang-tidy] Add options to bugprone-argument-comment to add missing argument 
comments to literals

bugprone-argument-comment only supports identifying those comments which do not 
match the function parameter name

This revision add 3 options to adding missing argument comments to literals 
(granularity on type is added to control verbosity of fixit)

```
CheckOptions:
  - key: bugprone-argument-comment.CommentBoolLiterals
value:   '1'
  - key: bugprone-argument-comment.CommentFloatLiterals
value:   '1'
  - key: bugprone-argument-comment.CommentIntegerLiterals
value:   '1'
  - key: bugprone-argument-comment.CommentStringLiterals
value:   '1'
  - key: bugprone-argument-comment.CommentCharacterLiterals
value:   '1'
  - key: bugprone-argument-comment.CommentUserDefinedLiterals
value:   '1'
  - key: bugprone-argument-comment.CommentNullPtrs
value:   '1'
```

After applying these options, literal arguments will be preceded with 
/*ParameterName=*/

Reviewers: JonasToth, Eugene.Zelenko, alexfh, hokein, aaron.ballman

Reviewed By: aaron.ballman, Eugene.Zelenko

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

Added:

clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment-literals.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.h
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-argument-comment.rst

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp?rev=353535&r1=353534&r2=353535&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp Fri 
Feb  8 09:00:01 2019
@@ -11,6 +11,7 @@
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Token.h"
+
 #include "../utils/LexerUtils.h"
 
 using namespace clang::ast_matchers;
@@ -23,17 +24,37 @@ ArgumentCommentCheck::ArgumentCommentChe
ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
   StrictMode(Options.getLocalOrGlobal("StrictMode", 0) != 0),
+  CommentBoolLiterals(Options.getLocalOrGlobal("CommentBoolLiterals", 0) !=
+  0),
+  CommentIntegerLiterals(
+  Options.getLocalOrGlobal("CommentIntegerLiterals", 0) != 0),
+  CommentFloatLiterals(
+  Options.getLocalOrGlobal("CommentFloatLiterals", 0) != 0),
+  CommentStringLiterals(
+  Options.getLocalOrGlobal("CommentStringLiterals", 0) != 0),
+  CommentUserDefinedLiterals(
+  Options.getLocalOrGlobal("CommentUserDefinedLiterals", 0) != 0),
+  CommentCharacterLiterals(
+  Options.getLocalOrGlobal("CommentCharacterLiterals", 0) != 0),
+  CommentNullPtrs(Options.getLocalOrGlobal("CommentNullPtrs", 0) != 0),
   IdentRE("^(/\\* *)([_A-Za-z][_A-Za-z0-9]*)( *= *\\*/)$") {}
 
 void ArgumentCommentCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "StrictMode", StrictMode);
+  Options.store(Opts, "CommentBoolLiterals", CommentBoolLiterals);
+  Options.store(Opts, "CommentIntegerLiterals", CommentIntegerLiterals);
+  Options.store(Opts, "CommentFloatLiterals", CommentFloatLiterals);
+  Options.store(Opts, "CommentStringLiterals", CommentStringLiterals);
+  Options.store(Opts, "CommentUserDefinedLiterals", 
CommentUserDefinedLiterals);
+  Options.store(Opts, "CommentCharacterLiterals", CommentCharacterLiterals);
+  Options.store(Opts, "CommentNullPtrs", CommentNullPtrs);
 }
 
 void ArgumentCommentCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
   callExpr(unless(cxxOperatorCallExpr()),
-   // NewCallback's arguments relate to the pointed function, don't
-   // check them against NewCallback's parameter names.
+   // NewCallback's arguments relate to the pointed function,
+   // don't check them against NewCallback's parameter names.
// FIXME: Make this configurable.
unless(hasDeclaration(functionDecl(
hasAnyName("NewCallback", "NewPermanentCallback")
@@ -126,8 +147,8 @@ static bool isLikelyTypo(llvm::ArrayRef<
 
 const unsigned Threshold = 2;
 // Other parameters must be an edit distance at least Threshold more away
-// from this parameter. This gives us greate

[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353535: [clang-tidy] Add options to 
bugprone-argument-comment to add missing argument… (authored by paulhoad, 
committed by ).
Herald added a project: LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D57674?vs=185947&id=185985#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57674

Files:
  clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
  clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.h
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-argument-comment.rst
  clang-tools-extra/trunk/test/clang-tidy/bugprone-argument-comment-literals.cpp

Index: clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.h
+++ clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.h
@@ -26,7 +26,8 @@
 ///
 ///   ...
 ///   f(/*bar=*/true);
-///   // warning: argument name 'bar' in comment does not match parameter name 'foo'
+///   // warning: argument name 'bar' in comment does not match parameter name
+///   'foo'
 /// \endcode
 ///
 /// The check tries to detect typos and suggest automated fixes for them.
@@ -39,12 +40,21 @@
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
 
 private:
-  const bool StrictMode;
+  const unsigned StrictMode : 1;
+  const unsigned CommentBoolLiterals : 1;
+  const unsigned CommentIntegerLiterals : 1;
+  const unsigned CommentFloatLiterals : 1;
+  const unsigned CommentStringLiterals : 1;
+  const unsigned CommentUserDefinedLiterals : 1;
+  const unsigned CommentCharacterLiterals : 1;
+  const unsigned CommentNullPtrs : 1;
   llvm::Regex IdentRE;
 
   void checkCallArgs(ASTContext *Ctx, const FunctionDecl *Callee,
  SourceLocation ArgBeginLoc,
  llvm::ArrayRef Args);
+
+  bool shouldAddComment(const Expr *Arg) const;
 };
 
 } // namespace bugprone
Index: clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -11,6 +11,7 @@
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Token.h"
+
 #include "../utils/LexerUtils.h"
 
 using namespace clang::ast_matchers;
@@ -23,17 +24,37 @@
ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
   StrictMode(Options.getLocalOrGlobal("StrictMode", 0) != 0),
+  CommentBoolLiterals(Options.getLocalOrGlobal("CommentBoolLiterals", 0) !=
+  0),
+  CommentIntegerLiterals(
+  Options.getLocalOrGlobal("CommentIntegerLiterals", 0) != 0),
+  CommentFloatLiterals(
+  Options.getLocalOrGlobal("CommentFloatLiterals", 0) != 0),
+  CommentStringLiterals(
+  Options.getLocalOrGlobal("CommentStringLiterals", 0) != 0),
+  CommentUserDefinedLiterals(
+  Options.getLocalOrGlobal("CommentUserDefinedLiterals", 0) != 0),
+  CommentCharacterLiterals(
+  Options.getLocalOrGlobal("CommentCharacterLiterals", 0) != 0),
+  CommentNullPtrs(Options.getLocalOrGlobal("CommentNullPtrs", 0) != 0),
   IdentRE("^(/\\* *)([_A-Za-z][_A-Za-z0-9]*)( *= *\\*/)$") {}
 
 void ArgumentCommentCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "StrictMode", StrictMode);
+  Options.store(Opts, "CommentBoolLiterals", CommentBoolLiterals);
+  Options.store(Opts, "CommentIntegerLiterals", CommentIntegerLiterals);
+  Options.store(Opts, "CommentFloatLiterals", CommentFloatLiterals);
+  Options.store(Opts, "CommentStringLiterals", CommentStringLiterals);
+  Options.store(Opts, "CommentUserDefinedLiterals", CommentUserDefinedLiterals);
+  Options.store(Opts, "CommentCharacterLiterals", CommentCharacterLiterals);
+  Options.store(Opts, "CommentNullPtrs", CommentNullPtrs);
 }
 
 void ArgumentCommentCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
   callExpr(unless(cxxOperatorCallExpr()),
-   // NewCallback's arguments relate to the pointed function, don't
-   // check them against NewCallback's parameter names.
+   // NewCallback's arguments relate to the pointed function,
+   // don't check them against NewCallback's parameter names.
// FIXME: Make this configurable.
unless(hasDeclaration(functionDecl(
hasAnyName("NewCallback", "NewPermanentCallback")
@@ -126,8 +147,8 @@
 
 const unsigned Threshold = 2;
 // Other parameters must be an edit distance at l

[PATCH] D54978: Move the SMT API to LLVM

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Thanks for the analysis. I think it's fine if you revert, given that.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



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

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 185990.
glider retitled this revision from "[RFC] Split constant structures generated 
by -ftrivial-auto-var-init when emitting initializators" to "CodeGen: Fix 
PR40605: split constant structures generated by -ftrivial-auto-var-init when 
emitting initializators".
glider edited the summary of this revision.
glider added a comment.

Updated the patch: instead of passing around forInit, match the constant type 
to pointer type.

I'm not quite sure how to show the resulting difference in code. Do you mean we 
need Clang to support both modes and to compare the resulting assembly?


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

https://reviews.llvm.org/D57898

Files:
  tools/clang/lib/CodeGen/CGDecl.cpp
  tools/clang/test/CodeGenCXX/auto-var-init.cpp

Index: tools/clang/test/CodeGenCXX/auto-var-init.cpp
===
--- tools/clang/test/CodeGenCXX/auto-var-init.cpp
+++ tools/clang/test/CodeGenCXX/auto-var-init.cpp
@@ -30,42 +30,24 @@
 // PATTERN-NOT: undef
 // ZERO-NOT: undef
 
-// PATTERN: @__const.test_empty_uninit.uninit = private unnamed_addr constant %struct.empty { i8 -86 }, align 1
 struct empty {};
-// PATTERN: @__const.test_small_uninit.uninit = private unnamed_addr constant %struct.small { i8 -86 }, align 1
 // PATTERN: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
 // ZERO: @__const.test_small_custom.custom = private unnamed_addr constant %struct.small { i8 42 }, align 1
 struct small { char c; };
-// PATTERN: @__const.test_smallinit_uninit.uninit = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
-// PATTERN: @__const.test_smallinit_braces.braces = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
-// PATTERN: @__const.test_smallinit_custom.custom = private unnamed_addr constant %struct.smallinit { i8 -86 }, align 1
 struct smallinit { char c = 42; };
-// PATTERN: @__const.test_smallpartinit_uninit.uninit = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
-// PATTERN: @__const.test_smallpartinit_braces.braces = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
-// PATTERN: @__const.test_smallpartinit_custom.custom = private unnamed_addr constant %struct.smallpartinit { i8 -86, i8 -86 }, align 1
 struct smallpartinit { char c = 42, d; };
-// PATTERN: @__const.test_nullinit_uninit.uninit = private unnamed_addr constant %struct.nullinit { i8* inttoptr (i64 -6148914691236517206 to i8*) }, align 8
-// PATTERN: @__const.test_nullinit_braces.braces = private unnamed_addr constant %struct.nullinit { i8* inttoptr (i64 -6148914691236517206 to i8*) }, align 8
-// PATTERN: @__const.test_nullinit_custom.custom = private unnamed_addr constant %struct.nullinit { i8* inttoptr (i64 -6148914691236517206 to i8*) }, align 8
 struct nullinit { char* null = nullptr; };
-// PATTERN: @__const.test_padded_uninit.uninit = private unnamed_addr constant %struct.padded { i8 -86, i32 -1431655766 }, align 4
 // PATTERN: @__const.test_padded_custom.custom = private unnamed_addr constant %struct.padded { i8 42, i32 13371337 }, align 4
 // ZERO: @__const.test_padded_custom.custom = private unnamed_addr constant %struct.padded { i8 42, i32 13371337 }, align 4
 struct padded { char c; int i; };
-// PATTERN: @__const.test_paddednullinit_uninit.uninit = private unnamed_addr constant %struct.paddednullinit { i8 -86, i32 -1431655766 }, align 4
-// PATTERN: @__const.test_paddednullinit_braces.braces = private unnamed_addr constant %struct.paddednullinit { i8 -86, i32 -1431655766 }, align 4
-// PATTERN: @__const.test_paddednullinit_custom.custom = private unnamed_addr constant %struct.paddednullinit { i8 -86, i32 -1431655766 }, align 4
 struct paddednullinit { char c = 0; int i = 0; };
-// PATTERN: @__const.test_bitfield_uninit.uninit = private unnamed_addr constant %struct.bitfield { i8 -86, [3 x i8] c"\AA\AA\AA" }, align 4
 // PATTERN: @__const.test_bitfield_custom.custom = private unnamed_addr constant %struct.bitfield { i8 20, [3 x i8] zeroinitializer }, align 4
 // ZERO: @__const.test_bitfield_custom.custom = private unnamed_addr constant %struct.bitfield { i8 20, [3 x i8] zeroinitializer }, align 4
 struct bitfield { int i : 4; int j : 2; };
-// PATTERN: @__const.test_bitfieldaligned_uninit.uninit = private unnamed_addr constant %struct.bitfieldaligned { i8 -86, [3 x i8] c"\AA\AA\AA", i8 -86, [3 x i8] c"\AA\AA\AA" }, align 4
 // PATTERN: @__const.test_bitfieldaligned_custom.custom = private unnamed_addr constant %struct.bitfieldaligned { i8 4, [3 x i8] zeroinitializer, i8 1, [3 x i8] zeroinitializer }, align 4
 // ZERO: @__const.test_bitfieldaligned_custom.custom = private unnamed_addr constant %struct.bitfieldaligned { i8 4, [3 x i8] zeroinitializer, i8 1, [3 x i8] zeroinitializer }, align 4
 struct bitfieldaligned { int i : 4; int : 0; int j : 2; };
 struct big { unsigned 

[PATCH] D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Overall it looks ok to me, but I'd like Chandler to comment regarding the 
preferred way to do this with the new PM, since we don't tend to use booleans 
there in the PassBuilder to control passes. Is it preferable to instead use a 
new function attribute instead of boolean flags on the PMs (e.g. the way 
-fno-inline is handled)?




Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1331
+  Opts.SplitColdCode =
+  (Opts.OptimizationLevel > 0) && (Opts.OptimizeSize != 2) &&
+  Args.hasFlag(OPT_fsplit_cold_code, OPT_fno_split_cold_code, false);

would it be appropriate to give a warning when being ignored?


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

https://reviews.llvm.org/D57265



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

r353540 - [OPENMP]Initial support for the delayed diagnostics.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Feb  8 09:38:09 2019
New Revision: 353540

URL: http://llvm.org/viewvc/llvm-project?rev=353540&view=rev
Log:
[OPENMP]Initial support for the delayed diagnostics.

It is important to delay the emission of the diagnostic messages for the
functions unless it is proved that the function is going to be used on
the device side. It is required to support compilation with some of the
target-specific system headers.

Modified:
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/test/OpenMP/nvptx_target_exceptions_messages.cpp

Modified: cfe/trunk/include/clang/Basic/LangOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.def?rev=353540&r1=353539&r2=353540&view=diff
==
--- cfe/trunk/include/clang/Basic/LangOptions.def (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.def Fri Feb  8 09:38:09 2019
@@ -203,7 +203,6 @@ LANGOPT(OpenMPUseTLS  , 1, 0, "Use T
 LANGOPT(OpenMPIsDevice, 1, 0, "Generate code only for OpenMP target 
device")
 LANGOPT(OpenMPCUDAMode, 1, 0, "Generate code for OpenMP pragmas in 
SIMT/SPMD mode")
 LANGOPT(OpenMPCUDAForceFullRuntime , 1, 0, "Force to use full runtime in all 
constructs when offloading to CUDA devices")
-LANGOPT(OpenMPHostCXXExceptions, 1, 0, "C++ exceptions handling in the 
host code.")
 LANGOPT(OpenMPCUDANumSMs  , 32, 0, "Number of SMs for CUDA devices.")
 LANGOPT(OpenMPCUDABlocksPerSM  , 32, 0, "Number of blocks per SM for CUDA 
devices.")
 LANGOPT(OpenMPOptimisticCollapse  , 1, 0, "Use at most 32 bits to represent 
the collapsed loop nest counter.")

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=353540&r1=353539&r2=353540&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Fri Feb  8 09:38:09 2019
@@ -8763,6 +8763,9 @@ private:
   /// Pop OpenMP function region for non-capturing function.
   void popOpenMPFunctionRegion(const sema::FunctionScopeInfo *OldFSI);
 
+  /// Check whether we're allowed to call Callee from the current function.
+  void checkOpenMPDeviceFunction(SourceLocation Loc, FunctionDecl *Callee);
+
   /// Checks if a type or a declaration is disabled due to the owning extension
   /// being disabled, and emits diagnostic messages if it is disabled.
   /// \param D type or declaration to be checked.
@@ -10249,6 +10252,23 @@ public:
   /// Same as CUDADiagIfDeviceCode, with "host" and "device" switched.
   DeviceDiagBuilder CUDADiagIfHostCode(SourceLocation Loc, unsigned DiagID);
 
+  /// Creates a DeviceDiagBuilder that emits the diagnostic if the current
+  /// context is "used as device code".
+  ///
+  /// - If CurContext is a `declare target` function or it is known that the
+  /// function is emitted for the device, emits the diagnostics immediately.
+  /// - If CurContext is a non-`declare target` function and we are compiling
+  ///   for the device, creates a diagnostic which is emitted if and when we
+  ///   realize that the function will be codegen'ed.
+  ///
+  /// Example usage:
+  ///
+  ///  // Variable-length arrays are not allowed in NVPTX device code.
+  ///  if (diagIfOpenMPDeviceCode(Loc, diag::err_vla_unsupported))
+  ///return ExprError();
+  ///  // Otherwise, continue parsing as normal.
+  DeviceDiagBuilder diagIfOpenMPDeviceCode(SourceLocation Loc, unsigned 
DiagID);
+
   enum CUDAFunctionTarget {
 CFT_Device,
 CFT_Global,

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=353540&r1=353539&r2=353540&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Fri Feb  8 09:38:09 2019
@@ -2827,7 +2827,6 @@ static void ParseLangArgs(LangOptions &O
 
   // Set the flag to prevent the implementation from emitting device exception
   // handling code for those requiring so.
-  Opts.OpenMPHostCXXExceptions = Opts.Exceptions && Opts.CXXExceptions;
   if ((Opts.OpenMPIsDevice && T.isNVPTX()) || Opts.OpenCLCPlusPlus) {
 Opts.Exceptions = 0;
 Opts.CXXExceptions = 0;

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=353540&r1=353539&r2=353540&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp F

r353541 - Revert "[OPENMP]Initial support for the delayed diagnostics."

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Feb  8 09:42:00 2019
New Revision: 353541

URL: http://llvm.org/viewvc/llvm-project?rev=353541&view=rev
Log:
Revert "[OPENMP]Initial support for the delayed diagnostics."

This reverts commit r353540. Erroneously committed, need to fix the
message and description.

Modified:
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/test/OpenMP/nvptx_target_exceptions_messages.cpp

Modified: cfe/trunk/include/clang/Basic/LangOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.def?rev=353541&r1=353540&r2=353541&view=diff
==
--- cfe/trunk/include/clang/Basic/LangOptions.def (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.def Fri Feb  8 09:42:00 2019
@@ -203,6 +203,7 @@ LANGOPT(OpenMPUseTLS  , 1, 0, "Use T
 LANGOPT(OpenMPIsDevice, 1, 0, "Generate code only for OpenMP target 
device")
 LANGOPT(OpenMPCUDAMode, 1, 0, "Generate code for OpenMP pragmas in 
SIMT/SPMD mode")
 LANGOPT(OpenMPCUDAForceFullRuntime , 1, 0, "Force to use full runtime in all 
constructs when offloading to CUDA devices")
+LANGOPT(OpenMPHostCXXExceptions, 1, 0, "C++ exceptions handling in the 
host code.")
 LANGOPT(OpenMPCUDANumSMs  , 32, 0, "Number of SMs for CUDA devices.")
 LANGOPT(OpenMPCUDABlocksPerSM  , 32, 0, "Number of blocks per SM for CUDA 
devices.")
 LANGOPT(OpenMPOptimisticCollapse  , 1, 0, "Use at most 32 bits to represent 
the collapsed loop nest counter.")

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=353541&r1=353540&r2=353541&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Fri Feb  8 09:42:00 2019
@@ -8763,9 +8763,6 @@ private:
   /// Pop OpenMP function region for non-capturing function.
   void popOpenMPFunctionRegion(const sema::FunctionScopeInfo *OldFSI);
 
-  /// Check whether we're allowed to call Callee from the current function.
-  void checkOpenMPDeviceFunction(SourceLocation Loc, FunctionDecl *Callee);
-
   /// Checks if a type or a declaration is disabled due to the owning extension
   /// being disabled, and emits diagnostic messages if it is disabled.
   /// \param D type or declaration to be checked.
@@ -10252,23 +10249,6 @@ public:
   /// Same as CUDADiagIfDeviceCode, with "host" and "device" switched.
   DeviceDiagBuilder CUDADiagIfHostCode(SourceLocation Loc, unsigned DiagID);
 
-  /// Creates a DeviceDiagBuilder that emits the diagnostic if the current
-  /// context is "used as device code".
-  ///
-  /// - If CurContext is a `declare target` function or it is known that the
-  /// function is emitted for the device, emits the diagnostics immediately.
-  /// - If CurContext is a non-`declare target` function and we are compiling
-  ///   for the device, creates a diagnostic which is emitted if and when we
-  ///   realize that the function will be codegen'ed.
-  ///
-  /// Example usage:
-  ///
-  ///  // Variable-length arrays are not allowed in NVPTX device code.
-  ///  if (diagIfOpenMPDeviceCode(Loc, diag::err_vla_unsupported))
-  ///return ExprError();
-  ///  // Otherwise, continue parsing as normal.
-  DeviceDiagBuilder diagIfOpenMPDeviceCode(SourceLocation Loc, unsigned 
DiagID);
-
   enum CUDAFunctionTarget {
 CFT_Device,
 CFT_Global,

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=353541&r1=353540&r2=353541&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Fri Feb  8 09:42:00 2019
@@ -2827,6 +2827,7 @@ static void ParseLangArgs(LangOptions &O
 
   // Set the flag to prevent the implementation from emitting device exception
   // handling code for those requiring so.
+  Opts.OpenMPHostCXXExceptions = Opts.Exceptions && Opts.CXXExceptions;
   if ((Opts.OpenMPIsDevice && T.isNVPTX()) || Opts.OpenCLCPlusPlus) {
 Opts.Exceptions = 0;
 Opts.CXXExceptions = 0;

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=353541&r1=353540&r2=353541&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Feb  8 09:42:00 2019
@@ -14896,9 +14896,6 @@ void Sema::MarkFunctionReferenced(Source
   }
 
   Func->markUsed(Context);
-
-  if (LangOpts.Open

Re: [PATCH] D57898: [RFC] Split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[Alexander Potapenko via cfe-commits]

Still unsure about the heuristic here. I believe that for
auto-initialization we want to be quite aggressive with these splits
(unlike for regular constant stores).
Perhaps we should do the split in the case all bytes are pattern bytes?
(This is probably another use case for `forInit`)

On Fri, Feb 8, 2019, 11:36 Alexander Potapenko via Phabricator <
revi...@reviews.llvm.org> wrote:

> glider marked 2 inline comments as not done.
> glider added a comment.
>
> Sorry, didn't mean to mark these done.
>
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D57898/new/
>
> https://reviews.llvm.org/D57898
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57961: [X86] Add explicit alignment to __m128/__m128i/__m128d/etc. to allow matching of MSVC behavior with #pragma pack.

[Craig Topper via Phabricator via cfe-commits]

craig.topper created this revision.
craig.topper added reviewers: rnk, erichkeane, spatel, RKSimon.
Herald added a project: clang.

With MSVC, #pragma pack is ignored when there is explicit alignment. This 
differs from gcc. Clang emulates this difference when compiling for Windows.

It appears that MSVC and headers consider the __m128/__m128i/__m128d/etc. types 
to be explicitly aligned and ignores #pragma pack for them. Since we don't have 
explicit alignment on them in our headers, we don't match the MSVC behavior 
here.

This patch adds explicit alignment to match this behavior. I'm hoping this 
won't cause any problems when we're not emulating MSVC. But if someone knows of 
something that would be different we can swith to conditionally adding the 
alignment based on _MSC_VER.

I had to add explicitly unaligned types as well so we could use them in the 
loadu/storeu intrinsics which use __attribute__(__packed__). So the explicitly 
aligned types wouldn't produce align 1 accesses when targeting Windows.


Repository:
  rC Clang

https://reviews.llvm.org/D57961

Files:
  lib/Headers/avx512bwintrin.h
  lib/Headers/avx512fintrin.h
  lib/Headers/avx512vlbwintrin.h
  lib/Headers/avx512vlintrin.h
  lib/Headers/avxintrin.h
  lib/Headers/emmintrin.h
  lib/Headers/xmmintrin.h
  test/CodeGen/x86-vec-struct-packing.c

Index: test/CodeGen/x86-vec-struct-packing.c
===
--- /dev/null
+++ test/CodeGen/x86-vec-struct-packing.c
@@ -0,0 +1,205 @@
+// RUN: %clang_cc1 -ffreestanding -emit-llvm-only  -triple x86_64-windows-coff -fdump-record-layouts %s | FileCheck %s --check-prefix=CHECK-MS
+// RUN: %clang_cc1 -ffreestanding -emit-llvm-only  -triple x86_64-apple-darwin -fdump-record-layouts %s | FileCheck %s --check-prefix=CHECK-NOTMS
+#include 
+
+#pragma pack(1)
+
+struct s_m128 {
+  int a;
+  __m128 b;
+};
+typedef struct s_m128 m128;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m128) == 32) - 1];
+#else
+static int a1[(sizeof(m128) == 20) - 1];
+#endif
+
+struct s_m128i {
+  int a;
+  __m128i b;
+};
+typedef struct s_m128i m128i;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m128i) == 32) - 1];
+#else
+static int a1[(sizeof(m128i) == 20) - 1];
+#endif
+
+struct s_m128d {
+  int a;
+  __m128d b;
+};
+typedef struct s_m128d m128d;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m128d) == 32) - 1];
+#else
+static int a1[(sizeof(m128d) == 20) - 1];
+#endif
+
+struct s_m256 {
+  int a;
+  __m256 b;
+};
+typedef struct s_m256 m256;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m256) == 64) - 1];
+#else
+static int a1[(sizeof(m256) == 36) - 1];
+#endif
+
+struct s_m256i {
+  int a;
+  __m256i b;
+};
+typedef struct s_m256i m256i;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m256i) == 64) - 1];
+#else
+static int a1[(sizeof(m256i) == 36) - 1];
+#endif
+
+struct s_m256d {
+  int a;
+  __m256d b;
+};
+typedef struct s_m256d m256d;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m256d) == 64) - 1];
+#else
+static int a1[(sizeof(m256d) == 36) - 1];
+#endif
+
+struct s_m512 {
+  int a;
+  __m512 b;
+};
+typedef struct s_m512 m512;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m512) == 128) - 1];
+#else
+static int a1[(sizeof(m512) == 68) - 1];
+#endif
+
+struct s_m512i {
+  int a;
+  __m512i b;
+};
+typedef struct s_m512i m512i;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m512i) == 128) - 1];
+#else
+static int a1[(sizeof(m512i) == 68) - 1];
+#endif
+
+struct s_m512d {
+  int a;
+  __m512d b;
+};
+typedef struct s_m512d m512d;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m512d) == 128) - 1];
+#else
+static int a1[(sizeof(m512d) == 68) - 1];
+#endif
+
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m128
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 16 |   __m128 b
+// CHECK-MS:| [sizeof=32, align=16]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m128i
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 16 |   __m128i b
+// CHECK-MS:| [sizeof=32, align=16]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m128d
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 16 |   __m128d b
+// CHECK-MS:| [sizeof=32, align=16]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m256
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 32 |   __m256 b
+// CHECK-MS:| [sizeof=64, align=32]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m256i
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 32 |   __m256i b
+// CHECK-MS:| [sizeof=64, align=32]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m256d
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 32 |   __m256d b
+// CHECK-MS:| [sizeof=64, align=32]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m512
+// CHECK

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

Re: case when STy != Loc.getElementType() - this is already covered by other 
Clang tests.

I'm still unsure about the heuristic here. I believe that for 
auto-initialization we want to be quite aggressive with these splits (unlike 
for regular constant stores).
Perhaps we should do the split in the case all bytes are pattern bytes? (This 
is probably another use case for `forInit`)


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

https://reviews.llvm.org/D57898



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

r353542 - [OPENMP]Delay emission of the error messages for the exceptions.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Feb  8 10:02:25 2019
New Revision: 353542

URL: http://llvm.org/viewvc/llvm-project?rev=353542&view=rev
Log:
[OPENMP]Delay emission of the error messages for the exceptions.

Fixed diagnostic emission for the exceptions support in case of the
compilation of OpenMP code for the devices. From now on, it uses delayed
diagnostics mechanism, previously used for CUDA only. It allow to
diagnose not allowed used of exceptions only in functions that are going
to be codegen'ed.

Modified:
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/test/OpenMP/nvptx_target_exceptions_messages.cpp

Modified: cfe/trunk/include/clang/Basic/LangOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.def?rev=353542&r1=353541&r2=353542&view=diff
==
--- cfe/trunk/include/clang/Basic/LangOptions.def (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.def Fri Feb  8 10:02:25 2019
@@ -203,7 +203,6 @@ LANGOPT(OpenMPUseTLS  , 1, 0, "Use T
 LANGOPT(OpenMPIsDevice, 1, 0, "Generate code only for OpenMP target 
device")
 LANGOPT(OpenMPCUDAMode, 1, 0, "Generate code for OpenMP pragmas in 
SIMT/SPMD mode")
 LANGOPT(OpenMPCUDAForceFullRuntime , 1, 0, "Force to use full runtime in all 
constructs when offloading to CUDA devices")
-LANGOPT(OpenMPHostCXXExceptions, 1, 0, "C++ exceptions handling in the 
host code.")
 LANGOPT(OpenMPCUDANumSMs  , 32, 0, "Number of SMs for CUDA devices.")
 LANGOPT(OpenMPCUDABlocksPerSM  , 32, 0, "Number of blocks per SM for CUDA 
devices.")
 LANGOPT(OpenMPOptimisticCollapse  , 1, 0, "Use at most 32 bits to represent 
the collapsed loop nest counter.")

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=353542&r1=353541&r2=353542&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Fri Feb  8 10:02:25 2019
@@ -8763,6 +8763,9 @@ private:
   /// Pop OpenMP function region for non-capturing function.
   void popOpenMPFunctionRegion(const sema::FunctionScopeInfo *OldFSI);
 
+  /// Check whether we're allowed to call Callee from the current function.
+  void checkOpenMPDeviceFunction(SourceLocation Loc, FunctionDecl *Callee);
+
   /// Checks if a type or a declaration is disabled due to the owning extension
   /// being disabled, and emits diagnostic messages if it is disabled.
   /// \param D type or declaration to be checked.
@@ -10249,6 +10252,23 @@ public:
   /// Same as CUDADiagIfDeviceCode, with "host" and "device" switched.
   DeviceDiagBuilder CUDADiagIfHostCode(SourceLocation Loc, unsigned DiagID);
 
+  /// Creates a DeviceDiagBuilder that emits the diagnostic if the current
+  /// context is "used as device code".
+  ///
+  /// - If CurContext is a `declare target` function or it is known that the
+  /// function is emitted for the device, emits the diagnostics immediately.
+  /// - If CurContext is a non-`declare target` function and we are compiling
+  ///   for the device, creates a diagnostic which is emitted if and when we
+  ///   realize that the function will be codegen'ed.
+  ///
+  /// Example usage:
+  ///
+  ///  // Variable-length arrays are not allowed in NVPTX device code.
+  ///  if (diagIfOpenMPDeviceCode(Loc, diag::err_vla_unsupported))
+  ///return ExprError();
+  ///  // Otherwise, continue parsing as normal.
+  DeviceDiagBuilder diagIfOpenMPDeviceCode(SourceLocation Loc, unsigned 
DiagID);
+
   enum CUDAFunctionTarget {
 CFT_Device,
 CFT_Global,

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=353542&r1=353541&r2=353542&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Fri Feb  8 10:02:25 2019
@@ -2827,7 +2827,6 @@ static void ParseLangArgs(LangOptions &O
 
   // Set the flag to prevent the implementation from emitting device exception
   // handling code for those requiring so.
-  Opts.OpenMPHostCXXExceptions = Opts.Exceptions && Opts.CXXExceptions;
   if ((Opts.OpenMPIsDevice && T.isNVPTX()) || Opts.OpenCLCPlusPlus) {
 Opts.Exceptions = 0;
 Opts.CXXExceptions = 0;

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=353542&r1=353541&r2=353542&view=diff
==
--- cfe/trunk/lib/Sema

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

Can you add a link to bug 40605 in the commit message?

> I'm not quite sure how to show the resulting difference in code. Do you mean 
> we need Clang to support both modes and to compare the resulting assembly?

I only meant tests that show codegen, as you've now added auto-var-init.cpp. 
There might be interesting cases which I wasn't testing when I wrote it, so if 
you think of any please do make sure you add some. You mentioned a Linux struct 
that used to crash? That would be a useful test (I imagine it's one with the 
`Loc` adjustment).




Comment at: tools/clang/lib/CodeGen/CGDecl.cpp:1143
+const llvm::StructLayout *Layout =
+CGM.getDataLayout().getStructLayout(cast(Ty));
+for (unsigned i = 0; i != constant->getNumOperands(); i++) {

glider wrote:
> jfb wrote:
> > I think you need a heuristic here, where you don't emit multiple stores if 
> > `getNumOperands` is greater than some number. In that case you'd keep doing 
> > memcpy instead. Note that a struct containing sub-structs (or arrays) 
> > should also count the number of sub-structs (so just checking 
> > `getNumOperands` isn't sufficient).
> > 
> > Other parts of this code chose 6 stores...
> Do we ever need to not split these stores? Can't the  MemCpyOpt pass take 
> care of them later on?
You're assuming the optimizer is running at all :-)
In general your approach seems to care about `-O2`, and I agree that's usually 
what we want to tune. However, clang also support `-O0` which won't touch these 
stores at all, and in `-O0` it's usually nice to just see a copy from a global 
while debugging. Further, configurations such as `-Os` and `-Oz` would be 
hampered by excessive codegen.

You also need to consider the compile-time hit the expansion you're making 
would add. More code means more time to compile, and I'd expect little to no 
win from emitting more than a handful of stores. If someone has a large thing 
on the stack, and it can be optimized, then we should teach the optimizer to 
deal with memcpy better. I don't think e.g. a 400+ byte struct makes sense to 
initialize with a bunch of store.

So I think you want to check what the size of the struct is (check out when 
x86-64 and ARM64 inline memcpy / memset, I assume those are decent guesses for 
size to inline).

Further, your current code sill do something silly for code like this:
```
struct S { int j; char arr[8]; float f; };
```
Before you'd have a single `memcpy`, but now it'll be a store, a `memset`, and 
a store. I think you really want to have the same heuristic for arrays (so 
you'd have just stores), and you want to make sure that if you're going to 
split up a struct you do so for all the fields inside that struct.



Comment at: tools/clang/lib/CodeGen/CGDecl.cpp:1765
+  emitStoresForConstant(CGM, D, Loc, isVolatile, Builder, constant,
+/*forInit*/ false);
 }

glider wrote:
> jfb wrote:
> > Can you explain why this case needs to be different? IIUC it's because the 
> > types can differ which makes the struct's elements not match up the ones 
> > from the constant in the loop above? I think the code above should 
> > automatically handle that case instead of passing in a flag here.
> > 
> > You also definitely need to test the case where that happens :)
> I just thought we shouldn't break the constants that we didn't create with 
> -ftrivial-var-auto-init.
> I'll take a closer look though (indeed, this crashes on one of the structs 
> from the Linux kernel)
Yeah I think it's from the `Loc` adjustment above. I think your change is worth 
doing for all cases, not just trivial var auto-init. It'll lead us to optimize 
all of it much better IMO (and support things like `-O0` `-Os` and `-Oz` 
uniformly well too).



Comment at: tools/clang/test/CodeGenCXX/auto-var-init.cpp:37
 // PATTERN: @__const.test_small_custom.custom = private unnamed_addr constant 
%struct.small { i8 42 }, align 1
 // ZERO: @__const.test_small_custom.custom = private unnamed_addr constant 
%struct.small { i8 42 }, align 1
 struct small { char c; };

How come `@__const.test_small_custom.custom` still gets emitted? I guess custom 
initialization goes through a different code path?



Comment at: tools/clang/test/CodeGenCXX/auto-var-init.cpp:496
 // ZERO-LABEL: @test_smallpartinit_uninit()
 // ZERO: call void @llvm.memset{{.*}}, i8 0,
 

I wonder if (maybe in a separate patch) you also want to avoid `memset` when 
something is pretty small.



Comment at: tools/clang/test/CodeGenCXX/auto-var-init.cpp:664
+// PATTERN: store i32 -1431655766, {{.*}} align 4
+// PATTERN: call void @llvm.memset.{{.*}}(i8* align 4 {{.*}}, i8 0, i64 0, i1 
false)
 // ZERO-LABEL: @test_arraytail_uninit()

This one is particularly silly since it's a zero-sized `memset` :)


CHANGES SINCE LAST ACTION
  https://revie

[PATCH] D57961: [X86] Add explicit alignment to __m128/__m128i/__m128d/etc. to allow matching of MSVC behavior with #pragma pack.

[Erich Keane via Phabricator via cfe-commits]

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

The justification is correct based on my research into this problem, and the 
code changes themselves look correct.  I cannot think of any reason why 
re-stating the alignment will matter in GCC mode, so I think this is OK.

I'll accept, but I'd really like to give @rnk a chance to take a look to make 
sure there isn't something I'm missing.




Comment at: test/CodeGen/x86-vec-struct-packing.c:14
+#if defined(_WIN32)
+static int a1[(sizeof(m128) == 32) - 1];
+#else

We default to C++14, do we not use static-asserts for this?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57961



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

[PATCH] D57961: [X86] Add explicit alignment to __m128/__m128i/__m128d/etc. to allow matching of MSVC behavior with #pragma pack.

[Craig Topper via Phabricator via cfe-commits]

craig.topper updated this revision to Diff 185996.
craig.topper added a comment.

Forgot to do MMX's __m64 type


Repository:
  rC Clang

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

https://reviews.llvm.org/D57961

Files:
  lib/Headers/avx512bwintrin.h
  lib/Headers/avx512fintrin.h
  lib/Headers/avx512vlbwintrin.h
  lib/Headers/avx512vlintrin.h
  lib/Headers/avxintrin.h
  lib/Headers/emmintrin.h
  lib/Headers/mmintrin.h
  lib/Headers/xmmintrin.h
  test/CodeGen/x86-vec-struct-packing.c

Index: test/CodeGen/x86-vec-struct-packing.c
===
--- /dev/null
+++ test/CodeGen/x86-vec-struct-packing.c
@@ -0,0 +1,227 @@
+// RUN: %clang_cc1 -ffreestanding -emit-llvm-only  -triple x86_64-windows-coff -fdump-record-layouts %s | FileCheck %s --check-prefix=CHECK-MS
+// RUN: %clang_cc1 -ffreestanding -emit-llvm-only  -triple x86_64-apple-darwin -fdump-record-layouts %s | FileCheck %s --check-prefix=CHECK-NOTMS
+#include 
+
+#pragma pack(1)
+
+struct s_m64 {
+  int a;
+  __m64 b;
+};
+typedef struct s_m64 m64;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m64) == 16) - 1];
+#else
+static int a1[(sizeof(m64) == 12) - 1];
+#endif
+
+struct s_m128 {
+  int a;
+  __m128 b;
+};
+typedef struct s_m128 m128;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m128) == 32) - 1];
+#else
+static int a1[(sizeof(m128) == 20) - 1];
+#endif
+
+struct s_m128i {
+  int a;
+  __m128i b;
+};
+typedef struct s_m128i m128i;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m128i) == 32) - 1];
+#else
+static int a1[(sizeof(m128i) == 20) - 1];
+#endif
+
+struct s_m128d {
+  int a;
+  __m128d b;
+};
+typedef struct s_m128d m128d;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m128d) == 32) - 1];
+#else
+static int a1[(sizeof(m128d) == 20) - 1];
+#endif
+
+struct s_m256 {
+  int a;
+  __m256 b;
+};
+typedef struct s_m256 m256;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m256) == 64) - 1];
+#else
+static int a1[(sizeof(m256) == 36) - 1];
+#endif
+
+struct s_m256i {
+  int a;
+  __m256i b;
+};
+typedef struct s_m256i m256i;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m256i) == 64) - 1];
+#else
+static int a1[(sizeof(m256i) == 36) - 1];
+#endif
+
+struct s_m256d {
+  int a;
+  __m256d b;
+};
+typedef struct s_m256d m256d;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m256d) == 64) - 1];
+#else
+static int a1[(sizeof(m256d) == 36) - 1];
+#endif
+
+struct s_m512 {
+  int a;
+  __m512 b;
+};
+typedef struct s_m512 m512;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m512) == 128) - 1];
+#else
+static int a1[(sizeof(m512) == 68) - 1];
+#endif
+
+struct s_m512i {
+  int a;
+  __m512i b;
+};
+typedef struct s_m512i m512i;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m512i) == 128) - 1];
+#else
+static int a1[(sizeof(m512i) == 68) - 1];
+#endif
+
+struct s_m512d {
+  int a;
+  __m512d b;
+};
+typedef struct s_m512d m512d;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m512d) == 128) - 1];
+#else
+static int a1[(sizeof(m512d) == 68) - 1];
+#endif
+
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m64
+// CHECK-MS:  0 |   int a
+// CHECK-MS:  8 |   __m64 b
+// CHECK-MS:| [sizeof=16, align=8]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m128
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 16 |   __m128 b
+// CHECK-MS:| [sizeof=32, align=16]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m128i
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 16 |   __m128i b
+// CHECK-MS:| [sizeof=32, align=16]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m128d
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 16 |   __m128d b
+// CHECK-MS:| [sizeof=32, align=16]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m256
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 32 |   __m256 b
+// CHECK-MS:| [sizeof=64, align=32]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m256i
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 32 |   __m256i b
+// CHECK-MS:| [sizeof=64, align=32]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m256d
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 32 |   __m256d b
+// CHECK-MS:| [sizeof=64, align=32]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m512
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 64 |   __m512 b
+// CHECK-MS:| [sizeof=128, align=64]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m512i
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 64 |   __m512i b
+// CHECK-MS:| [sizeof=128, align=64]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m512d
+// CHECK-MS:  

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

> Can you add a link to bug 40605 in the commit message?

It's in the title now, doesn't that count? :)


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

https://reviews.llvm.org/D57898



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

[PATCH] D57898: CodeGen: Fix PR40605: split constant structures generated by -ftrivial-auto-var-init when emitting initializators

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D57898#1390816 , @glider wrote:

> > Can you add a link to bug 40605 in the commit message?
>
> It's in the title now, doesn't that count? :)


Oh OK I hadn't seen that.


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

https://reviews.llvm.org/D57898



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

Re: r328173 - Improve -Winfinite-recursion

[Steven Wu via cfe-commits]

Hi Robert

I ping'ed this commit on Phabricator with an example of false positive 
introduced by this patch. Can you take a look?

Thanks

Steven

> On Mar 21, 2018, at 8:16 PM, Robert Widmann via cfe-commits 
>  wrote:
> 
> Author: codafi
> Date: Wed Mar 21 20:16:23 2018
> New Revision: 328173
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=328173&view=rev
> Log:
> Improve -Winfinite-recursion
> 
> Summary: Rewrites -Winfinite-recursion to remove the state dictionary and 
> explore paths in loops - especially infinite loops.  The new check now 
> detects recursion in loop bodies dominated by a recursive call.
> 
> Reviewers: rsmith, rtrieu
> 
> Reviewed By: rtrieu
> 
> Subscribers: lebedev.ri, cfe-commits
> 
> Differential Revision: https://reviews.llvm.org/D43737
> 
> Modified:
>cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
>cfe/trunk/test/SemaCXX/warn-infinite-recursion.cpp
> 
> Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=328173&r1=328172&r2=328173&view=diff
> ==
> --- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
> +++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Wed Mar 21 20:16:23 2018
> @@ -200,60 +200,41 @@ static bool hasRecursiveCallInPath(const
>   return false;
> }
> 
> -// All blocks are in one of three states.  States are ordered so that blocks
> -// can only move to higher states.
> -enum RecursiveState {
> -  FoundNoPath,
> -  FoundPath,
> -  FoundPathWithNoRecursiveCall
> -};
> -
> -// Returns true if there exists a path to the exit block and every path
> -// to the exit block passes through a call to FD.
> +// Returns true if every path from the entry block passes through a call to 
> FD.
> static bool checkForRecursiveFunctionCall(const FunctionDecl *FD, CFG *cfg) {
> +  llvm::SmallPtrSet Visited;
> +  llvm::SmallVector WorkList;
> +  // Keep track of whether we found at least one recursive path.
> +  bool foundRecursion = false;
> 
>   const unsigned ExitID = cfg->getExit().getBlockID();
> 
> -  // Mark all nodes as FoundNoPath, then set the status of the entry block.
> -  SmallVector States(cfg->getNumBlockIDs(), FoundNoPath);
> -  States[cfg->getEntry().getBlockID()] = FoundPathWithNoRecursiveCall;
> -
> -  // Make the processing stack and seed it with the entry block.
> -  SmallVector Stack;
> -  Stack.push_back(&cfg->getEntry());
> -
> -  while (!Stack.empty()) {
> -CFGBlock *CurBlock = Stack.back();
> -Stack.pop_back();
> -
> -unsigned ID = CurBlock->getBlockID();
> -RecursiveState CurState = States[ID];
> -
> -if (CurState == FoundPathWithNoRecursiveCall) {
> -  // Found a path to the exit node without a recursive call.
> -  if (ExitID == ID)
> -return false;
> -
> -  // Only change state if the block has a recursive call.
> -  if (hasRecursiveCallInPath(FD, *CurBlock))
> -CurState = FoundPath;
> -}
> +  // Seed the work list with the entry block.
> +  WorkList.push_back(&cfg->getEntry());
> 
> -// Loop over successor blocks and add them to the Stack if their state
> -// changes.
> -for (auto I = CurBlock->succ_begin(), E = CurBlock->succ_end(); I != E; 
> ++I)
> -  if (*I) {
> -unsigned next_ID = (*I)->getBlockID();
> -if (States[next_ID] < CurState) {
> -  States[next_ID] = CurState;
> -  Stack.push_back(*I);
> +  while (!WorkList.empty()) {
> +CFGBlock *Block = WorkList.pop_back_val();
> +
> +for (auto I = Block->succ_begin(), E = Block->succ_end(); I != E; ++I) {
> +  if (CFGBlock *SuccBlock = *I) {
> +if (!Visited.insert(SuccBlock).second)
> +  continue;
> +
> +// Found a path to the exit node without a recursive call.
> +if (ExitID == SuccBlock->getBlockID())
> +  return false;
> +
> +// If the successor block contains a recursive call, end analysis 
> there.
> +if (hasRecursiveCallInPath(FD, *SuccBlock)) {
> +  foundRecursion = true;
> +  continue;
> }
> +
> +WorkList.push_back(SuccBlock);
>   }
> +}
>   }
> -
> -  // Return true if the exit node is reachable, and only reachable through
> -  // a recursive call.
> -  return States[ExitID] == FoundPath;
> +  return foundRecursion;
> }
> 
> static void checkRecursiveFunction(Sema &S, const FunctionDecl *FD,
> @@ -269,10 +250,6 @@ static void checkRecursiveFunction(Sema
>   CFG *cfg = AC.getCFG();
>   if (!cfg) return;
> 
> -  // If the exit block is unreachable, skip processing the function.
> -  if (cfg->getExit().pred_empty())
> -return;
> -
>   // Emit diagnostic if a recursive function call is detected for all paths.
>   if (checkForRecursiveFunctionCall(FD, cfg))
> S.Diag(Body->getLocStart(), diag::warn_infinite_recursive_function);
> 
> Modified: cfe/trunk/test/SemaCXX/warn-infinite-recursion.cpp
> URL: 
> 

[PATCH] D57961: [X86] Add explicit alignment to __m128/__m128i/__m128d/etc. to allow matching of MSVC behavior with #pragma pack.

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a subscriber: rjmccall.
rnk added a comment.

Sounds good.

This change reminded me of D46042 , 
https://crbug.com/849251, and rL333791 , 
which I had to revert, and I don't think it relanded. I think your change to 
add the *_u typedefs fixes the issues encountered there, and perhaps @rjmccall 
will be able to reland his change after this lands.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57961



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

[PATCH] D54978: Move the SMT API to LLVM

[Brian Rzycki via Phabricator via cfe-commits]

brzycki added a comment.

In D54978#1390698 , @thakis wrote:

> Thanks for the analysis. I think it's fine if you revert, given that.


I'm running in to conflict dependency issues when attempting to revert rL353373 
. There is at least one other conflicting 
commit rL353465  on top of this code already.

I don't feel comfortable reverting 2+ patches in a sub-section of the code I 
know little about on a Friday afternoon. :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



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

[PATCH] D54978: Move the SMT API to LLVM

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

> There is at least one other conflicting commit rL353465 
>  on top of this code already.

Sorry about that. I think it would be fine to revert both or to replay the 
other commit on top of reverted version of this one.

@mikhail.ramalho could you take a look?


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



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

[PATCH] D57965: Clean up ObjCPropertyDecl printing

[David Goldman via Phabricator via cfe-commits]

dgoldman created this revision.
Herald added subscribers: cfe-commits, jfb, arphaman.
Herald added a project: clang.

- `@property(attr, attr2)` instead of `@property ( attr,attr2 )`.
- Change priority of attributes (see code/comments inline).
- Support for printing weak and unsafe_unretained attributes.


Repository:
  rC Clang

https://reviews.llvm.org/D57965

Files:
  lib/AST/DeclPrinter.cpp
  test/Index/comment-objc-decls.m
  test/Index/comment-unqualified-objc-pointer.m
  test/PCH/chain-remap-types.m

Index: test/PCH/chain-remap-types.m
===
--- test/PCH/chain-remap-types.m
+++ test/PCH/chain-remap-types.m
@@ -6,7 +6,7 @@
 
 // CHECK: @class X;
 // CHECK: struct Y 
-// CHECK: @property ( assign,readwrite,atomic ) X * prop
+// CHECK: @property(atomic, assign, unsafe_unretained, readwrite) X * prop
 // CHECK: void h(X *);
 // CHECK: @interface X(Blah)
 // CHECK: void g(X *);
Index: test/Index/comment-unqualified-objc-pointer.m
===
--- test/Index/comment-unqualified-objc-pointer.m
+++ test/Index/comment-unqualified-objc-pointer.m
@@ -19,7 +19,7 @@
 
 //! This is a property to get the Name.
 @property (copy) NSString *Name;
-// CHECK: @property(readwrite, copy, atomic) NSString *Name;
+// CHECK: @property(atomic, copy, readwrite) NSString *Name;
 @end
 
 @implementation NSMutableArray
Index: test/Index/comment-objc-decls.m
===
--- test/Index/comment-objc-decls.m
+++ test/Index/comment-objc-decls.m
@@ -32,7 +32,7 @@
 @end
 // CHECK: @protocol MyProto\n@end
 // CHECK: - (unsigned int)MethodMyProto:(nullable id)anObject inRange:(unsigned int)range;
-// CHECK: @optional\n@property(readwrite, copy, atomic, nonnull) id PropertyMyProto;
+// CHECK: @optional\n@property(atomic, copy, readwrite, nonnull) id PropertyMyProto;
 // CHECK: + (id)ClassMethodMyProto;
 
 /**
@@ -77,7 +77,7 @@
 // CHECK: id IvarMyClass
 // CHECK: - (id)MethodMyClass;
 // CHECK: + (id)ClassMethodMyClass;
-// CHECK: @property(readwrite, copy, atomic) id PropertyMyClass;@property(atomic, copy, readwrite) id PropertyMyClass;@interface MyClass (Category)\n@end
 // CHECK: - (void)MethodMyClassCategory;
-// CHECK: @property(readwrite, copy, atomic) id PropertyMyClassCategory;
+// CHECK: @property(atomic, copy, readwrite) id PropertyMyClassCategory;
 // CHECK: - (id)PropertyMyClassCategory;
 // CHECK: - (void)setPropertyMyClassCategory:(id)arg;
 
Index: lib/AST/DeclPrinter.cpp
===
--- lib/AST/DeclPrinter.cpp
+++ lib/AST/DeclPrinter.cpp
@@ -1389,6 +1389,13 @@
 
 /// PrintObjCPropertyDecl - print a property declaration.
 ///
+/// Print attributes in the following order:
+/// - class
+/// - nonatomic | atomic
+/// - assign | retain | strong | copy | weak | unsafe_unretained
+/// - readwrite | readonly
+/// - getter & setter
+/// - nullability
 void DeclPrinter::VisitObjCPropertyDecl(ObjCPropertyDecl *PDecl) {
   if (PDecl->getPropertyImplementation() == ObjCPropertyDecl::Required)
 Out << "@required\n";
@@ -1400,58 +1407,69 @@
   Out << "@property";
   if (PDecl->getPropertyAttributes() != ObjCPropertyDecl::OBJC_PR_noattr) {
 bool first = true;
-Out << " (";
-if (PDecl->getPropertyAttributes() &
-ObjCPropertyDecl::OBJC_PR_readonly) {
-  Out << (first ? ' ' : ',') << "readonly";
+Out << "(";
+if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_class) {
+  Out << (first ? "" : ", ") << "class";
   first = false;
 }
 
-if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_getter) {
-  Out << (first ? ' ' : ',') << "getter = ";
-  PDecl->getGetterName().print(Out);
+if (PDecl->getPropertyAttributes() &
+ObjCPropertyDecl::OBJC_PR_nonatomic) {
+  Out << (first ? "" : ", ") << "nonatomic";
   first = false;
 }
-if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_setter) {
-  Out << (first ? ' ' : ',') << "setter = ";
-  PDecl->getSetterName().print(Out);
+if (PDecl->getPropertyAttributes() &
+ObjCPropertyDecl::OBJC_PR_atomic) {
+  Out << (first ? "" : ", ") << "atomic";
   first = false;
 }
 
 if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_assign) {
-  Out << (first ? ' ' : ',') << "assign";
-  first = false;
-}
-
-if (PDecl->getPropertyAttributes() &
-ObjCPropertyDecl::OBJC_PR_readwrite) {
-  Out << (first ? ' ' : ',') << "readwrite";
+  Out << (first ? "" : ", ") << "assign";
   first = false;
 }
-
 if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_retain) {
-  Out << (first ? ' ' : ',') << "retain";
+  Out << (first ? "" : ", ") << "retain";
   first = false;
 }
 
 if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_strong) {
-  Out << (first ? ' 

[PATCH] D57966: [clang-tidy] add camelBackOrCase casing style to readability-identifier-naming to support change to variable naming policy (if adopted)

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: michaelplatings, aaron.ballman, JonasToth, 
hokein.
MyDeveloperDay added a project: clang-tools-extra.
Herald added a subscriber: xazax.hun.
Herald added a project: clang.

Introduction of a new variable policy (as outlined in D57896: Variable names 
rule ) could lead to clang tidy complaining 
about 1000's of variables

Consider add a new casing style to `readability-identifier-naming` to support 
allowing either `camelBack` and `camelCase` styles which could be applied to 
the VariableCase option in the LLVM .clang-tidy files

Users wishing to convert their files to new policy would simply change this 
setting to `camelBack` and run with -fix

Depends on D57896 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D57966

Files:
  clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tidy/readability/IdentifierNamingCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/readability-identifier-naming.rst
  test/clang-tidy/readability-identifier-naming-camelback-or-case.cpp

Index: test/clang-tidy/readability-identifier-naming-camelback-or-case.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-identifier-naming-camelback-or-case.cpp
@@ -0,0 +1,117 @@
+// Remove UNSUPPORTED for powerpc64le when the problem introduced by
+// r288563 is resolved.
+// UNSUPPORTED: powerpc64le
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: readability-identifier-naming.AbstractClassCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.ClassCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.ClassConstantCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.ClassMemberCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.ClassMethodCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.ConstantCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.ConstexprFunctionCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.ConstexprMethodCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.ConstexprVariableCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.EnumCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.EnumConstantCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.FunctionCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.GlobalConstantCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.GlobalFunctionCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.GlobalVariableCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.InlineNamespaceCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.LocalConstantCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.LocalVariableCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.MemberCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.ConstantMemberCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.PublicMemberCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.MethodCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.NamespaceCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.ParameterCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.ConstantParameterCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.ParameterPackCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.PureFunctionCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.PureMethodCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.StaticConstantCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.StaticVariableCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.StructCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.TemplateParameterCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.TemplateTemplateParameterCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.TemplateUsingCase, value: camelCaseOrBack}, \
+// RUN: {key: readability-identifier-naming.TypeTemplateParameterCase, value: camelBackOrCase}, \
+// RUN: {key: readability-identifier-naming.TypedefCase, value: came

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

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

rsmith added a comment.

You'll also need to update `TreeTransform::TransformCompoundStmt`. (And please 
add some tests for template instantiation.)


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

https://reviews.llvm.org/D57086



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

[PATCH] D57852: [clang-tidy] Don't use assignment for value-initialized enums

[Malcolm Parsons via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE353554: [clang-tidy] Don't use assignment for 
value-initialized enums (authored by malcolm.parsons, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57852?vs=185649&id=186014#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57852

Files:
  clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
  test/clang-tidy/modernize-use-default-member-init-assignment.cpp
  test/clang-tidy/modernize-use-default-member-init.cpp


Index: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
===
--- clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
+++ clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
@@ -255,17 +255,20 @@
   CharSourceRange InitRange =
   CharSourceRange::getCharRange(LParenEnd, Init->getRParenLoc());
 
+  bool ValueInit = isa(Init->getInit());
+  bool CanAssign = UseAssignment && (!ValueInit || 
!Init->getInit()->getType()->isEnumeralType());
+
   auto Diag =
   diag(Field->getLocation(), "use default member initializer for %0")
   << Field
-  << FixItHint::CreateInsertion(FieldEnd, UseAssignment ? " = " : "{")
+  << FixItHint::CreateInsertion(FieldEnd, CanAssign ? " = " : "{")
   << FixItHint::CreateInsertionFromRange(FieldEnd, InitRange);
 
-  if (UseAssignment && isa(Init->getInit()))
+  if (CanAssign && ValueInit)
 Diag << FixItHint::CreateInsertion(
 FieldEnd, getValueOfValueInit(Init->getInit()->getType()));
 
-  if (!UseAssignment)
+  if (!CanAssign)
 Diag << FixItHint::CreateInsertion(FieldEnd, "}");
 
   Diag << FixItHint::CreateRemoval(Init->getSourceRange());
Index: test/clang-tidy/modernize-use-default-member-init.cpp
===
--- test/clang-tidy/modernize-use-default-member-init.cpp
+++ test/clang-tidy/modernize-use-default-member-init.cpp
@@ -165,6 +165,14 @@
   // CHECK-FIXES: Enum e{Foo};
 };
 
+struct PositiveValueEnum {
+  PositiveValueEnum() : e() {}
+  // CHECK-FIXES: PositiveValueEnum()  {}
+  Enum e;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use default member initializer 
for 'e'
+  // CHECK-FIXES: Enum e{};
+};
+
 struct PositiveString {
   PositiveString() : s("foo") {}
   // CHECK-FIXES: PositiveString()  {}
Index: test/clang-tidy/modernize-use-default-member-init-assignment.cpp
===
--- test/clang-tidy/modernize-use-default-member-init-assignment.cpp
+++ test/clang-tidy/modernize-use-default-member-init-assignment.cpp
@@ -166,6 +166,14 @@
   // CHECK-FIXES: Enum e = Foo;
 };
 
+struct PositiveValueEnum {
+  PositiveValueEnum() : e() {}
+  // CHECK-FIXES: PositiveValueEnum()  {}
+  Enum e;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use default member initializer 
for 'e'
+  // CHECK-FIXES: Enum e{};
+};
+
 struct PositiveString {
   PositiveString() : s("foo") {}
   // CHECK-FIXES: PositiveString()  {}


Index: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
===
--- clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
+++ clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
@@ -255,17 +255,20 @@
   CharSourceRange InitRange =
   CharSourceRange::getCharRange(LParenEnd, Init->getRParenLoc());
 
+  bool ValueInit = isa(Init->getInit());
+  bool CanAssign = UseAssignment && (!ValueInit || !Init->getInit()->getType()->isEnumeralType());
+
   auto Diag =
   diag(Field->getLocation(), "use default member initializer for %0")
   << Field
-  << FixItHint::CreateInsertion(FieldEnd, UseAssignment ? " = " : "{")
+  << FixItHint::CreateInsertion(FieldEnd, CanAssign ? " = " : "{")
   << FixItHint::CreateInsertionFromRange(FieldEnd, InitRange);
 
-  if (UseAssignment && isa(Init->getInit()))
+  if (CanAssign && ValueInit)
 Diag << FixItHint::CreateInsertion(
 FieldEnd, getValueOfValueInit(Init->getInit()->getType()));
 
-  if (!UseAssignment)
+  if (!CanAssign)
 Diag << FixItHint::CreateInsertion(FieldEnd, "}");
 
   Diag << FixItHint::CreateRemoval(Init->getSourceRange());
Index: test/clang-tidy/modernize-use-default-member-init.cpp
===
--- test/clang-tidy/modernize-use-default-member-init.cpp
+++ test/clang-tidy/modernize-use-default-member-init.cpp
@@ -165,6 +165,14 @@
   // CHECK-FIXES: Enum e{Foo};
 };
 
+struct PositiveValueEnum {
+  PositiveValueEnum() : e() {}
+  // CHECK-FIXES: PositiveValueEnum()  {}
+  Enum e;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use default member initializer for 'e'
+  // CHECK-FIXES: Enum e{};
+};
+
 struct PositiveString {
   PositiveString() : s("foo") {}
   // CHECK-FIXES: PositiveString()  {}
Index: test/clang-tidy/modernize-use-default-member-init-assignment

[clang-tools-extra] r353554 - [clang-tidy] Don't use assignment for value-initialized enums

[Malcolm Parsons via cfe-commits]

Author: malcolm.parsons
Date: Fri Feb  8 11:44:42 2019
New Revision: 353554

URL: http://llvm.org/viewvc/llvm-project?rev=353554&view=rev
Log:
[clang-tidy] Don't use assignment for value-initialized enums

Summary:
The modernize-use-default-member-init check crashes when trying to
create an assignment value for a value-initialized enum because it isn't a
BuiltinType.
An enum cannot be initialized by assigning 0 to it unless a cast is added.
It could be initialized with an enumerator with the value 0, but there might not
be one.
Avoid these issues by ignoring the UseAssignment setting for value-initialized
enums.

Fixes PR35050.

Reviewers: aaron.ballman, alexfh, JonasToth

Reviewed By: JonasToth

Subscribers: xazax.hun, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp

clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init-assignment.cpp

clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp?rev=353554&r1=353553&r2=353554&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp 
Fri Feb  8 11:44:42 2019
@@ -255,17 +255,20 @@ void UseDefaultMemberInitCheck::checkDef
   CharSourceRange InitRange =
   CharSourceRange::getCharRange(LParenEnd, Init->getRParenLoc());
 
+  bool ValueInit = isa(Init->getInit());
+  bool CanAssign = UseAssignment && (!ValueInit || 
!Init->getInit()->getType()->isEnumeralType());
+
   auto Diag =
   diag(Field->getLocation(), "use default member initializer for %0")
   << Field
-  << FixItHint::CreateInsertion(FieldEnd, UseAssignment ? " = " : "{")
+  << FixItHint::CreateInsertion(FieldEnd, CanAssign ? " = " : "{")
   << FixItHint::CreateInsertionFromRange(FieldEnd, InitRange);
 
-  if (UseAssignment && isa(Init->getInit()))
+  if (CanAssign && ValueInit)
 Diag << FixItHint::CreateInsertion(
 FieldEnd, getValueOfValueInit(Init->getInit()->getType()));
 
-  if (!UseAssignment)
+  if (!CanAssign)
 Diag << FixItHint::CreateInsertion(FieldEnd, "}");
 
   Diag << FixItHint::CreateRemoval(Init->getSourceRange());

Modified: 
clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init-assignment.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init-assignment.cpp?rev=353554&r1=353553&r2=353554&view=diff
==
--- 
clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init-assignment.cpp
 (original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init-assignment.cpp
 Fri Feb  8 11:44:42 2019
@@ -166,6 +166,14 @@ struct PositiveEnum {
   // CHECK-FIXES: Enum e = Foo;
 };
 
+struct PositiveValueEnum {
+  PositiveValueEnum() : e() {}
+  // CHECK-FIXES: PositiveValueEnum()  {}
+  Enum e;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use default member initializer 
for 'e'
+  // CHECK-FIXES: Enum e{};
+};
+
 struct PositiveString {
   PositiveString() : s("foo") {}
   // CHECK-FIXES: PositiveString()  {}

Modified: 
clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init.cpp?rev=353554&r1=353553&r2=353554&view=diff
==
--- 
clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init.cpp 
(original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init.cpp 
Fri Feb  8 11:44:42 2019
@@ -165,6 +165,14 @@ struct PositiveEnum {
   // CHECK-FIXES: Enum e{Foo};
 };
 
+struct PositiveValueEnum {
+  PositiveValueEnum() : e() {}
+  // CHECK-FIXES: PositiveValueEnum()  {}
+  Enum e;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use default member initializer 
for 'e'
+  // CHECK-FIXES: Enum e{};
+};
+
 struct PositiveString {
   PositiveString() : s("foo") {}
   // CHECK-FIXES: PositiveString()  {}


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

r353555 - [X86] Add explicit alignment to __m128/__m128i/__m128d/etc. to allow matching of MSVC behavior with #pragma pack.

[Craig Topper via cfe-commits]

Author: ctopper
Date: Fri Feb  8 11:45:08 2019
New Revision: 353555

URL: http://llvm.org/viewvc/llvm-project?rev=353555&view=rev
Log:
[X86] Add explicit alignment to __m128/__m128i/__m128d/etc. to allow matching 
of MSVC behavior with #pragma pack.

Summary:
With MSVC, #pragma pack is ignored when there is explicit alignment. This 
differs from gcc. Clang emulates this difference when compiling for Windows.

It appears that MSVC and its headers consider the __m128/__m128i/__m128d/etc. 
types to be explicitly aligned and ignores #pragma pack for them. Since we 
don't have explicit alignment on them in our headers, we don't match the MSVC 
behavior here.

This patch adds explicit alignment to match this behavior. I'm hoping this 
won't cause any problems when we're not emulating MSVC. But if someone knows of 
something that would be different we can swith to conditionally adding the 
alignment based on _MSC_VER.

I had to add explicitly unaligned types as well so we could use them in the 
loadu/storeu intrinsics which use __attribute__(__packed__). Using the now 
explicitly aligned types wouldn't produce align 1 accesses when targeting 
Windows.

Reviewers: rnk, erichkeane, spatel, RKSimon

Subscribers: cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/CodeGen/x86-vec-struct-packing.c
Modified:
cfe/trunk/lib/Headers/avx512bwintrin.h
cfe/trunk/lib/Headers/avx512fintrin.h
cfe/trunk/lib/Headers/avx512vlbwintrin.h
cfe/trunk/lib/Headers/avx512vlintrin.h
cfe/trunk/lib/Headers/avxintrin.h
cfe/trunk/lib/Headers/emmintrin.h
cfe/trunk/lib/Headers/mmintrin.h
cfe/trunk/lib/Headers/xmmintrin.h

Modified: cfe/trunk/lib/Headers/avx512bwintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512bwintrin.h?rev=353555&r1=353554&r2=353555&view=diff
==
--- cfe/trunk/lib/Headers/avx512bwintrin.h (original)
+++ cfe/trunk/lib/Headers/avx512bwintrin.h Fri Feb  8 11:45:08 2019
@@ -1751,7 +1751,7 @@ static __inline __m512i __DEFAULT_FN_ATT
 _mm512_loadu_epi16 (void const *__P)
 {
   struct __loadu_epi16 {
-__m512i __v;
+__m512i_u __v;
   } __attribute__((__packed__, __may_alias__));
   return ((struct __loadu_epi16*)__P)->__v;
 }
@@ -1777,7 +1777,7 @@ static __inline __m512i __DEFAULT_FN_ATT
 _mm512_loadu_epi8 (void const *__P)
 {
   struct __loadu_epi8 {
-__m512i __v;
+__m512i_u __v;
   } __attribute__((__packed__, __may_alias__));
   return ((struct __loadu_epi8*)__P)->__v;
 }
@@ -1803,7 +1803,7 @@ static __inline void __DEFAULT_FN_ATTRS5
 _mm512_storeu_epi16 (void *__P, __m512i __A)
 {
   struct __storeu_epi16 {
-__m512i __v;
+__m512i_u __v;
   } __attribute__((__packed__, __may_alias__));
   ((struct __storeu_epi16*)__P)->__v = __A;
 }
@@ -1820,7 +1820,7 @@ static __inline void __DEFAULT_FN_ATTRS5
 _mm512_storeu_epi8 (void *__P, __m512i __A)
 {
   struct __storeu_epi8 {
-__m512i __v;
+__m512i_u __v;
   } __attribute__((__packed__, __may_alias__));
   ((struct __storeu_epi8*)__P)->__v = __A;
 }

Modified: cfe/trunk/lib/Headers/avx512fintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512fintrin.h?rev=353555&r1=353554&r2=353555&view=diff
==
--- cfe/trunk/lib/Headers/avx512fintrin.h (original)
+++ cfe/trunk/lib/Headers/avx512fintrin.h Fri Feb  8 11:45:08 2019
@@ -40,9 +40,13 @@ typedef unsigned short __v32hu __attribu
 typedef unsigned long long __v8du __attribute__((__vector_size__(64)));
 typedef unsigned int __v16su __attribute__((__vector_size__(64)));
 
-typedef float __m512 __attribute__((__vector_size__(64)));
-typedef double __m512d __attribute__((__vector_size__(64)));
-typedef long long __m512i __attribute__((__vector_size__(64)));
+typedef float __m512 __attribute__((__vector_size__(64), __aligned__(64)));
+typedef double __m512d __attribute__((__vector_size__(64), __aligned__(64)));
+typedef long long __m512i __attribute__((__vector_size__(64), 
__aligned__(64)));
+
+typedef float __m512_u __attribute__((__vector_size__(64), __aligned__(1)));
+typedef double __m512d_u __attribute__((__vector_size__(64), __aligned__(1)));
+typedef long long __m512i_u __attribute__((__vector_size__(64), 
__aligned__(1)));
 
 typedef unsigned char __mmask8;
 typedef unsigned short __mmask16;
@@ -4324,7 +4328,7 @@ static __inline __m512i __DEFAULT_FN_ATT
 _mm512_loadu_si512 (void const *__P)
 {
   struct __loadu_si512 {
-__m512i __v;
+__m512i_u __v;
   } __attribute__((__packed__, __may_alias__));
   return ((struct __loadu_si512*)__P)->__v;
 }
@@ -4333,7 +4337,7 @@ static __inline __m512i __DEFAULT_FN_ATT
 _mm512_loadu_epi32 (void const *__P)
 {
   struct __loadu_epi32 {
-__m512i __v;
+__m512i_u __v;
   } __attribute__((__packed__, __may_alias__));
   return ((struct __loadu_epi32*)__P)->__v;
 }
@@ -4360,7 +43

[PATCH] D57961: [X86] Add explicit alignment to __m128/__m128i/__m128d/etc. to allow matching of MSVC behavior with #pragma pack.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC353555: [X86] Add explicit alignment to 
__m128/__m128i/__m128d/etc. to allow matching… (authored by ctopper, committed 
by ).

Changed prior to commit:
  https://reviews.llvm.org/D57961?vs=185996&id=186015#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D57961

Files:
  lib/Headers/avx512bwintrin.h
  lib/Headers/avx512fintrin.h
  lib/Headers/avx512vlbwintrin.h
  lib/Headers/avx512vlintrin.h
  lib/Headers/avxintrin.h
  lib/Headers/emmintrin.h
  lib/Headers/mmintrin.h
  lib/Headers/xmmintrin.h
  test/CodeGen/x86-vec-struct-packing.c

Index: test/CodeGen/x86-vec-struct-packing.c
===
--- test/CodeGen/x86-vec-struct-packing.c
+++ test/CodeGen/x86-vec-struct-packing.c
@@ -0,0 +1,227 @@
+// RUN: %clang_cc1 -ffreestanding -emit-llvm-only  -triple x86_64-windows-coff -fdump-record-layouts %s | FileCheck %s --check-prefix=CHECK-MS
+// RUN: %clang_cc1 -ffreestanding -emit-llvm-only  -triple x86_64-apple-darwin -fdump-record-layouts %s | FileCheck %s --check-prefix=CHECK-NOTMS
+#include 
+
+#pragma pack(1)
+
+struct s_m64 {
+  int a;
+  __m64 b;
+};
+typedef struct s_m64 m64;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m64) == 16) - 1];
+#else
+static int a1[(sizeof(m64) == 12) - 1];
+#endif
+
+struct s_m128 {
+  int a;
+  __m128 b;
+};
+typedef struct s_m128 m128;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m128) == 32) - 1];
+#else
+static int a1[(sizeof(m128) == 20) - 1];
+#endif
+
+struct s_m128i {
+  int a;
+  __m128i b;
+};
+typedef struct s_m128i m128i;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m128i) == 32) - 1];
+#else
+static int a1[(sizeof(m128i) == 20) - 1];
+#endif
+
+struct s_m128d {
+  int a;
+  __m128d b;
+};
+typedef struct s_m128d m128d;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m128d) == 32) - 1];
+#else
+static int a1[(sizeof(m128d) == 20) - 1];
+#endif
+
+struct s_m256 {
+  int a;
+  __m256 b;
+};
+typedef struct s_m256 m256;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m256) == 64) - 1];
+#else
+static int a1[(sizeof(m256) == 36) - 1];
+#endif
+
+struct s_m256i {
+  int a;
+  __m256i b;
+};
+typedef struct s_m256i m256i;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m256i) == 64) - 1];
+#else
+static int a1[(sizeof(m256i) == 36) - 1];
+#endif
+
+struct s_m256d {
+  int a;
+  __m256d b;
+};
+typedef struct s_m256d m256d;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m256d) == 64) - 1];
+#else
+static int a1[(sizeof(m256d) == 36) - 1];
+#endif
+
+struct s_m512 {
+  int a;
+  __m512 b;
+};
+typedef struct s_m512 m512;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m512) == 128) - 1];
+#else
+static int a1[(sizeof(m512) == 68) - 1];
+#endif
+
+struct s_m512i {
+  int a;
+  __m512i b;
+};
+typedef struct s_m512i m512i;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m512i) == 128) - 1];
+#else
+static int a1[(sizeof(m512i) == 68) - 1];
+#endif
+
+struct s_m512d {
+  int a;
+  __m512d b;
+};
+typedef struct s_m512d m512d;
+
+#if defined(_WIN32)
+static int a1[(sizeof(m512d) == 128) - 1];
+#else
+static int a1[(sizeof(m512d) == 68) - 1];
+#endif
+
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m64
+// CHECK-MS:  0 |   int a
+// CHECK-MS:  8 |   __m64 b
+// CHECK-MS:| [sizeof=16, align=8]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m128
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 16 |   __m128 b
+// CHECK-MS:| [sizeof=32, align=16]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m128i
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 16 |   __m128i b
+// CHECK-MS:| [sizeof=32, align=16]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m128d
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 16 |   __m128d b
+// CHECK-MS:| [sizeof=32, align=16]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m256
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 32 |   __m256 b
+// CHECK-MS:| [sizeof=64, align=32]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m256i
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 32 |   __m256i b
+// CHECK-MS:| [sizeof=64, align=32]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m256d
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 32 |   __m256d b
+// CHECK-MS:| [sizeof=64, align=32]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m512
+// CHECK-MS:  0 |   int a
+// CHECK-MS: 64 |   __m512 b
+// CHECK-MS:| [sizeof=128, align=64]
+// CHECK-MS: *** Dumping AST Record Layout
+// CHECK-MS:  0 | struct s_m512i
+// CH

r353556 - Pass the base element type of an array type to the visit method instead

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Fri Feb  8 11:46:53 2019
New Revision: 353556

URL: http://llvm.org/viewvc/llvm-project?rev=353556&view=rev
Log:
Pass the base element type of an array type to the visit method instead
of the array type itself.

This fixes a bug found by inspection that was introduced in r353459. I
don't have a test case for this since we don't yet have types that would
make the containing C struct non-trivial to copy/move but wouldn't make
it non-trivial to default-initialize or destruct.

Modified:
cfe/trunk/lib/AST/Type.cpp

Modified: cfe/trunk/lib/AST/Type.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=353556&r1=353555&r2=353556&view=diff
==
--- cfe/trunk/lib/AST/Type.cpp (original)
+++ cfe/trunk/lib/AST/Type.cpp Fri Feb  8 11:46:53 2019
@@ -2258,7 +2258,7 @@ struct IsNonTrivialCopyMoveVisitor
 
   bool visitWithKind(QualType::PrimitiveCopyKind PCK, QualType QT) {
 if (const auto *AT = this->Ctx.getAsArrayType(QT))
-  return this->asDerived().visit(QualType(AT, 0));
+  return this->asDerived().visit(Ctx.getBaseElementType(AT));
 return Super::visitWithKind(PCK, QT);
   }
 


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

[PATCH] D57918: Add an attribute that causes clang to emit fortified calls to C stdlib functions

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington updated this revision to Diff 186013.
erik.pilkington marked 5 inline comments as done.
erik.pilkington added a comment.

Address John's comments.


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

https://reviews.llvm.org/D57918

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Builtins.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Basic/Builtins.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/fortify-std-lib.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/fortify-std-lib.c

Index: clang/test/Sema/fortify-std-lib.c
===
--- /dev/null
+++ clang/test/Sema/fortify-std-lib.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -fsyntax-only %s -verify
+
+typedef unsigned long size_t;
+
+__attribute__((fortify_stdlib(0, 0)))
+int not_anything_special(); // expected-error {{'fortify_stdlib' attribute applied to an unknown function}}
+
+__attribute__((fortify_stdlib(4, 0))) // expected-error {{'fortify_stdlib' attribute requires integer constant between 0 and 3 inclusive}}
+int sprintf(char *, const char *, ...);
+
+__attribute__((fortify_stdlib())) // expected-error {{'fortify_stdlib' attribute requires exactly 2 arguments}}
+int sprintf(char *, const char *, ...);
+
+__attribute__((fortify_stdlib(1, 2, 3))) // expected-error {{'fortify_stdlib' attribute requires exactly 2 arguments}}
+int sprintf(char *, const char *, ...);
+
+__attribute__((fortify_stdlib(-1, 2))) // expected-error {{'fortify_stdlib' attribute requires a non-negative integral compile time constant expression}}
+int sprintf(char *, const char *, ...);
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -52,6 +52,7 @@
 // CHECK-NEXT: ExternalSourceSymbol ((SubjectMatchRule_record, SubjectMatchRule_enum, SubjectMatchRule_enum_constant, SubjectMatchRule_field, SubjectMatchRule_function, SubjectMatchRule_namespace, SubjectMatchRule_objc_category, SubjectMatchRule_objc_interface, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property, SubjectMatchRule_objc_protocol, SubjectMatchRule_record, SubjectMatchRule_type_alias, SubjectMatchRule_variable))
 // CHECK-NEXT: FlagEnum (SubjectMatchRule_enum)
 // CHECK-NEXT: Flatten (SubjectMatchRule_function)
+// CHECK-NEXT: FortifyStdLib (SubjectMatchRule_function)
 // CHECK-NEXT: GNUInline (SubjectMatchRule_function)
 // CHECK-NEXT: Hot (SubjectMatchRule_function)
 // CHECK-NEXT: IBAction (SubjectMatchRule_objc_method_is_instance)
Index: clang/test/CodeGen/fortify-std-lib.c
===
--- /dev/null
+++ clang/test/CodeGen/fortify-std-lib.c
@@ -0,0 +1,220 @@
+// RUN: %clang_cc1   -triple x86_64-apple-macosx10.14.0 %s -emit-llvm -O0 -disable-llvm-passes -o - -Wno-format-security | FileCheck %s
+// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -emit-llvm -O0 -disable-llvm-passes -o - -Wno-format-security | FileCheck %s
+
+#ifdef __cplusplus
+#define EXTERN extern "C"
+#else
+#define EXTERN
+#endif
+
+#define FSL(x,y) __attribute__((fortify_stdlib(x,y)))
+typedef unsigned long size_t;
+
+FSL(0, 0) EXTERN
+void *memcpy(void *dst, const void *src, size_t sz);
+
+EXTERN
+void call_memcpy(void *dst, const void *src, size_t sz) {
+  memcpy(dst, src, sz);
+  // CHECK-LABEL: define void @call_memcpy
+  // CHECK: [[REG:%[0-9]+]] = call i64 @llvm.objectsize.i64.p0i8(i8*{{.*}}, i1 false, i1 true, i1 false)
+  // CHECK: call i8* @__memcpy_chk(i8* {{.*}}, i8* {{.*}}, i64 {{.*}}, i64 [[REG]])
+}
+
+FSL(0, 0) EXTERN
+void *memmove(void *dst, const void *src, size_t sz);
+
+EXTERN
+void call_memmove(void *dst, const void *src, size_t sz) {
+  memmove(dst, src, sz);
+  // CHECK-LABEL: define void @call_memmove
+  // CHECK: [[REG:%[0-9]+]] = call i64 @llvm.objectsize.i64.p0i8(i8*{{.*}}, i1 false, i1 true, i1 false)
+  // CHECK: call i8* @__memmove_chk(i8* {{.*}}, i8* {{.*}}, i64 {{.*}}, i64 [[REG]])
+}
+
+FSL(0, 0) EXTERN
+void *memset(void *dst, int c, size_t sz);
+
+EXTERN
+void call_memset(void *dst, int c, size_t sz) {
+  memset(dst, c, sz);
+  // CHECK-LABEL: define void @call_memset
+  // CHECK: [[REG:%[0-9]+]] = call i64 @llvm.objectsize.i64.p0i8(i8*{{.*}}, i1 false, i1 true, i1 false)
+  // CHECK: call i8* @__memset_chk(i8* {{.*}}, i32 {{.*}}, i64 {{.*}}, i64 [[REG]])
+}
+
+FSL(0, 0) EXTERN
+char *stpcpy(char* dst, const char *src);
+
+EXTERN
+void call_stpcpy(char *dst, const char *src) {
+  stpcpy(dst, src);
+  // CHECK-LABEL: define void @call_stpcpy
+  // CHECK: [[REG:%[0-9]+]] = call i64 @llvm.objectsize.i6

[PATCH] D57918: Add an attribute that causes clang to emit fortified calls to C stdlib functions

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:986
+  }
+  }];
+}

rjmccall wrote:
> Probably worth clarifying somewhere in here that only a specific set of 
> stdlib functions are supported.  I don't know that it's important to spell 
> that set out.
I just copied a list in, there really aren't that many, so may as well just be 
explicit.


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

https://reviews.llvm.org/D57918



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

[PATCH] D57918: Add an attribute that causes clang to emit fortified calls to C stdlib functions

[John McCall via Phabricator via cfe-commits]

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

LGTM, thanks!




Comment at: clang/include/clang/Basic/AttrDocs.td:986
+  }
+  }];
+}

erik.pilkington wrote:
> rjmccall wrote:
> > Probably worth clarifying somewhere in here that only a specific set of 
> > stdlib functions are supported.  I don't know that it's important to spell 
> > that set out.
> I just copied a list in, there really aren't that many, so may as well just 
> be explicit.
WFM


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

https://reviews.llvm.org/D57918



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