[clang-tools-extra] [clangd] Harden incomingCalls() against possible misinterpretation of a range as pertaining to the wrong file (PR #111616)

Nathan Ridge via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 22 23:25:06 PDT 2024


================
@@ -2272,18 +2273,14 @@ incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) {
   // 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<Range>> CallsIn;
+  llvm::DenseMap<SymbolID, std::vector<SymbolLocation>> 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;
   Index->refs(Request, [&](const Ref &R) {
-    auto Loc = indexToLSPLocation(R.Location, Item.uri.file());
-    if (!Loc) {
-      elog("incomingCalls failed to convert location: {0}", Loc.takeError());
-      return;
-    }
-    auto It = CallsIn.try_emplace(R.Container, std::vector<Range>{}).first;
-    It->second.push_back(Loc->range);
+    auto It =
+        CallsIn.try_emplace(R.Container, std::vector<SymbolLocation>{}).first;
+    It->second.push_back(R.Location);
----------------
HighCommander4 wrote:

Good catch, thanks. I figured `FileURI` would point to storage inside the ref slab which would stay alive for the lifetime of the index, but I do see now that the [comment above](https://searchfox.org/llvm/rev/91c11574e87b5c0f434688edac01e9580ef99a92/clang-tools-extra/clangd/index/Index.h#137) of `SymbolIndex::refs()` says "The returned result must be deep-copied if it's used outside Callback", so while it may work for some index implementations we can't rely on it.

https://github.com/llvm/llvm-project/pull/111616


More information about the cfe-commits mailing list