From 06f90b1f53809cdacf40cd6fc8e9ba3d1c240c1e Mon Sep 17 00:00:00 2001 From: Timon Ulrich Date: Thu, 9 Oct 2025 16:51:31 +0200 Subject: [PATCH 1/7] clangd: Make callHierarchy follow inheritance In case of the callHierarchy being invoked on a virtual function it would not display potential calls through the base function. This commit adds these potential calls and also gives the client the information to annotate them as such. Also adds a test for this. --- clang-tools-extra/clangd/Protocol.h | 4 + clang-tools-extra/clangd/XRefs.cpp | 100 ++++++++++-------- clang-tools-extra/clangd/index/Index.cpp | 6 ++ clang-tools-extra/clangd/index/Index.h | 12 +++ clang-tools-extra/clangd/index/MemIndex.cpp | 21 ++++ clang-tools-extra/clangd/index/MemIndex.h | 15 ++- clang-tools-extra/clangd/index/Merge.cpp | 24 +++++ clang-tools-extra/clangd/index/Merge.h | 3 + .../clangd/index/ProjectAware.cpp | 12 +++ clang-tools-extra/clangd/index/dex/Dex.cpp | 21 ++++ clang-tools-extra/clangd/index/dex/Dex.h | 14 ++- .../clangd/unittests/CallHierarchyTests.cpp | 37 +++++++ .../clangd/unittests/CodeCompleteTests.cpp | 4 + .../clangd/unittests/RenameTests.cpp | 8 ++ 14 files changed, 237 insertions(+), 44 deletions(-) diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h index 3a6bf155ee153..0f51665d7b896 100644 --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -1627,6 +1627,10 @@ struct CallHierarchyIncomingCall { /// The range at which the calls appear. /// This is relative to the caller denoted by `From`. std::vector fromRanges; + + /// This caller might be a false positive. + /// We currently have no way of 100% confirming this. + bool mightNeverCall = false; }; llvm::json::Value toJSON(const CallHierarchyIncomingCall &); diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 05e04ac161e54..677858bb8dcb3 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -21,6 +21,7 @@ #include "clang-include-cleaner/Types.h" #include "index/Index.h" #include "index/Merge.h" +#include "index/Ref.h" #include "index/Relation.h" #include "index/SymbolCollector.h" #include "index/SymbolID.h" @@ -56,6 +57,7 @@ #include "clang/Tooling/Syntax/Tokens.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/DenseSet.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/SmallSet.h" @@ -66,6 +68,7 @@ #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/Path.h" #include "llvm/Support/raw_ostream.h" +#include #include #include #include @@ -2350,51 +2353,64 @@ incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) { // to an AST node isn't cheap, particularly when the declaration isn't // in the main file. // FIXME: Consider also using AST information when feasible. - RefsRequest Request; - Request.IDs.insert(*ID); - Request.WantContainer = true; - // We could restrict more specifically to calls by introducing a new RefKind, - // but non-call references (such as address-of-function) can still be - // interesting as they can indicate indirect calls. - Request.Filter = RefKind::Reference; - // Initially store the ranges in a map keyed by SymbolID of the caller. - // This allows us to group different calls with the same caller - // into the same CallHierarchyIncomingCall. - llvm::DenseMap> CallsIn; - // We can populate the ranges based on a refs request only. As we do so, we - // also accumulate the container IDs into a lookup request. - LookupRequest ContainerLookup; - Index->refs(Request, [&](const Ref &R) { - auto Loc = indexToLSPLocation(R.Location, Item.uri.file()); - if (!Loc) { - elog("incomingCalls failed to convert location: {0}", Loc.takeError()); - return; - } - CallsIn[R.Container].push_back(*Loc); + auto QueryIndex = [&](llvm::DenseSet IDs, bool MightNeverCall) { + RefsRequest Request; + Request.IDs = std::move(IDs); + Request.WantContainer = true; + // We could restrict more specifically to calls by introducing a new + // RefKind, but non-call references (such as address-of-function) can still + // be interesting as they can indicate indirect calls. + Request.Filter = RefKind::Reference; + // Initially store the ranges in a map keyed by SymbolID of the caller. + // This allows us to group different calls with the same caller + // into the same CallHierarchyIncomingCall. + llvm::DenseMap> CallsIn; + // We can populate the ranges based on a refs request only. As we do so, we + // also accumulate the container IDs into a lookup request. + LookupRequest ContainerLookup; + Index->refs(Request, [&](const Ref &R) { + auto Loc = indexToLSPLocation(R.Location, Item.uri.file()); + if (!Loc) { + elog("incomingCalls failed to convert location: {0}", Loc.takeError()); + return; + } + CallsIn[R.Container].push_back(*Loc); - ContainerLookup.IDs.insert(R.Container); - }); - // Perform the lookup request and combine its results with CallsIn to - // get complete CallHierarchyIncomingCall objects. - Index->lookup(ContainerLookup, [&](const Symbol &Caller) { - auto It = CallsIn.find(Caller.ID); - assert(It != CallsIn.end()); - if (auto CHI = symbolToCallHierarchyItem(Caller, Item.uri.file())) { - std::vector FromRanges; - for (const Location &L : It->second) { - if (L.uri != CHI->uri) { - // Call location not in same file as caller. - // This can happen in some edge cases. There's not much we can do, - // since the protocol only allows returning ranges interpreted as - // being in the caller's file. - continue; + ContainerLookup.IDs.insert(R.Container); + }); + // Perform the lookup request and combine its results with CallsIn to + // get complete CallHierarchyIncomingCall objects. + Index->lookup(ContainerLookup, [&](const Symbol &Caller) { + auto It = CallsIn.find(Caller.ID); + assert(It != CallsIn.end()); + if (auto CHI = symbolToCallHierarchyItem(Caller, Item.uri.file())) { + std::vector FromRanges; + for (const Location &L : It->second) { + if (L.uri != CHI->uri) { + // Call location not in same file as caller. + // This can happen in some edge cases. There's not much we can do, + // since the protocol only allows returning ranges interpreted as + // being in the caller's file. + continue; + } + FromRanges.push_back(L.range); } - FromRanges.push_back(L.range); + Results.push_back(CallHierarchyIncomingCall{ + std::move(*CHI), std::move(FromRanges), MightNeverCall}); } - Results.push_back( - CallHierarchyIncomingCall{std::move(*CHI), std::move(FromRanges)}); - } - }); + }); + }; + QueryIndex({ID.get()}, false); + // In the case of being a virtual function we also want to return + // potential calls through the base function. + if (Item.kind == SymbolKind::Method) { + llvm::DenseSet IDs; + RelationsRequest Req{{ID.get()}, RelationKind::OverriddenBy}; + Index->reverseRelations(Req, [&](const SymbolID &, const Symbol &Caller) { + IDs.insert(Caller.ID); + }); + QueryIndex(std::move(IDs), true); + } // Sort results by name of container. llvm::sort(Results, [](const CallHierarchyIncomingCall &A, const CallHierarchyIncomingCall &B) { diff --git a/clang-tools-extra/clangd/index/Index.cpp b/clang-tools-extra/clangd/index/Index.cpp index 86dc6ed763344..a2ec910606d92 100644 --- a/clang-tools-extra/clangd/index/Index.cpp +++ b/clang-tools-extra/clangd/index/Index.cpp @@ -77,6 +77,12 @@ void SwapIndex::relations( return snapshot()->relations(R, CB); } +void SwapIndex::reverseRelations( + const RelationsRequest &R, + llvm::function_ref CB) const { + return snapshot()->reverseRelations(R, CB); +} + llvm::unique_function SwapIndex::indexedFiles() const { // The index snapshot should outlive this method return value. diff --git a/clang-tools-extra/clangd/index/Index.h b/clang-tools-extra/clangd/index/Index.h index a193b1a191216..3f2c9fca3cf52 100644 --- a/clang-tools-extra/clangd/index/Index.h +++ b/clang-tools-extra/clangd/index/Index.h @@ -181,6 +181,14 @@ class SymbolIndex { llvm::function_ref Callback) const = 0; + /// Finds all relations (O, P, S) stored in the index such that S is among + /// Req.Subjects and P is Req.Predicate, and invokes \p Callback for (S, O) in + /// each. Currently only allows the OverridenBy relation and only stored in Memory. + virtual void reverseRelations( + const RelationsRequest &Req, + llvm::function_ref + Callback) const = 0; + /// Returns function which checks if the specified file was used to build this /// index or not. The function must only be called while the index is alive. using IndexedFiles = @@ -214,6 +222,10 @@ class SwapIndex : public SymbolIndex { llvm::function_ref) const override; + void reverseRelations(const RelationsRequest &, + llvm::function_ref) + const override; + llvm::unique_function indexedFiles() const override; diff --git a/clang-tools-extra/clangd/index/MemIndex.cpp b/clang-tools-extra/clangd/index/MemIndex.cpp index 9c9d3942bdee6..238f68dd37d19 100644 --- a/clang-tools-extra/clangd/index/MemIndex.cpp +++ b/clang-tools-extra/clangd/index/MemIndex.cpp @@ -125,6 +125,27 @@ void MemIndex::relations( } } +void MemIndex::reverseRelations( + const RelationsRequest &Req, + llvm::function_ref Callback) const { + assert(Req.Predicate == RelationKind::OverriddenBy); + uint32_t Remaining = Req.Limit.value_or(std::numeric_limits::max()); + for (const SymbolID &Subject : Req.Subjects) { + LookupRequest LookupReq; + auto It = ReverseRelations.find( + std::make_pair(Subject, static_cast(Req.Predicate))); + if (It != ReverseRelations.end()) { + for (const auto &Obj : It->second) { + if (Remaining > 0) { + --Remaining; + LookupReq.IDs.insert(Obj); + } + } + } + lookup(LookupReq, [&](const Symbol &Object) { Callback(Subject, Object); }); + } +} + llvm::unique_function MemIndex::indexedFiles() const { return [this](llvm::StringRef FileURI) { diff --git a/clang-tools-extra/clangd/index/MemIndex.h b/clang-tools-extra/clangd/index/MemIndex.h index fb1052b0c7ca8..23cc8f030591c 100644 --- a/clang-tools-extra/clangd/index/MemIndex.h +++ b/clang-tools-extra/clangd/index/MemIndex.h @@ -10,6 +10,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_MEMINDEX_H #include "index/Index.h" +#include "index/Relation.h" #include "llvm/ADT/StringSet.h" #include @@ -27,10 +28,16 @@ class MemIndex : public SymbolIndex { Index[S.ID] = &S; for (const std::pair> &R : Refs) this->Refs.try_emplace(R.first, R.second.begin(), R.second.end()); - for (const Relation &R : Relations) + for (const Relation &R : Relations) { this->Relations[std::make_pair(R.Subject, static_cast(R.Predicate))] .push_back(R.Object); + if (R.Predicate == RelationKind::OverriddenBy) { + this->ReverseRelations[std::make_pair(R.Object, + static_cast(R.Predicate))] + .push_back(R.Subject); + } + } } // Symbols are owned by BackingData, Index takes ownership. template Callback) const override; + void reverseRelations(const RelationsRequest &Req, + llvm::function_ref + Callback) const override; + llvm::unique_function indexedFiles() const override; @@ -94,6 +105,8 @@ class MemIndex : public SymbolIndex { static_assert(sizeof(RelationKind) == sizeof(uint8_t), "RelationKind should be of same size as a uint8_t"); llvm::DenseMap, std::vector> Relations; + // Reverse relations, currently only for OverridenBy + llvm::DenseMap, std::vector> ReverseRelations; // Set of files which were used during this index build. llvm::StringSet<> Files; // Contents of the index (symbols, references, etc.) diff --git a/clang-tools-extra/clangd/index/Merge.cpp b/clang-tools-extra/clangd/index/Merge.cpp index aecca38a885b6..a7868b6f8688b 100644 --- a/clang-tools-extra/clangd/index/Merge.cpp +++ b/clang-tools-extra/clangd/index/Merge.cpp @@ -221,6 +221,30 @@ void MergedIndex::relations( }); } +void MergedIndex::reverseRelations( + const RelationsRequest &Req, + llvm::function_ref Callback) const { + uint32_t Remaining = Req.Limit.value_or(std::numeric_limits::max()); + // Return results from both indexes but avoid duplicates. + // We might return stale relations from the static index; + // we don't currently have a good way of identifying them. + llvm::DenseSet> SeenRelations; + Dynamic->reverseRelations(Req, [&](const SymbolID &Subject, const Symbol &Object) { + Callback(Subject, Object); + SeenRelations.insert(std::make_pair(Subject, Object.ID)); + --Remaining; + }); + if (Remaining == 0) + return; + Static->reverseRelations(Req, [&](const SymbolID &Subject, const Symbol &Object) { + if (Remaining > 0 && + !SeenRelations.count(std::make_pair(Subject, Object.ID))) { + --Remaining; + Callback(Subject, Object); + } + }); +} + // Returns true if \p L is (strictly) preferred to \p R (e.g. by file paths). If // neither is preferred, this returns false. static bool prefer(const SymbolLocation &L, const SymbolLocation &R) { diff --git a/clang-tools-extra/clangd/index/Merge.h b/clang-tools-extra/clangd/index/Merge.h index 7441be6e57e85..36c3567e7caec 100644 --- a/clang-tools-extra/clangd/index/Merge.h +++ b/clang-tools-extra/clangd/index/Merge.h @@ -44,6 +44,9 @@ class MergedIndex : public SymbolIndex { void relations(const RelationsRequest &, llvm::function_ref) const override; + void reverseRelations(const RelationsRequest &, + llvm::function_ref) + const override; llvm::unique_function indexedFiles() const override; size_t estimateMemoryUsage() const override { diff --git a/clang-tools-extra/clangd/index/ProjectAware.cpp b/clang-tools-extra/clangd/index/ProjectAware.cpp index 9836f0130362a..60d9710ab5640 100644 --- a/clang-tools-extra/clangd/index/ProjectAware.cpp +++ b/clang-tools-extra/clangd/index/ProjectAware.cpp @@ -51,6 +51,10 @@ class ProjectAwareIndex : public SymbolIndex { llvm::function_ref Callback) const override; + void reverseRelations(const RelationsRequest &, + llvm::function_ref) + const override; + llvm::unique_function indexedFiles() const override; @@ -124,6 +128,14 @@ void ProjectAwareIndex::relations( return Idx->relations(Req, Callback); } +void ProjectAwareIndex::reverseRelations( + const RelationsRequest &Req, + llvm::function_ref Callback) const { + trace::Span Tracer("ProjectAwareIndex::relations"); + if (auto *Idx = getIndex()) + return Idx->reverseRelations(Req, Callback); +} + llvm::unique_function ProjectAwareIndex::indexedFiles() const { trace::Span Tracer("ProjectAwareIndex::indexedFiles"); diff --git a/clang-tools-extra/clangd/index/dex/Dex.cpp b/clang-tools-extra/clangd/index/dex/Dex.cpp index 575a96a112979..925e256cc5759 100644 --- a/clang-tools-extra/clangd/index/dex/Dex.cpp +++ b/clang-tools-extra/clangd/index/dex/Dex.cpp @@ -379,6 +379,27 @@ void Dex::relations( } } +void Dex::reverseRelations( + const RelationsRequest &Req, + llvm::function_ref Callback) const { + assert(Req.Predicate == RelationKind::OverriddenBy); + uint32_t Remaining = Req.Limit.value_or(std::numeric_limits::max()); + for (const SymbolID &Subject : Req.Subjects) { + LookupRequest LookupReq; + auto It = ReverseRelations.find( + std::make_pair(Subject, static_cast(Req.Predicate))); + if (It != ReverseRelations.end()) { + for (const auto &Obj : It->second) { + if (Remaining > 0) { + --Remaining; + LookupReq.IDs.insert(Obj); + } + } + } + lookup(LookupReq, [&](const Symbol &Object) { Callback(Subject, Object); }); + } +} + llvm::unique_function Dex::indexedFiles() const { return [this](llvm::StringRef FileURI) { diff --git a/clang-tools-extra/clangd/index/dex/Dex.h b/clang-tools-extra/clangd/index/dex/Dex.h index 502f597d81ef0..274d2655cbebe 100644 --- a/clang-tools-extra/clangd/index/dex/Dex.h +++ b/clang-tools-extra/clangd/index/dex/Dex.h @@ -43,10 +43,16 @@ class Dex : public SymbolIndex { this->Symbols.push_back(&Sym); for (auto &&Ref : Refs) this->Refs.try_emplace(Ref.first, Ref.second); - for (auto &&Rel : Relations) + for (auto &&Rel : Relations) { this->Relations[std::make_pair(Rel.Subject, static_cast(Rel.Predicate))] .push_back(Rel.Object); + if (Rel.Predicate == RelationKind::OverriddenBy) { + this->ReverseRelations[std::make_pair(Rel.Object, + static_cast(Rel.Predicate))] + .push_back(Rel.Subject); + } + } buildIndex(SupportContainedRefs); } // Symbols and Refs are owned by BackingData, Index takes ownership. @@ -96,6 +102,10 @@ class Dex : public SymbolIndex { llvm::function_ref Callback) const override; + void reverseRelations(const RelationsRequest &, + llvm::function_ref) + const override; + llvm::unique_function indexedFiles() const override; @@ -142,6 +152,8 @@ class Dex : public SymbolIndex { static_assert(sizeof(RelationKind) == sizeof(uint8_t), "RelationKind should be of same size as a uint8_t"); llvm::DenseMap, std::vector> Relations; + // Reverse relations, currently only for OverridenBy + llvm::DenseMap, std::vector> ReverseRelations; std::shared_ptr KeepAlive; // poor man's move-only std::any // Set of files which were used during this index build. llvm::StringSet<> Files; diff --git a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp index 08cc80ff8981e..d906afe95703c 100644 --- a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp +++ b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp @@ -162,6 +162,43 @@ TEST(CallHierarchy, IncomingOneFileObjC) { EXPECT_THAT(IncomingLevel4, IsEmpty()); } +TEST(CallHierarchy, IncomingIncludeOverrides) { + Annotations Source(R"cpp( + void call^ee() {} + struct Interface { + virtual void Func() = 0; + }; + struct Implementation : public Interface { + void Func() override { + $Callee[[callee]](); + } + }; + void Test(Interface& cls){ + cls.$FuncCall[[Func]](); + } + )cpp"); + TestTU TU = TestTU::withCode(Source.code()); + auto AST = TU.build(); + auto Index = TU.index(); + + std::vector Items = + prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); + ASSERT_THAT(Items, ElementsAre(withName("callee"))); + auto IncomingLevel1 = incomingCalls(Items[0], Index.get()); + ASSERT_THAT( + IncomingLevel1, + ElementsAre(AllOf(from(AllOf(withName("Func"), withDetail("Implementation::Func"))), + iFromRanges(Source.range("Callee"))))); + auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get()); + ASSERT_THAT( + IncomingLevel2, + ElementsAre(AllOf(from(AllOf(withName("Test"), withDetail("Test"))), + iFromRanges(Source.range("FuncCall"))))); + + auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get()); + EXPECT_THAT(IncomingLevel3, IsEmpty()); +} + TEST(CallHierarchy, MainFileOnlyRef) { // In addition to testing that we store refs to main-file only symbols, // this tests that anonymous namespaces do not interfere with the diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index 7640569128172..78aff26ad858d 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -1830,6 +1830,10 @@ class IndexRequestCollector : public SymbolIndex { llvm::function_ref) const override {} + void reverseRelations(const RelationsRequest &, + llvm::function_ref) + const override {} + llvm::unique_function indexedFiles() const override { return [](llvm::StringRef) { return IndexContents::None; }; diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp index 5d2a77b62a219..ed0c63581d671 100644 --- a/clang-tools-extra/clangd/unittests/RenameTests.cpp +++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp @@ -1674,6 +1674,10 @@ TEST(CrossFileRenameTests, DirtyBuffer) { llvm::function_ref Callback) const override {} + void reverseRelations(const RelationsRequest &Req, + llvm::function_ref + Callback) const override {} + llvm::unique_function indexedFiles() const override { return [](llvm::StringRef) { return IndexContents::None; }; @@ -1729,6 +1733,10 @@ TEST(CrossFileRenameTests, DeduplicateRefsFromIndex) { llvm::function_ref) const override {} + void reverseRelations(const RelationsRequest &, + llvm::function_ref) + const override {} + llvm::unique_function indexedFiles() const override { return [](llvm::StringRef) { return IndexContents::None; }; From beb6c11ac61039f71b2ee7512934109aa5738f76 Mon Sep 17 00:00:00 2001 From: Timon Ulrich Date: Sun, 12 Oct 2025 02:43:38 +0200 Subject: [PATCH 2/7] clangd: Make callHierarchy follow inheritance - formatting --- clang-tools-extra/clangd/XRefs.cpp | 2 +- clang-tools-extra/clangd/index/Index.h | 8 +++--- clang-tools-extra/clangd/index/MemIndex.h | 16 +++++++----- clang-tools-extra/clangd/index/Merge.cpp | 26 ++++++++++--------- clang-tools-extra/clangd/index/Merge.h | 5 ++-- .../clangd/index/ProjectAware.cpp | 5 ++-- clang-tools-extra/clangd/index/dex/Dex.h | 14 +++++----- .../clangd/unittests/CallHierarchyTests.cpp | 8 +++--- .../clangd/unittests/CodeCompleteTests.cpp | 5 ++-- .../clangd/unittests/RenameTests.cpp | 12 +++++---- 10 files changed, 57 insertions(+), 44 deletions(-) diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 677858bb8dcb3..ef45acf501612 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -2405,7 +2405,7 @@ incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) { // potential calls through the base function. if (Item.kind == SymbolKind::Method) { llvm::DenseSet IDs; - RelationsRequest Req{{ID.get()}, RelationKind::OverriddenBy}; + RelationsRequest Req{{ID.get()}, RelationKind::OverriddenBy, std::nullopt}; Index->reverseRelations(Req, [&](const SymbolID &, const Symbol &Caller) { IDs.insert(Caller.ID); }); diff --git a/clang-tools-extra/clangd/index/Index.h b/clang-tools-extra/clangd/index/Index.h index 3f2c9fca3cf52..0d34ea010b35d 100644 --- a/clang-tools-extra/clangd/index/Index.h +++ b/clang-tools-extra/clangd/index/Index.h @@ -183,7 +183,8 @@ class SymbolIndex { /// Finds all relations (O, P, S) stored in the index such that S is among /// Req.Subjects and P is Req.Predicate, and invokes \p Callback for (S, O) in - /// each. Currently only allows the OverridenBy relation and only stored in Memory. + /// each. Currently only allows the OverridenBy relation and only stored in + /// memory. virtual void reverseRelations( const RelationsRequest &Req, llvm::function_ref @@ -222,8 +223,9 @@ class SwapIndex : public SymbolIndex { llvm::function_ref) const override; - void reverseRelations(const RelationsRequest &, - llvm::function_ref) + void + reverseRelations(const RelationsRequest &, + llvm::function_ref) const override; llvm::unique_function diff --git a/clang-tools-extra/clangd/index/MemIndex.h b/clang-tools-extra/clangd/index/MemIndex.h index 23cc8f030591c..b8761c8723a74 100644 --- a/clang-tools-extra/clangd/index/MemIndex.h +++ b/clang-tools-extra/clangd/index/MemIndex.h @@ -33,9 +33,9 @@ class MemIndex : public SymbolIndex { static_cast(R.Predicate))] .push_back(R.Object); if (R.Predicate == RelationKind::OverriddenBy) { - this->ReverseRelations[std::make_pair(R.Object, - static_cast(R.Predicate))] - .push_back(R.Subject); + this->ReverseRelations[std::make_pair( + R.Object, static_cast(R.Predicate))] + .push_back(R.Subject); } } } @@ -87,9 +87,10 @@ class MemIndex : public SymbolIndex { llvm::function_ref Callback) const override; - void reverseRelations(const RelationsRequest &Req, - llvm::function_ref - Callback) const override; + void + reverseRelations(const RelationsRequest &Req, + llvm::function_ref + Callback) const override; llvm::unique_function indexedFiles() const override; @@ -106,7 +107,8 @@ class MemIndex : public SymbolIndex { "RelationKind should be of same size as a uint8_t"); llvm::DenseMap, std::vector> Relations; // Reverse relations, currently only for OverridenBy - llvm::DenseMap, std::vector> ReverseRelations; + llvm::DenseMap, std::vector> + ReverseRelations; // Set of files which were used during this index build. llvm::StringSet<> Files; // Contents of the index (symbols, references, etc.) diff --git a/clang-tools-extra/clangd/index/Merge.cpp b/clang-tools-extra/clangd/index/Merge.cpp index a7868b6f8688b..625b1c6926a28 100644 --- a/clang-tools-extra/clangd/index/Merge.cpp +++ b/clang-tools-extra/clangd/index/Merge.cpp @@ -229,20 +229,22 @@ void MergedIndex::reverseRelations( // We might return stale relations from the static index; // we don't currently have a good way of identifying them. llvm::DenseSet> SeenRelations; - Dynamic->reverseRelations(Req, [&](const SymbolID &Subject, const Symbol &Object) { - Callback(Subject, Object); - SeenRelations.insert(std::make_pair(Subject, Object.ID)); - --Remaining; - }); + Dynamic->reverseRelations( + Req, [&](const SymbolID &Subject, const Symbol &Object) { + Callback(Subject, Object); + SeenRelations.insert(std::make_pair(Subject, Object.ID)); + --Remaining; + }); if (Remaining == 0) return; - Static->reverseRelations(Req, [&](const SymbolID &Subject, const Symbol &Object) { - if (Remaining > 0 && - !SeenRelations.count(std::make_pair(Subject, Object.ID))) { - --Remaining; - Callback(Subject, Object); - } - }); + Static->reverseRelations( + Req, [&](const SymbolID &Subject, const Symbol &Object) { + if (Remaining > 0 && + !SeenRelations.count(std::make_pair(Subject, Object.ID))) { + --Remaining; + Callback(Subject, Object); + } + }); } // Returns true if \p L is (strictly) preferred to \p R (e.g. by file paths). If diff --git a/clang-tools-extra/clangd/index/Merge.h b/clang-tools-extra/clangd/index/Merge.h index 36c3567e7caec..5910c27cab58a 100644 --- a/clang-tools-extra/clangd/index/Merge.h +++ b/clang-tools-extra/clangd/index/Merge.h @@ -44,8 +44,9 @@ class MergedIndex : public SymbolIndex { void relations(const RelationsRequest &, llvm::function_ref) const override; - void reverseRelations(const RelationsRequest &, - llvm::function_ref) + void + reverseRelations(const RelationsRequest &, + llvm::function_ref) const override; llvm::unique_function indexedFiles() const override; diff --git a/clang-tools-extra/clangd/index/ProjectAware.cpp b/clang-tools-extra/clangd/index/ProjectAware.cpp index 60d9710ab5640..34d037b854e3d 100644 --- a/clang-tools-extra/clangd/index/ProjectAware.cpp +++ b/clang-tools-extra/clangd/index/ProjectAware.cpp @@ -51,8 +51,9 @@ class ProjectAwareIndex : public SymbolIndex { llvm::function_ref Callback) const override; - void reverseRelations(const RelationsRequest &, - llvm::function_ref) + void + reverseRelations(const RelationsRequest &, + llvm::function_ref) const override; llvm::unique_function diff --git a/clang-tools-extra/clangd/index/dex/Dex.h b/clang-tools-extra/clangd/index/dex/Dex.h index 274d2655cbebe..4add20008cfcf 100644 --- a/clang-tools-extra/clangd/index/dex/Dex.h +++ b/clang-tools-extra/clangd/index/dex/Dex.h @@ -48,9 +48,9 @@ class Dex : public SymbolIndex { static_cast(Rel.Predicate))] .push_back(Rel.Object); if (Rel.Predicate == RelationKind::OverriddenBy) { - this->ReverseRelations[std::make_pair(Rel.Object, - static_cast(Rel.Predicate))] - .push_back(Rel.Subject); + this->ReverseRelations[std::make_pair(Rel.Object, static_cast( + Rel.Predicate))] + .push_back(Rel.Subject); } } buildIndex(SupportContainedRefs); @@ -102,8 +102,9 @@ class Dex : public SymbolIndex { llvm::function_ref Callback) const override; - void reverseRelations(const RelationsRequest &, - llvm::function_ref) + void + reverseRelations(const RelationsRequest &, + llvm::function_ref) const override; llvm::unique_function @@ -153,7 +154,8 @@ class Dex : public SymbolIndex { "RelationKind should be of same size as a uint8_t"); llvm::DenseMap, std::vector> Relations; // Reverse relations, currently only for OverridenBy - llvm::DenseMap, std::vector> ReverseRelations; + llvm::DenseMap, std::vector> + ReverseRelations; std::shared_ptr KeepAlive; // poor man's move-only std::any // Set of files which were used during this index build. llvm::StringSet<> Files; diff --git a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp index d906afe95703c..9859577c7cf7e 100644 --- a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp +++ b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp @@ -185,10 +185,10 @@ TEST(CallHierarchy, IncomingIncludeOverrides) { prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename)); ASSERT_THAT(Items, ElementsAre(withName("callee"))); auto IncomingLevel1 = incomingCalls(Items[0], Index.get()); - ASSERT_THAT( - IncomingLevel1, - ElementsAre(AllOf(from(AllOf(withName("Func"), withDetail("Implementation::Func"))), - iFromRanges(Source.range("Callee"))))); + ASSERT_THAT(IncomingLevel1, + ElementsAre(AllOf(from(AllOf(withName("Func"), + withDetail("Implementation::Func"))), + iFromRanges(Source.range("Callee"))))); auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get()); ASSERT_THAT( IncomingLevel2, diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index 78aff26ad858d..abbff45928f47 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -1830,8 +1830,9 @@ class IndexRequestCollector : public SymbolIndex { llvm::function_ref) const override {} - void reverseRelations(const RelationsRequest &, - llvm::function_ref) + void + reverseRelations(const RelationsRequest &, + llvm::function_ref) const override {} llvm::unique_function diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp index ed0c63581d671..42279b51230e7 100644 --- a/clang-tools-extra/clangd/unittests/RenameTests.cpp +++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp @@ -1674,9 +1674,10 @@ TEST(CrossFileRenameTests, DirtyBuffer) { llvm::function_ref Callback) const override {} - void reverseRelations(const RelationsRequest &Req, - llvm::function_ref - Callback) const override {} + void + reverseRelations(const RelationsRequest &Req, + llvm::function_ref + Callback) const override {} llvm::unique_function indexedFiles() const override { @@ -1733,8 +1734,9 @@ TEST(CrossFileRenameTests, DeduplicateRefsFromIndex) { llvm::function_ref) const override {} - void reverseRelations(const RelationsRequest &, - llvm::function_ref) + void + reverseRelations(const RelationsRequest &, + llvm::function_ref) const override {} llvm::unique_function From 2af024c7c2a0adf48876fb1205cc8f42fc559e59 Mon Sep 17 00:00:00 2001 From: Timon Ulrich Date: Tue, 14 Oct 2025 04:54:50 +0200 Subject: [PATCH 3/7] clangd: updated estimateMemoryUsage() to include ReverseRelations --- clang-tools-extra/clangd/index/MemIndex.cpp | 3 ++- clang-tools-extra/clangd/index/dex/Dex.cpp | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clangd/index/MemIndex.cpp b/clang-tools-extra/clangd/index/MemIndex.cpp index 238f68dd37d19..feac1cf4fb7a7 100644 --- a/clang-tools-extra/clangd/index/MemIndex.cpp +++ b/clang-tools-extra/clangd/index/MemIndex.cpp @@ -155,7 +155,8 @@ MemIndex::indexedFiles() const { size_t MemIndex::estimateMemoryUsage() const { return Index.getMemorySize() + Refs.getMemorySize() + - Relations.getMemorySize() + BackingDataSize; + Relations.getMemorySize() + ReverseRelations.getMemorySize() + + BackingDataSize; } } // namespace clangd diff --git a/clang-tools-extra/clangd/index/dex/Dex.cpp b/clang-tools-extra/clangd/index/dex/Dex.cpp index 925e256cc5759..d185b5313c53d 100644 --- a/clang-tools-extra/clangd/index/dex/Dex.cpp +++ b/clang-tools-extra/clangd/index/dex/Dex.cpp @@ -417,6 +417,7 @@ size_t Dex::estimateMemoryUsage() const { Bytes += Refs.getMemorySize(); Bytes += RevRefs.size() * sizeof(RevRef); Bytes += Relations.getMemorySize(); + Bytes += ReverseRelations.getMemorySize(); return Bytes + BackingDataSize; } From 6b6219fce1922652d86e052c0723ff8c7e1217d1 Mon Sep 17 00:00:00 2001 From: Timon Ulrich Date: Tue, 14 Oct 2025 04:59:42 +0200 Subject: [PATCH 4/7] clangd: fixed comment for ReverseRelations() --- clang-tools-extra/clangd/index/Index.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang-tools-extra/clangd/index/Index.h b/clang-tools-extra/clangd/index/Index.h index 0d34ea010b35d..f4b5871f8fe9b 100644 --- a/clang-tools-extra/clangd/index/Index.h +++ b/clang-tools-extra/clangd/index/Index.h @@ -183,8 +183,7 @@ class SymbolIndex { /// Finds all relations (O, P, S) stored in the index such that S is among /// Req.Subjects and P is Req.Predicate, and invokes \p Callback for (S, O) in - /// each. Currently only allows the OverridenBy relation and only stored in - /// memory. + /// each. Currently only allows the OverridenBy relation. virtual void reverseRelations( const RelationsRequest &Req, llvm::function_ref From 251d392151bea5d3787f81d9d3dfc2fc844f7338 Mon Sep 17 00:00:00 2001 From: Timon Ulrich Date: Tue, 14 Oct 2025 05:07:55 +0200 Subject: [PATCH 5/7] clangd: added ReverseRelations() to Client --- clang-tools-extra/clangd/index/remote/Client.cpp | 11 +++++++++++ clang-tools-extra/clangd/index/remote/Service.proto | 2 ++ 2 files changed, 13 insertions(+) diff --git a/clang-tools-extra/clangd/index/remote/Client.cpp b/clang-tools-extra/clangd/index/remote/Client.cpp index 79b827126b4ef..3b31a9fb67272 100644 --- a/clang-tools-extra/clangd/index/remote/Client.cpp +++ b/clang-tools-extra/clangd/index/remote/Client.cpp @@ -164,6 +164,17 @@ class IndexClient : public clangd::SymbolIndex { }); } + void reverseRelations( + const clangd::RelationsRequest &Request, + llvm::function_ref + Callback) const override { + streamRPC(Request, &remote::v1::SymbolIndex::Stub::ReverseRelations, + // Unpack protobuf Relation. + [&](std::pair SubjectAndObject) { + Callback(SubjectAndObject.first, SubjectAndObject.second); + }); + } + llvm::unique_function indexedFiles() const override { // FIXME: For now we always return IndexContents::None regardless of whether diff --git a/clang-tools-extra/clangd/index/remote/Service.proto b/clang-tools-extra/clangd/index/remote/Service.proto index 43023321cb9e1..3223298f608fc 100644 --- a/clang-tools-extra/clangd/index/remote/Service.proto +++ b/clang-tools-extra/clangd/index/remote/Service.proto @@ -24,4 +24,6 @@ service SymbolIndex { rpc ContainedRefs(ContainedRefsRequest) returns (stream ContainedRefsReply) {} rpc Relations(RelationsRequest) returns (stream RelationsReply) {} + + rpc ReverseRelations(RelationsRequest) returns (stream RelationsReply) {} } From aea66e457af8afd65461ffb8e832d44ecc0b44ad Mon Sep 17 00:00:00 2001 From: Timon Ulrich Date: Mon, 20 Oct 2025 09:45:36 +0200 Subject: [PATCH 6/7] clangd: Adding ReverseRelations to remote server and minox fixes --- clang-tools-extra/clangd/Protocol.h | 6 ++- clang-tools-extra/clangd/index/Index.h | 2 +- clang-tools-extra/clangd/index/MemIndex.h | 2 +- clang-tools-extra/clangd/index/dex/Dex.cpp | 1 + clang-tools-extra/clangd/index/dex/Dex.h | 2 +- .../clangd/index/remote/server/Server.cpp | 48 +++++++++++++++++++ 6 files changed, 56 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h index 0f51665d7b896..8946b8a5242db 100644 --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -1628,8 +1628,10 @@ struct CallHierarchyIncomingCall { /// This is relative to the caller denoted by `From`. std::vector fromRanges; - /// This caller might be a false positive. - /// We currently have no way of 100% confirming this. + /// For the case of being a virtual function we also return calls + /// to the base function. This caller might be a false positive. + /// We currently have no way of discerning this. + /// This is a clangd exntesion. bool mightNeverCall = false; }; llvm::json::Value toJSON(const CallHierarchyIncomingCall &); diff --git a/clang-tools-extra/clangd/index/Index.h b/clang-tools-extra/clangd/index/Index.h index f4b5871f8fe9b..b62b15d103112 100644 --- a/clang-tools-extra/clangd/index/Index.h +++ b/clang-tools-extra/clangd/index/Index.h @@ -183,7 +183,7 @@ class SymbolIndex { /// Finds all relations (O, P, S) stored in the index such that S is among /// Req.Subjects and P is Req.Predicate, and invokes \p Callback for (S, O) in - /// each. Currently only allows the OverridenBy relation. + /// each. Currently only allows the OverriddenBy relation. virtual void reverseRelations( const RelationsRequest &Req, llvm::function_ref diff --git a/clang-tools-extra/clangd/index/MemIndex.h b/clang-tools-extra/clangd/index/MemIndex.h index b8761c8723a74..8ece9994872a9 100644 --- a/clang-tools-extra/clangd/index/MemIndex.h +++ b/clang-tools-extra/clangd/index/MemIndex.h @@ -106,7 +106,7 @@ class MemIndex : public SymbolIndex { static_assert(sizeof(RelationKind) == sizeof(uint8_t), "RelationKind should be of same size as a uint8_t"); llvm::DenseMap, std::vector> Relations; - // Reverse relations, currently only for OverridenBy + // Reverse relations, currently only for OverriddenBy llvm::DenseMap, std::vector> ReverseRelations; // Set of files which were used during this index build. diff --git a/clang-tools-extra/clangd/index/dex/Dex.cpp b/clang-tools-extra/clangd/index/dex/Dex.cpp index d185b5313c53d..179d8c4da0b3e 100644 --- a/clang-tools-extra/clangd/index/dex/Dex.cpp +++ b/clang-tools-extra/clangd/index/dex/Dex.cpp @@ -382,6 +382,7 @@ void Dex::relations( void Dex::reverseRelations( const RelationsRequest &Req, llvm::function_ref Callback) const { + trace::Span Tracer("Dex reverseRelations"); assert(Req.Predicate == RelationKind::OverriddenBy); uint32_t Remaining = Req.Limit.value_or(std::numeric_limits::max()); for (const SymbolID &Subject : Req.Subjects) { diff --git a/clang-tools-extra/clangd/index/dex/Dex.h b/clang-tools-extra/clangd/index/dex/Dex.h index 4add20008cfcf..1ea7d8c06c67c 100644 --- a/clang-tools-extra/clangd/index/dex/Dex.h +++ b/clang-tools-extra/clangd/index/dex/Dex.h @@ -153,7 +153,7 @@ class Dex : public SymbolIndex { static_assert(sizeof(RelationKind) == sizeof(uint8_t), "RelationKind should be of same size as a uint8_t"); llvm::DenseMap, std::vector> Relations; - // Reverse relations, currently only for OverridenBy + // Reverse relations, currently only for OverriddenBy llvm::DenseMap, std::vector> ReverseRelations; std::shared_ptr KeepAlive; // poor man's move-only std::any diff --git a/clang-tools-extra/clangd/index/remote/server/Server.cpp b/clang-tools-extra/clangd/index/remote/server/Server.cpp index 890b6c27ed928..af9e9c3c8ff71 100644 --- a/clang-tools-extra/clangd/index/remote/server/Server.cpp +++ b/clang-tools-extra/clangd/index/remote/server/Server.cpp @@ -351,6 +351,54 @@ class RemoteIndexServer final : public v1::SymbolIndex::Service { return grpc::Status::OK; } + grpc::Status + ReverseRelations(grpc::ServerContext *Context, + const RelationsRequest *Request, + grpc::ServerWriter *Reply) override { + auto StartTime = stopwatch::now(); + WithContextValue WithRequestContext(CurrentRequest, Context); + logRequest(*Request); + trace::Span Tracer("ReverseRelationsRequest"); + auto Req = ProtobufMarshaller->fromProtobuf(Request); + if (!Req) { + elog("Can not parse ReverseRelationsRequest from protobuf: {0}", + Req.takeError()); + return grpc::Status::CANCELLED; + } + if (!Req->Limit || *Req->Limit > LimitResults) { + log("[public] Limiting result size for ReverseRelations request from {0} " + "to " + "{1}.", + Req->Limit, LimitResults); + Req->Limit = LimitResults; + } + unsigned Sent = 0; + unsigned FailedToSend = 0; + Index.reverseRelations( + *Req, [&](const SymbolID &Subject, const clangd::Symbol &Object) { + auto SerializedItem = ProtobufMarshaller->toProtobuf(Subject, Object); + if (!SerializedItem) { + elog("Unable to convert Relation to protobuf: {0}", + SerializedItem.takeError()); + ++FailedToSend; + return; + } + RelationsReply NextMessage; + *NextMessage.mutable_stream_result() = *SerializedItem; + logResponse(NextMessage); + Reply->Write(NextMessage); + ++Sent; + }); + RelationsReply LastMessage; + LastMessage.mutable_final_result()->set_has_more(true); + logResponse(LastMessage); + Reply->Write(LastMessage); + SPAN_ATTACH(Tracer, "Sent", Sent); + SPAN_ATTACH(Tracer, "Failed to send", FailedToSend); + logRequestSummary("v1/ReverseRelations", Sent, StartTime); + return grpc::Status::OK; + } + // Proxy object to allow proto messages to be lazily serialized as text. struct TextProto { const google::protobuf::Message &M; From 2cc4d00da1868cb608a01608f5561fac18961299 Mon Sep 17 00:00:00 2001 From: Nathan Ridge Date: Mon, 20 Oct 2025 22:08:02 -0400 Subject: [PATCH 7/7] fix typo in Protocol.h comment --- clang-tools-extra/clangd/Protocol.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h index 8946b8a5242db..2248572060431 100644 --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -1631,7 +1631,7 @@ struct CallHierarchyIncomingCall { /// For the case of being a virtual function we also return calls /// to the base function. This caller might be a false positive. /// We currently have no way of discerning this. - /// This is a clangd exntesion. + /// This is a clangd extension. bool mightNeverCall = false; }; llvm::json::Value toJSON(const CallHierarchyIncomingCall &);