fix(codegen): a taken throw out of a packed loop no longer loses loop-carried locals

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

@@ -5063,9 +5063,26 @@ fn stmt_is_packed_f64_loop_safe(
                     .as_ref()
                     .is_none_or(|expr| expr_is_packed_f64_loop_safe(ctx, expr, arr_id, counter_id))
         }
-        // `throw` stays out: the thrown value is typically constructed
-        // (`throw new Error(…)`), which is a call in the loop body.
-        Stmt::Throw(_) => packed_loop_abrupt_enabled(),
+        // `throw` stays out. Admitting it (#9185) was a silent wrong answer:
+        // `break`/`continue`/`return` leave the clone through normal CFG edges
+        // that flush the loop-carried locals back to their frame slots, but an
+        // unwind edge does not, so anything reading such a local AFTER the
+        // throw saw its stale pre-loop value:
+        //
+        //     let s = 0;
+        //     try { for (let i = 0; i < arr.length; i++) {
+        //             if (arr[i] === 40) throw PRE; s += arr[i]; } }
+        //     catch (e) { return s; }   // gave 0, node gives 780
+        //
+        // #9185's tests missed it because none of them read a loop-carried
+        // local after unwinding — they read the thrown value (which IS the
+        // clone's live value, and was correct) or an untouched variable. A
+        // closure-captured accumulator was also correct, being boxed rather
+        // than register-promoted, which is what kept the bug this narrow.
+        //
+        // Re-admitting this needs the writeback emitted at the throw site, not
+        // just the admission; see #9210.
+        Stmt::Throw(_) => false,
         Stmt::LabeledBreak(_)
         | Stmt::LabeledContinue(_)
         | Stmt::While { .. }

crates/perry-ui-macos/src/file_dialog.rs

@@ -93,7 +93,7 @@ pub fn save_dialog(callback: f64, default_name_ptr: *const u8, _allowed_types_pt
 
         // Set default filename if provided
         if !default_name_ptr.is_null() {
-            let name = unsafe { str_from_header(default_name_ptr) };
+            let name = str_from_header(default_name_ptr);
             if !name.is_empty() {
                 let ns_name = NSString::from_str(&name);
                 let _: () = msg_send![&*panel, setNameFieldStringValue: &*ns_name];

crates/perry-ui-macos/src/lib_ffi/system.rs

@@ -275,7 +275,7 @@ pub extern "C" fn perry_system_preferences_set(key_ptr: i64, value: f64) {
         if (bits >> 48) == 0x7FFF {
             // NaN-boxed string — extract string pointer
             let str_ptr = js_nanbox_get_pointer(value) as *const u8;
-            let s = unsafe { str_from_header(str_ptr) };
+            let s = str_from_header(str_ptr);
             let ns_str = objc2_foundation::NSString::from_str(&s);
             let _: () = objc2::msg_send![defaults, setObject: &*ns_str, forKey: &*ns_key];
         } else {

crates/perry-ui-macos/src/lib_ffi/window_misc.rs

@@ -408,7 +408,7 @@ pub extern "C" fn perry_ui_set_text(id_ptr: i64, value_ptr: i64) {
     let (value_data, value_len) = value
         .as_ref()
         .map_or((std::ptr::null(), 0), |value| (value.as_ptr(), value.len()));
-    unsafe {
+    {
         widgets::text_registry::set_text_handler(id.as_ptr(), id.len(), value_data, value_len);
     }
 }

crates/perry-ui-macos/src/widgets/alert.rs

@@ -47,7 +47,7 @@ pub fn show(title_ptr: *const u8, message_ptr: *const u8, arr_ptr: i64, callback
             for i in 0..len {
                 let elem = js_array_get_element(arr_ptr, i);
                 let str_ptr = js_get_string_pointer_unified(elem) as *const u8;
-                let label = unsafe { str_from_header(str_ptr) };
+                let label = str_from_header(str_ptr);
                 let ns_label = NSString::from_str(&label);
                 let _: Retained<AnyObject> = msg_send![&*alert, addButtonWithTitle: &*ns_label];
             }

crates/perry-ui-macos/src/widgets/combobox.rs

@@ -111,7 +111,7 @@ pub fn create(initial_ptr: *const u8, on_change: f64) -> i64 {
         let _: () = msg_send![&*combobox, setEditable: true];
         let _: () = msg_send![&*combobox, setNumberOfVisibleItems: 8i64];
 
-        let initial_str = unsafe { str_from_header(initial_ptr) };
+        let initial_str = str_from_header(initial_ptr);
         if !initial_str.is_empty() {
             let ns_initial = NSString::from_str(&initial_str);
             let _: () = msg_send![&*combobox, setStringValue: &*ns_initial];

crates/perry-ui-macos/src/widgets/webview.rs

@@ -747,7 +747,7 @@ pub fn set_allowed_domains(handle: i64, domains_arr_handle: i64) {
             let elem = js_array_get_element_f64(domains_arr_handle, i);
             let str_ptr = js_get_string_pointer_unified(elem) as *const u8;
             if !str_ptr.is_null() {
-                domains.push(unsafe { str_from_header(str_ptr) }.to_string());
+                domains.push(str_from_header(str_ptr).to_string());
             }
         }
     }

crates/perry/tests/packed_loop_abrupt_statements.rs

@@ -145,11 +145,16 @@ fn break_in_a_loop_whose_array_grows() {
 }
 
 #[test]
-fn a_throw_that_does_not_construct_takes_the_fast_path_correctly() {
-    // A `throw` whose value is already built is admitted: its block ends in
-    // `unreachable`, so control never returns to the loop and nothing reads
-    // what the clone cached. A throw that CONSTRUCTS its value is not, because
-    // the construction is emitted in blocks preceding the terminating one.
+fn a_throw_that_does_not_construct_is_still_correct() {
+    // This shape was admitted to the fast path by #9185 on the reasoning that
+    // the throw block ends in `unreachable`, so "control never returns to the
+    // loop and nothing reads what the clone cached". The second half of that
+    // is false — an unwind lands in a `catch`, which can read anything the
+    // loop wrote — and the loop is no longer admitted. See
+    // `a_loop_carried_local_survives_a_taken_throw` below for the case that
+    // proves it, and note what these assertions do NOT check: `throwPre`
+    // reads the thrown value and `k`, `throwValue` throws `s` itself. Both
+    // observe the clone's live value, never the frame slot left behind.
     let out = compile_and_run(&format!(
         "{PRELUDE}
         const PRE = new Error(\"boom\");
@@ -175,9 +180,7 @@ fn a_throw_that_does_not_construct_takes_the_fast_path_correctly() {
 
 #[test]
 fn throw_inside_the_loop_is_still_correct() {
-    // A throw that CONSTRUCTS its value is not admitted (the construction is a
-    // call in a block that does not end in `unreachable`), but it must keep
-    // working on the generic path.
+    // A throw that CONSTRUCTS its value must keep working on the generic path.
     let out = compile_and_run(&format!(
         "{PRELUDE}
         function throwAt(k: number): string {{
@@ -192,3 +195,64 @@ fn throw_inside_the_loop_is_still_correct() {
     ));
     assert_eq!(out, "hit15 none2016");
 }
+
+/// #9185 admitted `throw` to the packed fast path; this is the case that
+/// showed it was a silent wrong answer.
+///
+/// `break` and `continue` leave the clone through normal CFG edges, which
+/// flush the loop-carried locals back to their frame slots. An unwind edge
+/// does not, so the `catch` below read `s` from a slot the loop never
+/// updated and got its pre-loop `0` instead of `780`.
+///
+/// Every assertion here reads a loop-carried local AFTER the abrupt exit,
+/// which is precisely what #9185's own tests did not do. The `break` and
+/// `continue` rows are not padding: they are what established that the defect
+/// was specific to the unwind edge rather than to abrupt exits in general.
+#[test]
+fn a_loop_carried_local_survives_a_taken_throw() {
+    let out = compile_and_run(&format!(
+        "{PRELUDE}
+        const PRE = new Error(\"boom\");
+        function viaBreak(): string {{
+            let s = 0;
+            for (let i = 0; i < arr.length; i++) {{ if (arr[i] === 40) break; s += arr[i]; }}
+            return \"break \" + s;
+        }}
+        function viaContinue(): string {{
+            let s = 0;
+            for (let i = 0; i < arr.length; i++) {{ if (arr[i] % 2 === 0) continue; s += arr[i]; }}
+            return \"continue \" + s;
+        }}
+        function throwThenRead(): string {{
+            let s = 0;
+            try {{
+                for (let i = 0; i < arr.length; i++) {{ if (arr[i] === 40) throw PRE; s += arr[i]; }}
+            }} catch (e) {{ return \"throwBefore \" + s; }}
+            return \"none \" + s;
+        }}
+        function accumulateThenThrow(): string {{
+            let s = 0;
+            try {{
+                for (let i = 0; i < arr.length; i++) {{ s += arr[i]; if (arr[i] === 40) throw PRE; }}
+            }} catch (e) {{ return \"throwAfter \" + s; }}
+            return \"none \" + s;
+        }}
+        function throwThenReadViaClosure(): string {{
+            let s = 0;
+            const get = () => s;
+            try {{
+                for (let i = 0; i < arr.length; i++) {{ if (arr[i] === 40) throw PRE; s += arr[i]; }}
+            }} catch (e) {{ return \"closure \" + get(); }}
+            return \"none \" + get();
+        }}
+        console.log(
+            viaBreak() + \" | \" + viaContinue() + \" | \" + throwThenRead() + \" | \"
+            + accumulateThenThrow() + \" | \" + throwThenReadViaClosure()
+        );
+        "
+    ));
+    assert_eq!(
+        out,
+        "break 780 | continue 1024 | throwBefore 780 | throwAfter 820 | closure 780"
+    );
+}