Index: clangd/ClangdServer.cpp =================================================================== --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -287,11 +287,18 @@ assert(Resources && "Calling findDefinitions on non-added file"); std::vector Result; - Resources->getAST().get()->runUnderLock([Pos, &Result, this](ParsedAST *AST) { - if (!AST) - return; - Result = clangd::findDefinitions(*AST, Pos, Logger); - }); + std::map IncludeMap; + std::shared_ptr StalePreamble = + Resources->getPossiblyStalePreamble(); + if (StalePreamble) + IncludeMap = StalePreamble->IncludeMap; + Resources->getAST().get()->runUnderLock( + [Pos, &Result, IncludeMap, this](ParsedAST *AST) { + if (!AST) + return; + + Result = clangd::findDefinitions(*AST, Pos, Logger, IncludeMap); + }); return make_tagged(std::move(Result), TaggedFS.Tag); } @@ -344,7 +351,7 @@ return NewPath.str().str(); // First str() to convert from SmallString to // StringRef, second to convert from StringRef // to std::string - + // Also check NewExt in upper-case, just in case. llvm::sys::path::replace_extension(NewPath, NewExt.upper()); if (FS->exists(NewPath)) Index: clangd/ClangdUnit.h =================================================================== --- clangd/ClangdUnit.h +++ clangd/ClangdUnit.h @@ -127,11 +127,13 @@ struct PreambleData { PreambleData(PrecompiledPreamble Preamble, std::vector TopLevelDeclIDs, - std::vector Diags); + std::vector Diags, + std::map IncludeMap); PrecompiledPreamble Preamble; std::vector TopLevelDeclIDs; std::vector Diags; + std::map IncludeMap; }; /// Manages resources, required by clangd. Allows to rebuild file with new @@ -261,7 +263,8 @@ /// Get definition of symbol at a specified \p Pos. std::vector findDefinitions(ParsedAST &AST, Position Pos, - clangd::Logger &Logger); + clangd::Logger &Logger, + std::map); /// 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 @@ -78,6 +78,10 @@ return std::move(TopLevelDeclIDs); } + std::map takeIncludeMap() { + return std::move(IncludeMap); + } + void AfterPCHEmitted(ASTWriter &Writer) override { TopLevelDeclIDs.reserve(TopLevelDecls.size()); for (Decl *D : TopLevelDecls) { @@ -96,9 +100,55 @@ } } + void AfterExecute(CompilerInstance &CI) override { + const SourceManager &SM = CI.getSourceManager(); + unsigned n = SM.local_sloc_entry_size(); + SmallVector InclusionStack; + std::map::iterator it = IncludeMap.begin(); + + for (unsigned i = 0; i < n; ++i) { + bool Invalid = false; + const SrcMgr::SLocEntry &SL = SM.getLocalSLocEntry(i, &Invalid); + if (!SL.isFile() || Invalid) + continue; + const SrcMgr::FileInfo &FI = SL.getFile(); + SourceLocation L = FI.getIncludeLoc(); + InclusionStack.clear(); + + SourceLocation LocationToInsert; + + while (L.isValid()) { + PresumedLoc PLoc = SM.getPresumedLoc(L); + InclusionStack.push_back(L); + L = PLoc.isValid() ? PLoc.getIncludeLoc() : SourceLocation(); + } + if (InclusionStack.size() == 0) { + // Skip main file + continue; + } + + if (InclusionStack.size() > 1) { + // Don't care about includes of includes + continue; + } + + StringRef FilePath = + FI.getContentCache()->OrigEntry->tryGetRealPathName(); + if (FilePath.empty()) { + // FIXME: Does tryGetRealPathName return empty if and only if the path + // to the include doesn't exist? If that's the case we should skip this + // include. + FilePath = FI.getContentCache()->OrigEntry->getName(); + } + IncludeMap.insert(std::pair( + InclusionStack.front(), FilePath.str())); + } + } + private: std::vector TopLevelDecls; std::vector TopLevelDeclIDs; + std::map IncludeMap; }; /// Convert from clang diagnostic level to LSP severity. @@ -709,12 +759,15 @@ const SourceLocation &SearchedLocation; const ASTContext &AST; Preprocessor &PP; + std::map IncludeMap; public: DeclarationLocationsFinder(raw_ostream &OS, const SourceLocation &SearchedLocation, - ASTContext &AST, Preprocessor &PP) - : SearchedLocation(SearchedLocation), AST(AST), PP(PP) {} + ASTContext &AST, Preprocessor &PP, + std::map IncludeMap) + : SearchedLocation(SearchedLocation), AST(AST), PP(PP), + IncludeMap(IncludeMap) {} std::vector takeLocations() { // Don't keep the same location multiple times. @@ -744,7 +797,13 @@ SourceMgr.getFileID(SearchedLocation) == FID; } - void addDeclarationLocation(const SourceRange &ValSourceRange) { + bool isSameLine(unsigned line) const { + const SourceManager &SourceMgr = AST.getSourceManager(); + return line == SourceMgr.getSpellingLineNumber(SearchedLocation); + } + + void addDeclarationLocation(const SourceRange &ValSourceRange, + bool test = false) { const SourceManager &SourceMgr = AST.getSourceManager(); const LangOptions &LangOpts = AST.getLangOpts(); SourceLocation LocStart = ValSourceRange.getBegin(); @@ -757,14 +816,30 @@ 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; - L.uri = URI::fromFile( - SourceMgr.getFilename(SourceMgr.getSpellingLoc(LocStart))); + L.uri = uri; L.range = R; DeclarationLocations.push_back(L); } void finish() override { + + for (auto it = IncludeMap.begin(); it != IncludeMap.end(); ++it) { + SourceLocation L = it->first; + std::string &Path = it->second; + Range r = Range(); + unsigned line = AST.getSourceManager().getSpellingLineNumber(L); + if (isSameLine(line)) + addLocation(URI::fromFile(Path), Range()); + } + // Also handle possible macro at the searched location. Token Result; if (!Lexer::getRawToken(SearchedLocation, Result, AST.getSourceManager(), @@ -834,8 +909,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::map IncludeMap) { const SourceManager &SourceMgr = AST.getASTContext().getSourceManager(); const FileEntry *FE = SourceMgr.getFileEntryForID(SourceMgr.getMainFileID()); if (!FE) @@ -845,7 +921,7 @@ auto DeclLocationsFinder = std::make_shared( llvm::errs(), SourceLocationBeg, AST.getASTContext(), - AST.getPreprocessor()); + AST.getPreprocessor(), IncludeMap); index::IndexingOptions IndexOpts; IndexOpts.SystemSymbolFilter = index::IndexingOptions::SystemSymbolFilterKind::All; @@ -929,9 +1005,11 @@ PreambleData::PreambleData(PrecompiledPreamble Preamble, std::vector TopLevelDeclIDs, - std::vector Diags) + std::vector Diags, + std::map IncludeMap) : Preamble(std::move(Preamble)), - TopLevelDeclIDs(std::move(TopLevelDeclIDs)), Diags(std::move(Diags)) {} + TopLevelDeclIDs(std::move(TopLevelDeclIDs)), Diags(std::move(Diags)), + IncludeMap(std::move(IncludeMap)) {} std::shared_ptr CppFile::Create(PathRef FileName, tooling::CompileCommand Command, @@ -1093,7 +1171,8 @@ return std::make_shared( std::move(*BuiltPreamble), SerializedDeclsCollector.takeTopLevelDeclIDs(), - std::move(PreambleDiags)); + std::move(PreambleDiags), + SerializedDeclsCollector.takeIncludeMap()); } else { return nullptr; } 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 "clang/Basic/VirtualFileSystem.h" @@ -900,82 +899,55 @@ } } -TEST_F(ClangdVFSTest, CheckSourceHeaderSwitch) { +TEST_F(ClangdVFSTest, CheckDefinitionIncludes) { MockFSProvider FS; ErrorCheckingDiagConsumer DiagConsumer; MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); - ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), + ClangdServer Server(CDB, DiagConsumer, FS, 0, /*SnippetCompletions=*/false, EmptyLogger::getInstance()); - auto SourceContents = R"cpp( + auto FooCpp = getVirtualTestFilePath("foo.cpp"); + const auto SourceContents = R"cpp( #include "foo.h" + #include "invalid.h" int b = a; )cpp"; - - auto FooCpp = getVirtualTestFilePath("foo.cpp"); + FS.Files[FooCpp] = SourceContents; auto FooH = getVirtualTestFilePath("foo.h"); - auto Invalid = getVirtualTestFilePath("main.cpp"); + const auto HeaderContents = "int a;"; FS.Files[FooCpp] = SourceContents; - FS.Files[FooH] = "int a;"; - FS.Files[Invalid] = "int main() { \n return 0; \n }"; + FS.Files[FooH] = HeaderContents; - llvm::Optional PathResult = Server.switchSourceHeader(FooCpp); - EXPECT_TRUE(PathResult.hasValue()); - ASSERT_EQ(PathResult.getValue(), FooH); - - PathResult = Server.switchSourceHeader(FooH); - EXPECT_TRUE(PathResult.hasValue()); - ASSERT_EQ(PathResult.getValue(), FooCpp); + Server.addDocument(FooH, HeaderContents); + Server.addDocument(FooCpp, SourceContents); - SourceContents = R"c( - #include "foo.HH" - int b = a; - )c"; + Position P = Position{1, 11}; - // Test with header file in capital letters and different extension, source - // file with different extension - auto FooC = getVirtualTestFilePath("bar.c"); - auto FooHH = getVirtualTestFilePath("bar.HH"); - - FS.Files[FooC] = SourceContents; - FS.Files[FooHH] = "int a;"; - - PathResult = Server.switchSourceHeader(FooC); - EXPECT_TRUE(PathResult.hasValue()); - ASSERT_EQ(PathResult.getValue(), FooHH); - - // Test with both capital letters - auto Foo2C = getVirtualTestFilePath("foo2.C"); - auto Foo2HH = getVirtualTestFilePath("foo2.HH"); - FS.Files[Foo2C] = SourceContents; - FS.Files[Foo2HH] = "int a;"; - - PathResult = Server.switchSourceHeader(Foo2C); - EXPECT_TRUE(PathResult.hasValue()); - ASSERT_EQ(PathResult.getValue(), Foo2HH); - - // Test with source file as capital letter and .hxx header file - auto Foo3C = getVirtualTestFilePath("foo3.C"); - auto Foo3HXX = getVirtualTestFilePath("foo3.hxx"); + std::vector Locations = (Server.findDefinitions(FooCpp, P)).Value; + EXPECT_TRUE(!Locations.empty()); + std::string s("file:///"); + std::string check = URI::unparse(Locations[0].uri); + check = check.erase(0, s.size()); + check = check.substr(0, check.size() - 1); + 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 P3 = Position{1, 3}; - SourceContents = R"c( - #include "foo3.hxx" - int b = a; - )c"; + Locations = (Server.findDefinitions(FooCpp, P3)).Value; + EXPECT_TRUE(!Locations.empty()); - FS.Files[Foo3C] = SourceContents; - FS.Files[Foo3HXX] = "int a;"; + // Test invalid include + Position P2 = Position{2, 11}; - PathResult = Server.switchSourceHeader(Foo3C); - EXPECT_TRUE(PathResult.hasValue()); - ASSERT_EQ(PathResult.getValue(), Foo3HXX); - - // Test if asking for a corresponding file that doesn't exist returns an empty - // string. - PathResult = Server.switchSourceHeader(Invalid); - EXPECT_FALSE(PathResult.hasValue()); + Locations = (Server.findDefinitions(FooCpp, P2)).Value; + EXPECT_TRUE(Locations.empty()); } TEST_F(ClangdThreadingTest, NoConcurrentDiagnostics) {