[clang-tools-extra] c4efd04 - [clangd] Use URIs instead of paths in the index file list

Aleksandr Platonov via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 5 23:48:10 PST 2021


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),


        


More information about the cfe-commits mailing list