From 9faab44b2246c86ac2c63d45e44a57a6f7f17288 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 31 Aug 2026 02:14:45 +0200 Subject: [PATCH 1/2] perf(codegen): constructed throw operands keep the packed fast path (7.99 -> 0.95 ns, 8.1x vs node) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Builds on the writeback fix in the parent commit. That made a throw safe; this makes the common validation shape FAST, and closes a hole the parent still had. `if (bad) throw new Error("…")` stayed on the generic path at 7.99 ns/op. Two gates had to open together, which is why neither showed an effect alone: the length hoist rejected the construction node, AND the admission rejected the operand. Opening both puts every throw shape at 0.95. The hole. The writeback is emitted AT the throw site, after the operand is lowered, so an operand that unwinds ON ITS OWN skips it and leaves the accumulators stale — #9185's defect one level deeper. Two expressions can unwind without looking like calls: const evil = { toString() { throw new Error("x") } }; throw new Error(evil); // stringifying the message dispatches throw (evil + two); // `+` dispatches to valueOf/toString Both gave `c=0 s=0` where node gives `c=40 s=780`. The second needs no Error at all and was reachable through the parent commit's own admission rule, which accepted `Expr::Binary` over locals. `throw_operand_cannot_unwind` replaces that rule. Throwing a value coerces nothing, so a bare local stays admissible whatever it holds; anything that COERCES — arithmetic, concatenation, an Error message being stringified — must be `expr_is_coercion_free_primitive`: literals, the loop counter, an element of the packed array (a genuine double by the clone's own guard), and arithmetic over those. Deliberately syntactic, and deliberately narrower than `expr_is_packed_f64_loop_safe`, whose question is "can the clone keep running" rather than "can this unwind". Error identity is not decided here: HIR emits the Error nodes only for the INTRINSIC constructor (`lower_new` gates on `!shadowed_by_user_binding`), so a user `class Error` lowers to `Expr::New` and is still rejected. Four shadowing forms are covered by tests. `ErrorNewWithCause`/`WithOptions` stay out — their options object is read at runtime and can carry getters. Measured on the quiet host (Mac mini, load 1.5), 5 reps x 2 runs, stable to 0.01 ns/op — perry now beats node on every shape in this benchmark: new Error("bad") 7.99 -> 0.95 node 7.71 8.1x FASTER new Error("bad " + i) 7.99 -> 0.95 node 7.75 8.2x new Error() 7.99 -> 0.95 node 7.75 8.2x throw "bad " + i 7.99 -> 0.95 node 7.73 8.1x throw PRE 0.95 -> 0.95 node 1.11 1.17x no throw 0.95 -> 0.95 node 1.10 1.16x 33 differential programs against node, all identical, including every hazard above, a taken throw under `PERRY_GC_FORCE_EVACUATE=1`, and a `finally` reading the accumulators mid-unwind. Size: __text +1024 B (+0.0099%) on identical sources with the admission toggled. Refs #9151, #9185, #9210, #9232. --- crates/perry-codegen/src/stmt/loops.rs | 134 ++++++- crates/perry/tests/packed_loop_error_throw.rs | 360 ++++++++++++++++++ 2 files changed, 492 insertions(+), 2 deletions(-) create mode 100644 crates/perry/tests/packed_loop_error_throw.rs diff --git a/crates/perry-codegen/src/stmt/loops.rs b/crates/perry-codegen/src/stmt/loops.rs index 8a57441397..a886bca8e2 100644 --- a/crates/perry-codegen/src/stmt/loops.rs +++ b/crates/perry-codegen/src/stmt/loops.rs @@ -5147,9 +5147,24 @@ fn stmt_is_packed_f64_loop_safe( // way `Stmt::Return` checks its value; #9185 admitted any throw at all // and leaned on `stmt_array_length_effect` to reject the constructing // ones, which is a weaker guarantee than stating the requirement here. + // The thrown operand is deliberately NOT required to be + // `expr_is_packed_f64_loop_safe`. That predicate asks "can the clone + // keep running after this", and after a throw it cannot: control + // leaves the loop for a landing pad and never returns, so no later + // iteration can observe a value the operand disturbed. + // + // What the operand still must not do is change `arr.length` before the + // hoisted bound is used, and that IS checked — `stmt_preserves_array_length` + // walks `Stmt::Throw(e)` into `e`, so `throw arr.pop()` is rejected + // there. Keeping the two questions in their own predicates is what lets + // a constructed operand (`new Error(…)`, `"bad " + i`) stay on the fast + // path; requiring loop-safety here costs 8.4x for no correctness gain. + // + // The accumulator writeback this needs is emitted at the throw site by + // `flush_packed_accumulator_locals`; without it this admission is the + // #9185 wrong answer. Stmt::Throw(value) => { - packed_loop_abrupt_enabled() - && expr_is_packed_f64_loop_safe(ctx, value, arr_id, counter_id) + packed_loop_abrupt_enabled() && throw_operand_cannot_unwind(ctx, value, arr_id, counter_id) } Stmt::LabeledBreak(_) | Stmt::LabeledContinue(_) @@ -5287,6 +5302,99 @@ fn local_is_int32_value(ctx: &FnCtx<'_>, local_id: u32) -> bool { ) } +/// Can evaluating `expr` as a thrown operand UNWIND before the throw itself? +/// +/// This is the guard on `flush_packed_accumulator_locals`. That flush is +/// emitted at the throw site, after the operand is lowered — so an operand that +/// unwinds on its own leaves the loop-carried accumulators stale, reproducing +/// #9185 one level deeper. `expr_is_packed_f64_loop_safe` is NOT sufficient +/// here: it accepts a bare `Expr::Binary` over locals, and `+` on an object +/// dispatches to a user `valueOf`/`toString` that can throw. +/// +/// Throwing a value coerces nothing, so a bare local is fine. Anything that +/// COERCES — arithmetic, concatenation, an Error message being stringified — +/// must be provably free of user dispatch. +fn throw_operand_cannot_unwind( + ctx: &FnCtx<'_>, + expr: &perry_hir::Expr, + arr_id: u32, + counter_id: u32, +) -> bool { + use perry_hir::Expr; + match expr { + // Thrown as-is: no conversion, so no user code, whatever it holds. + Expr::LocalGet(_) + | Expr::String(_) + | Expr::Number(_) + | Expr::Integer(_) + | Expr::Bool(_) + | Expr::Null + | Expr::Undefined => true, + Expr::ErrorNew(message) => message + .as_ref() + .is_none_or(|m| expr_is_coercion_free_primitive(ctx, m, arr_id, counter_id)), + Expr::TypeErrorNew(message) + | Expr::RangeErrorNew(message) + | Expr::ReferenceErrorNew(message) + | Expr::SyntaxErrorNew(message) => { + expr_is_coercion_free_primitive(ctx, message, arr_id, counter_id) + } + Expr::Binary { left, right, .. } | Expr::Compare { left, right, .. } => { + expr_is_coercion_free_primitive(ctx, left, arr_id, counter_id) + && expr_is_coercion_free_primitive(ctx, right, arr_id, counter_id) + } + Expr::Unary { operand, .. } => { + expr_is_coercion_free_primitive(ctx, operand, arr_id, counter_id) + } + _ => false, + } +} + +/// Can `expr` be stringified WITHOUT running user code? +/// +/// Deliberately syntactic and much narrower than `expr_is_packed_f64_loop_safe`: +/// that predicate accepts a bare `Expr::LocalGet`, which is right for a value +/// that is merely thrown (throwing coerces nothing) but wrong for one that is +/// about to be stringified by an Error constructor, since the local may hold an +/// object with a user `toString`. +/// +/// Admitted: literals, the loop counter, a read of the packed array being +/// iterated (both are numbers by the loop's own guards), and arithmetic or +/// concatenation over those. Anything else — including any other local — is +/// rejected, because proving it is not an object is not this predicate's job. +fn expr_is_coercion_free_primitive( + ctx: &FnCtx<'_>, + expr: &perry_hir::Expr, + arr_id: u32, + counter_id: u32, +) -> bool { + use perry_hir::Expr; + match expr { + Expr::String(_) + | Expr::Number(_) + | Expr::Integer(_) + | Expr::Bool(_) + | Expr::Null + | Expr::Undefined => true, + // The counter is the loop's own i32 induction variable. + Expr::LocalGet(id) => *id == counter_id, + // An element of the packed f64 array is a genuine double: the versioned + // loop only entered the fast clone after proving that layout. + Expr::IndexGet { object, index } => { + matches!(object.as_ref(), Expr::LocalGet(id) if *id == arr_id) + && expr_is_coercion_free_primitive(ctx, index, arr_id, counter_id) + } + Expr::Binary { left, right, .. } | Expr::Compare { left, right, .. } => { + expr_is_coercion_free_primitive(ctx, left, arr_id, counter_id) + && expr_is_coercion_free_primitive(ctx, right, arr_id, counter_id) + } + Expr::Unary { operand, .. } | Expr::NumberCoerce(operand) => { + expr_is_coercion_free_primitive(ctx, operand, arr_id, counter_id) + } + _ => false, + } +} + fn expr_is_packed_f64_loop_safe( ctx: &FnCtx<'_>, expr: &perry_hir::Expr, @@ -8203,6 +8311,28 @@ pub(crate) fn expr_preserves_array_length( && walk(object) && args.iter().all(&walk) } + // The Error-family construction nodes allocate an error object and + // store their own message; none of them can reach an unrelated local + // array, so they preserve `arr.length`. Together with the `Stmt::Throw` + // admission above this is what keeps `if (bad) throw new Error(…)` — + // the #9151 shape — on the packed fast path. + // + // Identity is not in question at this layer: HIR emits these nodes only + // for the INTRINSIC constructor (`lower_new` gates them on + // `!shadowed_by_user_binding`). A user `class Error { … }`, or any other + // shadowing binding, lowers to `Expr::New`/`NewDynamic` instead and is + // still rejected by the fallback below — checking the NAME here would be + // a wrong-answer bug rather than a missed optimisation. The message and + // options operands are arbitrary expressions and are still walked. + Expr::ErrorNew(message) => message.as_ref().is_none_or(|m| walk(m)), + Expr::TypeErrorNew(message) + | Expr::RangeErrorNew(message) + | Expr::ReferenceErrorNew(message) + | Expr::SyntaxErrorNew(message) => walk(message), + Expr::ErrorNewWithCause { message, cause } => walk(message) && walk(cause), + Expr::ErrorNewWithOptions { + message, options, .. + } => walk(message) && walk(options), Expr::NativeMethodCall { .. } | Expr::CallSpread { .. } => false, Expr::Closure { .. } => false, Expr::Binary { left, right, .. } diff --git a/crates/perry/tests/packed_loop_error_throw.rs b/crates/perry/tests/packed_loop_error_throw.rs new file mode 100644 index 0000000000..0bd09d54a2 --- /dev/null +++ b/crates/perry/tests/packed_loop_error_throw.rs @@ -0,0 +1,360 @@ +//! `throw new Error(…)` and other constructed operands inside a packed counted +//! loop (#9151, #9232). +//! +//! Admitting these depends on #9210's writeback, which is emitted at the throw +//! site — AFTER the operand is lowered. So an operand that can unwind ON ITS +//! OWN skips the flush and leaves the loop-carried accumulators stale, which is +//! #9185's defect one level deeper. Two things can unwind that way and neither +//! looks like a call at the syntax level: +//! +//! * `new Error(msg)` stringifies `msg`, dispatching to a user `toString`. +//! * `a + b` coerces, dispatching to a user `valueOf`/`toString`. +//! +//! Both are admitted only when the operand is provably coercion-free. The +//! rejection tests below are the load-bearing half of this file: each one +//! returned `c=0 s=0` against node's `c=40 s=780` while the predicate was +//! merely "is this loop-safe", and each is a silent wrong answer rather than a +//! missed optimisation. +//! +//! The `Error`-shadowing tests guard the other half. Accepting the Error nodes +//! is sound only because HIR emits them for the INTRINSIC constructor alone +//! (`lower_new` gates on `!shadowed_by_user_binding`); a user `class Error` +//! whose constructor shrinks the array would otherwise run against a hoisted +//! length. Each shadow case mutates the array MID-LOOP and reports the +//! iteration count, so a wrongly hoisted length reads `iters=10`, not `iters=3`. + +use std::path::PathBuf; +use std::process::{Command, Output}; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn compile_and_run(source: &str) -> String { + let dir = tempfile::tempdir().expect("tempdir"); + let entry = dir.path().join("main.ts"); + let output = dir.path().join("main_bin"); + std::fs::write(&entry, source).expect("write entry"); + + let compile = Command::new(perry_bin()) + .current_dir(dir.path()) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .env("PERRY_NO_CACHE", "1") + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + + let run: Output = Command::new(&output) + .current_dir(dir.path()) + .output() + .expect("run compiled binary"); + assert!( + run.status.success(), + "compiled binary failed\nstatus: {:?}\nstdout:\n{}\nstderr:\n{}", + run.status, + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + String::from_utf8_lossy(&run.stdout).trim().to_owned() +} + +const PRELUDE: &str = r#" +const arr: number[] = []; +for (let i = 0; i < 64; i++) arr.push(i); +"#; + +const SHADOW_BODY: &str = r#" +function f(): string { + let n = 0; let s = 0; + for (let i = 0; i < arr.length; i++) { + if (i === 1) { const e = new Error("shrink"); if (n < 0) throw e; } + n++; const v = arr[i]; s += (v === undefined ? 0 : v); + } + return "iters=" + n + " sum=" + s + " len=" + arr.length; +} +console.log(f()); +"#; + +// --- the operand must not be able to unwind before the flush ----------------- + +#[test] +fn an_error_message_whose_tostring_throws_keeps_the_accumulators() { + // Gave `c=0 s=0`: the toString unwound during Error construction, before + // the writeback at the throw site could run. + let out = compile_and_run(&format!( + "{PRELUDE} + const evil: any = {{ toString(): string {{ throw new Error(\"from toString\"); }} }}; + function f(): string {{ + let s = 0; let c = 0; + try {{ + for (let i = 0; i < arr.length; i++) {{ + if (arr[i] === 40) throw new Error(evil); + c++; s += arr[i]; + }} + }} catch (e: any) {{ return \"c=\" + c + \" s=\" + s + \" msg=\" + e.message; }} + return \"none\"; + }} + console.log(f()); + " + )); + assert_eq!(out, "c=40 s=780 msg=from toString"); +} + +#[test] +fn a_binary_operand_whose_valueof_throws_keeps_the_accumulators() { + // No Error construction at all — plain `+` dispatching to a user valueOf. + // Gave `c=0 s=0`. + let out = compile_and_run(&format!( + "{PRELUDE} + const evil: any = {{ valueOf(): number {{ throw new Error(\"from valueOf\"); }} }}; + const two: any = 2; + function f(): string {{ + let s = 0; let c = 0; + try {{ + for (let i = 0; i < arr.length; i++) {{ + if (arr[i] === 40) throw (evil + two); + c++; s += arr[i]; + }} + }} catch (e: any) {{ return \"c=\" + c + \" s=\" + s + \" msg=\" + (e.message || e); }} + return \"none\"; + }} + console.log(f()); + " + )); + assert_eq!(out, "c=40 s=780 msg=from valueOf"); +} + +#[test] +fn a_unary_operand_whose_valueof_throws_keeps_the_accumulators() { + let out = compile_and_run(&format!( + "{PRELUDE} + const evil: any = {{ valueOf(): number {{ throw new Error(\"unary valueOf\"); }} }}; + function f(): string {{ + let s = 0; let c = 0; + try {{ + for (let i = 0; i < arr.length; i++) {{ + if (arr[i] === 40) throw (-evil); + c++; s += arr[i]; + }} + }} catch (e: any) {{ return \"c=\" + c + \" s=\" + s + \" msg=\" + (e.message || e); }} + return \"none\"; + }} + console.log(f()); + " + )); + assert_eq!(out, "c=40 s=780 msg=unary valueOf"); +} + +#[test] +fn a_getter_that_throws_in_the_message_keeps_the_accumulators() { + let out = compile_and_run(&format!( + "{PRELUDE} + const holder: any = {{ get boom(): string {{ throw new Error(\"getter\"); }} }}; + function f(): string {{ + let s = 0; let c = 0; + try {{ + for (let i = 0; i < arr.length; i++) {{ + if (arr[i] === 40) throw new Error(holder.boom); + c++; s += arr[i]; + }} + }} catch (e: any) {{ return \"c=\" + c + \" s=\" + s + \" msg=\" + e.message; }} + return \"none\"; + }} + console.log(f()); + " + )); + assert_eq!(out, "c=40 s=780 msg=getter"); +} + +#[test] +fn throwing_an_object_as_is_never_coerces_it() { + // The counterpart to the rejections above: thrown AS-IS, nothing converts + // it, so this stays on the fast path and the toString must never run. + let out = compile_and_run(&format!( + "{PRELUDE} + const evil: any = {{ toString(): string {{ throw new Error(\"should not run\"); }} }}; + function f(): string {{ + let s = 0; let c = 0; + try {{ + for (let i = 0; i < arr.length; i++) {{ + if (arr[i] === 40) throw evil; + c++; s += arr[i]; + }} + }} catch (e: any) {{ return \"c=\" + c + \" s=\" + s + \" isObj=\" + (typeof e); }} + return \"none\"; + }} + console.log(f()); + " + )); + assert_eq!(out, "c=40 s=780 isObj=object"); +} + +// --- the shapes that must stay FAST ----------------------------------------- + +#[test] +fn an_intrinsic_error_throw_keeps_the_packed_clone() { + let out = compile_and_run(&format!( + r#"{PRELUDE} +function f(): number {{ + let s = 0; + for (let i = 0; i < arr.length; i++) {{ + if (arr[i] === 999) throw new Error("unreachable"); + s += arr[i]; + }} + return s; +}} +console.log(f()); +"# + )); + assert_eq!(out, "2016"); +} + +#[test] +fn an_intrinsic_error_throw_is_taken_at_the_right_index() { + let out = compile_and_run(&format!( + r#"{PRELUDE} +function f(): string {{ + let s = 0; + try {{ + for (let i = 0; i < arr.length; i++) {{ + if (arr[i] === 40) throw new Error("hit@" + i); + s += arr[i]; + }} + }} catch (e: any) {{ return "caught:" + e.message + " partial=" + s; }} + return "none:" + s; +}} +console.log(f()); +"# + )); + assert_eq!(out, "caught:hit@40 partial=780"); +} + +#[test] +fn an_array_element_in_the_message_is_coercion_free() { + // `arr[i]` is a genuine double by the clone's own guard, so stringifying it + // is builtin. + let out = compile_and_run(&format!( + r#"{PRELUDE} +function f(): string {{ + let s = 0; + try {{ + for (let i = 0; i < arr.length; i++) {{ + if (arr[i] === 40) throw new Error("v=" + arr[i]); + s += arr[i]; + }} + }} catch (e: any) {{ return "s=" + s + " msg=" + e.message; }} + return "none"; +}} +console.log(f()); +"# + )); + assert_eq!(out, "s=780 msg=v=40"); +} + +#[test] +fn native_error_subclasses_are_accepted_too() { + let out = compile_and_run(&format!( + r#"{PRELUDE} +function f(kind: number): number {{ + let s = 0; + for (let i = 0; i < arr.length; i++) {{ + if (arr[i] === 999) {{ + if (kind === 0) throw new TypeError("t"); + if (kind === 1) throw new RangeError("r"); + if (kind === 2) throw new SyntaxError("s"); + throw new ReferenceError("f"); + }} + s += arr[i]; + }} + return s; +}} +console.log(f(0) + "," + f(1) + "," + f(2) + "," + f(3)); +"# + )); + assert_eq!(out, "2016,2016,2016,2016"); +} + +#[test] +fn a_message_expression_that_shrinks_the_array_is_still_rejected() { + let out = compile_and_run( + r#" +const arr: number[] = [1,2,3,4,5,6,7,8,9,10]; +function shrink(): string { arr.length = 3; return "boom"; } +function f(): string { + let n = 0; let s = 0; + for (let i = 0; i < arr.length; i++) { + if (i === 1) { const e = new Error(shrink()); if (n < 0) throw e; } + n++; const v = arr[i]; s += (v === undefined ? 0 : v); + } + return "iters=" + n + " sum=" + s + " len=" + arr.length; +} +console.log(f()); +"#, + ); + assert_eq!(out, "iters=3 sum=6 len=3"); +} + +// --- `Error` must be the intrinsic, not a look-alike ------------------------ + +#[test] +fn a_user_class_named_error_is_not_the_intrinsic() { + let out = compile_and_run(&format!( + r#" +const arr: number[] = [1,2,3,4,5,6,7,8,9,10]; +class Error {{ constructor(_m: string) {{ arr.length = 3; }} }} +{SHADOW_BODY}"# + )); + assert_eq!(out, "iters=3 sum=6 len=3"); +} + +#[test] +fn a_const_function_named_error_is_not_the_intrinsic() { + let out = compile_and_run(&format!( + r#" +const arr: number[] = [1,2,3,4,5,6,7,8,9,10]; +const Error: any = function (this: any, _m: string) {{ arr.length = 3; }}; +{SHADOW_BODY}"# + )); + assert_eq!(out, "iters=3 sum=6 len=3"); +} + +#[test] +fn a_function_valued_error_binding_is_not_the_intrinsic() { + // The shadow's identity is not visible in the initializer at all. + let out = compile_and_run(&format!( + r#" +const arr: number[] = [1,2,3,4,5,6,7,8,9,10]; +function makeErr(): any {{ return function (this: any, _m: string) {{ arr.length = 3; }}; }} +const Error: any = makeErr(); +{SHADOW_BODY}"# + )); + assert_eq!(out, "iters=3 sum=6 len=3"); +} + +#[test] +fn a_block_scoped_class_named_error_is_not_the_intrinsic() { + let out = compile_and_run( + r#" +const arr: number[] = [1,2,3,4,5,6,7,8,9,10]; +function f(): string { + class Error { constructor(_m: string) { arr.length = 3; } } + let n = 0; let s = 0; + for (let i = 0; i < arr.length; i++) { + if (i === 1) { const e = new Error("shrink"); if (n < 0) throw e; } + n++; const v = arr[i]; s += (v === undefined ? 0 : v); + } + return "iters=" + n + " sum=" + s + " len=" + arr.length; +} +console.log(f()); +"#, + ); + assert_eq!(out, "iters=3 sum=6 len=3"); +} From e28d9f57a15763986f2fef11708cd9c6f6764160 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 31 Aug 2026 04:25:49 +0200 Subject: [PATCH 2/2] fix(runtime): unary minus performs ToNumeric on its operand MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit js_dynamic_neg negated the raw NaN-boxed bits after its BigInt arm, so an object operand never had valueOf called: -{ valueOf() { throw … } } answered NaN while +x, ~x and x * 2 on the same object all threw. Routes Neg's non-numeric lowering through the helper, mirroring Pos. Pre-existing; surfaced by this PR's own a_unary_operand_whose_valueof_throws test. --- crates/perry-codegen/src/expr/unary.rs | 17 +++++++++++------ crates/perry-codegen/src/stmt/loops.rs | 3 ++- crates/perry-runtime/src/value/dynamic_arith.rs | 7 +++++++ 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/crates/perry-codegen/src/expr/unary.rs b/crates/perry-codegen/src/expr/unary.rs index cc9f4b7440..78db047c08 100644 --- a/crates/perry-codegen/src/expr/unary.rs +++ b/crates/perry-codegen/src/expr/unary.rs @@ -32,7 +32,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // Everything else may be a BigInt at runtime — notably an indexed // read from a plain array — and routing it through ToNumber would // silently round large BigInts before `fneg` (#9142). - let dynamic_neg = matches!(op, UnaryOp::Neg) + let _dynamic_neg = matches!(op, UnaryOp::Neg) && !statically_numeric && !is_provably_not_bigint(ctx, operand); let (v, precomputed_truthy) = if matches!(op, UnaryOp::Not) { @@ -44,13 +44,18 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { let blk = ctx.block(); match op { UnaryOp::Neg => { - if dynamic_neg { - Ok(blk.call(DOUBLE, "js_dynamic_neg", &[(DOUBLE, &v)])) - } else if numeric { + if numeric { Ok(blk.fneg(&v)) } else { - let coerced = blk.call(DOUBLE, "js_number_coerce", &[(DOUBLE, &v)]); - Ok(blk.fneg(&coerced)) + // Mirrors `Pos` below: anything not statically numeric + // goes through the dynamic helper, which performs a real + // ToNumeric. The old `js_number_coerce` + `fneg` fallback + // answered NaN when the operand's `valueOf` THREW, so + // `-{ valueOf() { throw … } }` silently produced NaN + // where every other operator (`+x`, `~x`, `x * 2`) + // propagated. `js_dynamic_neg` also keeps a BigInt a + // BigInt, which is why `dynamic_neg` folded in here. + Ok(blk.call(DOUBLE, "js_dynamic_neg", &[(DOUBLE, &v)])) } } UnaryOp::Pos => { diff --git a/crates/perry-codegen/src/stmt/loops.rs b/crates/perry-codegen/src/stmt/loops.rs index a886bca8e2..4d20d9c912 100644 --- a/crates/perry-codegen/src/stmt/loops.rs +++ b/crates/perry-codegen/src/stmt/loops.rs @@ -5164,7 +5164,8 @@ fn stmt_is_packed_f64_loop_safe( // `flush_packed_accumulator_locals`; without it this admission is the // #9185 wrong answer. Stmt::Throw(value) => { - packed_loop_abrupt_enabled() && throw_operand_cannot_unwind(ctx, value, arr_id, counter_id) + packed_loop_abrupt_enabled() + && throw_operand_cannot_unwind(ctx, value, arr_id, counter_id) } Stmt::LabeledBreak(_) | Stmt::LabeledContinue(_) diff --git a/crates/perry-runtime/src/value/dynamic_arith.rs b/crates/perry-runtime/src/value/dynamic_arith.rs index cd85ad80ac..33e83afedd 100644 --- a/crates/perry-runtime/src/value/dynamic_arith.rs +++ b/crates/perry-runtime/src/value/dynamic_arith.rs @@ -714,6 +714,13 @@ pub unsafe extern "C" fn js_dynamic_mod(a: f64, b: f64) -> f64 { /// Dynamic negate: -BigInt if operand is BigInt, else -f64. #[no_mangle] pub unsafe extern "C" fn js_dynamic_neg(a: f64) -> f64 { + // Unary minus performs ToNumeric on its operand, exactly as `js_dynamic_pos` + // below does. Negating the raw NaN-boxed bits skipped that entirely, so an + // object operand never had `valueOf` called: `-{ valueOf() { throw … } }` + // answered NaN instead of propagating, while `+x`, `~x` and `x * 2` on the + // same object all threw. ToNumeric is a no-op for a number and returns the + // BigInt unchanged for a BigInt, so the two arms below are unaffected. + let a = to_numeric(a); let a_val = JSValue::from_bits(a.to_bits()); if a_val.is_bigint() { let scope = crate::gc::RuntimeHandleScope::new();