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
3 changes: 3 additions & 0 deletions changelog.d/7851-pshape-add-result-coercion.md
Original file line number Diff line number Diff line change
@@ -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.
18 changes: 10 additions & 8 deletions crates/perry-codegen/src/expr/binary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 12 additions & 11 deletions crates/perry-codegen/src/expr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
26 changes: 10 additions & 16 deletions crates/perry-codegen/src/stmt/let_stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
11 changes: 6 additions & 5 deletions crates/perry-codegen/src/type_analysis/pod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
85 changes: 66 additions & 19 deletions crates/perry-codegen/tests/native_proof_regressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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<Shape> 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<Shape> 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<Shape> 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<Shape> clone reached on raw-f64 guard FAILURE must \
ToNumber the possibly boxed `+` result before multiplying it:\n\
{pshape_ir}"
);
}
assert!(
Expand Down
48 changes: 48 additions & 0 deletions test-files/test_gap_7506_pshape_add_result_coercion.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// A typed-receiver method call has three paths: dynamic method lookup, a
// raw-f64 receiver clone, and a Ptr<Shape> 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<Shape> 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));