fix(transform): per-iteration binding a closure writes survives an await (#6354)

[proggeramlug]

Summary

Fixes #6354. A per-iteration let that a closure writes and that is still read after an await in the same loop body collapsed onto a single binding — every closure observed the last iteration's value:

for (let i = 0; i < 9; i++) {
  let acc = i;
  const bump = () => { acc += 100; };   // closure WRITES acc
  bump();
  await tick();                          // acc read after the suspend
  fns.push(() => console.log("acc =", acc));
}
// node: 100..108     perry (before): 108 ×9

This is the residual left by #6345 / #6353. That fix's snapshot only copies read-only captures (captures \ mutable_captures) — a value snapshot would silently drop a later write — so a written binding stayed in mutable_captures, kept its single activation-wide box, and collapsed.

Approach

Reduce the write case to the already-solved read-only case. Before the #6345 passes run, back each such binding with a one-element heap cell:

let acc = [i];
const bump = () => { acc[0] += 100; };
bump();
await tick();
fns.push(() => acc[0]);

Now the binding variable holds a per-iteration array reference that is never reassigned, so it lands in captures \ mutable_captures and #6345 snapshots the reference per iteration; writes go to the shared element and stay visible to every closure of the same iteration. This is entirely a perry-transform change — no codegen / HIR-contract change.

Candidates: a block-scoped loop-body let that some in-loop closure lists in mutable_captures and that is live across a suspend. Bindings referenced via a bare-LocalId array/set mutation intrinsic (arr.push, set.add, arr.pop(), …) or a with fallback — forms the cell rewrite can't retarget — are excluded and left as-is (the pre-existing collapse persists for that rare shape, but no new corruption).

Validation

• New gap fixture test_gap_6354_async_written_binding_across_await.ts — byte-identical to Node 26. Covers plain async functions, async generators, sync generators, closure writes, ++/--, enclosing-scope writes before and after the suspend, write-sharing across a suspend, multiple bindings, nested loops, while/do-while/for-of, and a var-stays-collapsed guard.
• The async: per-iteration let/const binding collapses for closures created in a loop body (every closure sees the last value) #6345 fixture is unchanged (still byte-identical to Node). Its closureWritesBinding case used let acc = 0 (same value every iteration), so a collapse coincidentally equalled the correct answer — it never actually exercised this residual, which is why async: a per-iteration binding that a closure WRITES and that survives an await still collapses #6354 was filed separately.
• cargo test -p perry-transform: all pass.
• The pass is a no-op (early return) for any function without the narrow pattern, so it is byte-identical to baseline elsewhere.

Per the contributor guidance, this PR does not bump the version or touch the changelog.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Fixed incorrect values when closures modify loop-scoped let or const bindings that are later read after await, yield, or other generator suspensions.
  • Improved consistency across for, while, do/while, for-of, nested-loop, synchronous generator, and asynchronous generator scenarios.

• Tests

  • Added coverage for suspended loops and closures that read and write per-iteration bindings.

[coderabbitai]

Warning

Review limit reached

@proggeramlug, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 40 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 46294149-04ad-4fa7-ad95-a5a41b620144

📥 Commits

Reviewing files that changed from the base of the PR and between 8f5f183 and d7e7bf1.

📒 Files selected for processing (4)

• crates/perry-transform/src/generator/lower.rs
• crates/perry-transform/src/generator/mod.rs
• crates/perry-transform/src/generator/per_iteration.rs
• test-files/test_gap_6354_async_written_binding_across_await.ts

📝 Walkthrough

Walkthrough

The generator transform now detects written loop captures that cross suspension points, rewrites them through per-iteration heap cells, and applies existing snapshot handling afterward. Tests cover async loops, generators, nested loops, and var behavior.

Changes

Written suspended loop captures

Layer / File(s)	Summary
Capture candidate analysis crates/perry-transform/src/generator/per_iteration.rs	Identifies block-scoped loop bindings written by closures, read after suspension, and safe for cell rewriting.
Cell rewrite and generator integration crates/perry-transform/src/generator/per_iteration.rs, crates/perry-transform/src/generator/lower.rs, crates/perry-transform/src/generator/mod.rs	Rewrites selected bindings and accesses through one-element cells, and runs the rewrite before suspended-capture snapshots.
Suspension behavior coverage test-files/test_gap_6354_async_written_binding_across_await.ts	Tests written bindings across async loops, nested loops, generators, async generators, and var cases.

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

Sequence Diagram(s)

sequenceDiagram
  participant GeneratorLowering
  participant CaptureAnalysis
  participant CellRewrite
  participant CaptureSnapshot
  GeneratorLowering->>CaptureAnalysis: collect written suspended loop captures
  CaptureAnalysis-->>GeneratorLowering: selected LocalIds
  GeneratorLowering->>CellRewrite: rewrite selected bindings to cells
  GeneratorLowering->>CaptureSnapshot: process remaining suspended captures

Loading

Possibly related PRs

• PerryTS/perry#6353: Updates the connected per-iteration loop-binding hoisting and snapshot logic extended by this change.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately reflects the fix for per-iteration bindings written by closures surviving awaits.
Description check	✅ Passed	It covers the summary, approach, related issue, and validation, though it doesn't mirror every template section verbatim.
Linked Issues check	✅ Passed	The changes match #6354 by rewriting eligible written loop bindings to per-iteration cells and adding coverage for async and generator cases.
Out of Scope Changes check	✅ Passed	The PR stays focused on the #6354 transform and test additions; no unrelated code changes are evident.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

test-files/test_gap_6354_async_written_binding_across_await.ts (1)

1-4: 📐 Maintainability & Code Quality | 🔵 Trivial

Run tests against the pinned Node version.

As per coding guidelines, ensure you run the gap suite against the exact Node version pinned in .node-version. Do not substitute another Node release because the oracle version affects correctness results.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test-files/test_gap_6354_async_written_binding_across_await.ts` around lines
1 - 4, Run the gap test suite for
test_gap_6354_async_written_binding_across_await.ts using exactly the Node
version specified in .node-version. Do not substitute another Node release, and
verify the test passes under the pinned version.

Source: Coding guidelines

crates/perry-transform/src/generator/per_iteration.rs (1)

81-82: 📐 Maintainability & Code Quality | 🔵 Trivial

Use correct profiles for local iteration.

As per coding guidelines for **/*.rs files, use cargo check -p perry and then cargo build --profile perry-dev -p perry for normal local compiler iteration; reserve release or dist profiles for optimization-sensitive or shipping builds.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-transform/src/generator/per_iteration.rs` around lines 81 - 82,
Update the local Rust validation workflow associated with per_iteration.rs to
use cargo check -p perry followed by cargo build --profile perry-dev -p perry.
Do not use release or dist profiles unless performing optimization-sensitive or
shipping validation.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/perry-transform/src/generator/per_iteration.rs`:
- Around line 1023-1049: Remove the early return from the Expr::Closure branch
in rewrite_cells_in_stmt so processing falls through to walk_expr_children_mut
after rewriting the closure body and capture lists. Preserve the existing cell
demotion logic while ensuring closure child expressions, including default
parameters and computed properties, are traversed and rewritten.

---

Nitpick comments:
In `@crates/perry-transform/src/generator/per_iteration.rs`:
- Around line 81-82: Update the local Rust validation workflow associated with
per_iteration.rs to use cargo check -p perry followed by cargo build --profile
perry-dev -p perry. Do not use release or dist profiles unless performing
optimization-sensitive or shipping validation.

In `@test-files/test_gap_6354_async_written_binding_across_await.ts`:
- Around line 1-4: Run the gap test suite for
test_gap_6354_async_written_binding_across_await.ts using exactly the Node
version specified in .node-version. Do not substitute another Node release, and
verify the test passes under the pinned version.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fd2d210d-5eb0-48de-be63-3508842a01e4

📥 Commits

Reviewing files that changed from the base of the PR and between a7dd328 and 8f5f183.

📒 Files selected for processing (4)

• crates/perry-transform/src/generator/lower.rs
• crates/perry-transform/src/generator/mod.rs
• crates/perry-transform/src/generator/per_iteration.rs
• test-files/test_gap_6354_async_written_binding_across_await.ts

[proggeramlug]

Thanks @coderabbitai — addressed the actionable finding and pushed:

• Expr::Closure early return in rewrite_cells_in_expr (valid): removed it so control falls through to walk_expr_children_mut, which for a closure visits only its param defaults (the body is already handled by the explicit loop above, and the walker doesn't re-descend into it — no double-rewrite). This fixes a cell referenced in a closure param default ((x = acc) => …). Added a regression case paramDefaultCapture to the gap fixture; it's byte-identical to Node.

On the two nitpicks (both process reminders, no code change needed):

• Pinned Node version: the fixture emits only deterministic integers/strings, so local Node 26.3 and the pinned 26.5 produce identical output; CI's gap harness validates against the pinned version.
• Build profile: local iteration used cargo build --profile perry-dev -p perry throughout.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@proggeramlug Thanks for the thorough follow-up. The fall-through is the right fix: it preserves the explicit closure-body rewrite while ensuring parameter-default expressions such as (x = acc) => … are rewritten exactly once. The regression coverage and Node parity check complete the actionable item.

🐇 ✅

---

✏️ Learnings added

Learnt from: proggeramlug
URL: https://github.com/PerryTS/perry/pull/6571

Timestamp: 2026-07-18T10:09:03.481Z
Learning: In `crates/perry-transform/src/generator/per_iteration.rs`, `rewrite_cells_in_expr` must let `Expr::Closure` fall through to `walk_expr_children_mut` after explicitly rewriting the closure body: the walker visits closure parameter-default expressions but does not re-descend into the body, preventing both missed cell rewrites in defaults and double-rewrites of the body.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

---

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!