Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions crates/perry-codegen/src/expr/dynamic_add_tree_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,22 @@ fn any_local(id: u32, name: &str, init: Expr) -> Stmt {
}
}

fn erased_bigint_local(id: u32, name: &str, value: &str) -> Vec<Stmt> {
vec![
Stmt::Let {
id,
name: name.to_string(),
ty: Type::Any,
mutable: true,
init: Some(Expr::Undefined),
},
Stmt::Expr(Expr::LocalSet(
id,
Box::new(Expr::BigInt(value.to_string())),
)),
]
}

fn add(left: Expr, right: Expr) -> Expr {
Expr::Binary {
op: BinaryOp::Add,
Expand All @@ -28,6 +44,14 @@ fn add(left: Expr, right: Expr) -> Expr {
}
}

fn arithmetic(op: BinaryOp, left: Expr, right: Expr) -> Expr {
Expr::Binary {
op,
left: Box::new(left),
right: Box::new(right),
}
}

fn dynamic_locals() -> Vec<Stmt> {
vec![
any_local(A, "a", Expr::Undefined),
Expand Down Expand Up @@ -103,3 +127,32 @@ fn two_leaf_dynamic_add_takes_the_guard_too() {
"the cold arm must preserve exact dynamic `+` semantics:\n{ir}"
);
}

#[test]
fn dynamic_arithmetic_results_are_guarded_before_add() {
// #9143: for-of element bindings can be `Any` even when their runtime
// values are BigInts. The nested arithmetic helpers preserve BigInt, so
// their boxed results must not feed an unconditional native `fadd`.
let mut body = erased_bigint_local(A, "a", "123456789012345678901234567890");
body.extend(erased_bigint_local(B, "d", "1000000007"));
let quotient = arithmetic(BinaryOp::Div, Expr::LocalGet(A), Expr::LocalGet(B));
let product = arithmetic(BinaryOp::Mul, quotient, Expr::LocalGet(B));
let remainder = arithmetic(BinaryOp::Mod, Expr::LocalGet(A), Expr::LocalGet(B));
body.push(result(add(product, remainder)));
let ir = ir_for("dynamic_bigint_identity_add", body);

assert!(
ir.contains("\nguarded_add.numeric."),
"possibly-BigInt arithmetic results need a runtime number guard:\n{ir}"
);
assert!(
ir.contains("call double @js_dynamic_string_or_number_add("),
"the non-number arm must preserve BigInt addition:\n{ir}"
);
for helper in ["js_dynamic_div", "js_dynamic_mul", "js_dynamic_mod"] {
assert!(
ir.contains(&format!("call double @{helper}(")),
"the arithmetic subtree must retain {helper}:\n{ir}"
);
}
}
29 changes: 22 additions & 7 deletions crates/perry-codegen/src/type_analysis/numeric.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,24 @@ pub(crate) fn is_numeric_expr(ctx: &FnCtx<'_>, e: &Expr) -> bool {
left,
right,
} => is_numeric_expr(ctx, left) && is_numeric_expr(ctx, right),
Expr::Binary { op, .. } => !matches!(op, BinaryOp::Add),
// Non-`+` arithmetic is numeric only when it cannot successfully
// produce a BigInt. With two erased operands, the lowering uses a
// BigInt-aware dynamic helper whose result may be a NaN-boxed BigInt;
// claiming that value is a raw double lets an enclosing operation
// perform native floating-point arithmetic on the box. For example,
// #9143's `(a / d) * d + (a % d)` treated both subtrees as doubles and
// `fadd` propagated the left NaN payload, silently dropping `% d`.
//
// One provably non-BigInt operand is enough: a mixed BigInt operation
// throws, while every successful result is then a Number. `>>>` is
// always Number-producing (or throws on BigInt), regardless of its
// operand proofs.
Expr::Binary {
op: BinaryOp::UShr, ..
} => true,
Expr::Binary { left, right, .. } => {
is_provably_not_bigint(ctx, left) || is_provably_not_bigint(ctx, right)
}
// `x++`/`x--`/`++x`/`--x` evaluates to `ToNumeric(x) ± 1`, a Number —
// EXCEPT when `x` is a BigInt, where it stays a BigInt (`5n++` → `6n`).
// So this is NOT unconditionally numeric: mirror the `LocalGet` arm's
Expand Down Expand Up @@ -654,12 +671,10 @@ pub(crate) fn is_provably_not_bigint(ctx: &FnCtx<'_>, e: &Expr) -> bool {
return false;
}
// Handle arithmetic/bitwise/unary nodes STRUCTURALLY, before the
// `is_numeric_expr` shortcut below. `is_numeric_expr` blanket-treats every
// non-`Add` binary and every `-x`/`+x`/`~x` unary as numeric — fine for its
// own callers (they guard BigInt upstream), but it would over-approximate
// here: `anyA ^ anyB` could be `bigint ^ bigint` (a BigInt result), yet
// `is_bigint_expr` can't see it when both operands are `Any`. The
// structural rules recurse into the operands instead.
// `is_numeric_expr` shortcut below. This avoids a predicate cycle and
// directly answers the relevant ToNumeric question: `anyA ^ anyB` could
// be `bigint ^ bigint` (a BigInt result), even though `is_bigint_expr`
// cannot see it when both operands are `Any`.
match e {
// Every arithmetic / bitwise binary op yields a BigInt ONLY when BOTH
// operands `ToNumeric` to BigInt (a mixed operand throws; a string
Expand Down
16 changes: 16 additions & 0 deletions test-files/test_gap_9143_bigint_for_of_compound.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Regression for #9143: BigInt operands bound by for-of must stay tagged
// through nested arithmetic. The add previously treated the left subtree as
// a Number and silently discarded the remainder term.

for (const a of [123456789012345678901234567890n]) {
for (const d of [1000000007n]) {
console.log("identity", (a / d) * d + (a % d));
console.log("quotient", a / d);
console.log("remainder", a % d);
console.log("matches", (a / d) * d + (a % d) === a);
}
}

const plainA = 123456789012345678901234567890n;
const plainD = 1000000007n;
console.log("plain", (plainA / plainD) * plainD + (plainA % plainD));
Loading