Repository navigation
fix(cpp): bind x.m() by the receiver's type (builds on #3240) - #4946
nothariharan wants to merge 4 commits into
Conversation
…Graphify-Labs#3215) _resolve_cpp_member_calls types a receiver from cpp_type_table, which was populated ONLY from local variable declarations - parameters never entered it, so a call through `const Thing& T` / `Thing* P` / `Thing V` (the dominant C++ idiom for passing state) was silently skipped while the identical call through a local resolved. TS/JS have an explicit augmentation pass for exactly this; C++ had none. Two halves: * the per-file table now folds in the enclosing function's parameters (_cpp_parameter_types) and class/struct member declarations (_cpp_field_types), in C++'s own shadowing order - a local beats a parameter beats a field, enforced by the table's first-write-wins; the same precision rules as locals apply (class-like type nodes only, qualified names keyed by their simple tail, multi-declarator and builtin-typed entries contribute nothing); * the resolver's capitalized-receiver arm treated `T.IsOk()` as an explicit type name and bailed when no type called T exists - but a capitalized receiver is very often a VARIABLE (parameter T, field Inner). When the explicit-type lookup finds no unique type, it now falls back to the file's var->type table exactly like the lowercase arm, mirroring what the C# resolver already does for Pascal-cased locals; a table miss still bails rather than guessing. _cpp_declarator_name learns field_identifier (fields use it where locals use identifier). Chained receivers (B.Inner.IsOk()) stay deferred.
… a decisive edge Review follow-up: the old test declared a differently-named local (T2, not T), so it never exercised shadowing, and its assertion passed on an empty result. Now a parameter Thing T is shadowed by an inner-block local Other T, and the call binds to Other::Ready — which resolves only if the local wins the first-write-wins fold (locals before parameters); a params-first regression leaves Thing (no Ready) and yields no edge. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JJbLfztSxm2tH5cJwBbe9q
…med method
A C++ member call with a variable receiver fell through to the file-wide
bare-name map, so `void render(Base* b) { b->draw(); }` in a file that also
defines `Derived::draw` produced an EXTRACTED edge to Derived::draw, the one
the static type rules out, and no edge to Base::draw.
Defer `x.m()` / `x->m()` to _resolve_cpp_member_calls like JS/TS already
do (Graphify-Labs#4598). With the parameter and field typing from the previous commit,
`b` is typed as Base and the call reaches Base::draw (INFERRED). An untyped
receiver now gets no edge instead of a name guess. `this->m()` and
`Foo::m()` keep their existing in-file binding.
…d in-file
Review follow-ups to the previous commit.
The receiver table is one dict for the whole file, filled first-write-wins,
so `void g() { Response r; }` retyped `r` in `void f(Request& r)` and f's
`r.send()` went looking on Response. Build each function's table on its
own (a local still shadows a parameter there), then drop any name the file
types two different ways, including a field declared with different types
in two classes. An ambiguous receiver gets no type rather than a guess.
Defer a member call only when its receiver is typed. `auto w = make();
w.paint()`, smart pointers and range-for variables have no table entry, and
deferring them dropped the in-file edge they had on v8.
github-actions
Bot
added
the
contributor
|
Thanks for the pull request, @nothariharan. A maintainer will review it soon. Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions. A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic. |
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 2 advisory finding(s) below merit a look before merge.
Not checked: tests were not run; formal verification proved 0 of 3 changed function(s) (3 not verified).
Formal verification. PR-changed functions: 0/3 verified (0 proven, 0 may-equivalent, 0 distinguished) · 3 not verified (2 vacuous, 1 unsupported).
Not verified on this run: \_resolve\_cpp\_member\_calls (vacuous: never exercised), \_cpp\_declarator\_name (vacuous: never exercised), \_extract\_generic (unsupported).
Graphify review — findings
Adds C++ receiver typing from function parameters and class fields, so x.m() / x->m() through a const Thing& parameter or a member field resolves by the receiver's type instead of being skipped. Capitalized qualified receivers such as T::bar() now fall back to the file's variable-to-type table when no unique type of that name exists, and the fallback binding is only as reliable as that file-wide, name-keyed table (a same-named local in another function can retype a parameter). Defers C++ member calls on typed non-this receivers to _resolve_cpp_member_calls, so b->draw() through Base* b stops binding to the file's own Derived::draw, though that defer depends on type_table being populated before the call walk reaches it.
Worth a look
- File-wide name-keyed C++ type table lets a local in one function retype a same-named parameter in another —
graphify/extract.py· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
- C++ member-call defer may be dead code because type_table is filled after the call walk —
graphify/extractors/engine.py:10108· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 3971 functions depend on the 757 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
extract()— 1101 callers, 64 callees - worse:
_extract_generic()— 20 callers, 52 callees
Verification — 3971 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 3709 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
238 of 491 test file(s) selected (48%) via static blast radius.
tests/test_4874_jvm_resolution.py— impacttests/test_affected_overrides.py— impacttests/test_astro_extraction.py— impacttests/test_astro_import_ids.py— impacttests/test_blade_extractor.py— impacttests/test_build.py— impacttests/test_builtin_global_type_refs.py— impacttests/test_c_include_resolution.py— impacttests/test_cache.py— impacttests/test_case_only_decl_ids_4904.py— impacttests/test_case_sensitive_resolution.py— impacttests/test_cjs_module_extension.py— impacttests/test_cobol_extractor.py— impacttests/test_confidence_score_output.py— impacttests/test_cpp_member_call_by_receiver_type.py— impact, changed-testtests/test_cpp_method_declarations.py— impacttests/test_cpp_nested_and_cli.py— impacttests/test_cpp_objc_cross_file_calls.py— impacttests/test_cpp_receiver_parameters.py— impact, changed-testtests/test_cpp_template_type_params.py— impacttests/test_cpp_type_alias.py— impacttests/test_cross_extension_reexport_self_cycle.py— impacttests/test_cross_language_call_resolution.py— impacttests/test_cross_repo_external_call_guards.py— impacttests/test_cross_repo_member_calls.py— impacttests/test_csharp_call_site_generic_args.py— impacttests/test_csharp_delegate_declarations.py— impacttests/test_csharp_enum_members.py— impacttests/test_csharp_field_generic_args.py— impacttests/test_csharp_foreach_loop_variable.py— impacttests/test_csharp_generic_callsites.py— impacttests/test_csharp_interface_dispatch.py— impacttests/test_csharp_member_calls.py— impacttests/test_csharp_member_chain_calls.py— impacttests/test_csharp_member_nodes.py— impacttests/test_csharp_method_owner_labels.py— impacttests/test_csharp_object_creation.py— impacttests/test_csharp_overloads.py— impacttests/test_csharp_partial_classes.py— impacttests/test_csharp_tuple_type_refs.py— impacttests/test_csharp_type_resolution.py— impacttests/test_csharp_underscore_collisions.py— impacttests/test_csharp_var_call_return_type.py— impacttests/test_csharp_verbatim_interpolation.py— impacttests/test_definition_file_portability.py— impacttests/test_detect.py— impacttests/test_dotnet.py— impacttests/test_duplicate_annotation_edges.py— impacttests/test_edge_also_seen.py— impacttests/test_ejs_extractor.py— impact- … and 188 more
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
Docs that may be stale (advisory)
ARCHITECTURE.md§ Extraction output schema (lines 56-81): references changed symbolsnameCHANGELOG.md§ 0.9.97 (2026-10-27) (lines 68-93): references changed symbolsnameCHANGELOG.md§ 0.9.95 (2026-10-23) (lines 125-150): references changed symbolsnameCHANGELOG.md§ 0.9.93 (2026-10-20) (lines 173-199): references changed symbolsnameCHANGELOG.md§ 0.9.88 (2026-10-12) (lines 272-296): references changed symbolsnameCHANGELOG.md§ 0.9.87 (2026-10-12) (lines 297-319): references changed symbolsnameCHANGELOG.md§ 0.9.85 (2026-10-10) (lines 342-364): references changed symbolsnameCHANGELOG.md§ 0.9.83 (2026-10-09) (lines 379-393): references changed symbolsnameCHANGELOG.md§ 0.9.81 (2026-10-08) (lines 400-418): references changed symbolsnameCHANGELOG.md§ 0.9.76 (2026-10-04) (lines 479-497): references changed symbolsname
…and 10 more.
Formal verification
Could not verify: Could not verify \_resolve\_cpp\_member\_calls.
The verifier did not have enough to check \_resolve\_cpp\_member\_calls, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: no capturable inputs from the test suite; property tier: non-vacuity: domain too small (only 1 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous
Could not verify: Could not verify \_cpp\_declarator\_name.
The verifier did not have enough to check \_cpp\_declarator\_name, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: no capturable inputs from the test suite; property tier: not verifiable: all 228 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify \_extract\_generic.
The verifier did not have enough to check \_extract\_generic, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: no capturable inputs from the test suite; property tier: parameter `config` is annotated `LanguageConfig` — outside the synthesizable primitive/collection set
· 2 more finding(s) on lines outside this diff (see the check run).
#4951 (type-alias exact-spelling binding + salted alias contains edge), #4946 (receiver typing from parameters and fields), #4920 (split-header owner binding via _cpp_qualified), and #4921 (local-callable suppression) all change per-file C++ extractor output, so warm caches must re-run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Landed in 0.9.101 (published to PyPI), cherry-picked with your authorship preserved. Thanks @nothariharan. |
Builds on #3240 by @abhay-codes07, cherry-picked with authorship kept (it applies cleanly to v8), and fixes the case #3240 doesn't fix on its own.
Problem
In C++,
void render(Base* b) { b->draw(); }in a file that definesDerived::drawproduced an EXTRACTED edge toDerived::draw, which is the method the static type rules out, and no edge toBase::draw. A member call with a variable receiver fell through to the file-wide bare-name map. This came up in a public C++ firmware benchmark that reported calls into overridden functions weren't linked.Change
x.m()/x->m()to_resolve_cpp_member_calls, as JS/TS have done since [Bug]: JS/TS: member call binds to a same-named function in the file, so delegating wrappers become false self-loops (#4082 still reproduces on 0.9.93) #4598, but only when the receiver is typed. Untyped receivers (auto, smart pointers, range-for) keep their v8 in-file binding, so they don't lose edges.this->m()andFoo::m()are unchanged.Response rin one function retypedRequest& rin another. A name typed differently in two places, or a field typed differently in two classes, is dropped as ambiguous instead of guessed.b->draw()now reachesBase::draw(INFERRED).Evidence
tests/test_cpp_member_call_by_receiver_type.py: 5 tests. The base-pointer, cross-function retype andautocases each fail without the commit that fixes them.Not included: an edge from
Derived::drawto theBase::drawit overrides. The schema has no such relation today, so adding one is a design decision.