fix: preserve loop lexical TDZ and retained closure cells

changelog.d/10235-loop-lexical-tdz.md

@@ -0,0 +1,15 @@
+Captured forward `let` and `const` bindings now receive fresh TDZ cells at their
+own block entry. Repeated loop entries throw `ReferenceError` before each
+declaration, uninitialized `let` declarations end that entry's TDZ with
+`undefined`, and retained callbacks keep their original iteration's binding.
+Function-scoped `var` bindings continue to share one cell.
+
+Switch cases allocate one shared lexical environment after the discriminant,
+and TDZ cells precede hoisted block-function closures. Code generation also
+allocates a fresh cell in every emitted copy of a `finally` block, preserving
+the shared stack slot across its normal and exceptional paths.
+
+Adds HIR and LLVM regressions plus a bounded byte-for-byte Node/native suite
+covering script and module contexts at O0/Os/Oz with default and compact GC
+configurations, retained callbacks, recursion, skipped declarations, and
+exceptional `finally` paths.

crates/perry-codegen/src/stmt/mod.rs

@@ -33,6 +33,8 @@ mod loops;
 mod masked_window_region;
 #[cfg(test)]
 mod prealloc_module_global_tests;
+#[cfg(test)]
+mod prealloc_tdz_path_tests;
 pub(crate) mod stable_packed_accumulator;
 pub(crate) mod stable_packed_loop;
 mod stable_packed_typed_array;
@@ -690,14 +692,10 @@ fn emit_preallocate_boxes(ctx: &mut FnCtx<'_>, ids: &[u32], tdz: bool) -> Result
         if ctx.module_globals.contains_key(id) {
             continue;
         }
-        if ctx.locals.contains_key(id) {
-            // A previous PreallocateBoxes (or an unusual nesting)
-            // already set this up -- skip to keep the existing slot.
+        if !tdz && ctx.locals.contains_key(id) {
+            // Ordinary preallocation preserves a shared function-scoped cell.
             ctx.prealloc_boxes.insert(*id);
             ctx.boxed_vars.insert(*id);
-            if tdz {
-                ctx.tdz_boxes.insert(*id);
-            }
             continue;
         }
         let is_i32_control = crate::expr::is_compiler_private_async_i32_control_local(ctx, *id);
@@ -740,19 +738,29 @@ fn emit_preallocate_boxes(ctx: &mut FnCtx<'_>, ids: &[u32], tdz: bool) -> Result
                 "jsvalue_box_cell",
             )
         };
-        let slot = ctx.func.alloca_entry(crate::types::I64);
-        // perry#4926: PreallocateBoxes can sit nested inside an If/Try/Labeled
-        // body (e.g. the async state-machine wrapper), so this block's
-        // box-pointer store doesn't necessarily dominate every load of the
-        // slot. Entry-init the slot to TAG_UNDEFINED so paths that bypass this
-        // statement read a defined sentinel instead of `undef` (see the boxed
-        // `Stmt::Let` arm in let_stmt.rs). The slot holds a *box pointer*, not
-        // the value, so it is TAG_UNDEFINED-initialized in both the TDZ and
-        // non-TDZ cases -- the TAG_TDZ sentinel lives in the box cell, not the
-        // slot.
-        let undef_bits = crate::nanbox::TAG_UNDEFINED_I64.to_string();
-        ctx.func
-            .entry_allocas_push_store(crate::types::I64, &undef_bits, &slot);
+        // #10051: a TDZ statement creates this entry's lexical environment.
+        // Emit its allocation even when an earlier COPY of the statement was
+        // lowered already (normal/exceptional finally paths, for example).
+        // Reuse the stack slot, but never the previous entry's heap cell:
+        // retained closures must keep their original binding and value.
+        let slot = if let Some(slot) = ctx.locals.get(id) {
+            slot.clone()
+        } else {
+            let slot = ctx.func.alloca_entry(crate::types::I64);
+            // perry#4926: PreallocateBoxes can sit nested inside an If/Try/Labeled
+            // body (e.g. the async state-machine wrapper), so this block's
+            // box-pointer store doesn't necessarily dominate every load of the
+            // slot. Entry-init the slot to TAG_UNDEFINED so paths that bypass this
+            // statement read a defined sentinel instead of `undef` (see the boxed
+            // `Stmt::Let` arm in let_stmt.rs). The slot holds a *box pointer*, not
+            // the value, so it is TAG_UNDEFINED-initialized in both the TDZ and
+            // non-TDZ cases -- the TAG_TDZ sentinel lives in the box cell, not the
+            // slot.
+            let undef_bits = crate::nanbox::TAG_UNDEFINED_I64.to_string();
+            ctx.func
+                .entry_allocas_push_store(crate::types::I64, &undef_bits, &slot);
+            slot
+        };
         ctx.block().store(crate::types::I64, &box_ptr, &slot);
         record_boxed_slot_js_value_bits(ctx, *id, &box_ptr, "preallocate_boxes.box_ptr_slot");
         if cell_note != "jsvalue_box_cell" {

crates/perry-codegen/src/stmt/prealloc_tdz_path_tests.rs

@@ -0,0 +1,94 @@
+//! #10051: copies of a lexical scope must each execute their TDZ allocation.
+use perry_hir::{types::Type, Expr, Function, Module, Stmt};
+
+fn emit(body: Vec<Stmt>) -> String {
+    let mut module = Module::new("tdz_paths.ts");
+    module.functions.push(Function {
+        id: 1,
+        name: "test".into(),
+        type_params: Vec::new(),
+        params: Vec::new(),
+        return_type: Type::Any,
+        body,
+        is_async: false,
+        is_generator: false,
+        is_strict: true,
+        is_exported: true,
+        captures: Vec::new(),
+        decorators: Vec::new(),
+        was_plain_async: false,
+        was_unrolled: false,
+    });
+    String::from_utf8(
+        crate::compile_module(&module, super::prealloc_module_global_tests::ir_opts()).unwrap(),
+    )
+    .unwrap()
+}
+
+fn allocated_slots(ir: &str, seed: &str) -> Vec<String> {
+    ir.lines()
+        .filter_map(|line| {
+            if !line.contains(&format!("call i64 @js_box_alloc_bits(i64 {seed})")) {
+                return None;
+            }
+            let value = line.trim().split(" = ").next().unwrap();
+            let prefix = format!("store i64 {value}, ptr ");
+            Some(
+                ir.lines()
+                    .find_map(|store| store.trim().strip_prefix(&prefix).map(str::to_string))
+                    .expect("each allocated box is stored"),
+            )
+        })
+        .collect()
+}
+
+#[test]
+fn tdz_finally_allocates_on_normal_and_exception_paths() {
+    let ir = emit(vec![Stmt::Try {
+        body: vec![Stmt::Expr(Expr::Call {
+            callee: Box::new(Expr::LocalGet(99)),
+            args: Vec::new(),
+            type_args: Vec::new(),
+            byte_offset: 0,
+        })],
+        catch: None,
+        finally: Some(vec![
+            Stmt::PreallocateTdzBoxes(vec![10]),
+            Stmt::Let {
+                id: 10,
+                name: "value".into(),
+                ty: Type::Any,
+                mutable: true,
+                init: Some(Expr::Integer(42)),
+            },
+        ]),
+    }]);
+    let slots = allocated_slots(&ir, crate::nanbox::TAG_TDZ_I64);
+    assert_eq!(
+        slots.len(),
+        2,
+        "both finally paths need fresh TDZ cells:\n{ir}"
+    );
+    assert_eq!(slots[0], slots[1], "path copies share one stack slot");
+    assert!(
+        ir.contains(&format!(
+            "store i64 {}, ptr {}",
+            crate::nanbox::TAG_UNDEFINED_I64,
+            slots[0]
+        )),
+        "slot must be entry-initialized"
+    );
+}
+
+#[test]
+fn ordinary_preallocation_still_preserves_an_existing_cell() {
+    let ir = emit(vec![
+        Stmt::PreallocateBoxes(vec![10]),
+        Stmt::PreallocateBoxes(vec![10]),
+    ]);
+    assert_eq!(
+        allocated_slots(&ir, crate::nanbox::TAG_UNDEFINED_I64).len(),
+        1,
+        "ordinary function-scoped cells must not be freshened:\n{ir}"
+    );
+}

crates/perry-hir/src/ir/stmt.rs

@@ -72,6 +72,9 @@ pub enum Stmt {
     /// read of such a box before its `Stmt::Let` runs throws a spec
     /// ReferenceError; the `Stmt::Let` (or `let x;` with no init) overwrites
     /// the sentinel with the real value / `undefined`, ending the dead zone.
+    /// Nested lexical scopes emit this at block entry: every execution must
+    /// allocate a fresh cell, even when codegen emits multiple copies of that
+    /// block (such as a finally body on normal and exceptional paths).
     PreallocateTdzBoxes(Vec<LocalId>),
     /// Hand the heap box cells behind a set of boxed LocalIds to the async
     /// activation lifetime tracker (#7933 / #8213). A cell no closure captures

crates/perry-hir/src/lower/expr_function.rs

@@ -1211,6 +1211,7 @@ fn lower_fn_expr_anon(ctx: &mut LoweringContext, fn_expr: &ast::FnExpr) -> Resul
                     &combined,
                     &hoisted_id_set,
                 );
+                prealloc.retain(|id| !ctx.nested_forward_scope_ids.contains(id));
                 for id in &forward_boxed_ids {
                     if !prealloc.contains(id) {
                         prealloc.push(*id);

crates/perry-hir/src/lower/lowering_context.rs

@@ -589,6 +589,8 @@ pub struct LoweringContext {
     /// enclosing scope). Without this, a same-named `let` in a sibling block
     /// was skipped (deduped by name) and any post-block reference of the name
     /// resolved to the block's box instead of the outer binding.
+    /// Their TDZ cells are also allocated at block entry, rather than function
+    /// entry, to preserve per-entry binding identity and the TDZ in loops.
     pub(crate) nested_forward_scope_ids: HashSet<LocalId>,
     /// Shadow index: function name -> index in `functions` Vec (last entry for shadowing)
     pub(crate) functions_index: HashMap<String, usize>,

crates/perry-hir/src/lower/stmt.rs

@@ -1856,7 +1856,7 @@ pub(crate) fn lower_stmt(
             module.init.push(Stmt::Throw(expr));
         }
         ast::Stmt::Switch(switch_stmt) => {
-            let discriminant = lower_expr(ctx, &switch_stmt.discriminant)?;
+            let mut discriminant = lower_expr(ctx, &switch_stmt.discriminant)?;
             let mut cases = Vec::new();
             let switch_scope_mark = ctx.push_block_scope();
             // Case statement-lists share the switch's block scope without
@@ -1868,8 +1868,9 @@ pub(crate) fn lower_stmt(
             // one shared scope key: a second case re-declaring the name is a
             // redeclaration, not a shadow.
             let mut saved_class_renames = Vec::new();
+            let mut tdz_boxes = Vec::new();
             for case in &switch_stmt.cases {
-                rebind_nested_forward_scope_lets(ctx, &case.cons);
+                tdz_boxes.extend(rebind_nested_forward_scope_lets(ctx, &case.cons));
                 saved_class_renames.extend(enter_class_rename_scope(
                     ctx,
                     switch_stmt.span.lo.0,
@@ -1891,6 +1892,20 @@ pub(crate) fn lower_stmt(
             exit_class_rename_scope(ctx, saved_class_renames);
             ctx.pop_block_scope(switch_scope_mark);
 
+            if !tdz_boxes.is_empty() {
+                // Evaluate the discriminant before entering the shared case
+                // environment; fallthrough must not allocate a second cell.
+                let id = ctx.fresh_local();
+                module.init.push(Stmt::Let {
+                    id,
+                    name: "__switch_discriminant".into(),
+                    ty: Type::Any,
+                    mutable: false,
+                    init: Some(discriminant),
+                });
+                module.init.push(Stmt::PreallocateTdzBoxes(tdz_boxes));
+                discriminant = Expr::LocalGet(id);
+            }
             module.init.push(Stmt::Switch {
                 discriminant,
                 cases,

crates/perry-hir/src/lower_decl/block.rs

@@ -19,7 +19,7 @@ pub(crate) use var_names::{
 };
 
 pub fn lower_block_stmt(ctx: &mut LoweringContext, block: &ast::BlockStmt) -> Result<Vec<Stmt>> {
-    rebind_nested_forward_scope_lets(ctx, &block.stmts);
+    let tdz_boxes = rebind_nested_forward_scope_lets(ctx, &block.stmts);
     // #9466: `class` is block-scoped, so a `class X` here is a DISTINCT class
     // from any enclosing/prior `class X` and needs its own registration key.
     // This is the funnel every `{}`-shaped scope shares — bare block, `if` /
@@ -35,7 +35,12 @@ pub fn lower_block_stmt(ctx: &mut LoweringContext, block: &ast::BlockStmt) -> Re
     let saved_class_renames = enter_class_rename_scope(ctx, block.span.lo.0, &block.stmts);
     let lowered = lower_stmts_using_aware(ctx, &block.stmts);
     exit_class_rename_scope(ctx, saved_class_renames);
-    lowered
+    lowered.map(|mut body| {
+        if !tdz_boxes.is_empty() {
+            body.insert(0, Stmt::PreallocateTdzBoxes(tdz_boxes));
+        }
+        body
+    })
 }
 
 /// Make the forward-captured `let`/`const` bindings that
@@ -48,15 +53,22 @@ pub fn lower_block_stmt(ctx: &mut LoweringContext, block: &ast::BlockStmt) -> Re
 /// unwinds, so the binding is visible exactly within its block — a same-named
 /// `let` in a sibling block gets its own id/box, and references after the
 /// block resolve to the outer binding (or stay global) as in Node.
+/// Returns the cells to allocate at this scope's runtime entry. In particular,
+/// a loop must allocate NEW cells on every entry, both to restart the TDZ and
+/// to leave callbacks from previous iterations attached to their original cells.
 ///
 /// Called from [`lower_block_stmt`] (every `{}`-shaped scope: block, `try` /
 /// `catch` / `finally`, block-bodied `if` / loop / labeled bodies) and from
 /// the two switch-case lowering arms (`lower/stmt.rs`, `lower_decl/
 /// body_stmt.rs`), whose case statement-lists share the switch's block scope
 /// without being a `BlockStmt`.
-pub(crate) fn rebind_nested_forward_scope_lets(ctx: &mut LoweringContext, stmts: &[ast::Stmt]) {
+pub(crate) fn rebind_nested_forward_scope_lets(
+    ctx: &mut LoweringContext,
+    stmts: &[ast::Stmt],
+) -> Vec<LocalId> {
+    let mut tdz_boxes = Vec::new();
     if ctx.lexical_forward_decls.is_empty() {
-        return;
+        return tdz_boxes;
     }
     for stmt in stmts {
         let ast::Stmt::Decl(ast::Decl::Var(var_decl)) = stmt else {
@@ -75,11 +87,13 @@ pub(crate) fn rebind_nested_forward_scope_lets(ctx: &mut LoweringContext, stmts:
                 if let Some(&id) = ctx.lexical_forward_decls.get(&span_lo) {
                     if ctx.nested_forward_scope_ids.contains(&id) {
                         ctx.locals.push((name, id, Type::Any));
+                        tdz_boxes.push(id);
                     }
                 }
             }
         }
     }
+    tdz_boxes
 }
 
 /// Collect identifier names referenced INSIDE any closure (arrow / function
@@ -106,7 +120,9 @@ pub(crate) fn rebind_nested_forward_scope_lets(ctx: &mut LoweringContext, stmts:
 /// scope local now (so the earlier closure resolves it to the local and
 /// captures the live box) and span-keyed in `lexical_forward_decls` so the
 /// declaration — including a destructuring leaf — reuses the same id. Returns
-/// the pre-registered ids so the caller can prealloc their boxes at entry.
+/// the function-scoped ids so the caller can prealloc their boxes at function
+/// entry. Nested lexical ids are allocated at their own scope's entry by
+/// `rebind_nested_forward_scope_lets`'s callers.
 ///
 /// `body_entry_locals_len` is `ctx.locals.len()` captured before any of this
 /// body's own locals were defined — anything at or above it is in THIS scope,
@@ -131,9 +147,9 @@ pub(crate) fn pre_register_forward_captured_lets(
     // `try { let cb = () => x; let x = …; cb() }` (esbuild `__esm` streaming
     // closures in the compiled query async-generator) fell through to
     // `js_global_get_or_throw_unresolved` → `ReferenceError: x is not
-    // defined`. Forward-captured boxes from any depth still preallocate at
-    // function entry (Phase 4/5) and each declaration reuses its id by span
-    // (`lexical_forward_decls`).
+    // defined`. Function-scoped boxes preallocate at function entry (Phase
+    // 4/5); nested lexical boxes at their own block entry. Each declaration
+    // reuses its id by span (`lexical_forward_decls`).
     //
     // The bool is `is_nested`: only the function-body top level (front entry)
     // defines its pre-registrations as name-visible function-scope locals.
@@ -218,7 +234,6 @@ pub(crate) fn pre_register_forward_captured_lets(
                                 ctx.var_hoisted_ids.insert(id);
                                 ctx.tdz_forward_ids.insert(id);
                                 ctx.nested_forward_scope_ids.insert(id);
-                                forward_boxed_ids.push(id);
                                 ctx.lexical_forward_decls.insert(span_lo, id);
                                 registered_here.insert(name);
                             } else {
@@ -754,6 +769,7 @@ pub fn lower_fn_body_block_stmt(
     // the box before the declaration assigns through it.
     let combined: Vec<Stmt> = hoisted_lets.iter().chain(other.iter()).cloned().collect();
     let mut prealloc = compute_prealloc_for_hoisted_closures(&combined, &hoisted_id_set);
+    prealloc.retain(|id| !ctx.nested_forward_scope_ids.contains(id));
     for id in forward_boxed_ids {
         if !prealloc.contains(&id) {
             prealloc.push(id);
@@ -1075,7 +1091,7 @@ fn lower_strict_block_fn_decls(
 ) -> Result<Vec<Stmt>> {
     use std::collections::HashSet;
 
-    rebind_nested_forward_scope_lets(ctx, &block.stmts);
+    let tdz_boxes = rebind_nested_forward_scope_lets(ctx, &block.stmts);
 
     let mut hoisted_ids = HashSet::new();
     for stmt in &block.stmts {
@@ -1092,7 +1108,11 @@ fn lower_strict_block_fn_decls(
         hoisted_ids.insert(id);
     }
     if hoisted_ids.is_empty() {
-        return lower_stmts_using_aware(ctx, &block.stmts);
+        let mut body = lower_stmts_using_aware(ctx, &block.stmts)?;
+        if !tdz_boxes.is_empty() {
+            body.insert(0, Stmt::PreallocateTdzBoxes(tdz_boxes));
+        }
+        return Ok(body);
     }
 
     // Lower in source order first: a declaration body may capture lexical
@@ -1115,8 +1135,15 @@ fn lower_strict_block_fn_decls(
     }
 
     let combined: Vec<_> = hoisted.iter().chain(other.iter()).cloned().collect();
-    let prealloc = compute_prealloc_for_hoisted_closures(&combined, &hoisted_ids);
+    let mut prealloc = compute_prealloc_for_hoisted_closures(&combined, &hoisted_ids);
+    // A hoisted closure may capture a forward lexical from this block. Its
+    // TDZ cell is already allocated here; never replace it with an ordinary
+    // undefined-seeded cell or hoist a nested block's cell into this scope.
+    prealloc.retain(|id| !ctx.nested_forward_scope_ids.contains(id));
     let mut result = Vec::new();
+    if !tdz_boxes.is_empty() {
+        result.push(Stmt::PreallocateTdzBoxes(tdz_boxes));
+    }
     if !prealloc.is_empty() {
         result.push(Stmt::PreallocateBoxes(prealloc));
     }