[clang-tools-extra] bea110d - [clangd] Strip invalid fromRanges for outgoing calls (#134657)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 24 00:28:41 PDT 2025
Author: Hampus Adolfsson
Date: 2025-04-24T03:28:35-04:00
New Revision: bea110db3ed1fa1215bb8e22d2057019fcbd2d16
URL: https://github.com/llvm/llvm-project/commit/bea110db3ed1fa1215bb8e22d2057019fcbd2d16
DIFF: https://github.com/llvm/llvm-project/commit/bea110db3ed1fa1215bb8e22d2057019fcbd2d16.diff
LOG: [clangd] Strip invalid fromRanges for outgoing calls (#134657)
`CallHierarchyOutgoingCall::fromRanges` are interpreted as ranges in the
same file as the item for which 'outgoingCalls' was called.
It's possible for outgoing calls to be in a different file than that
item if the item is just a declaration (e.g. in a header file). Now,
such calls are dropped instead of being returned to the client.
This is the same as the change made in #111616, but now for outgoing
calls.
Fixes clangd/clangd#2350
---------
Co-authored-by: Nathan Ridge <zeratul976 at hotmail.com>
Added:
Modified:
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp
index 053e2c044c774..089f8158c9aa5 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -2380,7 +2380,7 @@ outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) {
// Initially store the ranges in a map keyed by SymbolID of the callee.
// This allows us to group
diff erent calls to the same function
// into the same CallHierarchyOutgoingCall.
- llvm::DenseMap<SymbolID, std::vector<Range>> CallsOut;
+ llvm::DenseMap<SymbolID, std::vector<Location>> CallsOut;
// We can populate the ranges based on a refs request only. As we do so, we
// also accumulate the callee IDs into a lookup request.
LookupRequest CallsOutLookup;
@@ -2390,8 +2390,8 @@ outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) {
elog("outgoingCalls failed to convert location: {0}", Loc.takeError());
return;
}
- auto It = CallsOut.try_emplace(R.Symbol, std::vector<Range>{}).first;
- It->second.push_back(Loc->range);
+ auto It = CallsOut.try_emplace(R.Symbol, std::vector<Location>{}).first;
+ It->second.push_back(*Loc);
CallsOutLookup.IDs.insert(R.Symbol);
});
@@ -2411,9 +2411,22 @@ outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) {
auto It = CallsOut.find(Callee.ID);
assert(It != CallsOut.end());
- if (auto CHI = symbolToCallHierarchyItem(Callee, Item.uri.file()))
+ if (auto CHI = symbolToCallHierarchyItem(Callee, Item.uri.file())) {
+ std::vector<Range> FromRanges;
+ for (const Location &L : It->second) {
+ if (L.uri != Item.uri) {
+ // Call location not in same file as the item that outgoingCalls was
+ // requested for. This can happen when Item is a declaration separate
+ // from the implementation. There's not much we can do, since the
+ // protocol only allows returning ranges interpreted as being in
+ // Item's file.
+ continue;
+ }
+ FromRanges.push_back(L.range);
+ }
Results.push_back(
- CallHierarchyOutgoingCall{std::move(*CHI), std::move(It->second)});
+ CallHierarchyOutgoingCall{std::move(*CHI), std::move(FromRanges)});
+ }
});
// Sort results by name of the callee.
llvm::sort(Results, [](const CallHierarchyOutgoingCall &A,
diff --git a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
index 316b94305c9ae..eb852ef5ee00b 100644
--- a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
@@ -45,6 +45,7 @@ using ::testing::UnorderedElementsAre;
// Helpers for matching call hierarchy data structures.
MATCHER_P(withName, N, "") { return arg.name == N; }
MATCHER_P(withDetail, N, "") { return arg.detail == N; }
+MATCHER_P(withFile, N, "") { return arg.uri.file() == N; }
MATCHER_P(withSelectionRange, R, "") { return arg.selectionRange == R; }
template <class ItemMatcher>
@@ -383,18 +384,28 @@ TEST(CallHierarchy, MultiFileCpp) {
EXPECT_THAT(IncomingLevel4, IsEmpty());
};
- auto CheckOutgoingCalls = [&](ParsedAST &AST, Position Pos, PathRef TUPath) {
+ auto CheckOutgoingCalls = [&](ParsedAST &AST, Position Pos, PathRef TUPath,
+ bool IsDeclaration) {
std::vector<CallHierarchyItem> Items =
prepareCallHierarchy(AST, Pos, TUPath);
- ASSERT_THAT(Items, ElementsAre(withName("caller3")));
+ ASSERT_THAT(
+ Items,
+ ElementsAre(AllOf(
+ withName("caller3"),
+ withFile(testPath(IsDeclaration ? "caller3.hh" : "caller3.cc")))));
auto OutgoingLevel1 = outgoingCalls(Items[0], Index.get());
ASSERT_THAT(
OutgoingLevel1,
+ // fromRanges are interpreted in the context of Items[0]'s file.
+ // If that's the header, we can't get ranges from the implementation
+ // file!
ElementsAre(
AllOf(to(AllOf(withName("caller1"), withDetail("nsa::caller1"))),
- oFromRanges(Caller3C.range("Caller1"))),
+ IsDeclaration ? oFromRanges()
+ : oFromRanges(Caller3C.range("Caller1"))),
AllOf(to(AllOf(withName("caller2"), withDetail("nsb::caller2"))),
- oFromRanges(Caller3C.range("Caller2")))));
+ IsDeclaration ? oFromRanges()
+ : oFromRanges(Caller3C.range("Caller2")))));
auto OutgoingLevel2 = outgoingCalls(OutgoingLevel1[1].to, Index.get());
ASSERT_THAT(OutgoingLevel2,
@@ -423,7 +434,7 @@ TEST(CallHierarchy, MultiFileCpp) {
CheckIncomingCalls(*AST, CalleeH.point(), testPath("callee.hh"));
AST = Workspace.openFile("caller3.hh");
ASSERT_TRUE(bool(AST));
- CheckOutgoingCalls(*AST, Caller3H.point(), testPath("caller3.hh"));
+ CheckOutgoingCalls(*AST, Caller3H.point(), testPath("caller3.hh"), true);
// Check that invoking from the definition site works.
AST = Workspace.openFile("callee.cc");
@@ -431,7 +442,7 @@ TEST(CallHierarchy, MultiFileCpp) {
CheckIncomingCalls(*AST, CalleeC.point(), testPath("callee.cc"));
AST = Workspace.openFile("caller3.cc");
ASSERT_TRUE(bool(AST));
- CheckOutgoingCalls(*AST, Caller3C.point(), testPath("caller3.cc"));
+ CheckOutgoingCalls(*AST, Caller3C.point(), testPath("caller3.cc"), false);
}
TEST(CallHierarchy, IncomingMultiFileObjC) {
More information about the cfe-commits
mailing list