https://github.com/TheOnionCutter1 updated https://github.com/llvm/llvm-project/pull/215018
>From 3e62c151ef8ac81f81cfd87ae43e5188d06d6046 Mon Sep 17 00:00:00 2001 From: Omer <[email protected]> Date: Sat, 8 Aug 2026 12:49:21 +0300 Subject: [PATCH] [clangd] Prefer project definitions for navigation Prevent definitions from open files in the dynamic index from overriding project-index definitions when navigating from a project file. Preserve dynamic definitions for external files and AST definitions for unsaved edits. Assisted-by: OpenAI Codex Make a different approach --- clang-tools-extra/clangd/ClangdServer.cpp | 9 ++- clang-tools-extra/clangd/ClangdServer.h | 5 ++ clang-tools-extra/clangd/index/Merge.cpp | 29 +++++++ clang-tools-extra/clangd/index/Merge.h | 18 +++++ .../clangd/unittests/ClangdTests.cpp | 39 ++++++++++ .../clangd/unittests/IndexTests.cpp | 77 +++++++++++++++++++ clang-tools-extra/docs/ReleaseNotes.md | 4 + 7 files changed, 180 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index 37eb82116f3a9..0d0a343d2c585 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -268,8 +268,15 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB, std::move(BGOpts)); AddIndex(BackgroundIdx.get()); } + const SymbolIndex *ProjectIndex = Index; if (DynamicIdx) AddIndex(DynamicIdx.get()); + NavigationIndex = Index; + if (DynamicIdx && ProjectIndex) { + ProjectDefinitionIdx = std::make_unique<ProjectDefinitionIndex>( + DynamicIdx.get(), ProjectIndex); + NavigationIndex = ProjectDefinitionIdx.get(); + } if (Opts.FeatureModules) { FeatureModule::Facilities F{ @@ -809,7 +816,7 @@ void ClangdServer::locateSymbolAt(PathRef File, Position Pos, this](llvm::Expected<InputsAndAST> InpAST) mutable { if (!InpAST) return CB(InpAST.takeError()); - CB(clangd::locateSymbolAt(InpAST->AST, Pos, Index)); + CB(clangd::locateSymbolAt(InpAST->AST, Pos, NavigationIndex)); }; WorkScheduler->runWithAST("Definitions", File, std::move(Action)); diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h index 264ab7437c248..f5e77ad370ac8 100644 --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -43,6 +43,7 @@ namespace clang { namespace clangd { +class ProjectDefinitionIndex; /// Manages a collection of source files and derived data (ASTs, indexes), /// and provides language-aware features such as code completion. /// @@ -494,12 +495,16 @@ class ClangdServer { // - the static index passed to the constructor // - a merged view of a static and dynamic index (MergedIndex) const SymbolIndex *Index = nullptr; + // Index used for navigation. Usually Index, but when both dynamic and + // project indexes exist this applies project-aware definition selection. + const SymbolIndex *NavigationIndex = nullptr; // If present, an index of symbols in open files. Read via *Index. std::unique_ptr<FileIndex> DynamicIdx; // If present, the new "auto-index" maintained in background threads. std::unique_ptr<BackgroundIndex> BackgroundIdx; // Storage for merged views of the various indexes. std::vector<std::unique_ptr<SymbolIndex>> MergedIdx; + std::unique_ptr<ProjectDefinitionIndex> ProjectDefinitionIdx; // Manage module files. ModulesBuilder *ModulesManager = nullptr; diff --git a/clang-tools-extra/clangd/index/Merge.cpp b/clang-tools-extra/clangd/index/Merge.cpp index 625b1c6926a28..ee1ec2ddec65e 100644 --- a/clang-tools-extra/clangd/index/Merge.cpp +++ b/clang-tools-extra/clangd/index/Merge.cpp @@ -122,6 +122,35 @@ void MergedIndex::lookup( Callback(*Sym); } +void ProjectDefinitionIndex::lookup( + const LookupRequest &Req, + llvm::function_ref<void(const Symbol &)> Callback) const { + trace::Span Tracer("ProjectDefinitionIndex lookup"); + SymbolSlab::Builder DynamicSymbols; + Dynamic->lookup(Req, [&](const Symbol &S) { DynamicSymbols.insert(S); }); + + auto RemainingIDs = Req.IDs; + auto DynamicContainsFile = Dynamic->indexedFiles(); + auto ProjectContainsFile = Project->indexedFiles(); + Project->lookup(Req, [&](const Symbol &ProjectSymbol) { + RemainingIDs.erase(ProjectSymbol.ID); + if (const Symbol *DynamicSymbol = DynamicSymbols.find(ProjectSymbol.ID)) { + Symbol Result = mergeSymbol(*DynamicSymbol, ProjectSymbol); + if (ProjectSymbol.Definition && DynamicSymbol->Definition && + !isIndexAuthoritative(ProjectContainsFile, *DynamicSymbol)) + Result.Definition = ProjectSymbol.Definition; + return Callback(Result); + } + + if (isIndexAuthoritative(DynamicContainsFile, ProjectSymbol)) + return; + Callback(ProjectSymbol); + }); + for (const auto &ID : RemainingIDs) + if (const Symbol *S = DynamicSymbols.find(ID)) + Callback(*S); +} + bool MergedIndex::refs(const RefsRequest &Req, llvm::function_ref<void(const Ref &)> Callback) const { trace::Span Tracer("MergedIndex refs"); diff --git a/clang-tools-extra/clangd/index/Merge.h b/clang-tools-extra/clangd/index/Merge.h index 5910c27cab58a..5f8f2c1b09a41 100644 --- a/clang-tools-extra/clangd/index/Merge.h +++ b/clang-tools-extra/clangd/index/Merge.h @@ -55,6 +55,24 @@ class MergedIndex : public SymbolIndex { } }; +// A merged view for definition lookup that treats project files specially. +// Dynamic definitions from project files are preferred because they may contain +// unsaved edits. Definitions from dynamic files outside the project are +// superseded by project definitions, so merely opening an inactive source file +// cannot redirect navigation. +// +// The provided indexes must outlive this non-owning view. +class ProjectDefinitionIndex : public MergedIndex { + const SymbolIndex *Dynamic, *Project; + +public: + ProjectDefinitionIndex(const SymbolIndex *Dynamic, const SymbolIndex *Project) + : MergedIndex(Dynamic, Project), Dynamic(Dynamic), Project(Project) {} + + void lookup(const LookupRequest &, + llvm::function_ref<void(const Symbol &)>) const override; +}; + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/unittests/ClangdTests.cpp b/clang-tools-extra/clangd/unittests/ClangdTests.cpp index 9ea7c3e02411d..433f7133466ef 100644 --- a/clang-tools-extra/clangd/unittests/ClangdTests.cpp +++ b/clang-tools-extra/clangd/unittests/ClangdTests.cpp @@ -368,6 +368,45 @@ TEST(ClangdServerTest, RespectsConfig) { EXPECT_NE(Result->front().PreferredDeclaration.range, Example.range()); } +TEST(ClangdServerTest, PrefersProjectDefinitionForNavigation) { + TestTU ActiveTU; + ActiveTU.HeaderCode = "void target();"; + ActiveTU.Code = "void target() {}"; + ActiveTU.Filename = "active.cpp"; + auto ProjectIndex = ActiveTU.index(); + + auto Opts = ClangdServer::optsForTest(); + Opts.BuildDynamicSymbolIndex = true; + Opts.StaticIndex = ProjectIndex.get(); + MockCompilationDatabase CDB; + MockFS FS; + ClangdServer Server(CDB, FS, Opts); + + runAddDocument(Server, testPath("inactive.cpp"), "void target() {}"); + Annotations Main(R"cpp( + void target(); + int main() { ^target(); } + )cpp"); + runAddDocument(Server, testPath("main.cpp"), Main.code()); + + auto Result = runLocateSymbolAt(Server, testPath("main.cpp"), Main.point()); + ASSERT_TRUE(bool(Result)) << Result.takeError(); + ASSERT_THAT(*Result, SizeIs(1)); + ASSERT_TRUE(Result->front().Definition); + EXPECT_EQ(Result->front().Definition->uri.file(), testPath("active.cpp")); + + Annotations Unsaved(R"cpp( + void target() {} + int main() { ^target(); } + )cpp"); + runAddDocument(Server, testPath("main.cpp"), Unsaved.code()); + Result = runLocateSymbolAt(Server, testPath("main.cpp"), Unsaved.point()); + ASSERT_TRUE(bool(Result)) << Result.takeError(); + ASSERT_THAT(*Result, SizeIs(1)); + ASSERT_TRUE(Result->front().Definition); + EXPECT_EQ(Result->front().Definition->uri.file(), testPath("main.cpp")); +} + TEST(ClangdServerTest, PropagatesVersion) { MockCompilationDatabase CDB; MockFS FS; diff --git a/clang-tools-extra/clangd/unittests/IndexTests.cpp b/clang-tools-extra/clangd/unittests/IndexTests.cpp index a66680d39c87d..6deed8f7fe156 100644 --- a/clang-tools-extra/clangd/unittests/IndexTests.cpp +++ b/clang-tools-extra/clangd/unittests/IndexTests.cpp @@ -41,6 +41,20 @@ MATCHER_P(refRange, Range, "") { } MATCHER_P(fileURI, F, "") { return StringRef(arg.Location.FileURI) == F; } +class CountingIndex : public SwapIndex { +public: + explicit CountingIndex(std::unique_ptr<SymbolIndex> Index) + : SwapIndex(std::move(Index)) {} + + void lookup(const LookupRequest &Req, + llvm::function_ref<void(const Symbol &)> Callback) const override { + ++LookupCount; + SwapIndex::lookup(Req, Callback); + } + + mutable unsigned LookupCount = 0; +}; + TEST(SymbolLocation, Position) { using Position = SymbolLocation::Position; Position Pos; @@ -291,6 +305,69 @@ TEST(MergeIndexTest, Lookup) { EXPECT_THAT(lookup(M, {}), UnorderedElementsAre()); } +TEST(ProjectDefinitionIndexTest, DefinitionPolicy) { + constexpr llvm::StringLiteral DynamicURI("unittest:///dynamic.cpp"); + constexpr llvm::StringLiteral ProjectURI("unittest:///project.cpp"); + Symbol DynamicSymbol = symbol("target"); + DynamicSymbol.Definition.FileURI = DynamicURI.data(); + DynamicSymbol.Documentation = "dynamic documentation"; + Symbol ProjectSymbol = symbol("target"); + ProjectSymbol.Definition.FileURI = ProjectURI.data(); + + auto MakeIndex = [](const Symbol &S, + llvm::ArrayRef<llvm::StringRef> IndexedFiles) { + SymbolSlab::Builder Symbols; + Symbols.insert(S); + SymbolSlab SymbolData = std::move(Symbols).build(); + RefSlab RefData; + auto Size = SymbolData.bytes() + RefData.bytes(); + auto Data = std::make_pair(std::move(SymbolData), std::move(RefData)); + llvm::StringSet<> Files; + for (llvm::StringRef File : IndexedFiles) + Files.insert(File); + return std::make_unique<MemIndex>(Data.first, Data.second, RelationSlab(), + std::move(Files), IndexContents::Symbols, + std::move(Data), Size); + }; + auto DefinitionURI = [](const SymbolIndex &Index) { + std::string Result; + LookupRequest Req; + Req.IDs.insert(SymbolID("target")); + Index.lookup(Req, [&](const Symbol &S) { + Result = S.Definition.FileURI; + EXPECT_EQ(S.Documentation, "dynamic documentation"); + }); + return Result; + }; + + auto Dynamic = MakeIndex(DynamicSymbol, {}); + auto Project = MakeIndex(ProjectSymbol, {ProjectURI}); + ProjectDefinitionIndex UnlistedDynamic(Dynamic.get(), Project.get()); + EXPECT_EQ(DefinitionURI(UnlistedDynamic), ProjectURI); + + auto ProjectCoveringDynamic = + MakeIndex(ProjectSymbol, {ProjectURI, DynamicURI}); + ProjectDefinitionIndex ActiveDynamic(Dynamic.get(), + ProjectCoveringDynamic.get()); + EXPECT_EQ(DefinitionURI(ActiveDynamic), DynamicURI); + + ProjectSymbol.Definition = {}; + auto ProjectWithoutDefinition = MakeIndex(ProjectSymbol, {ProjectURI}); + ProjectDefinitionIndex DynamicFallback(Dynamic.get(), + ProjectWithoutDefinition.get()); + EXPECT_EQ(DefinitionURI(DynamicFallback), DynamicURI); + + MergedIndex OrdinaryMerge(Dynamic.get(), Project.get()); + EXPECT_EQ(DefinitionURI(OrdinaryMerge), DynamicURI); + + CountingIndex CountedDynamic(MakeIndex(DynamicSymbol, {})); + CountingIndex CountedProject(MakeIndex(ProjectSymbol, {ProjectURI})); + ProjectDefinitionIndex Counted(&CountedDynamic, &CountedProject); + DefinitionURI(Counted); + EXPECT_EQ(CountedDynamic.LookupCount, 1u); + EXPECT_EQ(CountedProject.LookupCount, 1u); +} + TEST(MergeIndexTest, LookupRemovedDefinition) { FileIndex DynamicIndex(true), StaticIndex(true); MergedIndex Merge(&DynamicIndex, &StaticIndex); diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md index 5ded07934d906..bfd20e78d7446 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -76,6 +76,10 @@ infrastructure are described first, followed by tool-specific sections. #### Cross-references +- Improved Go to Definition in projects with alternative source + implementations. Definitions from files in the project index are now + preferred over definitions from open files outside the active project. + #### Objective-C #### Miscellaneous _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
