ArcsinX updated this revision to Diff 319602.
ArcsinX added a comment.
- Set `IdxContents` at `FileSymbols` object breation instead of at
`FileSymbols::buildIndex()` call.
- Revert change of the preamble index key scheme
- Add comment for `IndexContents`
- `IndexDataKind` => `IndexContents`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94952/new/
https://reviews.llvm.org/D94952
Files:
clang-tools-extra/clangd/index/Background.cpp
clang-tools-extra/clangd/index/FileIndex.cpp
clang-tools-extra/clangd/index/FileIndex.h
clang-tools-extra/clangd/index/Index.cpp
clang-tools-extra/clangd/index/Index.h
clang-tools-extra/clangd/index/MemIndex.cpp
clang-tools-extra/clangd/index/MemIndex.h
clang-tools-extra/clangd/index/Merge.cpp
clang-tools-extra/clangd/index/Merge.h
clang-tools-extra/clangd/index/ProjectAware.cpp
clang-tools-extra/clangd/index/dex/Dex.cpp
clang-tools-extra/clangd/index/dex/Dex.h
clang-tools-extra/clangd/index/remote/Client.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
clang-tools-extra/clangd/unittests/DexTests.cpp
clang-tools-extra/clangd/unittests/IndexTests.cpp
clang-tools-extra/clangd/unittests/RenameTests.cpp
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -1244,9 +1244,9 @@
llvm::function_ref<void(const SymbolID &, const Symbol &)>
Callback) const override {}
- llvm::unique_function<bool(llvm::StringRef) const>
+ llvm::unique_function<IndexContents(llvm::StringRef) const>
indexedFiles() const override {
- return [](llvm::StringRef) { return false; };
+ return [](llvm::StringRef) { return IndexContents::None; };
}
size_t estimateMemoryUsage() const override { return 0; }
@@ -1298,9 +1298,9 @@
llvm::function_ref<void(const SymbolID &, const Symbol &)>)
const override {}
- llvm::unique_function<bool(llvm::StringRef) const>
+ llvm::unique_function<IndexContents(llvm::StringRef) const>
indexedFiles() const override {
- return [](llvm::StringRef) { return false; };
+ return [](llvm::StringRef) { return IndexContents::None; };
}
size_t estimateMemoryUsage() const override { return 0; }
Index: clang-tools-extra/clangd/unittests/IndexTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/IndexTests.cpp
+++ 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) {
Index: clang-tools-extra/clangd/unittests/DexTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DexTests.cpp
+++ 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) {
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1390,9 +1390,9 @@
llvm::function_ref<void(const SymbolID &, const Symbol &)>)
const override {}
- llvm::unique_function<bool(llvm::StringRef) const>
+ llvm::unique_function<IndexContents(llvm::StringRef) const>
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
Index: clang-tools-extra/clangd/index/remote/Client.cpp
===================================================================
--- clang-tools-extra/clangd/index/remote/Client.cpp
+++ clang-tools-extra/clangd/index/remote/Client.cpp
@@ -152,13 +152,13 @@
});
}
- llvm::unique_function<bool(llvm::StringRef) const>
+ llvm::unique_function<IndexContents(llvm::StringRef) const>
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
Index: clang-tools-extra/clangd/index/dex/Dex.h
===================================================================
--- clang-tools-extra/clangd/index/dex/Dex.h
+++ clang-tools-extra/clangd/index/dex/Dex.h
@@ -70,11 +70,13 @@
template <typename SymbolRange, typename RefsRange, typename RelationsRange,
typename FileRange, typename Payload>
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<SymbolRange>(Symbols), std::forward<RefsRange>(Refs),
std::forward<RelationsRange>(Relations),
std::forward<Payload>(BackingData), BackingDataSize) {
this->Files = std::forward<FileRange>(Files);
+ this->IdxContents = IdxContents;
}
/// Builds an index from slabs. The index takes ownership of the slab.
@@ -94,7 +96,7 @@
llvm::function_ref<void(const SymbolID &, const Symbol &)>
Callback) const override;
- llvm::unique_function<bool(llvm::StringRef) const>
+ llvm::unique_function<IndexContents(llvm::StringRef) const>
indexedFiles() const override;
size_t estimateMemoryUsage() const override;
@@ -127,6 +129,8 @@
std::shared_ptr<void> 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;
};
Index: clang-tools-extra/clangd/index/dex/Dex.cpp
===================================================================
--- clang-tools-extra/clangd/index/dex/Dex.cpp
+++ clang-tools-extra/clangd/index/dex/Dex.cpp
@@ -313,14 +313,15 @@
}
}
-llvm::unique_function<bool(llvm::StringRef) const> Dex::indexedFiles() const {
+llvm::unique_function<IndexContents(llvm::StringRef) const>
+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;
};
}
Index: clang-tools-extra/clangd/index/ProjectAware.cpp
===================================================================
--- clang-tools-extra/clangd/index/ProjectAware.cpp
+++ clang-tools-extra/clangd/index/ProjectAware.cpp
@@ -54,7 +54,7 @@
llvm::function_ref<void(const SymbolID &, const Symbol &)>
Callback) const override;
- llvm::unique_function<bool(llvm::StringRef) const>
+ llvm::unique_function<IndexContents(llvm::StringRef) const>
indexedFiles() const override;
ProjectAwareIndex(IndexFactory Gen) : Gen(std::move(Gen)) {}
@@ -115,12 +115,12 @@
return Idx->relations(Req, Callback);
}
-llvm::unique_function<bool(llvm::StringRef) const>
+llvm::unique_function<IndexContents(llvm::StringRef) const>
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 {
Index: clang-tools-extra/clangd/index/Merge.h
===================================================================
--- clang-tools-extra/clangd/index/Merge.h
+++ clang-tools-extra/clangd/index/Merge.h
@@ -41,7 +41,7 @@
void relations(const RelationsRequest &,
llvm::function_ref<void(const SymbolID &, const Symbol &)>)
const override;
- llvm::unique_function<bool(llvm::StringRef) const>
+ llvm::unique_function<IndexContents(llvm::StringRef) const>
indexedFiles() const override;
size_t estimateMemoryUsage() const override {
return Dynamic->estimateMemoryUsage() + Static->estimateMemoryUsage();
Index: clang-tools-extra/clangd/index/Merge.cpp
===================================================================
--- clang-tools-extra/clangd/index/Merge.cpp
+++ 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<bool(llvm::StringRef) const>
+llvm::unique_function<IndexContents(llvm::StringRef) const>
MergedIndex::indexedFiles() const {
return [DynamicContainsFile{Dynamic->indexedFiles()},
StaticContainsFile{Static->indexedFiles()}](llvm::StringRef FileURI) {
- return DynamicContainsFile(FileURI) || StaticContainsFile(FileURI);
+ return DynamicContainsFile(FileURI) | StaticContainsFile(FileURI);
};
}
Index: clang-tools-extra/clangd/index/MemIndex.h
===================================================================
--- clang-tools-extra/clangd/index/MemIndex.h
+++ clang-tools-extra/clangd/index/MemIndex.h
@@ -48,12 +48,14 @@
template <typename SymbolRange, typename RefRange, typename RelationRange,
typename FileRange, typename Payload>
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<SymbolRange>(Symbols),
std::forward<RefRange>(Refs),
std::forward<RelationRange>(Relations),
std::forward<Payload>(BackingData), BackingDataSize) {
this->Files = std::forward<FileRange>(Files);
+ this->IdxContents = IdxContents;
}
/// Builds an index from slabs. The index takes ownership of the data.
@@ -74,7 +76,7 @@
llvm::function_ref<void(const SymbolID &, const Symbol &)>
Callback) const override;
- llvm::unique_function<bool(llvm::StringRef) const>
+ llvm::unique_function<IndexContents(llvm::StringRef) const>
indexedFiles() const override;
size_t estimateMemoryUsage() const override;
@@ -90,6 +92,8 @@
llvm::DenseMap<std::pair<SymbolID, uint8_t>, std::vector<SymbolID>> 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<void> KeepAlive; // poor man's move-only std::any
// Size of memory retained by KeepAlive.
size_t BackingDataSize = 0;
Index: clang-tools-extra/clangd/index/MemIndex.cpp
===================================================================
--- clang-tools-extra/clangd/index/MemIndex.cpp
+++ clang-tools-extra/clangd/index/MemIndex.cpp
@@ -109,15 +109,15 @@
}
}
-llvm::unique_function<bool(llvm::StringRef) const>
+llvm::unique_function<IndexContents(llvm::StringRef) const>
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;
};
}
Index: clang-tools-extra/clangd/index/Index.h
===================================================================
--- clang-tools-extra/clangd/index/Index.h
+++ clang-tools-extra/clangd/index/Index.h
@@ -82,6 +82,30 @@
llvm::Optional<uint32_t> 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<IndexContents>(static_cast<uint8_t>(L) &
+ static_cast<uint8_t>(R));
+}
+
+inline constexpr IndexContents operator|(IndexContents L, IndexContents R) {
+ return static_cast<IndexContents>(static_cast<uint8_t>(L) |
+ static_cast<uint8_t>(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 {
@@ -124,7 +148,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<bool(llvm::StringRef) const>
+ virtual llvm::unique_function<IndexContents(llvm::StringRef) const>
indexedFiles() const = 0;
/// Returns estimated size of index (in bytes).
@@ -151,7 +175,7 @@
llvm::function_ref<void(const SymbolID &, const Symbol &)>)
const override;
- llvm::unique_function<bool(llvm::StringRef) const>
+ llvm::unique_function<IndexContents(llvm::StringRef) const>
indexedFiles() const override;
size_t estimateMemoryUsage() const override;
Index: clang-tools-extra/clangd/index/Index.cpp
===================================================================
--- clang-tools-extra/clangd/index/Index.cpp
+++ clang-tools-extra/clangd/index/Index.cpp
@@ -76,7 +76,7 @@
return snapshot()->relations(R, CB);
}
-llvm::unique_function<bool(llvm::StringRef) const>
+llvm::unique_function<IndexContents(llvm::StringRef) const>
SwapIndex::indexedFiles() const {
// The index snapshot should outlive this method return value.
auto SnapShot = snapshot();
Index: clang-tools-extra/clangd/index/FileIndex.h
===================================================================
--- clang-tools-extra/clangd/index/FileIndex.h
+++ 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 = IndexContents::None);
/// 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<RefSlab> Slab;
bool CountReferences = false;
Index: clang-tools-extra/clangd/index/FileIndex.cpp
===================================================================
--- clang-tools-extra/clangd/index/FileIndex.cpp
+++ 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<SymbolSlab> Symbols,
std::unique_ptr<RefSlab> Refs,
@@ -376,14 +379,14 @@
case IndexType::Light:
return std::make_unique<MemIndex>(
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<dex::Dex>(
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);
@@ -413,7 +416,9 @@
FileIndex::FileIndex(bool UseDex, bool CollectMainFileRefs)
: MergedIndex(&MainFileIndex, &PreambleIndex), UseDex(UseDex),
CollectMainFileRefs(CollectMainFileRefs),
+ PreambleSymbols(IndexContents::Symbols),
PreambleIndex(std::make_unique<MemIndex>()),
+ MainFileSymbols(IndexContents::All),
MainFileIndex(std::make_unique<MemIndex>()) {}
void FileIndex::updatePreamble(PathRef Path, llvm::StringRef Version,
Index: clang-tools-extra/clangd/index/Background.cpp
===================================================================
--- clang-tools-extra/clangd/index/Background.cpp
+++ clang-tools-extra/clangd/index/Background.cpp
@@ -98,6 +98,7 @@
: SwapIndex(std::make_unique<MemIndex>()), TFS(TFS), CDB(CDB),
ContextProvider(std::move(Opts.ContextProvider)),
CollectMainFileRefs(Opts.CollectMainFileRefs),
+ IndexedSymbols(IndexContents::All),
Rebuilder(this, &IndexedSymbols, Opts.ThreadPoolSize),
IndexStorageFactory(std::move(IndexStorageFactory)),
Queue(std::move(Opts.OnProgress)),
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
