diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h --- a/clang-tools-extra/clangd/AST.h +++ b/clang-tools-extra/clangd/AST.h @@ -148,6 +148,21 @@ const NamedDecl *ND, llvm::ArrayRef 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/AST.cpp b/clang-tools-extra/clangd/AST.cpp --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -473,5 +473,12 @@ }); } +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(D); + return VD && !VD->getType().isNull() && VD->getType()->isUndeducedType(); +} + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -489,6 +489,9 @@ 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 @@ 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 --- a/clang-tools-extra/clangd/Quality.cpp +++ b/clang-tools-extra/clangd/Quality.cpp @@ -275,8 +275,9 @@ } 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 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -2664,6 +2664,17 @@ 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() {