[clangd] Do not take stale definition from the static index.

This is follow up to D93393.
Without this patch clangd takes the symbol definition from the static index if this definition was removed from the dynamic index.

Reviewed By: sammccall

Differential Revision: https://reviews.llvm.org/D93683

diff --git a/clang-tools-extra/clangd/index/Merge.cpp b/clang-tools-extra/clangd/index/Merge.cpp
index 97babac..f66f474 100644 (file)
--- a/clang-tools-extra/clangd/index/Merge.cpp
+++ b/clang-tools-extra/clangd/index/Merge.cpp
@@ -76,7 +76,13 @@ void MergedIndex::lookup(
   Dynamic->lookup(Req, [&](const Symbol &S) { B.insert(S); });
 
   auto RemainingIDs = Req.IDs;
+  auto DynamicContainsFile = Dynamic->indexedFiles();
   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))
+      return;
     const Symbol *Sym = B.find(S.ID);
     RemainingIDs.erase(S.ID);
     if (!Sym)

diff --git a/clang-tools-extra/clangd/unittests/IndexTests.cpp b/clang-tools-extra/clangd/unittests/IndexTests.cpp
index 8efc637..ce4845e 100644 (file)
--- a/clang-tools-extra/clangd/unittests/IndexTests.cpp
+++ b/clang-tools-extra/clangd/unittests/IndexTests.cpp
@@ -290,6 +290,40 @@ TEST(MergeIndexTest, Lookup) {
   EXPECT_THAT(lookup(M, {}), UnorderedElementsAre());
 }
 
+TEST(MergeIndexTest, LookupRemovedDefinition) {
+  FileIndex DynamicIndex, StaticIndex;
+  MergedIndex Merge(&DynamicIndex, &StaticIndex);
+
+  const char *HeaderCode = "class Foo;";
+  auto HeaderSymbols = TestTU::withHeaderCode(HeaderCode).headerSymbols();
+  auto Foo = findSymbol(HeaderSymbols, "Foo");
+
+  // Build static index for test.cc with Foo definition
+  TestTU Test;
+  Test.HeaderCode = HeaderCode;
+  Test.Code = "class Foo {};";
+  Test.Filename = "test.cc";
+  auto AST = Test.build();
+  StaticIndex.updateMain(testPath(Test.Filename), AST);
+
+  // Remove Foo definition from test.cc, i.e. build dynamic index for test.cc
+  // without Foo definition.
+  Test.Code = "class Foo;";
+  AST = Test.build();
+  DynamicIndex.updateMain(testPath(Test.Filename), AST);
+
+  // Merged index should not return the symbol definition if this definition
+  // location is inside a file from the dynamic index.
+  LookupRequest LookupReq;
+  LookupReq.IDs = {Foo.ID};
+  unsigned SymbolCounter = 0;
+  Merge.lookup(LookupReq, [&](const Symbol &Sym) {
+    ++SymbolCounter;
+    EXPECT_FALSE(Sym.Definition);
+  });
+  EXPECT_EQ(SymbolCounter, 1u);
+}
+
 TEST(MergeIndexTest, FuzzyFind) {
   auto I = MemIndex::build(generateSymbols({"ns::A", "ns::B"}), RefSlab(),
                            RelationSlab()),