hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: MaskRay, ioeric, jkorous-apple, ilya-biryukov, klimek.
This patch adds index support for GoToDefinition -- when we don't get the
definition from local AST, we query our index (Static&Dynamic) index to
get it.
Since we currently collect top-level symbol in the index, it doesn't support all
cases (e.g. class members), we will extend the index to include more symbols in
the future.
Note: the newly-added test fails because of a DynamicIndex issue -- the symbol
is
overwritten instead of merging.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D45717
Files:
clangd/ClangdServer.cpp
clangd/XRefs.cpp
clangd/XRefs.h
unittests/clangd/XRefsTests.cpp
Index: unittests/clangd/XRefsTests.cpp
===================================================================
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -106,6 +106,44 @@
MATCHER_P(RangeIs, R, "") { return arg.range == R; }
+TEST(GoToDefinition, WithIndex) {
+ MockFSProvider FS;
+ IgnoreDiagnostics DiagConsumer;
+ MockCompilationDatabase CDB;
+ auto option = ClangdServer::optsForTest();
+ option.BuildDynamicSymbolIndex = true;
+ ClangdServer Server(CDB, FS, DiagConsumer, option);
+
+ const char *DefHeaderSource = "void ff();";
+ auto DefCpp = testPath("def.cpp");
+ auto DefHeader = testPath("def.h");
+ auto TestCpp = testPath("test.cpp");
+ Annotations Def(R"cpp(
+ #include "def.h"
+ void [[ff]]() {}
+ )cpp");
+ Annotations Test(R"cpp(
+ #include "def.h"
+ void f() {
+ ^ff();
+ }
+ )cpp");
+ FS.Files[DefCpp] = Def.code();
+ FS.Files[TestCpp] = Test.code();
+ FS.Files[DefHeader] = DefHeaderSource;
+
+ runAddDocument(Server, TestCpp, Test.code());
+ runAddDocument(Server, DefCpp, Def.code());
+ std::vector<Matcher<Location>> ExpectedLocations;
+ for (const auto &R : Def.ranges())
+ ExpectedLocations.push_back(RangeIs(R));
+
+ auto Locations = runFindDefinitions(Server, TestCpp, Test.point());
+ EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
+ EXPECT_THAT(*Locations, ElementsAreArray(ExpectedLocations))
+ << Test.code();
+}
+
TEST(GoToDefinition, All) {
const char *Tests[] = {
R"cpp(// Local variable
Index: clangd/XRefs.h
===================================================================
--- clangd/XRefs.h
+++ clangd/XRefs.h
@@ -14,14 +14,16 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_XREFS_H
#include "ClangdUnit.h"
+#include "index/Index.h"
#include "Protocol.h"
#include <vector>
namespace clang {
namespace clangd {
/// Get definition of symbol at a specified \p Pos.
-std::vector<Location> findDefinitions(ParsedAST &AST, Position Pos);
+std::vector<Location> findDefinitions(ParsedAST &AST, Position Pos,
+ const SymbolIndex *const Index = nullptr);
/// Returns highlights for all usages of a symbol at \p Pos.
std::vector<DocumentHighlight> findDocumentHighlights(ParsedAST &AST,
Index: clangd/XRefs.cpp
===================================================================
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -14,6 +14,7 @@
#include "clang/AST/DeclTemplate.h"
#include "clang/Index/IndexDataConsumer.h"
#include "clang/Index/IndexingAction.h"
+#include "clang/Index/USRGeneration.h"
#include "llvm/Support/Path.h"
namespace clang {
namespace clangd {
@@ -128,6 +129,20 @@
}
};
+llvm::Optional<std::string>
+getAbsoluteFilePath(const FileEntry *F, const SourceManager &SourceMgr) {
+ SmallString<64> FilePath = F->tryGetRealPathName();
+ if (FilePath.empty())
+ FilePath = F->getName();
+ if (!llvm::sys::path::is_absolute(FilePath)) {
+ if (!SourceMgr.getFileManager().makeAbsolutePath(FilePath)) {
+ log("Could not turn relative path to absolute: " + FilePath);
+ return llvm::None;
+ }
+ }
+ return FilePath.str().str();
+}
+
llvm::Optional<Location>
makeLocation(ParsedAST &AST, const SourceRange &ValSourceRange) {
const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
@@ -145,24 +160,19 @@
Range R = {Begin, End};
Location L;
- SmallString<64> FilePath = F->tryGetRealPathName();
- if (FilePath.empty())
- FilePath = F->getName();
- if (!llvm::sys::path::is_absolute(FilePath)) {
- if (!SourceMgr.getFileManager().makeAbsolutePath(FilePath)) {
- log("Could not turn relative path to absolute: " + FilePath);
- return llvm::None;
- }
- }
-
- L.uri = URIForFile(FilePath.str());
+ auto FilePath = getAbsoluteFilePath(F, SourceMgr);
+ if (!FilePath)
+ return llvm::None;
+ L.uri = URIForFile(*FilePath);
L.range = R;
return L;
}
} // namespace
-std::vector<Location> findDefinitions(ParsedAST &AST, Position Pos) {
+std::vector<Location>
+findDefinitions(ParsedAST &AST, Position Pos,
+ const SymbolIndex *const Index) {
const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
const FileEntry *FE = SourceMgr.getFileEntryForID(SourceMgr.getMainFileID());
if (!FE)
@@ -196,13 +206,61 @@
std::vector<const Decl *> Decls = DeclMacrosFinder->takeDecls();
std::vector<MacroDecl> MacroInfos = DeclMacrosFinder->takeMacroInfos();
+ llvm::DenseSet<SymbolID> QuerySyms;
+ llvm::DenseMap<SymbolID, Location> ResultCandidates;
for (auto D : Decls) {
auto Loc = findNameLoc(D);
auto L = makeLocation(AST, SourceRange(Loc, Loc));
- if (L)
- Result.push_back(*L);
+ if (!L) {
+ log("Could not creat location for Loc");
+ continue;
+ }
+ llvm::SmallString<128> USR;
+ if (index::generateUSRForDecl(D, USR))
+ continue;
+ auto ID = SymbolID(USR);
+ ResultCandidates.insert({ID, *L});
+ bool IsDef = GetDefinition(D) == D;
+ // Query the index if there is no definition point in the local AST.
+ if (!IsDef)
+ QuerySyms.insert(ID);
}
+ if (Index && !QuerySyms.empty()) {
+ LookupRequest IndexQuery;
+ IndexQuery.IDs = QuerySyms;
+ std::string HintPath;
+ if (auto Path = getAbsoluteFilePath(FE, SourceMgr))
+ HintPath = *Path;
+ Index->lookup(
+ IndexQuery, [&HintPath, &ResultCandidates](const Symbol &Sym) {
+ auto it = ResultCandidates.find(Sym.ID);
+ assert(it != ResultCandidates.end());
+ if (Sym.Definition) {
+ auto Uri = URI::parse(Sym.Definition.FileURI);
+ if (!Uri) {
+ log("Could not parse URI: " + Sym.Definition.FileURI);
+ return;
+ }
+ auto Path = URI::resolve(*Uri, HintPath);
+ if (!Path) {
+ log("Could not resolve URI");
+ return;
+ }
+ Location LSPLoc;
+ LSPLoc.uri = URIForFile(*Path);
+ LSPLoc.range.start.line = Sym.Definition.Start.Line;
+ LSPLoc.range.start.character = Sym.Definition.Start.Column;
+ LSPLoc.range.end.line = Sym.Definition.End.Line;
+ LSPLoc.range.end.character = Sym.Definition.End.Column;
+ it->second = std::move(LSPLoc);
+ }
+ });
+ }
+
+ for (auto It : ResultCandidates)
+ Result.push_back(It.second);
+
for (auto Item : MacroInfos) {
auto Loc = Item.Info->getDefinitionLoc();
auto L = makeLocation(AST, SourceRange(Loc, Loc));
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -360,11 +360,11 @@
void ClangdServer::findDefinitions(PathRef File, Position Pos,
Callback<std::vector<Location>> CB) {
auto FS = FSProvider.getFileSystem();
- auto Action = [Pos, FS](Callback<std::vector<Location>> CB,
+ auto Action = [Pos, FS, this](Callback<std::vector<Location>> CB,
llvm::Expected<InputsAndAST> InpAST) {
if (!InpAST)
return CB(InpAST.takeError());
- CB(clangd::findDefinitions(InpAST->AST, Pos));
+ CB(clangd::findDefinitions(InpAST->AST, Pos, this->FileIdx.get()));
};
WorkScheduler.runWithAST("Definitions", File, Bind(Action, std::move(CB)));
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
