ReviewOS

zig-utils/zig-js

VM: honor formal parameters shadowing arguments in leaf calls

#1083
Closed chris opened this 9 days ago ·
9 days ago

Parent: #498; related: #465, #491.

Executed on 27d58465 using its saved production binary:

function leaf(arguments) { return arguments[0]; }
leaf(3)

Node and forced tree-walker execution return undefined; required bytecode returns 3.

function leaf(arguments, other) { return arguments[0] + arguments[1]; }
leaf(2, 3)

Node and tree-walker return NaN; required bytecode returns 5. Ordinary and duplicate-parameter controls without an arguments formal return 5 in all three executions.

compileQuickArgumentsLeaf treats every accepted arguments[index] AST read as an incoming argument. Its current simple-parameter guard does not establish that the identifier resolves to the function's arguments object. FunctionDeclarationInstantiation instead uses the formal parameter binding when the formal list binds arguments.

Required change:

  • Decline arguments-object scalar replacement when a formal parameter binds arguments; use ordinary parameter and identifier semantics.
  • Retain bounded specialization for genuine arguments-object reads and ordinary duplicate parameters.
  • Preserve live property access/coercion, exact steps, checkpoints, hooks, GC roots, and no-GIL behavior.

Definition of done:

  • Executed shadowed-formal regressions match Node in forced tree and required-bytecode modes.
  • Positive controls still reach the arguments-leaf fast path; rejected shadowed forms do not.
  • Focused Debug, ReleaseSafe, unsuppressed TSan, real-GC, and no-GIL coverage passes.
  • Before/after affected Test262 path sets are compared with a nonzero denominator; the quickening inventory describes the binding guard.
9 days ago

Fixed in c2c7061b. The bounded arguments-object leaf guard now declines any simple formal list that binds arguments. Ordinary parameter/identifier/property execution supplies the result; genuine arguments-object leaves retain their bounded shortcut.

Validation:

  • Both executed numeric shadowing reproductions now match Node (undefined and NaN). The expanded oracle also matches for formal position, duplicate formals, inherited Number properties, String indexing, live getters, coercion order, thrown identity, and nullish errors.
  • Both new regressions pass Debug, ReleaseSafe, and unsuppressed TSan with real GC. The worker case verifies four actual no-GIL workers. Counter assertions prove positive ordinary/duplicate controls retain leaf hits while shadowed forms do not.
  • Existing exact-step/guard, source-hook, termination/budget, moving-closure, and prepared-activation moving-collection tests pass.
  • Exact before/after diagnostics and forced tree/VM reports match across arguments-object (263), call (92), function-expression (264), function-declaration (451), and Function builtins (509): 1,579 cases, zero default-tier failures on both sides, and the same three existing call divergences.
  • Quickening inventory self-test/validation, 378-file link audit, and checked 51-page docs build pass.

Full local combined-unit linking remains blocked by #933's existing 3072 MB cap. The unrelated stale retired-allocation-kernel assertion reproduced in prior hosted unit/TSan shard 3/4 is tracked in #1084 and is being repaired separately. Full hosted integration is pending.

Sign in to comment on this issue.