| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
… CHA path emitChaCallEdgesForCall (the WASM/JS full-build inline CHA resolution path) tagged every dispatch edge 'cha' regardless of receiver kind, unlike buildChaPostPass and the native orchestrator's this/super handling, both of which already distinguish super-dispatch (not virtual dispatch) from typed-receiver interface dispatch. Added the same receiver === 'super' check, mirroring buildChaPostPass exactly. Verified against tests/fixtures/cha-dispatch/ with --engine wasm: Lion.speak -> Animal.speak and Car.constructor -> Vehicle.constructor now correctly get technique='super-dispatch'; typed-receiver dispatch (dispatch -> ConcreteWorker.doWork/MockWorker.doWork) keeps 'cha'. Extended tests/integration/phase-8.5-cha-dispatch.test.ts with technique assertions for both engines, pinning what's now consistent (super-dispatch) and what remains a known, separately-tracked gap (native's typed-receiver CHA expansion still tags 'cha-expanded' rather than 'cha' - see PR description for why that specific piece isn't fixed here). No README/CLAUDE.md/ROADMAP.md changes needed, docs check acknowledged. Refs #1996 Impact: 1 functions changed, 3 affected
Greptile SummaryThe PR aligns WASM/JS inline CHA edge metadata with the existing post-pass and native behavior by labeling super calls as super-dispatch.
Confidence Score: 5/5The PR appears safe to merge, with the metadata change matching existing super-dispatch handling across sibling build paths. The change only adjusts the technique stored for already-resolved super edges, and the added integration assertions cover super constructors, super methods, typed receivers, and transitive CHA behavior across the applicable engines. Important Files Changed
Reviews (1): Last reviewed commit: "fix(build-edges): tag super-dispatch dis..." | Re-trigger Greptile |
Sorry, something went wrong.
Codegraph Impact Analysis1 functions changed → 3 callers affected across 1 files
|
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Summary
This issue describes three independent technique-label inconsistencies for CHA/points-to edges across the WASM inline, native post-pass, and native-orchestrator paths. This PR fixes the first (the one with no correctness risk); the other two need their own follow-up.
Fixed here: (a) WASM/JS inline CHA path missing the super-dispatch distinction
emitChaCallEdgesForCall (stages/build-edges.ts, the WASM/JS full-build inline CHA resolution path) tagged every dispatch edge 'cha' regardless of receiver kind. buildChaPostPass (same file, a different integration point) and the native orchestrator's this/super handling both already distinguish super-dispatch (not virtual dispatch) from typed-receiver interface dispatch. Added the identical call.receiver === 'super' ? 'super-dispatch' : 'cha' check, mirroring buildChaPostPass exactly.
Verified against tests/fixtures/cha-dispatch/ with --engine wasm: Lion.speak -> Animal.speak and Car.constructor -> Vehicle.constructor now correctly get technique='super-dispatch'; typed-receiver dispatch (dispatch -> ConcreteWorker.doWork/MockWorker.doWork) keeps 'cha'.
Not fixed here — needs its own follow-up
(b) Renaming native's 'cha-expanded' to 'cha'. I initially attempted this as a straight string rename (per the issue's own suggestion) but found it breaks a real invariant: expandChaEdges's exclusion filter (e.technique != 'cha-expanded') exists specifically to avoid re-expanding this pass's own prior output on a later incremental rebuild, while still treating 'cha'-tagged this/super-dispatch edges (from runPostNativeThisDispatch) as expansion-eligible input in the same pass — collapsing both labels to the identical string 'cha' would make the exclusion filter also exclude the this/super-dispatch edges it's supposed to expand (e.g. the PostMixin.m → B.m → sibling-override scenario the code comments describe). Fixing this needs a distinct internal marker (not necessarily the same as the externally-reported label) — a small but real design task, not a mechanical rename.
(c) Native full-build's own points-to/alias resolution has no technique label at all. Traced this to the FFI boundary: Rust's ComputedEdge struct (crates/codegraph-core/.../build_edges.rs) has no technique field whatsoever — native never writes technique at insert time for any edge, resolved via alias or otherwise; it all comes from the later JS-side blanket 'ts-native' backfill, which has no way to know an edge was points-to-resolved specifically. Giving these edges a distinct 'points-to' label requires either a new Rust→JS data channel (a list of points-to-specific edge pairs alongside the main edge list) or adding a technique field to ComputedEdge and threading it through the napi binding — a cross-language interface change, out of scope for a quick fix.
I've left #1996 open (not closing it) so (b) and (c) stay tracked; this PR title/commit uses Refs #1996 rather than Closes.
Verification