[clang-tools-extra] [clangd] Strip invalid fromRanges for outgoing calls (PR #134657)

[Hampus Adolfsson via cfe-commits]

https://github.com/HampusAdolfsson created 
https://github.com/llvm/llvm-project/pull/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.

I originally added a standalone test for this, but realised the `MultiFileCpp` 
test already touches this code.

This is the same as the change made in #111616, but now for outgoing calls.

Closes clangd/clangd#2350

>From 162b0535fc2ae745441404231e6b3bf3882b974e Mon Sep 17 00:00:00 2001
From: Hampus Adolfsson 
Date: Mon, 7 Apr 2025 14:51:03 +0200
Subject: [PATCH] [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> CallsOut;
+  llvm::DenseMap> 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{}).first;
-It->second.push_back(Loc->range);
+auto It = CallsOut.try_emplace(R.Symbol, std::vector{}).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 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 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");
+  

[clang-tools-extra] [clangd] Strip invalid fromRanges for outgoing calls (PR #134657)

[Hampus Adolfsson via cfe-commits]

HampusAdolfsson wrote:

@HighCommander4 Thanks, looks good to me! I don't have commit access, so feel 
free to merge this if you're happy with it.

https://github.com/llvm/llvm-project/pull/134657
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Strip invalid fromRanges for outgoing calls (PR #134657)

[Hampus Adolfsson via cfe-commits]

https://github.com/HampusAdolfsson updated 
https://github.com/llvm/llvm-project/pull/134657

>From 162b0535fc2ae745441404231e6b3bf3882b974e Mon Sep 17 00:00:00 2001
From: Hampus Adolfsson 
Date: Mon, 7 Apr 2025 14:51:03 +0200
Subject: [PATCH 1/3] [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> CallsOut;
+  llvm::DenseMap> 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{}).first;
-It->second.push_back(Loc->range);
+auto It = CallsOut.try_emplace(R.Symbol, std::vector{}).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 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 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("cal