mirror of
https://github.com/oxc-project/oxc.git
synced 2026-09-14 19:36:11 +08:00
Kotaro Chikuba
af82a0707a
`substitute_unary_plus` removes a `+` when the enclosing binary operator will do the `ToNumber` conversion anyway. The existing doc comment reasons carefully about `ToNumber` vs `ToNumeric`, but not about *when* each runs. `+a` converts `a` while evaluating `+a`, before the right operand is evaluated at all. `a - n` converts it in `ApplyStringOrNumericBinaryOperator`, after `n` has been evaluated. Dropping the `+` therefore moves the conversion across `n`, and a right operand with a side effect can change what the conversion sees: ```js var xs = []; console.log((+xs) - (xs.push(1), 0)); // 0, converted while `xs` was still empty // oxc emitted: console.log(xs - (xs.push(1), 0)); // 1, converted after `xs` grew ``` This PR requires the right operand to be free of side effects before dropping a `+` in the left position. It cannot observe the conversion either: under the "Coercion Methods Are Pure" assumption the conversion runs no user code, so there is nothing to observe. The right position (`n - +a`) is left as is: `n` is evaluated first either way, and its `ToNumeric` is a no-op because the existing guard already established that `n` is a number, so nothing runs between evaluating `a` and converting it. `cargo minsize` moves the typescript.js gzip figure down by 10 bytes; every minified size is identical. The reproduction was found by the minifier fuzzer (#25594) at seed 12397. ### Differential testing Every case below was run through oxc and then executed in Node against the unminified source, comparing the result, the mutated state, and the error constructor *and message*. | shape | cases | value worlds | runs | | --- | ---: | ---: | ---: | | operator × 37 hand-written right operands | 814 | 41 | 33,374 | | right operand reading a closure variable the conversion may write | 374 | 41 | 15,334 | | argument shape (`+a.v`, `+f()`, `+(seq(), a)`, `+(+a)`, getter) × nesting | 3,168 | 41 | 129,888 | | random expressions, 3 seeds, depth 2-3 | 7,500 | 41 | 307,500 | The worlds cover `[]`, `{ valueOf: () => 10n }`, `Symbol()`, `Object.create(null)`, objects with getters, `Proxy`, and the numeric edge cases. No difference survives, other than one that predates this pass and is discussed in the doc comment: for a `BigInt` argument both forms throw a `TypeError`, but `+a` says "Cannot convert a BigInt value to a number" while `a - 1` says "Cannot mix BigInt and other types". The error kind never changes. --- Written with AI assistance (Claude Code). Reviewed by me, and the before/after behaviour was checked against Node.js. --------- Signed-off-by: Kotaro Chikuba <miz404@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Track Minification size
See minification-benchmarks for baseline.