Index: clangd/ClangdServer.cpp =================================================================== --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -22,6 +22,7 @@ #include "llvm/Support/Path.h" #include "llvm/Support/raw_ostream.h" #include +#include using namespace clang; using namespace clang::clangd; @@ -442,10 +443,16 @@ llvm::errc::invalid_argument); std::vector Result; - Resources->getAST().get()->runUnderLock([Pos, &Result, this](ParsedAST *AST) { + std::shared_future> Preamble = + Resources->getPreamble(); + + Resources->getAST().get()->runUnderLock([Pos, &Result, Preamble, + this](ParsedAST *AST) { if (!AST) return; - Result = clangd::findDefinitions(*AST, Pos, Logger); + + IncludeReferenceMap &IRM = AST->getIRM(); + Result = clangd::findDefinitions(*AST, Pos, Logger, IRM); }); return make_tagged(std::move(Result), TaggedFS.Tag); } Index: clangd/ClangdUnit.h =================================================================== --- clangd/ClangdUnit.h +++ clangd/ClangdUnit.h @@ -22,6 +22,7 @@ #include #include #include +#include namespace llvm { class raw_ostream; @@ -59,6 +60,8 @@ std::vector Diags; }; +using IncludeReferenceMap = std::unordered_map; + /// Stores and provides access to parsed AST. class ParsedAST { public: @@ -69,7 +72,8 @@ std::shared_ptr Preamble, std::unique_ptr Buffer, std::shared_ptr PCHs, - IntrusiveRefCntPtr VFS, clangd::Logger &Logger); + IntrusiveRefCntPtr VFS, clangd::Logger &Logger, + IncludeReferenceMap &IRM); ParsedAST(ParsedAST &&Other); ParsedAST &operator=(ParsedAST &&Other); @@ -89,12 +93,14 @@ const std::vector &getDiagnostics() const; + IncludeReferenceMap &getIRM() { return IRM; }; + private: ParsedAST(std::shared_ptr Preamble, std::unique_ptr Clang, std::unique_ptr Action, std::vector TopLevelDecls, - std::vector Diags); + std::vector Diags, IncludeReferenceMap IRM); private: void ensurePreambleDeclsDeserialized(); @@ -114,6 +120,7 @@ std::vector Diags; std::vector TopLevelDecls; bool PreambleDeclsDeserialized; + IncludeReferenceMap IRM; }; // Provides thread-safe access to ParsedAST. @@ -256,14 +263,14 @@ clangd::Logger &Logger; }; - /// Get the beginning SourceLocation at a specified \p Pos. SourceLocation getBeginningOfIdentifier(ParsedAST &Unit, const Position &Pos, const FileEntry *FE); /// Get definition of symbol at a specified \p Pos. -std::vector findDefinitions(ParsedAST &AST, Position Pos, - clangd::Logger &Logger); +std::vector +findDefinitions(ParsedAST &AST, Position Pos, clangd::Logger &Logger, + std::unordered_map IncludeLocationMap); /// For testing/debugging purposes. Note that this method deserializes all /// unserialized Decls, so use with care. Index: clangd/ClangdUnit.cpp =================================================================== --- clangd/ClangdUnit.cpp +++ clangd/ClangdUnit.cpp @@ -29,9 +29,6 @@ #include "llvm/Support/CrashRecoveryContext.h" #include "llvm/Support/Format.h" -#include -#include - using namespace clang::clangd; using namespace clang; @@ -73,12 +70,17 @@ std::vector TopLevelDecls; }; -class CppFilePreambleCallbacks : public PreambleCallbacks { +class CppFilePreambleCallbacks : public PreambleCallbacks, public PPCallbacks { public: std::vector takeTopLevelDeclIDs() { return std::move(TopLevelDeclIDs); } + CppFilePreambleCallbacks(SourceManager *SourceMgr, IncludeReferenceMap &IRM) + : SourceMgr(SourceMgr), IRM(IRM) {} + + IncludeReferenceMap &getIRM() { return IRM; }; + void AfterPCHEmitted(ASTWriter &Writer) override { TopLevelDeclIDs.reserve(TopLevelDecls.size()); for (Decl *D : TopLevelDecls) { @@ -97,9 +99,80 @@ } } + void AfterExecute(CompilerInstance &CI) override { + + SourceMgr = &CI.getSourceManager(); + for (auto InclusionLoc : TempPreambleIncludeLocations) + addIncludeLocation(InclusionLoc); + } + + void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok, + StringRef FileName, bool IsAngled, + CharSourceRange FilenameRange, const FileEntry *File, + StringRef SearchPath, StringRef RelativePath, + const Module *Imported) override { + auto SR = FilenameRange.getAsRange(); + if (SR.isInvalid() || !File || File->tryGetRealPathName().empty()) + return; + + if (SourceMgr) { + addIncludeLocation({SR, File->tryGetRealPathName()}); + } else { + // When we are processing the inclusion directives inside the preamble, + // we don't have access to a SourceManager, so we cannot convert + // SourceRange to Range. This is done instead in AfterExecute when a + // SourceManager becomes available. + TempPreambleIncludeLocations.push_back({SR, File->tryGetRealPathName()}); + } + } + + void addIncludeLocation(std::pair InclusionLoc) { + // Only inclusion directives in the main file make sense. The user cannot + // select directives not in the main file. + if (SourceMgr->getMainFileID() == SourceMgr->getFileID(InclusionLoc.first.getBegin()) && SourceMgr->isInMainFile(InclusionLoc.first.getBegin())) + IRM.insert({getRange(InclusionLoc.first), InclusionLoc.second}); + } + + Range getRange(SourceRange SR) { + Position Begin; + Begin.line = SourceMgr->getSpellingLineNumber(SR.getBegin()); + Begin.character = SourceMgr->getSpellingColumnNumber(SR.getBegin()); + Position End; + End.line = SourceMgr->getSpellingLineNumber(SR.getEnd()); + End.character = SourceMgr->getSpellingColumnNumber(SR.getEnd()); + return {Begin, End}; + } + + std::unique_ptr createPPCallbacks() { + class DelegatingPPCallbacks : public PPCallbacks { + public: + DelegatingPPCallbacks(CppFilePreambleCallbacks &PPCallbacks) : Collector(PPCallbacks) {} + + void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok, + StringRef FileName, bool IsAngled, + CharSourceRange FilenameRange, const FileEntry *File, + StringRef SearchPath, StringRef RelativePath, + const Module *Imported) override { + Collector.InclusionDirective(HashLoc, IncludeTok, FileName, IsAngled, + FilenameRange, File, SearchPath, RelativePath, + Imported); + } + + private: + CppFilePreambleCallbacks &Collector; + }; + + std::unique_ptr DelegatedPPCallbacks = + llvm::make_unique(*this); + return DelegatedPPCallbacks; + } + private: std::vector TopLevelDecls; std::vector TopLevelDeclIDs; + SourceManager *SourceMgr; + IncludeReferenceMap &IRM; + std::vector> TempPreambleIncludeLocations; }; /// Convert from clang diagnostic level to LSP severity. @@ -171,7 +244,7 @@ std::unique_ptr Buffer, std::shared_ptr PCHs, IntrusiveRefCntPtr VFS, - clangd::Logger &Logger) { + clangd::Logger &Logger, IncludeReferenceMap &IRM) { std::vector ASTDiags; StoreDiagsConsumer UnitDiagsConsumer(/*ref*/ ASTDiags); @@ -193,16 +266,22 @@ MainInput.getFile()); return llvm::None; } + + CppFilePreambleCallbacks SerializedDeclsCollector(&Clang->getSourceManager(), + IRM); + + Clang->getPreprocessor().addPPCallbacks(SerializedDeclsCollector.createPPCallbacks()); + if (!Action->Execute()) Logger.log("Execute() failed when building AST for " + MainInput.getFile()); - // UnitDiagsConsumer is local, we can not store it in CompilerInstance that // has a longer lifetime. Clang->getDiagnostics().setClient(new IgnoreDiagnostics); std::vector ParsedDecls = Action->takeTopLevelDecls(); return ParsedAST(std::move(Preamble), std::move(Clang), std::move(Action), - std::move(ParsedDecls), std::move(ASTDiags)); + std::move(ParsedDecls), std::move(ASTDiags), + std::move(std::move(SerializedDeclsCollector.getIRM()))); } namespace { @@ -227,12 +306,15 @@ const SourceLocation &SearchedLocation; const ASTContext &AST; Preprocessor &PP; + std::unordered_map IncludeLocationMap; public: - DeclarationLocationsFinder(raw_ostream &OS, - const SourceLocation &SearchedLocation, - ASTContext &AST, Preprocessor &PP) - : SearchedLocation(SearchedLocation), AST(AST), PP(PP) {} + DeclarationLocationsFinder( + raw_ostream &OS, const SourceLocation &SearchedLocation, ASTContext &AST, + Preprocessor &PP, + std::unordered_map IncludeLocationMap) + : SearchedLocation(SearchedLocation), AST(AST), PP(PP), + IncludeLocationMap(IncludeLocationMap) {} std::vector takeLocations() { // Don't keep the same location multiple times. @@ -262,6 +344,11 @@ SourceMgr.getFileID(SearchedLocation) == FID; } + bool isSameLine(unsigned Line) const { + const SourceManager &SourceMgr = AST.getSourceManager(); + return Line == SourceMgr.getSpellingLineNumber(SearchedLocation); + } + void addDeclarationLocation(const SourceRange &ValSourceRange) { const SourceManager &SourceMgr = AST.getSourceManager(); const LangOptions &LangOpts = AST.getLangOpts(); @@ -275,19 +362,28 @@ End.line = SourceMgr.getSpellingLineNumber(LocEnd) - 1; End.character = SourceMgr.getSpellingColumnNumber(LocEnd) - 1; Range R = {Begin, End}; + addLocation(URI::fromFile( + SourceMgr.getFilename(SourceMgr.getSpellingLoc(LocStart))), + R); + } + + void addLocation(URI Uri, Range R) { Location L; - if (const FileEntry *F = - SourceMgr.getFileEntryForID(SourceMgr.getFileID(LocStart))) { - StringRef FilePath = F->tryGetRealPathName(); - if (FilePath.empty()) - FilePath = F->getName(); - L.uri = URI::fromFile(FilePath); - L.range = R; - DeclarationLocations.push_back(L); - } + L.uri = Uri; + L.range = R; + DeclarationLocations.push_back(L); } void finish() override { + + for (auto &IncludeLoc : IncludeLocationMap) { + Range R = IncludeLoc.first; + if (isSameLine(R.start.line)) { + addLocation(URI::fromFile(IncludeLoc.second), Range{Position{0,0}, Position{0,0}}); + return; + } + } + // Also handle possible macro at the searched location. Token Result; if (!Lexer::getRawToken(SearchedLocation, Result, AST.getSourceManager(), @@ -319,8 +415,9 @@ } // namespace -std::vector clangd::findDefinitions(ParsedAST &AST, Position Pos, - clangd::Logger &Logger) { +std::vector clangd::findDefinitions( + ParsedAST &AST, Position Pos, clangd::Logger &Logger, + std::unordered_map IncludeLocationMap) { const SourceManager &SourceMgr = AST.getASTContext().getSourceManager(); const FileEntry *FE = SourceMgr.getFileEntryForID(SourceMgr.getMainFileID()); if (!FE) @@ -330,7 +427,7 @@ auto DeclLocationsFinder = std::make_shared( llvm::errs(), SourceLocationBeg, AST.getASTContext(), - AST.getPreprocessor()); + AST.getPreprocessor(), IncludeLocationMap); index::IndexingOptions IndexOpts; IndexOpts.SystemSymbolFilter = index::IndexingOptions::SystemSymbolFilterKind::All; @@ -405,11 +502,11 @@ std::unique_ptr Clang, std::unique_ptr Action, std::vector TopLevelDecls, - std::vector Diags) + std::vector Diags, IncludeReferenceMap IRM) : Preamble(std::move(Preamble)), Clang(std::move(Clang)), Action(std::move(Action)), Diags(std::move(Diags)), - TopLevelDecls(std::move(TopLevelDecls)), - PreambleDeclsDeserialized(false) { + TopLevelDecls(std::move(TopLevelDecls)), PreambleDeclsDeserialized(false), + IRM(std::move(IRM)) { assert(this->Clang); assert(this->Action); } @@ -565,6 +662,7 @@ } assert(CI && "Couldn't create CompilerInvocation"); + IncludeReferenceMap IRM; std::unique_ptr ContentsBuffer = llvm::MemoryBuffer::getMemBufferCopy(NewContents, That->FileName); @@ -590,7 +688,8 @@ IntrusiveRefCntPtr PreambleDiagsEngine = CompilerInstance::createDiagnostics( &CI->getDiagnosticOpts(), &PreambleDiagnosticsConsumer, false); - CppFilePreambleCallbacks SerializedDeclsCollector; + + CppFilePreambleCallbacks SerializedDeclsCollector(nullptr, IRM); auto BuiltPreamble = PrecompiledPreamble::Build( *CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine, VFS, PCHs, /*StoreInMemory=*/That->StorePreamblesInMemory, @@ -631,9 +730,9 @@ { trace::Span Tracer("Build"); SPAN_ATTACH(Tracer, "File", That->FileName); - NewAST = - ParsedAST::Build(std::move(CI), std::move(NewPreamble), - std::move(ContentsBuffer), PCHs, VFS, That->Logger); + NewAST = ParsedAST::Build( + std::move(CI), std::move(NewPreamble), std::move(ContentsBuffer), + PCHs, VFS, That->Logger, IRM); } if (NewAST) { Index: clangd/Protocol.h =================================================================== --- clangd/Protocol.h +++ clangd/Protocol.h @@ -108,6 +108,14 @@ bool fromJSON(const json::Expr &, Range &); json::Expr toJSON(const Range &); +class RangeHash { +public: + std::size_t operator()(const Range &R) const { + return ((R.start.line & 0x18) << 3) | ((R.start.character & 0x18) << 1) | + ((R.end.line & 0x18) >> 1) | ((R.end.character & 0x18) >> 3); + } +}; + struct Location { /// The text document's URI. URI uri; Index: unittests/clangd/ClangdTests.cpp =================================================================== --- unittests/clangd/ClangdTests.cpp +++ unittests/clangd/ClangdTests.cpp @@ -7,7 +7,6 @@ // //===----------------------------------------------------------------------===// -#include "ClangdLSPServer.h" #include "ClangdServer.h" #include "Logger.h" #include "TestFS.h" @@ -749,6 +748,75 @@ EXPECT_FALSE(PathResult.hasValue()); } +TEST_F(ClangdVFSTest, CheckDefinitionIncludes) { + MockFSProvider FS; + ErrorCheckingDiagConsumer DiagConsumer; + MockCompilationDatabase CDB; + + ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), + /*StorePreamblesInMemory=*/true, + EmptyLogger::getInstance()); + + auto FooCpp = getVirtualTestFilePath("foo.cpp"); + const auto SourceContents = R"cpp( + #include "foo.h" + #include "invalid.h" + int b = a; + // test + int foo; + #include "foo.h" + )cpp"; + FS.Files[FooCpp] = SourceContents; + auto FooH = getVirtualTestFilePath("foo.h"); + const auto HeaderContents = "int a;"; + + FS.Files[FooCpp] = SourceContents; + FS.Files[FooH] = HeaderContents; + + Server.addDocument(FooH, HeaderContents); + Server.addDocument(FooCpp, SourceContents); + + Position P = Position{1, 11}; + + auto ExpectedLocations = Server.findDefinitions(FooCpp, P); + ASSERT_TRUE(!!ExpectedLocations); + std::vector Locations = ExpectedLocations->Value; + EXPECT_TRUE(!Locations.empty()); + std::string s("file:///"); + std::string check = Locations[0].uri.uri; + check = check.erase(0, s.size() - 1); + check = check.substr(0, check.size()); + ASSERT_EQ(check, FooH); + ASSERT_EQ(Locations[0].range.start.line, 0); + ASSERT_EQ(Locations[0].range.start.character, 0); + ASSERT_EQ(Locations[0].range.end.line, 0); + ASSERT_EQ(Locations[0].range.end.character, 0); + + // Test ctrl-clicking on the #include part on the statement + Position P2 = Position{1, 3}; + + ExpectedLocations = Server.findDefinitions(FooCpp, P2); + ASSERT_TRUE(!!ExpectedLocations); + Locations = ExpectedLocations->Value; + EXPECT_TRUE(!Locations.empty()); + + // Test invalid include + Position P3 = Position{2, 11}; + + ExpectedLocations = Server.findDefinitions(FooCpp, P3); + ASSERT_TRUE(!!ExpectedLocations); + Locations = ExpectedLocations->Value; + EXPECT_TRUE(Locations.empty()); + + // Test include outside of Preamble + Position P4 = Position{6, 5}; + + ExpectedLocations = Server.findDefinitions(FooCpp, P4); + ASSERT_TRUE(!!ExpectedLocations); + Locations = ExpectedLocations->Value; + EXPECT_TRUE(!Locations.empty()); +} + TEST_F(ClangdThreadingTest, NoConcurrentDiagnostics) { class NoConcurrentAccessDiagConsumer : public DiagnosticsConsumer { public: