diff --git a/changelog.d/7851-pshape-add-result-coercion.md b/changelog.d/7851-pshape-add-result-coercion.md new file mode 100644 index 0000000000..f1f44e0cf9 --- /dev/null +++ b/changelog.d/7851-pshape-add-result-coercion.md @@ -0,0 +1,3 @@ +### Fixed + +- Preserve JavaScript coercion when a declared-number `+` result feeds later arithmetic in typed-receiver `$pshape` fallbacks (#7506). Numeric locals now inherit declared-only proof from their initializer even when the HIR already typed them as `Number`, and compound operands are coerced before non-`+` arithmetic. Proven raw-f64 clones remain coercion-free. diff --git a/crates/perry-codegen/src/expr/binary.rs b/crates/perry-codegen/src/expr/binary.rs index e388ef8e7e..85af0f640e 100644 --- a/crates/perry-codegen/src/expr/binary.rs +++ b/crates/perry-codegen/src/expr/binary.rs @@ -327,16 +327,18 @@ fn operand_needs_residual_coerce(ctx: &FnCtx<'_>, expr: &Expr, fallback_coerced: !fallback_coerced && (!is_numeric_expr(ctx, expr) || expr_may_return_boxed_value_from_raw_f64_fallback(ctx, expr) - // #7773: a local REFINED to `Number` from a declared field/element - // type is `is_numeric_expr`, but the hazard predicate above only - // knows how to look at reads, so `const v = o.x; v * 2` emitted a - // bare `fmul`. Arithmetic on a NaN-box preserves the payload, so - // that multiply returned the string unchanged — `typeof (v * 2)` - // answered `"string"`. Every non-`+` arithmetic operator is a plain + // #7773/#7506: a numeric local or compound expression initialized + // from a declared-only field/element expression is + // `is_numeric_expr`, but the hazard predicate above only knows how + // to look at reads. That made both `const sum = o.x + o.y; sum * + // scale` and `(o.x + o.y) * scale` emit a bare `fmul`. Arithmetic + // on a NaN-box preserves the payload, so the multiply returned the + // string unchanged. Every non-`+` arithmetic operator is a plain // `ToNumber` on its operands, so a coerce is the whole fix here; // `+` needs the concat dispatch and gets it from - // `lower_declared_only_numeric_add`. - || matches!(expr, Expr::LocalGet(_)) && numeric_proof_is_declared_only(ctx, expr)) + // `lower_declared_only_numeric_add`. Proven raw-f64 tiers answer + // false here, so asking about every expression keeps them exempt. + || numeric_proof_is_declared_only(ctx, expr)) } /// Lower an operand in number context: route through diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index dfd9306052..c2a02169af 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -747,18 +747,19 @@ pub(crate) struct FnCtx<'a> { /// protected temporary this function lowers. pub temp_roots: crate::rooting::TempRootPool, - /// #7773: LocalIds whose `Number`/`Int32` type was REFINED from a read - /// whose own numeric answer is only a declared type — `const v = o.x` on a - /// `x: number` field, or `const e = arr[i]` on a `number[]`. + /// #7773/#7506: LocalIds whose `Number`/`Int32` value came from an + /// initializer whose numeric answer is only a declared type — `const v = + /// o.x` on an `x: number` field, or `const sum = o.x + o.y`. This includes + /// both `Any` locals refined by codegen and locals the HIR already typed as + /// numeric. /// - /// The refinement is load-bearing (an un-annotated `const` is `Any` in the - /// HIR, so without it every ordinary field read loses the numeric fast - /// path), but it copies an annotation rather than proving anything. The - /// local then reads as `is_numeric_expr`, which licenses a bare `fadd` / - /// `fmul` on whatever the slot holds — and arithmetic on a NaN-boxed value - /// PRESERVES ITS PAYLOAD, so a string laundered in through `as any` came - /// back out of a multiply still tagged as a string (`typeof (v * 2)` was - /// `"string"`). + /// The `Any` refinement remains load-bearing (without it every ordinary + /// field read loses the numeric fast path), but both it and an HIR numeric + /// type can copy a declared field type rather than prove a runtime value. + /// The local then reads as `is_numeric_expr`, which licenses a bare `fadd` + /// / `fmul` on whatever the slot holds — and arithmetic on a NaN-boxed + /// value PRESERVES ITS PAYLOAD, so a string laundered in through `as any` + /// came back out of a multiply still tagged as a string. /// /// Consumed by `type_analysis::numeric_proof_is_declared_only`, which turns /// the trust into a four-instruction runtime tag test instead. diff --git a/crates/perry-codegen/src/stmt/let_stmt.rs b/crates/perry-codegen/src/stmt/let_stmt.rs index 4e23d1b3d4..6854f1f228 100644 --- a/crates/perry-codegen/src/stmt/let_stmt.rs +++ b/crates/perry-codegen/src/stmt/let_stmt.rs @@ -290,22 +290,16 @@ pub(crate) fn lower_let( ty.clone() }; - // #7773: the refinement above copies a DECLARED type — `const v = o.x` on a - // `x: number` field answers `Number` because the annotation says so, not - // because anything proved it. Nothing enforces annotations at runtime, so - // record the local as violable; `numeric_proof_is_declared_only` then makes - // arithmetic on it re-check the tag instead of trusting the type outright. - // - // Only the Any → numeric direction matters. A local the user DECLARED - // `number` is equally unenforced, but it is also the shape every honest - // program is made of; the refined case is the one where codegen invented - // the numeric claim itself, and it is the one both reported shapes need. - if matches!(ty, perry_hir::types::Type::Any) - && matches!( - refined_ty, - perry_hir::types::Type::Number | perry_hir::types::Type::Int32 - ) - { + // #7773/#7506: a numeric local inherits a DECLARED-ONLY proof from its + // initializer. `const v = o.x` reaches this as `Any` refined to `Number`, + // while TypeScript's inferred `const sum = o.x + o.y` already reaches the + // HIR as `Number`; neither form proves what the runtime slots contain. + // Record both as violable so a later arithmetic consumer re-checks the + // local instead of laundering a possibly boxed value through its type. + if matches!( + refined_ty, + perry_hir::types::Type::Number | perry_hir::types::Type::Int32 + ) { if init.is_some_and(|e| crate::type_analysis::numeric_proof_is_declared_only(ctx, e)) { ctx.declared_only_numeric_locals.insert(id); } diff --git a/crates/perry-codegen/src/type_analysis/pod.rs b/crates/perry-codegen/src/type_analysis/pod.rs index 4bdd9110bc..7b7f6a3947 100644 --- a/crates/perry-codegen/src/type_analysis/pod.rs +++ b/crates/perry-codegen/src/type_analysis/pod.rs @@ -547,11 +547,12 @@ pub(crate) fn numeric_proof_is_declared_only(ctx: &FnCtx<'_>, expr: &Expr) -> bo .as_deref() .is_some_and(is_numeric_typed_array_class) } - // A local whose `Number` type was REFINED from one of the reads above - // inherits its violability — the refinement copied a declared type, it - // did not prove anything (#7773 second shape: `const v: any = o.x` - // typed `v` numeric, so `v * 2` became a bare `fmul` on whatever the - // slot held and a NaN-boxed string passed straight through it). + // A numeric local initialized from one of the expressions above + // inherits its violability — its HIR type did not prove anything + // (#7773's `const v: any = o.x`, and #7506's already-number-typed + // `const sum = o.x + o.y`). Without this bit, a later `v * 2` or + // `sum * scale` becomes a bare `fmul` on whatever boxed value the slot + // holds, and a NaN-boxed string passes straight through it. Expr::LocalGet(id) => ctx.declared_only_numeric_locals.contains(id), // `a + b` is numeric only when both sides are, so it is violable when // either side is; `a || b` / `a && b` / `a ?? b` pass one operand diff --git a/crates/perry-codegen/tests/native_proof_regressions.rs b/crates/perry-codegen/tests/native_proof_regressions.rs index a7f22ee600..cd6a489291 100644 --- a/crates/perry-codegen/tests/native_proof_regressions.rs +++ b/crates/perry-codegen/tests/native_proof_regressions.rs @@ -11710,7 +11710,9 @@ fn typed_f64_receiver_method_clone_raw_loads_after_composed_guards() { && typed_ir.contains("getelementptr i8, ptr") && typed_ir.matches("load double").count() >= 2 && typed_ir.contains(" fadd ") - && typed_ir.contains(" fmul "), + && typed_ir.contains(" fmul ") + && !typed_ir.contains("js_number_coerce") + && !typed_ir.contains("js_dynamic_string_or_number_add"), "typed receiver clone should raw-load receiver fields and stay in f64 SSA:\n{typed_ir}" ); let method_guard = caller_ir @@ -11741,34 +11743,79 @@ fn typed_f64_receiver_method_clone_raw_loads_after_composed_guards() { // offsets but must NOT assume the slots hold canonical raw f64 // // What makes (3) sound is not which symbol it is, it is that the callee - // coerces what it loads. `$pshape` does: it emits `inttoptr` + - // `getelementptr` + `load double` — the shape guarantees the OFFSETS — and - // then routes every loaded slot through `js_number_coerce`, which is - // exactly the right handling for a slot that may hold a NaN-boxed value. - // `$generic` is also acceptable here; what must never appear on this edge - // is `$typed_f64_recv`, whose whole premise is the guard that just failed. + // preserves coercion semantics after loading. `$pshape` may use + // `inttoptr` + `getelementptr` + `load double` because the shape guarantees + // the OFFSETS, but it must tag-dispatch the `+` and then ToNumber that + // possibly boxed result before the following multiply. `$generic` is also + // acceptable here; what must never appear on this edge is + // `$typed_f64_recv`, whose whole premise is the guard that just failed. + let field_guard_branch = caller_ir[field_guard..] + .lines() + .find(|line| line.trim_start().starts_with("br i1 ")) + .unwrap_or_else(|| panic!("field guards should feed a conditional branch:\n{caller_ir}")); + let mut field_guard_successors = field_guard_branch + .split("label %") + .skip(1) + .map(|part| part.split([',', ' ']).next().unwrap()); + let success_label = field_guard_successors.next().unwrap_or_else(|| { + panic!("field-guard branch should have a success edge: {field_guard_branch}") + }); + let failure_label = field_guard_successors.next().unwrap_or_else(|| { + panic!("field-guard branch should have a failure edge: {field_guard_branch}") + }); + let basic_block = |label: &str| { + let marker = format!("\n{label}:\n"); + let start = caller_ir + .find(&marker) + .unwrap_or_else(|| panic!("basic block `{label}` not found:\n{caller_ir}")) + + marker.len(); + let rest = &caller_ir[start..]; + &rest[..rest.find("\n\n").unwrap_or(rest.len())] + }; + let success_block = basic_block(success_label); + let failure_block = basic_block(failure_label); + assert!( + success_block.contains(&format!("call double @{typed}(i64 ")), + "the raw-f64 field-guard success edge must call the raw-f64 receiver clone:\n\ + {success_block}" + ); + assert!( + !failure_block.contains(&format!("call double @{typed}(")), + "the raw-f64 field-guard failure edge must not call the raw-f64 receiver clone:\n\ + {failure_block}" + ); let failure_edge_callee = [generic_body, pshape_body] .into_iter() - .find(|sym| caller_ir.contains(&format!("call double @{sym}("))) + .find(|sym| failure_block.contains(&format!("call double @{sym}("))) .unwrap_or_else(|| { panic!( "raw-f64 field guard failure must reach a clone that does not \ - assume raw-f64 slots (`$generic` or `$pshape`):\n{caller_ir}" + assume raw-f64 slots (`$generic` or `$pshape`):\n{failure_block}" ) }); - assert!( - !caller_ir.contains(&format!("call double @{typed}(double ")), - "the guard-failure edge must not reach the raw-f64 receiver clone \ - (that clone is only valid when the guard PASSED):\n{caller_ir}" - ); if failure_edge_callee == pshape_body { let pshape_ir = defined_function_ir_section(&ir, pshape_body); + let dynamic_add = pshape_ir + .find("call double @js_dynamic_string_or_number_add(") + .unwrap_or_else(|| { + panic!("the Ptr clone must tag-dispatch declared-only `+`:\n{pshape_ir}") + }); + let result_coerce = pshape_ir + .find("call double @js_number_coerce(") + .unwrap_or_else(|| { + panic!( + "the Ptr clone must ToNumber the possibly boxed `+` result:\n\ + {pshape_ir}" + ) + }); + let multiply = pshape_ir + .find(" fmul ") + .unwrap_or_else(|| panic!("expected score's multiply in `$pshape`:\n{pshape_ir}")); assert!( - pshape_ir.contains("call double @js_number_coerce("), - "the Ptr clone reached on raw-f64 guard FAILURE must coerce \ - every slot it loads — without that it is the typed clone under \ - another name, and a receiver whose fields are not raw f64 would \ - take raw loads anyway:\n{pshape_ir}" + dynamic_add < result_coerce && result_coerce < multiply, + "the Ptr clone reached on raw-f64 guard FAILURE must \ + ToNumber the possibly boxed `+` result before multiplying it:\n\ + {pshape_ir}" ); } assert!( diff --git a/test-files/test_gap_7506_pshape_add_result_coercion.ts b/test-files/test_gap_7506_pshape_add_result_coercion.ts new file mode 100644 index 0000000000..64f8fee433 --- /dev/null +++ b/test-files/test_gap_7506_pshape_add_result_coercion.ts @@ -0,0 +1,48 @@ +// A typed-receiver method call has three paths: dynamic method lookup, a +// raw-f64 receiver clone, and a Ptr fallback when the method identity is +// stable but a numeric field no longer holds a raw double. The fallback may +// trust field offsets, but it must preserve JavaScript coercion semantics. + +class Point7506 { + x: number; + y: number; + + constructor(x: number, y: number) { + this.x = x; + this.y = y; + } + + score(scale: number): number { + const sum = this.x + this.y; + return sum * scale; + } + + scoreInline(scale: number): number { + return (this.x + this.y) * scale; + } +} + +function probe7506(receiver: Point7506, scale: number): number { + return receiver.score(scale); +} + +function probeInline7506(receiver: Point7506, scale: number): number { + return receiver.scoreInline(scale); +} + +function poison7506(receiver: Point7506): void { + (receiver as any).x = "1"; +} + +// Recursion keeps the receiver as a real heap parameter. Without it the whole +// top-level scenario is inlined and scalar-replaced, so it never exercises the +// typed-receiver guard and its Ptr fallback. +function run7506(receiver: Point7506, remaining: number): string { + if (remaining > 0) return run7506(receiver, remaining - 1); + poison7506(receiver); + const result = probe7506(receiver, 3); + const inline = probeInline7506(receiver, 3); + return `${result} ${typeof result} ${inline} ${typeof inline}`; +} + +console.log(run7506(new Point7506(1, 2), 1));