Author: Aleksandr Platonov Date: 2020-12-23T18:21:38+03:00 New Revision: 2522fa053b62520ae48b4b27117ca003a2c878ab
URL: https://github.com/llvm/llvm-project/commit/2522fa053b62520ae48b4b27117ca003a2c878ab DIFF: https://github.com/llvm/llvm-project/commit/2522fa053b62520ae48b4b27117ca003a2c878ab.diff LOG: [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 Added: Modified: clang-tools-extra/clangd/index/Merge.cpp clang-tools-extra/clangd/unittests/IndexTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/index/Merge.cpp b/clang-tools-extra/clangd/index/Merge.cpp index 97babacf2b38..f66f47499624 100644 --- 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 8efc637d1250..ce4845e3e144 100644 --- 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()), _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits