summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--clang-tools-extra/clangd/AST.cpp7
-rw-r--r--clang-tools-extra/clangd/AST.h15
-rw-r--r--clang-tools-extra/clangd/CodeComplete.cpp13
-rw-r--r--clang-tools-extra/clangd/Quality.cpp5
-rw-r--r--clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp11
5 files changed, 45 insertions, 6 deletions
diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp
index c800ee870dc..836eb6a3645 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -473,5 +473,12 @@ std::string getQualification(ASTContext &Context,
});
}
+bool hasUnstableLinkage(const Decl *D) {
+ // Linkage of a ValueDecl depends on the type.
+ // If that's not deduced yet, deducing it may change the linkage.
+ auto *VD = llvm::dyn_cast_or_null<ValueDecl>(D);
+ return VD && !VD->getType().isNull() && VD->getType()->isUndeducedType();
+}
+
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h
index 6cae719986a..a40aeaf32a7 100644
--- a/clang-tools-extra/clangd/AST.h
+++ b/clang-tools-extra/clangd/AST.h
@@ -148,6 +148,21 @@ std::string getQualification(ASTContext &Context,
const NamedDecl *ND,
llvm::ArrayRef<std::string> VisibleNamespaces);
+/// Whether we must avoid computing linkage for D during code completion.
+/// Clang aggressively caches linkage computation, which is stable after the AST
+/// is built. Unfortunately the AST is incomplete during code completion, so
+/// linkage may still change.
+///
+/// Example: `auto x = []{^}` at file scope.
+/// During code completion, the initializer for x hasn't been parsed yet.
+/// x has type `undeduced auto`, and external linkage.
+/// If we compute linkage at this point, the external linkage will be cached.
+///
+/// After code completion the initializer is attached, and x has a lambda type.
+/// This means x has "unique external" linkage. If we computed linkage above,
+/// the cached value is incorrect. (clang catches this with an assertion).
+bool hasUnstableLinkage(const Decl *D);
+
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp
index 86268b6c25e..275b7cc832d 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -489,6 +489,9 @@ llvm::Optional<SymbolID> getSymbolID(const CodeCompletionResult &R,
switch (R.Kind) {
case CodeCompletionResult::RK_Declaration:
case CodeCompletionResult::RK_Pattern: {
+ // Computing USR caches linkage, which may change after code completion.
+ if (hasUnstableLinkage(R.Declaration))
+ return llvm::None;
return clang::clangd::getSymbolID(R.Declaration);
}
case CodeCompletionResult::RK_Macro:
@@ -1001,10 +1004,12 @@ private:
ScoredSignature Result;
Result.Signature = std::move(Signature);
Result.Quality = Signal;
- Result.IDForDoc =
- Result.Signature.documentation.empty() && Candidate.getFunction()
- ? clangd::getSymbolID(Candidate.getFunction())
- : None;
+ const FunctionDecl *Func = Candidate.getFunction();
+ if (Func && Result.Signature.documentation.empty()) {
+ // Computing USR caches linkage, which may change after code completion.
+ if (!hasUnstableLinkage(Func))
+ Result.IDForDoc = clangd::getSymbolID(Func);
+ }
return Result;
}
diff --git a/clang-tools-extra/clangd/Quality.cpp b/clang-tools-extra/clangd/Quality.cpp
index bd25256904c..d80790fc980 100644
--- a/clang-tools-extra/clangd/Quality.cpp
+++ b/clang-tools-extra/clangd/Quality.cpp
@@ -275,8 +275,9 @@ computeScope(const NamedDecl *D) {
}
if (InClass)
return SymbolRelevanceSignals::ClassScope;
- // This threshold could be tweaked, e.g. to treat module-visible as global.
- if (D->getLinkageInternal() < ExternalLinkage)
+ // ExternalLinkage threshold could be tweaked, e.g. module-visible as global.
+ // Avoid caching linkage if it may change after enclosing code completion.
+ if (hasUnstableLinkage(D) || D->getLinkageInternal() < ExternalLinkage)
return SymbolRelevanceSignals::FileScope;
return SymbolRelevanceSignals::GlobalScope;
}
diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 3e9b97dc3b3..8189f76ba51 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2681,6 +2681,17 @@ TEST(CompletionTest, DerivedMethodsAreAlwaysVisible) {
ElementsAre(AllOf(ReturnType("int"), Named("size"))));
}
+TEST(CompletionTest, NoCrashWithIncompleteLambda) {
+ auto Completions = completions("auto&& x = []{^").Completions;
+ // The completion of x itself can cause a problem: in the code completion
+ // callback, its type is not known, which affects the linkage calculation.
+ // A bad linkage value gets cached, and subsequently updated.
+ EXPECT_THAT(Completions, Contains(Named("x")));
+
+ auto Signatures = signatures("auto x() { x(^").signatures;
+ EXPECT_THAT(Signatures, Contains(Sig("x() -> auto")));
+}
+
TEST(NoCompileCompletionTest, Basic) {
auto Results = completionsNoCompile(R"cpp(
void func() {
OpenPOWER on IntegriCloud