https://github.com/HighCommander4 updated
https://github.com/llvm/llvm-project/pull/134657
>From 162b0535fc2ae745441404231e6b3bf3882b974e Mon Sep 17 00:00:00 2001
From: Hampus Adolfsson <hampus.adolfs...@iar.com>
Date: Mon, 7 Apr 2025 14:51:03 +0200
Subject: [PATCH 1/2] [clangd] Strip invalid fromRanges for outgoing calls
`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.
---
clang-tools-extra/clangd/XRefs.cpp | 23 +++++++++++++++----
.../clangd/unittests/CallHierarchyTests.cpp | 13 +++++++----
2 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/clang-tools-extra/clangd/XRefs.cpp
b/clang-tools-extra/clangd/XRefs.cpp
index 8b9fffa3f64cd..54b99c9a66a68 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 different 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..7d69b64bafd7f 100644
--- a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
@@ -383,7 +383,8 @@ 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")));
@@ -392,9 +393,11 @@ TEST(CallHierarchy, MultiFileCpp) {
OutgoingLevel1,
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 +426,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 +434,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) {
>From e091335555cc0deaa6a051616a867767f9e6c326 Mon Sep 17 00:00:00 2001
From: Nathan Ridge <zeratul...@hotmail.com>
Date: Fri, 11 Apr 2025 02:00:50 -0400
Subject: [PATCH 2/2] Tweak to test
---
.../clangd/unittests/CallHierarchyTests.cpp | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
b/clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
index 7d69b64bafd7f..b9b3325c57618 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>
@@ -387,10 +388,17 @@ TEST(CallHierarchy, MultiFileCpp) {
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"))),
IsDeclaration ? oFromRanges()
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
