diff --git a/clang-tools-extra/clangd/IncludeFixer.h b/clang-tools-extra/clangd/IncludeFixer.h --- a/clang-tools-extra/clangd/IncludeFixer.h +++ b/clang-tools-extra/clangd/IncludeFixer.h @@ -58,9 +58,7 @@ struct UnresolvedName { std::string Name; // E.g. "X" in foo::X. SourceLocation Loc; // Start location of the unresolved name. - // Lazily get the possible scopes of the unresolved name. `Sema` must be - // alive when this is called. - std::function()> GetScopes; + std::vector Scopes; // Namespace scopes we should search in. }; /// Records the last unresolved name seen by Sema. diff --git a/clang-tools-extra/clangd/IncludeFixer.cpp b/clang-tools-extra/clangd/IncludeFixer.cpp --- a/clang-tools-extra/clangd/IncludeFixer.cpp +++ b/clang-tools-extra/clangd/IncludeFixer.cpp @@ -16,6 +16,7 @@ #include "index/Symbol.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclBase.h" +#include "clang/AST/DeclarationName.h" #include "clang/AST/NestedNameSpecifier.h" #include "clang/AST/Type.h" #include "clang/Basic/Diagnostic.h" @@ -34,6 +35,7 @@ #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/None.h" #include "llvm/ADT/Optional.h" +#include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/ADT/StringSet.h" #include "llvm/Support/Error.h" @@ -301,6 +303,24 @@ return Result; } +/// Returns all namespace scopes that the unqualified lookup would visit. +std::vector +collectAccessibleScopes(Sema &Sem, const DeclarationNameInfo &Typo, Scope *S, + Sema::LookupNameKind LookupKind) { + std::vector Scopes; + VisitedContextCollector Collector; + Sem.LookupVisibleDecls(S, LookupKind, Collector, + /*IncludeGlobalScope=*/false, + /*LoadExternal=*/false); + + Scopes.push_back(""); + for (const auto *Ctx : Collector.takeVisitedContexts()) { + if (isa(Ctx)) + Scopes.push_back(printNamespaceScope(*Ctx)); + } + return Scopes; +} + class IncludeFixer::UnresolvedNameRecorder : public ExternalSemaSource { public: UnresolvedNameRecorder(llvm::Optional &LastUnresolvedName) @@ -321,48 +341,30 @@ if (!isInsideMainFile(Typo.getLoc(), SemaPtr->SourceMgr)) return clang::TypoCorrection(); - // This is not done lazily because `SS` can get out of scope and it's - // relatively cheap. auto Extracted = extractUnresolvedNameCheaply( SemaPtr->SourceMgr, Typo, SS, SemaPtr->LangOpts, static_cast(LookupKind) == Sema::LookupNameKind::LookupNestedNameSpecifierName); if (!Extracted) return TypoCorrection(); - auto CheapUnresolved = std::move(*Extracted); + UnresolvedName Unresolved; - Unresolved.Name = CheapUnresolved.Name; + Unresolved.Name = Extracted->Name; Unresolved.Loc = Typo.getBeginLoc(); - - if (!CheapUnresolved.ResolvedScope && !S) // Give up if no scope available. + if (!Extracted->ResolvedScope && !S) // Give up if no scope available. return TypoCorrection(); - auto *Sem = SemaPtr; // Avoid capturing `this`. - Unresolved.GetScopes = [Sem, CheapUnresolved, S, LookupKind]() { - std::vector Scopes; - if (CheapUnresolved.ResolvedScope) { - Scopes.push_back(*CheapUnresolved.ResolvedScope); - } else { - assert(S); - // No scope specifier is specified. Collect all accessible scopes in the - // context. - VisitedContextCollector Collector; - Sem->LookupVisibleDecls( - S, static_cast(LookupKind), Collector, - /*IncludeGlobalScope=*/false, - /*LoadExternal=*/false); - - Scopes.push_back(""); - for (const auto *Ctx : Collector.takeVisitedContexts()) - if (isa(Ctx)) - Scopes.push_back(printNamespaceScope(*Ctx)); - } + if (Extracted->ResolvedScope) + Unresolved.Scopes.push_back(*Extracted->ResolvedScope); + else // no qualifier or qualifier is unresolved. + Unresolved.Scopes = collectAccessibleScopes( + *SemaPtr, Typo, S, static_cast(LookupKind)); + + if (Extracted->UnresolvedScope) { + for (std::string &Scope : Unresolved.Scopes) + Scope += *Extracted->UnresolvedScope; + } - if (CheapUnresolved.UnresolvedScope) - for (auto &Scope : Scopes) - Scope.append(*CheapUnresolved.UnresolvedScope); - return Scopes; - }; LastUnresolvedName = std::move(Unresolved); // Never return a valid correction to try to recover. Our suggested fixes @@ -384,14 +386,13 @@ std::vector IncludeFixer::fixUnresolvedName() const { assert(LastUnresolvedName.hasValue()); auto &Unresolved = *LastUnresolvedName; - std::vector Scopes = Unresolved.GetScopes(); vlog("Trying to fix unresolved name \"{0}\" in scopes: [{1}]", - Unresolved.Name, llvm::join(Scopes.begin(), Scopes.end(), ", ")); + Unresolved.Name, llvm::join(Unresolved.Scopes, ", ")); FuzzyFindRequest Req; Req.AnyScope = false; Req.Query = Unresolved.Name; - Req.Scopes = Scopes; + Req.Scopes = Unresolved.Scopes; Req.RestrictForCodeCompletion = true; Req.Limit = 100; diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -797,6 +797,27 @@ "Add include \"x.h\" for symbol a::X"))))); } +TEST(IncludeFixerTest, NoCrashOnTemplateInstantiations) { + Annotations Test(R"cpp( + template struct Templ { + template + typename U::type operator=(const U &); + }; + + struct A { + Templ s; + A() { [[a]]; } // this caused crashes if we compute scopes lazily. + }; + )cpp"); + + auto TU = TestTU::withCode(Test.code()); + auto Index = buildIndexWithSymbol({}); + TU.ExternalIndex = Index.get(); + + EXPECT_THAT(TU.build().getDiagnostics(), + ElementsAre(Diag(Test.range(), "use of undeclared identifier 'a'"))); +} + TEST(DiagsInHeaders, DiagInsideHeader) { Annotations Main(R"cpp( #include [["a.h"]]