[PATCH] D60463: [ASTImporter] Add check for correct import of source locations.

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

balazske added a comment.

This test will work theoretically only if the order of every imported Decl (and 
not only FieldDecl) is correct, this is not the case now. So probably a better 
solution for the problem should be found: Enumerate and match (the From and To) 
SourceLocations with AST visitor. There should be some existing code that is 
doing somewhat similar in clang-query but I did not find it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60463



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

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

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

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

ASTImporter contained wrong or missing imports of SourceLocation
and SourceRange for some objects. At least a part of such errors
is fixed now.
Tests for SourceLocation import do not exist yet. A separate
patch is needed to add a general SourceLocation import test
that runs on every import as part of the already existing tests.


Repository:
  rC Clang

https://reviews.llvm.org/D60499

Files:
  lib/AST/ASTImporter.cpp

Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2146,6 +2146,9 @@
   ExpectedSLoc BeginLocOrErr = import(D->getBeginLoc());
   if (!BeginLocOrErr)
 return BeginLocOrErr.takeError();
+  ExpectedSLoc RBraceLocOrErr = import(D->getRBraceLoc());
+  if (!RBraceLocOrErr)
+return RBraceLocOrErr.takeError();
 
   // Create the "to" namespace, if needed.
   NamespaceDecl *ToNamespace = MergeWithNamespace;
@@ -2155,6 +2158,7 @@
 *BeginLocOrErr, Loc, Name.getAsIdentifierInfo(),
 /*PrevDecl=*/nullptr))
   return ToNamespace;
+ToNamespace->setRBraceLoc(*RBraceLocOrErr);
 ToNamespace->setLexicalDeclContext(LexicalDC);
 LexicalDC->addDeclInternal(ToNamespace);
 
@@ -2697,6 +2701,10 @@
 LexicalDC->addDeclInternal(D2);
   }
 
+  if (auto BraceRangeOrErr = import(D->getBraceRange()))
+D2->setBraceRange(*BraceRangeOrErr);
+  else
+return BraceRangeOrErr.takeError();
   if (auto QualifierLocOrErr = import(D->getQualifierLoc()))
 D2->setQualifierInfo(*QualifierLocOrErr);
   else
@@ -5118,6 +5126,11 @@
 LexicalDC->addDeclInternal(D2);
   }
 
+  if (auto BraceRangeOrErr = import(D->getBraceRange()))
+D2->setBraceRange(*BraceRangeOrErr);
+  else
+return BraceRangeOrErr.takeError();
+
   // Import the qualifier, if any.
   if (auto LocOrErr = import(D->getQualifierLoc()))
 D2->setQualifierInfo(*LocOrErr);
@@ -6111,7 +6124,8 @@
   TemplateArgumentListInfo *ToResInfo = nullptr;
   if (E->hasExplicitTemplateArgs()) {
 if (Error Err =
-ImportTemplateArgumentListInfo(E->template_arguments(), ToTAInfo))
+ImportTemplateArgumentListInfo(E->getLAngleLoc(), E->getRAngleLoc(),
+   E->template_arguments(), ToTAInfo))
   return std::move(Err);
 ToResInfo = &ToTAInfo;
   }
@@ -7130,20 +7144,18 @@
 
 ExpectedStmt
 ASTNodeImporter::VisitDependentScopeDeclRefExpr(DependentScopeDeclRefExpr *E) {
-  auto Imp = importSeq(
-  E->getQualifierLoc(), E->getTemplateKeywordLoc(), E->getDeclName(),
-  E->getExprLoc(), E->getLAngleLoc(), E->getRAngleLoc());
+  auto Imp = importSeq(E->getQualifierLoc(), E->getTemplateKeywordLoc(),
+   E->getDeclName(), E->getNameInfo().getLoc(),
+   E->getLAngleLoc(), E->getRAngleLoc());
   if (!Imp)
 return Imp.takeError();
 
   NestedNameSpecifierLoc ToQualifierLoc;
-  SourceLocation ToTemplateKeywordLoc, ToExprLoc, ToLAngleLoc, ToRAngleLoc;
+  SourceLocation ToTemplateKeywordLoc, ToNameLoc, ToLAngleLoc, ToRAngleLoc;
   DeclarationName ToDeclName;
-  std::tie(
-  ToQualifierLoc, ToTemplateKeywordLoc, ToDeclName, ToExprLoc,
-  ToLAngleLoc, ToRAngleLoc) = *Imp;
-
-  DeclarationNameInfo ToNameInfo(ToDeclName, ToExprLoc);
+  std::tie(ToQualifierLoc, ToTemplateKeywordLoc, ToDeclName, ToNameLoc,
+   ToLAngleLoc, ToRAngleLoc) = *Imp;
+  DeclarationNameInfo ToNameInfo(ToDeclName, ToNameLoc);
   if (Error Err = ImportDeclarationNameLoc(E->getNameInfo(), ToNameInfo))
 return std::move(Err);
 
@@ -7208,7 +7220,7 @@
 else
   return ToDOrErr.takeError();
 
-  if (E->hasExplicitTemplateArgs() && E->getTemplateKeywordLoc().isValid()) {
+  if (E->hasExplicitTemplateArgs()) {
 TemplateArgumentListInfo ToTAInfo;
 if (Error Err = ImportTemplateArgumentListInfo(
 E->getLAngleLoc(), E->getRAngleLoc(), E->template_arguments(),
@@ -7262,8 +7274,9 @@
   TemplateArgumentListInfo ToTAInfo;
   TemplateArgumentListInfo *ResInfo = nullptr;
   if (E->hasExplicitTemplateArgs()) {
-if (Error Err =
-ImportTemplateArgumentListInfo(E->template_arguments(), ToTAInfo))
+TemplateArgumentListInfo FromTAInfo;
+E->copyTemplateArgumentsInto(FromTAInfo);
+if (Error Err = ImportTemplateArgumentListInfo(FromTAInfo, ToTAInfo))
   return std::move(Err);
 ResInfo = &ToTAInfo;
   }
@@ -8023,8 +8036,14 @@
 return std::move(Err);
   TypeSourceInfo *TSI = getToContext().getTrivialTypeSourceInfo(
 QualType(Spec->getAsType(), 0), ToTLoc);
-  Builder.Extend(getToContext(), ToLocalBeginLoc, TSI->getTypeLoc(),
- ToLocalEndLoc);
+  if (Kind == NestedNameSpecifier::TypeSpecWithTemplate)
+// To

[PATCH] D60500: [clangd] Refactor speculateCompletionFilter and also extract scope.

[Sam McCall via Phabricator via cfe-commits]

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

Intent is to use the heuristically-parsed scope in cases where we get bogus
results from sema, such as in complex macro expansions.
Added a motivating testcase we currently get wrong.

Name changed because we (already) use this for things other than speculation.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D60500

Files:
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1915,28 +1915,35 @@
   )cpp");
 }
 
-TEST(SpeculateCompletionFilter, Filters) {
-  Annotations F(R"cpp($bof^
-  $bol^
-  ab$ab^
-  x.ab$dot^
-  x.$dotempty^
-  x::ab$scoped^
-  x::$scopedempty^
-
-  )cpp");
-  auto speculate = [&](StringRef PointName) {
-auto Filter = speculateCompletionFilter(F.code(), F.point(PointName));
-assert(Filter);
-return *Filter;
-  };
-  EXPECT_EQ(speculate("bof"), "");
-  EXPECT_EQ(speculate("bol"), "");
-  EXPECT_EQ(speculate("ab"), "ab");
-  EXPECT_EQ(speculate("dot"), "ab");
-  EXPECT_EQ(speculate("dotempty"), "");
-  EXPECT_EQ(speculate("scoped"), "ab");
-  EXPECT_EQ(speculate("scopedempty"), "");
+TEST(GuessCompletionPrefix, Filters) {
+  for (llvm::StringRef Case : {
+"[[scope::]][[ident]]^",
+"[[]][[]]^",
+"\n[[]][[]]^",
+"[[]][[ab]]^",
+"x.[[]][[ab]]^",
+"x.[[]][[]]^",
+"[[x::]][[ab]]^",
+"[[x::]][[]]^",
+"some text [[scope::more::]][[identif]]^ier",
+"some text [[scope::]][[mor]]^e::identifier",
+  }) {
+Annotations F(Case);
+auto Offset = cantFail(positionToOffset(F.code(), F.point()));
+auto ToStringRef = [&](Range R) {
+  return F.code().slice(cantFail(positionToOffset(F.code(), R.start)),
+cantFail(positionToOffset(F.code(), R.end)));
+};
+auto WantQualifier = ToStringRef(F.ranges()[0]),
+ WantName = ToStringRef(F.ranges()[1]);
+
+auto Prefix = guessCompletionPrefix(F.code(), Offset);
+// Even when components are empty, check their offsets are correct.
+EXPECT_EQ(WantQualifier, Prefix.Qualifier) << Case;
+EXPECT_EQ(WantQualifier.begin(), Prefix.Qualifier.begin()) << Case;
+EXPECT_EQ(WantName, Prefix.Name) << Case;
+EXPECT_EQ(WantName.begin(), Prefix.Name.begin()) << Case;
+  }
 }
 
 TEST(CompletionTest, EnableSpeculativeIndexRequest) {
@@ -2366,6 +2373,23 @@
   UnorderedElementsAre(AllOf(Qualifier(""), Named("XYZ";
 }
 
+// Regression test: clang parser gets confused here and doesn't report the ns::
+// prefix - naive behavior is to insert it again.
+// However we can recognize this from the source code.
+// Test disabled until we can make it pass.
+TEST(CompletionTest, DISABLED_NamespaceDoubleInsertion) {
+  clangd::CodeCompleteOptions Opts = {};
+
+  auto Results = completions(R"cpp(
+namespace ns {}
+#define M(X) < X
+M(ns::ABC^
+  )cpp",
+ {cls("ns::ABCDE")}, Opts);
+  EXPECT_THAT(Results.Completions,
+  UnorderedElementsAre(AllOf(Qualifier(""), Named("ABCDE";
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/CodeComplete.h
===
--- clangd/CodeComplete.h
+++ clangd/CodeComplete.h
@@ -248,10 +248,21 @@
 // completion.
 bool isIndexedForCodeCompletion(const NamedDecl &ND, ASTContext &ASTCtx);
 
-/// Retrives a speculative code completion filter text before the cursor.
-/// Exposed for testing only.
-llvm::Expected
-speculateCompletionFilter(llvm::StringRef Content, Position Pos);
+// Text immediately before the completion point that should be completed.
+// This is heuristically derived from the source code, and is used when:
+//   - semantic analysis fails
+//   - semantic analysis may be slow, and we speculatively query the index
+struct CompletionPrefix {
+  // The unqualified partial name.
+  // If there is none, begin() == end() == completion position.
+  llvm::StringRef Name;
+  // The spelled scope qualifier, such as Foo::.
+  // If there is none, begin() == end() == Name.begin().
+  llvm::StringRef Qualifier;
+};
+// Heuristically parses before Offset to determine what should be completed.
+CompletionPrefix guessCompletionPrefix(llvm::StringRef Content,
+   unsigned Offset);
 
 } // namespace clangd
 } // namespace clang
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -37,6 +37,7 @@
 #include "index/Symbol.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
+#inc

[PATCH] D60500: [clangd] Refactor speculateCompletionFilter and also extract scope.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 194462.
sammccall added a comment.

Handle  edge-case correctly.
Add test for leading :: case.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60500

Files:
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -32,7 +32,6 @@
 using ::llvm::Failed;
 using ::testing::AllOf;
 using ::testing::Contains;
-using ::testing::Each;
 using ::testing::ElementsAre;
 using ::testing::Field;
 using ::testing::HasSubstr;
@@ -1915,28 +1914,37 @@
   )cpp");
 }
 
-TEST(SpeculateCompletionFilter, Filters) {
-  Annotations F(R"cpp($bof^
-  $bol^
-  ab$ab^
-  x.ab$dot^
-  x.$dotempty^
-  x::ab$scoped^
-  x::$scopedempty^
-
-  )cpp");
-  auto speculate = [&](StringRef PointName) {
-auto Filter = speculateCompletionFilter(F.code(), F.point(PointName));
-assert(Filter);
-return *Filter;
-  };
-  EXPECT_EQ(speculate("bof"), "");
-  EXPECT_EQ(speculate("bol"), "");
-  EXPECT_EQ(speculate("ab"), "ab");
-  EXPECT_EQ(speculate("dot"), "ab");
-  EXPECT_EQ(speculate("dotempty"), "");
-  EXPECT_EQ(speculate("scoped"), "ab");
-  EXPECT_EQ(speculate("scopedempty"), "");
+TEST(GuessCompletionPrefix, Filters) {
+  for (llvm::StringRef Case : {
+"[[scope::]][[ident]]^",
+"[[]][[]]^",
+"\n[[]][[]]^",
+"[[]][[ab]]^",
+"x.[[]][[ab]]^",
+"x.[[]][[]]^",
+"[[x::]][[ab]]^",
+"[[x::]][[]]^",
+"[[::x::]][[ab]]^",
+"some text [[scope::more::]][[identif]]^ier",
+"some text [[scope::]][[mor]]^e::identifier",
+"weird case foo::[[::bar::]][[baz]]^",
+  }) {
+Annotations F(Case);
+auto Offset = cantFail(positionToOffset(F.code(), F.point()));
+auto ToStringRef = [&](Range R) {
+  return F.code().slice(cantFail(positionToOffset(F.code(), R.start)),
+cantFail(positionToOffset(F.code(), R.end)));
+};
+auto WantQualifier = ToStringRef(F.ranges()[0]),
+ WantName = ToStringRef(F.ranges()[1]);
+
+auto Prefix = guessCompletionPrefix(F.code(), Offset);
+// Even when components are empty, check their offsets are correct.
+EXPECT_EQ(WantQualifier, Prefix.Qualifier) << Case;
+EXPECT_EQ(WantQualifier.begin(), Prefix.Qualifier.begin()) << Case;
+EXPECT_EQ(WantName, Prefix.Name) << Case;
+EXPECT_EQ(WantName.begin(), Prefix.Name.begin()) << Case;
+  }
 }
 
 TEST(CompletionTest, EnableSpeculativeIndexRequest) {
@@ -2366,6 +2374,23 @@
   UnorderedElementsAre(AllOf(Qualifier(""), Named("XYZ";
 }
 
+// Regression test: clang parser gets confused here and doesn't report the ns::
+// prefix - naive behavior is to insert it again.
+// However we can recognize this from the source code.
+// Test disabled until we can make it pass.
+TEST(CompletionTest, DISABLED_NamespaceDoubleInsertion) {
+  clangd::CodeCompleteOptions Opts = {};
+
+  auto Results = completions(R"cpp(
+namespace ns {}
+#define M(X) < X
+M(ns::ABC^
+  )cpp",
+ {cls("ns::ABCDE")}, Opts);
+  EXPECT_THAT(Results.Completions,
+  UnorderedElementsAre(AllOf(Qualifier(""), Named("ABCDE";
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/CodeComplete.h
===
--- clangd/CodeComplete.h
+++ clangd/CodeComplete.h
@@ -248,10 +248,21 @@
 // completion.
 bool isIndexedForCodeCompletion(const NamedDecl &ND, ASTContext &ASTCtx);
 
-/// Retrives a speculative code completion filter text before the cursor.
-/// Exposed for testing only.
-llvm::Expected
-speculateCompletionFilter(llvm::StringRef Content, Position Pos);
+// Text immediately before the completion point that should be completed.
+// This is heuristically derived from the source code, and is used when:
+//   - semantic analysis fails
+//   - semantic analysis may be slow, and we speculatively query the index
+struct CompletionPrefix {
+  // The unqualified partial name.
+  // If there is none, begin() == end() == completion position.
+  llvm::StringRef Name;
+  // The spelled scope qualifier, such as Foo::.
+  // If there is none, begin() == end() == Name.begin().
+  llvm::StringRef Qualifier;
+};
+// Heuristically parses before Offset to determine what should be completed.
+CompletionPrefix guessCompletionPrefix(llvm::StringRef Content,
+   unsigned Offset);
 
 } // namespace clangd
 } // namespace clang
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -37,6 +37,7 @@
 #include "index/Symbol.h"
 #include "clang/AST/Decl.h"
 #include "clang/A

[PATCH] D60500: [clangd] Refactor speculateCompletionFilter and also extract scope.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 194463.
sammccall added a comment.

Fix terrible variable name.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60500

Files:
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -32,7 +32,6 @@
 using ::llvm::Failed;
 using ::testing::AllOf;
 using ::testing::Contains;
-using ::testing::Each;
 using ::testing::ElementsAre;
 using ::testing::Field;
 using ::testing::HasSubstr;
@@ -1915,28 +1914,37 @@
   )cpp");
 }
 
-TEST(SpeculateCompletionFilter, Filters) {
-  Annotations F(R"cpp($bof^
-  $bol^
-  ab$ab^
-  x.ab$dot^
-  x.$dotempty^
-  x::ab$scoped^
-  x::$scopedempty^
-
-  )cpp");
-  auto speculate = [&](StringRef PointName) {
-auto Filter = speculateCompletionFilter(F.code(), F.point(PointName));
-assert(Filter);
-return *Filter;
-  };
-  EXPECT_EQ(speculate("bof"), "");
-  EXPECT_EQ(speculate("bol"), "");
-  EXPECT_EQ(speculate("ab"), "ab");
-  EXPECT_EQ(speculate("dot"), "ab");
-  EXPECT_EQ(speculate("dotempty"), "");
-  EXPECT_EQ(speculate("scoped"), "ab");
-  EXPECT_EQ(speculate("scopedempty"), "");
+TEST(GuessCompletionPrefix, Filters) {
+  for (llvm::StringRef Case : {
+"[[scope::]][[ident]]^",
+"[[]][[]]^",
+"\n[[]][[]]^",
+"[[]][[ab]]^",
+"x.[[]][[ab]]^",
+"x.[[]][[]]^",
+"[[x::]][[ab]]^",
+"[[x::]][[]]^",
+"[[::x::]][[ab]]^",
+"some text [[scope::more::]][[identif]]^ier",
+"some text [[scope::]][[mor]]^e::identifier",
+"weird case foo::[[::bar::]][[baz]]^",
+  }) {
+Annotations F(Case);
+auto Offset = cantFail(positionToOffset(F.code(), F.point()));
+auto ToStringRef = [&](Range R) {
+  return F.code().slice(cantFail(positionToOffset(F.code(), R.start)),
+cantFail(positionToOffset(F.code(), R.end)));
+};
+auto WantQualifier = ToStringRef(F.ranges()[0]),
+ WantName = ToStringRef(F.ranges()[1]);
+
+auto Prefix = guessCompletionPrefix(F.code(), Offset);
+// Even when components are empty, check their offsets are correct.
+EXPECT_EQ(WantQualifier, Prefix.Qualifier) << Case;
+EXPECT_EQ(WantQualifier.begin(), Prefix.Qualifier.begin()) << Case;
+EXPECT_EQ(WantName, Prefix.Name) << Case;
+EXPECT_EQ(WantName.begin(), Prefix.Name.begin()) << Case;
+  }
 }
 
 TEST(CompletionTest, EnableSpeculativeIndexRequest) {
@@ -2366,6 +2374,23 @@
   UnorderedElementsAre(AllOf(Qualifier(""), Named("XYZ";
 }
 
+// Regression test: clang parser gets confused here and doesn't report the ns::
+// prefix - naive behavior is to insert it again.
+// However we can recognize this from the source code.
+// Test disabled until we can make it pass.
+TEST(CompletionTest, DISABLED_NamespaceDoubleInsertion) {
+  clangd::CodeCompleteOptions Opts = {};
+
+  auto Results = completions(R"cpp(
+namespace ns {}
+#define M(X) < X
+M(ns::ABC^
+  )cpp",
+ {cls("ns::ABCDE")}, Opts);
+  EXPECT_THAT(Results.Completions,
+  UnorderedElementsAre(AllOf(Qualifier(""), Named("ABCDE";
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/CodeComplete.h
===
--- clangd/CodeComplete.h
+++ clangd/CodeComplete.h
@@ -248,10 +248,21 @@
 // completion.
 bool isIndexedForCodeCompletion(const NamedDecl &ND, ASTContext &ASTCtx);
 
-/// Retrives a speculative code completion filter text before the cursor.
-/// Exposed for testing only.
-llvm::Expected
-speculateCompletionFilter(llvm::StringRef Content, Position Pos);
+// Text immediately before the completion point that should be completed.
+// This is heuristically derived from the source code, and is used when:
+//   - semantic analysis fails
+//   - semantic analysis may be slow, and we speculatively query the index
+struct CompletionPrefix {
+  // The unqualified partial name.
+  // If there is none, begin() == end() == completion position.
+  llvm::StringRef Name;
+  // The spelled scope qualifier, such as Foo::.
+  // If there is none, begin() == end() == Name.begin().
+  llvm::StringRef Qualifier;
+};
+// Heuristically parses before Offset to determine what should be completed.
+CompletionPrefix guessCompletionPrefix(llvm::StringRef Content,
+   unsigned Offset);
 
 } // namespace clangd
 } // namespace clang
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -37,6 +37,7 @@
 #include "index/Symbol.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
+#include "clang/Bas

[PATCH] D60029: Add const children() accessors to all AST nodes.

[Nicolas Manichon via Phabricator via cfe-commits]

nicolas added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60029



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

[PATCH] D60055: Check i < FD->getNumParams() before querying

[Dmitri Gribenko via Phabricator via cfe-commits]

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

This fix makes sense to me, however, Richard Smith @rsmith is the best person 
to review this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60055



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

[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang/include/clang/AST/ASTContext.h:756
   /// lazily.
   mutable llvm::DenseMap RedeclComments;
 

The name of this variable (and `RawCommentAndCacheFlags`) don't make sense 
after the change.



Comment at: clang/lib/AST/ASTContext.cpp:372
   D = adjustDeclToTemplate(D);
+  const Decl* CanonicalDecl = D->getCanonicalDecl();
 

jkorous wrote:
> arphaman wrote:
> > Why are we now checking for the canonical declaration instead of `D` as 
> > before?
> Before this we were caching comment for every redeclaration separately but 
> for every redeclaration the comment was the same.
> 
> As I understand it - for a given declaration we found the first comment in 
> the redeclaration chain and then saved it to the cache for every 
> redeclaration (the same comment).
> Later, during lookup, we iterated the redeclaration chain again and did a 
> lookup for every redeclaration (see L392 in the original implementation).
> 
> Unless I missed something, it's suboptimal in both memory consumption and 
> runtime overhead.
> 
> That's the reason why I want to cache the comment for the redeclaration group 
> as a whole. The only thing I am not entirely sure about is what key to use to 
> represent the whole redeclaration chain - maybe the first declaration in the 
> redeclaration chain would be better then the canonical declaration...
> 
> WDYT?
> As I understand it - for a given declaration we found the first comment in 
> the redeclaration chain and then saved it to the cache for every 
> redeclaration (the same comment).

Only if there was only one comment in the redeclaration chain.  If a 
redeclaration had a different comment, this function would return that comment.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60432



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

[PATCH] D60267: [clangd] Support relatedInformation in diagnostics.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/Diagnostics.cpp:185
   OS << Note.Message;
-  OS << "\n\n";
-  printDiag(OS, Main);
+  // If there's no structured link between the note and the original diagnostic
+  // then emit the main diagnostic to give context.

NIT: the comment looks open to misinterpretation, "no structured link" refers 
to the fact the clients don't support it, but could be read that we don't know 
which notes correspond to a main diagnostic.

Maybe rephrase to something link "If the client does not support structured 
links …"?



Comment at: clangd/Diagnostics.cpp:280
+  Main.relatedInformation->push_back(std::move(RelInfo));
+}
   }

NIT: maybe call `OutFn` and return here to avoid checking for 
`EmitRelatedLocations` again?
Would arguably make the code simpler, although would require another call to 
`OutFn(Main)` outside the if branch.



Comment at: clangd/SourceCode.cpp:310
+  if (!F) {
 return None;
+  }

NIT: seems unrelated. Maybe revert?



Comment at: unittests/clangd/DiagnosticsTests.cpp:62
+return false;
+  if (toJSON(arg) != toJSON(LSPDiag)) {
+*result_listener << llvm::formatv("expected:\n{0:2}\ngot\n{1:2}",

Maybe omit the `std::tie()`-based comparison altogether?
This would not change the semantics of the matcher, right?



Comment at: unittests/clangd/DiagnosticsTests.cpp:259
+#ifdef _WIN32
+  "c:\\path\\to\\foo\\bar\\main.cpp",
+#else

maybe use `testPath()` here to avoid PP directives?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60267



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

[PATCH] D60267: [clangd] Support relatedInformation in diagnostics.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/Diagnostics.cpp:271
+  if (!Note.AbsFile) {
+log("Dropping note from unknown file: {0}", Note);
+continue;

Maybe `vlog`? This is what we use for dropped diagnostics, should probably 
stick to the same level with dropped notes (even though the dropped notes 
probably come up less often in practice).


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60267



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

[PATCH] D60485: [AArch64] Add support for MTE intrinsics

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

Do we need a separate file for that one "__arm_mte_ptrdiff" test? Wouldn't it 
be easier to wrap the whole function in "__cplusplus" and put it in the same 
file.


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

https://reviews.llvm.org/D60485



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

[PATCH] D60267: [clangd] Support relatedInformation in diagnostics.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clangd/Protocol.cpp:280
 R.DiagnosticFixes = *CodeActions;
+  if (auto CodeActions = Diagnostics->getBoolean("relatedInformation"))
+R.DiagnosticRelatedInformation = *CodeActions;

`s/CodeActions/RelatedInfoSupport/`


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60267



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

[PATCH] D60500: [clangd] Refactor speculateCompletionFilter and also extract scope.

[Eric Liu via Phabricator via cfe-commits]

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

lgtm




Comment at: clangd/CodeComplete.cpp:1542
+  assert(Offset <= Content.size());
+  StringRef Suffix = Content.take_front(Offset);
+  CompletionPrefix Result;

`Suffix` is actually a prefix of Content? This seems a bit confusing...


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60500



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

[PATCH] D60267: [clangd] Support relatedInformation in diagnostics.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/Protocol.cpp:280
 R.DiagnosticFixes = *CodeActions;
+  if (auto CodeActions = Diagnostics->getBoolean("relatedInformation"))
+R.DiagnosticRelatedInformation = *CodeActions;

kadircet wrote:
> `s/CodeActions/RelatedInfoSupport/`
copy-paste detected? :-)))


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60267



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

[PATCH] D59599: [clangd] Fix a crash while printing Template Arguments

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 194470.
kadircet marked 3 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59599

Files:
  clangd/AST.cpp
  unittests/clangd/SymbolCollectorTests.cpp


Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -1222,6 +1222,22 @@
   EXPECT_THAT(Symbols, Contains(QName("std::foo")));
 }
 
+TEST_F(SymbolCollectorTest, TemplateSpecForwardDecl) {
+  // FIXME: getTypeAsWritten returns null for friend decls, this should be 
fixed
+  // in AST. Testing just to make sure we don't crash.
+  Annotations Header(R"(
+  template  struct [[Foo]];
+  struct Bar {
+  friend class Foo;
+  };
+  template <> struct Foo {};
+  )");
+  runSymbolCollector(Header.code(), /*Main=*/"");
+  EXPECT_THAT(Symbols,
+  Contains(AllOf(QName("Foo"), DeclRange(Header.range()),
+ ForCodeCompletion(true;
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/AST.cpp
===
--- clangd/AST.cpp
+++ clangd/AST.cpp
@@ -12,6 +12,7 @@
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/TemplateBase.h"
+#include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
@@ -82,14 +83,17 @@
   if (auto Args = getTemplateSpecializationArgLocs(ND))
 printTemplateArgumentList(OS, *Args, Policy);
   else if (auto *Cls = llvm::dyn_cast(&ND)) {
-if (auto STL = Cls->getTypeAsWritten()
-   ->getTypeLoc()
-   .getAs()) {
-  llvm::SmallVector ArgLocs;
-  ArgLocs.reserve(STL.getNumArgs());
-  for (unsigned I = 0; I < STL.getNumArgs(); ++I)
-ArgLocs.push_back(STL.getArgLoc(I));
-  printTemplateArgumentList(OS, ArgLocs, Policy);
+if (const TypeSourceInfo *TSI = Cls->getTypeAsWritten()) {
+  if (auto STL = TSI->getTypeLoc().getAs()) 
{
+llvm::SmallVector ArgLocs;
+ArgLocs.reserve(STL.getNumArgs());
+for (unsigned I = 0; I < STL.getNumArgs(); ++I)
+  ArgLocs.push_back(STL.getArgLoc(I));
+printTemplateArgumentList(OS, ArgLocs, Policy);
+  }
+} else {
+  // FIXME: Fix cases when getTypeAsWritten returns null, e.g. friend 
decls.
+  printTemplateArgumentList(OS, Cls->getTemplateArgs().asArray(), Policy);
 }
   }
   OS.flush();


Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -1222,6 +1222,22 @@
   EXPECT_THAT(Symbols, Contains(QName("std::foo")));
 }
 
+TEST_F(SymbolCollectorTest, TemplateSpecForwardDecl) {
+  // FIXME: getTypeAsWritten returns null for friend decls, this should be fixed
+  // in AST. Testing just to make sure we don't crash.
+  Annotations Header(R"(
+  template  struct [[Foo]];
+  struct Bar {
+  friend class Foo;
+  };
+  template <> struct Foo {};
+  )");
+  runSymbolCollector(Header.code(), /*Main=*/"");
+  EXPECT_THAT(Symbols,
+  Contains(AllOf(QName("Foo"), DeclRange(Header.range()),
+ ForCodeCompletion(true;
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/AST.cpp
===
--- clangd/AST.cpp
+++ clangd/AST.cpp
@@ -12,6 +12,7 @@
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/TemplateBase.h"
+#include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
@@ -82,14 +83,17 @@
   if (auto Args = getTemplateSpecializationArgLocs(ND))
 printTemplateArgumentList(OS, *Args, Policy);
   else if (auto *Cls = llvm::dyn_cast(&ND)) {
-if (auto STL = Cls->getTypeAsWritten()
-   ->getTypeLoc()
-   .getAs()) {
-  llvm::SmallVector ArgLocs;
-  ArgLocs.reserve(STL.getNumArgs());
-  for (unsigned I = 0; I < STL.getNumArgs(); ++I)
-ArgLocs.push_back(STL.getArgLoc(I));
-  printTemplateArgumentList(OS, ArgLocs, Policy);
+if (const TypeSourceInfo *TSI = Cls->getTypeAsWritten()) {
+  if (auto STL = TSI->getTypeLoc().getAs()) {
+llvm::SmallVector ArgLocs;
+ArgLocs.reserve(STL.getNumArgs());
+for (unsigned I = 0; I < STL.getNumArgs(); ++I)
+  ArgLocs.push_back(STL.getArgLoc(I));
+printTemplateArgumentList(OS, ArgLocs, Policy);
+  }
+} else {
+  // FIXME: Fix cases when getTypeAsWritten returns null, e.g. friend decls.
+ 

[PATCH] D60503: [clangd] Don't insert extra namespace qualifiers when Sema gets lost.

[Sam McCall via Phabricator via cfe-commits]

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

There are cases where Sema can't tell that "foo" in foo::Bar is a
namespace qualifier, like in incomplete macro expansions.

After this patch, if sema reports no specifier but it looks like there's one in
the source code, then we take it into account.

Reworked structure and comments in getQueryScopes to try to fight
creeping complexity - unsure if I succeeded.

I made the test harder (the original version also passes).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D60503

Files:
  clangd/CodeComplete.cpp
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -2378,15 +2378,17 @@
 // prefix - naive behavior is to insert it again.
 // However we can recognize this from the source code.
 // Test disabled until we can make it pass.
-TEST(CompletionTest, DISABLED_NamespaceDoubleInsertion) {
+TEST(CompletionTest, NamespaceDoubleInsertion) {
   clangd::CodeCompleteOptions Opts = {};
 
   auto Results = completions(R"cpp(
+namespace foo {
 namespace ns {}
 #define M(X) < X
 M(ns::ABC^
+}
   )cpp",
- {cls("ns::ABCDE")}, Opts);
+ {cls("foo::ns::ABCDE")}, Opts);
   EXPECT_THAT(Results.Completions,
   UnorderedElementsAre(AllOf(Qualifier(""), Named("ABCDE";
 }
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -45,6 +45,7 @@
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
+#include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/Sema.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/None.h"
@@ -539,54 +540,59 @@
 // (e.g. enclosing namespace).
 std::pair, bool>
 getQueryScopes(CodeCompletionContext &CCContext, const Sema &CCSema,
+   const CompletionPrefix &HeuristicPrefix,
const CodeCompleteOptions &Opts) {
-  auto GetAllAccessibleScopes = [](CodeCompletionContext &CCContext) {
-SpecifiedScope Info;
-for (auto *Context : CCContext.getVisitedContexts()) {
-  if (isa(Context))
-Info.AccessibleScopes.push_back(""); // global namespace
-  else if (isa(Context))
-Info.AccessibleScopes.push_back(printNamespaceScope(*Context));
-}
-return Info;
-  };
-
-  auto SS = CCContext.getCXXScopeSpecifier();
+  SpecifiedScope Scopes;
+  for (auto *Context : CCContext.getVisitedContexts()) {
+if (isa(Context))
+  Scopes.AccessibleScopes.push_back(""); // global namespace
+else if (isa(Context))
+  Scopes.AccessibleScopes.push_back(printNamespaceScope(*Context));
+  }
 
-  // Unqualified completion (e.g. "vec^").
-  if (!SS) {
-std::vector Scopes;
+  const CXXScopeSpec *SemaSpecifier =
+  CCContext.getCXXScopeSpecifier().getValueOr(nullptr);
+  // Case 1: unqualified completion.
+  if (!SemaSpecifier) {
+// Case 2 (exception): sema saw no qualifier, but there appears to be one!
+// This can happen e.g. in incomplete macro expansions. Use heuristics.
+if (!HeuristicPrefix.Qualifier.empty()) {
+  vlog("Sema said no scope specifier, but we saw {0} in the source code",
+   HeuristicPrefix.Qualifier);
+  StringRef SpelledSpecifier = HeuristicPrefix.Qualifier;
+  if (SpelledSpecifier.consume_front("::"))
+Scopes.AccessibleScopes = {""};
+  Scopes.UnresolvedQualifier = SpelledSpecifier;
+  return {Scopes.scopesForIndexQuery(), false};
+}
+// The enclosing namespace must be first, it gets a quality boost.
+std::vector EnclosingAtFront;
 std::string EnclosingScope = printNamespaceScope(*CCSema.CurContext);
-Scopes.push_back(EnclosingScope);
-for (auto &S : GetAllAccessibleScopes(CCContext).scopesForIndexQuery()) {
+EnclosingAtFront.push_back(EnclosingScope);
+for (auto &S : Scopes.scopesForIndexQuery()) {
   if (EnclosingScope != S)
-Scopes.push_back(std::move(S));
+EnclosingAtFront.push_back(std::move(S));
 }
-// Allow AllScopes completion only for there is no explicit scope qualifier.
-return {Scopes, Opts.AllScopes};
+// Allow AllScopes completion as there is no explicit scope qualifier.
+return {EnclosingAtFront, Opts.AllScopes};
   }
-
-  // Qualified completion ("std::vec^"), we have two cases depending on whether
-  // the qualifier can be resolved by Sema.
-  if ((*SS)->isValid()) { // Resolved qualifier.
-return {GetAllAccessibleScopes(CCContext).scopesForIndexQuery(), false};
-  }
-
-  // Unresolved qualifier.
-  SpecifiedScope Info = GetAllAc

[PATCH] D60503: [clangd] Don't insert extra namespace qualifiers when Sema gets lost.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 194486.
sammccall added a comment.

Update comment


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60503

Files:
  clangd/CodeComplete.cpp
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -2374,19 +2374,19 @@
   UnorderedElementsAre(AllOf(Qualifier(""), Named("XYZ";
 }
 
-// Regression test: clang parser gets confused here and doesn't report the ns::
-// prefix - naive behavior is to insert it again.
-// However we can recognize this from the source code.
-// Test disabled until we can make it pass.
-TEST(CompletionTest, DISABLED_NamespaceDoubleInsertion) {
+// Clang parser gets confused here and doesn't report the ns:: prefix.
+// Naive behavior is to insert it again. We examine the source and recover.
+TEST(CompletionTest, NamespaceDoubleInsertion) {
   clangd::CodeCompleteOptions Opts = {};
 
   auto Results = completions(R"cpp(
+namespace foo {
 namespace ns {}
 #define M(X) < X
 M(ns::ABC^
+}
   )cpp",
- {cls("ns::ABCDE")}, Opts);
+ {cls("foo::ns::ABCDE")}, Opts);
   EXPECT_THAT(Results.Completions,
   UnorderedElementsAre(AllOf(Qualifier(""), Named("ABCDE";
 }
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -45,6 +45,7 @@
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
+#include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/Sema.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/None.h"
@@ -539,54 +540,59 @@
 // (e.g. enclosing namespace).
 std::pair, bool>
 getQueryScopes(CodeCompletionContext &CCContext, const Sema &CCSema,
+   const CompletionPrefix &HeuristicPrefix,
const CodeCompleteOptions &Opts) {
-  auto GetAllAccessibleScopes = [](CodeCompletionContext &CCContext) {
-SpecifiedScope Info;
-for (auto *Context : CCContext.getVisitedContexts()) {
-  if (isa(Context))
-Info.AccessibleScopes.push_back(""); // global namespace
-  else if (isa(Context))
-Info.AccessibleScopes.push_back(printNamespaceScope(*Context));
-}
-return Info;
-  };
-
-  auto SS = CCContext.getCXXScopeSpecifier();
+  SpecifiedScope Scopes;
+  for (auto *Context : CCContext.getVisitedContexts()) {
+if (isa(Context))
+  Scopes.AccessibleScopes.push_back(""); // global namespace
+else if (isa(Context))
+  Scopes.AccessibleScopes.push_back(printNamespaceScope(*Context));
+  }
 
-  // Unqualified completion (e.g. "vec^").
-  if (!SS) {
-std::vector Scopes;
+  const CXXScopeSpec *SemaSpecifier =
+  CCContext.getCXXScopeSpecifier().getValueOr(nullptr);
+  // Case 1: unqualified completion.
+  if (!SemaSpecifier) {
+// Case 2 (exception): sema saw no qualifier, but there appears to be one!
+// This can happen e.g. in incomplete macro expansions. Use heuristics.
+if (!HeuristicPrefix.Qualifier.empty()) {
+  vlog("Sema said no scope specifier, but we saw {0} in the source code",
+   HeuristicPrefix.Qualifier);
+  StringRef SpelledSpecifier = HeuristicPrefix.Qualifier;
+  if (SpelledSpecifier.consume_front("::"))
+Scopes.AccessibleScopes = {""};
+  Scopes.UnresolvedQualifier = SpelledSpecifier;
+  return {Scopes.scopesForIndexQuery(), false};
+}
+// The enclosing namespace must be first, it gets a quality boost.
+std::vector EnclosingAtFront;
 std::string EnclosingScope = printNamespaceScope(*CCSema.CurContext);
-Scopes.push_back(EnclosingScope);
-for (auto &S : GetAllAccessibleScopes(CCContext).scopesForIndexQuery()) {
+EnclosingAtFront.push_back(EnclosingScope);
+for (auto &S : Scopes.scopesForIndexQuery()) {
   if (EnclosingScope != S)
-Scopes.push_back(std::move(S));
+EnclosingAtFront.push_back(std::move(S));
 }
-// Allow AllScopes completion only for there is no explicit scope qualifier.
-return {Scopes, Opts.AllScopes};
+// Allow AllScopes completion as there is no explicit scope qualifier.
+return {EnclosingAtFront, Opts.AllScopes};
   }
-
-  // Qualified completion ("std::vec^"), we have two cases depending on whether
-  // the qualifier can be resolved by Sema.
-  if ((*SS)->isValid()) { // Resolved qualifier.
-return {GetAllAccessibleScopes(CCContext).scopesForIndexQuery(), false};
-  }
-
-  // Unresolved qualifier.
-  SpecifiedScope Info = GetAllAccessibleScopes(CCContext);
-  Info.AccessibleScopes.push_back(""); // Make sure global scope is included.
-
-  llvm::StringRef SpelledSpecifier =
- 

[clang-tools-extra] r358074 - [clangd] Refactor speculateCompletionFilter and also extract scope.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Wed Apr 10 04:50:40 2019
New Revision: 358074

URL: http://llvm.org/viewvc/llvm-project?rev=358074&view=rev
Log:
[clangd] Refactor speculateCompletionFilter and also extract scope.

Summary:
Intent is to use the heuristically-parsed scope in cases where we get bogus
results from sema, such as in complex macro expansions.
Added a motivating testcase we currently get wrong.

Name changed because we (already) use this for things other than speculation.

Reviewers: ioeric

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/CodeComplete.h
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=358074&r1=358073&r2=358074&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Wed Apr 10 04:50:40 2019
@@ -37,6 +37,7 @@
 #include "index/Symbol.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
+#include "clang/Basic/CharInfo.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Format/Format.h"
@@ -1110,14 +,12 @@ llvm::Optional
 speculativeFuzzyFindRequestForCompletion(FuzzyFindRequest CachedReq,
  PathRef File, llvm::StringRef Content,
  Position Pos) {
-  auto Filter = speculateCompletionFilter(Content, Pos);
-  if (!Filter) {
-elog("Failed to speculate filter text for code completion at Pos "
- "{0}:{1}: {2}",
- Pos.line, Pos.character, Filter.takeError());
-return None;
+  auto Offset = positionToOffset(Content, Pos);
+  if (!Offset) {
+elog("No speculative filter: bad offset {0} in {1}", Pos, File);
+return llvm::None;
   }
-  CachedReq.Query = *Filter;
+  CachedReq.Query = guessCompletionPrefix(Content, *Offset).Name;
   return CachedReq;
 }
 
@@ -1537,29 +1536,26 @@ clang::CodeCompleteOptions CodeCompleteO
   return Result;
 }
 
-llvm::Expected
-speculateCompletionFilter(llvm::StringRef Content, Position Pos) {
-  auto Offset = positionToOffset(Content, Pos);
-  if (!Offset)
-return llvm::make_error(
-"Failed to convert position to offset in content.",
-llvm::inconvertibleErrorCode());
-  if (*Offset == 0)
-return "";
+CompletionPrefix
+guessCompletionPrefix(llvm::StringRef Content, unsigned Offset) {
+  assert(Offset <= Content.size());
+  StringRef Rest = Content.take_front(Offset);
+  CompletionPrefix Result;
+
+  // Consume the unqualified name. We only handle ASCII characters.
+  // isIdentifierBody will let us match "0invalid", but we don't mind.
+  while (!Rest.empty() && isIdentifierBody(Rest.back()))
+Rest = Rest.drop_back();
+  Result.Name = Content.slice(Rest.size(), Offset);
+
+  // Consume qualifiers.
+  while (Rest.consume_back("::") && !Rest.endswith(":")) // reject 
+while (!Rest.empty() && isIdentifierBody(Rest.back()))
+  Rest = Rest.drop_back();
+  Result.Qualifier =
+  Content.slice(Rest.size(), Result.Name.begin() - Content.begin());
 
-  // Start from the character before the cursor.
-  int St = *Offset - 1;
-  // FIXME(ioeric): consider UTF characters?
-  auto IsValidIdentifierChar = [](char C) {
-return ((C >= 'a' && C <= 'z') || (C >= 'A' && C <= 'Z') ||
-(C >= '0' && C <= '9') || (C == '_'));
-  };
-  size_t Len = 0;
-  for (; (St >= 0) && IsValidIdentifierChar(Content[St]); --St, ++Len) {
-  }
-  if (Len > 0)
-St++; // Shift to the first valid character.
-  return Content.substr(St, Len);
+  return Result;
 }
 
 CodeCompleteResult

Modified: clang-tools-extra/trunk/clangd/CodeComplete.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.h?rev=358074&r1=358073&r2=358074&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.h (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.h Wed Apr 10 04:50:40 2019
@@ -254,10 +254,21 @@ SignatureHelp signatureHelp(PathRef File
 // completion.
 bool isIndexedForCodeCompletion(const NamedDecl &ND, ASTContext &ASTCtx);
 
-/// Retrives a speculative code completion filter text before the cursor.
-/// Exposed for testing only.
-llvm::Expected
-speculateCompletionFilter(llvm::StringRef Content, Position Pos);
+// Text immediately before the completion point that should be completed.
+// This is heuristically derived from the source code, and is used when:
+//   - semantic analysis fails
+//   - semantic analysis may be slow, and we speculatively query the index
+struct Co

[PATCH] D60500: [clangd] Refactor speculateCompletionFilter and also extract scope.

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clangd/CodeComplete.cpp:1542
+  assert(Offset <= Content.size());
+  StringRef Suffix = Content.take_front(Offset);
+  CompletionPrefix Result;

ioeric wrote:
> `Suffix` is actually a prefix of Content? This seems a bit confusing...
Haha, just checking we're all paying attention...


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60500



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

[PATCH] D60500: [clangd] Refactor speculateCompletionFilter and also extract scope.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rL358074: [clangd] Refactor speculateCompletionFilter and also 
extract scope. (authored by sammccall, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60500

Files:
  clang-tools-extra/trunk/clangd/CodeComplete.cpp
  clang-tools-extra/trunk/clangd/CodeComplete.h
  clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
@@ -32,7 +32,6 @@
 using ::llvm::Failed;
 using ::testing::AllOf;
 using ::testing::Contains;
-using ::testing::Each;
 using ::testing::ElementsAre;
 using ::testing::Field;
 using ::testing::HasSubstr;
@@ -1915,28 +1914,37 @@
   )cpp");
 }
 
-TEST(SpeculateCompletionFilter, Filters) {
-  Annotations F(R"cpp($bof^
-  $bol^
-  ab$ab^
-  x.ab$dot^
-  x.$dotempty^
-  x::ab$scoped^
-  x::$scopedempty^
+TEST(GuessCompletionPrefix, Filters) {
+  for (llvm::StringRef Case : {
+"[[scope::]][[ident]]^",
+"[[]][[]]^",
+"\n[[]][[]]^",
+"[[]][[ab]]^",
+"x.[[]][[ab]]^",
+"x.[[]][[]]^",
+"[[x::]][[ab]]^",
+"[[x::]][[]]^",
+"[[::x::]][[ab]]^",
+"some text [[scope::more::]][[identif]]^ier",
+"some text [[scope::]][[mor]]^e::identifier",
+"weird case foo::[[::bar::]][[baz]]^",
+  }) {
+Annotations F(Case);
+auto Offset = cantFail(positionToOffset(F.code(), F.point()));
+auto ToStringRef = [&](Range R) {
+  return F.code().slice(cantFail(positionToOffset(F.code(), R.start)),
+cantFail(positionToOffset(F.code(), R.end)));
+};
+auto WantQualifier = ToStringRef(F.ranges()[0]),
+ WantName = ToStringRef(F.ranges()[1]);
 
-  )cpp");
-  auto speculate = [&](StringRef PointName) {
-auto Filter = speculateCompletionFilter(F.code(), F.point(PointName));
-assert(Filter);
-return *Filter;
-  };
-  EXPECT_EQ(speculate("bof"), "");
-  EXPECT_EQ(speculate("bol"), "");
-  EXPECT_EQ(speculate("ab"), "ab");
-  EXPECT_EQ(speculate("dot"), "ab");
-  EXPECT_EQ(speculate("dotempty"), "");
-  EXPECT_EQ(speculate("scoped"), "ab");
-  EXPECT_EQ(speculate("scopedempty"), "");
+auto Prefix = guessCompletionPrefix(F.code(), Offset);
+// Even when components are empty, check their offsets are correct.
+EXPECT_EQ(WantQualifier, Prefix.Qualifier) << Case;
+EXPECT_EQ(WantQualifier.begin(), Prefix.Qualifier.begin()) << Case;
+EXPECT_EQ(WantName, Prefix.Name) << Case;
+EXPECT_EQ(WantName.begin(), Prefix.Name.begin()) << Case;
+  }
 }
 
 TEST(CompletionTest, EnableSpeculativeIndexRequest) {
@@ -2366,6 +2374,23 @@
   UnorderedElementsAre(AllOf(Qualifier(""), Named("XYZ";
 }
 
+// Regression test: clang parser gets confused here and doesn't report the ns::
+// prefix - naive behavior is to insert it again.
+// However we can recognize this from the source code.
+// Test disabled until we can make it pass.
+TEST(CompletionTest, DISABLED_NamespaceDoubleInsertion) {
+  clangd::CodeCompleteOptions Opts = {};
+
+  auto Results = completions(R"cpp(
+namespace ns {}
+#define M(X) < X
+M(ns::ABC^
+  )cpp",
+ {cls("ns::ABCDE")}, Opts);
+  EXPECT_THAT(Results.Completions,
+  UnorderedElementsAre(AllOf(Qualifier(""), Named("ABCDE";
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp
@@ -37,6 +37,7 @@
 #include "index/Symbol.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
+#include "clang/Basic/CharInfo.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Format/Format.h"
@@ -1110,14 +,12 @@
 speculativeFuzzyFindRequestForCompletion(FuzzyFindRequest CachedReq,
  PathRef File, llvm::StringRef Content,
  Position Pos) {
-  auto Filter = speculateCompletionFilter(Content, Pos);
-  if (!Filter) {
-elog("Failed to speculate filter text for code completion at Pos "
- "{0}:{1}: {2}",
- Pos.line, Pos.character, Filter.takeError());
-return None;
+  auto Offset = positionToOffset(Content, Pos);
+  if (!Offset) {
+elog("No speculative filter: bad offset {0} in {1}", Pos, File);
+return llvm::None;
   }
-  CachedReq.Query = *Filter;
+  CachedReq.Query = guessCom

[PATCH] D60409: [clangd] Add -header-insertion=never flag to disable include insertion in code completion

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall marked 2 inline comments as done.
sammccall added inline comments.
This revision is now accepted and ready to land.



Comment at: clangd/CodeComplete.cpp:1176
   // This is available after Sema has run.
-  llvm::Optional Inserter;  // Available during runWithSema.
+  llvm::Optional Inserter;  // Optional during runWithSema.
   llvm::Optional FileProximity; // Initialized once Sema runs.

ioeric wrote:
> Why optional? In the current implementation, it's always initialized.
Oops, this was left-over from a previous iteration.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60409



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

[PATCH] D60503: [clangd] Don't insert extra namespace qualifiers when Sema gets lost.

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clangd/CodeComplete.cpp:545
const CodeCompleteOptions &Opts) {
-  auto GetAllAccessibleScopes = [](CodeCompletionContext &CCContext) {
-SpecifiedScope Info;
-for (auto *Context : CCContext.getVisitedContexts()) {
-  if (isa(Context))
-Info.AccessibleScopes.push_back(""); // global namespace
-  else if (isa(Context))
-Info.AccessibleScopes.push_back(printNamespaceScope(*Context));
-}
-return Info;
-  };
-
-  auto SS = CCContext.getCXXScopeSpecifier();
+  SpecifiedScope Scopes;
+  for (auto *Context : CCContext.getVisitedContexts()) {

(AFAICT this lambda was executed the same way on every code path, so I just 
inlined it)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60503



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

[PATCH] D59221: [asan] Add gcc 8's driver option -fsanitize=pointer-compare and -fsanitize=pointer-substract.

[pierre gousseau via Phabricator via cfe-commits]

pgousseau added a comment.

Ping!


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

https://reviews.llvm.org/D59221



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

[clang-tools-extra] r358075 - [clangd] Add -header-insertion=never flag to disable include insertion in code completion

[Sam McCall via cfe-commits]

Author: sammccall
Date: Wed Apr 10 05:15:35 2019
New Revision: 358075

URL: http://llvm.org/viewvc/llvm-project?rev=358075&view=rev
Log:
[clangd] Add -header-insertion=never flag to disable include insertion in code 
completion

Summary: One clear use case: use with an editor that reacts poorly to edits 
above the cursor.

Reviewers: ioeric

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

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/CodeComplete.h
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=358075&r1=358074&r2=358075&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Wed Apr 10 05:15:35 2019
@@ -191,7 +191,9 @@ struct CompletionCandidate {
 
   // Returns a token identifying the overload set this is part of.
   // 0 indicates it's not part of any overload set.
-  size_t overloadSet() const {
+  size_t overloadSet(const CodeCompleteOptions &Opts) const {
+if (!Opts.BundleOverloads)
+  return 0;
 llvm::SmallString<256> Scratch;
 if (IndexResult) {
   switch (IndexResult->SymInfo.Kind) {
@@ -208,7 +210,7 @@ struct CompletionCandidate {
 // This could break #include insertion.
 return llvm::hash_combine(
 (IndexResult->Scope + IndexResult->Name).toStringRef(Scratch),
-headerToInsertIfAllowed().getValueOr(""));
+headerToInsertIfAllowed(Opts).getValueOr(""));
   default:
 return 0;
   }
@@ -223,12 +225,14 @@ struct CompletionCandidate {
   D->printQualifiedName(OS);
 }
 return llvm::hash_combine(Scratch,
-  headerToInsertIfAllowed().getValueOr(""));
+  headerToInsertIfAllowed(Opts).getValueOr(""));
   }
 
   // The best header to include if include insertion is allowed.
-  llvm::Optional headerToInsertIfAllowed() const {
-if (RankedIncludeHeaders.empty())
+  llvm::Optional
+  headerToInsertIfAllowed(const CodeCompleteOptions &Opts) const {
+if (Opts.InsertIncludes == CodeCompleteOptions::NeverInsert ||
+RankedIncludeHeaders.empty())
   return None;
 if (SemaResult && SemaResult->Declaration) {
   // Avoid inserting new #include if the declaration is found in the 
current
@@ -338,7 +342,7 @@ struct CodeCompletionBuilder {
   Includes.calculateIncludePath(*ResolvedDeclaring, *ResolvedInserted),
   Includes.shouldInsertInclude(*ResolvedDeclaring, *ResolvedInserted));
 };
-bool ShouldInsert = C.headerToInsertIfAllowed().hasValue();
+bool ShouldInsert = C.headerToInsertIfAllowed(Opts).hasValue();
 // Calculate include paths and edits for all possible headers.
 for (const auto &Inc : C.RankedIncludeHeaders) {
   if (auto ToInclude = Inserted(Inc)) {
@@ -1373,7 +1377,7 @@ private:
   if (C.IndexResult)
 C.RankedIncludeHeaders = getRankedIncludes(*C.IndexResult);
   C.Name = IndexResult ? IndexResult->Name : 
Recorder->getName(*SemaResult);
-  if (auto OverloadSet = Opts.BundleOverloads ? C.overloadSet() : 0) {
+  if (auto OverloadSet = C.overloadSet(Opts)) {
 auto Ret = BundleLookup.try_emplace(OverloadSet, Bundles.size());
 if (Ret.second)
   Bundles.emplace_back();

Modified: clang-tools-extra/trunk/clangd/CodeComplete.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.h?rev=358075&r1=358074&r2=358075&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.h (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.h Wed Apr 10 05:15:35 2019
@@ -68,6 +68,11 @@ struct CodeCompleteOptions {
   /// If more results are available, we set CompletionList.isIncomplete.
   size_t Limit = 0;
 
+  enum IncludeInsertion {
+IWYU,
+NeverInsert,
+  } InsertIncludes = IncludeInsertion::IWYU;
+
   /// A visual indicator to prepend to the completion label to indicate whether
   /// completion result would trigger an #include insertion or not.
   struct IncludeInsertionIndicator {

Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=358075&r1=358074&r2=358075&view=diff
==
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Wed Apr 10 05:15:35 2019
@@ -6,8 +6,9 @@
 //
 
//

[PATCH] D60409: [clangd] Add -header-insertion=never flag to disable include insertion in code completion

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rL358075: [clangd] Add -header-insertion=never flag to disable 
include insertion in code… (authored by sammccall, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60409?vs=194150&id=194493#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60409

Files:
  clang-tools-extra/trunk/clangd/CodeComplete.cpp
  clang-tools-extra/trunk/clangd/CodeComplete.h
  clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
  clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Index: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
@@ -6,8 +6,9 @@
 //
 //===--===//
 
-#include "Features.inc"
 #include "ClangdLSPServer.h"
+#include "CodeComplete.h"
+#include "Features.inc"
 #include "Path.h"
 #include "Protocol.h"
 #include "Trace.h"
@@ -154,6 +155,20 @@
 "debug-origin", llvm::cl::desc("Show origins of completion items"),
 llvm::cl::init(CodeCompleteOptions().ShowOrigins), llvm::cl::Hidden);
 
+static llvm::cl::opt HeaderInsertion(
+"header-insertion",
+llvm::cl::desc("Add #include directives when accepting code completions"),
+llvm::cl::init(CodeCompleteOptions().InsertIncludes),
+llvm::cl::values(
+clEnumValN(CodeCompleteOptions::IWYU, "iwyu",
+   "Include what you use. "
+   "Insert the owning header for top-level symbols, unless the "
+   "header is already directly included or the symbol is "
+   "forward-declared."),
+clEnumValN(
+CodeCompleteOptions::NeverInsert, "never",
+"Never insert #include directives as part of code completion")));
+
 static llvm::cl::opt HeaderInsertionDecorators(
 "header-insertion-decorators",
 llvm::cl::desc("Prepend a circular dot or space before the completion "
@@ -438,6 +453,7 @@
   CCOpts.Limit = LimitResults;
   CCOpts.BundleOverloads = CompletionStyle != Detailed;
   CCOpts.ShowOrigins = ShowOrigins;
+  CCOpts.InsertIncludes = HeaderInsertion;
   if (!HeaderInsertionDecorators) {
 CCOpts.IncludeIndicator.Insert.clear();
 CCOpts.IncludeIndicator.NoInsert.clear();
Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp
@@ -191,7 +191,9 @@
 
   // Returns a token identifying the overload set this is part of.
   // 0 indicates it's not part of any overload set.
-  size_t overloadSet() const {
+  size_t overloadSet(const CodeCompleteOptions &Opts) const {
+if (!Opts.BundleOverloads)
+  return 0;
 llvm::SmallString<256> Scratch;
 if (IndexResult) {
   switch (IndexResult->SymInfo.Kind) {
@@ -208,7 +210,7 @@
 // This could break #include insertion.
 return llvm::hash_combine(
 (IndexResult->Scope + IndexResult->Name).toStringRef(Scratch),
-headerToInsertIfAllowed().getValueOr(""));
+headerToInsertIfAllowed(Opts).getValueOr(""));
   default:
 return 0;
   }
@@ -223,12 +225,14 @@
   D->printQualifiedName(OS);
 }
 return llvm::hash_combine(Scratch,
-  headerToInsertIfAllowed().getValueOr(""));
+  headerToInsertIfAllowed(Opts).getValueOr(""));
   }
 
   // The best header to include if include insertion is allowed.
-  llvm::Optional headerToInsertIfAllowed() const {
-if (RankedIncludeHeaders.empty())
+  llvm::Optional
+  headerToInsertIfAllowed(const CodeCompleteOptions &Opts) const {
+if (Opts.InsertIncludes == CodeCompleteOptions::NeverInsert ||
+RankedIncludeHeaders.empty())
   return None;
 if (SemaResult && SemaResult->Declaration) {
   // Avoid inserting new #include if the declaration is found in the current
@@ -338,7 +342,7 @@
   Includes.calculateIncludePath(*ResolvedDeclaring, *ResolvedInserted),
   Includes.shouldInsertInclude(*ResolvedDeclaring, *ResolvedInserted));
 };
-bool ShouldInsert = C.headerToInsertIfAllowed().hasValue();
+bool ShouldInsert = C.headerToInsertIfAllowed(Opts).hasValue();
 // Calculate include paths and edits for all possible headers.
 for (const auto &Inc : C.RankedIncludeHeaders) {
   if (auto ToInclude = Inserted(Inc)) {
@@ -1373,7 +1377,7 @@
   if (C.IndexResult)
 C.RankedIncludeHeaders = getRankedIncludes(*C.IndexResult);
   C.Name = IndexResult ? IndexResult->Name : Reco

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas created this revision.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.
Herald added a project: clang.

This check searches for copy assignment operators which might not handle 
self-assignment properly. There are three patterns of
handling a self assignment situation: self check, copy-and-swap or the less 
common copy-and-move. The new check warns if none of
these patterns is found in a user defined implementation.

See also:
OOP54-CPP. Gracefully handle self-copy assignment
https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP54-CPP.+Gracefully+handle+self-copy+assignment


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D60507

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment.cpp

Index: clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment.cpp
@@ -0,0 +1,351 @@
+// RUN: %check_clang_tidy %s bugprone-unhandled-self-assignment %t
+
+namespace std {
+
+template 
+void swap(T x, T y) {
+}
+
+template 
+T &&move(T x) {
+}
+
+template 
+class unique_ptr {
+};
+
+template 
+class shared_ptr {
+};
+
+template 
+class weak_ptr {
+};
+
+} // namespace std
+
+void assert(int expression){};
+
+///
+/// Test cases correctly caught by the check.
+
+class PtrField {
+public:
+  PtrField &operator=(const PtrField &object);
+
+private:
+  int *p;
+};
+
+PtrField &PtrField::operator=(const PtrField &object) {
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+  // ...
+  return *this;
+}
+
+// Class with an inline operator definition.
+class InlineDefinition {
+public:
+  InlineDefinition &operator=(const InlineDefinition &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:21: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  int *p;
+};
+
+class UniquePtrField {
+public:
+  UniquePtrField &operator=(const UniquePtrField &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:19: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  std::unique_ptr p;
+};
+
+class SharedPtrField {
+public:
+  SharedPtrField &operator=(const SharedPtrField &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:19: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  std::shared_ptr p;
+};
+
+class WeakPtrField {
+public:
+  WeakPtrField &operator=(const WeakPtrField &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  std::weak_ptr p;
+};
+
+// Class with C array field.
+class CArrayField {
+public:
+  CArrayField &operator=(const CArrayField &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:16: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+// ...
+return *this;
+  }
+
+private:
+  int array[256];
+};
+
+// Make sure to not ignore cases when the operator definition calls
+// a copy constructor of another class.
+class CopyConstruct {
+public:
+  CopyConstruct &operator=(const CopyConstruct &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:18: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+WeakPtrField a;
+WeakPtrField b(a);
+// ...
+return *this;
+  }
+
+private:
+  int *p;
+};
+
+// Make sure to not ignore cases when the operator definition calls
+// a copy assignment operator of another class.
+class AssignOperator {
+public:
+  AssignOperator &operator=(const AssignOperator &object) {
+// CHECK-MESSAGES: [[@LINE-1]]:19: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment]
+a.operator=(object.a);
+// ...
+return *this;
+  }
+
+private:
+  int *p;
+  WeakPtrField a;
+};
+
+///
+/// Test cases correctly ignored by the check.
+
+// Self-assignment is checked using the equality operator.
+class SelfCheck1 {
+public:
+  SelfCheck1 &operator=(const SelfCheck1 &obje

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas added a comment.

On llvm source code the check finds three suspicious methods:

  
/home/zolnai/libreoffice/clang/llvm-project/clang/lib/AST/NestedNameSpecifier.cpp:534:1:
 warning: operator=() might not handle self-assignment properly 
[bugprone-unhandled-self-assignment]
  operator=(const NestedNameSpecifierLocBuilder &Other) {
  
  
/home/zolnai/libreoffice/clang/llvm-project/llvm/unittests/ADT/ArrayRefTest.cpp:75:10:
 warning: operator=() might not handle self-assignment properly 
[bugprone-unhandled-self-assignment]
  void operator=(const NonAssignable &RHS) { assert(RHS.Ptr != nullptr); }
  
  
/home/zolnai/libreoffice/clang/llvm-project/llvm/lib/Analysis/TargetLibraryInfo.cpp:594:47:
 warning: operator=() might not handle self-assignment properly 
[bugprone-unhandled-self-assignment]
  TargetLibraryInfoImpl &TargetLibraryInfoImpl::operator=(const 
TargetLibraryInfoImpl &TLI) {

Two of them seems a good catch to me. NestedNameSpecifierLocBuilder and 
TargetLibraryInfoImpl are using `memcpy` without self-check, and using `memcpy` 
on overlapping regions leads to undefined behavior.

The third one is not an actual working copy assignment operator, as I see. It 
is used only for some testing purposes.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60507



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

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas added a comment.

On LibreOffice source code I found 36 catches with this check.
16 of them was worth to fix because in those cases the object state was changed 
in some way after a self-assignment:
https://cgit.freedesktop.org/libreoffice/core/commit/?id=3a5d78365dd172881c16c03e67f2d170ffc6d7d4

The remaining 20 are false positives. They are working copy assignment 
operators without using any of the three checked patterns.
However, some of these warnings could be suppressed with modernizing the code 
or with removing code duplication:
https://cgit.freedesktop.org/libreoffice/core/commit/?id=adfba503c792fdbd4748d6680c2dd8d8d5bb0d69


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60507



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

[PATCH] D60126: [clangd] Use identifiers in file as completion candidates when build is not ready.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Very nice! D60503  will conflict, feel free to 
stall that until this is landed.
On the other hand it will simplify some things, e.g. the prefix is already 
calculated, and typed scope is available if you want that (no enclosing 
namespaces though).




Comment at: clangd/CodeComplete.cpp:1341
+
+// Carve out the typed filter from the content so that we don't treat it as
+// an identifier.

ioeric wrote:
> sammccall wrote:
> > you could just erase the typed filter from the suggestion list.
> > (It may be a valid word spelled elsewhere, but there's no point suggesting 
> > it)
> This is following conventions of other sources. Both index and sema provide 
> results for fully-typed names. Considering that users might be already used 
> to this, and completion results tend to give reassurance for correctly typed 
> names, I am inclined to keep the typed filter if it's seen somewhere else in 
> the file.
OK, but we should now be able to just decrement the count for the right entry 
in the collected identifiers then? rather than carving up the string



Comment at: clangd/CodeComplete.cpp:182
 
+// Identifier code completion result.
+struct Identifier {

This is a bit vague - most results are for identifiers in some sense.
Maybe `RawIdentifier`?



Comment at: clangd/CodeComplete.cpp:274
 struct CodeCompletionBuilder {
-  CodeCompletionBuilder(ASTContext &ASTCtx, const CompletionCandidate &C,
+  // ASTCtx can be nullptr if not run with sema.
+  CodeCompletionBuilder(ASTContext *ASTCtx, const CompletionCandidate &C,

move this comment to the member?
The invariant is important to readers, not just callers of the constructor.



Comment at: clangd/CodeComplete.cpp:1177
+
+  int NSema = 0, NIndex = 0, NBoth = 0, NIdent = 0; // Counters for logging.
+  bool Incomplete = false; // Would more be available with a higher limit?

rename NBoth -> NSemaAndIndex?



Comment at: clangd/CodeComplete.cpp:1303
+// FIXME: initialize ScopeProximity when scopes are added.
+AllScopes = true; // Identifiers have no scope.
+auto Style = getFormatStyleForFile(FileName, Content, VFS.get());

it doesn't make sense to set this here, as long as we're not querying the index.



Comment at: clangd/CodeComplete.cpp:1360
 getQueryScopes(Recorder->CCContext, *Recorder->CCSema, Opts);
 if (!QueryScopes.empty())
   ScopeProximity.emplace(QueryScopes);

add this to the non-sema case too (though the if is always false for now), or 
add a fixme?
Worried about forgetting this.



Comment at: clangd/CodeComplete.cpp:1521
 Relevance.Query = SymbolRelevanceSignals::CodeComplete;
-Relevance.FileProximityMatch = FileProximity.getPointer();
+if (FileProximity)
+  Relevance.FileProximityMatch = FileProximity.getPointer();

why are we not initializing fileproximity in the fallback case?
(To only a single source)



Comment at: clangd/CodeComplete.cpp:1565
+Relevance.Scope = SymbolRelevanceSignals::FileScope;
+IsIdentifier = true;
+  }

can we have a SymbolOrigin bit for identifiers, and use that instead?



Comment at: clangd/Headers.cpp:180
 return false;
+  if (!HeaderSearchInfo && !InsertedHeader.Verbatim)
+return false;

nit: i'd move this above the previous case, it seems both more fundamental and 
cheaper



Comment at: clangd/Headers.cpp:196
+  if (!HeaderSearchInfo)
+return "\"" + InsertedHeader.File + "\"";
+  std::string Suggested = HeaderSearchInfo->suggestPathToFileForDiagnostics(

Do we expect this code path to ever be called?
We may want to assert or elog here, depending on how this can be hit.



Comment at: clangd/SourceCode.cpp:403
+  llvm::StringMap Identifiers;
+  auto Add = [&Identifiers](llvm::StringRef Identifier) {
+auto I = Identifiers.try_emplace(Identifier, 1);

why isn't Add just `++Identifiers[Id]`?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60126



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

[PATCH] D59415: Do not resolve directory junctions for `-fdiagnostics-absolute-paths` on Windows.

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

I don't feel like I have enough background on what you're trying to do to 
review this. It sounds related to clangd issues maybe?

"If the source file path contains directory junctions, and we resolve them when 
printing diagnostic messages"

I didn't think Clang tried to resolve symlinks or otherwise canonicalize paths 
when refering to files in diagnostics in the first place?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59415



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

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp:102
 "bugprone-too-small-loop-variable");
+CheckFactories.registerCheck(
+"bugprone-unhandled-self-assignment");

please order this list alphabetically.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:51
+  // Matcher for standard smart pointers.
+  const auto SmartPointerType = qualType(hasUnqualifiedDesugaredType(
+  recordType(hasDeclaration(classTemplateSpecializationDecl(

what about `auto_ptr`? I am actually not sure if we should care, as its 
deprecated and removed already, on the other hand legacy code probably still 
has it.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst:10
+
+This check corresponds to the CERT C++ Coding Standard rule
+`OOP54-CPP. Gracefully handle self-copy assignment

It is worth an alias into the cert module. Please add this with this revision 
already.



Comment at: 
clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment.cpp:351
+  int *p;
+};

Please add tests with templated classes and self-assignment.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60507



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

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Thanks for the useful check! I have a few comments, see inline.




Comment at: 
clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:34-35
+  binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
+ anyOf(hasLHS(ignoringParenCasts(cxxThisExpr())),
+   hasRHS(ignoringParenCasts(cxxThisExpr(;
+

I guess, `has(ignoringParenCasts(cxxThisExpr())` will have the same effect as 
`anyOf(hasLHS(...), hasRHS(...))`.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:53
+  recordType(hasDeclaration(classTemplateSpecializationDecl(
+  anyOf(hasName("std::shared_ptr"), hasName("std::unique_ptr"),
+hasName("std::weak_ptr")),

Please use hasAnyName. It's more efficient and readable.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst:10-12
+This check corresponds to the CERT C++ Coding Standard rule
+`OOP54-CPP. Gracefully handle self-copy assignment
+`_.

JonasToth wrote:
> It is worth an alias into the cert module. Please add this with this revision 
> already.
Please add a corresponding alias into the cert module.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60507



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

[PATCH] D60510: [ADT] Fix template parameter names of llvm::{upper|lower}_bound

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added subscribers: llvm-commits, dexonsmith.
Herald added a project: LLVM.

Rename template parameter for a search value from 'ForwardIt' to 'T'.
While here, also use perfect forwarding to pass the value to STL algos.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60510

Files:
  llvm/include/llvm/ADT/STLExtras.h


Index: llvm/include/llvm/ADT/STLExtras.h
===
--- llvm/include/llvm/ADT/STLExtras.h
+++ llvm/include/llvm/ADT/STLExtras.h
@@ -1277,28 +1277,32 @@
 
 /// Provide wrappers to std::lower_bound which take ranges instead of having to
 /// pass begin/end explicitly.
-template 
-auto lower_bound(R &&Range, ForwardIt I) -> decltype(adl_begin(Range)) {
-  return std::lower_bound(adl_begin(Range), adl_end(Range), I);
+template 
+auto lower_bound(R &&Range, T &&Value) -> decltype(adl_begin(Range)) {
+  return std::lower_bound(adl_begin(Range), adl_end(Range),
+  std::forward(Value));
 }
 
-template 
-auto lower_bound(R &&Range, ForwardIt I, Compare C)
+template 
+auto lower_bound(R &&Range, T &&Value, Compare C)
 -> decltype(adl_begin(Range)) {
-  return std::lower_bound(adl_begin(Range), adl_end(Range), I, C);
+  return std::lower_bound(adl_begin(Range), adl_end(Range),
+  std::forward(Value), C);
 }
 
 /// Provide wrappers to std::upper_bound which take ranges instead of having to
 /// pass begin/end explicitly.
-template 
-auto upper_bound(R &&Range, ForwardIt I) -> decltype(adl_begin(Range)) {
-  return std::upper_bound(adl_begin(Range), adl_end(Range), I);
+template 
+auto upper_bound(R &&Range, T && Value) -> decltype(adl_begin(Range)) {
+  return std::upper_bound(adl_begin(Range), adl_end(Range),
+  std::forward(Value));
 }
 
-template 
-auto upper_bound(R &&Range, ForwardIt I, Compare C)
+template 
+auto upper_bound(R &&Range, T && Value, Compare C)
 -> decltype(adl_begin(Range)) {
-  return std::upper_bound(adl_begin(Range), adl_end(Range), I, C);
+  return std::upper_bound(adl_begin(Range), adl_end(Range),
+  std::forward(Value), C);
 }
 /// Wrapper function around std::equal to detect if all elements
 /// in a container are same.


Index: llvm/include/llvm/ADT/STLExtras.h
===
--- llvm/include/llvm/ADT/STLExtras.h
+++ llvm/include/llvm/ADT/STLExtras.h
@@ -1277,28 +1277,32 @@
 
 /// Provide wrappers to std::lower_bound which take ranges instead of having to
 /// pass begin/end explicitly.
-template 
-auto lower_bound(R &&Range, ForwardIt I) -> decltype(adl_begin(Range)) {
-  return std::lower_bound(adl_begin(Range), adl_end(Range), I);
+template 
+auto lower_bound(R &&Range, T &&Value) -> decltype(adl_begin(Range)) {
+  return std::lower_bound(adl_begin(Range), adl_end(Range),
+  std::forward(Value));
 }
 
-template 
-auto lower_bound(R &&Range, ForwardIt I, Compare C)
+template 
+auto lower_bound(R &&Range, T &&Value, Compare C)
 -> decltype(adl_begin(Range)) {
-  return std::lower_bound(adl_begin(Range), adl_end(Range), I, C);
+  return std::lower_bound(adl_begin(Range), adl_end(Range),
+  std::forward(Value), C);
 }
 
 /// Provide wrappers to std::upper_bound which take ranges instead of having to
 /// pass begin/end explicitly.
-template 
-auto upper_bound(R &&Range, ForwardIt I) -> decltype(adl_begin(Range)) {
-  return std::upper_bound(adl_begin(Range), adl_end(Range), I);
+template 
+auto upper_bound(R &&Range, T && Value) -> decltype(adl_begin(Range)) {
+  return std::upper_bound(adl_begin(Range), adl_end(Range),
+  std::forward(Value));
 }
 
-template 
-auto upper_bound(R &&Range, ForwardIt I, Compare C)
+template 
+auto upper_bound(R &&Range, T && Value, Compare C)
 -> decltype(adl_begin(Range)) {
-  return std::upper_bound(adl_begin(Range), adl_end(Range), I, C);
+  return std::upper_bound(adl_begin(Range), adl_end(Range),
+  std::forward(Value), C);
 }
 /// Wrapper function around std::equal to detect if all elements
 /// in a container are same.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60503: [clangd] Don't insert extra namespace qualifiers when Sema gets lost.

[Eric Liu via Phabricator via cfe-commits]

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

Feel free to land this. I'll rebase D60126  on 
this when it's in.




Comment at: clangd/CodeComplete.cpp:580
+  // Case 3: sema saw and resolved a scope qualifier.
+  if (SemaSpecifier && SemaSpecifier->isValid())
+return {Scopes.scopesForIndexQuery(), false};

nit: Sema Specifier can't be nullptr at this point.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60503



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

[PATCH] D60139: [clang-tidy] Add bugprone-placement-new-target-type-mismatch check

[Dennis Luxen via Phabricator via cfe-commits]

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

In D60139#1460233 , @JonasToth wrote:

> Hey Dennis,
>
> my 2cents on the check. I think it is very good to have! Did you check coding 
> guidelines if they say something to this issue? (e.g. cppcoreguidelines, 
> hicpp, cert) As we have modules for them it would be great to make aliases to 
> this check if they demand this to be checked.


Thanks for the great suggestions. Updated the diff according to the feedback. 
Also checked with cppcoreguidelines, hicpp as well as cert. Only cert has a 
related, yet different rule 

 stating that calls to placement new shall be provided with properly aligned 
pointers. I'd say this should be a distinct check. Happy to work on it after 
this one.




Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:42
+
+  assert((Cast->getSubExpr()->getType()->isPointerType() ||
+ Cast->getSubExpr()->getType()->isArrayType()) &&

JonasToth wrote:
> Is this universally true? What about the nothrow-overload, would that 
> interfere?
Thanks, rewrote that check.


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

https://reviews.llvm.org/D60139



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

[PATCH] D60139: [clang-tidy] Add bugprone-placement-new-target-type-mismatch check

[Dennis Luxen via Phabricator via cfe-commits]

DennisL updated this revision to Diff 194505.
DennisL marked an inline comment as done.
DennisL added a comment.

Changed the following

- addressed reviewer feedback
- fetch the placement parameter as written
- add further test cases as requested
- stylistic changes
- add nothrow parameter support
- ignore matches with unresolved template parameters
- add examples of bad and good code


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

https://reviews.llvm.org/D60139

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp
  clang-tidy/bugprone/PlacementNewTargetTypeMismatch.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp

Index: test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp
@@ -0,0 +1,118 @@
+// RUN: %check_clang_tidy %s bugprone-placement-new-target-type-mismatch %t
+
+// definitions
+	
+namespace std {
+struct nothrow_t { explicit nothrow_t() = default; } nothrow;
+template T* addressof(T& arg) noexcept;
+template< class T > struct remove_reference  {typedef T type;};
+template< class T > struct remove_reference  {typedef T type;};
+template< class T > struct remove_reference {typedef T type;};
+template< class T >
+T&& forward( typename std::remove_reference::type& t ) noexcept;
+} // namespace std
+
+using size_type = unsigned long;
+void *operator new(size_type, void *);
+void *operator new[](size_type, void *);
+void* operator new(size_type size, const std::nothrow_t&) noexcept;
+void* operator new(size_type size, const std::nothrow_t&) noexcept;
+void* operator new[](size_type size, const std::nothrow_t&) noexcept;
+
+struct Foo {
+  int a;
+  int b;
+  int c;
+  int d;
+};
+
+template
+T& getT() {
+  static T f;
+  return f;
+}
+
+// instances emitting warnings
+
+void f1() {
+  struct Dummy {
+int a;
+int b;
+  };
+  int *ptr = new int;
+  new (ptr) Dummy;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f2() {
+  int * ptr = new int;
+  new (ptr) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f3() {
+  char *ptr = new char[17*sizeof(char)];
+  new (ptr) float[13];
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f4() {
+  new (std::addressof(getT())) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f5() {
+  char *ptr = new char[17*sizeof(char)];
+  new (ptr) float{13.f};
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f6() {
+  char array[17*sizeof(char)];
+  new (array) float{13.f};
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+// instances not emitting a warning
+
+void g1() {
+  Foo * ptr = new Foo;
+  new (ptr) Foo;
+}
+
+void g2() {
+  char *ptr = new char[17*sizeof(char)];
+  new ((float *)ptr) float{13.f};
+}
+
+void g3() {
+  char array[17*sizeof(char)];
+  new (array) char('A');
+}
+
+void g4() {
+  new ((void *)std::addressof(getT())) Foo;
+}
+
+union
+{
+  char * buffer;
+} Union;
+
+template 
+void g5(U &&... V) {
+  new (static_cast(Union.buffer)) T(std::forward(V)...);
+}
+
+template 
+void g6(U &&... V) {
+  new (std::nothrow) T(std::forward(V)...);
+}
+
+template  void g7(U &&... Us) {
+  new (static_cast(Union.buffer)) T(std::forward(Us)...);
+}
+
+template  void g8(U &&... Us) {
+  new (Union.buffer) T(std::forward(Us)...);
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -55,6 +55,7 @@
bugprone-move-forwarding-reference
bugprone-multiple-statement-macro
bugprone-parent-virtual-call
+   bugprone-placement-new-target-type-mismatch
bugprone-sizeof-container
bugprone-sizeof-expression
bugprone-string-constructor
Index: docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
@@ -0,0 +1,23 @@
+.. title:: clang-tidy - misc-placement-new-target-type-mismatch
+
+misc-placement-new-target-t

[PATCH] D60055: Check i < FD->getNumParams() before querying

[Violet via Phabricator via cfe-commits]

Violet added a comment.

Thanks. Can you land it for me? I'm a newcommer without landing privilege.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60055



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

[PATCH] D60055: Check i < FD->getNumParams() before querying

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added a comment.

I'd be happy to land it, but I do want @rsmith to take a look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60055



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

[PATCH] D60472: [AArch64][PowerPC][Driver] Allow setting crypto feature through -mcrypto for ARM/AArch64

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

What's the motivation for this change, are you working towards common flags for 
both platforms? The current way to select crypto on AArch64 is 
'-march=armv8.x-a+crypto/nocrypto'. I can see that would be an issue if Power 
PC doesn't support that syntax, or doesn't have a specific crypto extension.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60472



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

[PATCH] D60472: [AArch64][PowerPC][Driver] Allow setting crypto feature through -mcrypto for ARM/AArch64

[Peter Smith via Phabricator via cfe-commits]

peter.smith added a comment.

I'm not in favour of adding AArch64 support to -mcrypto -mnocrypto for a few 
reasons:

- Arm are trying to keep the options for controlling target features as 
consistent as possible with GCC and this option isn't supported in GCC 
(https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html)
- There is http://lists.llvm.org/pipermail/llvm-dev/2018-September/126346.html 
which is Arm's proposal for how target options can be better supported in 
Clang. I think that supporting -mcrypto as well would add more complexity as 
there are interactions between the options.
- Arm 32-bit also supports crypto so this would need to be added to Arm as well 
for consistency.

Can you expand on why you need -m[no]crypto?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60472



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

r358087 - clang-cl: Fix parsing of the /F option (PR41405)

[Hans Wennborg via cfe-commits]

Author: hans
Date: Wed Apr 10 07:27:47 2019
New Revision: 358087

URL: http://llvm.org/viewvc/llvm-project?rev=358087&view=rev
Log:
clang-cl: Fix parsing of the /F option (PR41405)

Modified:
cfe/trunk/include/clang/Driver/CLCompatOptions.td
cfe/trunk/test/Driver/cl-options.c

Modified: cfe/trunk/include/clang/Driver/CLCompatOptions.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CLCompatOptions.td?rev=358087&r1=358086&r2=358087&view=diff
==
--- cfe/trunk/include/clang/Driver/CLCompatOptions.td (original)
+++ cfe/trunk/include/clang/Driver/CLCompatOptions.td Wed Apr 10 07:27:47 2019
@@ -404,7 +404,7 @@ def _SLASH_d2 : CLJoined<"d2">;
 def _SLASH_doc : CLJoined<"doc">;
 def _SLASH_FA_joined : CLJoined<"FA">;
 def _SLASH_favor : CLJoined<"favor">;
-def _SLASH_F : CLFlag<"F">;
+def _SLASH_F : CLJoinedOrSeparate<"F">;
 def _SLASH_Fm : CLJoined<"Fm">;
 def _SLASH_Fr : CLJoined<"Fr">;
 def _SLASH_FR : CLJoined<"FR">;

Modified: cfe/trunk/test/Driver/cl-options.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=358087&r1=358086&r2=358087&view=diff
==
--- cfe/trunk/test/Driver/cl-options.c (original)
+++ cfe/trunk/test/Driver/cl-options.c Wed Apr 10 07:27:47 2019
@@ -400,7 +400,7 @@
 // RUN: /d2FH4 \
 // RUN: /docname \
 // RUN: /EHsc \
-// RUN: /F \
+// RUN: /F 42 \
 // RUN: /FA \
 // RUN: /FAc \
 // RUN: /Fafilename \


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

[PATCH] D60472: [AArch64][PowerPC][Driver] Allow setting crypto feature through -mcrypto for ARM/AArch64

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added a comment.

The motivation for this change is to make "crypto" setting an additive option 
e.g. like "-mavx" used in many media packages.  Some packages in Chrome want to 
enable crypto conditionally for a few files to allow crypto feature to be used 
based on runtime cpu detection. They set "-march=armv8+crypto" flag but it gets 
overridden by the global "-march=armv8a" flag set by the build system in Chrome 
OS because the target cpu does not support crypto causing compile-time errors. 
Ability to specify "-mcrypto"  standalone makes it an additive option and 
ensures that it it is not lost. i.e. this will help in decoupling  "-mcrypto" 
from "-march" so that they could be set independently. The current additive 
alternate is  '-Xclang -target-feature -Xclang "+crypto" ' which is ugly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60472



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

[PATCH] D60472: [AArch64][PowerPC][Driver] Allow setting crypto feature through -mcrypto for ARM/AArch64

[Peter Smith via Phabricator via cfe-commits]

peter.smith added a comment.

In D60472#1461336 , @manojgupta wrote:

> The motivation for this change is to make "crypto" setting an additive option 
> e.g. like "-mavx" used in many media packages.  Some packages in Chrome want 
> to enable crypto conditionally for a few files to allow crypto feature to be 
> used based on runtime cpu detection. They set "-march=armv8+crypto" flag but 
> it gets overridden by the global "-march=armv8a" flag set by the build system 
> in Chrome OS because the target cpu does not support crypto causing 
> compile-time errors. 
>  Ability to specify "-mcrypto"  standalone makes it an additive option and 
> ensures that it it is not lost. i.e. this will help in decoupling  "-mcrypto" 
> from "-march" so that they could be set independently. The current additive 
> alternate is  '-Xclang -target-feature -Xclang "+crypto" ' which is ugly.


Is that not a limitation of the build system? I'd expect a package to be able 
to locally override a global default rather than vice-versa. Although crypto 
might be the area of concern here and there may be a conveniently named option 
for PPC, where does this stop? Crypto is not the only architectural extension, 
for example see https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html . To 
maintain a consistent interface we'd need command line options for all the 
extensions. May I encourage you to reply to the RFC on command line options 
that I mentioned earlier if it doesn't work for you? I think the extensions 
need to be considered as a whole and not just individually.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60472



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

[PATCH] D60472: [AArch64][PowerPC][Driver] Allow setting crypto feature through -mcrypto for ARM/AArch64

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added a comment.

In D60472#1461351 , @peter.smith wrote:

>




> Is that not a limitation of the build system? I'd expect a package to be able 
> to locally override a global default rather than vice-versa. Although crypto 
> might be the area of concern here and there may be a conveniently named 
> option for PPC, where does this stop? Crypto is not the only architectural 
> extension, for example see 
> https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html . To maintain a 
> consistent interface we'd need command line options for all the extensions. 
> May I encourage you to reply to the RFC on command line options that I 
> mentioned earlier if it doesn't work for you? I think the extensions need to 
> be considered as a whole and not just individually.

While it partly is a build system issue, another problem is enabling crypto via 
"-march" requires picking an architecture as well. So even if it could override 
the global default, it would also override the global "-march" as well. If the 
global "-march" was a better/higher option, it does not make sense to override 
it e.g. "-march=armv8-a+crypto" overriding "-march=armv8.3-a" is not helpful 
since it will disable 8.3 features.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60472



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

[PATCH] D60472: [AArch64][PowerPC][Driver] Allow setting crypto feature through -mcrypto for ARM/AArch64

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added a comment.

In D60472#1461351 , @peter.smith wrote:

>




> Is that not a limitation of the build system? I'd expect a package to be able 
> to locally override a global default rather than vice-versa. Although crypto 
> might be the area of concern here and there may be a conveniently named 
> option for PPC, where does this stop? Crypto is not the only architectural 
> extension, for example see 
> https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html . To maintain a 
> consistent interface we'd need command line options for all the extensions. 
> May I encourage you to reply to the RFC on command line options that I 
> mentioned earlier if it doesn't work for you? I think the extensions need to 
> be considered as a whole and not just individually.

I'll also read the RFC and respond.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60472



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

[PATCH] D60513: [HIP] Use -mlink-builtin-bitcode to link device library

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: tra, ashi1.
Herald added a subscriber: jdoerfert.

Use -mlink-builtin-bitcode instead of llvm-link to link
device library so that device library bitcode and user
device code can be compiled in a consistent way.

This is the same approach used by CUDA and OpenMP.


https://reviews.llvm.org/D60513

Files:
  lib/Driver/ToolChains/HIP.cpp
  test/Driver/hip-device-libs.hip
  test/Driver/hip-toolchain-no-rdc.hip
  test/Driver/hip-toolchain-rdc.hip

Index: test/Driver/hip-toolchain-rdc.hip
===
--- test/Driver/hip-toolchain-rdc.hip
+++ test/Driver/hip-toolchain-rdc.hip
@@ -18,6 +18,7 @@
 // CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-target-cpu" "gfx803"
 // CHECK-SAME: "-fcuda-is-device" "-fgpu-rdc" "-fvisibility" "hidden"
 // CHECK-SAME: "-fapply-global-visibility-to-externs"
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: {{.*}} "-o" [[A_BC:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[A_SRC:".*a.cu"]]
 
@@ -27,11 +28,11 @@
 // CHECK-SAME: {{.*}} "-main-file-name" "b.hip" {{.*}} "-target-cpu" "gfx803"
 // CHECK-SAME: "-fcuda-is-device" "-fgpu-rdc" "-fvisibility" "hidden"
 // CHECK-SAME: "-fapply-global-visibility-to-externs"
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: {{.*}} "-o" [[B_BC:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[B_SRC:".*b.hip"]]
 
 // CHECK: [[LLVM_LINK:"*.llvm-link"]] [[A_BC]] [[B_BC]]
-// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: "-o" [[LINKED_BC_DEV1:".*-gfx803-linked-.*bc"]]
 
 // CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV1]] "-mtriple=amdgcn-amd-amdhsa"
@@ -49,18 +50,21 @@
 // CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu"
 // CHECK-SAME: "-emit-llvm-bc"
 // CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-target-cpu" "gfx900"
-// CHECK-SAME: "-fcuda-is-device" {{.*}} "-o" [[A_BC:".*bc"]] "-x" "hip"
+// CHECK-SAME: "-fcuda-is-device" "-fgpu-rdc"
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
+// CHECK-SAME: {{.*}} "-o" [[A_BC:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[A_SRC]]
 
 // CHECK: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
 // CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu"
 // CHECK-SAME: "-emit-llvm-bc"
 // CHECK-SAME: {{.*}} "-main-file-name" "b.hip" {{.*}} "-target-cpu" "gfx900"
-// CHECK-SAME: "-fcuda-is-device" {{.*}} "-o" [[B_BC:".*bc"]] "-x" "hip"
+// CHECK-SAME: "-fcuda-is-device" "-fgpu-rdc"
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
+// CHECK-SAME: {{.*}} "-o" [[B_BC:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[B_SRC]]
 
 // CHECK: [[LLVM_LINK]] [[A_BC]] [[B_BC]]
-// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: "-o" [[LINKED_BC_DEV2:".*-gfx900-linked-.*bc"]]
 
 // CHECK: [[OPT]] [[LINKED_BC_DEV2]] "-mtriple=amdgcn-amd-amdhsa"
Index: test/Driver/hip-toolchain-no-rdc.hip
===
--- test/Driver/hip-toolchain-no-rdc.hip
+++ test/Driver/hip-toolchain-no-rdc.hip
@@ -22,11 +22,11 @@
 // CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-target-cpu" "gfx803"
 // CHECK-SAME: "-fcuda-is-device" "-fvisibility" "hidden"
 // CHECK-SAME: "-fapply-global-visibility-to-externs"
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: {{.*}} "-o" [[A_BC_803:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[A_SRC:".*a.cu"]]
 
 // CHECK: [[LLVM_LINK:"*.llvm-link"]] [[A_BC_803]]
-// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: "-o" [[LINKED_BC_DEV_A_803:".*-gfx803-linked-.*bc"]]
 
 // CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV_A_803]] "-mtriple=amdgcn-amd-amdhsa"
@@ -50,11 +50,11 @@
 // CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-target-cpu" "gfx900"
 // CHECK-SAME: "-fcuda-is-device" "-fvisibility" "hidden"
 // CHECK-SAME: "-fapply-global-visibility-to-externs"
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: {{.*}} "-o" [[A_BC_900:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[A_SRC]]
 
 // CHECK: [[LLVM_LINK:"*.llvm-link"]] [[A_BC_900]]
-// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: "-o" [[LINKED_BC_DEV_A_900:".*-gfx900-linked-.*bc"]]
 
 // CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV_A_900]] "-mtriple=amdgcn-amd-amdhsa"
@@ -94,11 +94,11 @@
 // CHECK-SAME: {{.*}} "-main-file-name" "b.hip" {{.*}} "-target-cpu" "gfx803"
 // CHECK-SAME: "-fcuda-is-device" "-fvisibility" "hidden"
 // CHECK-SAME: "-fapply-global-visibility-to-externs"
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: {{.*}} "-o" [[B_BC_803:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[B_SRC:".*b.hip"]]
 
 // CHECK: [[LLVM_LINK:"*.llvm-link"]] [[B_BC_803]]
-// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: "-o" [[LINKED_BC_DEV_B_803:".*-gfx803-linked-.*bc"]]
 
 // CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV_B_803]] "-mtriple=amdgcn-amd-amdhsa"
@@ -122,11 +122,11 @@
 // CHECK-SAME: {{.*}} "-main-file-name" "b.hip" {{.*}} "-target-cpu" "gfx900"
 // CHECK-SAME: "-fcuda-is-device" "-fvi

[clang-tools-extra] r358091 - [clangd] Don't insert extra namespace qualifiers when Sema gets lost.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Wed Apr 10 08:16:54 2019
New Revision: 358091

URL: http://llvm.org/viewvc/llvm-project?rev=358091&view=rev
Log:
[clangd] Don't insert extra namespace qualifiers when Sema gets lost.

Summary:
There are cases where Sema can't tell that "foo" in foo::Bar is a
namespace qualifier, like in incomplete macro expansions.

After this patch, if sema reports no specifier but it looks like there's one in
the source code, then we take it into account.

Reworked structure and comments in getQueryScopes to try to fight
creeping complexity - unsure if I succeeded.

I made the test harder (the original version also passes).

Reviewers: ioeric

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

Tags: #clang

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

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

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=358091&r1=358090&r2=358091&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Wed Apr 10 08:16:54 2019
@@ -45,6 +45,7 @@
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
+#include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/Sema.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/None.h"
@@ -543,54 +544,59 @@ struct SpecifiedScope {
 // (e.g. enclosing namespace).
 std::pair, bool>
 getQueryScopes(CodeCompletionContext &CCContext, const Sema &CCSema,
+   const CompletionPrefix &HeuristicPrefix,
const CodeCompleteOptions &Opts) {
-  auto GetAllAccessibleScopes = [](CodeCompletionContext &CCContext) {
-SpecifiedScope Info;
-for (auto *Context : CCContext.getVisitedContexts()) {
-  if (isa(Context))
-Info.AccessibleScopes.push_back(""); // global namespace
-  else if (isa(Context))
-Info.AccessibleScopes.push_back(printNamespaceScope(*Context));
+  SpecifiedScope Scopes;
+  for (auto *Context : CCContext.getVisitedContexts()) {
+if (isa(Context))
+  Scopes.AccessibleScopes.push_back(""); // global namespace
+else if (isa(Context))
+  Scopes.AccessibleScopes.push_back(printNamespaceScope(*Context));
+  }
+
+  const CXXScopeSpec *SemaSpecifier =
+  CCContext.getCXXScopeSpecifier().getValueOr(nullptr);
+  // Case 1: unqualified completion.
+  if (!SemaSpecifier) {
+// Case 2 (exception): sema saw no qualifier, but there appears to be one!
+// This can happen e.g. in incomplete macro expansions. Use heuristics.
+if (!HeuristicPrefix.Qualifier.empty()) {
+  vlog("Sema said no scope specifier, but we saw {0} in the source code",
+   HeuristicPrefix.Qualifier);
+  StringRef SpelledSpecifier = HeuristicPrefix.Qualifier;
+  if (SpelledSpecifier.consume_front("::"))
+Scopes.AccessibleScopes = {""};
+  Scopes.UnresolvedQualifier = SpelledSpecifier;
+  return {Scopes.scopesForIndexQuery(), false};
 }
-return Info;
-  };
-
-  auto SS = CCContext.getCXXScopeSpecifier();
-
-  // Unqualified completion (e.g. "vec^").
-  if (!SS) {
-std::vector Scopes;
+// The enclosing namespace must be first, it gets a quality boost.
+std::vector EnclosingAtFront;
 std::string EnclosingScope = printNamespaceScope(*CCSema.CurContext);
-Scopes.push_back(EnclosingScope);
-for (auto &S : GetAllAccessibleScopes(CCContext).scopesForIndexQuery()) {
+EnclosingAtFront.push_back(EnclosingScope);
+for (auto &S : Scopes.scopesForIndexQuery()) {
   if (EnclosingScope != S)
-Scopes.push_back(std::move(S));
+EnclosingAtFront.push_back(std::move(S));
 }
-// Allow AllScopes completion only for there is no explicit scope 
qualifier.
-return {Scopes, Opts.AllScopes};
-  }
-
-  // Qualified completion ("std::vec^"), we have two cases depending on whether
-  // the qualifier can be resolved by Sema.
-  if ((*SS)->isValid()) { // Resolved qualifier.
-return {GetAllAccessibleScopes(CCContext).scopesForIndexQuery(), false};
+// Allow AllScopes completion as there is no explicit scope qualifier.
+return {EnclosingAtFront, Opts.AllScopes};
   }
-
-  // Unresolved qualifier.
-  SpecifiedScope Info = GetAllAccessibleScopes(CCContext);
-  Info.AccessibleScopes.push_back(""); // Make sure global scope is included.
-
-  llvm::StringRef SpelledSpecifier =
-  Lexer::getSourceText(CharSourceRange::getCharRange((*SS)->getRange()),
-   CCSema.SourceMgr, clang::LangOptions());
+  // Case 3: sema saw and resolved a scope qualifier.
+  if (Specifier && SemaSpecifier->isValid())
+return {Scopes.scopesForIndexQuery(), false};
+
+  

[PATCH] D60503: [clangd] Don't insert extra namespace qualifiers when Sema gets lost.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL358091: [clangd] Don't insert extra namespace 
qualifiers when Sema gets lost. (authored by sammccall, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60503?vs=194486&id=194520#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60503

Files:
  clang-tools-extra/trunk/clangd/CodeComplete.cpp
  clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
@@ -2384,19 +2384,19 @@
   UnorderedElementsAre(AllOf(Qualifier(""), Named("XYZ";
 }
 
-// Regression test: clang parser gets confused here and doesn't report the ns::
-// prefix - naive behavior is to insert it again.
-// However we can recognize this from the source code.
-// Test disabled until we can make it pass.
-TEST(CompletionTest, DISABLED_NamespaceDoubleInsertion) {
+// Clang parser gets confused here and doesn't report the ns:: prefix.
+// Naive behavior is to insert it again. We examine the source and recover.
+TEST(CompletionTest, NamespaceDoubleInsertion) {
   clangd::CodeCompleteOptions Opts = {};
 
   auto Results = completions(R"cpp(
+namespace foo {
 namespace ns {}
 #define M(X) < X
 M(ns::ABC^
+}
   )cpp",
- {cls("ns::ABCDE")}, Opts);
+ {cls("foo::ns::ABCDE")}, Opts);
   EXPECT_THAT(Results.Completions,
   UnorderedElementsAre(AllOf(Qualifier(""), Named("ABCDE";
 }
Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp
@@ -45,6 +45,7 @@
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
+#include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/Sema.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/None.h"
@@ -543,54 +544,59 @@
 // (e.g. enclosing namespace).
 std::pair, bool>
 getQueryScopes(CodeCompletionContext &CCContext, const Sema &CCSema,
+   const CompletionPrefix &HeuristicPrefix,
const CodeCompleteOptions &Opts) {
-  auto GetAllAccessibleScopes = [](CodeCompletionContext &CCContext) {
-SpecifiedScope Info;
-for (auto *Context : CCContext.getVisitedContexts()) {
-  if (isa(Context))
-Info.AccessibleScopes.push_back(""); // global namespace
-  else if (isa(Context))
-Info.AccessibleScopes.push_back(printNamespaceScope(*Context));
+  SpecifiedScope Scopes;
+  for (auto *Context : CCContext.getVisitedContexts()) {
+if (isa(Context))
+  Scopes.AccessibleScopes.push_back(""); // global namespace
+else if (isa(Context))
+  Scopes.AccessibleScopes.push_back(printNamespaceScope(*Context));
+  }
+
+  const CXXScopeSpec *SemaSpecifier =
+  CCContext.getCXXScopeSpecifier().getValueOr(nullptr);
+  // Case 1: unqualified completion.
+  if (!SemaSpecifier) {
+// Case 2 (exception): sema saw no qualifier, but there appears to be one!
+// This can happen e.g. in incomplete macro expansions. Use heuristics.
+if (!HeuristicPrefix.Qualifier.empty()) {
+  vlog("Sema said no scope specifier, but we saw {0} in the source code",
+   HeuristicPrefix.Qualifier);
+  StringRef SpelledSpecifier = HeuristicPrefix.Qualifier;
+  if (SpelledSpecifier.consume_front("::"))
+Scopes.AccessibleScopes = {""};
+  Scopes.UnresolvedQualifier = SpelledSpecifier;
+  return {Scopes.scopesForIndexQuery(), false};
 }
-return Info;
-  };
-
-  auto SS = CCContext.getCXXScopeSpecifier();
-
-  // Unqualified completion (e.g. "vec^").
-  if (!SS) {
-std::vector Scopes;
+// The enclosing namespace must be first, it gets a quality boost.
+std::vector EnclosingAtFront;
 std::string EnclosingScope = printNamespaceScope(*CCSema.CurContext);
-Scopes.push_back(EnclosingScope);
-for (auto &S : GetAllAccessibleScopes(CCContext).scopesForIndexQuery()) {
+EnclosingAtFront.push_back(EnclosingScope);
+for (auto &S : Scopes.scopesForIndexQuery()) {
   if (EnclosingScope != S)
-Scopes.push_back(std::move(S));
+EnclosingAtFront.push_back(std::move(S));
 }
-// Allow AllScopes completion only for there is no explicit scope qualifier.
-return {Scopes, Opts.AllScopes};
-  }
-
-  // Qualified completion ("std::vec^"), we have two cases depending on whether
-  // the qualifier can be resolved by Sema.
-  if ((*SS)->isValid()) 

[PATCH] D60516: [LTO] Add plumbing to save stats during LTO on MacOS.

[Florian Hahn via Phabricator via cfe-commits]

fhahn created this revision.
fhahn added reviewers: anemet, tejohnson, thegameg.
Herald added subscribers: cfe-commits, dang, dexonsmith, steven_wu, hiraditya, 
inglorion, mehdi_amini.
Herald added projects: clang, LLVM.

Gold and ld on Linux already support saving stats, but the
infrastructure is missing on MacOS. Unfortunately it seems like the
configuration from lib/LTO/LTO.cpp is not used.

This patch adds a new LTOStatsFile option and adds plumbing in Clang to
use it on MacOS, similar to the way remarks are handled.

Currnetly the handling of LTO flags seems quite spread out, with a bunch
of duplication. But I am not sure if there is an easy way to improve
that?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60516

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  llvm/include/llvm/LTO/LTO.h
  llvm/include/llvm/LTO/legacy/LTOCodeGenerator.h
  llvm/lib/LTO/LTO.cpp
  llvm/lib/LTO/LTOCodeGenerator.cpp

Index: llvm/lib/LTO/LTOCodeGenerator.cpp
===
--- llvm/lib/LTO/LTOCodeGenerator.cpp
+++ llvm/lib/LTO/LTOCodeGenerator.cpp
@@ -95,6 +95,11 @@
 "lto-pass-remarks-with-hotness",
 cl::desc("With PGO, include profile count in optimization remarks"),
 cl::Hidden);
+
+cl::opt LTOStatsFile(
+"lto-stats-file",
+cl::desc("With PGO, include profile count in optimization remarks"),
+cl::Hidden);
 }
 
 LTOCodeGenerator::LTOCodeGenerator(LLVMContext &Context)
@@ -518,6 +523,14 @@
   }
   DiagnosticOutputFile = std::move(*DiagFileOrErr);
 
+  // Setup output file to emit statistics.
+  auto StatsFileOrErr = lto::setupStatsFile(LTOStatsFile);
+  if (!StatsFileOrErr) {
+errs() << "Error: " << toString(StatsFileOrErr.takeError()) << "\n";
+report_fatal_error("Can't get an output file for the statistics");
+  }
+  StatsFile = std::move(StatsFileOrErr.get());
+
   // We always run the verifier once on the merged module, the `DisableVerify`
   // parameter only applies to subsequent verify.
   verifyMergedModuleOnce();
@@ -585,8 +598,12 @@
   ShouldRestoreGlobalsLinkage);
 
   // If statistics were requested, print them out after codegen.
-  if (llvm::AreStatisticsEnabled())
+  if (llvm::AreStatisticsEnabled() && !StatsFile)
 llvm::PrintStatistics();
+
+  if (StatsFile)
+PrintStatisticsJSON(StatsFile->os());
+
   reportAndResetTimings();
 
   finishOptimizationRemarks();
Index: llvm/lib/LTO/LTO.cpp
===
--- llvm/lib/LTO/LTO.cpp
+++ llvm/lib/LTO/LTO.cpp
@@ -880,16 +880,10 @@
   isPrevailing, Conf.OptLevel > 0);
 
   // Setup output file to emit statistics.
-  std::unique_ptr StatsFile = nullptr;
-  if (!Conf.StatsFile.empty()) {
-EnableStatistics(false);
-std::error_code EC;
-StatsFile =
-llvm::make_unique(Conf.StatsFile, EC, sys::fs::F_None);
-if (EC)
-  return errorCodeToError(EC);
-StatsFile->keep();
-  }
+  auto StatsFileOrErr = setupStatsFile(Conf.StatsFile);
+  if (!StatsFileOrErr)
+return StatsFileOrErr.takeError();
+  std::unique_ptr StatsFile = std::move(StatsFileOrErr.get());
 
   // Finalize linking of regular LTO modules containing summaries now that
   // we have computed liveness information.
@@ -1338,3 +1332,20 @@
   DiagnosticFile->keep();
   return std::move(DiagnosticFile);
 }
+
+Expected>
+lto::setupStatsFile(StringRef StatsFilename) {
+  // Setup output file to emit statistics.
+  if (StatsFilename.empty())
+return nullptr;
+
+  llvm::EnableStatistics(false);
+  std::error_code EC;
+  auto StatsFile =
+  llvm::make_unique(StatsFilename, EC, sys::fs::F_None);
+  if (EC)
+return errorCodeToError(EC);
+
+  StatsFile->keep();
+  return std::move(StatsFile);
+}
Index: llvm/include/llvm/LTO/legacy/LTOCodeGenerator.h
===
--- llvm/include/llvm/LTO/legacy/LTOCodeGenerator.h
+++ llvm/include/llvm/LTO/legacy/LTOCodeGenerator.h
@@ -241,6 +241,7 @@
   TargetMachine::CodeGenFileType FileType = TargetMachine::CGFT_ObjectFile;
   std::unique_ptr DiagnosticOutputFile;
   bool Freestanding = false;
+  std::unique_ptr StatsFile = nullptr;
 };
 }
 #endif
Index: llvm/include/llvm/LTO/LTO.h
===
--- llvm/include/llvm/LTO/LTO.h
+++ llvm/include/llvm/LTO/LTO.h
@@ -87,6 +87,10 @@
  StringRef LTORemarksPasses,
  bool LTOPassRemarksWithHotness, int Count = -1);
 
+/// Setups the output file for saving statistics.
+Expected>
+setupStatsFile(StringRef StatsFilename);
+
 class LTO;
 struct SymbolResolution;
 class ThinBackendProc;
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -514,6 +514,14 @@
 }
   }
 
+  // Setup stat

[PATCH] D59746: [LibTooling] Fix '-h' option

[James Henderson via Phabricator via cfe-commits]

jhenderson added a comment.

Just chiming in from a LLVM binutils developer perspective. Some of the 
binutils are missing -h as an alias, when they really should have it for 
compatibility (specifically I checked llvm-nm and llvm-symbolizer). As a 
result, a solution that auto-adds -h as an alias would be good, as long as we 
have the ability to override it somehow. I wouldn't mind having logic in 
llvm-objdump/llvm-readobj to explicitly override the short alias if it is 
required.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



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

[PATCH] D59692: [ASTImporter] Fix name conflict handling

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Ping @a_sidorin @shafik


Repository:
  rC Clang

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

https://reviews.llvm.org/D59692



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

[PATCH] D59692: [ASTImporter] Fix name conflict handling

[Gabor Marton via Phabricator via cfe-commits]

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

- Put back the call to GetOriginalDecl


Repository:
  rC Clang

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

https://reviews.llvm.org/D59692

Files:
  include/clang/AST/ASTImporter.h
  lib/AST/ASTImporter.cpp
  test/Analysis/Inputs/ctu-other.c
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1982,7 +1982,7 @@
   auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
   // We expect one (ODR) warning during the import.
   EXPECT_EQ(1u, ToTU->getASTContext().getDiagnostics().getNumWarnings());
-  EXPECT_EQ(2u,
+  EXPECT_EQ(1u,
 DeclCounter().match(ToTU, recordDecl(hasName("X";
 }
 
@@ -2692,6 +2692,64 @@
 functionDecl(hasName("f"), hasDescendant(declRefExpr()));
 }
 
+struct ImportFunctionTemplates : ASTImporterOptionSpecificTestBase {};
+
+TEST_P(ImportFunctionTemplates,
+   ImportFunctionWhenThereIsAFunTemplateWithSameName) {
+  getToTuDecl(
+  R"(
+  template 
+  void foo(T) {}
+  void foo();
+  )",
+  Lang_CXX);
+  Decl *FromTU = getTuDecl("void foo();", Lang_CXX);
+  auto *FromD = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasName("foo")));
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
+TEST_P(ImportFunctionTemplates,
+   ImportConstructorWhenThereIsAFunTemplateWithSameName) {
+  auto Code =
+  R"(
+  struct Foo {
+template 
+Foo(T) {}
+Foo();
+  };
+  )";
+  getToTuDecl(Code, Lang_CXX);
+  Decl *FromTU = getTuDecl(Code, Lang_CXX);
+  auto *FromD =
+  LastDeclMatcher().match(FromTU, cxxConstructorDecl());
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
+TEST_P(ImportFunctionTemplates,
+   ImportOperatorWhenThereIsAFunTemplateWithSameName) {
+  getToTuDecl(
+  R"(
+  template 
+  void operator<(T,T) {}
+  struct X{};
+  void operator<(X, X);
+  )",
+  Lang_CXX);
+  Decl *FromTU = getTuDecl(
+  R"(
+  struct X{};
+  void operator<(X, X);
+  )",
+  Lang_CXX);
+  auto *FromD = LastDeclMatcher().match(
+  FromTU, functionDecl(hasOverloadedOperatorName("<")));
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
 struct ImportFriendFunctions : ImportFunctions {};
 
 TEST_P(ImportFriendFunctions, ImportFriendFunctionRedeclChainProto) {
@@ -5552,6 +5610,9 @@
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctions,
 DefaultTestValuesForRunOptions, );
 
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctionTemplates,
+DefaultTestValuesForRunOptions, );
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFriendFunctionTemplates,
 DefaultTestValuesForRunOptions, );
 
Index: test/Analysis/Inputs/ctu-other.c
===
--- test/Analysis/Inputs/ctu-other.c
+++ test/Analysis/Inputs/ctu-other.c
@@ -12,11 +12,11 @@
 }
 
 // Test enums.
-enum B { x = 42,
- l,
- s };
+enum B { x2 = 42,
+ y2,
+ z2 };
 int enumCheck(void) {
-  return x;
+  return x2;
 }
 
 // Test reporting an error in macro definition
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -79,6 +79,7 @@
   using ExpectedExpr = llvm::Expected;
   using ExpectedDecl = llvm::Expected;
   using ExpectedSLoc = llvm::Expected;
+  using ExpectedName = llvm::Expected;
 
   std::string ImportError::toString() const {
 // FIXME: Improve error texts.
@@ -2135,11 +2136,11 @@
 }
 
 if (!ConflictingDecls.empty()) {
-  Name = Importer.HandleNameConflict(Name, DC, Decl::IDNS_Namespace,
- ConflictingDecls.data(),
- ConflictingDecls.size());
-  if (!Name)
-return make_error(ImportError::NameConflict);
+  ExpectedName NameOrErr = Importer.HandleNameConflict(
+  Name, DC, Decl::IDNS_Namespace, ConflictingDecls.data(),
+  ConflictingDecls.size());
+  if (!NameOrErr)
+return NameOrErr.takeError();
 }
   }
 
@@ -2243,20 +2244,17 @@
   // already have a complete underlying type then return with that.
   if (!FromUT->isIncompleteType() && !FoundUT->isIncompleteType())
 return Importer.MapImported(D, FoundTypedef);
+} else {
+  ConflictingDecls.push_back(FoundDecl);
 }
-// FIXME Handle redecl chain.
-break;
   }
-
-  ConflictingDecls.push_back(FoundDecl);
 }
 
 if (!ConflictingDecls.empty()) {
-  Name = Importe

[clang-tools-extra] r358093 - clangd: repair the build after SVN r358091

[Saleem Abdulrasool via cfe-commits]

Author: compnerd
Date: Wed Apr 10 08:45:05 2019
New Revision: 358093

URL: http://llvm.org/viewvc/llvm-project?rev=358093&view=rev
Log:
clangd: repair the build after SVN r358091

Fix the name of the variable being checked.  NFCI.

Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=358093&r1=358092&r2=358093&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Wed Apr 10 08:45:05 2019
@@ -581,7 +581,7 @@ getQueryScopes(CodeCompletionContext &CC
 return {EnclosingAtFront, Opts.AllScopes};
   }
   // Case 3: sema saw and resolved a scope qualifier.
-  if (Specifier && SemaSpecifier->isValid())
+  if (SemaSpecifier && SemaSpecifier->isValid())
 return {Scopes.scopesForIndexQuery(), false};
 
   // Case 4: There was a qualifier, and Sema didn't resolve it.


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

[clang-tools-extra] r358094 - [clangd] Use #if CLANGD_BUILD_XPC because it may be defined as 0

[Fangrui Song via cfe-commits]

Author: maskray
Date: Wed Apr 10 08:45:54 2019
New Revision: 358094

URL: http://llvm.org/viewvc/llvm-project?rev=358094&view=rev
Log:
[clangd] Use #if CLANGD_BUILD_XPC because it may be defined as 0

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

Modified: clang-tools-extra/trunk/clangd/Transport.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Transport.h?rev=358094&r1=358093&r2=358094&view=diff
==
--- clang-tools-extra/trunk/clangd/Transport.h (original)
+++ clang-tools-extra/trunk/clangd/Transport.h Wed Apr 10 08:45:54 2019
@@ -85,7 +85,7 @@ newJSONTransport(std::FILE *In, llvm::ra
  llvm::raw_ostream *InMirror, bool Pretty,
  JSONStreamStyle = JSONStreamStyle::Standard);
 
-#ifdef CLANGD_BUILD_XPC
+#if CLANGD_BUILD_XPC
 // Returns a Transport for macOS based on XPC.
 // Clangd with this transport is meant to be run as bundled XPC service.
 std::unique_ptr newXPCTransport();


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

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clangd/Diagnostics.cpp:396
+  for (llvm::StringRef Inc : IncludeStack)
+OS << "In file included from: " << Inc << ":\n";
+}

ilya-biryukov wrote:
> kadircet wrote:
> > ilya-biryukov wrote:
> > > NIT: could we reuse the function from clang that does the same thing?
> > > 
> > > The code here is pretty simple, though, so writing it here is fine. Just 
> > > wanted to make sure we considered this option and found it's easier to 
> > > redo this work ourselves.
> > there is `TextDiagnostic` which prints the desired output expect the fact 
> > that it also prints the first line saying the header was included in main 
> > file. Therefore, I didn't make use of it since we decided that was not very 
> > useful for our use case. And it requires inheriting from 
> > `DiagnosticRenderer` which was requiring too much boiler plate code just to 
> > get this functionality so instead I've chosen doing it like this.
> > 
> > Can fallback to `TextDiagnostic` if you believe showing `Included in 
> > mainfile.cc:x:y:` at the beginning of the diagnostic won't be annoying.
> LG, it's not too hard to reconstruct the same output.
> Note that D60267 starts outputting notes in a structured way, using the 
> `RelatedInformation` struct from the LSP.
> 
> We should eventually add the include stack into related information too. With 
> that in mind, we should probably add the include stack as a new field to the 
> struct we use for diagnostics.
> 
SG, delaying serialization to LSP layer then.



Comment at: clangd/Diagnostics.cpp:372
+auto IncludeLocation = Info.getSourceManager()
+   .getPresumedLoc(Info.getLocation(),
+   /*UseLineDirectives=*/false)

ilya-biryukov wrote:
> Why use `getPresumedLoc`? Making clangd sensitive to pp directives that 
> rename file or change line numbers does not make any sense.
> 
> Could you also add tests for that?
It was the way `DiagnosticRenderer` emitted include stacks, I had just copied 
it over.
Changing with `SourceManger::getIncludeLoc`


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59302



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

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 194528.
kadircet marked 10 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59302

Files:
  clangd/ClangdUnit.cpp
  clangd/Diagnostics.cpp
  clangd/Diagnostics.h
  unittests/clangd/DiagnosticsTests.cpp

Index: unittests/clangd/DiagnosticsTests.cpp
===
--- unittests/clangd/DiagnosticsTests.cpp
+++ unittests/clangd/DiagnosticsTests.cpp
@@ -9,6 +9,7 @@
 #include "Annotations.h"
 #include "ClangdUnit.h"
 #include "SourceCode.h"
+#include "TestFS.h"
 #include "TestIndex.h"
 #include "TestTU.h"
 #include "index/MemIndex.h"
@@ -45,6 +46,17 @@
   return arg.Range == Range && arg.Message == Message;
 }
 
+MATCHER_P3(Diag, Range, Message, IncludeStack,
+   "Diag at " + llvm::to_string(Range) + " = [" + Message + "]") {
+  if (arg.Range != Range || arg.Message != Message ||
+  arg.IncludeStack.size() != IncludeStack.size())
+return false;
+  for (size_t I = 0, E = IncludeStack.size(); I < E; ++I)
+if (IncludeStack[I] != arg.IncludeStack[I])
+  return false;
+  return true;
+}
+
 MATCHER_P3(Fix, Range, Replacement, Message,
"Fix " + llvm::to_string(Range) + " => " +
testing::PrintToString(Replacement) + " = [" + Message + "]") {
@@ -73,7 +85,6 @@
   return true;
 }
 
-
 // Helper function to make tests shorter.
 Position pos(int line, int character) {
   Position Res;
@@ -251,6 +262,8 @@
   D.InsideMainFile = true;
   D.Severity = DiagnosticsEngine::Error;
   D.File = "foo/bar/main.cpp";
+  D.IncludeStack.push_back("a/b.h:1:2");
+  D.IncludeStack.push_back("a/c.h:2:2");
 
   clangd::Note NoteInMain;
   NoteInMain.Message = "declared somewhere in the main file";
@@ -282,7 +295,8 @@
 
   // Diagnostics should turn into these:
   clangd::Diagnostic MainLSP =
-  MatchingLSP(D, R"(Something terrible happened (fix available)
+  MatchingLSP(D, R"(In file included from: a/b.h:1:2: 
+a/c.h:2:2: something terrible happened (fix available)
 
 main.cpp:6:7: remark: declared somewhere in the main file
 
@@ -603,7 +617,185 @@
   "Add include \"x.h\" for symbol a::X");
 }
 
+ParsedAST build(const std::string &MainFile,
+const llvm::StringMap &Files) {
+  std::vector Cmd = {"clang", MainFile.c_str()};
+  ParseInputs Inputs;
+  Inputs.CompileCommand.Filename = MainFile;
+  Inputs.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()};
+  Inputs.CompileCommand.Directory = testRoot();
+  Inputs.Contents = Files.lookup(MainFile);
+  Inputs.FS = buildTestFS(Files);
+  Inputs.Opts = ParseOptions();
+  auto PCHs = std::make_shared();
+  auto CI = buildCompilerInvocation(Inputs);
+  assert(CI && "Failed to build compilation invocation.");
+  auto Preamble =
+  buildPreamble(MainFile, *CI,
+/*OldPreamble=*/nullptr,
+/*OldCompileCommand=*/Inputs.CompileCommand, Inputs,
+/*StoreInMemory=*/true, /*PreambleCallback=*/nullptr);
+  auto AST = buildAST(MainFile, createInvocationFromCommandLine(Cmd), Inputs,
+  Preamble);
+  if (!AST.hasValue()) {
+ADD_FAILURE() << "Failed to build code:\n" << Files.lookup(MainFile);
+llvm_unreachable("Failed to build TestTU!");
+  }
+  return std::move(*AST);
+}
+
+TEST(DiagsInHeaders, DiagInsideHeader) {
+  Annotations Main(R"cpp(
+#include [["a.h"]]
+void foo() {})cpp");
+  std::string MainFile = testPath("main.cc");
+  llvm::StringMap Files = {{MainFile, Main.code()},
+{testPath("a.h"), "no_type_spec;"}};
+  auto FS = buildTestFS(Files);
+
+  EXPECT_THAT(build(MainFile, Files).getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Main.range(),
+   "C++ requires a type specifier for all declarations",
+   std::vector{"/clangd-test/a.h:1:1"})));
+}
+
+TEST(DiagsInHeaders, DiagInTransitiveInclude) {
+  Annotations Main(R"cpp(
+#include [["a.h"]]
+void foo() {})cpp");
+  std::string MainFile = testPath("main.cc");
+  llvm::StringMap Files = {{MainFile, Main.code()},
+{testPath("a.h"), "#include \"b.h\""},
+{testPath("b.h"), "no_type_spec;"}};
+  auto FS = buildTestFS(Files);
+
+  EXPECT_THAT(build(MainFile, Files).getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Main.range(),
+   "C++ requires a type specifier for all declarations",
+   std::vector{"/clangd-test/a.h:1:10",
+"/clangd-test/b.h:1:1"})));
+}
+
+TEST(DiagsInHeaders, DiagInMultipleHeaders) {
+  Annotations Main(R"cpp(
+#include $a[["a.h"]]
+#include $b[["b.h"]]
+void foo() {})cpp");
+

[PATCH] D60161: Expose non-trivial struct helpers for Swift.

[Tony Allevato via Phabricator via cfe-commits]

allevato updated this revision to Diff 194530.
allevato added a comment.

- Rename generate* to get* and cleanup param names.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60161

Files:
  clang/include/clang/CodeGen/CodeGenABITypes.h
  clang/lib/CodeGen/CGNonTrivialStruct.cpp

Index: clang/lib/CodeGen/CGNonTrivialStruct.cpp
===
--- clang/lib/CodeGen/CGNonTrivialStruct.cpp
+++ clang/lib/CodeGen/CGNonTrivialStruct.cpp
@@ -14,6 +14,7 @@
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "clang/AST/NonTrivialTypeVisitor.h"
+#include "clang/CodeGen/CodeGenABITypes.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include 
 
@@ -822,6 +823,29 @@
   Gen.callFunc(FuncName, QT, Addrs, CGF);
 }
 
+template  std::array createNullAddressArray();
+
+template <> std::array createNullAddressArray() {
+  return std::array({Address(nullptr, CharUnits::Zero())});
+}
+
+template <> std::array createNullAddressArray() {
+  return std::array({Address(nullptr, CharUnits::Zero()),
+ Address(nullptr, CharUnits::Zero())});
+}
+
+template 
+static llvm::Function *
+getSpecialFunction(G &&Gen, StringRef FuncName, QualType QT, bool IsVolatile,
+   std::array Alignments, CodeGenModule &CGM) {
+  QT = IsVolatile ? QT.withVolatile() : QT;
+  // The following call requires an array of addresses as arguments, but doesn't
+  // actually use them (it overwrites them with the addresses of the arguments
+  // of the created function).
+  return Gen.getFunction(FuncName, QT, createNullAddressArray(), Alignments,
+ CGM);
+}
+
 // Functions to emit calls to the special functions of a non-trivial C struct.
 void CodeGenFunction::callCStructDefaultConstructor(LValue Dst) {
   bool IsVolatile = Dst.isVolatile();
@@ -907,3 +931,69 @@
   callSpecialFunction(GenMoveAssignment(getContext()), FuncName, QT, IsVolatile,
   *this, std::array({{DstPtr, SrcPtr}}));
 }
+
+llvm::Function *clang::CodeGen::getNonTrivialCStructDefaultConstructor(
+CodeGenModule &CGM, CharUnits DstAlignment, bool IsVolatile, QualType QT) {
+  ASTContext &Ctx = CGM.getContext();
+  GenDefaultInitializeFuncName GenName(DstAlignment, Ctx);
+  std::string FuncName = GenName.getName(QT, IsVolatile);
+  return getSpecialFunction(GenDefaultInitialize(Ctx), FuncName, QT, IsVolatile,
+std::array({{DstAlignment}}), CGM);
+}
+
+llvm::Function *clang::CodeGen::getNonTrivialCStructCopyConstructor(
+CodeGenModule &CGM, CharUnits DstAlignment, CharUnits SrcAlignment,
+bool IsVolatile, QualType QT) {
+  ASTContext &Ctx = CGM.getContext();
+  GenBinaryFuncName GenName("__copy_constructor_", DstAlignment,
+   SrcAlignment, Ctx);
+  std::string FuncName = GenName.getName(QT, IsVolatile);
+  return getSpecialFunction(
+  GenCopyConstructor(Ctx), FuncName, QT, IsVolatile,
+  std::array({{DstAlignment, SrcAlignment}}), CGM);
+}
+
+llvm::Function *clang::CodeGen::getNonTrivialCStructMoveConstructor(
+CodeGenModule &CGM, CharUnits DstAlignment, CharUnits SrcAlignment,
+bool IsVolatile, QualType QT) {
+  ASTContext &Ctx = CGM.getContext();
+  GenBinaryFuncName GenName("__move_constructor_", DstAlignment,
+  SrcAlignment, Ctx);
+  std::string FuncName = GenName.getName(QT, IsVolatile);
+  return getSpecialFunction(
+  GenMoveConstructor(Ctx), FuncName, QT, IsVolatile,
+  std::array({{DstAlignment, SrcAlignment}}), CGM);
+}
+
+llvm::Function *clang::CodeGen::getNonTrivialCStructCopyAssignmentOperator(
+CodeGenModule &CGM, CharUnits DstAlignment, CharUnits SrcAlignment,
+bool IsVolatile, QualType QT) {
+  ASTContext &Ctx = CGM.getContext();
+  GenBinaryFuncName GenName("__copy_assignment_", DstAlignment,
+   SrcAlignment, Ctx);
+  std::string FuncName = GenName.getName(QT, IsVolatile);
+  return getSpecialFunction(
+  GenCopyAssignment(Ctx), FuncName, QT, IsVolatile,
+  std::array({{DstAlignment, SrcAlignment}}), CGM);
+}
+
+llvm::Function *clang::CodeGen::getNonTrivialCStructMoveAssignmentOperator(
+CodeGenModule &CGM, CharUnits DstAlignment, CharUnits SrcAlignment,
+bool IsVolatile, QualType QT) {
+  ASTContext &Ctx = CGM.getContext();
+  GenBinaryFuncName GenName("__move_assignment_", DstAlignment,
+  SrcAlignment, Ctx);
+  std::string FuncName = GenName.getName(QT, IsVolatile);
+  return getSpecialFunction(
+  GenMoveAssignment(Ctx), FuncName, QT, IsVolatile,
+  std::array({{DstAlignment, SrcAlignment}}), CGM);
+}
+
+llvm::Function *clang::CodeGen::getNonTrivialCStructDestructor(
+CodeGenModule &CGM, CharUnits DstAlignment, bool IsVolatile, QualType QT) {
+  ASTContext &Ctx = CGM.getContext();
+  GenDestructorFuncName GenName("__

[PATCH] D60161: Expose non-trivial struct helpers for Swift.

[Tony Allevato via Phabricator via cfe-commits]

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

Thanks for the review! This is ready to go on my end if there aren't any other 
comments.




Comment at: clang/lib/CodeGen/CGNonTrivialStruct.cpp:845
+  // actually use them (it overwrites them with the addresses of the arguments
+  // of the created function).
+  return Gen.getFunction(FuncName, QT, createNullAddressArray(), Alignments,

ahatanak wrote:
> I think `getFunction` shouldn't require passing addresses, but that's not 
> this patch's fault. I'll see if I can remove the parameter. 
Yeah, this hack bummed me out and I tried cleaning up the other related 
functions to have them not reuse the array in this way, but the way std::array 
and the template arg size_t N are currently being used throughout made those 
attempts ripple through in ways that would have required a deeper refactor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60161



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

[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.

[Samuel Benzaquen via Phabricator via cfe-commits]

sbenza added inline comments.



Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:45
+  /// Evaluates this part to a string and appends it to `result`.
+  virtual llvm::Error eval(const ast_matchers::MatchFinder::MatchResult &Match,
+   std::string *Result) const = 0;

Why not use `llvm::Expected` as the return type?



Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:75
+  // Copy constructor/assignment produce a deep copy.
+  StencilPart(const StencilPart &P) : Impl(P.Impl->clone()) {}
+  StencilPart(StencilPart &&) = default;

The interface API is all const. Why not keep a `shared_ptr` instead?
That way we don't have to have users implement a clone function.



Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:94
+  return false;
+return Impl->isEqual(*(Other.Impl));
+  }

Parens around Other.Impl are not necessary.



Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:110
+  template  static Stencil cat(Ts &&... Parts) {
+Stencil Stencil;
+Stencil.Parts.reserve(sizeof...(Parts));

I don't like naming the variable the same as the type.
You could just use `S` as the variable name. That is ok with llvm style for 
small functions.



Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:127
+  // Allow Stencils to operate as std::function, for compatibility with
+  // Transformer's TextGenerator. Calls `eval()` and asserts on failure.
+  std::string operator()(const ast_matchers::MatchFinder::MatchResult &) const;

"asserts" as it only fails in debug mode?



Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:130
+
+  bool operator==(const Stencil &Other) const {
+return Parts == Other.Parts;

I usually prefer free functions for binary operators (friends is fine).
It makes them symmetric.



Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:155
+/// statement.
+StencilPart snode(llvm::StringRef Id);
+

`sNode` ?
ie camelCase



Comment at: clang/lib/Tooling/Refactoring/Stencil.cpp:45
+
+namespace {
+// An arbitrary fragment of code within a stencil.

maybe this anon namespace should cover the functions above?



Comment at: clang/lib/Tooling/Refactoring/Stencil.cpp:76
+
+static bool operator==(const RawTextData &A, const RawTextData &B) {
+  return A.Text == B.Text;

If you define ==, imo you should define != too.
Otherwise just call it `isEqual`



Comment at: clang/lib/Tooling/Refactoring/Stencil.cpp:94
+Error evalData(const T &Data, const MatchFinder::MatchResult &Match,
+   std::string *Result);
+

alignment.
(below too)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59371



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

[clang-tools-extra] r358098 - [clangd] Fix non-indexing of builtin functions like printf when the TU is C

[Sam McCall via cfe-commits]

Author: sammccall
Date: Wed Apr 10 09:26:58 2019
New Revision: 358098

URL: http://llvm.org/viewvc/llvm-project?rev=358098&view=rev
Log:
[clangd] Fix non-indexing of builtin functions like printf when the TU is C

Modified:
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp

Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=358098&r1=358097&r2=358098&view=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Wed Apr 10 
09:26:58 2019
@@ -236,8 +236,6 @@ bool SymbolCollector::shouldCollectSymbo
   const ASTContext &ASTCtx,
   const Options &Opts,
   bool IsMainFileOnly) {
-  if (ND.isImplicit())
-return false;
   // Skip anonymous declarations, e.g (anonymous enum/class/struct).
   if (ND.getDeclName().isEmpty())
 return false;
@@ -328,7 +326,9 @@ bool SymbolCollector::handleDeclOccurenc
   // then no public declaration was visible, so assume it's main-file only.
   bool IsMainFileOnly =
   SM.isWrittenInMainFile(SM.getExpansionLoc(ND->getBeginLoc()));
-  if (!shouldCollectSymbol(*ND, *ASTCtx, Opts, IsMainFileOnly))
+  // In C, printf is a redecl of an implicit builtin! So check OrigD instead.
+  if (ASTNode.OrigD->isImplicit() ||
+  !shouldCollectSymbol(*ND, *ASTCtx, Opts, IsMainFileOnly))
 return true;
   // Do not store references to main-file symbols.
   if (CollectRef && !IsMainFileOnly && !isa(ND) &&

Modified: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp?rev=358098&r1=358097&r2=358098&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Wed Apr 
10 09:26:58 2019
@@ -254,9 +254,8 @@ public:
 auto Factory = llvm::make_unique(
 CollectorOpts, PragmaHandler.get());
 
-std::vector Args = {
-"symbol_collector", "-fsyntax-only", "-xc++",
-"-std=c++11",   "-include",  TestHeaderName};
+std::vector Args = {"symbol_collector", "-fsyntax-only",
+ "-xc++", "-include", TestHeaderName};
 Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end());
 // This allows to override the "-xc++" with something else, i.e.
 // -xobjective-c++.
@@ -1165,6 +1164,15 @@ TEST_F(SymbolCollectorTest, UsingDecl) {
   EXPECT_THAT(Symbols, Contains(QName("std::foo")));
 }
 
+TEST_F(SymbolCollectorTest, CBuiltins) {
+  // In C, printf in stdio.h is a redecl of an implicit builtin.
+  const char *Header = R"(
+extern int printf(const char*, ...);
+  )";
+  runSymbolCollector(Header, /**/ "", {"-xc"});
+  EXPECT_THAT(Symbols, Contains(QName("printf")));
+}
+
 } // 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] D60161: Expose non-trivial struct helpers for Swift.

[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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60161



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

[PATCH] D60161: Expose non-trivial struct helpers for Swift.

[Tony Allevato via Phabricator via cfe-commits]

allevato added a comment.

Oh, and in case I need to mention this (I don't contribute to llvm/clang 
frequently), I don't have commit access so I'll need someone else to merge it 
in. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60161



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

[PATCH] D59746: [LibTooling] Fix '-h' option

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 194539.
hintonda added a comment.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

- Add DefaultOption logic.

Still needs tests, but wanted to get early feedback on basic approach.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746

Files:
  clang/lib/Tooling/CommonOptionsParser.cpp
  llvm/docs/CommandLine.rst
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp

Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -144,6 +144,13 @@
   }
 
   void addOption(Option *O, SubCommand *SC) {
+// Handle DefaultOptions
+if (O->getMiscFlags() & cl::DefaultOption) {
+  std::string DefaultArg(O->ArgStr.str() + ":default_option");
+  SC->DefaultOptions.push_back(DefaultArg);
+  O->setArgStr(SC->DefaultOptions.back());
+}
+
 bool HadErrors = false;
 if (O->hasArgStr()) {
   // Add argument to the argument map!
@@ -1167,6 +1174,22 @@
   auto &SinkOpts = ChosenSubCommand->SinkOpts;
   auto &OptionsMap = ChosenSubCommand->OptionsMap;
 
+  // Handle DefaultOptions.
+  for (auto const &DA : ChosenSubCommand->DefaultOptions) {
+StringRef Arg(DA);
+StringRef Val;
+if (Option *Opt = LookupOption(*ChosenSubCommand, Arg, Val)) {
+  StringRef NewArg = Arg.substr(0, Arg.find(":"));
+  if (LookupOption(*ChosenSubCommand, NewArg, Val)) {
+removeOption(Opt, ChosenSubCommand);
+  } else {
+// FIXME: This is needed, but not sure why.
+updateArgStr(Opt, NewArg, ChosenSubCommand);
+Opt->setArgStr(NewArg);
+  }
+}
+  }
+
   if (ConsumeAfterOpt) {
 assert(PositionalOpts.size() > 0 &&
"Cannot specify cl::ConsumeAfter without a positional argument!");
@@ -2146,6 +2169,10 @@
 cl::location(WrappedNormalPrinter), cl::ValueDisallowed,
 cl::cat(GenericCategory), cl::sub(*AllSubCommands));
 
+static cl::alias HOpA("h", cl::desc("Alias for -help"), cl::aliasopt(HOp),
+  cl::cat(GenericCategory), cl::sub(*AllSubCommands),
+  cl::DefaultOption);
+
 static cl::opt>
 HHOp("help-hidden", cl::desc("Display all available options"),
  cl::location(WrappedHiddenPrinter), cl::Hidden, cl::ValueDisallowed,
Index: llvm/include/llvm/Support/CommandLine.h
===
--- llvm/include/llvm/Support/CommandLine.h
+++ llvm/include/llvm/Support/CommandLine.h
@@ -175,7 +175,10 @@
   // If this is enabled, multiple letter options are allowed to bunch together
   // with only a single hyphen for the whole group.  This allows emulation
   // of the behavior that ls uses for example: ls -la === ls -l -a
-  Grouping = 0x08
+  Grouping = 0x08,
+
+  // Default option
+  DefaultOption = 0x10
 };
 
 //===--===//
@@ -231,6 +234,7 @@
   SmallVector PositionalOpts;
   SmallVector SinkOpts;
   StringMap OptionsMap;
+  SmallVector DefaultOptions;
 
   Option *ConsumeAfterOpt = nullptr; // The ConsumeAfter option if it exists.
 };
@@ -270,7 +274,7 @@
   unsigned Value : 2;
   unsigned HiddenFlag : 2; // enum OptionHidden
   unsigned Formatting : 2; // enum FormattingFlags
-  unsigned Misc : 4;
+  unsigned Misc : 5;
   unsigned Position = 0;   // Position of last occurrence of the option
   unsigned AdditionalVals = 0; // Greater than 0 for multi-valued option.
 
Index: llvm/docs/CommandLine.rst
===
--- llvm/docs/CommandLine.rst
+++ llvm/docs/CommandLine.rst
@@ -128,6 +128,7 @@
   USAGE: compiler [options]
 
   OPTIONS:
+-h- Alias for -help
 -help - display available options (-help-hidden for more)
 -o  - Specify output filename
 
@@ -194,6 +195,7 @@
   USAGE: compiler [options] 
 
   OPTIONS:
+-h- Alias for -help
 -help - display available options (-help-hidden for more)
 -o  - Specify output filename
 
@@ -1251,6 +1253,14 @@
   with ``cl::CommaSeparated``, this modifier only makes sense with a `cl::list`_
   option.
 
+.. _cl::DefaultOption:
+
+* The **cl::DefaultOption** modifier is used to specify that the option is a
+  default that can be overridden by application specific parsers. For example,
+  the ``-help`` alias, ``-h``, is registered this way, so it can be overridden
+  by applications that need to use the ``-h`` option for another purpose,
+  either as a regular option or an alias for another option.
+
 .. _response files:
 
 Response files
Index: clang/lib/Tooling/CommonOptionsParser.cpp
===
--- clang/lib/Tooling/CommonOptionsParser.cpp
+++ c

[clang-tools-extra] r358103 - clangd: fix the build with XPC

[Saleem Abdulrasool via cfe-commits]

Author: compnerd
Date: Wed Apr 10 09:48:52 2019
New Revision: 358103

URL: http://llvm.org/viewvc/llvm-project?rev=358103&view=rev
Log:
clangd: fix the build with XPC

`Transport.h` does not include `Features.inc`.  However, since it is used in a
subdirectory, it cannot directly include the header as it is not available.
Include `Features.inc` in `ClangdLSPServer.h` prior to the inclusion of
`Transport.h` which will provide the interfaces in `ClangdMain.cpp` where the
symbol `newXPCTransport` will not be defined due to it being preprocessed away
since the configuration is not passed along to the initial inclusion.

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

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=358103&r1=358102&r2=358103&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Wed Apr 10 09:48:52 2019
@@ -11,6 +11,7 @@
 
 #include "ClangdServer.h"
 #include "DraftStore.h"
+#include "Features.inc"
 #include "FindSymbols.h"
 #include "GlobalCompilationDatabase.h"
 #include "Path.h"


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

[PATCH] D58920: [Modules][PR39287] Consolidate multiple std's

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg added a comment.

Sorry for the delay.  Just catching up on the code this covers, so apologies if 
the questions don't make sense.




Comment at: lib/Sema/SemaDeclCXX.cpp:9214-9215
 getStdNamespace()->setImplicit(true);
+if (getLangOpts().Modules)
+  getStdNamespace()->setHasExternalVisibleStorage(true);
   }

I think you mentioned in another forum that one side effect of this change is 
that it's causing multiple candidates for names in `std`.  Is this the implicit 
align constructros/destructors?  Is this because we're marking the implicit 
namespace as being externally visible?



Comment at: lib/Serialization/ASTReader.cpp:9291-9295
 // Load pending declaration chains.
 for (unsigned I = 0; I != PendingDeclChains.size(); ++I)
   loadPendingDeclChain(PendingDeclChains[I].first,
PendingDeclChains[I].second);
 PendingDeclChains.clear();

How does modules normally "connect up" definitions of the same namspace 
occurring in the imported module?  Is it done here (e.g. so that a name lookup 
will search all namespace definitions)?  Is an alternate solution to this diff 
to make sure this handles the `std` implicit special case?  Apologies for the 
naivety here -- still getting up to speed on this code.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58920



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

r358104 - Don't emit an unreachable return block.

[John McCall via cfe-commits]

Author: rjmccall
Date: Wed Apr 10 10:03:09 2019
New Revision: 358104

URL: http://llvm.org/viewvc/llvm-project?rev=358104&view=rev
Log:
Don't emit an unreachable return block.

Patch by Brad Moody.

Added:
cfe/trunk/test/CodeGen/unreachable-ret.c
Modified:
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=358104&r1=358103&r2=358104&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Wed Apr 10 10:03:09 2019
@@ -2873,15 +2873,6 @@ void CodeGenFunction::EmitFunctionEpilog
 RV = SI->getValueOperand();
 SI->eraseFromParent();
 
-// If that was the only use of the return value, nuke it as well now.
-auto returnValueInst = ReturnValue.getPointer();
-if (returnValueInst->use_empty()) {
-  if (auto alloca = dyn_cast(returnValueInst)) {
-alloca->eraseFromParent();
-ReturnValue = Address::invalid();
-  }
-}
-
   // Otherwise, we have to do a simple load.
   } else {
 RV = Builder.CreateLoad(ReturnValue);

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=358104&r1=358103&r2=358104&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Apr 10 10:03:09 2019
@@ -255,6 +255,7 @@ llvm::DebugLoc CodeGenFunction::EmitRetu
 if (CurBB->empty() || ReturnBlock.getBlock()->use_empty()) {
   ReturnBlock.getBlock()->replaceAllUsesWith(CurBB);
   delete ReturnBlock.getBlock();
+  ReturnBlock = JumpDest();
 } else
   EmitBlock(ReturnBlock.getBlock());
 return llvm::DebugLoc();
@@ -274,6 +275,7 @@ llvm::DebugLoc CodeGenFunction::EmitRetu
   Builder.SetInsertPoint(BI->getParent());
   BI->eraseFromParent();
   delete ReturnBlock.getBlock();
+  ReturnBlock = JumpDest();
   return Loc;
 }
   }
@@ -448,6 +450,19 @@ void CodeGenFunction::FinishFunction(Sou
   // 5. Width of vector aguments and return types for functions called by this
   //function.
   CurFn->addFnAttr("min-legal-vector-width", llvm::utostr(LargestVectorWidth));
+
+  // If we generated an unreachable return block, delete it now.
+  if (ReturnBlock.isValid() && ReturnBlock.getBlock()->use_empty()) {
+Builder.ClearInsertionPoint();
+ReturnBlock.getBlock()->eraseFromParent();
+  }
+  if (ReturnValue.isValid()) {
+auto *RetAlloca = dyn_cast(ReturnValue.getPointer());
+if (RetAlloca && RetAlloca->use_empty()) {
+  RetAlloca->eraseFromParent();
+  ReturnValue = Address::invalid();
+}
+  }
 }
 
 /// ShouldInstrumentFunction - Return true if the current function should be

Added: cfe/trunk/test/CodeGen/unreachable-ret.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/unreachable-ret.c?rev=358104&view=auto
==
--- cfe/trunk/test/CodeGen/unreachable-ret.c (added)
+++ cfe/trunk/test/CodeGen/unreachable-ret.c Wed Apr 10 10:03:09 2019
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
+
+extern void abort() __attribute__((noreturn));
+
+void f1() {
+  abort();
+}
+// CHECK-LABEL: define void @f1()
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   call void @abort()
+// CHECK-NEXT:   unreachable
+// CHECK-NEXT: }
+
+void *f2() {
+  abort();
+  return 0;
+}
+// CHECK-LABEL: define i8* @f2()
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   call void @abort()
+// CHECK-NEXT:   unreachable
+// CHECK-NEXT: }
+

Modified: cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp?rev=358104&r1=358103&r2=358104&view=diff
==
--- cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp Wed Apr 10 10:03:09 
2019
@@ -622,7 +622,7 @@ int main() {
 
 // CHECK-NOT: call i32 @__kmpc_reduce
 
-// CHECK: ret void
+// CHECK: }
 
 // CHECK: define {{.*}} i{{[0-9]+}} [[TMAIN_INT]]()
 // CHECK: [[TEST:%.+]] = alloca [[S_INT_TY]],


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

[PATCH] D57072: Don't codegen an unreachable return block

[John McCall via Phabricator via cfe-commits]

rjmccall commandeered this revision.
rjmccall edited reviewers, added: bmoody; removed: rjmccall.
rjmccall added a comment.
This revision now requires review to proceed.

Committed as r358104, thanks for your patience.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57072



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

[PATCH] D59555: [analyzer] Add yaml parser to GenericTaintChecker

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

Szelethus added a comment.

We agreed to disagree on the static function stuff -- it doesn't really matter, 
and I don't insist. I have no objections against this patch, great job! I won't 
formally accept to make it stand out a little more. Thanks!


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

https://reviews.llvm.org/D59555



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

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:20
+void UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
+
+  // We don't care about deleted, default or implicit operator implementations.

Unnecessary empty line.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:76
+const MatchFinder::MatchResult &Result) {
+
+  const auto *MatchedDecl =

Unnecessary empty line.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst:6
+
+Finds user-defined copy assignment operators which do not protect the code
+against self-assignment either by checking self-assignment explicitly or

Please synchronize this statement with Release Notes.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst:10-12
+This check corresponds to the CERT C++ Coding Standard rule
+`OOP54-CPP. Gracefully handle self-copy assignment
+`_.

alexfh wrote:
> JonasToth wrote:
> > It is worth an alias into the cert module. Please add this with this 
> > revision already.
> Please add a corresponding alias into the cert module.
See also other aliased checks for documentation examples.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst:41
+
+
+There are two common C++ patterns to avoid this problem. The first is

Unnecessary empty line.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst:66
+
+
+The second one is the copy-and-swap method when we create a temporary copy

Unnecessary empty line.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60507



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

[PATCH] D59988: [PR41276] Generate address space cast of 'this' for objects attributed by an address space in C++

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: test/CodeGenCXX/address-space-of-this.cpp:9
+//CHECK: call void @_ZN6MyTypeC1Ei(%struct.MyType* addrspacecast 
(%struct.MyType addrspace(10)* @m to %struct.MyType*), i32 123)
+MyType __attribute__((address_space(10))) m = 123;

Sorry I didn't catch this before, but I don't see why this test is expected to 
work.  We can't actually pass a pointer in address space 10 to that constructor.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59988



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

[PATCH] D60409: [clangd] Add -header-insertion=never flag to disable include insertion in code completion

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

Our Mac builders have started failing after this change with the following:

  [3145/3502] Building CXX object 
tools/clang/tools/extra/clangd/tool/CMakeFiles/clangd.dir/ClangdMain.cpp.o
  FAILED: 
tools/clang/tools/extra/clangd/tool/CMakeFiles/clangd.dir/ClangdMain.cpp.o 
  /b/s/w/ir/cache/goma/client/gomacc /b/s/w/ir/k/cipd/bin/clang++  
-DCLANG_VENDOR="\"Fuchsia \"" -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE 
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
-Itools/clang/tools/extra/clangd/tool 
-I/b/s/w/ir/k/llvm-project/clang-tools-extra/clangd/tool 
-I/b/s/w/ir/k/llvm-project/clang/include -Itools/clang/include 
-I/usr/include/libxml2 -Iinclude -I/b/s/w/ir/k/llvm-project/llvm/include 
-I/b/s/w/ir/k/llvm-project/clang-tools-extra/clangd/tool/.. 
-Itools/clang/tools/extra/clangd/tool/.. -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 -fno-common -Woverloaded-virtual 
-Wno-nested-anon-types -O3 -gline-tables-only   -UNDEBUG  -fno-exceptions 
-fno-rtti -MD -MT 
tools/clang/tools/extra/clangd/tool/CMakeFiles/clangd.dir/ClangdMain.cpp.o -MF 
tools/clang/tools/extra/clangd/tool/CMakeFiles/clangd.dir/ClangdMain.cpp.o.d -o 
tools/clang/tools/extra/clangd/tool/CMakeFiles/clangd.dir/ClangdMain.cpp.o -c 
/b/s/w/ir/k/llvm-project/clang-tools-extra/clangd/tool/ClangdMain.cpp
  /b/s/w/ir/k/llvm-project/clang-tools-extra/clangd/tool/ClangdMain.cpp:474:22: 
error: use of undeclared identifier 'newXPCTransport'
  TransportLayer = newXPCTransport();
   ^
  1 error generated.

I don't understand why since this change hasn't touched the failing line.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60409



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

[clang-tools-extra] r358107 - build: add binary dir to the unittests

[Saleem Abdulrasool via cfe-commits]

Author: compnerd
Date: Wed Apr 10 10:25:14 2019
New Revision: 358107

URL: http://llvm.org/viewvc/llvm-project?rev=358107&view=rev
Log:
build: add binary dir to the unittests

Missed part of the change to make clangd build on Darwin.  Fixes the build after
SVN r358103.

Modified:
clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt

Modified: clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt?rev=358107&r1=358106&r2=358107&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt Wed Apr 10 10:25:14 
2019
@@ -4,8 +4,11 @@ set(LLVM_LINK_COMPONENTS
 
 get_filename_component(CLANGD_SOURCE_DIR
   ${CMAKE_CURRENT_SOURCE_DIR}/../../clangd REALPATH)
+get_filename_component(CLANGD_BINARY_DIR
+  ${CMAKE_CURRENT_BINARY_DIR}/../../clangd REALPATH)
 include_directories(
   ${CLANGD_SOURCE_DIR}
+  ${CLANGD_BINARY_DIR}
   )
 
 add_extra_unittest(ClangdTests


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

[PATCH] D59746: [LibTooling] Fix '-h' option

[Don Hinton via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Support/CommandLine.cpp:1187
+// FIXME: This is needed, but not sure why.
+updateArgStr(Opt, NewArg, ChosenSubCommand);
+Opt->setArgStr(NewArg);

Looks like this is a bug in the way sub commands are handled, but I'd like to 
get feedback on how it *should* work.  

The problem is that if an option is added with `cl::sub(*AllSubCommands)` it 
gets added to all registered subcommands, including `TopLevelSubCommand`.  
However, `TopLevelSubCommand` isn't included in `Option::Subs`, so I have to 
update/remove via two commands, one at the parser level for the 
`ChosenSubCommand`, which happens to be `TopLevelSubCommand` in this case, and 
another for the Option itself, which doesn't have `TopLevelSubCommand` in it's 
subs.

Should `CommandLineParser::addOption` specifically exclude `TopLevelSubCommand` 
when it add subs to an Option?  That seems like a bug to me, but I'm not sure I 
grok the reason it was excluded.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



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

[PATCH] D60513: [HIP] Use -mlink-builtin-bitcode to link device library

[Aaron Enye Shi via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D60513



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

r358115 - Fix an off-by-one mistake in IRGen's copy-construction

[John McCall via cfe-commits]

Author: rjmccall
Date: Wed Apr 10 11:07:18 2019
New Revision: 358115

URL: http://llvm.org/viewvc/llvm-project?rev=358115&view=rev
Log:
Fix an off-by-one mistake in IRGen's copy-construction
special cases in the presence of zero-length arrays.

Patch by Joran Bigalet!

Modified:
cfe/trunk/lib/CodeGen/CGClass.cpp
cfe/trunk/test/CodeGenCXX/pod-member-memcpys.cpp

Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=358115&r1=358114&r2=358115&view=diff
==
--- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGClass.cpp Wed Apr 10 11:07:18 2019
@@ -1008,7 +1008,7 @@ namespace {
   if (FOffset < FirstFieldOffset) {
 FirstField = F;
 FirstFieldOffset = FOffset;
-  } else if (FOffset > LastFieldOffset) {
+  } else if (FOffset >= LastFieldOffset) {
 LastField = F;
 LastFieldOffset = FOffset;
   }

Modified: cfe/trunk/test/CodeGenCXX/pod-member-memcpys.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/pod-member-memcpys.cpp?rev=358115&r1=358114&r2=358115&view=diff
==
--- cfe/trunk/test/CodeGenCXX/pod-member-memcpys.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/pod-member-memcpys.cpp Wed Apr 10 11:07:18 2019
@@ -45,6 +45,13 @@ struct ArrayMember {
   int w, x, y, z;
 };
 
+struct ZeroLengthArrayMember {
+NonPOD np;
+int a;
+int b[0];
+int c;
+};
+
 struct VolatileMember {
   int a, b, c, d;
   volatile int v;
@@ -109,6 +116,7 @@ CALL_AO(Basic)
 CALL_AO(PODMember)
 CALL_AO(PODLikeMember)
 CALL_AO(ArrayMember)
+CALL_AO(ZeroLengthArrayMember)
 CALL_AO(VolatileMember)
 CALL_AO(BitfieldMember)
 CALL_AO(InnerClassMember)
@@ -142,6 +150,12 @@ CALL_AO(PackedMembers)
 // CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}} align 4 {{.*}} align 4 
{{.*}}i64 64, i1 {{.*}})
 // CHECK: ret %struct.ArrayMember*
 
+// ZeroLengthArrayMember copy-assignment:
+// CHECK-LABEL: define linkonce_odr dereferenceable({{[0-9]+}}) 
%struct.ZeroLengthArrayMember* 
@_ZN21ZeroLengthArrayMemberaSERKS_(%struct.ZeroLengthArrayMember* %this, 
%struct.ZeroLengthArrayMember* dereferenceable({{[0-9]+}}))
+// CHECK: call dereferenceable({{[0-9]+}}) %struct.NonPOD* @_ZN6NonPODaSERKS_
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}} align 4 {{.*}} align 4 
{{.*}}i64 8, i1 {{.*}})
+// CHECK: ret %struct.ZeroLengthArrayMember*
+
 // VolatileMember copy-assignment:
 // CHECK-LABEL: define linkonce_odr dereferenceable({{[0-9]+}}) 
%struct.VolatileMember* @_ZN14VolatileMemberaSERKS_(%struct.VolatileMember* 
%this, %struct.VolatileMember* dereferenceable({{[0-9]+}}))
 // CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}} align 4 {{.*}} align 4 
{{.*}}i64 16, i1 {{.*}})


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

[PATCH] D58016: fixes copy constructor generation of classes containing 0-length arrays followed by exactly 1 trivial field (fixes #40682)

[John McCall via Phabricator via cfe-commits]

rjmccall closed this revision.
rjmccall added a comment.

Committed as r358115.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58016



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

[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 194549.
ymandel marked 13 inline comments as done.
ymandel added a comment.

Responded to comments and added wrapper for Stencil::cat in stencil namespace.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59371

Files:
  clang/include/clang/Tooling/Refactoring/Stencil.h
  clang/lib/Tooling/Refactoring/CMakeLists.txt
  clang/lib/Tooling/Refactoring/Stencil.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/StencilTest.cpp

Index: clang/unittests/Tooling/StencilTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -0,0 +1,222 @@
+//===- unittest/Tooling/StencilTest.cpp ---===//
+//
+// 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 "clang/Tooling/Refactoring/Stencil.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/FixIt.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace tooling;
+using namespace ast_matchers;
+
+namespace {
+using ::testing::AllOf;
+using ::testing::Eq;
+using ::testing::HasSubstr;
+using MatchResult = MatchFinder::MatchResult;
+using tooling::stencil::node;
+using tooling::stencil::sNode;
+using tooling::stencil::text;
+
+// In tests, we can't directly match on llvm::Expected since its accessors
+// mutate the object. So, we collapse it to an Optional.
+static llvm::Optional toOptional(llvm::Expected V) {
+  if (V)
+return *V;
+  ADD_FAILURE() << "Losing error in conversion to IsSomething: "
+<< llvm::toString(V.takeError());
+  return llvm::None;
+}
+
+// A very simple matcher for llvm::Optional values.
+MATCHER_P(IsSomething, ValueMatcher, "") {
+  if (!arg)
+return false;
+  return ::testing::ExplainMatchResult(ValueMatcher, *arg, result_listener);
+}
+
+// Create a valid translation-unit from a statement.
+static std::string wrapSnippet(llvm::Twine StatementCode) {
+  return ("auto stencil_test_snippet = []{" + StatementCode + "};").str();
+}
+
+static DeclarationMatcher wrapMatcher(const StatementMatcher &Matcher) {
+  return varDecl(hasName("stencil_test_snippet"),
+ hasDescendant(compoundStmt(hasAnySubstatement(Matcher;
+}
+
+struct TestMatch {
+  // The AST unit from which `result` is built. We bundle it because it backs
+  // the result. Users are not expected to access it.
+  std::unique_ptr AstUnit;
+  // The result to use in the test. References `ast_unit`.
+  MatchResult Result;
+};
+
+// Matches `Matcher` against the statement `StatementCode` and returns the
+// result. Handles putting the statement inside a function and modifying the
+// matcher correspondingly. `Matcher` should match `StatementCode` exactly --
+// that is, produce exactly one match.
+static llvm::Optional matchStmt(llvm::Twine StatementCode,
+   StatementMatcher Matcher) {
+  auto AstUnit = buildASTFromCode(wrapSnippet(StatementCode));
+  if (AstUnit == nullptr) {
+ADD_FAILURE() << "AST construction failed";
+return llvm::None;
+  }
+  ASTContext &Context = AstUnit->getASTContext();
+  auto Matches = ast_matchers::match(wrapMatcher(Matcher), Context);
+  // We expect a single, exact match for the statement.
+  if (Matches.size() != 1) {
+ADD_FAILURE() << "Wrong number of matches: " << Matches.size();
+return llvm::None;
+  }
+  return TestMatch{std::move(AstUnit), MatchResult(Matches[0], &Context)};
+}
+
+class StencilTest : public ::testing::Test {
+protected:
+  // Verifies that the given stencil fails when evaluated on a valid match
+  // result. Binds a statement to "stmt", a (non-member) ctor-initializer to
+  // "init", an expression to "expr" and a (nameless) declaration to "decl".
+  void testError(const Stencil &Stencil,
+ ::testing::Matcher Matcher) {
+const std::string Snippet = R"cc(
+  struct A {};
+  class F : public A {
+   public:
+F(int) {}
+  };
+  F(1);
+)cc";
+auto StmtMatch = matchStmt(
+Snippet,
+stmt(hasDescendant(
+ cxxConstructExpr(
+ hasDeclaration(decl(hasDescendant(cxxCtorInitializer(
+   isBaseInitializer())
+   .bind("init")))
+.bind("decl")))
+ .bind("expr")))
+.bind("stmt"));
+ASSERT_TRUE(StmtMatch);
+if (auto ResultOrErr = Stencil.eval(StmtMatch->Result)) {
+  ADD_FAILURE() <<

[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

deleted some stray comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59371

Files:
  clang/include/clang/Tooling/Refactoring/Stencil.h
  clang/lib/Tooling/Refactoring/CMakeLists.txt
  clang/lib/Tooling/Refactoring/Stencil.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/StencilTest.cpp

Index: clang/unittests/Tooling/StencilTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -0,0 +1,222 @@
+//===- unittest/Tooling/StencilTest.cpp ---===//
+//
+// 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 "clang/Tooling/Refactoring/Stencil.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/FixIt.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace tooling;
+using namespace ast_matchers;
+
+namespace {
+using ::testing::AllOf;
+using ::testing::Eq;
+using ::testing::HasSubstr;
+using MatchResult = MatchFinder::MatchResult;
+using tooling::stencil::node;
+using tooling::stencil::sNode;
+using tooling::stencil::text;
+
+// In tests, we can't directly match on llvm::Expected since its accessors
+// mutate the object. So, we collapse it to an Optional.
+static llvm::Optional toOptional(llvm::Expected V) {
+  if (V)
+return *V;
+  ADD_FAILURE() << "Losing error in conversion to IsSomething: "
+<< llvm::toString(V.takeError());
+  return llvm::None;
+}
+
+// A very simple matcher for llvm::Optional values.
+MATCHER_P(IsSomething, ValueMatcher, "") {
+  if (!arg)
+return false;
+  return ::testing::ExplainMatchResult(ValueMatcher, *arg, result_listener);
+}
+
+// Create a valid translation-unit from a statement.
+static std::string wrapSnippet(llvm::Twine StatementCode) {
+  return ("auto stencil_test_snippet = []{" + StatementCode + "};").str();
+}
+
+static DeclarationMatcher wrapMatcher(const StatementMatcher &Matcher) {
+  return varDecl(hasName("stencil_test_snippet"),
+ hasDescendant(compoundStmt(hasAnySubstatement(Matcher;
+}
+
+struct TestMatch {
+  // The AST unit from which `result` is built. We bundle it because it backs
+  // the result. Users are not expected to access it.
+  std::unique_ptr AstUnit;
+  // The result to use in the test. References `ast_unit`.
+  MatchResult Result;
+};
+
+// Matches `Matcher` against the statement `StatementCode` and returns the
+// result. Handles putting the statement inside a function and modifying the
+// matcher correspondingly. `Matcher` should match `StatementCode` exactly --
+// that is, produce exactly one match.
+static llvm::Optional matchStmt(llvm::Twine StatementCode,
+   StatementMatcher Matcher) {
+  auto AstUnit = buildASTFromCode(wrapSnippet(StatementCode));
+  if (AstUnit == nullptr) {
+ADD_FAILURE() << "AST construction failed";
+return llvm::None;
+  }
+  ASTContext &Context = AstUnit->getASTContext();
+  auto Matches = ast_matchers::match(wrapMatcher(Matcher), Context);
+  // We expect a single, exact match for the statement.
+  if (Matches.size() != 1) {
+ADD_FAILURE() << "Wrong number of matches: " << Matches.size();
+return llvm::None;
+  }
+  return TestMatch{std::move(AstUnit), MatchResult(Matches[0], &Context)};
+}
+
+class StencilTest : public ::testing::Test {
+protected:
+  // Verifies that the given stencil fails when evaluated on a valid match
+  // result. Binds a statement to "stmt", a (non-member) ctor-initializer to
+  // "init", an expression to "expr" and a (nameless) declaration to "decl".
+  void testError(const Stencil &Stencil,
+ ::testing::Matcher Matcher) {
+const std::string Snippet = R"cc(
+  struct A {};
+  class F : public A {
+   public:
+F(int) {}
+  };
+  F(1);
+)cc";
+auto StmtMatch = matchStmt(
+Snippet,
+stmt(hasDescendant(
+ cxxConstructExpr(
+ hasDeclaration(decl(hasDescendant(cxxCtorInitializer(
+   isBaseInitializer())
+   .bind("init")))
+.bind("decl")))
+ .bind("expr")))
+.bind("stmt"));
+ASSERT_TRUE(StmtMatch);
+if (auto ResultOrErr = Stencil.eval(StmtMatch->Result)) {
+  ADD_FAILURE() << "Expected failure but succeeded: " << *ResultOrErr;
+} else {
+  auto Err = llvm::ha

[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

tweaks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59371

Files:
  clang/include/clang/Tooling/Refactoring/Stencil.h
  clang/lib/Tooling/Refactoring/CMakeLists.txt
  clang/lib/Tooling/Refactoring/Stencil.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/StencilTest.cpp

Index: clang/unittests/Tooling/StencilTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -0,0 +1,222 @@
+//===- unittest/Tooling/StencilTest.cpp ---===//
+//
+// 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 "clang/Tooling/Refactoring/Stencil.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/FixIt.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace tooling;
+using namespace ast_matchers;
+
+namespace {
+using ::testing::AllOf;
+using ::testing::Eq;
+using ::testing::HasSubstr;
+using MatchResult = MatchFinder::MatchResult;
+using tooling::stencil::node;
+using tooling::stencil::sNode;
+using tooling::stencil::text;
+
+// In tests, we can't directly match on llvm::Expected since its accessors
+// mutate the object. So, we collapse it to an Optional.
+static llvm::Optional toOptional(llvm::Expected V) {
+  if (V)
+return *V;
+  ADD_FAILURE() << "Losing error in conversion to IsSomething: "
+<< llvm::toString(V.takeError());
+  return llvm::None;
+}
+
+// A very simple matcher for llvm::Optional values.
+MATCHER_P(IsSomething, ValueMatcher, "") {
+  if (!arg)
+return false;
+  return ::testing::ExplainMatchResult(ValueMatcher, *arg, result_listener);
+}
+
+// Create a valid translation-unit from a statement.
+static std::string wrapSnippet(llvm::Twine StatementCode) {
+  return ("auto stencil_test_snippet = []{" + StatementCode + "};").str();
+}
+
+static DeclarationMatcher wrapMatcher(const StatementMatcher &Matcher) {
+  return varDecl(hasName("stencil_test_snippet"),
+ hasDescendant(compoundStmt(hasAnySubstatement(Matcher;
+}
+
+struct TestMatch {
+  // The AST unit from which `result` is built. We bundle it because it backs
+  // the result. Users are not expected to access it.
+  std::unique_ptr AstUnit;
+  // The result to use in the test. References `ast_unit`.
+  MatchResult Result;
+};
+
+// Matches `Matcher` against the statement `StatementCode` and returns the
+// result. Handles putting the statement inside a function and modifying the
+// matcher correspondingly. `Matcher` should match `StatementCode` exactly --
+// that is, produce exactly one match.
+static llvm::Optional matchStmt(llvm::Twine StatementCode,
+   StatementMatcher Matcher) {
+  auto AstUnit = buildASTFromCode(wrapSnippet(StatementCode));
+  if (AstUnit == nullptr) {
+ADD_FAILURE() << "AST construction failed";
+return llvm::None;
+  }
+  ASTContext &Context = AstUnit->getASTContext();
+  auto Matches = ast_matchers::match(wrapMatcher(Matcher), Context);
+  // We expect a single, exact match for the statement.
+  if (Matches.size() != 1) {
+ADD_FAILURE() << "Wrong number of matches: " << Matches.size();
+return llvm::None;
+  }
+  return TestMatch{std::move(AstUnit), MatchResult(Matches[0], &Context)};
+}
+
+class StencilTest : public ::testing::Test {
+protected:
+  // Verifies that the given stencil fails when evaluated on a valid match
+  // result. Binds a statement to "stmt", a (non-member) ctor-initializer to
+  // "init", an expression to "expr" and a (nameless) declaration to "decl".
+  void testError(const Stencil &Stencil,
+ ::testing::Matcher Matcher) {
+const std::string Snippet = R"cc(
+  struct A {};
+  class F : public A {
+   public:
+F(int) {}
+  };
+  F(1);
+)cc";
+auto StmtMatch = matchStmt(
+Snippet,
+stmt(hasDescendant(
+ cxxConstructExpr(
+ hasDeclaration(decl(hasDescendant(cxxCtorInitializer(
+   isBaseInitializer())
+   .bind("init")))
+.bind("decl")))
+ .bind("expr")))
+.bind("stmt"));
+ASSERT_TRUE(StmtMatch);
+if (auto ResultOrErr = Stencil.eval(StmtMatch->Result)) {
+  ADD_FAILURE() << "Expected failure but succeeded: " << *ResultOrErr;
+} else {
+  auto Err = llvm::handleErrors(ResultOrEr

[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

Thanks for the review. I've changed most of the things with only a few open 
questions. PTAL.




Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:45
+  /// Evaluates this part to a string and appends it to `result`.
+  virtual llvm::Error eval(const ast_matchers::MatchFinder::MatchResult &Match,
+   std::string *Result) const = 0;

sbenza wrote:
> Why not use `llvm::Expected` as the return type?
so that each stencil part can append to a single string that we construct when 
evaluating the whole stencil.  this was an (attempted?) optimization. if 
concatenating is about as efficient, I'd prefer that approach. What do you 
advise?



Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:75
+  // Copy constructor/assignment produce a deep copy.
+  StencilPart(const StencilPart &P) : Impl(P.Impl->clone()) {}
+  StencilPart(StencilPart &&) = default;

sbenza wrote:
> The interface API is all const. Why not keep a `shared_ptr` instead?
> That way we don't have to have users implement a clone function.
Excellent! I guess I'm allergic to shared_ptr because of its atomics but its 
clearly a win here. Thanks!



Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:127
+  // Allow Stencils to operate as std::function, for compatibility with
+  // Transformer's TextGenerator. Calls `eval()` and asserts on failure.
+  std::string operator()(const ast_matchers::MatchFinder::MatchResult &) const;

sbenza wrote:
> "asserts" as it only fails in debug mode?
I thought `assert()` also fails in normal mode, based on my reading of the llvm 
docs, but it's not explicit about this: 
http://llvm.org/docs/ProgrammersManual.html#programmatic-errors

FWIW, I'm on the fence whether `eval()` should actually have the same signature 
and similarly just crash the program on errors. Its not clear there's any value 
to propogating the errors up the stack via `Expected`.



Comment at: clang/lib/Tooling/Refactoring/Stencil.cpp:45
+
+namespace {
+// An arbitrary fragment of code within a stencil.

sbenza wrote:
> maybe this anon namespace should cover the functions above?
the style guide advises against. I've been told to only put type definitions 
inside anon namespaces.



Comment at: clang/lib/Tooling/Refactoring/Stencil.cpp:76
+
+static bool operator==(const RawTextData &A, const RawTextData &B) {
+  return A.Text == B.Text;

sbenza wrote:
> If you define ==, imo you should define != too.
> Otherwise just call it `isEqual`
Since I don't have a general need for these as operators, just went w/ the 
latter. Used the same style as `evalData()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59371



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

[PATCH] D60523: [clang] Bugfixe for 41400

[Gauthier via Phabricator via cfe-commits]

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

this is a bugfixe for bugtracker 

added nullptr check at the relevent place and test


Repository:
  rC Clang

https://reviews.llvm.org/D60523

Files:
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/using-decl-1.cpp


Index: clang/test/SemaCXX/using-decl-1.cpp
===
--- clang/test/SemaCXX/using-decl-1.cpp
+++ clang/test/SemaCXX/using-decl-1.cpp
@@ -396,3 +396,10 @@
   using N::Y;
   using N::Z;
 }
+
+// expected-error@+5 {{requires a qualified name}}
+// expected-error@+4 {{expected ';'}}
+// expected-error@+3 {{expected '}'}}
+// expected-note@+2 {{to match this '{'}}
+// expected-error@+1 {{expected ';'}}
+template struct S { using S
\ No newline at end of file
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -90,7 +90,7 @@
   // When naming a constructor as a member of a dependent context (eg, in a
   // friend declaration or an inherited constructor declaration), form an
   // unresolved "typename" type.
-  if (CurClass->isDependentContext() && !EnteringContext) {
+  if (CurClass->isDependentContext() && !EnteringContext && SS.getScopeRep()) {
 QualType T = Context.getDependentNameType(ETK_None, SS.getScopeRep(), &II);
 return ParsedType::make(T);
   }


Index: clang/test/SemaCXX/using-decl-1.cpp
===
--- clang/test/SemaCXX/using-decl-1.cpp
+++ clang/test/SemaCXX/using-decl-1.cpp
@@ -396,3 +396,10 @@
   using N::Y;
   using N::Z;
 }
+
+// expected-error@+5 {{requires a qualified name}}
+// expected-error@+4 {{expected ';'}}
+// expected-error@+3 {{expected '}'}}
+// expected-note@+2 {{to match this '{'}}
+// expected-error@+1 {{expected ';'}}
+template struct S { using S
\ No newline at end of file
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -90,7 +90,7 @@
   // When naming a constructor as a member of a dependent context (eg, in a
   // friend declaration or an inherited constructor declaration), form an
   // unresolved "typename" type.
-  if (CurClass->isDependentContext() && !EnteringContext) {
+  if (CurClass->isDependentContext() && !EnteringContext && SS.getScopeRep()) {
 QualType T = Context.getDependentNameType(ETK_None, SS.getScopeRep(), &II);
 return ParsedType::make(T);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60263: [clang-format] Preserve include blocks in ObjC Google style

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

Looks good to me.




Comment at: cfe/trunk/lib/Format/Format.cpp:881
+// "Regroup" doesn't work well for ObjC yet (main header heuristic,
+// relationship between ObjC standard library headers and other heades,
+// #imports, etc.)

nit: "headers"


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60263



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

r358126 - [OPENMP]Improve detection of number of teams, threads in target

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Wed Apr 10 12:11:33 2019
New Revision: 358126

URL: http://llvm.org/viewvc/llvm-project?rev=358126&view=rev
Log:
[OPENMP]Improve detection of number of teams, threads in target
regions.

Added more complex analysis for number of teams and number of threads in
the target regions, also merged related common code between CGOpenMPRuntime
and CGOpenMPRuntimeNVPTX classes.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/test/OpenMP/distribute_simd_reduction_codegen.cpp
cfe/trunk/test/OpenMP/target_map_codegen.cpp
cfe/trunk/test/OpenMP/target_simd_codegen.cpp
cfe/trunk/test/OpenMP/target_simd_depend_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_private_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_reduction_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_simd_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_simd_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_simd_private_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_simd_reduction_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=358126&r1=358125&r2=358126&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Wed Apr 10 12:11:33 2019
@@ -6475,12 +6475,59 @@ void CGOpenMPRuntime::emitTargetOutlined
   OffloadEntriesInfoManagerTy::OMPTargetRegionEntryTargetRegion);
 }
 
-/// discard all CompoundStmts intervening between two constructs
-static const Stmt *ignoreCompoundStmts(const Stmt *Body) {
-  while (const auto *CS = dyn_cast_or_null(Body))
-Body = CS->body_front();
+/// Checks if the expression is constant or does not have non-trivial function
+/// calls.
+static bool isTrivial(ASTContext &Ctx, const Expr * E) {
+  // We can skip constant expressions.
+  // We can skip expressions with trivial calls or simple expressions.
+  return (E->isEvaluatable(Ctx, Expr::SE_AllowUndefinedBehavior) ||
+  !E->hasNonTrivialCall(Ctx)) &&
+ !E->HasSideEffects(Ctx, /*IncludePossibleEffects=*/true);
+}
 
-  return Body;
+const Stmt *CGOpenMPRuntime::getSingleCompoundChild(ASTContext &Ctx,
+const Stmt *Body) {
+  const Stmt *Child = Body->IgnoreContainers();
+  while (const auto *C = dyn_cast_or_null(Child)) {
+Child = nullptr;
+for (const Stmt *S : C->body()) {
+  if (const auto *E = dyn_cast(S)) {
+if (isTrivial(Ctx, E))
+  continue;
+  }
+  // Some of the statements can be ignored.
+  if (isa(S) || isa(S) || isa(S) ||
+  isa(S) || isa(S))
+continue;
+  // Analyze declarations.
+  if (const auto *DS = dyn_cast(S)) {
+if (llvm::all_of(DS->decls(), [&Ctx](const Decl *D) {
+  if (isa(D) || isa(D) ||
+  isa(D) || isa(D) ||
+  isa(D) || isa(D) ||
+  isa(D) ||
+  isa(D) ||
+  isa(D) || isa(D))
+return true;
+  const auto *VD = dyn_cast(D);
+  if (!VD)
+return false;
+  return VD->isConstexpr() ||
+ ((VD->getType().isTrivialType(Ctx) ||
+   VD->getType()->isReferenceType()) &&
+  (!VD->hasInit() || isTrivial(Ctx, VD->getInit(;
+}))
+  continue;
+  }
+  // Found multiple children - cannot get the one child only.
+  if (Child)
+return nullptr;
+  Child = S;
+}
+if (Child)
+  Child = Child->IgnoreContainers();
+  }
+  return Child;
 }
 
 /// Emit the number of teams for a target directive.  Inspect the num_teams
@@ -6492,63 +6539,163 @@ static const Stmt *ignoreCompoundStmts(c
 ///
 /// Otherwise, return nullptr.
 static llvm::Value *
-emitNumTeamsForTargetDirective(CGOpenMPRuntime &OMPRuntime,
-   CodeGenFunction &CGF,
+emitNumTeamsForTargetDirective(CodeGenFunction &CGF,
const OMPExecutableDirective &D) {
-  assert(!CGF.getLangOpts().OpenMPIsDevice && "Clauses associated with the "
-  "teams directive expected to be "
-  "emitted only for the host!");
-
+  assert(!CGF.getLangOpts().OpenMPIsDevice &&
+ "Clauses associated with the teams directive expected to be emitted "
+ "only for the host!");
+  OpenMPDirectiveKind Di

r358125 - Fix for different build configurations.

[John McCall via cfe-commits]

Author: rjmccall
Date: Wed Apr 10 12:11:32 2019
New Revision: 358125

URL: http://llvm.org/viewvc/llvm-project?rev=358125&view=rev
Log:
Fix for different build configurations.

Modified:
cfe/trunk/test/CodeGen/unreachable-ret.c

Modified: cfe/trunk/test/CodeGen/unreachable-ret.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/unreachable-ret.c?rev=358125&r1=358124&r2=358125&view=diff
==
--- cfe/trunk/test/CodeGen/unreachable-ret.c (original)
+++ cfe/trunk/test/CodeGen/unreachable-ret.c Wed Apr 10 12:11:32 2019
@@ -5,7 +5,7 @@ extern void abort() __attribute__((noret
 void f1() {
   abort();
 }
-// CHECK-LABEL: define void @f1()
+// CHECK-LABEL: define {{.*}}void @f1()
 // CHECK-NEXT: entry:
 // CHECK-NEXT:   call void @abort()
 // CHECK-NEXT:   unreachable
@@ -15,7 +15,7 @@ void *f2() {
   abort();
   return 0;
 }
-// CHECK-LABEL: define i8* @f2()
+// CHECK-LABEL: define {{.*}}i8* @f2()
 // CHECK-NEXT: entry:
 // CHECK-NEXT:   call void @abort()
 // CHECK-NEXT:   unreachable


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

[clang-tools-extra] r358127 - clangd-fuzzer: repair the build

[Saleem Abdulrasool via cfe-commits]

Author: compnerd
Date: Wed Apr 10 12:16:14 2019
New Revision: 358127

URL: http://llvm.org/viewvc/llvm-project?rev=358127&view=rev
Log:
clangd-fuzzer: repair the build

The inclusion of private headers of clangd percolates into the fuzzer.

Modified:
clang-tools-extra/trunk/clangd/fuzzer/CMakeLists.txt

Modified: clang-tools-extra/trunk/clangd/fuzzer/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/fuzzer/CMakeLists.txt?rev=358127&r1=358126&r2=358127&view=diff
==
--- clang-tools-extra/trunk/clangd/fuzzer/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/fuzzer/CMakeLists.txt Wed Apr 10 12:16:14 
2019
@@ -1,4 +1,5 @@
-include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..
+  ${CMAKE_CURRENT_BINARY_DIR}/..)
 
 set(LLVM_LINK_COMPONENTS
   FuzzMutate


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

RE: [clang-tools-extra] r358103 - clangd: fix the build with XPC

[via cfe-commits]

Hi Saleem,

This fix does not seem to work on the PS4 linux build bot. Can you take a look?

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/46575

FAILED: 
tools/clang/tools/extra/clangd/fuzzer/CMakeFiles/clangd-fuzzer.dir/clangd-fuzzer.cpp.o
 
/usr/bin/clang++  -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE 
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
-Itools/clang/tools/extra/clangd/fuzzer 
-I/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/clangd/fuzzer
 
-I/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/include
 -Itools/clang/include -Iinclude 
-I/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/include
 
-I/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/clangd/fuzzer/..
 -std=c++11 -Wdocumentation -Wno-documentation-deprecated-sync -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 
-fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3-UNDEBUG  
-fno-exceptions -fno-rtti -MD -MT 
tools/clang/tools/extra/clangd/fuzzer/CMakeFiles/clangd-fuzzer.dir/clangd-fuzzer.cpp.o
 -MF 
tools/clang/tools/extra/clangd/fuzzer/CMakeFiles/clangd-fuzzer.dir/clangd-fuzzer.cpp.o.d
 -o 
tools/clang/tools/extra/clangd/fuzzer/CMakeFiles/clangd-fuzzer.dir/clangd-fuzzer.cpp.o
 -c 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/clangd/fuzzer/clangd-fuzzer.cpp
In file included from 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/clangd/fuzzer/clangd-fuzzer.cpp:15:
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/clangd/fuzzer/../ClangdLSPServer.h:14:10:
 fatal error: 'Features.inc' file not found
#include "Features.inc"
 ^~

Douglas Yung

-Original Message-
From: cfe-commits  On Behalf Of Saleem 
Abdulrasool via cfe-commits
Sent: Wednesday, April 10, 2019 9:49
To: cfe-commits@lists.llvm.org
Subject: [clang-tools-extra] r358103 - clangd: fix the build with XPC

Author: compnerd
Date: Wed Apr 10 09:48:52 2019
New Revision: 358103

URL: http://llvm.org/viewvc/llvm-project?rev=358103&view=rev
Log:
clangd: fix the build with XPC

`Transport.h` does not include `Features.inc`.  However, since it is used in a 
subdirectory, it cannot directly include the header as it is not available.
Include `Features.inc` in `ClangdLSPServer.h` prior to the inclusion of 
`Transport.h` which will provide the interfaces in `ClangdMain.cpp` where the 
symbol `newXPCTransport` will not be defined due to it being preprocessed away 
since the configuration is not passed along to the initial inclusion.

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

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=358103&r1=358102&r2=358103&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Wed Apr 10 09:48:52 
+++ 2019
@@ -11,6 +11,7 @@
 
 #include "ClangdServer.h"
 #include "DraftStore.h"
+#include "Features.inc"
 #include "FindSymbols.h"
 #include "GlobalCompilationDatabase.h"
 #include "Path.h"


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

[PATCH] D58920: [Modules][PR39287] Consolidate multiple std's

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

rsmith added a comment.

I expect that we get this wrong in the same way for all the `SemaDeclRefs` 
declarations (`StdNamespace`, `StdBadAlloc`, and `StdAlignValT`).

I think the place where this is going wrong is `ASTDeclReader::findExisting`, 
which is assuming that the prior declaration for normal `NamedDecl`s can be 
found by name lookup; that's not true for these particular implicitly-declared 
entities. Perhaps the simplest fix would be to add a check around here:

  } else if (DeclContext *MergeDC = getPrimaryContextForMerging(Reader, DC)) {
DeclContext::lookup_result R = MergeDC->noload_lookup(Name);
for (DeclContext::lookup_iterator I = R.begin(), E = R.end(); I != E; ++I) {
  if (NamedDecl *Existing = getDeclForMerging(*I, TypedefNameForLinkage))
if (isSameEntity(Existing, D))
  return FindExistingResult(Reader, D, Existing, AnonymousDeclNumber,
TypedefNameForLinkage);
}
// [HERE]
  }

... to see if `D` is one of our three special entities, and if so to ask `Sema` 
for its current declaration of that entity (without triggering deserialization) 
and if not, set `D` as the declaration of that entity.




Comment at: lib/Sema/SemaDeclCXX.cpp:8915-8919
+  if (II->isStr("std") && PrevNS->isStdNamespace() &&
+  PrevNS != getStdNamespace()) {
+PrevNS = getStdNamespace();
+IsStd = true;
+  }

This doesn't seem right to me. This change appears to affect the case where we 
already have two distinct `std` namespaces and are declaring a third, which is 
too late -- things already went wrong if we reach that situation.



Comment at: lib/Sema/SemaDeclCXX.cpp:9215
+if (getLangOpts().Modules)
+  getStdNamespace()->setHasExternalVisibleStorage(true);
   }

This also doesn't look right: the "has external visible storage" flag should 
only be set by the external source, to indicate that it has lookup results for 
the decl context.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58920



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

r358132 - Add IRGen APIs to fetch ctor/dtor helper functions for non-trivial structs.

[John McCall via cfe-commits]

Author: rjmccall
Date: Wed Apr 10 12:57:20 2019
New Revision: 358132

URL: http://llvm.org/viewvc/llvm-project?rev=358132&view=rev
Log:
Add IRGen APIs to fetch ctor/dtor helper functions for non-trivial structs.

Patch by Tony Allevato!

Modified:
cfe/trunk/include/clang/CodeGen/CodeGenABITypes.h
cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp

Modified: cfe/trunk/include/clang/CodeGen/CodeGenABITypes.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/CodeGen/CodeGenABITypes.h?rev=358132&r1=358131&r2=358132&view=diff
==
--- cfe/trunk/include/clang/CodeGen/CodeGenABITypes.h (original)
+++ cfe/trunk/include/clang/CodeGen/CodeGenABITypes.h Wed Apr 10 12:57:20 2019
@@ -30,6 +30,7 @@
 namespace llvm {
   class DataLayout;
   class Module;
+  class Function;
   class FunctionType;
   class Type;
 }
@@ -83,6 +84,59 @@ llvm::Type *convertTypeForMemory(CodeGen
 unsigned getLLVMFieldNumber(CodeGenModule &CGM,
 const RecordDecl *RD, const FieldDecl *FD);
 
+/// Returns the default constructor for a C struct with non-trivially copyable
+/// fields, generating it if necessary. The returned function uses the `cdecl`
+/// calling convention, returns void, and takes a single argument that is a
+/// pointer to the address of the struct.
+llvm::Function *getNonTrivialCStructDefaultConstructor(CodeGenModule &GCM,
+   CharUnits DstAlignment,
+   bool IsVolatile,
+   QualType QT);
+
+/// Returns the copy constructor for a C struct with non-trivially copyable
+/// fields, generating it if necessary. The returned function uses the `cdecl`
+/// calling convention, returns void, and takes two arguments: pointers to the
+/// addresses of the destination and source structs, respectively.
+llvm::Function *getNonTrivialCStructCopyConstructor(CodeGenModule &CGM,
+CharUnits DstAlignment,
+CharUnits SrcAlignment,
+bool IsVolatile,
+QualType QT);
+
+/// Returns the move constructor for a C struct with non-trivially copyable
+/// fields, generating it if necessary. The returned function uses the `cdecl`
+/// calling convention, returns void, and takes two arguments: pointers to the
+/// addresses of the destination and source structs, respectively.
+llvm::Function *getNonTrivialCStructMoveConstructor(CodeGenModule &CGM,
+CharUnits DstAlignment,
+CharUnits SrcAlignment,
+bool IsVolatile,
+QualType QT);
+
+/// Returns the copy assignment operator for a C struct with non-trivially
+/// copyable fields, generating it if necessary. The returned function uses the
+/// `cdecl` calling convention, returns void, and takes two arguments: pointers
+/// to the addresses of the destination and source structs, respectively.
+llvm::Function *getNonTrivialCStructCopyAssignmentOperator(
+CodeGenModule &CGM, CharUnits DstAlignment, CharUnits SrcAlignment,
+bool IsVolatile, QualType QT);
+
+/// Return the move assignment operator for a C struct with non-trivially
+/// copyable fields, generating it if necessary. The returned function uses the
+/// `cdecl` calling convention, returns void, and takes two arguments: pointers
+/// to the addresses of the destination and source structs, respectively.
+llvm::Function *getNonTrivialCStructMoveAssignmentOperator(
+CodeGenModule &CGM, CharUnits DstAlignment, CharUnits SrcAlignment,
+bool IsVolatile, QualType QT);
+
+/// Returns the destructor for a C struct with non-trivially copyable fields,
+/// generating it if necessary. The returned function uses the `cdecl` calling
+/// convention, returns void, and takes a single argument that is a pointer to
+/// the address of the struct.
+llvm::Function *getNonTrivialCStructDestructor(CodeGenModule &CGM,
+   CharUnits DstAlignment,
+   bool IsVolatile, QualType QT);
+
 }  // end namespace CodeGen
 }  // end namespace clang
 

Modified: cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp?rev=358132&r1=358131&r2=358132&view=diff
==
--- cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp Wed Apr 10 12:57:20 2019
@@ -14,6 +14,7 @@
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h

[PATCH] D60161: Expose non-trivial struct helpers for Swift.

[John McCall via Phabricator via cfe-commits]

rjmccall closed this revision.
rjmccall added a comment.

Committed as r358132.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60161



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

r358133 - [clang][ASTContext] Try to exit early before loading serialized comments from AST files

[Jan Korous via cfe-commits]

Author: jkorous
Date: Wed Apr 10 13:23:33 2019
New Revision: 358133

URL: http://llvm.org/viewvc/llvm-project?rev=358133&view=rev
Log:
[clang][ASTContext] Try to exit early before loading serialized comments from 
AST files

Loading external comments is expensive. This change probably doesn't apply to 
common cases but is almost for free and would save some work in case none of 
the declaration needs external comments to be loaded.

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

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

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=358133&r1=358132&r2=358133&view=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Wed Apr 10 13:23:33 2019
@@ -99,20 +99,13 @@ enum FloatingRank {
 };
 
 RawComment *ASTContext::getRawCommentForDeclNoCache(const Decl *D) const {
-  if (!CommentsLoaded && ExternalSource) {
-ExternalSource->ReadComments();
-
-#ifndef NDEBUG
-ArrayRef RawComments = Comments.getComments();
-assert(std::is_sorted(RawComments.begin(), RawComments.end(),
-  BeforeThanCompare(SourceMgr)));
-#endif
-
-CommentsLoaded = true;
-  }
-
   assert(D);
 
+  // If we already tried to load comments but there are none,
+  // we won't find anything.
+  if (CommentsLoaded && Comments.getComments().empty())
+return nullptr;
+
   // User can not attach documentation to implicit declarations.
   if (D->isImplicit())
 return nullptr;
@@ -162,12 +155,6 @@ RawComment *ASTContext::getRawCommentFor
   isa(D))
 return nullptr;
 
-  ArrayRef RawComments = Comments.getComments();
-
-  // If there are no comments anywhere, we won't find anything.
-  if (RawComments.empty())
-return nullptr;
-
   // Find declaration location.
   // For Objective-C declarations we generally don't expect to have multiple
   // declarators, thus use declaration starting location as the "declaration
@@ -206,6 +193,23 @@ RawComment *ASTContext::getRawCommentFor
   if (DeclLoc.isInvalid() || !DeclLoc.isFileID())
 return nullptr;
 
+  if (!CommentsLoaded && ExternalSource) {
+ExternalSource->ReadComments();
+
+#ifndef NDEBUG
+ArrayRef RawComments = Comments.getComments();
+assert(std::is_sorted(RawComments.begin(), RawComments.end(),
+  BeforeThanCompare(SourceMgr)));
+#endif
+
+CommentsLoaded = true;
+  }
+
+  ArrayRef RawComments = Comments.getComments();
+  // If there are no comments anywhere, we won't find anything.
+  if (RawComments.empty())
+return nullptr;
+
   // Find the comment that occurs just after this declaration.
   ArrayRef::iterator Comment;
   {


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

r358134 - Check i < FD->getNumParams() before querying

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Wed Apr 10 13:25:07 2019
New Revision: 358134

URL: http://llvm.org/viewvc/llvm-project?rev=358134&view=rev
Log:
Check i < FD->getNumParams() before querying

Summary:
As was already stated in a previous comment, the parameter isn't
necessarily referring to one of the DeclContext's parameter. We
should check the index is within the range to avoid out-of-boundary
access.

Reviewers: gribozavr, rsmith, lebedev.ri

Reviewed By: gribozavr, rsmith

Subscribers: lebedev.ri, cfe-commits

Tags: #clang

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

Patch by Violet.

Added:
cfe/trunk/test/SemaCXX/PR41139.cpp
Modified:
cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
cfe/trunk/test/SemaCXX/cxx1y-generic-lambdas.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp?rev=358134&r1=358133&r2=358134&view=diff
==
--- cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp Wed Apr 10 13:25:07 2019
@@ -2892,7 +2892,7 @@ static const Decl *getCanonicalParmVarDe
   unsigned i = PV->getFunctionScopeIndex();
   // This parameter might be from a freestanding function type within the
   // function and isn't necessarily referring to one of FD's parameters.
-  if (FD->getParamDecl(i) == PV)
+  if (i < FD->getNumParams() && FD->getParamDecl(i) == PV)
 return FD->getCanonicalDecl()->getParamDecl(i);
 }
   }

Added: cfe/trunk/test/SemaCXX/PR41139.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/PR41139.cpp?rev=358134&view=auto
==
--- cfe/trunk/test/SemaCXX/PR41139.cpp (added)
+++ cfe/trunk/test/SemaCXX/PR41139.cpp Wed Apr 10 13:25:07 2019
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++17 -verify %s
+
+// expected-no-diagnostics
+
+// This test should not crash.
+int f1( unsigned ) { return 0; }
+
+template 
+struct S1 {
+S1( R(*f)(Args...) ) {}
+};
+
+int main() {
+S1 s1( f1 );
+}

Modified: cfe/trunk/test/SemaCXX/cxx1y-generic-lambdas.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx1y-generic-lambdas.cpp?rev=358134&r1=358133&r2=358134&view=diff
==
--- cfe/trunk/test/SemaCXX/cxx1y-generic-lambdas.cpp (original)
+++ cfe/trunk/test/SemaCXX/cxx1y-generic-lambdas.cpp Wed Apr 10 13:25:07 2019
@@ -944,6 +944,15 @@ namespace PR22117 {
   }(0)(0);
 }
 
+namespace PR41139 {
+  int y = [](auto outer) {
+return [](auto inner) {
+  using T = int(decltype(outer), decltype(inner));
+  return 0;
+};
+  }(0)(0);
+}
+
 namespace PR23716 {
 template
 auto f(T x) {


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

[PATCH] D60055: Check i < FD->getNumParams() before querying

[Dmitri Gribenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC358134: Check i < FD->getNumParams() before querying 
(authored by gribozavr, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60055?vs=194152&id=194581#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D60055

Files:
  lib/Sema/SemaTemplateInstantiate.cpp
  test/SemaCXX/PR41139.cpp
  test/SemaCXX/cxx1y-generic-lambdas.cpp


Index: lib/Sema/SemaTemplateInstantiate.cpp
===
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -2892,7 +2892,7 @@
   unsigned i = PV->getFunctionScopeIndex();
   // This parameter might be from a freestanding function type within the
   // function and isn't necessarily referring to one of FD's parameters.
-  if (FD->getParamDecl(i) == PV)
+  if (i < FD->getNumParams() && FD->getParamDecl(i) == PV)
 return FD->getCanonicalDecl()->getParamDecl(i);
 }
   }
Index: test/SemaCXX/cxx1y-generic-lambdas.cpp
===
--- test/SemaCXX/cxx1y-generic-lambdas.cpp
+++ test/SemaCXX/cxx1y-generic-lambdas.cpp
@@ -944,6 +944,15 @@
   }(0)(0);
 }
 
+namespace PR41139 {
+  int y = [](auto outer) {
+return [](auto inner) {
+  using T = int(decltype(outer), decltype(inner));
+  return 0;
+};
+  }(0)(0);
+}
+
 namespace PR23716 {
 template
 auto f(T x) {
Index: test/SemaCXX/PR41139.cpp
===
--- test/SemaCXX/PR41139.cpp
+++ test/SemaCXX/PR41139.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++17 -verify %s
+
+// expected-no-diagnostics
+
+// This test should not crash.
+int f1( unsigned ) { return 0; }
+
+template 
+struct S1 {
+S1( R(*f)(Args...) ) {}
+};
+
+int main() {
+S1 s1( f1 );
+}


Index: lib/Sema/SemaTemplateInstantiate.cpp
===
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -2892,7 +2892,7 @@
   unsigned i = PV->getFunctionScopeIndex();
   // This parameter might be from a freestanding function type within the
   // function and isn't necessarily referring to one of FD's parameters.
-  if (FD->getParamDecl(i) == PV)
+  if (i < FD->getNumParams() && FD->getParamDecl(i) == PV)
 return FD->getCanonicalDecl()->getParamDecl(i);
 }
   }
Index: test/SemaCXX/cxx1y-generic-lambdas.cpp
===
--- test/SemaCXX/cxx1y-generic-lambdas.cpp
+++ test/SemaCXX/cxx1y-generic-lambdas.cpp
@@ -944,6 +944,15 @@
   }(0)(0);
 }
 
+namespace PR41139 {
+  int y = [](auto outer) {
+return [](auto inner) {
+  using T = int(decltype(outer), decltype(inner));
+  return 0;
+};
+  }(0)(0);
+}
+
 namespace PR23716 {
 template
 auto f(T x) {
Index: test/SemaCXX/PR41139.cpp
===
--- test/SemaCXX/PR41139.cpp
+++ test/SemaCXX/PR41139.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++17 -verify %s
+
+// expected-no-diagnostics
+
+// This test should not crash.
+int f1( unsigned ) { return 0; }
+
+template 
+struct S1 {
+S1( R(*f)(Args...) ) {}
+};
+
+int main() {
+S1 s1( f1 );
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59923: [Driver] Simplify -g level computation and its interaction with -gsplit-dwarf

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D59923#1460696 , @MaskRay wrote:

> > is that to imply that the first block all do not use split DWARF?
>
> The first block do not use split DWARF.


That doesn't sound like what I'd expect (& would represent a change in behavior 
as well). The first block reads:

-gsplit-dwarf -g0 => 0
-gsplit-dwarf -gline-directives-only => DebugDirectivesOnly
-gsplit-dwarf -gmlt -fsplit-dwarf-inlining => 1
-gsplit-dwarf -gmlt -fno-split-dwarf-inlining => 1

This last one currently produces split-dwarf (if there's any DWARF worth 
splitting - if there are any subprogram descriptions, etc, otherwise it saves 
the indirection and produces an empty .dwo file).

>> In a previous message I think you said that the only change was "-gmlt 
>> -gsplit-dwarf -fno-split-dwarf-inlining => 1 (before) 2 (after)" - which I'm 
>> not sure is an improvement.
> 
> Yes, this is the only behavioral change.
> 
>> You mentioned that the inconsistency between "-g0 -gsplit-dwarf" and "-gmlt 
>> -gsplit-dwarf -fno-split-dwarf-inlining" was confusing. But still there will 
>> be an inconsistency between "-gsplit-dwarf -g0" and "-gsplit-dwarf -gmlt 
>> -fno-split-dwarf-inlining", yes?
> 
> The debug info level will be consistent after this patch:  the last of 
> `-gsplit-dwarf -g0 -g1 -g2 -g3 -ggdb[0-3] -gdwarf-*` will decide the debug 
> info level (`-gsplit-dwarf -gdwarf-*` have level 2). Next, a separate rule 
> decides if the `-gsplit-dwarf` takes effect (not if `DebugInfoKind == 
> codegenoptions::NoDebugInfo || DebugInfoKind == 
> codegenoptions::DebugDirectivesOnly || (DebugInfoKind == 
> codegenoptions::DebugLineTablesOnly && SplitDWARFInlining)`)
> 
>> I think that under -fno-split-dwarf-inlining, -gmlt and -gsplit-dwarf should 
>> be order independent and compositional rather than overriding. Having them 
>> compose in one order but not the other seems confusing to me.
> 
> The existence of `-fno-split-dwarf-inlining` changing the position dependence 
> makes me confused:
> 
> - Without it, the latter of `-gmlt` and `-gsplit-dwarf` decides the debug 
> info level
> - With it, `-gmlt` decides the debug info level

Seems there's going to be confusion either way, though - either the 
presence/absence of -fno-split-dwarf-inlining changes whether -gsplit-dwarf is 
respected/ignored (in the presence of -gmlt), or changes whethe -gmlt composes 
with -gsplit-dwarf or overrides it? Seems these are both a bit confusing, no?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59923



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

r358136 - Fix a test, NFC

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Wed Apr 10 14:18:21 2019
New Revision: 358136

URL: http://llvm.org/viewvc/llvm-project?rev=358136&view=rev
Log:
Fix a test, NFC

This test was duplicated, and the last declaration had some syntax errors since
the invalid attribute caused the @implementation to be skipped by the parser.

Removed:
cfe/trunk/test/CodeGenObjC/objc-asm-attribute-neg-test.m
Modified:
cfe/trunk/test/SemaObjC/objc-asm-attribute-neg-test.m

Removed: cfe/trunk/test/CodeGenObjC/objc-asm-attribute-neg-test.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/objc-asm-attribute-neg-test.m?rev=358135&view=auto
==
--- cfe/trunk/test/CodeGenObjC/objc-asm-attribute-neg-test.m (original)
+++ cfe/trunk/test/CodeGenObjC/objc-asm-attribute-neg-test.m (removed)
@@ -1,34 +0,0 @@
-// RUN: %clang_cc1  -fsyntax-only -verify -Wno-objc-root-class %s
-// rdar://16462586
-
-__attribute__((objc_runtime_name("MySecretNamespace.Protocol")))
-@protocol Protocol
-@end
-
-__attribute__((objc_runtime_name("MySecretNamespace.Message")))
-@interface Message  { 
-__attribute__((objc_runtime_name("MySecretNamespace.Message"))) // 
expected-error {{'objc_runtime_name' attribute only applies to Objective-C 
interfaces and Objective-C protocols}}
-  id MyIVAR;
-}
-__attribute__((objc_runtime_name("MySecretNamespace.Message")))
-@property int MyProperty; // expected-error {{prefix attribute must be 
followed by an interface or protocol
-
-- (int) getMyProperty 
__attribute__((objc_runtime_name("MySecretNamespace.Message"))); // 
expected-error {{'objc_runtime_name' attribute only applies to}}
-
-- (void) setMyProperty : (int) arg 
__attribute__((objc_runtime_name("MySecretNamespace.Message"))); // 
expected-error {{'objc_runtime_name' attribute only applies to}}
-
-@end
-
-__attribute__((objc_runtime_name("MySecretNamespace.ForwardClass")))
-@class ForwardClass; // expected-error {{prefix attribute must be followed by 
an interface or protocol}}
-
-__attribute__((objc_runtime_name("MySecretNamespace.ForwardProtocol")))
-@protocol ForwardProtocol;
-
-__attribute__((objc_runtime_name("MySecretNamespace.Message")))
-@implementation Message // expected-error {{prefix attribute must be followed 
by an interface or protocol}}
-__attribute__((objc_runtime_name("MySecretNamespace.Message")))
-- (id) MyMethod {
-  return MyIVAR;
-}
-@end

Modified: cfe/trunk/test/SemaObjC/objc-asm-attribute-neg-test.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/objc-asm-attribute-neg-test.m?rev=358136&r1=358135&r2=358136&view=diff
==
--- cfe/trunk/test/SemaObjC/objc-asm-attribute-neg-test.m (original)
+++ cfe/trunk/test/SemaObjC/objc-asm-attribute-neg-test.m Wed Apr 10 14:18:21 
2019
@@ -33,10 +33,18 @@ __attribute__((objc_runtime_name("MySecr
 __attribute__((objc_runtime_name("MySecretNamespace.ForwardProtocol")))
 @protocol ForwardProtocol;
 
-__attribute__((objc_runtime_name("MySecretNamespace.Message")))
-@implementation Message // expected-error {{prefix attribute must be followed 
by an interface or protocol}}
-__attribute__((objc_runtime_name("MySecretNamespace.Message")))
-- (id) MyMethod {
+@implementation Message
+// expected-error@+1 {{'objc_runtime_name' attribute only applies to 
Objective-C interfaces and Objective-C protocols}}
+- (id) MyMethod 
__attribute__((objc_runtime_name("MySecretNamespace.Message"))) {
   return MyIVAR;
 }
+
+-(int)getMyProperty { return 0; }
+-(void)setMyProperty:(int)arg {}
 @end
+
+@interface NoImpl @end
+
+__attribute__((objc_runtime_name("MySecretNamespace.Message")))
+// expected-error@+1 {{prefix attribute must be followed by an interface or 
protocol}}
+@implementation NoImpl @end


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

[PATCH] D60539: Add -std=c++14 language standard option to tests that require C++14 default

[Amy Kwan via Phabricator via cfe-commits]

amyk created this revision.
amyk added reviewers: ilya-biryukov, sammccall, ioeric, hokein, akyrtzi, yvvan.
amyk added projects: clang, LLVM.
Herald added subscribers: kadircet, arphaman, dexonsmith, jkorous.

On one of the platforms that we build on, we build with the CMake macro, 
`CLANG_DEFAULT_STD_CXX` to set the default language level when building Clang 
and LLVM.

In our case, we set the default to be `gnucxx11`. However, doing so will cause 
the test cases in this patch to fail as they rely on the C++14 default.

This patch explicitly adds the `-std=c++14` to the affected test cases so they 
will work when the default language level is set.

I have added individuals who have worked with these test cases in the past as 
reviewers. I would greatly appreciate it if any of you can inform me on whether 
or not this change is acceptable.


https://reviews.llvm.org/D60539

Files:
  clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
  clang/test/Index/print-type-size.cpp


Index: clang/test/Index/print-type-size.cpp
===
--- clang/test/Index/print-type-size.cpp
+++ clang/test/Index/print-type-size.cpp
@@ -1,6 +1,6 @@
 // from SemaCXX/class-layout.cpp
-// RUN: c-index-test -test-print-type-size %s -target x86_64-pc-linux-gnu | 
FileCheck -check-prefix=CHECK64 %s
-// RUN: c-index-test -test-print-type-size %s -target i386-apple-darwin9 | 
FileCheck -check-prefix=CHECK32 %s
+// RUN: c-index-test -test-print-type-size %s -target x86_64-pc-linux-gnu 
-std=c++14 | FileCheck -check-prefix=CHECK64 %s
+// RUN: c-index-test -test-print-type-size %s -target i386-apple-darwin9 
-std=c++14 | FileCheck -check-prefix=CHECK32 %s
 
 namespace basic {
 
Index: clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
===
--- clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
+++ clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
@@ -109,6 +109,7 @@
 File.Filename = FileName;
 File.HeaderCode = HeaderCode;
 File.Code = Code;
+File.ExtraArgs.push_back("-std=c++14");
 AST = File.build();
   }
 


Index: clang/test/Index/print-type-size.cpp
===
--- clang/test/Index/print-type-size.cpp
+++ clang/test/Index/print-type-size.cpp
@@ -1,6 +1,6 @@
 // from SemaCXX/class-layout.cpp
-// RUN: c-index-test -test-print-type-size %s -target x86_64-pc-linux-gnu | FileCheck -check-prefix=CHECK64 %s
-// RUN: c-index-test -test-print-type-size %s -target i386-apple-darwin9 | FileCheck -check-prefix=CHECK32 %s
+// RUN: c-index-test -test-print-type-size %s -target x86_64-pc-linux-gnu -std=c++14 | FileCheck -check-prefix=CHECK64 %s
+// RUN: c-index-test -test-print-type-size %s -target i386-apple-darwin9 -std=c++14 | FileCheck -check-prefix=CHECK32 %s
 
 namespace basic {
 
Index: clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
===
--- clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
+++ clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
@@ -109,6 +109,7 @@
 File.Filename = FileName;
 File.HeaderCode = HeaderCode;
 File.Code = Code;
+File.ExtraArgs.push_back("-std=c++14");
 AST = File.build();
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60539: Add -std=c++14 language standard option to tests that require C++14 default

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

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

LGTM

Generally it's a good thing for our test suite to be robust against changes to 
Clang's default language mode.


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

https://reviews.llvm.org/D60539



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

[PATCH] D59673: [Driver] Allow setting the DWO name DWARF attribute separately

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D59673#1460605 , @aaronpuchert 
wrote:

> Ok, here is an idea. We introduce `-split-dwarf-output` in Clang instead of 
> `-fsplit-dwarf-dwo-name-attr`. If given, it overrides the output file name 
> for the Split DWARF file, which we otherwise take from `-split-dwarf-file`. 
> The option is obviously incompatible with `-enable-split-dwarf=single`, so we 
> will disallow that. This should be backwards-compatible, and bring the 
> behavior of `llc` and `clang -cc1` closer together. What do you think?


Sure, I think the naming's a bit weird (but hard to come up with good names for 
any of this) - but consistency seems like a good first pass at least, given 
we're halfway there anyway.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59673



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

[PATCH] D59347: [DebugInfo] Combine Trivial and NonTrivial flags

[David Blaikie via Phabricator via cfe-commits]

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

Approved pending the LLVM side changes/discussion.


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

https://reviews.llvm.org/D59347



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

[PATCH] D60112: [analyzer] Treat write into a top-level parameter variable with destructor as escape.

[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.

Okay, I played around with this patch, I see now where this is going! LGTM!

> Do you think i should document it somehow?

Aye, the description you gave was enlightening, thanks! If you can squeeze it 
somewhere in the code where it isn't out of place, it's all the better! :)




Comment at: clang/test/Analysis/malloc.cpp:151
+  char *getName() {
+if (!name) {
+  name = static_cast(malloc(10));

NoQ wrote:
> Szelethus wrote:
> > Is this relevant? `name` will never be null.
> Not really, just makes the code look a bit more sensible and idiomatic and 
> less warning-worthy-anyway, to make it as clear as possible that the positive 
> here is indeed false. We don't really have a constructor in this class, but 
> we can imagine that it zero-initializes name. Without this check calling 
> `getName()` multiple times would immediately result in a leak.
Convinced ;)


Repository:
  rC Clang

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

https://reviews.llvm.org/D60112



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

[PATCH] D59555: [analyzer] Add yaml parser to GenericTaintChecker

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

Szelethus added a comment.

Sorry, I rushed my earlier comment -- I definitely think that we should get rid 
of the `UINT_MAX` thing before landing this.


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

https://reviews.llvm.org/D59555



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