[clangd] Show callers of base functions in incomingCalls (#163024)

If call hierarchy incoming calls is invoked on a virtual function, clangd
now returns the callers of base functions as well.

The patch also introduces a protocol extension to annotate such calls
differently (as they may or may not actually call the target function),
so that clients can visualize these callers differently if they wish.

Author: timon-ul

clang-tools-extra/clangd/Protocol.h

@@ -1627,6 +1627,12 @@ struct CallHierarchyIncomingCall {
   /// The range at which the calls appear.
   /// This is relative to the caller denoted by `From`.
   std::vector<Range> fromRanges;
+
+  /// For the case of being a virtual function we also return calls
+  /// to the base function. This caller might be a false positive.
+  /// We currently have no way of discerning this.
+  /// This is a clangd extension.
+  bool mightNeverCall = false;
 };
 llvm::json::Value toJSON(const CallHierarchyIncomingCall &);
 

clang-tools-extra/clangd/XRefs.cpp

@@ -21,6 +21,7 @@
 #include "clang-include-cleaner/Types.h"
 #include "index/Index.h"
 #include "index/Merge.h"
+#include "index/Ref.h"
 #include "index/Relation.h"
 #include "index/SymbolCollector.h"
 #include "index/SymbolID.h"
@@ -56,6 +57,7 @@
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallSet.h"
@@ -66,6 +68,7 @@
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
+#include <algorithm>
 #include <optional>
 #include <string>
 #include <vector>
@@ -2350,51 +2353,64 @@ incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) {
   // to an AST node isn't cheap, particularly when the declaration isn't
   // in the main file.
   // FIXME: Consider also using AST information when feasible.
-  RefsRequest Request;
-  Request.IDs.insert(*ID);
-  Request.WantContainer = true;
-  // We could restrict more specifically to calls by introducing a new RefKind,
-  // but non-call references (such as address-of-function) can still be
-  // interesting as they can indicate indirect calls.
-  Request.Filter = RefKind::Reference;
-  // 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<Location>> 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;
-    }
-    CallsIn[R.Container].push_back(*Loc);
+  auto QueryIndex = [&](llvm::DenseSet<SymbolID> IDs, bool MightNeverCall) {
+    RefsRequest Request;
+    Request.IDs = std::move(IDs);
+    Request.WantContainer = true;
+    // We could restrict more specifically to calls by introducing a new
+    // RefKind, but non-call references (such as address-of-function) can still
+    // be interesting as they can indicate indirect calls.
+    Request.Filter = RefKind::Reference;
+    // 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<Location>> 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;
+      }
+      CallsIn[R.Container].push_back(*Loc);
 
-    ContainerLookup.IDs.insert(R.Container);
-  });
-  // Perform the lookup request and combine its results with CallsIn to
-  // get complete CallHierarchyIncomingCall objects.
-  Index->lookup(ContainerLookup, [&](const Symbol &Caller) {
-    auto It = CallsIn.find(Caller.ID);
-    assert(It != CallsIn.end());
-    if (auto CHI = symbolToCallHierarchyItem(Caller, Item.uri.file())) {
-      std::vector<Range> FromRanges;
-      for (const Location &L : It->second) {
-        if (L.uri != CHI->uri) {
-          // Call location not in same file as caller.
-          // This can happen in some edge cases. There's not much we can do,
-          // since the protocol only allows returning ranges interpreted as
-          // being in the caller's file.
-          continue;
+      ContainerLookup.IDs.insert(R.Container);
+    });
+    // Perform the lookup request and combine its results with CallsIn to
+    // get complete CallHierarchyIncomingCall objects.
+    Index->lookup(ContainerLookup, [&](const Symbol &Caller) {
+      auto It = CallsIn.find(Caller.ID);
+      assert(It != CallsIn.end());
+      if (auto CHI = symbolToCallHierarchyItem(Caller, Item.uri.file())) {
+        std::vector<Range> FromRanges;
+        for (const Location &L : It->second) {
+          if (L.uri != CHI->uri) {
+            // Call location not in same file as caller.
+            // This can happen in some edge cases. There's not much we can do,
+            // since the protocol only allows returning ranges interpreted as
+            // being in the caller's file.
+            continue;
+          }
+          FromRanges.push_back(L.range);
         }
-        FromRanges.push_back(L.range);
+        Results.push_back(CallHierarchyIncomingCall{
+            std::move(*CHI), std::move(FromRanges), MightNeverCall});
       }
-      Results.push_back(
-          CallHierarchyIncomingCall{std::move(*CHI), std::move(FromRanges)});
-    }
-  });
+    });
+  };
+  QueryIndex({ID.get()}, false);
+  // In the case of being a virtual function we also want to return
+  // potential calls through the base function.
+  if (Item.kind == SymbolKind::Method) {
+    llvm::DenseSet<SymbolID> IDs;
+    RelationsRequest Req{{ID.get()}, RelationKind::OverriddenBy, std::nullopt};
+    Index->reverseRelations(Req, [&](const SymbolID &, const Symbol &Caller) {
+      IDs.insert(Caller.ID);
+    });
+    QueryIndex(std::move(IDs), true);
+  }
   // Sort results by name of container.
   llvm::sort(Results, [](const CallHierarchyIncomingCall &A,
                          const CallHierarchyIncomingCall &B) {

clang-tools-extra/clangd/index/Index.cpp

@@ -77,6 +77,12 @@ void SwapIndex::relations(
   return snapshot()->relations(R, CB);
 }
 
+void SwapIndex::reverseRelations(
+    const RelationsRequest &R,
+    llvm::function_ref<void(const SymbolID &, const Symbol &)> CB) const {
+  return snapshot()->reverseRelations(R, CB);
+}
+
 llvm::unique_function<IndexContents(llvm::StringRef) const>
 SwapIndex::indexedFiles() const {
   // The index snapshot should outlive this method return value.

clang-tools-extra/clangd/index/Index.h

@@ -181,6 +181,14 @@ class SymbolIndex {
       llvm::function_ref<void(const SymbolID &Subject, const Symbol &Object)>
           Callback) const = 0;
 
+  /// Finds all relations (O, P, S) stored in the index such that S is among
+  /// Req.Subjects and P is Req.Predicate, and invokes \p Callback for (S, O) in
+  /// each. Currently only allows the OverriddenBy relation.
+  virtual void reverseRelations(
+      const RelationsRequest &Req,
+      llvm::function_ref<void(const SymbolID &Subject, const Symbol &Object)>
+          Callback) const = 0;
+
   /// Returns function which checks if the specified file was used to build this
   /// index or not. The function must only be called while the index is alive.
   using IndexedFiles =
@@ -214,6 +222,11 @@ class SwapIndex : public SymbolIndex {
                  llvm::function_ref<void(const SymbolID &, const Symbol &)>)
       const override;
 
+  void
+  reverseRelations(const RelationsRequest &,
+                   llvm::function_ref<void(const SymbolID &, const Symbol &)>)
+      const override;
+
   llvm::unique_function<IndexContents(llvm::StringRef) const>
   indexedFiles() const override;
 

clang-tools-extra/clangd/index/MemIndex.cpp

@@ -125,6 +125,27 @@ void MemIndex::relations(
   }
 }
 
+void MemIndex::reverseRelations(
+    const RelationsRequest &Req,
+    llvm::function_ref<void(const SymbolID &, const Symbol &)> Callback) const {
+  assert(Req.Predicate == RelationKind::OverriddenBy);
+  uint32_t Remaining = Req.Limit.value_or(std::numeric_limits<uint32_t>::max());
+  for (const SymbolID &Subject : Req.Subjects) {
+    LookupRequest LookupReq;
+    auto It = ReverseRelations.find(
+        std::make_pair(Subject, static_cast<uint8_t>(Req.Predicate)));
+    if (It != ReverseRelations.end()) {
+      for (const auto &Obj : It->second) {
+        if (Remaining > 0) {
+          --Remaining;
+          LookupReq.IDs.insert(Obj);
+        }
+      }
+    }
+    lookup(LookupReq, [&](const Symbol &Object) { Callback(Subject, Object); });
+  }
+}
+
 llvm::unique_function<IndexContents(llvm::StringRef) const>
 MemIndex::indexedFiles() const {
   return [this](llvm::StringRef FileURI) {
@@ -134,7 +155,8 @@ MemIndex::indexedFiles() const {
 
 size_t MemIndex::estimateMemoryUsage() const {
   return Index.getMemorySize() + Refs.getMemorySize() +
-         Relations.getMemorySize() + BackingDataSize;
+         Relations.getMemorySize() + ReverseRelations.getMemorySize() +
+         BackingDataSize;
 }
 
 } // namespace clangd

clang-tools-extra/clangd/index/MemIndex.h

@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_MEMINDEX_H
 
 #include "index/Index.h"
+#include "index/Relation.h"
 #include "llvm/ADT/StringSet.h"
 #include <mutex>
 
@@ -27,10 +28,16 @@ class MemIndex : public SymbolIndex {
       Index[S.ID] = &S;
     for (const std::pair<SymbolID, llvm::ArrayRef<Ref>> &R : Refs)
       this->Refs.try_emplace(R.first, R.second.begin(), R.second.end());
-    for (const Relation &R : Relations)
+    for (const Relation &R : Relations) {
       this->Relations[std::make_pair(R.Subject,
                                      static_cast<uint8_t>(R.Predicate))]
           .push_back(R.Object);
+      if (R.Predicate == RelationKind::OverriddenBy) {
+        this->ReverseRelations[std::make_pair(
+                                   R.Object, static_cast<uint8_t>(R.Predicate))]
+            .push_back(R.Subject);
+      }
+    }
   }
   // Symbols are owned by BackingData, Index takes ownership.
   template <typename SymbolRange, typename RefRange, typename RelationRange,
@@ -80,6 +87,11 @@ class MemIndex : public SymbolIndex {
                  llvm::function_ref<void(const SymbolID &, const Symbol &)>
                      Callback) const override;
 
+  void
+  reverseRelations(const RelationsRequest &Req,
+                   llvm::function_ref<void(const SymbolID &, const Symbol &)>
+                       Callback) const override;
+
   llvm::unique_function<IndexContents(llvm::StringRef) const>
   indexedFiles() const override;
 
@@ -94,6 +106,9 @@ class MemIndex : public SymbolIndex {
   static_assert(sizeof(RelationKind) == sizeof(uint8_t),
                 "RelationKind should be of same size as a uint8_t");
   llvm::DenseMap<std::pair<SymbolID, uint8_t>, std::vector<SymbolID>> Relations;
+  // Reverse relations, currently only for OverriddenBy
+  llvm::DenseMap<std::pair<SymbolID, uint8_t>, std::vector<SymbolID>>
+      ReverseRelations;
   // Set of files which were used during this index build.
   llvm::StringSet<> Files;
   // Contents of the index (symbols, references, etc.)

clang-tools-extra/clangd/index/Merge.cpp

@@ -221,6 +221,32 @@ void MergedIndex::relations(
   });
 }
 
+void MergedIndex::reverseRelations(
+    const RelationsRequest &Req,
+    llvm::function_ref<void(const SymbolID &, const Symbol &)> Callback) const {
+  uint32_t Remaining = Req.Limit.value_or(std::numeric_limits<uint32_t>::max());
+  // Return results from both indexes but avoid duplicates.
+  // We might return stale relations from the static index;
+  // we don't currently have a good way of identifying them.
+  llvm::DenseSet<std::pair<SymbolID, SymbolID>> SeenRelations;
+  Dynamic->reverseRelations(
+      Req, [&](const SymbolID &Subject, const Symbol &Object) {
+        Callback(Subject, Object);
+        SeenRelations.insert(std::make_pair(Subject, Object.ID));
+        --Remaining;
+      });
+  if (Remaining == 0)
+    return;
+  Static->reverseRelations(
+      Req, [&](const SymbolID &Subject, const Symbol &Object) {
+        if (Remaining > 0 &&
+            !SeenRelations.count(std::make_pair(Subject, Object.ID))) {
+          --Remaining;
+          Callback(Subject, Object);
+        }
+      });
+}
+
 // Returns true if \p L is (strictly) preferred to \p R (e.g. by file paths). If
 // neither is preferred, this returns false.
 static bool prefer(const SymbolLocation &L, const SymbolLocation &R) {

clang-tools-extra/clangd/index/Merge.h

@@ -44,6 +44,10 @@ class MergedIndex : public SymbolIndex {
   void relations(const RelationsRequest &,
                  llvm::function_ref<void(const SymbolID &, const Symbol &)>)
       const override;
+  void
+  reverseRelations(const RelationsRequest &,
+                   llvm::function_ref<void(const SymbolID &, const Symbol &)>)
+      const override;
   llvm::unique_function<IndexContents(llvm::StringRef) const>
   indexedFiles() const override;
   size_t estimateMemoryUsage() const override {

clang-tools-extra/clangd/index/ProjectAware.cpp

@@ -51,6 +51,11 @@ class ProjectAwareIndex : public SymbolIndex {
                  llvm::function_ref<void(const SymbolID &, const Symbol &)>
                      Callback) const override;
 
+  void
+  reverseRelations(const RelationsRequest &,
+                   llvm::function_ref<void(const SymbolID &, const Symbol &)>)
+      const override;
+
   llvm::unique_function<IndexContents(llvm::StringRef) const>
   indexedFiles() const override;
 
@@ -124,6 +129,14 @@ void ProjectAwareIndex::relations(
     return Idx->relations(Req, Callback);
 }
 
+void ProjectAwareIndex::reverseRelations(
+    const RelationsRequest &Req,
+    llvm::function_ref<void(const SymbolID &, const Symbol &)> Callback) const {
+  trace::Span Tracer("ProjectAwareIndex::relations");
+  if (auto *Idx = getIndex())
+    return Idx->reverseRelations(Req, Callback);
+}
+
 llvm::unique_function<IndexContents(llvm::StringRef) const>
 ProjectAwareIndex::indexedFiles() const {
   trace::Span Tracer("ProjectAwareIndex::indexedFiles");

clang-tools-extra/clangd/index/dex/Dex.cpp

@@ -379,6 +379,28 @@ void Dex::relations(
   }
 }
 
+void Dex::reverseRelations(
+    const RelationsRequest &Req,
+    llvm::function_ref<void(const SymbolID &, const Symbol &)> Callback) const {
+  trace::Span Tracer("Dex reverseRelations");
+  assert(Req.Predicate == RelationKind::OverriddenBy);
+  uint32_t Remaining = Req.Limit.value_or(std::numeric_limits<uint32_t>::max());
+  for (const SymbolID &Subject : Req.Subjects) {
+    LookupRequest LookupReq;
+    auto It = ReverseRelations.find(
+        std::make_pair(Subject, static_cast<uint8_t>(Req.Predicate)));
+    if (It != ReverseRelations.end()) {
+      for (const auto &Obj : It->second) {
+        if (Remaining > 0) {
+          --Remaining;
+          LookupReq.IDs.insert(Obj);
+        }
+      }
+    }
+    lookup(LookupReq, [&](const Symbol &Object) { Callback(Subject, Object); });
+  }
+}
+
 llvm::unique_function<IndexContents(llvm::StringRef) const>
 Dex::indexedFiles() const {
   return [this](llvm::StringRef FileURI) {
@@ -396,6 +418,7 @@ size_t Dex::estimateMemoryUsage() const {
   Bytes += Refs.getMemorySize();
   Bytes += RevRefs.size() * sizeof(RevRef);
   Bytes += Relations.getMemorySize();
+  Bytes += ReverseRelations.getMemorySize();
   return Bytes + BackingDataSize;
 }