Skip to content

Commit 2522fa0

Browse files
committed
[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
1 parent 1d0dc9b commit 2522fa0

File tree

2 files changed

+40
-0
lines changed

2 files changed

+40
-0
lines changed

clang-tools-extra/clangd/index/Merge.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,13 @@ void MergedIndex::lookup(
7676
Dynamic->lookup(Req, [&](const Symbol &S) { B.insert(S); });
7777

7878
auto RemainingIDs = Req.IDs;
79+
auto DynamicContainsFile = Dynamic->indexedFiles();
7980
Static->lookup(Req, [&](const Symbol &S) {
81+
// We expect the definition to see the canonical declaration, so it seems
82+
// to be enough to check only the definition if it exists.
83+
if (DynamicContainsFile(S.Definition ? S.Definition.FileURI
84+
: S.CanonicalDeclaration.FileURI))
85+
return;
8086
const Symbol *Sym = B.find(S.ID);
8187
RemainingIDs.erase(S.ID);
8288
if (!Sym)

clang-tools-extra/clangd/unittests/IndexTests.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,40 @@ TEST(MergeIndexTest, Lookup) {
290290
EXPECT_THAT(lookup(M, {}), UnorderedElementsAre());
291291
}
292292

293+
TEST(MergeIndexTest, LookupRemovedDefinition) {
294+
FileIndex DynamicIndex, StaticIndex;
295+
MergedIndex Merge(&DynamicIndex, &StaticIndex);
296+
297+
const char *HeaderCode = "class Foo;";
298+
auto HeaderSymbols = TestTU::withHeaderCode(HeaderCode).headerSymbols();
299+
auto Foo = findSymbol(HeaderSymbols, "Foo");
300+
301+
// Build static index for test.cc with Foo definition
302+
TestTU Test;
303+
Test.HeaderCode = HeaderCode;
304+
Test.Code = "class Foo {};";
305+
Test.Filename = "test.cc";
306+
auto AST = Test.build();
307+
StaticIndex.updateMain(testPath(Test.Filename), AST);
308+
309+
// Remove Foo definition from test.cc, i.e. build dynamic index for test.cc
310+
// without Foo definition.
311+
Test.Code = "class Foo;";
312+
AST = Test.build();
313+
DynamicIndex.updateMain(testPath(Test.Filename), AST);
314+
315+
// Merged index should not return the symbol definition if this definition
316+
// location is inside a file from the dynamic index.
317+
LookupRequest LookupReq;
318+
LookupReq.IDs = {Foo.ID};
319+
unsigned SymbolCounter = 0;
320+
Merge.lookup(LookupReq, [&](const Symbol &Sym) {
321+
++SymbolCounter;
322+
EXPECT_FALSE(Sym.Definition);
323+
});
324+
EXPECT_EQ(SymbolCounter, 1u);
325+
}
326+
293327
TEST(MergeIndexTest, FuzzyFind) {
294328
auto I = MemIndex::build(generateSymbols({"ns::A", "ns::B"}), RefSlab(),
295329
RelationSlab()),

0 commit comments

Comments
 (0)