Skip to content

Commit d683c7d

Browse files
committed
Park cross-repo member calls in C++, C# and Swift too (#3152)
The merge-time pass only had Java residue to work with, so a C++, C# or Swift call whose receiver type lives in another repo was still dropped with the type already in hand. Each of those three resolvers has the same bail-out Java had — receiver typed, type declared nowhere in this corpus — so each now parks the call by name for the merge to finish. Two details the added languages force: * Swift shares `all_raw_calls` with every other language and its raw_calls carry no `lang` tag, so the parked entry's language comes from the declaring file's suffix rather than from a tag that isn't there. * C# collapses "absent", "ambiguous" and "scoping was decisive" into one `None` from `_resolve_type_name_nid`, and only the first is a cross-repo candidate, so the park re-checks the bare-name index instead of trusting the `None`. On the merge side, a C++ class that only declares `void bar();` in a header owns it through `defines`, not `method`, so the member index is kept per relation: a C++ entry may fall back to `defines` when no `method` of that name exists, and for every other language a `defines` target is a field and cannot answer a call. Tests cover a real two-repo build per language (both C++ receiver shapes), the `defines` owner and its language restriction, the definition winning over a same-named header declaration, and C++ not binding to a C# class of the same name.
1 parent 70df839 commit d683c7d

3 files changed

Lines changed: 314 additions & 107 deletions

File tree

‎graphify/cross_repo_calls.py‎

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,17 @@
3939
# just as happily to a Python class of the same name in another repo. Extend this
4040
# map when another extractor starts parking calls.
4141
_LANG_SUFFIXES: dict[str, frozenset[str]] = {
42+
"cpp": frozenset({".cpp", ".cc", ".cxx", ".hpp", ".hh", ".hxx", ".h", ".cu", ".cuh"}),
43+
"csharp": frozenset({".cs"}),
4244
"java": frozenset({".java"}),
45+
"swift": frozenset({".swift"}),
4346
}
4447

48+
# A declaration owns its members through a `method` edge, except in C++, where an
49+
# in-class declaration (`void bar();` in a header) is modelled as a field and
50+
# carries `defines` instead. `method` wins when both name the same member.
51+
_MEMBER_RELATIONS = ("defines", "method")
52+
4553

4654
def _key(label: object) -> str:
4755
"""Normalize a node label or a parked name to its bare identifier.
@@ -97,17 +105,26 @@ def _index_declarations(merged: "nx.Graph") -> tuple[dict[str, list[str]], set[s
97105
return by_name, type_ids
98106

99107

100-
def _index_methods(merged: "nx.Graph", type_ids: set[str]) -> dict[tuple[str, str], list[str]]:
101-
"""Index each declaration's methods by name.
108+
def _index_members(
109+
merged: "nx.Graph", type_ids: set[str]
110+
) -> dict[str, dict[tuple[str, str], list[str]]]:
111+
"""Index each declaration's members by relation, then by name.
102112
103113
The merged graph is undirected, and a ``method`` edge carries no reliable
104114
direction once composed, so the owner is identified as the endpoint that is a
105115
type declaration. A nested declaration puts a type on both ends; that pair is
106116
skipped rather than guessed at.
117+
118+
Kept per relation rather than pooled: ``defines`` covers fields as well as
119+
C++'s in-class member declarations, so only a language that needs it may look
120+
there, and only when no ``method`` of that name exists.
107121
"""
108-
by_owner: dict[tuple[str, str], list[str]] = defaultdict(list)
122+
by_relation: dict[str, dict[tuple[str, str], list[str]]] = {
123+
relation: defaultdict(list) for relation in _MEMBER_RELATIONS
124+
}
109125
for u, v, data in merged.edges(data=True):
110-
if data.get("relation") != "method":
126+
relation = data.get("relation")
127+
if relation not in by_relation:
111128
continue
112129
if u in type_ids and v not in type_ids:
113130
owner, member = u, v
@@ -117,8 +134,18 @@ def _index_methods(merged: "nx.Graph", type_ids: set[str]) -> dict[tuple[str, st
117134
continue
118135
name = _key(merged.nodes[member].get("label"))
119136
if name:
120-
by_owner[(owner, name)].append(member)
121-
return by_owner
137+
by_relation[relation][(owner, name)].append(member)
138+
return by_relation
139+
140+
141+
def _member_relations(lang: str) -> tuple[str, ...]:
142+
"""Which owner→member relations may answer a call parked by ``lang``.
143+
144+
Only C++ models an in-class declaration (``void bar();`` in a header) as a
145+
field, so only a C++ entry may fall back to ``defines``; for every other
146+
language a ``defines`` target is a field, and a field cannot answer a call.
147+
"""
148+
return ("method", "defines") if lang == "cpp" else ("method",)
122149

123150

124151
def link_cross_repo_member_calls(merged: "nx.Graph") -> int:
@@ -136,7 +163,7 @@ def link_cross_repo_member_calls(merged: "nx.Graph") -> int:
136163
by_name, type_ids = _index_declarations(merged)
137164
if not by_name:
138165
return 0
139-
methods_by_owner = _index_methods(merged, type_ids)
166+
members_by_relation = _index_members(merged, type_ids)
140167

141168
added = 0
142169
for caller, caller_data in parked_nodes:
@@ -146,7 +173,8 @@ def link_cross_repo_member_calls(merged: "nx.Graph") -> int:
146173
# deliberately never re-decides a call inside one repo.
147174
continue
148175
for entry in _parked_entries(caller_data):
149-
suffixes = _LANG_SUFFIXES.get(str(entry.get("lang") or ""))
176+
lang = str(entry.get("lang") or "")
177+
suffixes = _LANG_SUFFIXES.get(lang)
150178
receiver_type = _key(entry.get("receiver_type"))
151179
callee = _key(entry.get("callee"))
152180
if not suffixes or not receiver_type or not callee:
@@ -160,7 +188,11 @@ def link_cross_repo_member_calls(merged: "nx.Graph") -> int:
160188
# The same guard the single-repo resolvers apply: two repos
161189
# declaring the same name is an ambiguity, not a hit.
162190
continue
163-
targets = methods_by_owner.get((candidates[0], callee), [])
191+
targets: list[str] = []
192+
for relation in _member_relations(lang):
193+
targets = members_by_relation[relation].get((candidates[0], callee), [])
194+
if targets:
195+
break
164196
if len(targets) != 1:
165197
continue
166198
target = targets[0]

‎graphify/extract.py‎

Lines changed: 91 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -2861,6 +2861,52 @@ def _assembly_of_node(nid: str) -> str:
28612861
rc["caller_nid"] = remap[cn]
28622862

28632863

2864+
UNRESOLVED_CALLS_KEY = "unresolved_calls"
2865+
_MAX_PARKED_CALLS_PER_NODE = 64
2866+
2867+
2868+
def _park_unresolved_member_call(
2869+
caller_node: dict | None,
2870+
callee: str,
2871+
receiver_type: str,
2872+
lang: str,
2873+
raw_call: dict,
2874+
) -> None:
2875+
"""Keep a member call whose receiver type is declared nowhere in this corpus.
2876+
2877+
A single-repo build can only bind ``obj.method()`` when the receiver's type is
2878+
declared in the same build, so a call into another repository is dropped with
2879+
the receiver type already in hand and nothing about it reaches ``graph.json``
2880+
— the one artifact ``merge-graphs`` and ``global add`` consume. Parking the
2881+
pair on the caller node lets a merged graph finish the edge (#3152).
2882+
2883+
The payload carries names only, never node ids: ids are rewritten by the
2884+
remaps and again by the repo prefixing, and a stale id inside metadata would
2885+
fail silently (#3150 was that bug). Names survive every rewrite.
2886+
"""
2887+
if not caller_node or not callee or not receiver_type:
2888+
return
2889+
metadata = caller_node.setdefault("metadata", {})
2890+
if not isinstance(metadata, dict):
2891+
return
2892+
parked = metadata.setdefault(UNRESOLVED_CALLS_KEY, [])
2893+
if not isinstance(parked, list) or len(parked) >= _MAX_PARKED_CALLS_PER_NODE:
2894+
return
2895+
callee, receiver_type = str(callee), str(receiver_type)
2896+
for previous in parked:
2897+
if (
2898+
isinstance(previous, dict)
2899+
and previous.get("callee") == callee
2900+
and previous.get("receiver_type") == receiver_type
2901+
):
2902+
return
2903+
entry = {"callee": callee, "receiver_type": receiver_type, "lang": lang}
2904+
location = raw_call.get("source_location")
2905+
if location:
2906+
entry["line"] = str(location)
2907+
parked.append(entry)
2908+
2909+
28642910
def _resolve_swift_member_calls(
28652911
per_file: list[dict],
28662912
all_nodes: list[dict],
@@ -2977,7 +3023,8 @@ def _key(label: str) -> str:
29773023
continue
29783024
receiver = rc.get("receiver")
29793025
callee = rc.get("callee")
2980-
if not receiver or not callee:
3026+
caller = rc.get("caller_nid")
3027+
if not receiver or not callee or not caller:
29813028
continue
29823029
# Determine the receiver's type. An upper-cased receiver is itself a type
29833030
# (Type.staticMethod(), Singleton.shared.x()); otherwise look it up in the
@@ -2997,12 +3044,20 @@ def _key(label: str) -> str:
29973044
if type_name in _LANGUAGE_BUILTIN_GLOBALS:
29983045
continue
29993046
type_defs = type_def_nids.get(_key(type_name), [])
3000-
if len(type_defs) != 1: # ambiguous or absent -> bail (god-node guard)
3047+
if not type_defs:
3048+
# Declared nowhere here — in a multi-repo setup that usually means "in
3049+
# a repo this build does not contain", so park it for the merge
3050+
# (#3152). This resolver shares `all_raw_calls` with every other
3051+
# language and Swift raw_calls carry no `lang` tag, so the parked
3052+
# entry's language comes from the declaring file's suffix.
3053+
if str(rc.get("source_file", "")).lower().endswith(".swift"):
3054+
_park_unresolved_member_call(
3055+
node_by_id.get(caller), callee, type_name, "swift", rc,
3056+
)
30013057
continue
3002-
type_nid = type_defs[0]
3003-
caller = rc.get("caller_nid")
3004-
if not caller:
3058+
if len(type_defs) != 1: # ambiguous -> bail (god-node guard)
30053059
continue
3060+
type_nid = type_defs[0]
30063061
method_nid = method_index.get((type_nid, _key(callee)))
30073062
target = method_nid or type_nid
30083063
relation = "calls" if method_nid else "references"
@@ -3410,7 +3465,17 @@ def _key(label: str) -> str:
34103465
elif receiver[:1].isupper():
34113466
# Foo::bar(): the type is named explicitly in source.
34123467
type_defs = type_def_nids.get(_key(receiver), [])
3413-
if len(type_defs) != 1: # ambiguous or absent -> bail (god-node guard)
3468+
if not type_defs:
3469+
# Declared nowhere here, which in a multi-repo setup usually means
3470+
# "in a repo this build does not contain" (#3152). `Foo::bar()` is
3471+
# also the shape of a namespace-qualified free function, so the
3472+
# merge side's guards do the deciding: it acts only when exactly
3473+
# one other repo declares a `Foo` owning exactly one `bar`.
3474+
_park_unresolved_member_call(
3475+
node_by_id.get(caller), callee, receiver, "cpp", rc,
3476+
)
3477+
continue
3478+
if len(type_defs) != 1: # ambiguous -> bail (god-node guard)
34143479
continue
34153480
type_nid = type_defs[0]
34163481
type_qualified = True
@@ -3420,7 +3485,12 @@ def _key(label: str) -> str:
34203485
if not type_name:
34213486
continue
34223487
type_defs = type_def_nids.get(_key(type_name), [])
3423-
if len(type_defs) != 1: # ambiguous or absent -> bail (god-node guard)
3488+
if not type_defs:
3489+
_park_unresolved_member_call(
3490+
node_by_id.get(caller), callee, type_name, "cpp", rc,
3491+
)
3492+
continue
3493+
if len(type_defs) != 1: # ambiguous -> bail (god-node guard)
34243494
continue
34253495
type_nid = type_defs[0]
34263496
type_qualified = False
@@ -3584,6 +3654,18 @@ def _resolve_type_name_nid(type_name: str | None, caller_node: dict | None,
35843654
type_defs = type_def_nids.get(_key(type_name), [])
35853655
return type_defs[0] if len(type_defs) == 1 else None
35863656

3657+
def _park_if_absent(type_name: str | None, caller_node: dict | None, rc: dict) -> None:
3658+
"""Park a call whose receiver type is declared nowhere in this corpus (#3152).
3659+
3660+
``_resolve_type_name_nid`` collapses "absent", "ambiguous" and "scoping was
3661+
decisive" into one ``None``, and only the first is a cross-repo candidate,
3662+
so re-check the bare-name index instead of trusting the ``None``.
3663+
"""
3664+
if type_name and not type_def_nids.get(_key(type_name)):
3665+
_park_unresolved_member_call(
3666+
caller_node, rc.get("callee"), type_name, "csharp", rc,
3667+
)
3668+
35873669
all_raw_calls: list[dict] = []
35883670
for result in per_file:
35893671
all_raw_calls.extend(result.get("raw_calls", []))
@@ -3622,6 +3704,7 @@ def _resolve_type_name_nid(type_name: str | None, caller_node: dict | None,
36223704
type_name = rc.get("receiver_type")
36233705
type_nid = _resolve_type_name_nid(type_name, caller_node, src_file)
36243706
if not type_nid:
3707+
_park_if_absent(type_name or receiver, caller_node, rc)
36253708
continue
36263709
type_qualified = True
36273710
else:
@@ -3630,6 +3713,7 @@ def _resolve_type_name_nid(type_name: str | None, caller_node: dict | None,
36303713
continue
36313714
type_nid = _resolve_type_name_nid(type_name, caller_node, src_file)
36323715
if not type_nid: # ambiguous or absent -> bail (god-node guard)
3716+
_park_if_absent(type_name, caller_node, rc)
36333717
continue
36343718
type_qualified = False
36353719
method_nid = _method_on_type_or_bases(type_nid, _key(callee))
@@ -3702,52 +3786,6 @@ def _bind_member_field_tables(
37023786
return bound
37033787

37043788

3705-
UNRESOLVED_CALLS_KEY = "unresolved_calls"
3706-
_MAX_PARKED_CALLS_PER_NODE = 64
3707-
3708-
3709-
def _park_unresolved_member_call(
3710-
caller_node: dict | None,
3711-
callee: str,
3712-
receiver_type: str,
3713-
lang: str,
3714-
raw_call: dict,
3715-
) -> None:
3716-
"""Keep a member call whose receiver type is declared nowhere in this corpus.
3717-
3718-
A single-repo build can only bind ``obj.method()`` when the receiver's type is
3719-
declared in the same build, so a call into another repository is dropped with
3720-
the receiver type already in hand and nothing about it reaches ``graph.json``
3721-
— the one artifact ``merge-graphs`` and ``global add`` consume. Parking the
3722-
pair on the caller node lets a merged graph finish the edge (#3152).
3723-
3724-
The payload carries names only, never node ids: ids are rewritten by the
3725-
remaps and again by the repo prefixing, and a stale id inside metadata would
3726-
fail silently (#3150 was that bug). Names survive every rewrite.
3727-
"""
3728-
if not caller_node or not callee or not receiver_type:
3729-
return
3730-
metadata = caller_node.setdefault("metadata", {})
3731-
if not isinstance(metadata, dict):
3732-
return
3733-
parked = metadata.setdefault(UNRESOLVED_CALLS_KEY, [])
3734-
if not isinstance(parked, list) or len(parked) >= _MAX_PARKED_CALLS_PER_NODE:
3735-
return
3736-
callee, receiver_type = str(callee), str(receiver_type)
3737-
for previous in parked:
3738-
if (
3739-
isinstance(previous, dict)
3740-
and previous.get("callee") == callee
3741-
and previous.get("receiver_type") == receiver_type
3742-
):
3743-
return
3744-
entry = {"callee": callee, "receiver_type": receiver_type, "lang": lang}
3745-
location = raw_call.get("source_location")
3746-
if location:
3747-
entry["line"] = str(location)
3748-
parked.append(entry)
3749-
3750-
37513789
def _resolve_java_member_calls(
37523790
per_file: list[dict],
37533791
all_nodes: list[dict],

0 commit comments

Comments
 (0)