This revision was automatically updated to reflect the committed changes.
Closed by commit rL368019: [clangd] Compute scopes eagerly in IncludeFixer
(authored by ibiryukov, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D65796?vs=213557&id=213582#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65796/new/
https://reviews.llvm.org/D65796
Files:
clang-tools-extra/trunk/clangd/IncludeFixer.cpp
clang-tools-extra/trunk/clangd/IncludeFixer.h
clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp
Index: clang-tools-extra/trunk/clangd/IncludeFixer.h
===================================================================
--- clang-tools-extra/trunk/clangd/IncludeFixer.h
+++ clang-tools-extra/trunk/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<std::vector<std::string>()> GetScopes;
+ std::vector<std::string> Scopes; // Namespace scopes we should search in.
};
/// Records the last unresolved name seen by Sema.
Index: clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp
@@ -797,6 +797,27 @@
"Add include \"x.h\" for symbol a::X")))));
}
+TEST(IncludeFixerTest, NoCrashOnTemplateInstantiations) {
+ Annotations Test(R"cpp(
+ template <typename T> struct Templ {
+ template <typename U>
+ typename U::type operator=(const U &);
+ };
+
+ struct A {
+ Templ<char> 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"]]
Index: clang-tools-extra/trunk/clangd/IncludeFixer.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/IncludeFixer.cpp
+++ clang-tools-extra/trunk/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<std::string>
+collectAccessibleScopes(Sema &Sem, const DeclarationNameInfo &Typo, Scope *S,
+ Sema::LookupNameKind LookupKind) {
+ std::vector<std::string> Scopes;
+ VisitedContextCollector Collector;
+ Sem.LookupVisibleDecls(S, LookupKind, Collector,
+ /*IncludeGlobalScope=*/false,
+ /*LoadExternal=*/false);
+
+ Scopes.push_back("");
+ for (const auto *Ctx : Collector.takeVisitedContexts()) {
+ if (isa<NamespaceDecl>(Ctx))
+ Scopes.push_back(printNamespaceScope(*Ctx));
+ }
+ return Scopes;
+}
+
class IncludeFixer::UnresolvedNameRecorder : public ExternalSemaSource {
public:
UnresolvedNameRecorder(llvm::Optional<UnresolvedName> &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<Sema::LookupNameKind>(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<std::string> 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<Sema::LookupNameKind>(LookupKind), Collector,
- /*IncludeGlobalScope=*/false,
- /*LoadExternal=*/false);
-
- Scopes.push_back("");
- for (const auto *Ctx : Collector.takeVisitedContexts())
- if (isa<NamespaceDecl>(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<Sema::LookupNameKind>(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<Fix> IncludeFixer::fixUnresolvedName() const {
assert(LastUnresolvedName.hasValue());
auto &Unresolved = *LastUnresolvedName;
- std::vector<std::string> 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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
