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 <zeratul...@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) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
