correctness: a 3+ operand + chain reads all operands before the adds — a mutating valueOf/toString sees a stale later operand

Summary

In a left-associative + chain of three or more operands, perry evaluates every operand up front and only then performs the additions. The specification evaluates left to right with the additions interleaved — a + b + c is read a, read b, add, read c, add. When that first add runs user code through ToPrimitive (valueOf/toString), any mutation it makes to a later operand's source is invisible to perry, which already read it.

No crash, no diagnostic — a plain wrong number.

const O = { a: null, b: 1, c: 7 };
O.a = { valueOf() { O.c = 100; return 1; } };
console.log(O.a + O.b + O.c);
// node : 102   (a.valueOf() sets c=100, then 1 + 1 + 100)
// perry:   9   (a, b, c all read first: 1 + 1 + 7)

Reproduced on pristine v0.5.1618 (train 239, b9ba951ff) and on train 252. Nothing lane-specific.

The split is exact — only a `+` chain of 3+ operands

Same object and the same mutating valueOf in every row ({ valueOf() { O.c = 100; return 1; } } bound to O.a):

expression	node	perry
`O.a + O.b + O.c`	`102`	`9`	wrong
`O.a + O.b + O.c + O.c`	`202`	`16`	wrong
`O.a + O.c` (2 operands)	`8`	`8`	correct
`O.a + (O.b + O.c)`	`9`	`9`	correct
`O.a - O.b - O.c`	`-100`	`-100`	correct
`O.a * O.b * O.c`	`100`	`100`	correct
`O.a + O.b * O.c`	`8`	`8`	correct
`O.a < O.b`, then `O.c`	`false:100`	`false:100`	correct
`O.a == O.b`, then `O.c`	`true:100`	`true:100`	correct
`${O.a}${O.b}${O.c}`	`[object Object]17`	`[object Object]17`	correct
`O.a + bump() + O.c` (explicit call)	`102`	`102`	correct

So: left-associative +, three or more operands, and the user code reached implicitly. An explicit call in the same position is handled correctly, which is the tell — the chain fold does not know that + itself can call.

The mutation does not have to change the shape. All three of these are wrong, and the plain overwrite is the sharpest because nothing structural happens at all:

O.a = { valueOf() { O.c = 100; return 1; } };        // plain overwrite → perry 9,   node 102
O.a = { valueOf() { delete O.c; return 1; } };       // delete          → perry 9,   node NaN
O.a = { valueOf() { Object.defineProperty(O, "c", { get: () => 50 }); return 1; } };
                                                      // accessor        → perry 9,   node 52

toString reaches it too, and shows the stale read directly:

const O = { a: 1, b: null, c: 7 };
O.b = { toString() { delete O.c; return "2"; } };
console.log(O.a + O.b + O.c);
// node : "12undefined"
// perry: "127"

Receiver form is irrelevant — module-level const, function-local const, class instance and array element all reproduce.

Relationship to #10775

Same family, different defect, and they do not overlap:

correctness: a folded + chain coerces objects with ToString instead of ToPrimitive(default) — valueOf-only objects print [object Object] #10775 is the hint: a folded chain coerces with ToString where the spec wants ToPrimitive(default). Its text states explicitly that "Evaluation order and a throwing valueOf both still behave correctly."
This is the order: the operands are read before the additions that can mutate them. It reproduces with no string operand anywhere in the chain, so it is not the string-concat fold correctness: a folded + chain coerces objects with ToString instead of ToPrimitive(default) — valueOf-only objects print [object Object] #10775 analyses — it is whichever fold gathers a numeric + chain's operands.

Fixing #10775 would not fix this, and vice versa.

Why it matters beyond the repro

o.x + o.y + o.z over one receiver is an extremely common shape, and the operands only have to be possibly non-primitive for the hazard to exist — which, without a representation fact, is every any/number|object field.

It is also a constraint on the object-model campaign's step 4b (region-scoped guards, #10884): a region that guards a receiver once and then loads slots by offset must not hoist a load above an operator that can call user code. The rule that makes 4b sound — load, then verify every loaded operand is primitive, and only then run the operators, with the single bail edge before any of them — is the same rule that fixes this, because verifying primitiveness first is exactly what proves the reordering unobservable.

Fixture

test-files/test_parity_region_guards.ts on feat/region-guards carries these as differential cases against node (A, A2, A3, A5 fail today; A4, B, B2, C, D, E pass).

Root cause — located

lower_guarded_numeric_add (crates/perry-codegen/src/expr/binary.rs:187), reached from the gate at binary.rs:1134:

if dynamic_add_tree_benefits_shared_guard(expr) && !materialization_hazard {
    return lower_guarded_numeric_add(ctx, expr);
}
return lower_rooted_dynamic_binary(ctx, "js_dynamic_string_or_number_add", left, right, …);

lower_guarded_numeric_add collects the whole + tree's leaves and evaluates all of them up front, then applies one shared tag test over them, then rebuilds the tree node-for-node in either arm:

let mut leaves = Vec::new();
add_tree_leaves(expr, &mut leaves);
let needs_test: Vec<bool> = leaves.iter()
    .map(|leaf| !expr_produces_canonical_raw_f64(ctx, leaf)).collect();
with_operands_rooted(ctx, &leaves, |ctx, values| { … })   // ← every leaf lowered here

Fusing the tree into one diamond instead of one per node is deliberate and load-bearing — the doc comment at :155-165 records that per-node diamonds cost 86 ms -> 119 ms on a bench mini, because the outer add consumes a PHI that LLVM cannot prove is a canonical double. Associativity is preserved (rebuild_add_tree, :465). What is not preserved is the interleaving of evaluation with conversion.

Why two leaves are correct and three are not

This is the whole boundary, and it is a spec detail rather than an implementation accident:

a + c — the spec evaluates both operands, then ToPrimitives both, then adds. Pre-evaluating both is exactly right. ✔
(a + b) + c — the spec evaluates a, evaluates b, ToPrimitives both, adds, and only then evaluates c. So c must be read after a's conversion has run. The fold reads it before. ✘

That is why O.a + O.c is correct, O.a + (O.b + O.c) is correct (the parenthesised node is a leaf, evaluated before a's conversion — which is also what node does), and O.a + O.b + O.c is wrong.

It also explains the rest of the split. - and * never reach this path (ToNumber, not ToPrimitive-with-a-string-outcome, and a different lowering); an explicit call in the same position is a leaf whose effects run during with_operands_rooted in source order, so it is correct; and the fall-through at :1137 (lower_rooted_dynamic_binary) is spec-correct for 3+ leaves because it lowers left — including left's own helper call and therefore its conversions — before it lowers right.

The cold arm does not rescue it: rebuild_add_tree(…, fast = false) rebuilds over the already-lowered leaf values, so even the slow, spec-+ path adds the stale c. That is why the wrong answer is 9 rather than a crash.

The minimal fix

Admit the 3+-leaf fold only when no leaf needs a test — i.e. every leaf is expr_produces_canonical_raw_f64. If no leaf can be non-primitive, no ToPrimitive can run, no user code can run, and pre-evaluating every leaf is unobservable. Two leaves keep the fold unconditionally, per the spec order above.

That is a change to dynamic_add_tree_benefits_shared_guard (binary.rs:448) — it needs ctx to ask the vouching question — and to its single call site at :1134. lower_guarded_numeric_add and rebuild_add_tree are not modified.

Expected cost, stated up front

h += o.a + o.b + o.c + o.d is a five-leaf tree in which nothing is vouched (expr_produces_canonical_raw_f64 deliberately declines a LocalGet, :180-182). It currently folds and after the fix will not, so it returns to per-node diamonds — the 86 -> 119 ms shape the fold exists to avoid. This will measurably slow the k/w fixtures and that is the correct trade; the numbers will be in the PR rather than tuned away.

It is also exactly what step 4b stage 1 (#10884) buys back: a region verifies every loaded operand is a primitive number before any operator runs, which makes the leaves vouched and the fold admissible again — with the pre-evaluation licensed rather than assumed.

Affected versions / trigger

Reproduced on v0.5.1618 (train 239, b9ba951ff) and train 252; the fold predates both. The trigger needs an object operand with a side-effecting valueOf/toString in a 3+ operand + chain — rare enough to explain how it survived, but it is the shape Decimal/BigNumber-style wrapper libraries and test mocks use, and the mutation does not have to be structural: a plain overwrite of a later operand's field is enough.

Scope note, so this is not read as "a+b+c is broken": plain-number and plain-string chains were never wrong, and neither is any chain whose operands are all primitives. The fold pre-evaluates every leaf, and that is only observable when converting an earlier leaf runs user code — which requires an object operand with a side-effecting valueOf/toString (or Symbol.toPrimitive) in a 3+ operand + chain. 1 + 2 + 3, s1 + s2 + s3, and o.x + o.y + o.z over numeric fields all produce correct results today.

That is also why it survived: the trigger is the wrapper-object pattern (Decimal/BigNumber-style, test mocks, lazily-computed accessors), not everyday arithmetic.

fix(#10904): the + fold may read a late leaf no conversion can change

fix(#10904): decide faithfulness inside the fold, which has two entries

test(#10904): pin the declared-number entry on a `this` receiver

For anyone auditing whether their code is affected: the same hazard has a second entry that this issue's title and repro do not cover, filed as #10937. This issue is the dynamic add path (Any operands). #10937 is the declared-number path — a[0] + a[1] + a[2] over a number[] prints 6 against node's 103, and this.a + this.b + this.c + this.e over number fields prints 10 against node's 107, both on main 841b605c9 (v0.5.1632). #10921 fixes both, by moving the check to the top of the fold where every entry passes.

fixtures: the step-4b region counterexamples, committed before the im…

fix(#10904): a + tree must not read a leaf after a conversion that pr…

fix(#10904): the + fold may read a late leaf no conversion can change

fix(#10904): decide faithfulness inside the fold, which has two entries

test(#10904): pin the declared-number entry on a `this` receiver

step 4b stage 1: read regions over a + tree, guarded ONCE (#10884)

Navigation Menu

Repository navigation

correctness: a 3+ operand `+` chain reads all operands before the adds — a mutating valueOf/toString sees a stale later operand #10904

Summary

The split is exact — only a `+` chain of 3+ operands

Relationship to #10775

Why it matters beyond the repro

Fixture

Root cause — located

Why two leaves are correct and three are not

The minimal fix

Expected cost, stated up front

Affected versions / trigger

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

correctness: a 3+ operand + chain reads all operands before the adds — a mutating valueOf/toString sees a stale later operand #10904

Description

Summary

The split is exact — only a + chain of 3+ operands

Relationship to #10775

Why it matters beyond the repro

Fixture

Activity

proggeramlug commented on Sep 21, 2026

Root cause — located

Why two leaves are correct and three are not

The minimal fix

Expected cost, stated up front

Affected versions / trigger

proggeramlug commented on Sep 21, 2026

proggeramlug commented on Sep 22, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

correctness: a 3+ operand `+` chain reads all operands before the adds — a mutating valueOf/toString sees a stale later operand #10904

The split is exact — only a `+` chain of 3+ operands