fix(transform): preserve per-iteration let/const bindings across the async state machine (#6345)

[proggeramlug]

Fixes #6345.

Symptom

A let/const declared in a loop and captured by a closure lost its per-iteration binding inside an async function — every closure saw the last iteration's value. Silent wrong answer, exit 0. The same code in a non-async function was correct.

async function main() {
  const fns: (() => void)[] = [];
  for (let i = 0; i < 9; i++) {
    const j = i;
    fns.push(() => console.log("j =", j));
  }
  fns.forEach((f) => f());
  await 0;                       // <- removing this made it correct
}
main();

	output
node 26	j = 0 … j = 8
perry main	j = 8 ×9

It was not limited to the filed shape: every loop kind was affected — while, do-while, for-of, for-in, for-await-of, nested loops, labeled loops, and direct capture of the loop variable (() => i printed 9 nine times). Sync generators too, since they share the lowering.

Root cause

generator::lower hoists every Stmt::Let in the body into one activation-wide frame:

• collect_hoisted_vars collects all of them (recursing into loop bodies and for-inits),
• Stmt::PreallocateBoxes allocates one box per id in the entry block — once per call,
• rewrite_hoisted_lets_in_stmts demotes each Stmt::Let to a bare Expr(LocalSet(id, init)).

That is genuinely required for a binding whose live range crosses a suspend: the step closure returns at every await and is re-entered later, so its plain locals do not survive — only the boxes it captured do.

But it is wrong for a loop binding. The declaration is gone, so the box is never re-created; all N iterations share one cell, and every closure captures that one cell.

What the non-async path does that the async path lost: it simply leaves the Stmt::Let inside the loop body. Codegen re-executes it every iteration, re-allocating its box when the binding needs one (boxed_vars.rs), and for a binding nothing ever writes it declines to box at all so the closure snapshot-captures the value at creation. Either route yields a distinct per-iteration binding. Hoisting destroys both signals at once — and PreallocateBoxes force-marks the id boxed regardless of what boxed_vars' own mutation analysis concluded.

Worth noting the HIR was right all along: post-transform the user closure still carries captures: [j], mutable_captures: [] (i.e. by value). Codegen just materializes a capture from boxed_vars alone (expr/closure.rs: let _ = mutable_captures;), so the forced box wins.

Fix (perry-transform, compiler-side only)

New generator/per_iteration.rs. Hoist only what must be hoisted.

1. collect_per_iteration_ids — bindings whose live range provably stays inside one state keep their in-loop Stmt::Let; lower subtracts them from the hoisted set so they never reach PreallocateBoxes. A binding qualifies only when it is declared in a loop, is genuinely block-scoped, and is not read after a suspend that follows its declaration. A for-init counter is loop-carried (condition/update re-read it after the body), so it qualifies only when the loop contains no suspend at all.

2. snapshot_suspended_loop_captures — for a binding that really is read after an await, the cross-state box must stay. The step closure's capture slots are fixed for its lifetime, so the box pointer cannot be swapped per iteration. Instead we give the closure the value rather than the cell: at the moment the closure is built the shared box holds the current iteration's value, so it is copied into a fresh local declared immediately before the closure — a local that lives and dies inside one state. Runs before linearization so the Let lands in the same state as its closure.

Why this is safe

• var stays var. HIR emits a var as a function-top pre-declaration Let plus an in-place Let sharing the same id, so decl_sites > 1 identifies it; a var read outside the loop also trips total_refs > refs_inside_loop (the bug(codegen): var declared in a constant-bound (unrolled) loop loses its value when read after the loop #2308 test, whose exhaustive ref-counter is reused here rather than re-implemented). Both guards keep it hoisted: one function-scoped binding, closures see the last value — matching node, and matching what perry already did. (A "no references outside the loop" test alone would have silently broken var; the fixture pins this.)
• Snapshots only where HIR says read-only. We snapshot exactly captures \ mutable_captures. HIR puts an id in mutable_captures when it is assigned anywhere — by a closure or by the enclosing scope (verified for both) — so a binding a closure writes (let acc = 0; const add = () => acc += 5) keeps its shared box untouched, and a binding the outer scope mutates after the capture (fns.push(() => z); z += 100) keeps sharing too. A for-init counter is deliberately kept out of mutable_captures by HIR despite i++, which is exactly the per-iteration semantics we want.
• Rollback verifier. The snapshot rewrite redirects an id throughout a closure subtree, including capture lists, nested closures, and array/set fast-path expressions (ArrayPush { array_id } &c.). Rather than trust a hand-rolled variant list, the rewrite is applied and then proved — with the same exhaustive counter the var analysis relies on — to have left no reference to the original id. If one survives, the whole closure is rolled back and keeps the (merely stale) shared-box behaviour instead of inventing a new bug.
• The conservative direction is always to hoist; anything unproven keeps today's behaviour. Non-async functions never reach this code.

Verification

Every shape below diffed byte-for-byte against node --experimental-strip-types. Trip counts are 9 on purpose — the static-loop unroller (MAX_TRIP_COUNT = 8) mints fresh ids per copy and masks the bug entirely at ≤ 8, which is why the issue's repro used 9.

shape	before	after
const j = i capture, await after loop (filed repro)	✗	✓
direct loop-var capture () => i	✗	✓
await inside body, capture before / after it	✗	✓
loop-var capture with await inside body	✗	✓
while / do-while	✗	✓
for-of / for-in / for-await-of	✗	✓
nested loops; closure capturing two levels	✗	✓
labeled loop + continue	✗	✓
try/catch/finally, switch in a suspending loop	✗	✓
captured array binding mutated inside the closure	✗	✓
destructured binding; nested closure	✗	✓
sync function* generator	✗	✓
var — function-scoped, ONE binding, last value	✓	✓ (unchanged)
closure-written binding — shared box	✓	✓ (unchanged)
outer-written-after-capture — shared binding	✗	✓
non-async control	✓	✓ (unchanged)

• Regression fixture: test-files/test_gap_6345_async_loop_per_iteration_binding.ts (213 lines of output, byte-identical to node; 159 lines differ on pre-fix main, so it genuinely gates).
• Full gap suite A/B against a real pristine origin/main toolchain (compiler and both static archives, both arms release): no regressions.
• cargo test -p perry-transform: 48 lib + 16 unroll tests pass.
• cargo fmt --all -- --check and scripts/check_file_size.sh clean.

Known residual (pre-existing, unchanged)

A binding that a closure writes and that survives an await still collapses:

for (let i = 0; i < 9; i++) {
  let acc = i;
  const bump = () => { acc += 100; };
  bump();
  await tick();
  fns.push(() => acc);   // node: 100..108; perry: 108 x9
}

This one needs a shared per-iteration cell that outlives a suspend — the step closure's fixed capture slots can only reach one box per activation, so it requires a second level of indirection rather than a snapshot. It behaves identically before and after this PR; filed separately as #6354 so the two fixes stay untangled.

Independent of #6328 / #6344 (a runtime fix for numeric-key computed calls); this one is entirely in perry-transform and touches no runtime code.

Summary by CodeRabbit

• Bug Fixes

  • Fixed async and generator loops so let and const variables retain distinct per-iteration values across suspension and resumption.
  • Improved closure captures in suspended loops, including nested loops and varied loop constructs.
  • Preserved expected function-scoped behavior for var declarations.

• Tests

  • Added comprehensive coverage for async loops, generators, closures, control flow, and suspension scenarios.

[coderabbitai]

📝 Walkthrough

Walkthrough

The async/generator transform now preserves per-iteration let/const bindings across suspension. It analyzes loop live ranges, snapshots eligible closure captures, excludes protected locals from hoisting, and adds broad async loop coverage for issue 6345.

Changes

Async per-iteration binding preservation

Layer / File(s)	Summary
Analysis dependencies and module wiring crates/perry-transform/src/generator/..., crates/perry-transform/src/unroll/...	The generator exposes per-iteration analysis and reuses crate-visible local-reference counting helpers.
Per-iteration binding analysis crates/perry-transform/src/generator/per_iteration.rs	The transform identifies loop-scoped bindings that can remain inside loop bodies based on declarations, references, suspension points, and live ranges.
Suspended closure capture snapshotting crates/perry-transform/src/generator/per_iteration.rs	Eligible read-only captures are rewritten to per-closure snapshot locals, with verification that original captures are removed.
State-machine integration and coverage crates/perry-transform/src/generator/lower.rs, test-files/test_gap_6345_async_loop_per_iteration_binding.ts	Protected locals are excluded from hoisted Let rewriting, and tests cover async loop forms, mutations, control flow, nested closures, and synchronous comparisons.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant AsyncGeneratorLowering
  participant PerIterationAnalysis
  participant SnapshotSuspendedCaptures
  participant StateMachineLinearization
  participant AsyncLoopTests
  AsyncGeneratorLowering->>PerIterationAnalysis: collect_per_iteration_ids
  AsyncGeneratorLowering->>SnapshotSuspendedCaptures: snapshot_suspended_loop_captures
  SnapshotSuspendedCaptures-->>AsyncGeneratorLowering: protected local IDs
  AsyncGeneratorLowering->>StateMachineLinearization: exclude protected IDs from hoisted rewrites
  StateMachineLinearization-->>AsyncLoopTests: transformed async loop behavior

Loading

Possibly related issues

• async: a per-iteration binding that a closure WRITES and that survives an await still collapses #6354 — Covers the remaining mutable-capture case that this implementation intentionally excludes.

Suggested labels: run-extended-tests

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 78.26% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The async lowering changes preserve per-iteration loop bindings and direct loop-variable captures as required by #6345.
Out of Scope Changes check	✅ Passed	The changes are focused on generator lowering, supporting analysis, and a regression test for #6345; no unrelated work stands out.
Title check	✅ Passed	The title clearly and concisely summarizes the main fix: preserving per-iteration let/const bindings across async lowering.
Description check	✅ Passed	The description covers the bug, root cause, fix, issue link, and verification, though it doesn't follow the template sections exactly.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/6345-async-loop-binding-collapse

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}