Index: clangd/ClangdServer.cpp =================================================================== --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -360,11 +360,11 @@ void ClangdServer::findDefinitions(PathRef File, Position Pos, Callback> CB) { auto FS = FSProvider.getFileSystem(); - auto Action = [Pos, FS](Callback> CB, - llvm::Expected InpAST) { + auto Action = [Pos, FS, this](Callback> CB, + llvm::Expected 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))); Index: clangd/XRefs.h =================================================================== --- clangd/XRefs.h +++ clangd/XRefs.h @@ -15,13 +15,15 @@ #include "ClangdUnit.h" #include "Protocol.h" +#include "index/Index.h" #include namespace clang { namespace clangd { /// Get definition of symbol at a specified \p Pos. -std::vector findDefinitions(ParsedAST &AST, Position Pos); +std::vector findDefinitions(ParsedAST &AST, Position Pos, + const SymbolIndex *Index = nullptr); /// Returns highlights for all usages of a symbol at \p Pos. std::vector 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,38 @@ } }; +struct IdentifiedSymbol { + std::vector Decls; + std::vector Macros; +}; + +IdentifiedSymbol getSymbolAtPosition(ParsedAST &AST, SourceLocation Pos) { + auto DeclMacrosFinder = std::make_shared( + llvm::errs(), Pos, AST.getASTContext(), AST.getPreprocessor()); + index::IndexingOptions IndexOpts; + IndexOpts.SystemSymbolFilter = + index::IndexingOptions::SystemSymbolFilterKind::All; + IndexOpts.IndexFunctionLocals = true; + indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(), + DeclMacrosFinder, IndexOpts); + + return {DeclMacrosFinder->takeDecls(), DeclMacrosFinder->takeMacroInfos()}; +} + +llvm::Optional +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 makeLocation(ParsedAST &AST, const SourceRange &ValSourceRange) { const SourceManager &SourceMgr = AST.getASTContext().getSourceManager(); @@ -145,24 +178,18 @@ 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 findDefinitions(ParsedAST &AST, Position Pos) { +std::vector findDefinitions(ParsedAST &AST, Position Pos, + const SymbolIndex *Index) { const SourceManager &SourceMgr = AST.getASTContext().getSourceManager(); const FileEntry *FE = SourceMgr.getFileEntryForID(SourceMgr.getMainFileID()); if (!FE) @@ -182,32 +209,108 @@ if (!Result.empty()) return Result; - auto DeclMacrosFinder = std::make_shared( - llvm::errs(), SourceLocationBeg, AST.getASTContext(), - AST.getPreprocessor()); - index::IndexingOptions IndexOpts; - IndexOpts.SystemSymbolFilter = - index::IndexingOptions::SystemSymbolFilterKind::All; - IndexOpts.IndexFunctionLocals = true; + // Identified symbols at a specific position. + auto Symbols = getSymbolAtPosition(AST, SourceLocationBeg); - indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(), - DeclMacrosFinder, IndexOpts); + // Handle goto definition for macros. + if (!Symbols.Macros.empty()) { + for (auto Item : Symbols.Macros) { + auto Loc = Item.Info->getDefinitionLoc(); + auto L = makeLocation(AST, SourceRange(Loc, Loc)); + if (L) + Result.push_back(*L); + } + return Result; + } - std::vector Decls = DeclMacrosFinder->takeDecls(); - std::vector MacroInfos = DeclMacrosFinder->takeMacroInfos(); + // Handle goto definition for typical declarations. + // + // Declaration and definition are different terms in C-family languages, and + // LSP only defines the "GoToDefinition" specification, so we try to perform + // the "most sensible" GoTo operation: + // + // - We use the location from AST and index (if available) to provide the + // final results. Prefer AST over index. + // + // - For each symbol, we will return a location of the cannonical declaration + // (e.g. function declaration in header), and a location of definition if + // they are available, definition comes first. + struct CandidateLocation { + llvm::Optional Def; + llvm::Optional Decl; + }; + llvm::DenseMap ResultCandidates; + + LookupRequest QueryRequest; + + for (auto D : Symbols.Decls) { + llvm::SmallString<128> USR; + if (index::generateUSRForDecl(D, USR)) + continue; - for (auto D : Decls) { auto Loc = findNameLoc(D); auto L = makeLocation(AST, SourceRange(Loc, Loc)); - if (L) - Result.push_back(*L); + auto ID = SymbolID(USR); + QueryRequest.IDs.insert(ID); + CandidateLocation &Candidate = ResultCandidates[ID]; + bool IsDef = GetDefinition(D) == D; + // Populate one of the slots with location for the AST. + if (!IsDef) + Candidate.Decl = L; + else + Candidate.Def = L; } - for (auto Item : MacroInfos) { - auto Loc = Item.Info->getDefinitionLoc(); - auto L = makeLocation(AST, SourceRange(Loc, Loc)); - if (L) - Result.push_back(*L); + if (Index) { + log("query index..."); + std::string HintPath; + if (auto Path = getAbsoluteFilePath(FE, SourceMgr)) + HintPath = *Path; + // Query the index and populate the empty slot. + Index->lookup(QueryRequest, + [&HintPath, &ResultCandidates](const Symbol &Sym) { + auto It = ResultCandidates.find(Sym.ID); + assert(It != ResultCandidates.end()); + + // FIXME: duplicate function with workspace symbols, share + // it. + auto ToLSPLocation = [&HintPath]( + const SymbolLocation &Loc) -> llvm::Optional { + if (!Loc) + return llvm::None; + auto Uri = URI::parse(Loc.FileURI); + if (!Uri) { + log("Could not parse URI: " + Loc.FileURI); + return llvm::None; + } + auto Path = URI::resolve(*Uri, HintPath); + if (!Path) { + log("Could not resolve URI"); + return llvm::None; + } + Location LSPLoc; + LSPLoc.uri = URIForFile(*Path); + LSPLoc.range.start.line = Loc.Start.Line; + LSPLoc.range.start.character = Loc.Start.Column; + LSPLoc.range.end.line = Loc.End.Line; + LSPLoc.range.end.character = Loc.End.Column; + return LSPLoc; + }; + + if (!It->second.Def) + It->second.Def = ToLSPLocation(Sym.Definition); + if (!It->second.Decl) + It->second.Decl = ToLSPLocation(Sym.CanonicalDeclaration); + }); + } + + // Populate the results, definition first. + for (auto It : ResultCandidates) { + auto Loc = It.second; + if (Loc.Def) + Result.push_back(*Loc.Def); + if (Loc.Decl && Loc.Decl != Loc.Def) // Decl and Def might be the same + Result.push_back(*(Loc.Decl)); } return Result; @@ -286,23 +389,16 @@ SourceLocation SourceLocationBeg = getBeginningOfIdentifier(AST, Pos, FE); - auto DeclMacrosFinder = std::make_shared( - llvm::errs(), SourceLocationBeg, AST.getASTContext(), - AST.getPreprocessor()); - index::IndexingOptions IndexOpts; - IndexOpts.SystemSymbolFilter = - index::IndexingOptions::SystemSymbolFilterKind::All; - IndexOpts.IndexFunctionLocals = true; - - // Macro occurences are not currently handled. - indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(), - DeclMacrosFinder, IndexOpts); - - std::vector SelectedDecls = DeclMacrosFinder->takeDecls(); + auto Symbols = getSymbolAtPosition(AST, SourceLocationBeg); + std::vector SelectedDecls = Symbols.Decls; auto DocHighlightsFinder = std::make_shared( llvm::errs(), AST.getASTContext(), AST.getPreprocessor(), SelectedDecls); + index::IndexingOptions IndexOpts; + IndexOpts.SystemSymbolFilter = + index::IndexingOptions::SystemSymbolFilterKind::All; + IndexOpts.IndexFunctionLocals = true; indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(), DocHighlightsFinder, IndexOpts); Index: unittests/clangd/XRefsTests.cpp =================================================================== --- unittests/clangd/XRefsTests.cpp +++ unittests/clangd/XRefsTests.cpp @@ -13,11 +13,13 @@ #include "SyncAPI.h" #include "TestFS.h" #include "XRefs.h" +#include "gmock/gmock.h" +#include "index/SymbolCollector.h" #include "clang/Frontend/CompilerInvocation.h" #include "clang/Frontend/PCHContainerOperations.h" #include "clang/Frontend/Utils.h" +#include "clang/Index/IndexingAction.h" #include "llvm/Support/Path.h" -#include "gmock/gmock.h" #include "gtest/gtest.h" namespace clang { @@ -37,17 +39,51 @@ }; // FIXME: this is duplicated with FileIndexTests. Share it. -ParsedAST build(StringRef Code) { - auto CI = createInvocationFromCommandLine( - {"clang", "-xc++", testPath("Foo.cpp").c_str()}); - auto Buf = MemoryBuffer::getMemBuffer(Code); +ParsedAST build(StringRef MainCode, StringRef HeaderCode = "", + StringRef BaseName = "Main", + llvm::IntrusiveRefCntPtr + InMemoryFileSystem = nullptr) { + auto HeaderPath = testPath((BaseName + ".h").str()); + auto MainPath = testPath((BaseName + ".cpp").str()); + if (!InMemoryFileSystem) + InMemoryFileSystem = new vfs::InMemoryFileSystem(); + + std::vector Cmd = {"clang", "-xc++", MainPath.c_str()}; + if (!HeaderCode.empty()) { + InMemoryFileSystem->addFile(HeaderPath, 0, + llvm::MemoryBuffer::getMemBuffer(HeaderCode)); + std::vector args = {"-include", HeaderPath.c_str()}; + Cmd.insert(Cmd.begin() + 1, args.begin(), args.end()); + } + + InMemoryFileSystem->addFile(MainPath, 0, + llvm::MemoryBuffer::getMemBuffer(MainCode)); + + auto CI = createInvocationFromCommandLine(Cmd); + + auto Buf = MemoryBuffer::getMemBuffer(MainCode); auto AST = ParsedAST::Build(std::move(CI), nullptr, std::move(Buf), std::make_shared(), - vfs::getRealFileSystem()); + InMemoryFileSystem); assert(AST.hasValue()); return std::move(*AST); } +std::unique_ptr buildIndexFromAST(ParsedAST *AST) { + SymbolCollector::Options CollectorOpts; + CollectorOpts.CollectIncludePath = false; + CollectorOpts.CountReferences = false; + index::IndexingOptions IndexOpts; + IndexOpts.SystemSymbolFilter = + index::IndexingOptions::SystemSymbolFilterKind::DeclarationsOnly; + IndexOpts.IndexFunctionLocals = false; + auto Collector = std::make_shared(std::move(CollectorOpts)); + Collector->setPreprocessor(AST->getPreprocessorPtr()); + index::indexTopLevelDecls(AST->getASTContext(), AST->getTopLevelDecls(), + Collector, IndexOpts); + return MemIndex::build(Collector->takeSymbols()); +} + // Extracts ranges from an annotated example, and constructs a matcher for a // highlight set. Ranges should be named $read/$write as appropriate. Matcher &> @@ -106,6 +142,80 @@ MATCHER_P(RangeIs, R, "") { return arg.range == R; } +TEST(GoToDefinition, WithIndex) { + llvm::IntrusiveRefCntPtr InMemoryFileSystem( + new vfs::InMemoryFileSystem); + Annotations SymbolHeader(R"cpp( + class $forward[[Forward]]; + class $foo[[Foo]] {}; + + void $f1[[f1]](); + + inline void $f2[[f2]]() {} + )cpp"); + Annotations SymbolCpp(R"cpp( + class $forward[[forward]] {}; + void $f1[[f1]]() {} + )cpp"); + + // Build index. + auto IndexAST = build(SymbolCpp.code(), SymbolHeader.code(), "symbol", + InMemoryFileSystem); + auto Index = buildIndexFromAST(&IndexAST); + + auto runFindDefinitionsWithIndex = + [&Index, &InMemoryFileSystem](const Annotations &Main) { + auto AST = build(/*MainCode=*/Main.code(), + /*HeaderCode=*/"", + /*BasePath=*/"main", InMemoryFileSystem); + return clangd::findDefinitions(AST, Main.point(), Index.get()); + }; + + Annotations Test(R"cpp(// only declaration in AST. + void [[f1]](); + int main() { + ^f1(); + } + )cpp"); + EXPECT_THAT(runFindDefinitionsWithIndex(Test), + testing::ElementsAreArray( + {RangeIs(SymbolCpp.range("f1")), RangeIs(Test.range())})); + + Test = Annotations(R"cpp(// definition in AST. + void [[f1]]() {} + int main() { + ^f1(); + } + )cpp"); + EXPECT_THAT(runFindDefinitionsWithIndex(Test), + testing::ElementsAreArray( + {RangeIs(Test.range()), RangeIs(SymbolHeader.range("f1"))})); + + Test = Annotations(R"cpp(// forward declaration in AST. + class [[Foo]]; + F^oo* create(); + )cpp"); + EXPECT_THAT(runFindDefinitionsWithIndex(Test), + testing::ElementsAreArray( + {RangeIs(SymbolHeader.range("foo")), RangeIs(Test.range())})); + + Test = Annotations(R"cpp(// defintion in AST. + class [[Forward]] {}; + F^orward create(); + )cpp"); + EXPECT_THAT(runFindDefinitionsWithIndex(Test), + testing::ElementsAreArray({ + RangeIs(Test.range()), RangeIs(SymbolHeader.range("forward")), + })); + + Test = Annotations(R"cpp(// forward declaration in AST. + #include "symbol.h" + F^oo* create(); + )cpp"); + EXPECT_THAT(runFindDefinitionsWithIndex(Test), + testing::ElementsAreArray({RangeIs(SymbolHeader.range("foo"))})); +} + TEST(GoToDefinition, All) { const char *Tests[] = { R"cpp(// Local variable