diff --git a/clang-tools-extra/clangd/index/Background.cpp b/clang-tools-extra/clangd/index/Background.cpp --- a/clang-tools-extra/clangd/index/Background.cpp +++ b/clang-tools-extra/clangd/index/Background.cpp @@ -97,6 +97,7 @@ BackgroundIndexStorage::Factory IndexStorageFactory, Options Opts) : SwapIndex(std::make_unique()), TFS(TFS), CDB(CDB), ContextProvider(std::move(Opts.ContextProvider)), + IndexedSymbols(IndexContents::All), Rebuilder(this, &IndexedSymbols, Opts.ThreadPoolSize), IndexStorageFactory(std::move(IndexStorageFactory)), Queue(std::move(Opts.OnProgress)), diff --git a/clang-tools-extra/clangd/index/FileIndex.h b/clang-tools-extra/clangd/index/FileIndex.h --- a/clang-tools-extra/clangd/index/FileIndex.h +++ b/clang-tools-extra/clangd/index/FileIndex.h @@ -71,6 +71,7 @@ /// locking when we swap or obtain references to snapshots. class FileSymbols { public: + FileSymbols(IndexContents IdxContents); /// Updates all slabs associated with the \p Key. /// If either is nullptr, corresponding data for \p Key will be removed. /// If CountReferences is true, \p Refs will be used for counting references @@ -91,6 +92,8 @@ void profile(MemoryTree &MT) const; private: + IndexContents IdxContents; + struct RefSlabAndCountReferences { std::shared_ptr Slab; bool CountReferences = false; diff --git a/clang-tools-extra/clangd/index/FileIndex.cpp b/clang-tools-extra/clangd/index/FileIndex.cpp --- a/clang-tools-extra/clangd/index/FileIndex.cpp +++ b/clang-tools-extra/clangd/index/FileIndex.cpp @@ -236,6 +236,9 @@ /*CollectMainFileRefs=*/false); } +FileSymbols::FileSymbols(IndexContents IdxContents) + : IdxContents(IdxContents) {} + void FileSymbols::update(llvm::StringRef Key, std::unique_ptr Symbols, std::unique_ptr Refs, @@ -376,14 +379,14 @@ case IndexType::Light: return std::make_unique( llvm::make_pointee_range(AllSymbols), std::move(AllRefs), - std::move(AllRelations), std::move(Files), + std::move(AllRelations), std::move(Files), IdxContents, std::make_tuple(std::move(SymbolSlabs), std::move(RefSlabs), std::move(RefsStorage), std::move(SymsStorage)), StorageSize); case IndexType::Heavy: return std::make_unique( llvm::make_pointee_range(AllSymbols), std::move(AllRefs), - std::move(AllRelations), std::move(Files), + std::move(AllRelations), std::move(Files), IdxContents, std::make_tuple(std::move(SymbolSlabs), std::move(RefSlabs), std::move(RefsStorage), std::move(SymsStorage)), StorageSize); @@ -412,7 +415,9 @@ FileIndex::FileIndex() : MergedIndex(&MainFileIndex, &PreambleIndex), + PreambleSymbols(IndexContents::Symbols | IndexContents::Relations), PreambleIndex(std::make_unique()), + MainFileSymbols(IndexContents::All), MainFileIndex(std::make_unique()) {} void FileIndex::updatePreamble(PathRef Path, llvm::StringRef Version, diff --git a/clang-tools-extra/clangd/index/Index.h b/clang-tools-extra/clangd/index/Index.h --- a/clang-tools-extra/clangd/index/Index.h +++ b/clang-tools-extra/clangd/index/Index.h @@ -82,6 +82,30 @@ llvm::Optional Limit; }; +/// Describes what data is covered by an index. +/// +/// Indexes may contain symbols but not references from a file, etc. +/// This affects merging: if a staler index contains a reference but a fresher +/// one does not, we want to trust the fresher index *only* if it actually +/// includes references in general. +enum class IndexContents : uint8_t { + None = 0, + Symbols = 1 << 1, + References = 1 << 2, + Relations = 1 << 3, + All = Symbols | References | Relations +}; + +inline constexpr IndexContents operator&(IndexContents L, IndexContents R) { + return static_cast(static_cast(L) & + static_cast(R)); +} + +inline constexpr IndexContents operator|(IndexContents L, IndexContents R) { + return static_cast(static_cast(L) | + static_cast(R)); +} + /// Interface for symbol indexes that can be used for searching or /// matching symbols among a set of symbols based on names or unique IDs. class SymbolIndex { @@ -125,7 +149,7 @@ /// Returns function which checks if the specified file was used to build this /// index or not. The function must only be called while the index is alive. - virtual llvm::unique_function + virtual llvm::unique_function indexedFiles() const = 0; /// Returns estimated size of index (in bytes). @@ -152,7 +176,7 @@ llvm::function_ref) const override; - llvm::unique_function + llvm::unique_function indexedFiles() const override; size_t estimateMemoryUsage() const override; diff --git a/clang-tools-extra/clangd/index/Index.cpp b/clang-tools-extra/clangd/index/Index.cpp --- a/clang-tools-extra/clangd/index/Index.cpp +++ b/clang-tools-extra/clangd/index/Index.cpp @@ -76,7 +76,7 @@ return snapshot()->relations(R, CB); } -llvm::unique_function +llvm::unique_function SwapIndex::indexedFiles() const { // The index snapshot should outlive this method return value. auto SnapShot = snapshot(); diff --git a/clang-tools-extra/clangd/index/MemIndex.h b/clang-tools-extra/clangd/index/MemIndex.h --- a/clang-tools-extra/clangd/index/MemIndex.h +++ b/clang-tools-extra/clangd/index/MemIndex.h @@ -48,12 +48,14 @@ template MemIndex(SymbolRange &&Symbols, RefRange &&Refs, RelationRange &&Relations, - FileRange &&Files, Payload &&BackingData, size_t BackingDataSize) + FileRange &&Files, IndexContents IdxContents, Payload &&BackingData, + size_t BackingDataSize) : MemIndex(std::forward(Symbols), std::forward(Refs), std::forward(Relations), std::forward(BackingData), BackingDataSize) { this->Files = std::forward(Files); + this->IdxContents = IdxContents; } /// Builds an index from slabs. The index takes ownership of the data. @@ -74,7 +76,7 @@ llvm::function_ref Callback) const override; - llvm::unique_function + llvm::unique_function indexedFiles() const override; size_t estimateMemoryUsage() const override; @@ -90,6 +92,8 @@ llvm::DenseMap, std::vector> Relations; // Set of files which were used during this index build. llvm::StringSet<> Files; + // Contents of the index (symbols, references, etc.) + IndexContents IdxContents; std::shared_ptr KeepAlive; // poor man's move-only std::any // Size of memory retained by KeepAlive. size_t BackingDataSize = 0; diff --git a/clang-tools-extra/clangd/index/MemIndex.cpp b/clang-tools-extra/clangd/index/MemIndex.cpp --- a/clang-tools-extra/clangd/index/MemIndex.cpp +++ b/clang-tools-extra/clangd/index/MemIndex.cpp @@ -109,15 +109,15 @@ } } -llvm::unique_function +llvm::unique_function MemIndex::indexedFiles() const { return [this](llvm::StringRef FileURI) { auto Path = URI::resolve(FileURI); if (!Path) { llvm::consumeError(Path.takeError()); - return false; + return IndexContents::None; } - return Files.contains(*Path); + return Files.contains(*Path) ? IdxContents : IndexContents::None; }; } diff --git a/clang-tools-extra/clangd/index/Merge.h b/clang-tools-extra/clangd/index/Merge.h --- a/clang-tools-extra/clangd/index/Merge.h +++ b/clang-tools-extra/clangd/index/Merge.h @@ -41,7 +41,7 @@ void relations(const RelationsRequest &, llvm::function_ref) const override; - llvm::unique_function + llvm::unique_function indexedFiles() const override; size_t estimateMemoryUsage() const override { return Dynamic->estimateMemoryUsage() + Static->estimateMemoryUsage(); diff --git a/clang-tools-extra/clangd/index/Merge.cpp b/clang-tools-extra/clangd/index/Merge.cpp --- a/clang-tools-extra/clangd/index/Merge.cpp +++ b/clang-tools-extra/clangd/index/Merge.cpp @@ -49,8 +49,9 @@ More |= Static->fuzzyFind(Req, [&](const Symbol &S) { // We expect the definition to see the canonical declaration, so it seems // to be enough to check only the definition if it exists. - if (DynamicContainsFile(S.Definition ? S.Definition.FileURI - : S.CanonicalDeclaration.FileURI)) + if ((DynamicContainsFile(S.Definition ? S.Definition.FileURI + : S.CanonicalDeclaration.FileURI) & + IndexContents::Symbols) != IndexContents::None) return; auto DynS = Dyn.find(S.ID); ++StaticCount; @@ -84,8 +85,9 @@ Static->lookup(Req, [&](const Symbol &S) { // We expect the definition to see the canonical declaration, so it seems // to be enough to check only the definition if it exists. - if (DynamicContainsFile(S.Definition ? S.Definition.FileURI - : S.CanonicalDeclaration.FileURI)) + if ((DynamicContainsFile(S.Definition ? S.Definition.FileURI + : S.CanonicalDeclaration.FileURI) & + IndexContents::Symbols) != IndexContents::None) return; const Symbol *Sym = B.find(S.ID); RemainingIDs.erase(S.ID); @@ -121,7 +123,8 @@ // We return less than Req.Limit if static index returns more refs for dirty // files. bool StaticHadMore = Static->refs(Req, [&](const Ref &O) { - if (DynamicContainsFile(O.Location.FileURI)) + if ((DynamicContainsFile(O.Location.FileURI) & IndexContents::References) != + IndexContents::None) return; // ignore refs that have been seen from dynamic index. if (Remaining == 0) { More = true; @@ -133,11 +136,11 @@ return More || StaticHadMore; } -llvm::unique_function +llvm::unique_function MergedIndex::indexedFiles() const { return [DynamicContainsFile{Dynamic->indexedFiles()}, StaticContainsFile{Static->indexedFiles()}](llvm::StringRef FileURI) { - return DynamicContainsFile(FileURI) || StaticContainsFile(FileURI); + return DynamicContainsFile(FileURI) | StaticContainsFile(FileURI); }; } diff --git a/clang-tools-extra/clangd/index/ProjectAware.cpp b/clang-tools-extra/clangd/index/ProjectAware.cpp --- a/clang-tools-extra/clangd/index/ProjectAware.cpp +++ b/clang-tools-extra/clangd/index/ProjectAware.cpp @@ -54,7 +54,7 @@ llvm::function_ref Callback) const override; - llvm::unique_function + llvm::unique_function indexedFiles() const override; ProjectAwareIndex(IndexFactory Gen) : Gen(std::move(Gen)) {} @@ -115,12 +115,12 @@ return Idx->relations(Req, Callback); } -llvm::unique_function +llvm::unique_function ProjectAwareIndex::indexedFiles() const { trace::Span Tracer("ProjectAwareIndex::indexedFiles"); if (auto *Idx = getIndex()) return Idx->indexedFiles(); - return [](llvm::StringRef) { return false; }; + return [](llvm::StringRef) { return IndexContents::None; }; } SymbolIndex *ProjectAwareIndex::getIndex() const { diff --git a/clang-tools-extra/clangd/index/dex/Dex.h b/clang-tools-extra/clangd/index/dex/Dex.h --- a/clang-tools-extra/clangd/index/dex/Dex.h +++ b/clang-tools-extra/clangd/index/dex/Dex.h @@ -70,11 +70,13 @@ template Dex(SymbolRange &&Symbols, RefsRange &&Refs, RelationsRange &&Relations, - FileRange &&Files, Payload &&BackingData, size_t BackingDataSize) + FileRange &&Files, IndexContents IdxContents, Payload &&BackingData, + size_t BackingDataSize) : Dex(std::forward(Symbols), std::forward(Refs), std::forward(Relations), std::forward(BackingData), BackingDataSize) { this->Files = std::forward(Files); + this->IdxContents = IdxContents; } /// Builds an index from slabs. The index takes ownership of the slab. @@ -94,7 +96,7 @@ llvm::function_ref Callback) const override; - llvm::unique_function + llvm::unique_function indexedFiles() const override; size_t estimateMemoryUsage() const override; @@ -127,6 +129,8 @@ std::shared_ptr KeepAlive; // poor man's move-only std::any // Set of files which were used during this index build. llvm::StringSet<> Files; + // Contents of the index (symbols, references, etc.) + IndexContents IdxContents; // Size of memory retained by KeepAlive. size_t BackingDataSize = 0; }; diff --git a/clang-tools-extra/clangd/index/dex/Dex.cpp b/clang-tools-extra/clangd/index/dex/Dex.cpp --- a/clang-tools-extra/clangd/index/dex/Dex.cpp +++ b/clang-tools-extra/clangd/index/dex/Dex.cpp @@ -313,14 +313,15 @@ } } -llvm::unique_function Dex::indexedFiles() const { +llvm::unique_function +Dex::indexedFiles() const { return [this](llvm::StringRef FileURI) { auto Path = URI::resolve(FileURI); if (!Path) { llvm::consumeError(Path.takeError()); - return false; + return IndexContents::None; } - return Files.contains(*Path); + return Files.contains(*Path) ? IdxContents : IndexContents::None; }; } diff --git a/clang-tools-extra/clangd/index/remote/Client.cpp b/clang-tools-extra/clangd/index/remote/Client.cpp --- a/clang-tools-extra/clangd/index/remote/Client.cpp +++ b/clang-tools-extra/clangd/index/remote/Client.cpp @@ -152,13 +152,13 @@ }); } - llvm::unique_function + llvm::unique_function indexedFiles() const override { - // FIXME: For now we always return "false" regardless of whether the file - // was indexed or not. A possible implementation could be based on - // the idea that we do not want to send a request at every + // FIXME: For now we always return IndexContents::None regardless of whether + // the file was indexed or not. A possible implementation could be + // based on the idea that we do not want to send a request at every // call of a function returned by IndexClient::indexedFiles(). - return [](llvm::StringRef) { return false; }; + return [](llvm::StringRef) { return IndexContents::None; }; } // IndexClient does not take any space since the data is stored on the diff --git a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp --- a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp +++ b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp @@ -683,7 +683,7 @@ class BackgroundIndexRebuilderTest : public testing::Test { protected: BackgroundIndexRebuilderTest() - : Target(std::make_unique()), + : Source(IndexContents::All), Target(std::make_unique()), Rebuilder(&Target, &Source, /*Threads=*/10) { // Prepare FileSymbols with TestSymbol in it, for checkRebuild. TestSymbol.ID = SymbolID("foo"); diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -1390,9 +1390,9 @@ llvm::function_ref) const override {} - llvm::unique_function + llvm::unique_function indexedFiles() const override { - return [](llvm::StringRef) { return false; }; + return [](llvm::StringRef) { return IndexContents::None; }; } // This is incorrect, but IndexRequestCollector is not an actual index and it diff --git a/clang-tools-extra/clangd/unittests/DexTests.cpp b/clang-tools-extra/clangd/unittests/DexTests.cpp --- a/clang-tools-extra/clangd/unittests/DexTests.cpp +++ b/clang-tools-extra/clangd/unittests/DexTests.cpp @@ -739,11 +739,11 @@ auto Data = std::make_pair(std::move(Symbols), std::move(Refs)); llvm::StringSet<> Files = {testPath("foo.cc"), testPath("bar.cc")}; Dex I(std::move(Data.first), std::move(Data.second), RelationSlab(), - std::move(Files), std::move(Data), Size); + std::move(Files), IndexContents::All, std::move(Data), Size); auto ContainsFile = I.indexedFiles(); - EXPECT_TRUE(ContainsFile("unittest:///foo.cc")); - EXPECT_TRUE(ContainsFile("unittest:///bar.cc")); - EXPECT_FALSE(ContainsFile("unittest:///foobar.cc")); + EXPECT_EQ(ContainsFile("unittest:///foo.cc"), IndexContents::All); + EXPECT_EQ(ContainsFile("unittest:///bar.cc"), IndexContents::All); + EXPECT_EQ(ContainsFile("unittest:///foobar.cc"), IndexContents::None); } TEST(DexTest, PreferredTypesBoosting) { diff --git a/clang-tools-extra/clangd/unittests/FileIndexTests.cpp b/clang-tools-extra/clangd/unittests/FileIndexTests.cpp --- a/clang-tools-extra/clangd/unittests/FileIndexTests.cpp +++ b/clang-tools-extra/clangd/unittests/FileIndexTests.cpp @@ -102,7 +102,7 @@ } TEST(FileSymbolsTest, UpdateAndGet) { - FileSymbols FS; + FileSymbols FS(IndexContents::All); EXPECT_THAT(runFuzzyFind(*FS.buildIndex(IndexType::Light), ""), IsEmpty()); FS.update("f1", numSlab(1, 3), refSlab(SymbolID("1"), "f1.cc"), nullptr, @@ -114,7 +114,7 @@ } TEST(FileSymbolsTest, Overlap) { - FileSymbols FS; + FileSymbols FS(IndexContents::All); FS.update("f1", numSlab(1, 3), nullptr, nullptr, false); FS.update("f2", numSlab(3, 5), nullptr, nullptr, false); for (auto Type : {IndexType::Light, IndexType::Heavy}) @@ -124,7 +124,7 @@ } TEST(FileSymbolsTest, MergeOverlap) { - FileSymbols FS; + FileSymbols FS(IndexContents::All); auto OneSymboSlab = [](Symbol Sym) { SymbolSlab::Builder S; S.insert(Sym); @@ -145,7 +145,7 @@ } TEST(FileSymbolsTest, SnapshotAliveAfterRemove) { - FileSymbols FS; + FileSymbols FS(IndexContents::All); SymbolID ID("1"); FS.update("f1", numSlab(1, 3), refSlab(ID, "f1.cc"), nullptr, false); @@ -495,7 +495,7 @@ } TEST(FileSymbolsTest, CountReferencesNoRefSlabs) { - FileSymbols FS; + FileSymbols FS(IndexContents::All); FS.update("f1", numSlab(1, 3), nullptr, nullptr, true); FS.update("f2", numSlab(1, 3), nullptr, nullptr, false); EXPECT_THAT( @@ -507,7 +507,7 @@ } TEST(FileSymbolsTest, CountReferencesWithRefSlabs) { - FileSymbols FS; + FileSymbols FS(IndexContents::All); FS.update("f1cpp", numSlab(1, 3), refSlab(SymbolID("1"), "f1.cpp"), nullptr, true); FS.update("f1h", numSlab(1, 3), refSlab(SymbolID("1"), "f1.h"), nullptr, @@ -709,7 +709,7 @@ } TEST(FileSymbolsTest, Profile) { - FileSymbols FS; + FileSymbols FS(IndexContents::All); FS.update("f1", numSlab(1, 2), nullptr, nullptr, false); FS.update("f2", nullptr, refSlab(SymbolID("1"), "f1"), nullptr, false); FS.update("f3", nullptr, nullptr, diff --git a/clang-tools-extra/clangd/unittests/IndexTests.cpp b/clang-tools-extra/clangd/unittests/IndexTests.cpp --- a/clang-tools-extra/clangd/unittests/IndexTests.cpp +++ b/clang-tools-extra/clangd/unittests/IndexTests.cpp @@ -231,11 +231,11 @@ auto Data = std::make_pair(std::move(Symbols), std::move(Refs)); llvm::StringSet<> Files = {testPath("foo.cc"), testPath("bar.cc")}; MemIndex I(std::move(Data.first), std::move(Data.second), RelationSlab(), - std::move(Files), std::move(Data), Size); + std::move(Files), IndexContents::All, std::move(Data), Size); auto ContainsFile = I.indexedFiles(); - EXPECT_TRUE(ContainsFile("unittest:///foo.cc")); - EXPECT_TRUE(ContainsFile("unittest:///bar.cc")); - EXPECT_FALSE(ContainsFile("unittest:///foobar.cc")); + EXPECT_EQ(ContainsFile("unittest:///foo.cc"), IndexContents::All); + EXPECT_EQ(ContainsFile("unittest:///bar.cc"), IndexContents::All); + EXPECT_EQ(ContainsFile("unittest:///foobar.cc"), IndexContents::None); } TEST(MemIndexTest, TemplateSpecialization) { @@ -508,23 +508,24 @@ auto DynData = std::make_pair(std::move(DynSymbols), std::move(DynRefs)); llvm::StringSet<> DynFiles = {testPath("foo.cc")}; MemIndex DynIndex(std::move(DynData.first), std::move(DynData.second), - RelationSlab(), std::move(DynFiles), std::move(DynData), - DynSize); + RelationSlab(), std::move(DynFiles), IndexContents::Symbols, + std::move(DynData), DynSize); SymbolSlab StaticSymbols; RefSlab StaticRefs; auto StaticData = std::make_pair(std::move(StaticSymbols), std::move(StaticRefs)); - llvm::StringSet<> StaticFiles = {testPath("bar.cc")}; - MemIndex StaticIndex(std::move(StaticData.first), - std::move(StaticData.second), RelationSlab(), - std::move(StaticFiles), std::move(StaticData), - StaticSymbols.bytes() + StaticRefs.bytes()); + llvm::StringSet<> StaticFiles = {testPath("foo.cc"), testPath("bar.cc")}; + MemIndex StaticIndex( + std::move(StaticData.first), std::move(StaticData.second), RelationSlab(), + std::move(StaticFiles), IndexContents::References, std::move(StaticData), + StaticSymbols.bytes() + StaticRefs.bytes()); MergedIndex Merge(&DynIndex, &StaticIndex); auto ContainsFile = Merge.indexedFiles(); - EXPECT_TRUE(ContainsFile("unittest:///foo.cc")); - EXPECT_TRUE(ContainsFile("unittest:///bar.cc")); - EXPECT_FALSE(ContainsFile("unittest:///foobar.cc")); + EXPECT_EQ(ContainsFile("unittest:///foo.cc"), + IndexContents::Symbols | IndexContents::References); + EXPECT_EQ(ContainsFile("unittest:///bar.cc"), IndexContents::References); + EXPECT_EQ(ContainsFile("unittest:///foobar.cc"), IndexContents::None); } TEST(MergeIndexTest, NonDocumentation) { diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp --- a/clang-tools-extra/clangd/unittests/RenameTests.cpp +++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp @@ -1290,7 +1290,7 @@ std::string BarPath = testPath("bar.cc"); // Build the index, the index has "Foo" references from foo.cc and "Bar" // references from bar.cc. - FileSymbols FSymbols; + FileSymbols FSymbols(IndexContents::All); FSymbols.update(FooPath, nullptr, buildRefSlab(FooCode, "Foo", FooPath), nullptr, false); FSymbols.update(BarPath, nullptr, buildRefSlab(BarCode, "Bar", BarPath), @@ -1367,9 +1367,9 @@ llvm::function_ref Callback) const override {} - llvm::unique_function + llvm::unique_function indexedFiles() const override { - return [](llvm::StringRef) { return false; }; + return [](llvm::StringRef) { return IndexContents::None; }; } size_t estimateMemoryUsage() const override { return 0; } @@ -1421,9 +1421,9 @@ llvm::function_ref) const override {} - llvm::unique_function + llvm::unique_function indexedFiles() const override { - return [](llvm::StringRef) { return false; }; + return [](llvm::StringRef) { return IndexContents::None; }; } size_t estimateMemoryUsage() const override { return 0; }