[clang-tools-extra] d9d9a2c - [clangd] Use index for go-to-type
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 21 14:11:02 PDT 2023
Author: Sam McCall
Date: 2023-07-21T23:10:33+02:00
New Revision: d9d9a2cb2f0db7a92eb7d5ef0c619fb41aa5c8a8
URL: https://github.com/llvm/llvm-project/commit/d9d9a2cb2f0db7a92eb7d5ef0c619fb41aa5c8a8
DIFF: https://github.com/llvm/llvm-project/commit/d9d9a2cb2f0db7a92eb7d5ef0c619fb41aa5c8a8.diff
LOG: [clangd] Use index for go-to-type
This ensures it finds the definition even if not visible.
Differential Revision: https://reviews.llvm.org/D155898
Added:
Modified:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/XRefs.h
clang-tools-extra/clangd/unittests/XRefsTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index 29390196a6d977..d44d1e272b9b77 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -956,12 +956,12 @@ void ClangdServer::foldingRanges(llvm::StringRef File,
void ClangdServer::findType(llvm::StringRef File, Position Pos,
Callback<std::vector<LocatedSymbol>> CB) {
- auto Action =
- [Pos, CB = std::move(CB)](llvm::Expected<InputsAndAST> InpAST) mutable {
- if (!InpAST)
- return CB(InpAST.takeError());
- CB(clangd::findType(InpAST->AST, Pos));
- };
+ auto Action = [Pos, CB = std::move(CB),
+ this](llvm::Expected<InputsAndAST> InpAST) mutable {
+ if (!InpAST)
+ return CB(InpAST.takeError());
+ CB(clangd::findType(InpAST->AST, Pos, Index));
+ };
WorkScheduler->runWithAST("FindType", File, std::move(Action));
}
diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp
index 5853870db77918..da1a803daebb0d 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -341,6 +341,50 @@ std::vector<LocatedSymbol> findImplementors(llvm::DenseSet<SymbolID> IDs,
return Results;
}
+// Given LocatedSymbol results derived from the AST, query the index to obtain
+// definitions and preferred declarations.
+void enhanceLocatedSymbolsFromIndex(llvm::MutableArrayRef<LocatedSymbol> Result,
+ const SymbolIndex *Index,
+ llvm::StringRef MainFilePath) {
+ LookupRequest QueryRequest;
+ llvm::DenseMap<SymbolID, unsigned> ResultIndex;
+ for (unsigned I = 0; I < Result.size(); ++I) {
+ if (auto ID = Result[I].ID) {
+ ResultIndex.try_emplace(ID, I);
+ QueryRequest.IDs.insert(ID);
+ }
+ }
+ if (!Index || QueryRequest.IDs.empty())
+ return;
+ std::string Scratch;
+ Index->lookup(QueryRequest, [&](const Symbol &Sym) {
+ auto &R = Result[ResultIndex.lookup(Sym.ID)];
+
+ if (R.Definition) { // from AST
+ // Special case: if the AST yielded a definition, then it may not be
+ // the right *declaration*. Prefer the one from the index.
+ if (auto Loc = toLSPLocation(Sym.CanonicalDeclaration, MainFilePath))
+ R.PreferredDeclaration = *Loc;
+
+ // We might still prefer the definition from the index, e.g. for
+ // generated symbols.
+ if (auto Loc = toLSPLocation(
+ getPreferredLocation(*R.Definition, Sym.Definition, Scratch),
+ MainFilePath))
+ R.Definition = *Loc;
+ } else {
+ R.Definition = toLSPLocation(Sym.Definition, MainFilePath);
+
+ // Use merge logic to choose AST or index declaration.
+ if (auto Loc = toLSPLocation(
+ getPreferredLocation(R.PreferredDeclaration,
+ Sym.CanonicalDeclaration, Scratch),
+ MainFilePath))
+ R.PreferredDeclaration = *Loc;
+ }
+ });
+}
+
// Decls are more complicated.
// The AST contains at least a declaration, maybe a definition.
// These are up-to-date, and so generally preferred over index results.
@@ -352,8 +396,6 @@ locateASTReferent(SourceLocation CurLoc, const syntax::Token *TouchedIdentifier,
const SourceManager &SM = AST.getSourceManager();
// Results follow the order of Symbols.Decls.
std::vector<LocatedSymbol> Result;
- // Keep track of SymbolID -> index mapping, to fill in index data later.
- llvm::DenseMap<SymbolID, size_t> ResultIndex;
static constexpr trace::Metric LocateASTReferentMetric(
"locate_ast_referent", trace::Metric::Counter, "case");
@@ -371,10 +413,6 @@ locateASTReferent(SourceLocation CurLoc, const syntax::Token *TouchedIdentifier,
if (const NamedDecl *Def = getDefinition(D))
Result.back().Definition = makeLocation(
AST.getASTContext(), nameLocation(*Def, SM), MainFilePath);
-
- // Record SymbolID for index lookup later.
- if (auto ID = getSymbolID(D))
- ResultIndex[ID] = Result.size() - 1;
};
// Emit all symbol locations (declaration or definition) from AST.
@@ -448,40 +486,7 @@ locateASTReferent(SourceLocation CurLoc, const syntax::Token *TouchedIdentifier,
// Otherwise the target declaration is the right one.
AddResultDecl(D);
}
-
- // Now query the index for all Symbol IDs we found in the AST.
- if (Index && !ResultIndex.empty()) {
- LookupRequest QueryRequest;
- for (auto It : ResultIndex)
- QueryRequest.IDs.insert(It.first);
- std::string Scratch;
- Index->lookup(QueryRequest, [&](const Symbol &Sym) {
- auto &R = Result[ResultIndex.lookup(Sym.ID)];
-
- if (R.Definition) { // from AST
- // Special case: if the AST yielded a definition, then it may not be
- // the right *declaration*. Prefer the one from the index.
- if (auto Loc = toLSPLocation(Sym.CanonicalDeclaration, MainFilePath))
- R.PreferredDeclaration = *Loc;
-
- // We might still prefer the definition from the index, e.g. for
- // generated symbols.
- if (auto Loc = toLSPLocation(
- getPreferredLocation(*R.Definition, Sym.Definition, Scratch),
- MainFilePath))
- R.Definition = *Loc;
- } else {
- R.Definition = toLSPLocation(Sym.Definition, MainFilePath);
-
- // Use merge logic to choose AST or index declaration.
- if (auto Loc = toLSPLocation(
- getPreferredLocation(R.PreferredDeclaration,
- Sym.CanonicalDeclaration, Scratch),
- MainFilePath))
- R.PreferredDeclaration = *Loc;
- }
- });
- }
+ enhanceLocatedSymbolsFromIndex(Result, Index, MainFilePath);
auto Overrides = findImplementors(VirtualMethods, RelationKind::OverriddenBy,
Index, MainFilePath);
@@ -490,7 +495,8 @@ locateASTReferent(SourceLocation CurLoc, const syntax::Token *TouchedIdentifier,
}
std::vector<LocatedSymbol> locateSymbolForType(const ParsedAST &AST,
- const QualType &Type) {
+ const QualType &Type,
+ const SymbolIndex *Index) {
const auto &SM = AST.getSourceManager();
auto MainFilePath = AST.tuPath();
@@ -520,6 +526,7 @@ std::vector<LocatedSymbol> locateSymbolForType(const ParsedAST &AST,
Results.back().Definition =
makeLocation(ASTContext, nameLocation(*Def, SM), MainFilePath);
}
+ enhanceLocatedSymbolsFromIndex(Results, Index, MainFilePath);
return Results;
}
@@ -784,7 +791,7 @@ std::vector<LocatedSymbol> locateSymbolAt(ParsedAST &AST, Position Pos,
// go-to-definition on auto should find the definition of the deduced
// type, if possible
if (auto Deduced = getDeducedType(AST.getASTContext(), Tok.location())) {
- auto LocSym = locateSymbolForType(AST, *Deduced);
+ auto LocSym = locateSymbolForType(AST, *Deduced, Index);
if (!LocSym.empty())
return LocSym;
}
@@ -2053,13 +2060,13 @@ static void unwrapFindType(
// Convenience overload, to allow calling this without the out-parameter
static llvm::SmallVector<QualType> unwrapFindType(
QualType T, const HeuristicResolver* H) {
- llvm::SmallVector<QualType> Result;
- unwrapFindType(T, H, Result);
- return Result;
+ llvm::SmallVector<QualType> Result;
+ unwrapFindType(T, H, Result);
+ return Result;
}
-
-std::vector<LocatedSymbol> findType(ParsedAST &AST, Position Pos) {
+std::vector<LocatedSymbol> findType(ParsedAST &AST, Position Pos,
+ const SymbolIndex *Index) {
const SourceManager &SM = AST.getSourceManager();
auto Offset = positionToOffset(SM.getBufferData(SM.getMainFileID()), Pos);
std::vector<LocatedSymbol> Result;
@@ -2070,7 +2077,7 @@ std::vector<LocatedSymbol> findType(ParsedAST &AST, Position Pos) {
}
// The general scheme is: position -> AST node -> type -> declaration.
auto SymbolsFromNode =
- [&AST](const SelectionTree::Node *N) -> std::vector<LocatedSymbol> {
+ [&](const SelectionTree::Node *N) -> std::vector<LocatedSymbol> {
std::vector<LocatedSymbol> LocatedSymbols;
// NOTE: unwrapFindType might return duplicates for something like
@@ -2078,7 +2085,8 @@ std::vector<LocatedSymbol> findType(ParsedAST &AST, Position Pos) {
// information about the type you may have not known before
// (since unique_ptr<unique_ptr<T>> != unique_ptr<T>).
for (const QualType& Type : unwrapFindType(typeForNode(N), AST.getHeuristicResolver()))
- llvm::copy(locateSymbolForType(AST, Type), std::back_inserter(LocatedSymbols));
+ llvm::copy(locateSymbolForType(AST, Type, Index),
+ std::back_inserter(LocatedSymbols));
return LocatedSymbols;
};
diff --git a/clang-tools-extra/clangd/XRefs.h b/clang-tools-extra/clangd/XRefs.h
index d8a1dc9181f2f6..df91dd15303c18 100644
--- a/clang-tools-extra/clangd/XRefs.h
+++ b/clang-tools-extra/clangd/XRefs.h
@@ -107,7 +107,8 @@ std::vector<LocatedSymbol> findImplementations(ParsedAST &AST, Position Pos,
///
/// For example, given `b^ar()` wher bar return Foo, this function returns the
/// definition of class Foo.
-std::vector<LocatedSymbol> findType(ParsedAST &AST, Position Pos);
+std::vector<LocatedSymbol> findType(ParsedAST &AST, Position Pos,
+ const SymbolIndex *Index);
/// Returns references of the symbol at a specified \p Pos.
/// \p Limit limits the number of results returned (0 means no limit).
diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index ac19bcce7ea914..05cf891e71db40 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -33,12 +33,10 @@ namespace clangd {
namespace {
using ::testing::AllOf;
-using ::testing::Contains;
using ::testing::ElementsAre;
using ::testing::Eq;
using ::testing::IsEmpty;
using ::testing::Matcher;
-using ::testing::Not;
using ::testing::UnorderedElementsAre;
using ::testing::UnorderedElementsAreArray;
using ::testing::UnorderedPointwise;
@@ -1879,7 +1877,7 @@ TEST(FindType, All) {
ASSERT_GT(A.points().size(), 0u) << Case;
for (auto Pos : A.points())
- EXPECT_THAT(findType(AST, Pos),
+ EXPECT_THAT(findType(AST, Pos, nullptr),
ElementsAre(
sym("Target", HeaderA.range("Target"), HeaderA.range("Target"))))
<< Case;
@@ -1892,7 +1890,7 @@ TEST(FindType, All) {
TU.Code = A.code().str();
ParsedAST AST = TU.build();
- EXPECT_THAT(findType(AST, A.point()),
+ EXPECT_THAT(findType(AST, A.point(), nullptr),
UnorderedElementsAre(
sym("Target", HeaderA.range("Target"), HeaderA.range("Target")),
sym("smart_ptr", HeaderA.range("smart_ptr"), HeaderA.range("smart_ptr"))
@@ -1901,6 +1899,39 @@ TEST(FindType, All) {
}
}
+TEST(FindType, Definition) {
+ Annotations A(R"cpp(
+ class $decl[[X]];
+ X *^x;
+ class $def[[X]] {};
+ )cpp");
+ auto TU = TestTU::withCode(A.code().str());
+ ParsedAST AST = TU.build();
+
+ EXPECT_THAT(findType(AST, A.point(), nullptr),
+ ElementsAre(sym("X", A.range("decl"), A.range("def"))));
+}
+
+TEST(FindType, Index) {
+ Annotations Def(R"cpp(
+ // This definition is only available through the index.
+ class [[X]] {};
+ )cpp");
+ TestTU DefTU = TestTU::withHeaderCode(Def.code());
+ DefTU.HeaderFilename = "def.h";
+ auto DefIdx = DefTU.index();
+
+ Annotations A(R"cpp(
+ class [[X]];
+ X *^x;
+ )cpp");
+ auto TU = TestTU::withCode(A.code().str());
+ ParsedAST AST = TU.build();
+
+ EXPECT_THAT(findType(AST, A.point(), DefIdx.get()),
+ ElementsAre(sym("X", A.range(), Def.range())));
+}
+
void checkFindRefs(llvm::StringRef Test, bool UseIndex = false) {
Annotations T(Test);
auto TU = TestTU::withCode(T.code());
More information about the cfe-commits
mailing list