ArcsinX created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman.
ArcsinX requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.
Without this patch the file list of the index is derived from the keys, but
initial `FileSymbols` design was that the keys are arbitrary, so we can not
assume that the keys are always paths/URIs (e.g. the preamble index uses URIs
for the keys and other indexes use file paths).
This patch introduces the file list extraction from the index data, thus it
makes `FileSymbols` usage more flexible (we do not need to think about the
keys) and helps to avoid a lot of URI <=> path conversions, but from the other
hand the index file list could be empty if there are no symbols/references
(empty files seems to be a very rare case).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D97535
Files:
clang-tools-extra/clangd/index/FileIndex.cpp
clang-tools-extra/clangd/index/MemIndex.cpp
clang-tools-extra/clangd/index/dex/Dex.cpp
clang-tools-extra/clangd/unittests/DexTests.cpp
clang-tools-extra/clangd/unittests/IndexTests.cpp
Index: clang-tools-extra/clangd/unittests/IndexTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/IndexTests.cpp
+++ clang-tools-extra/clangd/unittests/IndexTests.cpp
@@ -229,7 +229,7 @@
RefSlab Refs;
auto Size = Symbols.bytes() + Refs.bytes();
auto Data = std::make_pair(std::move(Symbols), std::move(Refs));
- llvm::StringSet<> Files = {testPath("foo.cc"), testPath("bar.cc")};
+ llvm::StringSet<> Files = {"unittest:///foo.cc", "unittest:///bar.cc"};
MemIndex I(std::move(Data.first), std::move(Data.second), RelationSlab(),
std::move(Files), IndexContents::All, std::move(Data), Size);
auto ContainsFile = I.indexedFiles();
@@ -339,21 +339,21 @@
FileIndex DynamicIndex, StaticIndex;
MergedIndex Merge(&DynamicIndex, &StaticIndex);
- const char *HeaderCode = "class Foo;";
+ const char *HeaderCode = "class Foo; class Bar;";
auto HeaderSymbols = TestTU::withHeaderCode(HeaderCode).headerSymbols();
auto Foo = findSymbol(HeaderSymbols, "Foo");
- // Build static index for test.cc with Foo symbol
+ // Build static index for test.cc with Foo and Bar symbols
TestTU Test;
Test.HeaderCode = HeaderCode;
- Test.Code = "class Foo {};";
+ Test.Code = "class Foo {}; class Bar {};";
Test.Filename = "test.cc";
auto AST = Test.build();
StaticIndex.updateMain(testPath(Test.Filename), AST);
- // Remove Foo symbol, i.e. build dynamic index for test.cc, which is empty.
- Test.HeaderCode = "";
- Test.Code = "";
+ // Remove Foo symbol.
+ Test.HeaderCode = "class Bar;";
+ Test.Code = "class Bar {};";
AST = Test.build();
DynamicIndex.updateMain(testPath(Test.Filename), AST);
@@ -487,7 +487,7 @@
// Remove all refs for test.cc from dynamic index,
// merged index should not return results from static index for test.cc.
- Test.Code = "";
+ Test.Code = "int I;"; // test.cc should not be empty
AST = Test.build();
Dyn.updateMain(testPath(Test.Filename), AST);
@@ -506,7 +506,7 @@
RefSlab DynRefs;
auto DynSize = DynSymbols.bytes() + DynRefs.bytes();
auto DynData = std::make_pair(std::move(DynSymbols), std::move(DynRefs));
- llvm::StringSet<> DynFiles = {testPath("foo.cc")};
+ llvm::StringSet<> DynFiles = {"unittest:///foo.cc"};
MemIndex DynIndex(std::move(DynData.first), std::move(DynData.second),
RelationSlab(), std::move(DynFiles), IndexContents::Symbols,
std::move(DynData), DynSize);
@@ -514,7 +514,7 @@
RefSlab StaticRefs;
auto StaticData =
std::make_pair(std::move(StaticSymbols), std::move(StaticRefs));
- llvm::StringSet<> StaticFiles = {testPath("foo.cc"), testPath("bar.cc")};
+ llvm::StringSet<> StaticFiles = {"unittest:///foo.cc", "unittest:///bar.cc"};
MemIndex StaticIndex(
std::move(StaticData.first), std::move(StaticData.second), RelationSlab(),
std::move(StaticFiles), IndexContents::References, std::move(StaticData),
Index: clang-tools-extra/clangd/unittests/DexTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DexTests.cpp
+++ clang-tools-extra/clangd/unittests/DexTests.cpp
@@ -737,7 +737,7 @@
RefSlab Refs;
auto Size = Symbols.bytes() + Refs.bytes();
auto Data = std::make_pair(std::move(Symbols), std::move(Refs));
- llvm::StringSet<> Files = {testPath("foo.cc"), testPath("bar.cc")};
+ llvm::StringSet<> Files = {"unittest:///foo.cc", "unittest:///bar.cc"};
Dex I(std::move(Data.first), std::move(Data.second), RelationSlab(),
std::move(Files), IndexContents::All, std::move(Data), Size);
auto ContainsFile = I.indexedFiles();
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
@@ -316,14 +316,7 @@
llvm::unique_function<IndexContents(llvm::StringRef) const>
Dex::indexedFiles() const {
return [this](llvm::StringRef FileURI) {
- if (Files.empty())
- return IndexContents::None;
- auto Path = URI::resolve(FileURI, Files.begin()->first());
- if (!Path) {
- vlog("Failed to resolve the URI {0} : {1}", FileURI, Path.takeError());
- return IndexContents::None;
- }
- return Files.contains(*Path) ? IdxContents : IndexContents::None;
+ return Files.contains(FileURI) ? IdxContents : IndexContents::None;
};
}
Index: clang-tools-extra/clangd/index/MemIndex.cpp
===================================================================
--- clang-tools-extra/clangd/index/MemIndex.cpp
+++ clang-tools-extra/clangd/index/MemIndex.cpp
@@ -112,14 +112,7 @@
llvm::unique_function<IndexContents(llvm::StringRef) const>
MemIndex::indexedFiles() const {
return [this](llvm::StringRef FileURI) {
- if (Files.empty())
- return IndexContents::None;
- auto Path = URI::resolve(FileURI, Files.begin()->first());
- if (!Path) {
- vlog("Failed to resolve the URI {0} : {1}", FileURI, Path.takeError());
- return IndexContents::None;
- }
- return Files.contains(*Path) ? IdxContents : IndexContents::None;
+ return Files.contains(FileURI) ? IdxContents : IndexContents::None;
};
}
Index: clang-tools-extra/clangd/index/FileIndex.cpp
===================================================================
--- clang-tools-extra/clangd/index/FileIndex.cpp
+++ clang-tools-extra/clangd/index/FileIndex.cpp
@@ -276,6 +276,12 @@
std::lock_guard<std::mutex> Lock(Mutex);
for (const auto &FileAndSymbols : SymbolsSnapshot) {
SymbolSlabs.push_back(FileAndSymbols.second);
+ for (const auto &S : *FileAndSymbols.second) {
+ if (S.Definition)
+ Files.insert(S.Definition.FileURI);
+ else
+ Files.insert(S.CanonicalDeclaration.FileURI);
+ }
Files.insert(FileAndSymbols.first());
}
for (const auto &FileAndRefs : RefsSnapshot) {
@@ -283,11 +289,13 @@
Files.insert(FileAndRefs.first());
if (FileAndRefs.second.CountReferences)
MainFileRefs.push_back(RefSlabs.back().get());
+ for (const auto &S : *FileAndRefs.second.Slab) {
+ for (const auto &R : S.second)
+ Files.insert(R.Location.FileURI);
+ }
}
- for (const auto &FileAndRelations : RelationsSnapshot) {
- Files.insert(FileAndRelations.first());
+ for (const auto &FileAndRelations : RelationsSnapshot)
RelationSlabs.push_back(FileAndRelations.second);
- }
if (Version)
*Version = this->Version;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
