Author: Aleksandr Platonov Date: 2021-03-06T10:47:05+03:00 New Revision: c4efd04f18c7e10c11de4a790f4d0c42f694d49b
URL: https://github.com/llvm/llvm-project/commit/c4efd04f18c7e10c11de4a790f4d0c42f694d49b DIFF: https://github.com/llvm/llvm-project/commit/c4efd04f18c7e10c11de4a790f4d0c42f694d49b.diff LOG: [clangd] Use URIs instead of paths in the index file list Without this patch the file list of the preamble index contains URIs, but other indexes file lists contain file paths. This makes `indexedFiles()` always returns `IndexContents::None` for the preamble index, because current implementation expects file paths inside the file list of the index. This patch fixes this problem and also helps to avoid a lot of URI to path conversions during indexes merge. Reviewed By: kadircet Differential Revision: https://reviews.llvm.org/D97535 Added: Modified: clang-tools-extra/clangd/index/Background.cpp 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/test/memory_tree.test clang-tools-extra/clangd/unittests/DexTests.cpp clang-tools-extra/clangd/unittests/FileIndexTests.cpp clang-tools-extra/clangd/unittests/IndexTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/index/Background.cpp b/clang-tools-extra/clangd/index/Background.cpp index f97f13d8dabe..ddfe962d3189 100644 --- a/clang-tools-extra/clangd/index/Background.cpp +++ b/clang-tools-extra/clangd/index/Background.cpp @@ -243,7 +243,7 @@ void BackgroundIndex::update( // this thread sees the older version but finishes later. This should be // rare in practice. IndexedSymbols.update( - Path, std::make_unique<SymbolSlab>(std::move(*IF->Symbols)), + Uri, std::make_unique<SymbolSlab>(std::move(*IF->Symbols)), std::make_unique<RefSlab>(std::move(*IF->Refs)), std::make_unique<RelationSlab>(std::move(*IF->Relations)), Path == MainFile); @@ -390,8 +390,9 @@ BackgroundIndex::loadProject(std::vector<std::string> MainFiles) { SV.HadErrors = LS.HadErrors; ++LoadedShards; - IndexedSymbols.update(LS.AbsolutePath, std::move(SS), std::move(RS), - std::move(RelS), LS.CountReferences); + IndexedSymbols.update(URI::create(LS.AbsolutePath).toString(), + std::move(SS), std::move(RS), std::move(RelS), + LS.CountReferences); } } Rebuilder.loadedShard(LoadedShards); diff --git a/clang-tools-extra/clangd/index/FileIndex.cpp b/clang-tools-extra/clangd/index/FileIndex.cpp index 528630f9232a..b91c66b88770 100644 --- a/clang-tools-extra/clangd/index/FileIndex.cpp +++ b/clang-tools-extra/clangd/index/FileIndex.cpp @@ -463,7 +463,8 @@ void FileIndex::updatePreamble(PathRef Path, llvm::StringRef Version, void FileIndex::updateMain(PathRef Path, ParsedAST &AST) { auto Contents = indexMainDecls(AST); MainFileSymbols.update( - Path, std::make_unique<SymbolSlab>(std::move(std::get<0>(Contents))), + URI::create(Path).toString(), + std::make_unique<SymbolSlab>(std::move(std::get<0>(Contents))), std::make_unique<RefSlab>(std::move(std::get<1>(Contents))), std::make_unique<RelationSlab>(std::move(std::get<2>(Contents))), /*CountReferences=*/true); diff --git a/clang-tools-extra/clangd/index/MemIndex.cpp b/clang-tools-extra/clangd/index/MemIndex.cpp index e2a8eb7f8e3f..9dc8e0aec944 100644 --- a/clang-tools-extra/clangd/index/MemIndex.cpp +++ b/clang-tools-extra/clangd/index/MemIndex.cpp @@ -112,14 +112,7 @@ void MemIndex::relations( 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; }; } diff --git a/clang-tools-extra/clangd/index/dex/Dex.cpp b/clang-tools-extra/clangd/index/dex/Dex.cpp index a6a8f23cab4c..7ebd5f685685 100644 --- a/clang-tools-extra/clangd/index/dex/Dex.cpp +++ b/clang-tools-extra/clangd/index/dex/Dex.cpp @@ -316,14 +316,7 @@ void Dex::relations( 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; }; } diff --git a/clang-tools-extra/clangd/test/memory_tree.test b/clang-tools-extra/clangd/test/memory_tree.test index c871c3f74494..207f5abd55e1 100644 --- a/clang-tools-extra/clangd/test/memory_tree.test +++ b/clang-tools-extra/clangd/test/memory_tree.test @@ -23,7 +23,9 @@ # CHECK-NEXT: "_total": {{[0-9]+}} # CHECK-NEXT: }, # CHECK-NEXT: "slabs": { -# CHECK-NEXT: "{{.*}}main.cpp": { +# CHECK-NEXT: "_self": {{[0-9]+}}, +# CHECK-NEXT: "_total": {{[0-9]+}}, +# CHECK-NEXT: "test:///main.cpp": { # CHECK-NEXT: "_self": {{[0-9]+}}, # CHECK-NEXT: "_total": {{[0-9]+}}, # CHECK-NEXT: "references": { @@ -38,9 +40,7 @@ # CHECK-NEXT: "_self": {{[0-9]+}}, # CHECK-NEXT: "_total": {{[0-9]+}} # CHECK-NEXT: } -# CHECK-NEXT: }, -# CHECK-NEXT: "_self": {{[0-9]+}}, -# CHECK-NEXT: "_total": {{[0-9]+}} +# CHECK-NEXT: } # CHECK-NEXT: } # CHECK-NEXT: }, # CHECK-NEXT: "preamble": { diff --git a/clang-tools-extra/clangd/unittests/DexTests.cpp b/clang-tools-extra/clangd/unittests/DexTests.cpp index 8ff319b694df..60d0db081dbb 100644 --- a/clang-tools-extra/clangd/unittests/DexTests.cpp +++ b/clang-tools-extra/clangd/unittests/DexTests.cpp @@ -737,7 +737,7 @@ TEST(DexIndex, IndexedFiles) { 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(); diff --git a/clang-tools-extra/clangd/unittests/FileIndexTests.cpp b/clang-tools-extra/clangd/unittests/FileIndexTests.cpp index 4ad5c3e18347..5f1998af3c21 100644 --- a/clang-tools-extra/clangd/unittests/FileIndexTests.cpp +++ b/clang-tools-extra/clangd/unittests/FileIndexTests.cpp @@ -352,14 +352,14 @@ TEST(FileIndexTest, Refs) { Test.Code = std::string(MainCode.code()); Test.Filename = "test.cc"; auto AST = Test.build(); - Index.updateMain(Test.Filename, AST); + Index.updateMain(testPath(Test.Filename), AST); // Add test2.cc TestTU Test2; Test2.HeaderCode = HeaderCode; Test2.Code = std::string(MainCode.code()); Test2.Filename = "test2.cc"; AST = Test2.build(); - Index.updateMain(Test2.Filename, AST); + Index.updateMain(testPath(Test2.Filename), AST); EXPECT_THAT(getRefs(Index, Foo.ID), RefsAre({AllOf(RefRange(MainCode.range("foo")), @@ -387,7 +387,7 @@ TEST(FileIndexTest, MacroRefs) { Test.Code = std::string(MainCode.code()); Test.Filename = "test.cc"; auto AST = Test.build(); - Index.updateMain(Test.Filename, AST); + Index.updateMain(testPath(Test.Filename), AST); auto HeaderMacro = findSymbol(Test.headerSymbols(), "HEADER_MACRO"); EXPECT_THAT(getRefs(Index, HeaderMacro.ID), diff --git a/clang-tools-extra/clangd/unittests/IndexTests.cpp b/clang-tools-extra/clangd/unittests/IndexTests.cpp index 64aafc9f883e..2b4b3af85613 100644 --- a/clang-tools-extra/clangd/unittests/IndexTests.cpp +++ b/clang-tools-extra/clangd/unittests/IndexTests.cpp @@ -229,7 +229,7 @@ TEST(MemIndexTest, IndexedFiles) { 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(); @@ -506,7 +506,7 @@ TEST(MergeIndexTest, IndexedFiles) { 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 @@ TEST(MergeIndexTest, IndexedFiles) { 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), _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
