Skip to content

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

Merged
proggeramlug merged 2 commits into
mainfrom
fix/7773-declared-type-add-coercion
Aug 11, 2026
Merged

fix(codegen): a declared numeric type is not a proof that the value is a number#7831
proggeramlug merged 2 commits into
mainfrom
fix/7773-declared-type-add-coercion

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Closes #7773. Closes #7776.

The bug

Perry does not enforce type annotations at runtime — CLAUDE.md says so under 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 getting 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. A NaN-boxed string comes back out of the instruction still tagged as that string, and flows on as if nothing happened. typeof (v * 2) answered "string".

shape Node Perry (before)
o.x + 1 where (o as any).x = "s" s1 NaN
const v = o.x; v + 1 s1 s — the + 1 looked like it evaporated
const v = o.x; v * 2 NaN s, and typeof says "string"
s += r.x + r.y over a heterogeneous P[] 16zw1113151719 NaN

The escape is required in every case — a non-escaping receiver gets scalar-replaced, which is a real proof, and already printed correctly. That is why the trivial forms never showed it.

The fix

A 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 even for reads that end up with no boxed fallback at all. Every arm carrying a genuine proof answers false, so these keep their bare loads untouched: element-shape loop facts, class-field loop facts, Ptr<Shape> numeric fields, scalar replacement, POD records, and typed arrays (whose storage converts on store).

What is left is the guarded class-field / element diamond — whose cold arm exists precisely because the declared type can be wrong.

Two consumers:

  • + routes to 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 operator needs the dispatch, not a coerce.
  • every other arithmetic operator is a plain ToNumber, so the existing residual js_number_coerce rule suffices — it just could not see a refined local before.

expr/mod.rs::lower_numeric_binary_value turned out to be 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. It now hands declared-only operands down to binary::lower, the same way its two existing Mod cases already do.

Two things the first attempt got wrong

Both are now pinned by the test, and both are worth reading if you touch guarded arithmetic:

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, its cold arm stayed live in the loop, and the hot loop lost its fadd to an unconditional call — measured +38%. Fusing the tree removes the phi: one test over the tree's leaves, one branch, then either all-fadd or all-helper. Both arms rebuild the original tree shape, because + is not associative across strings — 1 + (2 + "x") is "12x" while (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 types s as Number while it holds a string the moment this lowering's own cold arm concatenates. That summed 16zw1113151719 down to 16zw: the original bug, one level up.

Cost

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

shape base fix delta
element-shape clone (a[i].x + a[i].y) 218 ms 217 ms −0.5% — untouched, as intended
this.v + 1 in a method 70 ms 76 ms +8.6%
s += p.x + p.y, escaped receiver 196 ms 263 ms +34.2%

I want to be straightforward about that last row rather than bury it. The cost falls only on reads the compiler could prove nothing about — those already pay an inline header precheck or a js_typed_feedback_class_field_get_guard call for their shape check, so the tag test rides alongside work that is already happening. But +34% on a tight loop is a real cost, and the tradeoff being made is: that, versus silently wrong arithmetic that also corrupts typeof. If you'd rather take a different tradeoff on that shape, this is the knob to argue about.

A corpus-wide measurement across 19 programs is being run against this branch by the perf session to confirm the containment claim empirically; I'll post the deltas when they land.

Validation

  • test-files/test_gap_declared_numeric_field_holds_string_7773.tsbyte-identical to Node across 10 assertions: both reported shapes, array elements, inherited fields, chained adds, the accumulator, and the other direction asserted for value (honest arithmetic, an honest guard failure, and a typed array must all still answer as numbers). A fix that coerced or dispatched everything would pass the first half and fail the second.
  • cargo test -p perry-codegen (the full suite — these are invisible to per-PR CI): 856 + 8 suites pass. The single failure, large_local_array_push_inbounds_store_emits_precise_slot_barrier, is pre-existing — verified BASE_EXIT=101 on a clean-main build in a separate worktree.
  • cargo fmt --all -- --check clean.

Depends on nothing, but note lint is currently red on main for an unrelated file-size cap — #7830 fixes that.

Summary by CodeRabbit

  • Bug Fixes

    • Corrected arithmetic involving values declared as numeric but containing non-numeric runtime values.
    • Added runtime checks and appropriate coercion for numeric additions, including chained expressions and mixed-type operands.
    • Preserved optimized behavior for proven numeric values and typed arrays.
  • Tests

    • Added regression coverage for declared numeric fields, array elements, inherited values, accumulators, guard failures, and operand-order variations.
  • Documentation

    • Documented the corrected numeric handling and performance results.

…s 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.
@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Declared numeric annotations are no longer treated as runtime proof. Codegen tracks declared-only numeric values, applies runtime-checked addition and residual coercion, preserves proven numeric fast paths, and adds regression coverage for invalid fields and heterogeneous arrays.

Changes

Declared numeric runtime safety

Layer / File(s) Summary
Track declared-only numeric proofs
crates/perry-codegen/src/expr/mod.rs, crates/perry-codegen/src/type_analysis/..., crates/perry-codegen/src/stmt/let_stmt.rs, crates/perry-codegen/src/codegen/...
FnCtx records numeric locals supported only by declarations. Numeric analysis identifies declared-only field, array, local, addition, and logical-expression proofs. All codegen context constructors initialize the collection.
Lower runtime-checked arithmetic
crates/perry-codegen/src/expr/binary.rs, crates/perry-codegen/src/expr/mod.rs, crates/perry-codegen/src/stmt/loops.rs, crates/perry-codegen/src/stmt/mod.rs
Declared-only additions perform runtime tag checks and dynamic string-or-number dispatch. Other arithmetic applies residual numeric coercion. Proven numeric values retain native fast paths.
Validate runtime behavior
test-files/test_gap_declared_numeric_field_holds_string_7773.ts, changelog.d/7831-declared-numeric-type-is-not-a-proof.md
Regression coverage exercises invalid declared fields, heterogeneous arrays, inherited fields, chained additions, valid numeric values, guards, and typed arrays. The changelog records the lowering changes and measurements.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant NumericAnalysis
  participant BinaryLowering
  participant RuntimeTagCheck
  participant Arithmetic
  NumericAnalysis->>BinaryLowering: identify declared-only numeric operand
  BinaryLowering->>RuntimeTagCheck: validate operand tags across addition tree
  RuntimeTagCheck-->>BinaryLowering: numeric or non-numeric runtime result
  BinaryLowering->>Arithmetic: use fadd or dynamic string-or-number addition
  BinaryLowering->>Arithmetic: apply ToNumber coercion for residual operators
Loading

Possibly related issues

Possibly related PRs

  • PerryTS/perry#7835 — Both modify binary lowering to distinguish declaration-based proofs from runtime value handling.
  • PerryTS/perry#6915 — Both modify numeric expression analysis and lowering in binary.rs and type_analysis.
  • PerryTS/perry#7810 — Both refine numeric proofs for indexed values and prevent unchecked numeric lowering.

Suggested labels: parity, ready

Suggested reviewers: thehypnoo, andrewtdiz

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary code-generation fix for declared numeric types that do not prove runtime numeric values.
Description check ✅ Passed The description provides detailed bug context, linked issues, implementation changes, validation results, and performance measurements.
Linked Issues check ✅ Passed The changes address both #7773 and #7776 with runtime-guarded numeric handling, corrected addition lowering, and regression coverage.
Out of Scope Changes check ✅ Passed The changes are focused on declared-only numeric proof tracking, guarded arithmetic lowering, initialization, and related regression documentation and tests.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/7773-declared-type-add-coercion

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

proggeramlug pushed a commit that referenced this pull request Aug 11, 2026
Reverts the previous commit's framing. The empty-string result is reachable on
1ee158d through is_definitely_string_expr's LocalGet arm, which already
trusts a declared type:

    const t: string = (99 as any);
    console.log(t + "x");   // node "99x", perry "x"

Verified on a clean 1ee158d build and on this branch. A declared FIELD and a
(string, number) parameter pair both route elsewhere and were already correct,
which is why three negative probes read as absence.

#7837 records two defects of the same premise. This PR fixes the dropped
operand (defect 2) and NOT the wrong-operator selection (defect 1, `s + 7`
printing 427 instead of 49), which lives in the one-sided arm and needs
#7831-style guarded lowering rather than a runtime delegation.

Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2
@proggeramlug

Copy link
Copy Markdown
Contributor Author

Measurement of this PR against the 25-program corpus is in progress but not yet reportable — I am posting the delay rather than leaving you waiting, and the reason is worth recording because it affects anyone else benchmarking today.

Setup (done, verified)

Built at 6666e8099 with -p perry -p perry-runtime-static -p perry-stdlib-static. Confirmed this is a pure codegen changegit diff --name-only origin/main...HEAD touches only crates/perry-codegen/**, one test file and a changelog fragment, with zero files under perry-runtime or perry-stdlib. So the isolation is two compilers against one runtime, exactly as the author specified: both arms pin PERRY_RUNTIME_DIR=$HOME/cargo-targets/m2/release (main's runtime), and only the compiler differs.

Both compilers report perry 0.5.1463, so I checked they are actually different binaries rather than trusting the version string — sha256 differs. All 25 programs compile under the PR arm and print byte-identical output to node --experimental-strip-types with exit code 0.

Why there is no timing yet

The bench mini has been unusable for the last few hours. Five parties have been measuring on one host, and the mkdir-based lock failed in three distinct ways today:

  1. An agent broke a 6-minute-old lock believing it was 52 minutes stale — check-then-break is not atomic.
  2. Another agent's release ran unconditionally from a queued task and deleted a third party's lock, twice, one of which was mine while my sweep was running.
  3. Two agents ran benchmarks without taking the lock at all, at load 7.96.

My first attempt at this measurement was running p_retain concurrently with another party's p_tree_wide, at load 6.28 against a normal 1.4. I killed it and discarded the partial results rather than post contaminated numbers to your PR.

The retry now gates on verified quiescence, not just the lock: it waits for load < 2.5 and zero foreign benchmark processes, takes a token-guarded lock, re-checks both at close, and marks the run DIRTY and discards it if load rose. I will post the deltas when a clean window opens.

What I will report

Per the author's priority order: shapes, interp, iso_miss, pipeline (expected to move), then churn_read, retain, tree (should be flat — a move there falsifies the containment claim). I will also report churn, churn_alloc, cycles and push_cls specifically because of the accumulator point the author raised — acc += <anything> puts the accumulator in the tested set regardless of the right-hand side, and all four are acc += loops whose reads are element-shape-proven but whose accumulators are not. If the containment claim breaks anywhere, I expect it there.

One independent corroboration, already in hand

A separate agent working on pipeline/shapes (#7835) tried widening is_numeric_expr in the same spirit and backed it out, having hit exactly the failure this PR fixes: a true there means "lowers to a real double", and the guarded class-field diamond's cold arm hands back a NaN-boxed value that fadd propagates. It landed a codegen test (alias_declared_number_field_is_deliberately_not_routed) pinning the refusal. That is the same bug reached from the opposite direction and is a point in this PR's favour.

Related: I filed #7837 for the string mirror of this premise — a lying string-declared local silently drops an operand (t + "x" returns "x"). Same root cause: an erased TypeScript annotation is a hint, not a runtime proof. Worth one policy decision across both predicates rather than two independent patches.

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Corpus result: no measurable cost — but the corpus barely exercises this PR, and that is the more important finding

Measured against the 25-program corpus at 1ee158d27. Two compilers, one runtime (PERRY_RUNTIME_DIR pinned to main's libperry_{runtime,stdlib}.a on both arms), as you specified — I confirmed the diff touches only crates/perry-codegen/** plus a test and a changelog fragment, zero files under perry-runtime/perry-stdlib. Both compilers report 0.5.1463, so I checked sha256 to confirm they are actually different binaries.

24 of 25 programs compile to byte-identical executables

IDENTICAL  churn churn_alloc churn_read push_num push_cls cycles deeplist
           tree tree_wide retain retain1 retain_wide retain_wide1 fib40
           interp iso_miss asyncpipe pipeline + all 6 *_real arms
DIFFERS    shapes

That is your containment claim, proved statically rather than by timing. For 24 programs the compiler emits the same machine code with and without this change, so their cost is provably unchanged — no measurement can improve on that, and no accumulator concern can materialise where no instruction differs. Notably churn, churn_alloc, cycles, push_cls and retain are all acc += … loops, and the accumulator you flagged does not get pulled into the tested set on any of them.

The one program that differs

The 24 identical binaries double as a noise calibration — their A/B ratio is this run's floor:

bench main #7831 ratio role
shapes 0.1895 0.1888 0.996 subject
churn 0.4328 0.4203 0.971 control (identical binary)
tree 1.6396 1.6416 1.001 control
retain 0.3439 0.3444 1.001 control
pipeline 0.5176 0.5174 1.000 control
interp 1.4954 1.4964 1.001 control

Control spread 0.971–1.001; shapes at 0.996 sits inside it. best-of-15 per cell, interleaved, exit codes checked, outputs byte-identical to node first.

Verdict: no measurable regression on this corpus.

The caveat that matters more than the result

Do not read this as "the guarded lowering is free." It is evidence that this corpus does not exercise it. One program in twenty-five even compiles differently. Your own microbenchmarks — p_this +8.6%, p_guard +34.2% — remain the only measurements of what the change costs on code that actually triggers it, and nothing here contradicts them.

So the honest summary is: the shapes that pay are rare in this corpus, and where they occur the cost is under its noise floor. Whether they are rare in real code is a different question that neither of us has measured.

Method note

Load on the bench mini was 3.15 falling to 2.40 during the run — above the quiet threshold, because another party has been compiling on that host. I would normally discard a run taken under those conditions, and I killed an earlier full-corpus attempt for exactly that reason. This one survives because the 24 byte-identical controls calibrate the noise within the same run: a contaminated window shows up as control spread, and 0.971–1.001 is what we got. Technique borrowed from #7833's author, who used it to prove a no-op without needing a quiet host at all.

Related

#7837 is the string mirror of this PR's premise — a lying string-declared local silently drops an operand (t + "x" returns "x") and picks concat over a numeric add. Same root: an erased TypeScript annotation is a hint, not a runtime proof. An agent is working it now and has been told to read this diff first, because the right outcome is probably one shared mechanism and one policy across both predicates rather than two independently-invented guards. Separately, #7835's author independently tried widening is_numeric_expr and backed it out after hitting exactly the failure this PR fixes, landing a test (alias_declared_number_field_is_deliberately_not_routed) pinning the refusal.

proggeramlug added a commit that referenced this pull request Aug 11, 2026
…ng (#7835)

* perf(codegen,runtime): let a declared `string` pick the concat lowering

`"shape:" + this.tag` — a string literal plus a field declared `string` —
lowered to `js_dynamic_string_or_number_add`: a RuntimeHandleScope, four
root_nanbox_f64s and two ToPrimitive calls spent rediscovering what the
declaration already stated. 88.8 ns per concatenation; 29.4 ns after.

Four changes:

1. `js_string_concat_box` forwards a non-string operand to
   `js_dynamic_string_or_number_add` instead of treating it as the empty
   string (`"ab" + 42` used to render as `"ab"`). This is a standalone
   silent-wrong-answer fix, and it is what makes (2) unable to change any
   program's output.

2. A new `is_declared_string_expr`, kept SEPARATE from
   `is_definitely_string_expr` because an annotation is evidence, not proof
   (#7831). Its only consumer is the two-operand concat, which emits
   `js_string_concat_box` — so after (1) the declaration selects a lowering
   and never an answer. The one-sided arm, the N-way chain fold and the Map
   string-key paths all keep the strict predicate; each could otherwise
   change a result.

3. `static_type_of` resolves `type X = { ... }` property types, as it
   already did for `interface X { ... }`. An object-type alias is
   structurally the same declaration; only the filing cabinet differed
   (`module.type_aliases` vs `module.interfaces`).

4. `class_dynamic_prop_root_store` takes `&str` and updates an existing key
   in place. Codegen emits `js_class_register_static_field` after every
   `Expr::StaticFieldSet`, so `Shape.made = Shape.made + 1` in a constructor
   allocated and dropped a `String` once per construction.

Quiet M1 mini, best-of-7, outputs byte-identical to node 26.5.1 with exit 0:
concat probe 0.2742 -> 0.1553, pipeline 0.5164 -> 0.4847, shapes 0.1894 ->
0.1833. No corpus regression; iso_miss canary clean under
PERRY_GC_SCHEDULE_RATE=1, PERRY_GC_PROTECT_FROMSPACE and
PERRY_GC_VERIFY_EVACUATION.

Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2

* chore: name the changelog fragment for PR #7835

Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2

* docs: the concat_box wrong answer is latent on main, not live

Codegen only selected js_string_concat_box when both operands satisfied the
strict is_definitely_string_expr, so reaching it with a non-string required a
lying `string`-declared local. A declared field and a (string, number)
parameter pair both route elsewhere and answer correctly on 1ee158d. The fix
is still required — widening the operand test to accept a declaration is what
would make the wrong answer reachable — but describing it as a live bug
overclaimed.

Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2

* docs: the concat_box wrong answer IS live; scope #7835 against #7837

Reverts the previous commit's framing. The empty-string result is reachable on
1ee158d through is_definitely_string_expr's LocalGet arm, which already
trusts a declared type:

    const t: string = (99 as any);
    console.log(t + "x");   // node "99x", perry "x"

Verified on a clean 1ee158d build and on this branch. A declared FIELD and a
(string, number) parameter pair both route elsewhere and were already correct,
which is why three negative probes read as absence.

#7837 records two defects of the same premise. This PR fixes the dropped
operand (defect 2) and NOT the wrong-operator selection (defect 1, `s + 7`
printing 427 instead of 49), which lives in the one-sided arm and needs
#7831-style guarded lowering rather than a runtime delegation.

Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2

---------

Co-authored-by: Ralph Küpper <ralph@skelpo.com>
@proggeramlug
proggeramlug marked this pull request as ready for review August 11, 2026 11:03
@proggeramlug

Copy link
Copy Markdown
Contributor Author

Cross-referencing from #7842 (the string mirror of this, closing #7837), since the two are the same premise and it is worth having one policy rather than two invented guards.

Same rule, applied in both: a static type may select a lowering, never an answer. The mechanisms came out different, and I think for a principled reason rather than an accidental one.

Your + fast arm is a single fadd. A guard is expensive beside one instruction, and its phi is what stops LLVM proving the merged value is a canonical double — which is where the +8.6% / +34.2% came from, and why the one-diamond-per-TREE note is the load-bearing implementation detail. On the string side every arm of the concat lowering is a heap-allocating runtime call, so the guard can ride inside a call that was already happening: I emit no diamond at all, just pass the operand NaN-boxed to a helper that tests the tag (js_string_add_value / js_value_add_string) instead of pre-unboxing it to a StringHeader*. Compiling all 19 corpus programs with both compilers against the same runtime archives, the emitted LLVM IR differs by exactly two lines — the two declares.

So the shared policy is real but the cost profile is not symmetric, and I do not think your numbers imply anything about mine or vice versa.

Two things from my side that bear on this PR, neither of which needs a change here:

  1. numeric_proof_is_declared_only may have a companion gap in the same file. emit_js_value_is_number tests "tag outside [SHORT_STRING_TAG, STRING_TAG]", i.e. "is a canonical raw double" — so an INT32-tagged or pointer-tagged operand takes the cold arm, which is correct and conservative. I checked it because a looser reading would have let fadd see a pointer. Noting it only because I read the code closely and it is worth someone else having confirmed it.
  2. alias_declared_number_field_is_deliberately_not_routed in perf(codegen,runtime): let a declared string pick the concat lowering #7835 stays green under my change, because "t:" is a string LITERAL and my predicate answers "proof" for it — the site never reaches the guard. No interaction to resolve.

Happy to fold the two predicates behind one shared name if you would rather have *_proof_is_declared_only be a single concept with a numeric and a string instance; right now they are two functions in two modules that happen to agree.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/perry-codegen/src/expr/binary.rs`:
- Around line 305-318: Update operand_needs_residual_coerce to apply the
numeric_proof_is_declared_only check to every declared-only expression shape,
not only Expr::LocalGet. Remove the LocalGet pattern restriction while
preserving the existing fallback-coercion and numeric-expression conditions, so
declared-only Binary{Add} operands receive residual coercion.

In `@crates/perry-codegen/src/stmt/let_stmt.rs`:
- Around line 303-312: Track declared-only numeric locals in the let-statement
handling for explicit Number and Int32 types as well as Any refined to numeric,
using numeric_proof_is_declared_only on initializers and preserving or
invalidating the marker on subsequent writes. In
crates/perry-codegen/src/codegen/function.rs:773, classify generic
declared-numeric parameters as declared-only unless a specialized entry supplies
runtime representation proof. Add regressions covering an explicit number local
and a number parameter receiving a poisoned value through any.

In `@crates/perry-codegen/src/type_analysis/pod.rs`:
- Around line 475-479: Restrict the `length` exemption in the relevant
type-analysis logic around the `property == "length"` check to native array and
string length reads only; user-defined class fields named `length` must continue
through the poisoned-value safety path. Add a regression case covering an `any`
write of a string into a numeric class field followed by `o.length + 1`, and
verify it does not use bare numeric lowering.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 501d55db-049d-4487-a5dd-a3a3cd8d91c7

📥 Commits

Reviewing files that changed from the base of the PR and between 1ee158d and 6666e80.

📒 Files selected for processing (13)
  • changelog.d/7831-declared-numeric-type-is-not-a-proof.md
  • crates/perry-codegen/src/codegen/closure.rs
  • crates/perry-codegen/src/codegen/entry.rs
  • crates/perry-codegen/src/codegen/function.rs
  • crates/perry-codegen/src/codegen/method.rs
  • crates/perry-codegen/src/expr/binary.rs
  • crates/perry-codegen/src/expr/mod.rs
  • crates/perry-codegen/src/stmt/let_stmt.rs
  • crates/perry-codegen/src/stmt/loops.rs
  • crates/perry-codegen/src/stmt/mod.rs
  • crates/perry-codegen/src/type_analysis.rs
  • crates/perry-codegen/src/type_analysis/pod.rs
  • test-files/test_gap_declared_numeric_field_holds_string_7773.ts

Comment on lines 305 to +318
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))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Remove the LocalGet restriction so a declared-only + subtree also gets the residual coerce.

numeric_proof_is_declared_only answers true for PropertyGet, IndexGet, LocalGet, Binary{Add}, and Logical. This clause matches only LocalGet.

PropertyGet and IndexGet are already covered, because expr_may_return_boxed_value_from_raw_f64_fallback is a precondition inside those arms of numeric_proof_is_declared_only. Logical is covered too, because lower_numeric_logical_for_number_context applies lower_operand_as_number per leaf.

Binary{Add} is not covered. Consider (o.x + 1) * 2 where o.x holds a string:

  1. The inner + routes to lower_declared_only_numeric_add and its slow arm returns a concatenated string.
  2. The outer Mul calls operand_needs_residual_coerce on the inner Binary{Add}. is_numeric_expr is true, the boxed-fallback predicate is false, and the expression is not a LocalGet, so no coerce is emitted.
  3. The outer fmul receives a NaN-boxed string and propagates the payload.

That is the same wrong-typeof failure this PR fixes, one operator out. The LocalGet restriction buys nothing for the other variants, so dropping it closes the gap without widening behavior elsewhere.

🐛 Proposed fix to cover every declared-only operand shape
             // `#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_declared_only_numeric_add`. A declared-only `+` SUBTREE
+            // consumed by a non-`+` operator needs the coerce too: its slow arm
+            // can return a string, and the enclosing `fmul` would propagate the
+            // payload.
+            || numeric_proof_is_declared_only(ctx, expr))
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
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))
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)
// `#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`. A declared-only `+` SUBTREE
// consumed by a non-`+` operator needs the coerce too: its slow arm
// can return a string, and the enclosing `fmul` would propagate the
// payload.
|| numeric_proof_is_declared_only(ctx, expr))
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-codegen/src/expr/binary.rs` around lines 305 - 318, Update
operand_needs_residual_coerce to apply the numeric_proof_is_declared_only check
to every declared-only expression shape, not only Expr::LocalGet. Remove the
LocalGet pattern restriction while preserving the existing fallback-coercion and
numeric-expression conditions, so declared-only Binary{Add} operands receive
residual coercion.

Comment on lines +303 to +312
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);
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Track all declared-only numeric locals.

The current tracking only covers Any locals refined at their declaration. It misses explicit numeric locals and generic numeric parameters. For example, const v: number = o.x and function f(v: number) { return v * 2; } can receive a NaN-boxed string through valid TypeScript typing paths and still emit bare arithmetic.

  • crates/perry-codegen/src/stmt/let_stmt.rs#L303-L312: mark explicit Number and Int32 locals when their initializer has a declared-only numeric proof. Maintain or invalidate this state on later writes.
  • crates/perry-codegen/src/codegen/function.rs#L773-L773: classify generic declared-numeric parameters as declared-only unless a specialized entry provides a runtime representation proof.

Add regressions for an explicit number local and a number parameter poisoned through any.

📍 Affects 2 files
  • crates/perry-codegen/src/stmt/let_stmt.rs#L303-L312 (this comment)
  • crates/perry-codegen/src/codegen/function.rs#L773-L773
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-codegen/src/stmt/let_stmt.rs` around lines 303 - 312, Track
declared-only numeric locals in the let-statement handling for explicit Number
and Int32 types as well as Any refined to numeric, using
numeric_proof_is_declared_only on initializers and preserving or invalidating
the marker on subsequent writes. In
crates/perry-codegen/src/codegen/function.rs:773, classify generic
declared-numeric parameters as declared-only unless a specialized entry supplies
runtime representation proof. Add regressions covering an explicit number local
and a number parameter receiving a poisoned value through any.

Comment on lines +475 to +479
// `.length` is produced by the runtime, not read out of a
// user-writable slot.
if property == "length" {
return false;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not exempt every length property.

Line 477 also exempts a user-defined length: number class field. If any writes a string into that field, o.length + 1 can still take bare numeric lowering and preserve the NaN-box payload.

Remove this broad exemption, or restrict it to native array and string length reads. Add a regression case for a poisoned class field named length.

Proposed fix
-            // `.length` is produced by the runtime, not read out of a
-            // user-writable slot.
-            if property == "length" {
-                return false;
-            }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// `.length` is produced by the runtime, not read out of a
// user-writable slot.
if property == "length" {
return false;
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-codegen/src/type_analysis/pod.rs` around lines 475 - 479,
Restrict the `length` exemption in the relevant type-analysis logic around the
`property == "length"` check to native array and string length reads only;
user-defined class fields named `length` must continue through the
poisoned-value safety path. Add a regression case covering an `any` write of a
string into a numeric class field followed by `o.length + 1`, and verify it does
not use bare numeric lowering.

proggeramlug pushed a commit that referenced this pull request Aug 11, 2026
…ot pick the `+` operator

`is_definitely_string_expr` answered `true` on the strength of an erased
TypeScript annotation, and `+` chose string concatenation from it. Perry does
not enforce declared types at runtime, so `const s: string = (42 as any)` puts
a number in the slot and thirteen shapes came out silently wrong, exit 0:
`s + 7` printed "427" instead of 49, and `t + "x"` printed "x" — the operand
was decoded as the empty string and vanished.

The policy, matching #7831 on the numeric side: a static type may select a
lowering, never an answer. Applied where each site can afford it.

* `js_string_concat_box` becomes total: a non-string operand is delegated to
  `js_dynamic_string_or_number_add` instead of `unwrap_or((null, 0))`-ing to
  the empty string. (Same hunk as #7835; whichever lands second drops its copy.)
* The one-sided `l ^ r` arm cannot be repaired that way — codegen unboxes to a
  `StringHeader*` before the call, so the tag is gone. A declared-only operand
  is now passed NaN-boxed to `js_string_add_value` / `js_value_add_string`,
  which test the tag and then run either the identical fused concat or the
  spec's `+`.
* The N-way chain fold requires a proven string in its head pair, since it
  formats every part as a string and only reproduces the source tree when the
  first node really concatenates.

`string_value_is_runtime_guaranteed` separates the two kinds of evidence the
predicate had been mixing. Its whitelist is closed: an unclassified arm answers
"claim" and gets guarded, which costs a compare rather than an answer.

Compiling all 19 corpus programs with the base and fixed compilers against the
same runtime archives yields LLVM IR differing by exactly two lines — the
`declare`s for the new helpers. No call site moved.

Refs #7837.

Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2
@proggeramlug
proggeramlug merged commit bc2777d into main Aug 11, 2026
1 of 19 checks passed
@proggeramlug
proggeramlug deleted the fix/7773-declared-type-add-coercion branch August 11, 2026 11:21
proggeramlug pushed a commit that referenced this pull request Aug 11, 2026
…push guard

Two sabotage-verified IR gates, prompted by review of the #7831/#7837 family
against #7839's guard.

`a_declared_type_lie_is_routed_to_the_runtime_tier_not_the_guard` — a
`number[]` really can hold heap strings at runtime, and `is_numeric_expr`
admits an element read off one (#7810). What keeps that value off the inline
guard is `expr_produces_canonical_raw_f64` excluding every READ, which routes
it to the pre-existing runtime numeric tier instead. Widening that predicate to
admit a read fails this test.

`the_guard_branches_on_the_live_bits_not_on_a_constant` — pins the guard's
condition to a computed register and its predicate to the full heap-tag set.
Hard-wiring the branch to `false` fails this test; it is invisible to every
output-equality probe, because the elided bookkeeping is a GC-liveness fact
rather than an arithmetic one.
proggeramlug pushed a commit that referenced this pull request Aug 11, 2026
…ot pick the `+` operator

`is_definitely_string_expr` answered `true` on the strength of an erased
TypeScript annotation, and `+` chose string concatenation from it. Perry does
not enforce declared types at runtime, so `const s: string = (42 as any)` puts
a number in the slot and thirteen shapes came out silently wrong, exit 0:
`s + 7` printed "427" instead of 49, and `t + "x"` printed "x" — the operand
was decoded as the empty string and vanished.

The policy, matching #7831 on the numeric side: a static type may select a
lowering, never an answer. Applied where each site can afford it.

* `js_string_concat_box` becomes total: a non-string operand is delegated to
  `js_dynamic_string_or_number_add` instead of `unwrap_or((null, 0))`-ing to
  the empty string. (Same hunk as #7835; whichever lands second drops its copy.)
* The one-sided `l ^ r` arm cannot be repaired that way — codegen unboxes to a
  `StringHeader*` before the call, so the tag is gone. A declared-only operand
  is now passed NaN-boxed to `js_string_add_value` / `js_value_add_string`,
  which test the tag and then run either the identical fused concat or the
  spec's `+`.
* The N-way chain fold requires a proven string in its head pair, since it
  formats every part as a string and only reproduces the source tree when the
  first node really concatenates.

`string_value_is_runtime_guaranteed` separates the two kinds of evidence the
predicate had been mixing. Its whitelist is closed: an unclassified arm answers
"claim" and gets guarded, which costs a compare rather than an answer.

Compiling all 19 corpus programs with the base and fixed compilers against the
same runtime archives yields LLVM IR differing by exactly two lines — the
`declare`s for the new helpers. No call site moved.

Refs #7837.

Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2
proggeramlug added a commit that referenced this pull request Aug 11, 2026
… live test (push_num 0.149 -> 0.069) (#7839)

* perf(codegen): put the numeric array push's GC bookkeeping behind one live test

The inline array-append tier emitted `js_string_addref_if_heap_string`,
`js_gc_note_slot_layout` and a seq_cst load of
`PERRY_INCREMENTAL_MARK_BARRIER_ACTIVE_COUNT` on EVERY element. On
`bench/push_num.ts` — 20,000,000 pushes of a double into a `number[]` — all
three are dead on all 20M of them.

The static proof that retires them cannot be made for the shape that matters:
`keep.push(base + j)` is an `Expr::Binary { Add }`, and
`expr_produces_non_pointer_bits_by_construction` answers `false` there
unconditionally, because `+` is string concatenation for non-numeric operands.

This is #7511's answer to the identical problem on class-field stores, applied
to the array append: ask the question ONCE inline, on the live bits, and branch
over all three calls. The array's half of the proof rides the header test the
`nofwd` block already performs — the integrity mask widens from 0x0407 to
0x3C07, so reaching the inline store additionally proves ELEMENT_SHAPE,
TYPED_LAYOUT_INTACT and ALL_POINTERS clear, the three states in which
`js_gc_note_slot_layout` does real work for a non-pointer value.

A guard, not an elision: Perry does not validate declared types, so a
`number`-annotated value that is a heap string at runtime takes the guarded arm
and records the slot exactly as it always did.

* test(codegen): pin that a declared-type lie cannot reach the numeric push guard

Two sabotage-verified IR gates, prompted by review of the #7831/#7837 family
against #7839's guard.

`a_declared_type_lie_is_routed_to_the_runtime_tier_not_the_guard` — a
`number[]` really can hold heap strings at runtime, and `is_numeric_expr`
admits an element read off one (#7810). What keeps that value off the inline
guard is `expr_produces_canonical_raw_f64` excluding every READ, which routes
it to the pre-existing runtime numeric tier instead. Widening that predicate to
admit a read fails this test.

`the_guard_branches_on_the_live_bits_not_on_a_constant` — pins the guard's
condition to a computed register and its predicate to the full heap-tag set.
Hard-wiring the branch to `false` fails this test; it is invisible to every
output-equality probe, because the elided bookkeeping is a GC-liveness fact
rather than an arithmetic one.

---------

Co-authored-by: Ralph Küpper <ralph@skelpo.com>
proggeramlug pushed a commit that referenced this pull request Aug 11, 2026
…ot pick the `+` operator

`is_definitely_string_expr` answered `true` on the strength of an erased
TypeScript annotation, and `+` chose string concatenation from it. Perry does
not enforce declared types at runtime, so `const s: string = (42 as any)` puts
a number in the slot and thirteen shapes came out silently wrong, exit 0:
`s + 7` printed "427" instead of 49, and `t + "x"` printed "x" — the operand
was decoded as the empty string and vanished.

The policy, matching #7831 on the numeric side: a static type may select a
lowering, never an answer. Applied where each site can afford it.

* `js_string_concat_box` becomes total: a non-string operand is delegated to
  `js_dynamic_string_or_number_add` instead of `unwrap_or((null, 0))`-ing to
  the empty string. (Same hunk as #7835; whichever lands second drops its copy.)
* The one-sided `l ^ r` arm cannot be repaired that way — codegen unboxes to a
  `StringHeader*` before the call, so the tag is gone. A declared-only operand
  is now passed NaN-boxed to `js_string_add_value` / `js_value_add_string`,
  which test the tag and then run either the identical fused concat or the
  spec's `+`.
* The N-way chain fold requires a proven string in its head pair, since it
  formats every part as a string and only reproduces the source tree when the
  first node really concatenates.

`string_value_is_runtime_guaranteed` separates the two kinds of evidence the
predicate had been mixing. Its whitelist is closed: an unclassified arm answers
"claim" and gets guarded, which costs a compare rather than an answer.

Compiling all 19 corpus programs with the base and fixed compilers against the
same runtime archives yields LLVM IR differing by exactly two lines — the
`declare`s for the new helpers. No call site moved.

Refs #7837.

Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2
proggeramlug pushed a commit that referenced this pull request Aug 11, 2026
…ot pick the `+` operator

`is_definitely_string_expr` answered `true` on the strength of an erased
TypeScript annotation, and `+` chose string concatenation from it. Perry does
not enforce declared types at runtime, so `const s: string = (42 as any)` puts
a number in the slot and thirteen shapes came out silently wrong, exit 0:
`s + 7` printed "427" instead of 49, and `t + "x"` printed "x" — the operand
was decoded as the empty string and vanished.

The policy, matching #7831 on the numeric side: a static type may select a
lowering, never an answer. Applied where each site can afford it.

* `js_string_concat_box` becomes total: a non-string operand is delegated to
  `js_dynamic_string_or_number_add` instead of `unwrap_or((null, 0))`-ing to
  the empty string. (Same hunk as #7835; whichever lands second drops its copy.)
* The one-sided `l ^ r` arm cannot be repaired that way — codegen unboxes to a
  `StringHeader*` before the call, so the tag is gone. A declared-only operand
  is now passed NaN-boxed to `js_string_add_value` / `js_value_add_string`,
  which test the tag and then run either the identical fused concat or the
  spec's `+`.
* The N-way chain fold requires a proven string in its head pair, since it
  formats every part as a string and only reproduces the source tree when the
  first node really concatenates.

`string_value_is_runtime_guaranteed` separates the two kinds of evidence the
predicate had been mixing. Its whitelist is closed: an unclassified arm answers
"claim" and gets guarded, which costs a compare rather than an answer.

Compiling all 19 corpus programs with the base and fixed compilers against the
same runtime archives yields LLVM IR differing by exactly two lines — the
`declare`s for the new helpers. No call site moved.

Refs #7837.

Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2
proggeramlug added a commit that referenced this pull request Aug 11, 2026
…ot pick the `+` operator (#7842)

`is_definitely_string_expr` answered `true` on the strength of an erased
TypeScript annotation, and `+` chose string concatenation from it. Perry does
not enforce declared types at runtime, so `const s: string = (42 as any)` puts
a number in the slot and thirteen shapes came out silently wrong, exit 0:
`s + 7` printed "427" instead of 49, and `t + "x"` printed "x" — the operand
was decoded as the empty string and vanished.

The policy, matching #7831 on the numeric side: a static type may select a
lowering, never an answer. Applied where each site can afford it.

* `js_string_concat_box` becomes total: a non-string operand is delegated to
  `js_dynamic_string_or_number_add` instead of `unwrap_or((null, 0))`-ing to
  the empty string. (Same hunk as #7835; whichever lands second drops its copy.)
* The one-sided `l ^ r` arm cannot be repaired that way — codegen unboxes to a
  `StringHeader*` before the call, so the tag is gone. A declared-only operand
  is now passed NaN-boxed to `js_string_add_value` / `js_value_add_string`,
  which test the tag and then run either the identical fused concat or the
  spec's `+`.
* The N-way chain fold requires a proven string in its head pair, since it
  formats every part as a string and only reproduces the source tree when the
  first node really concatenates.

`string_value_is_runtime_guaranteed` separates the two kinds of evidence the
predicate had been mixing. Its whitelist is closed: an unclassified arm answers
"claim" and gets guarded, which costs a compare rather than an answer.

Compiling all 19 corpus programs with the base and fixed compilers against the
same runtime archives yields LLVM IR differing by exactly two lines — the
`declare`s for the new helpers. No call site moved.

Refs #7837.

Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2

Co-authored-by: Ralph Küpper <ralph@skelpo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant