fix(codegen): a declared numeric type is not a proof that the value is a number

Perry does not enforce annotations at runtime (CLAUDE.md, Known
Limitations), but codegen answered `is_numeric_expr` = true on the
strength of one and then emitted bare f64 arithmetic on whatever the slot
actually held.

That is worse than a NaN, because arithmetic on a NaN-BOXED value is not a
no-op that yields NaN: `fadd`/`fmul` propagate the input NaN's payload, so
a NaN-boxed string comes back out of the instruction STILL TAGGED AS THAT
STRING. `typeof (v * 2)` answered "string", and `v + 1` looked as though
the `+ 1` had evaporated.

Three divergences from Node, all silent:

  #7773 shape 1  `o.x + 1` gave NaN (the number-context read's cold arm
                 coerces unconditionally). Node concatenates: `s1`.
  #7773 shape 2  through a refined local (`const v = o.x`) there was no
                 coerce at all, so the string passed straight through.
  #7776          a heterogeneous element stored via `as any`, then summed.

New predicate `numeric_proof_is_declared_only` separates "an annotation
said so" from a real proof. It is deliberately narrower than
`expr_may_return_boxed_value_from_raw_f64_fallback`, which answers "is
there a raw-f64 tier worth trying" and stays true for reads that end up
with no boxed fallback: every arm carrying a guard, a closed store
universe or scalar replacement answers false, so element-shape and
class-field loop facts, `Ptr<Shape>` numeric fields, POD records, scalar
replacement and typed arrays all keep their bare loads.

Two consumers:

* `+` with a declared-only operand lowers through
  `lower_declared_only_numeric_add`: an inline NaN-box tag test, `fadd` on
  the fast arm, `js_dynamic_string_or_number_add` on the cold one. The
  spec's `+` dispatches on the runtime value, so this is the operator that
  needs the dispatch rather than a coerce.
* every other arithmetic operator is a plain ToNumber, so the existing
  residual `js_number_coerce` rule is enough — it just could not see a
  refined LOCAL before.

`expr/mod.rs::lower_numeric_binary_value` is a second arithmetic tier that
bypasses `binary::lower` entirely and emits bare `fadd`/`fmul` with no
residual coerce at all; it is the path both refined-local shapes took, and
it now hands declared-only operands down to `binary::lower` the same way
its two existing Mod cases do.

Two things the first attempt got wrong, both now pinned by the test:

* ONE diamond per `+` TREE, not one per node. Per-node diamonds make the
  outer add of `s += o.x + 1` consume a phi, and LLVM cannot prove a phi
  over (`fadd`, runtime call) is a canonical double — the outer test never
  folded and the hot loop lost its `fadd` to an unconditional call. Fusing
  took that shape from +38% to +8.6%. Both arms rebuild the ORIGINAL tree
  shape, because `+` is not associative across strings: `1 + (2 + "x")` is
  `"12x"` and `(1 + 2) + "x"` is `"3x"`.
* every leaf is tested except those `expr_produces_canonical_raw_f64`
  vouches for. Testing only the declared-only leaves skips the
  ACCUMULATOR, and `let s = 0; s += r.x + r.y` holds a string the moment
  this lowering's own cold arm concatenates — that summed
  `16zw1113151719` down to `16zw`, the original bug one level up.

Measured on the quiet M1 mini (load 1.68, 7 alternating runs, same runtime
for both arms so only codegen differs):

  element-shape clone   218 -> 217 ms   -0.5%   (untouched, as intended)
  this.v + 1 in method    70 ->  76 ms   +8.6%
  s += p.x + p.y          196 -> 263 ms  +34.2%

The cost falls only on reads the compiler could prove nothing about, which
already pay an inline header precheck or a
`js_typed_feedback_class_field_get_guard` call for their shape check. It
is a real cost and the alternative is silently wrong arithmetic.

test-files/test_gap_declared_numeric_field_holds_string_7773.ts covers both
reported shapes plus array elements, inherited fields, chained adds and the
accumulator, and asserts the other direction for VALUE — honest arithmetic,
an honest guard failure and a typed array must all still answer as numbers.

Author: Ralph Küpper

crates/perry-codegen/src/codegen/closure.rs

@@ -978,6 +978,7 @@ pub(super) fn compile_closure(
         temp_roots: crate::rooting::TempRootPool::default(),
         shadow_slot_map,
         persistent_shadow_slots: std::collections::HashSet::new(),
+        declared_only_numeric_locals: std::collections::HashSet::new(),
         shadow_slot_clears_after_stmt,
         arena_state_slot: None,
         class_keys_slots: HashMap::new(),

crates/perry-codegen/src/codegen/entry.rs

@@ -816,6 +816,7 @@ pub(super) fn compile_module_entry(
             temp_roots: crate::rooting::TempRootPool::default(),
             shadow_slot_map: main_shadow_slot_map,
             persistent_shadow_slots: std::collections::HashSet::new(),
+            declared_only_numeric_locals: std::collections::HashSet::new(),
             shadow_slot_clears_after_stmt: main_shadow_slot_clears_after_stmt,
             arena_state_slot: None,
             class_keys_slots: HashMap::new(),
@@ -1485,6 +1486,7 @@ pub(super) fn compile_module_entry(
             temp_roots: crate::rooting::TempRootPool::default(),
             shadow_slot_map: init_shadow_slot_map,
             persistent_shadow_slots: std::collections::HashSet::new(),
+            declared_only_numeric_locals: std::collections::HashSet::new(),
             shadow_slot_clears_after_stmt: init_shadow_slot_clears_after_stmt,
             arena_state_slot: None,
             class_keys_slots: HashMap::new(),

crates/perry-codegen/src/codegen/function.rs

@@ -770,6 +770,7 @@ pub(super) fn compile_function(
         unsigned_i32_locals: native_facts.unsigned_i32_locals(),
         shadow_slot_map,
         persistent_shadow_slots: std::collections::HashSet::new(),
+        declared_only_numeric_locals: std::collections::HashSet::new(),
         shadow_slot_clears_after_stmt,
         shadow_slots_bound: bound_param_slots,
         temp_roots: crate::rooting::TempRootPool::default(),

crates/perry-codegen/src/codegen/method.rs

@@ -509,6 +509,7 @@ pub(super) fn compile_method(
         temp_roots: crate::rooting::TempRootPool::default(),
         shadow_slot_map,
         persistent_shadow_slots: std::collections::HashSet::new(),
+        declared_only_numeric_locals: std::collections::HashSet::new(),
         shadow_slot_clears_after_stmt,
         arena_state_slot: None,
         class_keys_slots: HashMap::new(),
@@ -1572,6 +1573,7 @@ pub(super) fn compile_static_method(
         temp_roots: crate::rooting::TempRootPool::default(),
         shadow_slot_map,
         persistent_shadow_slots: std::collections::HashSet::new(),
+        declared_only_numeric_locals: std::collections::HashSet::new(),
         shadow_slot_clears_after_stmt,
         arena_state_slot: None,
         class_keys_slots: HashMap::new(),

crates/perry-codegen/src/expr/binary.rs

@@ -18,7 +18,7 @@ use crate::native_value::{
 use crate::type_analysis::{
     add_operands_have_pod_materialization_hazard,
     expr_may_return_boxed_value_from_raw_f64_fallback, is_bigint_expr, is_bool_expr,
-    is_numeric_expr,
+    is_numeric_expr, numeric_proof_is_declared_only,
 };
 use crate::types::{DOUBLE, I1, I128, I32, I64};
 
@@ -51,6 +51,166 @@ fn lower_rooted_dynamic_binary(
     })
 }
 
+/// `+` where both operands are statically numeric but at least one of them is
+/// numeric only because a DECLARED type said so (#7773, #7776).
+///
+/// Nothing enforces annotations at runtime, so a `x: number` slot reached
+/// through `as any` really can hold a string — and then the spec says `+` is
+/// string concatenation, which is what Node does. Trusting the annotation cost
+/// two different wrong answers, both silent:
+///
+/// * `o.x + 1` produced `NaN`, because the number-context read's cold arm
+///   `js_number_coerce`s unconditionally; Node prints `s1`.
+/// * through a refined local the add did not even coerce — `fadd` on a
+///   NaN-BOXED value propagates the input payload on both AArch64 and x86-64,
+///   so the string came back out of the add still a string, and the `+ 1`
+///   looked like it had evaporated.
+///
+/// So re-check at runtime instead of assuming. The fast arm keeps the inline
+/// `fadd`; only a value that is not a canonical double reaches the dynamic
+/// helper, which is the one that implements the spec's `+`.
+///
+/// **The whole `+` TREE becomes one diamond, not one per node.** That is a
+/// correctness-neutral but performance-critical detail, and doing it the
+/// obvious way first is what showed why. Per-node diamonds make the outer add
+/// of `s += o.x + 1` consume a PHI, and LLVM cannot prove a phi over
+/// (`fadd`, runtime call) is a canonical double — so the outer test never
+/// folded, its cold arm stayed live in the loop, and `Acc.run`'s hot loop lost
+/// its `fadd` to an unconditional call. Measured on the bench mini that shape
+/// went 86 ms -> 119 ms. Fusing the tree removes the phi entirely: one test
+/// over the tree's violable LEAVES, one branch, then either all-`fadd` or
+/// all-`js_dynamic_string_or_number_add`.
+///
+/// Associativity is preserved rather than assumed away: both arms rebuild the
+/// ORIGINAL tree shape. `1 + (2 + "x")` is `"12x"` and `(1 + 2) + "x"` is
+/// `"3x"`, so a flattened re-association would be a wrong answer — the leaves
+/// are collected in evaluation order for rooting, but the arms are rebuilt
+/// node-for-node.
+///
+/// Every leaf is tested EXCEPT those that `expr_produces_canonical_raw_f64`
+/// vouches for (literals, `Math.*`, an explicit coerce, non-`+` arithmetic).
+/// Testing only the declared-only leaves is not enough, and the accumulator is
+/// the counter-example: `let s = 0; s += r.x + r.y` types `s` as `Number`, but
+/// the moment this very lowering's cold arm concatenates, `s` HOLDS A STRING
+/// while its static type still says otherwise. Skipping it summed
+/// `16zw1113151719` down to `16zw` — the fast arm `fadd`ed a NaN-boxed string
+/// and passed it through unchanged, which is the original bug reintroduced one
+/// level up. `expr_produces_canonical_raw_f64` declines to vouch for a
+/// `LocalGet` precisely because a local is a slot somebody can store into.
+///
+/// The residual cost lands where it is already small: every read that reaches
+/// here is one the compiler could NOT prove, so it pays an inline header
+/// precheck or a `js_typed_feedback_class_field_get_guard` call for its shape
+/// check regardless. The proven tiers (element-shape / class-field loop facts,
+/// `Ptr<Shape>` numeric fields, scalar replacement, POD records, typed arrays)
+/// never get here at all — `numeric_proof_is_declared_only` answers `false`.
+fn lower_declared_only_numeric_add(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
+    let mut leaves = Vec::new();
+    add_tree_leaves(expr, &mut leaves);
+    let needs_test: Vec<bool> = leaves
+        .iter()
+        .map(|leaf| !crate::type_analysis::expr_produces_canonical_raw_f64(ctx, leaf))
+        .collect();
+
+    with_operands_rooted(ctx, &leaves, |ctx, values| {
+        let mut cond: Option<String> = None;
+        for (value, is_tested) in values.iter().zip(needs_test.iter()) {
+            if !is_tested {
+                continue;
+            }
+            let is_num = crate::stmt::emit_js_value_is_number(ctx, value);
+            cond = Some(match cond {
+                Some(prev) => ctx.block().and(I1, &prev, &is_num),
+                None => is_num,
+            });
+        }
+        // The caller only routes here when a leaf is declared-only, and every
+        // such leaf is a field / element / local read — none of which
+        // `expr_produces_canonical_raw_f64` vouches for. So there is always at
+        // least one test; an empty condition would mean the two predicates had
+        // drifted apart, which is worth a hard error rather than a silent
+        // unguarded `fadd`.
+        let Some(all_num) = cond else {
+            anyhow::bail!(
+                "declared-only `+` tree has no testable leaf: \
+                 numeric_proof_is_declared_only and expr_produces_canonical_raw_f64 disagree"
+            );
+        };
+
+        let fast_idx = ctx.new_block("declared_add.numeric");
+        let slow_idx = ctx.new_block("declared_add.dynamic");
+        let merge_idx = ctx.new_block("declared_add.merge");
+        let fast_label = ctx.block_label(fast_idx);
+        let slow_label = ctx.block_label(slow_idx);
+        let merge_label = ctx.block_label(merge_idx);
+        ctx.block().cond_br(&all_num, &fast_label, &slow_label);
+
+        ctx.current_block = fast_idx;
+        let fast_val = rebuild_add_tree(ctx, expr, values, &mut 0, true);
+        let fast_end = ctx.block().label.clone();
+        ctx.block().br(&merge_label);
+
+        ctx.current_block = slow_idx;
+        let slow_val = rebuild_add_tree(ctx, expr, values, &mut 0, false);
+        let slow_end = ctx.block().label.clone();
+        ctx.block().br(&merge_label);
+
+        ctx.current_block = merge_idx;
+        Ok(ctx
+            .block()
+            .phi(DOUBLE, &[(&fast_val, &fast_end), (&slow_val, &slow_end)]))
+    })
+}
+
+/// The `+` tree's operand leaves, in evaluation order — a left-to-right walk,
+/// so `with_operands_rooted` lowers them in the order JS evaluates them.
+fn add_tree_leaves<'a>(expr: &'a Expr, out: &mut Vec<&'a Expr>) {
+    if let Expr::Binary {
+        op: BinaryOp::Add,
+        left,
+        right,
+    } = expr
+    {
+        add_tree_leaves(left, out);
+        add_tree_leaves(right, out);
+    } else {
+        out.push(expr);
+    }
+}
+
+/// Rebuild the `+` tree over already-lowered leaf values, node for node, so the
+/// original associativity survives. `fast` picks the inline `fadd`; otherwise
+/// every node goes through the spec-`+` helper.
+fn rebuild_add_tree(
+    ctx: &mut FnCtx<'_>,
+    expr: &Expr,
+    values: &[String],
+    next_leaf: &mut usize,
+    fast: bool,
+) -> String {
+    if let Expr::Binary {
+        op: BinaryOp::Add,
+        left,
+        right,
+    } = expr
+    {
+        let l = rebuild_add_tree(ctx, left, values, next_leaf, fast);
+        let r = rebuild_add_tree(ctx, right, values, next_leaf, fast);
+        return if fast {
+            ctx.block().fadd(&l, &r)
+        } else {
+            ctx.block().call(
+                DOUBLE,
+                "js_dynamic_string_or_number_add",
+                &[(DOUBLE, &l), (DOUBLE, &r)],
+            )
+        };
+    }
+    let value = values[*next_leaf].clone();
+    *next_leaf += 1;
+    value
+}
+
 fn lower_arithmetic_operand(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<(String, bool)> {
     // #6884: a statically typed numeric TypedArray read is Number|undefined,
     // not an unconditional raw f64. In arithmetic context the OOB `undefined`
@@ -145,7 +305,17 @@ fn lower_arithmetic_operand(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<(String,
 fn operand_needs_residual_coerce(ctx: &FnCtx<'_>, expr: &Expr, fallback_coerced: bool) -> bool {
     !fallback_coerced
         && (!is_numeric_expr(ctx, expr)
-            || expr_may_return_boxed_value_from_raw_f64_fallback(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
+            // `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 an operand in number context: route through
@@ -519,6 +689,15 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
                         right,
                     );
                 }
+                // Both sides are statically numeric — but "statically" can mean
+                // "an annotation said so", and annotations are not enforced
+                // (#7773, #7776). Re-check the tag at runtime rather than
+                // emitting a bare `fadd` on a value that may be NaN-boxed.
+                if numeric_proof_is_declared_only(ctx, left)
+                    || numeric_proof_is_declared_only(ctx, right)
+                {
+                    return lower_declared_only_numeric_add(ctx, expr);
+                }
             }
             // BigInt arithmetic fast path. NaN-tagged bigints compare
             // unordered under `fadd`/`fsub`/`fmul`/`fdiv`/`frem` (the

crates/perry-codegen/src/expr/mod.rs

@@ -744,6 +744,23 @@ 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[]`.
+    ///
+    /// 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"`).
+    ///
+    /// Consumed by `type_analysis::numeric_proof_is_declared_only`, which turns
+    /// the trust into a four-instruction runtime tag test instead.
+    pub declared_only_numeric_locals: std::collections::HashSet<u32>,
+
     /// Cached pointer to this function's `InlineArenaState` slot —
     /// allocated lazily on the first `new ClassName()` site that uses
     /// the inline bump-allocator path. The slot lives in the function
@@ -2393,6 +2410,26 @@ fn lower_numeric_binary_value(
         return Ok(None);
     }
 
+    // #7773: `is_numeric_expr` answering `true` is not always a PROOF — for a
+    // class-field read, an array element, or a local refined from one, it is
+    // just the declared type repeated back, and nothing enforces declared types
+    // at runtime. This tier emits a bare `fadd`/`fmul` with no residual coerce
+    // at all, and arithmetic on a NaN-BOXED value propagates the payload
+    // instead of producing NaN — so a string laundered into a `x: number` slot
+    // came back out of `v * 2` still a string (`typeof` said `"string"`).
+    //
+    // Hand those to `binary::lower`, which has both remedies: the runtime tag
+    // test that keeps `+` on the spec's string-concat dispatch, and the
+    // residual `js_number_coerce` that gives every other operator its
+    // `ToNumber`. Same hand-off shape as the two Mod cases below, and for the
+    // same reason — it must run before operand lowering so an `Ok(None)` emits
+    // no dead loads or duplicate records.
+    if crate::type_analysis::numeric_proof_is_declared_only(ctx, left)
+        || crate::type_analysis::numeric_proof_is_declared_only(ctx, right)
+    {
+        return Ok(None);
+    }
+
     // Hand this proven shape to `binary::lower`, which owns the existing
     // integer remainder and negative-zero repair. This must run before operand
     // lowering so returning `None` emits no dead loads or duplicate records.

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

@@ -290,6 +290,27 @@ 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
+        )
+    {
+        if init.is_some_and(|e| crate::type_analysis::numeric_proof_is_declared_only(ctx, e)) {
+            ctx.declared_only_numeric_locals.insert(id);
+        }
+    }
+
     // Track closure func_id → local_id mapping so the closure
     // call site in lower_call can look up rest param info.
     if let Some(perry_hir::Expr::Closure {

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

@@ -4569,7 +4569,7 @@ fn dynamic_bound_private_counter_is_safe(
     advanced_by_increment && !stmts_mutate_local(body, counter_id)
 }
 
-pub(super) fn emit_js_value_is_number(ctx: &mut FnCtx<'_>, value: &str) -> String {
+pub(crate) fn emit_js_value_is_number(ctx: &mut FnCtx<'_>, value: &str) -> String {
     let n_bits = ctx.block().bitcast_double_to_i64(value);
     let tag = ctx.block().and(
         I64,

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

@@ -31,7 +31,7 @@ mod unused_expr;
 
 pub(crate) use if_stmt::lower_if;
 pub(crate) use let_stmt::lower_let;
-pub(crate) use loops::{lower_do_while, lower_for, lower_while};
+pub(crate) use loops::{emit_js_value_is_number, lower_do_while, lower_for, lower_while};
 pub(crate) use switch_stmt::lower_switch;
 pub(crate) use try_stmt::lower_try;
 

crates/perry-codegen/src/type_analysis.rs

@@ -38,7 +38,8 @@ pub(crate) use numeric::{
 pub(crate) use pod::{
     add_operands_have_pod_materialization_hazard,
     expr_may_return_boxed_value_from_raw_f64_fallback, expression_has_numeric_length,
-    is_fixed_width_buffer_numeric_read, is_numeric_typed_array_class, pod_record_field_is_numeric,
+    is_fixed_width_buffer_numeric_read, is_numeric_typed_array_class,
+    numeric_proof_is_declared_only, pod_record_field_is_numeric,
     scalar_replaced_array_element_is_raw_f64, scalar_replaced_field_is_raw_f64,
     scalar_replaced_field_raw_f64_store_state,
 };