[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:36:35 PDT 2024


================
@@ -2292,9 +2289,26 @@ incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) {
   Index->lookup(ContainerLookup, [&](const Symbol &Caller) {
     auto It = CallsIn.find(Caller.ID);
     assert(It != CallsIn.end());
-    if (auto CHI = symbolToCallHierarchyItem(Caller, Item.uri.file()))
+    if (auto CHI = symbolToCallHierarchyItem(Caller, Item.uri.file())) {
+      SymbolLocation CallerLoc =
+          Caller.Definition ? Caller.Definition : Caller.CanonicalDeclaration;
+      std::vector<Range> FromRanges;
+      for (const SymbolLocation &L : It->second) {
+        if (StringRef{L.FileURI} != StringRef{CallerLoc.FileURI}) {
+          elog("incomingCalls: call location not in same file as caller, this "
----------------
HighCommander4 wrote:

Thanks for the example! This is exactly the sort of case the hardening was intended to catch, though I didn't think of it.

The case raises an interesting question: what **should** our behaviour be for an example like this, where the function definition is spread across multiple files?

The [protocol representation](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#callHierarchy_incomingCalls) only allows specifying one file per caller:

```ts
export interface CallHierarchyIncomingCall {

	/**
	 * The item that makes the call.
	 */
	from: CallHierarchyItem;

	/**
	 * The ranges at which the calls appear. This is relative to the caller
	 * denoted by [`this.from`](#CallHierarchyIncomingCall.from).
	 */
	fromRanges: Range[];
}

```

Here, `from.uri` is the only thing that encodes a file; the ranges in `fromRanges` encode a pair of line/column positions only, and are assumed to be in the same file as `from.uri`.

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


More information about the cfe-commits mailing list