Index: clangd/ClangdServer.cpp =================================================================== --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -73,11 +73,12 @@ void onPreambleAST(PathRef Path, ASTContext &Ctx, std::shared_ptr PP) override { if (FileIdx) - FileIdx->update(Path, &Ctx, std::move(PP)); + FileIdx->updatePreamble(Path, &Ctx, std::move(PP)); } void onMainAST(PathRef Path, ParsedAST &AST) override { - // FIXME: merge results from the main file into the index too. + if (FileIdx) + FileIdx->updateMainFile(Path, AST); } private: Index: clangd/index/FileIndex.h =================================================================== --- clangd/index/FileIndex.h +++ clangd/index/FileIndex.h @@ -41,7 +41,8 @@ public: /// \brief Updates all symbols in a file. If \p Slab is nullptr, symbols for /// \p Path will be removed. - void update(PathRef Path, std::unique_ptr Slab); + void updatePreamble(PathRef Path, std::unique_ptr Slab); + void updateMainFile(PathRef Path, SymbolSlab::Builder Builder); // The shared_ptr keeps the symbols alive std::shared_ptr> allSymbols(); @@ -49,8 +50,14 @@ private: mutable std::mutex Mutex; + struct FileSlabs { + /// Contains symbols in the preamble. + std::shared_ptr Preamble; + /// Contains symbol in the main file. + std::shared_ptr MainFile; + }; /// \brief Stores the latest snapshots for all active files. - llvm::StringMap> FileToSlabs; + llvm::StringMap FileToSlabs; }; /// \brief This manages symbls from files and an in-memory index on all symbols. @@ -64,7 +71,9 @@ /// nullptr, this removes all symbols in the file. /// If \p AST is not null, \p PP cannot be null and it should be the /// preprocessor that was used to build \p AST. - void update(PathRef Path, ASTContext *AST, std::shared_ptr PP); + void updatePreamble(PathRef Path, ASTContext *AST, std::shared_ptr PP); + /// FIXME(ibiryukov): add comments before submitting this. + void updateMainFile(PathRef Path, ParsedAST &AST); bool fuzzyFind(const FuzzyFindRequest &Req, @@ -86,8 +95,8 @@ /// Retrieves namespace and class level symbols in \p AST. /// Exposed to assist in unit tests. /// If URISchemes is empty, the default schemes in SymbolCollector will be used. -SymbolSlab indexAST(ASTContext &AST, std::shared_ptr PP, - llvm::ArrayRef URISchemes = {}); +SymbolSlab indexPreambleAST(ASTContext &AST, std::shared_ptr PP, + llvm::ArrayRef URISchemes = {}); } // namespace clangd } // namespace clang Index: clangd/index/FileIndex.cpp =================================================================== --- clangd/index/FileIndex.cpp +++ clangd/index/FileIndex.cpp @@ -8,22 +8,25 @@ //===----------------------------------------------------------------------===// #include "FileIndex.h" -#include "SymbolCollector.h" #include "../Logger.h" +#include "Merge.h" +#include "SymbolCollector.h" #include "clang/Index/IndexingAction.h" #include "clang/Lex/Preprocessor.h" namespace clang { namespace clangd { -SymbolSlab indexAST(ASTContext &AST, std::shared_ptr PP, - llvm::ArrayRef URISchemes) { +namespace { +SymbolCollector setupSymbolCollector(std::shared_ptr PP, + llvm::ArrayRef URISchemes) { SymbolCollector::Options CollectorOpts; - // FIXME(ioeric): we might also want to collect include headers. We would need - // to make sure all includes are canonicalized (with CanonicalIncludes), which - // is not trivial given the current way of collecting symbols: we only have - // AST at this point, but we also need preprocessor callbacks (e.g. - // CommentHandler for IWYU pragma) to canonicalize includes. + // FIXME(ioeric): we might also want to collect include headers. We would + // need to make sure all includes are canonicalized (with + // CanonicalIncludes), which is not trivial given the current way of + // collecting symbols: we only have AST at this point, but we also need + // preprocessor callbacks (e.g. CommentHandler for IWYU pragma) to + // canonicalize includes. CollectorOpts.CollectIncludePath = false; CollectorOpts.CountReferences = false; if (!URISchemes.empty()) @@ -32,6 +35,28 @@ SymbolCollector Collector(std::move(CollectorOpts)); Collector.setPreprocessor(PP); + return Collector; +} + +SymbolSlab::Builder indexMainAST(ASTContext &AST, + std::shared_ptr PP, + llvm::ArrayRef TopLevelDecls, + llvm::ArrayRef URISchemes) { + SymbolCollector Collector = setupSymbolCollector(PP, URISchemes); + index::IndexingOptions IndexOpts; + // We only need declarations, because we don't count references. + IndexOpts.SystemSymbolFilter = + index::IndexingOptions::SystemSymbolFilterKind::DeclarationsOnly; + IndexOpts.IndexFunctionLocals = false; + + index::indexTopLevelDecls(AST, TopLevelDecls, Collector, IndexOpts); + return Collector.takeBuilder(); +} +} // namespace + +SymbolSlab indexPreambleAST(ASTContext &AST, std::shared_ptr PP, + llvm::ArrayRef URISchemes) { + SymbolCollector Collector = setupSymbolCollector(PP, URISchemes); index::IndexingOptions IndexOpts; // We only need declarations, because we don't count references. IndexOpts.SystemSymbolFilter = @@ -49,17 +74,39 @@ FileIndex::FileIndex(std::vector URISchemes) : URISchemes(std::move(URISchemes)) {} -void FileSymbols::update(PathRef Path, std::unique_ptr Slab) { +void FileSymbols::updatePreamble(PathRef Path, + std::unique_ptr Slab) { std::lock_guard Lock(Mutex); if (!Slab) FileToSlabs.erase(Path); else - FileToSlabs[Path] = std::move(Slab); + FileToSlabs[Path].Preamble = std::move(Slab); +} + +void FileSymbols::updateMainFile(PathRef Path, SymbolSlab::Builder Builder) { + // FIXME(ibiryukov): most of this work should be done outside the lock. + std::lock_guard Lock(Mutex); + auto &FileSlabs = FileToSlabs[Path]; + if (FileSlabs.Preamble) { + // Merge all missing data from preamble. We need this because documentation + // may be missing in symbols with definition in the main file and + // declaration in the preamble. + for (const Symbol &MainFileSym : Builder.symbols()) { + auto PreambleSymIt = FileSlabs.Preamble->find(MainFileSym.ID); + if (PreambleSymIt == FileSlabs.Preamble->end()) + continue; + + Symbol::Details Scratch; + Builder.insert(mergeSymbol(MainFileSym, *PreambleSymIt, &Scratch)); + } + } + + FileSlabs.MainFile = std::make_shared(std::move(Builder).build()); } std::shared_ptr> FileSymbols::allSymbols() { - // The snapshot manages life time of symbol slabs and provides pointers of all - // symbols in all slabs. + // The snapshot manages life time of symbol slabs and provides pointers of + // all symbols in all slabs. struct Snapshot { std::vector Pointers; std::vector> KeepAlive; @@ -69,9 +116,27 @@ std::lock_guard Lock(Mutex); for (const auto &FileAndSlab : FileToSlabs) { - Snap->KeepAlive.push_back(FileAndSlab.second); - for (const auto &Iter : *FileAndSlab.second) - Snap->Pointers.push_back(&Iter); + // Add symbol from the main file. + llvm::DenseSet MainFileSymbols; + if (FileAndSlab.second.MainFile) { + Snap->KeepAlive.push_back(FileAndSlab.second.MainFile); + for (const auto &Iter : *FileAndSlab.second.MainFile) { + Snap->Pointers.push_back(&Iter); + MainFileSymbols.insert(Iter.ID); + } + } + + // Add symbols from preamble. + if (FileAndSlab.second.Preamble) { + Snap->KeepAlive.push_back(FileAndSlab.second.Preamble); + for (const auto &Iter : *FileAndSlab.second.Preamble) { + /// Prefer symbols from the main file, we ensure they have strictly + /// more information than the matching ones from preamble. + if (MainFileSymbols.find(Iter.ID) != MainFileSymbols.end()) + continue; + Snap->Pointers.push_back(&Iter); + } + } } } auto *Pointers = &Snap->Pointers; @@ -80,20 +145,29 @@ return {std::move(Snap), Pointers}; } -void FileIndex::update(PathRef Path, ASTContext *AST, - std::shared_ptr PP) { +void FileIndex::updatePreamble(PathRef Path, ASTContext *AST, + std::shared_ptr PP) { if (!AST) { - FSymbols.update(Path, nullptr); + FSymbols.updatePreamble(Path, nullptr); } else { assert(PP); auto Slab = llvm::make_unique(); - *Slab = indexAST(*AST, PP, URISchemes); - FSymbols.update(Path, std::move(Slab)); + *Slab = indexPreambleAST(*AST, PP, URISchemes); + FSymbols.updatePreamble(Path, std::move(Slab)); } auto Symbols = FSymbols.allSymbols(); Index.build(std::move(Symbols)); } +void FileIndex::updateMainFile(PathRef Path, ParsedAST &AST) { + SymbolSlab::Builder Builder = + indexMainAST(AST.getASTContext(), AST.getPreprocessorPtr(), + AST.getLocalTopLevelDecls(), URISchemes); + FSymbols.updateMainFile(Path, std::move(Builder)); + auto Symbols = FSymbols.allSymbols(); + Index.build(std::move(Symbols)); +} + bool FileIndex::fuzzyFind( const FuzzyFindRequest &Req, llvm::function_ref Callback) const { Index: clangd/index/Index.h =================================================================== --- clangd/index/Index.h +++ clangd/index/Index.h @@ -267,6 +267,8 @@ return I == SymbolIndex.end() ? nullptr : &Symbols[I->second]; } + llvm::ArrayRef symbols() const { return Symbols; } + // Consumes the builder to finalize the slab. SymbolSlab build() &&; Index: clangd/index/SymbolCollector.h =================================================================== --- clangd/index/SymbolCollector.h +++ clangd/index/SymbolCollector.h @@ -85,6 +85,7 @@ SourceLocation Loc) override; SymbolSlab takeSymbols() { return std::move(Symbols).build(); } + SymbolSlab::Builder takeBuilder() { return std::move(Symbols); } void finish() override; Index: unittests/clangd/FileIndexTests.cpp =================================================================== --- unittests/clangd/FileIndexTests.cpp +++ unittests/clangd/FileIndexTests.cpp @@ -51,15 +51,15 @@ FileSymbols FS; EXPECT_THAT(getSymbolNames(*FS.allSymbols()), UnorderedElementsAre()); - FS.update("f1", numSlab(1, 3)); + FS.updatePreamble("f1", numSlab(1, 3)); EXPECT_THAT(getSymbolNames(*FS.allSymbols()), UnorderedElementsAre("1", "2", "3")); } TEST(FileSymbolsTest, Overlap) { FileSymbols FS; - FS.update("f1", numSlab(1, 3)); - FS.update("f2", numSlab(3, 5)); + FS.updatePreamble("f1", numSlab(1, 3)); + FS.updatePreamble("f2", numSlab(3, 5)); EXPECT_THAT(getSymbolNames(*FS.allSymbols()), UnorderedElementsAre("1", "2", "3", "3", "4", "5")); } @@ -67,12 +67,12 @@ TEST(FileSymbolsTest, SnapshotAliveAfterRemove) { FileSymbols FS; - FS.update("f1", numSlab(1, 3)); + FS.updatePreamble("f1", numSlab(1, 3)); auto Symbols = FS.allSymbols(); EXPECT_THAT(getSymbolNames(*Symbols), UnorderedElementsAre("1", "2", "3")); - FS.update("f1", nullptr); + FS.updatePreamble("f1", nullptr); EXPECT_THAT(getSymbolNames(*FS.allSymbols()), UnorderedElementsAre()); EXPECT_THAT(getSymbolNames(*Symbols), UnorderedElementsAre("1", "2", "3")); } @@ -93,7 +93,7 @@ File.HeaderFilename = (Basename + ".h").str(); File.HeaderCode = Code; auto AST = File.build(); - M.update(File.Filename, &AST.getASTContext(), AST.getPreprocessorPtr()); + M.updatePreamble(File.Filename, &AST.getASTContext(), AST.getPreprocessorPtr()); } TEST(FileIndexTest, CustomizedURIScheme) { @@ -149,13 +149,13 @@ Req.Scopes = {"ns::"}; EXPECT_THAT(match(M, Req), UnorderedElementsAre("ns::f", "ns::X")); - M.update("f1.cpp", nullptr, nullptr); + M.updatePreamble("f1.cpp", nullptr, nullptr); EXPECT_THAT(match(M, Req), UnorderedElementsAre()); } TEST(FileIndexTest, RemoveNonExisting) { FileIndex M; - M.update("no.cpp", nullptr, nullptr); + M.updatePreamble("no.cpp", nullptr, nullptr); EXPECT_THAT(match(M, FuzzyFindRequest()), UnorderedElementsAre()); } @@ -255,7 +255,7 @@ std::shared_ptr PP) { EXPECT_FALSE(IndexUpdated) << "Expected only a single index update"; IndexUpdated = true; - Index.update(FilePath, &Ctx, std::move(PP)); + Index.updatePreamble(FilePath, &Ctx, std::move(PP)); }); ASSERT_TRUE(IndexUpdated); Index: unittests/clangd/TestTU.cpp =================================================================== --- unittests/clangd/TestTU.cpp +++ unittests/clangd/TestTU.cpp @@ -45,7 +45,7 @@ SymbolSlab TestTU::headerSymbols() const { auto AST = build(); - return indexAST(AST.getASTContext(), AST.getPreprocessorPtr()); + return indexPreambleAST(AST.getASTContext(), AST.getPreprocessorPtr()); } std::unique_ptr TestTU::index() const {