fix(wasm): emit dyn=0 for f.call/bind alias calls (closes #1687) - #1693
fix(wasm): emit dyn=0 for f.call/bind alias calls (closes #1687)#1693carlos-alm merged 1 commit into
Conversation
Sorry, something went wrong.
When `f.call({})` or `f.apply({})` is called and `f` is a plain
identifier, the callee is statically known — no dynamic dispatch is
required. The previous code emitted `{ dynamic: true, dynamicKind:
'reflection' }` which caused the `dynZeroEdgeRows` upgrade path in
`emitDirectCallEdgesForCall` to wrongly promote a prior dyn=0 edge
(from a direct `f()` call) to dyn=1, diverging from native Rust which
simply drops the second call once the edge is in `seenEdges`.
Fix: omit the `dynamic` flag for `.call/.apply/.bind` on plain
identifiers in `extractMemberExprCallInfo`. Member-expression objects
(`obj.method.call({})`) still receive `dynamicKind: 'reflection'`
since the inner callee requires a second resolution hop.
Update tests in dynamic-call-ffi.test.ts and javascript.test.ts to
reflect the new static-call behavior for plain-identifier receivers.
Impact: 1 functions changed, 8 affected
Greptile SummaryThis PR fixes a WASM/native parity divergence where f.call({}) / f.apply() / f.bind() on plain identifiers was emitting dyn=1 (dynamic reflection) in WASM while the native Rust engine emitted dyn=0. The fix removes { dynamic: true, dynamicKind: 'reflection' } from the plain-identifier branch of extractMemberExprCallInfo, returning a bare static-call record instead; member-expression receivers (obj.method.call({})) retain the dynamic/reflection flags unchanged.
Confidence Score: 4/5Safe to merge — the core logic change is a one-liner with a clear causal chain, and the new static-call behavior is confirmed by updated tests and matches the native Rust engine output. The production code change is minimal and well-reasoned. The one gap is that the test titled for .bind on a plain identifier doesn't actually exercise that code path (the fixture uses a function_expression receiver, not an identifier), so the .bind variant of the fix is untested by the new suite. All other changed and added tests are accurate. tests/parsers/javascript.test.ts — the new test at line 645 doesn't cover the plain-identifier .bind path it advertises in its name. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["extractMemberExprCallInfo\ncallee is member_expression"] --> B{propText is\n.call/.apply/.bind?}
B -- No --> C[Other method call\nreturn static call with receiver]
B -- Yes --> D{obj.type?}
D -- identifier\ne.g. f.call --> E["Return static call\n{ name: obj.text, line }\ndyn=0"]
D -- member_expression\ne.g. obj.method.call --> F["Return dynamic/reflection call\n{ name: innerProp.text, dynamic: true,\ndynamicKind: 'reflection' }\ndyn=1"]
D -- other\ne.g. function expr --> G[Fall through to\ndefault handling]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A["extractMemberExprCallInfo\ncallee is member_expression"] --> B{propText is\n.call/.apply/.bind?}
B -- No --> C[Other method call\nreturn static call with receiver]
B -- Yes --> D{obj.type?}
D -- identifier\ne.g. f.call --> E["Return static call\n{ name: obj.text, line }\ndyn=0"]
D -- member_expression\ne.g. obj.method.call --> F["Return dynamic/reflection call\n{ name: innerProp.text, dynamic: true,\ndynamicKind: 'reflection' }\ndyn=1"]
D -- other\ne.g. function expr --> G[Fall through to\ndefault handling]
Reviews (1): Last reviewed commit: "fix(wasm): emit dyn=0 for f.call/bind al..." | Re-trigger Greptile |
Sorry, something went wrong.
| it('emits static call for .call/.apply/.bind on plain identifier (#1687)', () => { | ||
| // `f.call({})` where f is a plain identifier — target is statically known; | ||
| // must emit dyn=0 (no dynamic flag) to match native Rust engine parity. | ||
| const symbols = parseJS(`const f = function () {}.bind({}); f(); f.call({});`); | ||
| const fCallCalls = symbols.calls.filter((c) => c.name === 'f'); | ||
| expect(fCallCalls.length).toBeGreaterThanOrEqual(1); | ||
| for (const c of fCallCalls) { | ||
| expect(c.dynamic).toBeFalsy(); // all f() / f.call({}) calls must be dyn=0 | ||
| } | ||
| }); |
There was a problem hiding this comment.
Test title over-promises on .bind coverage
The test is named "emits static call for .call/.apply/.bind on plain identifier" but the only .bind call in the fixture is function () {}.bind({}), where the receiver is a function_expression — not a plain identifier. That expression falls through the new identifier-check branch and is emitted as { name: 'bind', … }, so it is never included in fCallCalls. The actual plain-identifier .bind code path (e.g. f.bind(ctx) → { name: 'f' }) isn't exercised by any new test.
Sorry, something went wrong.
Codegraph Impact Analysis1 functions changed → 8 callers affected across 1 files
|
Sorry, something went wrong.
Summary
Root Cause
When f.call({}) precedes or follows a direct f() call for the same target, the WASM edge builder's dynZeroEdgeRows upgrade path in emitDirectCallEdgesForCall was converting a dyn=0 edge (emitted by f()) to dyn=1 because the f.call({}) call had dynamicKind: 'reflection' set, triggering the upgrade condition isDynamic === 1 && hasDynamicKind.
The native Rust engine silently drops the second call (the f.call({}) one) once the edge is already in seenEdges, preserving dyn=0. The fix aligns WASM behavior: when the .call/.apply/.bind receiver is a plain identifier, the target is statically known so we emit a static call — isDynamic = 0 — which never triggers the upgrade.
Member-expression objects (obj.method.call({})) still receive dynamicKind: 'reflection' since the inner callee requires a second resolution hop.
Closes #1687