You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
correctness: a declared-number + chain (number[] elements, this fields) reads all operands before the adds — the SECOND entry into the fused fold, not #10904's dynamic one #10937
A + chain over operands that are numeric only because an annotation says so is fused into one guarded tree, which reads every operand before any addition converts anything. A valueOf running in an earlier conversion can then change what a later operand reads, and perry adds the stale value.
This is the same hazard as #10904 reached through a different entry. #10904 is the dynamic add path (Any operands); this one is the declared-number path (numeric_proof_is_declared_only), and it is the spelling most real TypeScript has, so a reader auditing "is my code affected" will not recognise themselves in #10904.
Repro
// 1. element reads through a `number[]`constA: number[]=[1,2,3];(Aasany)[0]={valueOf(){A[2]=100;return1;}};functionsum(a: number[]): number{returna[0]+a[1]+a[2];}console.log("elements:",sum(A));// node 103, perry 6// 2. field reads through `this`, the commonest shape in class codeclassH{a: number=1;b: number=2;c: number=3;e: number=4;sum(): number{returnthis.a+this.b+this.c+this.e;}}constH1=newH();(H1asany).a={valueOf(){H1.c=100;return1;}};console.log("this fields:",H1.sum());// node 107, perry 10
$ node --experimental-strip-types a.ts
elements: 103
this fields: 107
$ perry build a.ts -o a && ./a # main 841b605c9 (v0.5.1632, train 252), Linux x86-64
elements: 6
this fields: 10
No crash and no diagnostic — just a wrong number.
(as any) is only how the repro installs the object; nothing about the bug needs it. Any value that reaches a number-annotated field or element while actually holding an object with a side-effecting valueOf/toString does it, which is exactly what annotations do not prevent (CLAUDE.md, Known Limitations: annotations are erased).
Why the operand order matters
For Add(L, R) the specification evaluates L, evaluates R, and only then ToPrimitives both. So in (a[0] + a[1]) + a[2] — which is what source-level a[0] + a[1] + a[2] parses to — a[0]'s and a[1]'s conversions run before a[2] is evaluated, and a valueOf in one of them can assign a[2]. The fold reads all three first.
The cold arm does not rescue it: rebuild_add_tree(.., fast = false) rebuilds over the already-loaded operand values, so even the spec-+ path adds the stale one. That is why the symptom is a wrong number rather than a crash.
Where it enters
lower_guarded_numeric_add (crates/perry-codegen/src/expr/binary.rs) is reached from two call sites:
both_numeric && numeric_proof_is_declared_only(..) — no check at all. A read whose "it is a Number" answer rests only on a declared type takes the guarded diamond precisely because the annotation can be wrong, and then the whole tree is fused unconditionally.
Entry 2 is this issue.
Scope
Needs three or more operands with an inner left-leaning+. Two-operand chains are faithful by construction (both operands are evaluated before either is converted), and h + (a + b) is faithful too.
Needs a read the compiler could not prove numeric by anything stronger than the annotation. A proven tier (Ptr<Shape> numeric field, element-shape loop fact, scalar replacement, typed array storage, POD record) answers false to numeric_proof_is_declared_only and never reaches the fold.
Reproduced on main 841b605c9 (v0.5.1632, train 252), Linux x86-64. Not reproduced on the released 0.5.1220 on macOS arm64, which prints 103/107 — I have not bisected whether that is the version or the target.
Fix
Being fixed in #10921 by moving the faithfulness check to the top of lower_guarded_numeric_add, where every entry passes, rather than to either call site. A tree that would be read out of order lowers node by node through the spec helper. Fixture cases G (elements) and H (this fields) in test-files/test_parity_region_guards.ts pin both spellings, and a unit test pins the declared-number entry at the IR level.
This is worth knowing when reading matrix numbers: lane 13's read4__this__cctor/cfield cells got slower once this gate landed, because they had been taking the fused path and computing the wrong answer.
A
+chain over operands that are numeric only because an annotation says so is fused into one guarded tree, which reads every operand before any addition converts anything. AvalueOfrunning in an earlier conversion can then change what a later operand reads, and perry adds the stale value.This is the same hazard as #10904 reached through a different entry. #10904 is the dynamic add path (
Anyoperands); this one is the declared-number path (numeric_proof_is_declared_only), and it is the spelling most real TypeScript has, so a reader auditing "is my code affected" will not recognise themselves in #10904.Repro
No crash and no diagnostic — just a wrong number.
(as any)is only how the repro installs the object; nothing about the bug needs it. Any value that reaches anumber-annotated field or element while actually holding an object with a side-effectingvalueOf/toStringdoes it, which is exactly what annotations do not prevent (CLAUDE.md, Known Limitations: annotations are erased).Why the operand order matters
For
Add(L, R)the specification evaluatesL, evaluatesR, and only thenToPrimitives both. So in(a[0] + a[1]) + a[2]— which is what source-levela[0] + a[1] + a[2]parses to —a[0]'s anda[1]'s conversions run beforea[2]is evaluated, and avalueOfin one of them can assigna[2]. The fold reads all three first.The cold arm does not rescue it:
rebuild_add_tree(.., fast = false)rebuilds over the already-loaded operand values, so even the spec-+path adds the stale one. That is why the symptom is a wrong number rather than a crash.Where it enters
lower_guarded_numeric_add(crates/perry-codegen/src/expr/binary.rs) is reached from two call sites:dynamic_add_tree_benefits_shared_guard, which fix(#10904): a + tree must not read a leaf after a conversion that precedes it #10921 taught to refuse an unfaithful tree;both_numeric && numeric_proof_is_declared_only(..)— no check at all. A read whose "it is a Number" answer rests only on a declared type takes the guarded diamond precisely because the annotation can be wrong, and then the whole tree is fused unconditionally.Entry 2 is this issue.
Scope
+. Two-operand chains are faithful by construction (both operands are evaluated before either is converted), andh + (a + b)is faithful too.Ptr<Shape>numeric field, element-shape loop fact, scalar replacement, typed array storage, POD record) answersfalsetonumeric_proof_is_declared_onlyand never reaches the fold.841b605c9(v0.5.1632, train 252), Linux x86-64. Not reproduced on the released 0.5.1220 on macOS arm64, which prints 103/107 — I have not bisected whether that is the version or the target.Fix
Being fixed in #10921 by moving the faithfulness check to the top of
lower_guarded_numeric_add, where every entry passes, rather than to either call site. A tree that would be read out of order lowers node by node through the spec helper. Fixture casesG(elements) andH(thisfields) intest-files/test_parity_region_guards.tspin both spellings, and a unit test pins the declared-number entry at the IR level.This is worth knowing when reading matrix numbers: lane 13's
read4__this__cctor/cfieldcells got slower once this gate landed, because they had been taking the fused path and computing the wrong answer.