https://github.com/yunusemreayhan updated https://github.com/llvm/llvm-project/pull/214344
>From cc3d34cd21e7c751c7436b6d673db6740b200299 Mon Sep 17 00:00:00 2001 From: Yunus Emre Ayhan <[email protected]> Date: Thu, 6 Aug 2026 00:36:37 +0300 Subject: [PATCH] [clangd] Fix call hierarchy to show all callers with same signature When multiple functions with the same signature (and therefore the same SymbolID) are called from different files - e.g. main() in different binaries calling a shared library function - incoming call hierarchy only showed one of the callers. The index stores a single Symbol per SymbolID, so incomingCalls() grouped all refs by SymbolID and collapsed them into a single caller. Group refs by (SymbolID, FileURI) so that each distinct call file is treated as a separate caller, and point each CallHierarchyItem at the file where its calls occur. To avoid a regression where a function declared in a different file than the calls (e.g. a macro-expanded function from a header) gets its call ranges misrepresented, only adopt the call file as the item's URI when the caller is actually defined in one of the call files. Fixes: https://github.com/clangd/clangd/issues/2361 --- clang-tools-extra/clangd/XRefs.cpp | 73 +++- .../clangd/unittests/CMakeLists.txt | 1 + .../CallHierarchyMultipleCallersTest.cpp | 347 ++++++++++++++++++ 3 files changed, 402 insertions(+), 19 deletions(-) create mode 100644 clang-tools-extra/clangd/unittests/CallHierarchyMultipleCallersTest.cpp diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index 6886d235811d53..0d815d69d8e085 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -2430,10 +2430,10 @@ incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) { // 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<SymbolID, std::vector<Location>> CallsIn; + // Group by (SymbolID, FileURI) to handle multiple callers with the same + // signature in different files (e.g., main() in different binaries). + using CallerKey = std::pair<SymbolID, std::string>; + std::map<CallerKey, std::vector<Location>> 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; @@ -2443,29 +2443,64 @@ incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) { elog("incomingCalls failed to convert location: {0}", Loc.takeError()); return; } - CallsIn[R.Container].push_back(*Loc); + // Group by both SymbolID and file to distinguish same-signature functions + CallerKey Key = {R.Container, Loc->uri.file().str()}; + CallsIn[Key].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<Range> 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; + // The caller's own location tells us which file it is defined in. If + // that file is one of the call files, then each distinct call file + // corresponds to a separate definition of the (same-signature) caller, + // so we point each item at the file where its calls occur. Otherwise the + // caller is declared in a different file than the calls (e.g. a + // macro-expanded function from a header), which the protocol cannot + // represent as ranges, so we keep the caller's own location. + auto SymLoc = Caller.Definition ? Caller.Definition + : Caller.CanonicalDeclaration; + auto SymFile = indexToLSPLocation(SymLoc, Item.uri.file()); + bool IsDefinedInCallFile = false; + if (SymFile) { + for (const auto &Other : CallsIn) { + if (Other.first.first == Caller.ID && + SymFile->uri.file() == Other.first.second) { + IsDefinedInCallFile = true; + break; } - FromRanges.push_back(L.range); } - Results.push_back(CallHierarchyIncomingCall{ - std::move(*CHI), std::move(FromRanges), MightNeverCall}); + } + + // Find all entries for this SymbolID (may be in multiple files) + for (auto &Entry : CallsIn) { + if (Entry.first.first != Caller.ID) + continue; + + if (auto CHI = symbolToCallHierarchyItem(Caller, Item.uri.file())) { + // Use the file from the key to ensure correct URI when the caller is + // defined in the call file (handles multiple functions with the same + // signature in different files, e.g. main() in different binaries). + if (IsDefinedInCallFile) { + CHI->uri = + URIForFile::canonicalize(Entry.first.second, Item.uri.file()); + } + + std::vector<Range> FromRanges; + for (const Location &L : Entry.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); + } + Results.push_back(CallHierarchyIncomingCall{ + std::move(*CHI), std::move(FromRanges), MightNeverCall}); + } } }); }; diff --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt b/clang-tools-extra/clangd/unittests/CMakeLists.txt index d596ba77efd4ae..960bddaab068a5 100644 --- a/clang-tools-extra/clangd/unittests/CMakeLists.txt +++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt @@ -36,6 +36,7 @@ add_unittest(ClangdUnitTests ClangdTests ASTSignalsTests.cpp BackgroundIndexTests.cpp CallHierarchyTests.cpp + CallHierarchyMultipleCallersTest.cpp CanonicalIncludesTests.cpp ClangdTests.cpp ClangdLSPServerTests.cpp diff --git a/clang-tools-extra/clangd/unittests/CallHierarchyMultipleCallersTest.cpp b/clang-tools-extra/clangd/unittests/CallHierarchyMultipleCallersTest.cpp new file mode 100644 index 00000000000000..445ef31be8c6b8 --- /dev/null +++ b/clang-tools-extra/clangd/unittests/CallHierarchyMultipleCallersTest.cpp @@ -0,0 +1,347 @@ +//===-- CallHierarchyMultipleCallersTest.cpp ---------------*- C++ -*------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// Regression tests for call hierarchy when multiple callers share the same +// function signature (and thus the same SymbolID) but are defined in different +// files. This is a common scenario when multiple binaries each define their +// own main() or other identically-named helper functions calling a shared +// library function. +// +// See https://github.com/clangd/clangd/issues/2361 +// +//===----------------------------------------------------------------------===// + +#include "Annotations.h" +#include "TestFS.h" +#include "TestWorkspace.h" +#include "XRefs.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace clang { +namespace clangd { +namespace { + +using ::testing::Field; +using ::testing::UnorderedElementsAre; + +MATCHER_P(withName, N, "") { return arg.name == N; } + +template <class ItemMatcher> +::testing::Matcher<CallHierarchyIncomingCall> from(ItemMatcher M) { + return Field(&CallHierarchyIncomingCall::from, M); +} + +// Reproduces a bug where multiple callers with the same function signature +// (e.g. main() in different binaries) only show up as a single caller in the +// call hierarchy. +// +// Scenario: +// - lib.cpp defines a shared function: util::doWork() +// - binary1_main.cpp has main() calling util::doWork() +// - binary2_main.cpp has main() calling util::doWork() +// - Expected: incomingCalls(util::doWork) shows main() from BOTH files +// - Bug: only one main() appears because they share the same SymbolID +TEST(CallHierarchyMultipleCallers, IncomingSameSignatureDifferentFiles) { + TestWorkspace Workspace; + + Workspace.addSource("util.h", R"cpp( + namespace util { + int doWork(int x); + } + )cpp"); + + Workspace.addMainFile("lib.cpp", R"cpp( + #include "util.h" + namespace util { + int doWork(int x) { return x * 2; } + } + )cpp"); + + // Binary 1: defines its own main() calling util::doWork() + Workspace.addMainFile("binary1_main.cpp", R"cpp( + #include "util.h" + int main() { + return util::doWork(42); + } + )cpp"); + + // Binary 2: defines its own main() calling util::doWork() + // Both main() functions have the same signature -> same SymbolID + Workspace.addMainFile("binary2_main.cpp", R"cpp( + #include "util.h" + int main() { + return util::doWork(99); + } + )cpp"); + + auto Index = Workspace.index(); + auto AST = Workspace.openFile("lib.cpp"); + ASSERT_TRUE(bool(AST)); + + Annotations Source(R"cpp( + #include "util.h" + namespace util { + int doW^ork(int x) { return x * 2; } + } + )cpp"); + + auto Items = prepareCallHierarchy(*AST, Source.point(), testPath("lib.cpp")); + ASSERT_EQ(Items.size(), 1u); + EXPECT_EQ(Items[0].name, "doWork"); + + auto Incoming = incomingCalls(Items[0], Index.get()); + + // main() from both binary1_main.cpp and binary2_main.cpp should appear. + EXPECT_EQ(Incoming.size(), 2u) + << "Expected 2 callers (main from binary1_main and binary2_main), got " + << Incoming.size(); + if (Incoming.size() >= 2) { + EXPECT_THAT(Incoming, + UnorderedElementsAre(from(withName("main")), + from(withName("main")))); + } +} + +// Variant where the callers are not main() but ordinary functions with the +// same name in different files, and each caller's URI points to the correct +// file. +TEST(CallHierarchyMultipleCallers, IncomingSameHelperInDifferentBinaries) { + TestWorkspace Workspace; + + Workspace.addSource("util.h", R"cpp( + namespace util { + int add(int a, int b); + } + )cpp"); + + Workspace.addMainFile("lib.cpp", R"cpp( + #include "util.h" + namespace util { + int add(int a, int b) { return a + b; } + } + )cpp"); + + // Binary 1: defines process() calling util::add() + Workspace.addMainFile("binary1_process.cpp", R"cpp( + #include "util.h" + int process() { + return util::add(1, 2); + } + )cpp"); + + // Binary 2: defines process() calling util::add() + // Same function name & signature -> same SymbolID + Workspace.addMainFile("binary2_process.cpp", R"cpp( + #include "util.h" + int process() { + return util::add(3, 4); + } + )cpp"); + + auto Index = Workspace.index(); + auto AST = Workspace.openFile("lib.cpp"); + ASSERT_TRUE(bool(AST)); + + Annotations Source(R"cpp( + #include "util.h" + namespace util { + int ad^d(int a, int b) { return a + b; } + } + )cpp"); + + auto Items = prepareCallHierarchy(*AST, Source.point(), testPath("lib.cpp")); + ASSERT_EQ(Items.size(), 1u); + EXPECT_EQ(Items[0].name, "add"); + + auto Incoming = incomingCalls(Items[0], Index.get()); + + // process() from both binary1_process.cpp and binary2_process.cpp. + EXPECT_EQ(Incoming.size(), 2u) + << "Expected 2 callers (process from binary1 and binary2), got " + << Incoming.size(); + if (Incoming.size() >= 2) { + EXPECT_THAT(Incoming, + UnorderedElementsAre(from(withName("process")), + from(withName("process")))); + } +} + +// Verifies that each caller's URI points to the correct file (not both to the +// same one). +TEST(CallHierarchyMultipleCallers, IncomingCallerURIPointsToCorrectFile) { + TestWorkspace Workspace; + + Workspace.addSource("util.h", R"cpp( + namespace util { + int add(int a, int b); + } + )cpp"); + + Workspace.addMainFile("lib.cpp", R"cpp( + #include "util.h" + namespace util { + int add(int a, int b) { return a + b; } + } + )cpp"); + + Workspace.addMainFile("binary1_main.cpp", R"cpp( + #include "util.h" + int main() { + return util::add(1, 2); + } + )cpp"); + + Workspace.addMainFile("binary2_main.cpp", R"cpp( + #include "util.h" + int main() { + return util::add(3, 4); + } + )cpp"); + + auto Index = Workspace.index(); + auto AST = Workspace.openFile("lib.cpp"); + ASSERT_TRUE(bool(AST)); + + Annotations Source(R"cpp( + #include "util.h" + namespace util { + int ad^d(int a, int b) { return a + b; } + } + )cpp"); + + auto Items = prepareCallHierarchy(*AST, Source.point(), testPath("lib.cpp")); + ASSERT_EQ(Items.size(), 1u); + + auto Incoming = incomingCalls(Items[0], Index.get()); + + ASSERT_EQ(Incoming.size(), 2u) + << "Expected 2 callers, got " << Incoming.size(); + + std::vector<std::string> Files; + for (const auto &Call : Incoming) + Files.push_back(Call.from.uri.file().str()); + + // Each caller should point to a different file. + ASSERT_NE(Files[0], Files[1]) + << "Both callers incorrectly point to the same file: " << Files[0]; + + auto Binary1Path = testPath("binary1_main.cpp"); + auto Binary2Path = testPath("binary2_main.cpp"); + EXPECT_EQ(Files[0], Binary1Path); + EXPECT_EQ(Files[1], Binary2Path); +} + +// Distinct callers with different names in the same file still work correctly +// (non-regression for the basic single-file case). +TEST(CallHierarchyMultipleCallers, IncomingDistinctCallersInSameFile) { + TestWorkspace Workspace; + + Workspace.addSource("util.h", R"cpp( + namespace util { + int add(int a, int b); + } + )cpp"); + + Workspace.addMainFile("lib.cpp", R"cpp( + #include "util.h" + namespace util { + int add(int a, int b) { return a + b; } + } + )cpp"); + + Workspace.addMainFile("main.cpp", R"cpp( + #include "util.h" + int caller1() { + return util::add(1, 2); + } + int caller2() { + return util::add(3, 4); + } + )cpp"); + + auto Index = Workspace.index(); + auto AST = Workspace.openFile("lib.cpp"); + ASSERT_TRUE(bool(AST)); + + Annotations Source(R"cpp( + #include "util.h" + namespace util { + int ad^d(int a, int b) { return a + b; } + } + )cpp"); + + auto Items = prepareCallHierarchy(*AST, Source.point(), testPath("lib.cpp")); + ASSERT_EQ(Items.size(), 1u); + + auto Incoming = incomingCalls(Items[0], Index.get()); + + EXPECT_EQ(Incoming.size(), 2u) + << "Expected 2 callers (caller1 and caller2), got " << Incoming.size(); + if (Incoming.size() >= 2) { + EXPECT_THAT(Incoming, + UnorderedElementsAre(from(withName("caller1")), + from(withName("caller2")))); + } +} + +// Stress test: many binaries with same-named functions calling the same +// library function. Each should appear as a separate caller. +TEST(CallHierarchyMultipleCallers, IncomingManySameSignatureCallers) { + TestWorkspace Workspace; + + Workspace.addSource("util.h", R"cpp( + namespace util { + int add(int a, int b); + } + )cpp"); + + Workspace.addMainFile("lib.cpp", R"cpp( + #include "util.h" + namespace util { + int add(int a, int b) { return a + b; } + } + )cpp"); + + // Create 5 binaries, each with their own process() calling util::add(). + const int NumBinaries = 5; + for (int I = 0; I < NumBinaries; ++I) { + std::string Filename = "binary" + std::to_string(I) + "_process.cpp"; + std::string Code = + "#include \"util.h\"\n" + "int process() {\n" + " return util::add(" + std::to_string(I) + ", " + + std::to_string(I + 1) + ");\n" + "}\n"; + Workspace.addMainFile(Filename, Code); + } + + auto Index = Workspace.index(); + auto AST = Workspace.openFile("lib.cpp"); + ASSERT_TRUE(bool(AST)); + + Annotations Source(R"cpp( + #include "util.h" + namespace util { + int ad^d(int a, int b) { return a + b; } + } + )cpp"); + + auto Items = prepareCallHierarchy(*AST, Source.point(), testPath("lib.cpp")); + ASSERT_EQ(Items.size(), 1u); + + auto Incoming = incomingCalls(Items[0], Index.get()); + + EXPECT_EQ(Incoming.size(), static_cast<size_t>(NumBinaries)) + << "Expected " << NumBinaries + << " callers (process from each binary), got " << Incoming.size(); +} + +} // namespace +} // namespace clangd +} // namespace clang \ No newline at end of file _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
