fix(codegen): restore the numeric array-index fast path for call-returned arrays (#6299)

crates/perry-codegen/src/collectors/index_uses.rs

@@ -496,17 +496,51 @@ pub fn walk_index_uses_in_expr(e: &perry_hir::Expr, out: &mut HashSet<u32>) {
                 walk_index_uses_in_expr(a, out);
             }
         }
+        // (#6299) `arr[i] = v` does NOT always lower to `IndexSet`: when the
+        // receiver isn't a statically-known local array — e.g. it arrived as a
+        // function's return value — the lowerer emits the spec-compliant
+        // `PutValue` form instead. Its `key` IS the array index, so it must be
+        // seeded exactly like `IndexSet`'s `index`; the stored `value` subtree
+        // also holds the matching `arr[i]` read.
+        //
+        // Missing this arm kept the loop counter out of `index_used_locals`, so
+        // it lost its i32 shadow slot at the Let site, which in turn dropped the
+        // guarded numeric array-index fast path for every `arr[i]` in the loop
+        // (6.8x). It was masked until #6258 (`x++`/`x--` no longer counts as
+        // strictly-i32-bounded, correctly — that was an unsound overflow path),
+        // which removed the crutch that had been covering this blind spot.
+        Expr::PutValueSet {
+            target,
+            key,
+            value,
+            receiver,
+            ..
+        } => {
+            collect_index_refs(key, out);
+            walk_index_uses_in_expr(target, out);
+            walk_index_uses_in_expr(key, out);
+            walk_index_uses_in_expr(value, out);
+            walk_index_uses_in_expr(receiver, out);
+        }
         // Closure bodies are intentionally NOT walked: a captured local can't
         // use the i32 slot anyway (boxed captures route through
         // `js_box_get`/`js_box_set` and non-boxed ones through
         // `js_closure_get_capture_bits`), so marking them as index-used would
-        // have no effect at the Let-site emission gate.
+        // have no effect at the Let-site emission gate. This arm precedes the
+        // catch-all below, so the generic walker never descends into a closure.
         Expr::Closure { .. } => {}
-        // Everything else: conservatively skipped. Missing a variant means we
-        // don't recurse further into that subtree — a local used as an index
-        // deeper inside may not be marked, in which case its i32 shadow is
-        // not emitted and the per-iteration `fptosi` cost returns. That's a
-        // missed optimization, not a correctness bug.
-        _ => {}
+        // Everything else: recurse structurally via the centralized walker
+        // (same contract as `collect_ref_ids_in_expr`). Only the explicit
+        // index-using arms above ever *mark* a local, so a total walk cannot
+        // widen the set beyond "locals that flow into an array index" — it just
+        // stops a variant that merely *wraps* an index use (as `PutValueSet`
+        // did) from silently hiding the whole subtree. The old `_ => {}` made
+        // every unlisted variant an opaque wall, which is what let #6299 hide
+        // in plain sight.
+        _ => {
+            perry_hir::walker::walk_expr_children(e, &mut |sub| {
+                walk_index_uses_in_expr(sub, out);
+            });
+        }
     }
 }

crates/perry-codegen/tests/native_proof_regressions.rs

@@ -13277,5 +13277,106 @@ fn raw_numeric_class_field_rejects_unknown_or_dynamic_shape_receiver() {
     }
 }
 
+/// #6299: a numeric array that arrives as a call's return value must still get
+/// the guarded numeric array-index fast path.
+///
+/// `arr[i] = arr[i] + 1` only lowers to `Expr::IndexSet` when the receiver is a
+/// statically-known local array; for a call-returned array the lowerer emits the
+/// spec-compliant `Expr::PutValueSet` instead. `collect_index_used_locals` had no
+/// arm for that variant (it fell into a `_ => {}` catch-all), so the loop counter
+/// never joined `index_used_locals`, lost its i32 shadow slot, and every `arr[i]`
+/// in the loop fell back from `js_typed_feedback_numeric_array_index_{get,set}_guard`
+/// to the generic `js_array_get_index_or_string` path — a 6.8x cliff.
+///
+/// Asserting on the emitted helper (rather than wall-clock time) pins the actual
+/// codegen decision and stays meaningful under any optimization level.
+#[test]
+fn put_value_set_index_keeps_the_numeric_array_fast_path() {
+    // for (let i = 0; i < arr.length; i++) arr[i] = arr[i] + 1;
+    // with `arr: number[]` written through the generic PutValue form.
+    let arr = 1u32;
+    let i = 2u32;
+    let read_arr_i = Expr::IndexGet {
+        object: Box::new(Expr::LocalGet(arr)),
+        index: Box::new(Expr::LocalGet(i)),
+    };
+    let body = vec![
+        Stmt::Let {
+            id: arr,
+            name: "arr".to_string(),
+            ty: Type::Array(Box::new(Type::Number)),
+            mutable: false,
+            init: Some(Expr::Array(vec![])),
+        },
+        // The array escapes into a call — this is what `const arr = build()`
+        // lowers to (the callee fills the array through a return slot). It is
+        // load-bearing for the repro: an array that stays local is still
+        // hoistable, so `stmt/loops.rs` hands the counter an i32 slot from the
+        // length-hoist path and the fast path survives even with the collector
+        // blind spot. Once the array escapes, that path bails and the counter's
+        // only route to an i32 slot is `index_used_locals` — the set this fix
+        // repairs.
+        Stmt::Expr(Expr::Call {
+            callee: Box::new(Expr::FuncRef(1)),
+            args: vec![Expr::LocalGet(arr)],
+            type_args: Vec::new(),
+            byte_offset: 0,
+        }),
+        Stmt::For {
+            init: Some(Box::new(Stmt::Let {
+                id: i,
+                name: "i".to_string(),
+                ty: Type::Any,
+                mutable: true,
+                init: Some(Expr::Integer(0)),
+            })),
+            condition: Some(Expr::Compare {
+                op: CompareOp::Lt,
+                left: Box::new(Expr::LocalGet(i)),
+                right: Box::new(Expr::PropertyGet {
+                    object: Box::new(Expr::LocalGet(arr)),
+                    property: "length".to_string(),
+                }),
+            }),
+            update: Some(Expr::Update {
+                id: i,
+                op: UpdateOp::Increment,
+                prefix: false,
+            }),
+            body: vec![Stmt::Expr(Expr::PutValueSet {
+                target: Box::new(Expr::LocalGet(arr)),
+                key: Box::new(Expr::LocalGet(i)),
+                value: Box::new(Expr::Binary {
+                    op: BinaryOp::Add,
+                    left: Box::new(read_arr_i),
+                    right: Box::new(Expr::Integer(1)),
+                }),
+                receiver: Box::new(Expr::LocalGet(arr)),
+                strict: false,
+            })],
+        },
+    ];
+
+    let ir = compile_ir("put_value_set_fast_path", body);
+
+    // The loop counter must keep its i32 shadow slot: that is what lets the
+    // guarded helpers take a native i32 index.
+    assert!(
+        ir.contains("alloca i32"),
+        "the `arr[i]` loop counter lost its i32 shadow slot — `index_used_locals` \
+         no longer sees the PutValueSet key (#6299):\n{ir}"
+    );
+    assert!(
+        ir.contains("@js_typed_feedback_numeric_array_index_get_guard"),
+        "`arr[i]` read through PutValueSet's value subtree must keep the guarded \
+         numeric fast path, not fall back to js_array_get_index_or_string (#6299):\n{ir}"
+    );
+    assert!(
+        ir.contains("@js_typed_feedback_numeric_array_index_set_guard"),
+        "`arr[i] = ...` through PutValueSet must keep the guarded numeric store \
+         fast path (#6299):\n{ir}"
+    );
+}
+
 #[path = "native_proof_regressions/invalidation.rs"]
 mod invalidation;