From fd8ac00e63ab536e4065ab55cb06932d00c7c7c9 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Wed, 21 Oct 2026 08:53:09 -0500 Subject: [PATCH 1/3] Convert global isFinite's argument with ToNumber Global isFinite now converts any argument with ToNumber before the test, the same way global isNaN does, and only Number.isFinite stays strict. Before, an argument that wasn't typed as a number, such as a checker-unknown or dynamic value, refused with SC2012. The shared ToNumber conversion for these arguments (isNaN, isFinite, the Math functions, String.fromCharCode, DataView offsets) now handles every value kind: strings parse as numeric literals, booleans and null convert to numbers, undefined is NaN, and dynamic values call valueOf/toString or throw Node's TypeError for a Symbol or BigInt. Before, a dynamic value went through a checked number cast and threw a TypeError for a string, and a string or boolean in a JavaScript file reached a run-time SC1090. Fixes #1386 Co-authored-by: espetro <26749475+espetro@users.noreply.github.com> --- .../src/frontend/lowering/lower-calls.ts | 84 +++++++------------ .../src/frontend/lowering/lower-exprs.ts | 22 +++++ .../corpus/6905-isfinite-coercion.ts.json | 6 ++ .../corpus/6906-tonumber-arguments.mjs.json | 6 ++ tests/corpus/6905-isfinite-coercion.ts | 38 +++++++++ tests/corpus/6906-tonumber-arguments.mjs | 32 +++++++ 6 files changed, 135 insertions(+), 53 deletions(-) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6905-isfinite-coercion.ts.json create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6906-tonumber-arguments.mjs.json create mode 100644 tests/corpus/6905-isfinite-coercion.ts create mode 100644 tests/corpus/6906-tonumber-arguments.mjs diff --git a/packages/compiler/src/frontend/lowering/lower-calls.ts b/packages/compiler/src/frontend/lowering/lower-calls.ts index d768cec9b..9d871ca8b 100644 --- a/packages/compiler/src/frontend/lowering/lower-calls.ts +++ b/packages/compiler/src/frontend/lowering/lower-calls.ts @@ -166,7 +166,6 @@ import { lowerReflectCall } from "./builtins/reflection.js"; import { lowerAbsenceProbe, v8CalleeText, - lowerOptionalNumber, lowerPromiseAllTupleCall, lowerPromiseRejectCall, lowerToNumberArgument, @@ -1008,10 +1007,9 @@ export function lowerCall(lowerer: Lowerer, expr: ts.CallExpression): IrExpr { // excludes shadowed names. parseInt's omitted radix completes // to 0 — the spec's "undefined" (base 10 with the 0x hex escape); // parseFloat lowers the STRING form only (Node would ToString other - // values — no static story); isNaN/isFinite's arguments are - // checker-pinned (or checked) to number, where the global's ToNumber - // coercion is the identity and the tests are Number.isNaN / - // Number.isFinite exactly (ms's `isFinite(val)` guard). + // values — no static story); isNaN/isFinite apply the global's + // ToNumber to any argument, then test like Number.isNaN / + // Number.isFinite. if ( (expr.expression.text === "parseInt" || expr.expression.text === "isNaN" || @@ -1130,18 +1128,14 @@ export function lowerCall(lowerer: Lowerer, expr: ts.CallExpression): IrExpr { } if (islFn && expr.arguments.nodes.length === islFn.args.length) { // The static forms above already took the exactly-typed argument - // (a string for parseFloat, a number for isFinite): name the - // argument type that refused the call, so the message states the - // condition rather than reading as if the global itself were - // unsupported. + // (a string for parseFloat): name the argument type that refused + // the call, so the message states the condition rather than + // reading as if the global itself were unsupported. const argText = loweredArgumentText(lowerer, expr.arguments.nodes[0]!); - const exact = expr.expression.text === "isFinite" ? "number" : "string"; lowerer.refuseUnsupportedApi( `'${expr.expression.text}' with a '${argText}' argument`, expr, - exact === "string" - ? "only a string argument compiles statically: convert the argument explicitly first (for example String(x), or narrow the union)" - : "only a number argument compiles statically: narrow the argument to a number first", + "only a string argument compiles statically: convert the argument explicitly first (for example String(x), or narrow the union)", ); } } @@ -10298,7 +10292,7 @@ export function lowerStaticNumberGlobal( args: readonly ts.Expression[], loc: SrcLoc, ): IrExpr | null { - if (name === "parseInt" || name === "isNaN") { + if (name === "parseInt" || name === "isNaN" || name === "isFinite") { const maxArgs = name === "parseInt" ? 2 : 1; if (args.length < 1 || args.length > maxArgs) { lowerer.noLowering( @@ -10306,10 +10300,12 @@ export function lowerStaticNumberGlobal( call, ); } - if (name === "isNaN") { - // The global's ToNumber turns a missing value into NaN. + if (name === "isNaN" || name === "isFinite") { + // The global's ToNumber turns a missing value into NaN, converts + // strings, booleans and null, and calls valueOf/toString on objects. const x = lowerToNumberArgument(lowerer, args[0]!); - return { kind: "libCall", fn: "num.isNaN", args: [x], type: BOOL, loc }; + const fn = name === "isNaN" ? "num.isNaN" : "number.isFinite"; + return { kind: "libCall", fn, args: [x], type: BOOL, loc }; } const radix: IrExpr = args[1] ? lowerer.lowerExprExpecting(args[1], F64) @@ -10445,32 +10441,28 @@ export function lowerStaticNumberGlobal( : lowerer.lowerExprExpecting(args[0]!, STRING); return { kind: "libCall", fn: "num.parseInt", args: [s, radix], type: F64, loc }; } - // STATIC parseFloat/isFinite over exactly-typed arguments — - // parseInt's siblings (num.parseFloat is ECMA 19.2.4's decimal- - // literal prefix parse in scr_string.c; a number-typed isFinite IS - // Number.isFinite — the global's ToNumber coercion is the identity - // there, ms's `isFinite(val)` guard). Other argument types retain - // their native coercion checks or the SC2012 refusal. The probe never - // emits — lowering is IR construction. - if ((name === "parseFloat" || name === "isFinite") && args.length === 1) { - if (name === "parseFloat") { - const optional = optionalCallValue(lowerer, args[0]!); - if (optional) { - const parsed = lowerOptionalStringNumber( - lowerer, - optional, - loc, - "num.parseFloat", - "parseFloat", - ); - if (parsed) return parsed; - } + // STATIC parseFloat over string arguments — parseInt's sibling + // (num.parseFloat is ECMA 19.2.4's decimal-literal prefix parse in + // scr_string.c). Other argument types retain their native coercion + // checks or the SC2012 refusal. The probe never emits — lowering is IR + // construction. + if (name === "parseFloat" && args.length === 1) { + const optional = optionalCallValue(lowerer, args[0]!); + if (optional) { + const parsed = lowerOptionalStringNumber( + lowerer, + optional, + loc, + "num.parseFloat", + "parseFloat", + ); + if (parsed) return parsed; } const probed = tryLowerExpression(lowerer, args[0]!); - if (name === "parseFloat" && probed?.type.kind === "string") { + if (probed?.type.kind === "string") { return { kind: "libCall", fn: "num.parseFloat", args: [probed], type: F64, loc }; } - if (name === "parseFloat" && probed?.type.kind === "dyn") { + if (probed?.type.kind === "dyn") { return { kind: "libCall", fn: "num.parseFloat", @@ -10479,20 +10471,6 @@ export function lowerStaticNumberGlobal( loc, }; } - if (name === "isFinite" && probed?.type.kind === "f64") { - return { kind: "libCall", fn: "number.isFinite", args: [probed], type: BOOL, loc }; - } - // A number the checker trusts but storage may hold missing (an - // out-of-range read): the global's ToNumber makes undefined NaN. - if ( - name === "isFinite" && - probed?.type.kind === "union" && - lowerer.armTag(probed.type.unionId, UNDEFINED_T) >= 0 && - lowerer.stripUndefinedArm(probed.type).kind === "f64" - ) { - const x = lowerOptionalNumber(lowerer, probed, loc, args[0]); - return { kind: "libCall", fn: "number.isFinite", args: [x], type: BOOL, loc }; - } } return null; } diff --git a/packages/compiler/src/frontend/lowering/lower-exprs.ts b/packages/compiler/src/frontend/lowering/lower-exprs.ts index 591057cab..4b03e915d 100644 --- a/packages/compiler/src/frontend/lowering/lower-exprs.ts +++ b/packages/compiler/src/frontend/lowering/lower-exprs.ts @@ -1438,6 +1438,28 @@ export function lowerToNumberArgument(lowerer: Lowerer, node: ts.Expression): Ir if (ts.isConditionalExpression(inner)) return lowerer.lowerExprExpecting(node, F64); const value = lowerer.lowerExpr(node); if (isOptionalNumber(value)) return lowerOptionalNumber(lowerer, value, loc, inner); + // Other values convert with ToNumber: strings parse as numeric literals, + // booleans and null are numbers, and checked-dynamic values call + // valueOf/toString or throw Node's TypeError for a Symbol or BigInt. + const num = (value: number): IrExpr => ({ kind: "numLit", value, type: F64, loc }); + switch (value.type.kind) { + case "string": + return { kind: "libCall", fn: "num.fromString", args: [value], type: F64, loc }; + case "bool": + return { kind: "ternary", cond: value, then: num(1), else_: num(0), type: F64, loc }; + case "nullT": + return defaultAfterUndefined(value, num(0)); + case "undefinedT": + return defaultAfterUndefined(value, num(NaN)); + case "dyn": + return { kind: "libCall", fn: "dyn.toNumberCoerce", args: [value], type: F64, loc }; + case "union": + if (lowerer.dynConvertible(value.type)) { + const boxed = lowerer.coerceToExpected(value, DYN); + return { kind: "libCall", fn: "dyn.toNumberCoerce", args: [boxed], type: F64, loc }; + } + break; + } return lowerer.coerceInto(node, value, F64); } diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6905-isfinite-coercion.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6905-isfinite-coercion.ts.json new file mode 100644 index 000000000..210987798 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6905-isfinite-coercion.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/6905-isfinite-coercion.ts" + ], + "diags": [] +} diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6906-tonumber-arguments.mjs.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6906-tonumber-arguments.mjs.json new file mode 100644 index 000000000..2fc650f3a --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6906-tonumber-arguments.mjs.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/6906-tonumber-arguments.mjs" + ], + "diags": [] +} diff --git a/tests/corpus/6905-isfinite-coercion.ts b/tests/corpus/6905-isfinite-coercion.ts new file mode 100644 index 000000000..57e158c72 --- /dev/null +++ b/tests/corpus/6905-isfinite-coercion.ts @@ -0,0 +1,38 @@ +// Global isFinite applies ToNumber to its argument before the test, like +// global isNaN: strings parse as numeric literals, booleans and null are 0 +// and 1, undefined is NaN, and objects convert through valueOf/toString. +// Number.isFinite stays strict and answers false for anything not a number. +const values: unknown[] = ["12", "", " 0x10 ", "abc", "1e400", true, false, null, undefined, {}, [], [7], [1, 2]]; +for (const value of values) { + console.log(JSON.stringify(value) ?? "undefined", isFinite(value as any), Number.isFinite(value)); +} + +const u: unknown = 12; +console.log(isFinite(u as any)); + +let calls = ""; +const boxed: unknown = { valueOf: () => ((calls += "v"), 5) }; +console.log(isFinite(boxed as any), calls); +const text: unknown = { toString: () => "Infinity" }; +console.log(isFinite(text as any)); + +class Cache { + #maxAge: unknown; + constructor(maxAge?: number) { + this.#maxAge = maxAge; + } + expires(): boolean { + return isFinite(this.#maxAge as any); + } +} +console.log(new Cache(1000).expires(), new Cache().expires()); + +const throwing: unknown[] = [Symbol("s"), 10n]; +for (const value of throwing) { + try { + isFinite(value as any); + } catch (e) { + console.log(e instanceof TypeError); + } +} +console.log(isFinite(1 / 0), isFinite(-1 / 0), isFinite(0 / 0), isFinite(-0)); diff --git a/tests/corpus/6906-tonumber-arguments.mjs b/tests/corpus/6906-tonumber-arguments.mjs new file mode 100644 index 000000000..23b150eb8 --- /dev/null +++ b/tests/corpus/6906-tonumber-arguments.mjs @@ -0,0 +1,32 @@ +// Arguments that the global number functions convert with ToNumber, in a +// JavaScript file where they arrive as strings, booleans, null, undefined +// or dynamic values: isNaN, isFinite, Math functions and +// String.fromCharCode match Node for each. +console.log(isNaN("12"), isNaN("abc"), isNaN(true), isNaN(null), isNaN(undefined)); +console.log(isFinite("12"), isFinite("abc"), isFinite(true), isFinite(null), isFinite(undefined)); +console.log(Math.floor("3.7"), Math.max("4", 2), Math.abs(false), Math.sqrt(null), String.fromCharCode("65", true)); + +const parsed = JSON.parse('["12", " 0x10 ", "", "1e400", true, null, {}, [], [7]]'); +for (const value of parsed) console.log(JSON.stringify(value), isNaN(value), isFinite(value), Math.trunc(value)); + +let calls = ""; +const boxed = { valueOf: () => ((calls += "v"), 4.5), toString: () => ((calls += "s"), "9") }; +console.log(isFinite(boxed), isNaN(boxed), Math.round(boxed), calls); + +class LruCache { + constructor(config) { + this.maxAge = config.maxAge; + } + hasMaxAge() { + return isFinite(this.maxAge); + } +} +console.log(new LruCache({ maxAge: 1000 }).hasMaxAge(), new LruCache({}).hasMaxAge()); + +for (const value of [Symbol("s"), 10n]) { + try { + isFinite(value); + } catch (e) { + console.log(e instanceof TypeError, e.message); + } +} From 349ab75298a9c7534b5b8f084645a9824eb0a5b9 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Wed, 21 Oct 2026 12:50:14 -0500 Subject: [PATCH 2/3] Evaluate every ToNumber argument before converting, and convert conditionals The Math functions and String.fromCharCode/fromCodePoint evaluate all of their arguments before they convert any, as in Node. When an argument's conversion can run program code (valueOf or toString on a dynamic value) and a later argument follows, the arguments now evaluate into temporaries first and convert in order after (lowerToNumberArguments). Before, a dynamic argument converted before the next one was evaluated. A conditional argument whose branches are not both numbers boxes its value and converts it like any dynamic value, so `isFinite(flag ? "12" : "abc")` in a JavaScript file returns a boolean instead of reaching a run-time fence. A statement inside an expression can end its block (an argument call that always throws). The expression's later statements and result now emit in a block with no predecessors, so the instructions there read only values defined in it. --- .../src/backend/llvm/expr-primitives.ts | 11 +- .../src/frontend/lowering/builtins/strings.ts | 18 ++- .../src/frontend/lowering/lower-exprs.ts | 106 +++++++++++++++--- .../src/frontend/lowering/lower-stdlib.ts | 26 +++-- tests/corpus/6906-tonumber-arguments.mjs | 45 ++++++++ 5 files changed, 174 insertions(+), 32 deletions(-) diff --git a/packages/compiler/src/backend/llvm/expr-primitives.ts b/packages/compiler/src/backend/llvm/expr-primitives.ts index 69f4def2e..3ab628d69 100644 --- a/packages/compiler/src/backend/llvm/expr-primitives.ts +++ b/packages/compiler/src/backend/llvm/expr-primitives.ts @@ -524,8 +524,15 @@ export function emitOperatorExpr( case "seqExpr": { // Statements mid-expression: each emits in place (its own frame, // exactly statement position); the result is an ordinary temp of - // the current frame. No jump can leave the expression's region. - for (const s of e.stmts) host.emitStmt(s); + // the current frame. No jump can leave the expression's region. A + // statement can still end the block (a call that always throws): + // what follows then emits in a block with no predecessors, so every + // value it reads is defined there, and LLVM deletes it. + for (const s of e.stmts) { + if (host.B.isTerminated()) host.B.startBlock(host.B.newLabel("seq.dead")); + host.emitStmt(s); + } + if (host.B.isTerminated()) host.B.startBlock(host.B.newLabel("seq.dead")); return host.emitExpr(e.result); } default: { diff --git a/packages/compiler/src/frontend/lowering/builtins/strings.ts b/packages/compiler/src/frontend/lowering/builtins/strings.ts index 54ab22c05..4b4a8b29b 100644 --- a/packages/compiler/src/frontend/lowering/builtins/strings.ts +++ b/packages/compiler/src/frontend/lowering/builtins/strings.ts @@ -3,7 +3,7 @@ import * as ts from "../../ts7/adapter.js"; import { type Lowerer } from "../lowerer.js"; import { locOf } from "../../program.js"; import { lowerOptionalArgument, lowerStringSearchArgument } from "../optional-arguments.js"; -import { lowerToNumberArgument } from "../lower-exprs.js"; +import { lowerToNumberArguments } from "../lower-exprs.js"; import { BOOL, DYN, @@ -115,11 +115,19 @@ export function lowerStringStaticCall( const packed = lowerer.lowerExprExpecting(spread.expression, arrayOf(F64)); return { kind: "libCall", fn: `string.${member}`, args: [packed], type: STRING, loc }; } - // Each code converts with ToNumber: a missing value is NaN (code unit 0 - // for fromCharCode, a RangeError for fromCodePoint), exactly Node. - const elems = call.arguments.nodes.map((a) => lowerToNumberArgument(lowerer, a)); + // Each code converts with ToNumber after every argument is evaluated: a + // missing value is NaN (code unit 0 for fromCharCode, a RangeError for + // fromCodePoint), exactly Node. + const { stmts, values: elems } = lowerToNumberArguments(lowerer, call.arguments.nodes); const packed: IrExpr = { kind: "arrayLit", elems, type: arrayOf(F64), loc }; - return { kind: "libCall", fn: `string.${member}`, args: [packed], type: STRING, loc }; + const result: IrExpr = { + kind: "libCall", + fn: `string.${member}`, + args: [packed], + type: STRING, + loc, + }; + return stmts.length === 0 ? result : { kind: "seqExpr", stmts, result, type: STRING, loc }; } /** `String.fromCharCode.apply(thisArg, codes)`, the btoa chunking idiom diff --git a/packages/compiler/src/frontend/lowering/lower-exprs.ts b/packages/compiler/src/frontend/lowering/lower-exprs.ts index 4b03e915d..23892d43e 100644 --- a/packages/compiler/src/frontend/lowering/lower-exprs.ts +++ b/packages/compiler/src/frontend/lowering/lower-exprs.ts @@ -1420,9 +1420,65 @@ export function lowerOptionalNumber( * out-of-range read, a local or field holding one, or a hole — converts to * NaN instead of trusting the checker's number type. */ export function lowerToNumberArgument(lowerer: Lowerer, node: ts.Expression): IrExpr { + const operand = numberOperand(lowerer, node); + return operand.convert ? operand.convert(operand.value) : operand.value; +} + +/** The ToNumber arguments of one call (the Math functions, + * String.fromCharCode): Node evaluates every argument before it converts + * any. When an argument's conversion can run program code (valueOf or + * toString on a dynamic value) and a later argument follows, the arguments + * evaluate into temporaries first and convert in order after. `lowerOther` + * lowers an argument that takes no conversion (a spread), or returns null + * for a ToNumber argument. Returns the temporaries' statements (empty when + * none are needed) and the converted values. */ +export function lowerToNumberArguments( + lowerer: Lowerer, + nodes: readonly ts.Expression[], + lowerOther?: (node: ts.Expression) => IrExpr | null, +): { stmts: IrStmt[]; values: IrExpr[] } { + const operands = nodes.map((node): NumberOperand => { + const other = lowerOther?.(node); + return other ? { value: other, convert: null, runsCode: false } : numberOperand(lowerer, node); + }); + const late = operands.slice(0, -1).some((operand) => operand.runsCode); + if (!late) { + const values = operands.map((o) => (o.convert ? o.convert(o.value) : o.value)); + return { stmts: [], values }; + } + const stmts: IrStmt[] = []; + const values = operands.map((operand) => { + if (operand.value.kind === "numLit") return operand.value; + const temp = lowerer.declareHiddenLocal("%numberArgument", operand.value.type); + const loc = operand.value.loc; + stmts.push({ kind: "varDecl", localId: temp.id, init: operand.value, loc }); + const read = varRef(temp.id, operand.value.type, loc); + return operand.convert ? operand.convert(read) : read; + }); + return { stmts, values }; +} + +/** An argument a ToNumber site lowers: its evaluated value and the + * conversion that makes it a number (null when the value already is one). + * `runsCode`: the conversion can call program code. */ +interface NumberOperand { + value: IrExpr; + convert: ((value: IrExpr) => IrExpr) | null; + runsCode: boolean; +} + +function numberOperand(lowerer: Lowerer, node: ts.Expression): NumberOperand { let inner = node; while (ts.isParenthesizedExpression(inner)) inner = inner.expression; const loc = locOf(inner); + const number = (value: IrExpr): NumberOperand => ({ value, convert: null, runsCode: false }); + const dynToNumber = (value: IrExpr): IrExpr => ({ + kind: "libCall", + fn: "dyn.toNumberCoerce", + args: [value], + type: F64, + loc, + }); const isOptionalNumber = (value: IrExpr): boolean => value.type.kind === "union" && lowerer.armTag(value.type.unionId, UNDEFINED_T) >= 0 && @@ -1433,34 +1489,54 @@ export function lowerToNumberArgument(lowerer: Lowerer, node: ts.Expression): Ir ? lowerAbsenceProbe(lowerer, inner) : null; if (optional && isOptionalNumber(optional)) - return lowerOptionalNumber(lowerer, optional, loc, inner); - // A conditional builds each branch for the destination. - if (ts.isConditionalExpression(inner)) return lowerer.lowerExprExpecting(node, F64); + return number(lowerOptionalNumber(lowerer, optional, loc, inner)); + // A conditional with number branches builds each branch as a number; + // other branches box, and the value converts like any dynamic value. + if (ts.isConditionalExpression(inner)) { + const branch = (b: ts.Expression) => lowerer.mapTypeOf(lowerer.typeOf(b))?.kind; + if (branch(inner.whenTrue) === "f64" && branch(inner.whenFalse) === "f64") + return number(lowerer.lowerExprExpecting(node, F64)); + return { value: lowerer.lowerExprExpecting(node, DYN), convert: dynToNumber, runsCode: true }; + } const value = lowerer.lowerExpr(node); - if (isOptionalNumber(value)) return lowerOptionalNumber(lowerer, value, loc, inner); + if (isOptionalNumber(value)) return number(lowerOptionalNumber(lowerer, value, loc, inner)); // Other values convert with ToNumber: strings parse as numeric literals, // booleans and null are numbers, and checked-dynamic values call // valueOf/toString or throw Node's TypeError for a Symbol or BigInt. - const num = (value: number): IrExpr => ({ kind: "numLit", value, type: F64, loc }); + const num = (n: number): IrExpr => ({ kind: "numLit", value: n, type: F64, loc }); + const pure = (convert: (value: IrExpr) => IrExpr): NumberOperand => ({ + value, + convert, + runsCode: false, + }); switch (value.type.kind) { case "string": - return { kind: "libCall", fn: "num.fromString", args: [value], type: F64, loc }; + return pure((v) => ({ kind: "libCall", fn: "num.fromString", args: [v], type: F64, loc })); case "bool": - return { kind: "ternary", cond: value, then: num(1), else_: num(0), type: F64, loc }; + return pure((v) => ({ + kind: "ternary", + cond: v, + then: num(1), + else_: num(0), + type: F64, + loc, + })); case "nullT": - return defaultAfterUndefined(value, num(0)); + return pure((v) => defaultAfterUndefined(v, num(0))); case "undefinedT": - return defaultAfterUndefined(value, num(NaN)); + return pure((v) => defaultAfterUndefined(v, num(NaN))); case "dyn": - return { kind: "libCall", fn: "dyn.toNumberCoerce", args: [value], type: F64, loc }; + return { value, convert: dynToNumber, runsCode: true }; case "union": - if (lowerer.dynConvertible(value.type)) { - const boxed = lowerer.coerceToExpected(value, DYN); - return { kind: "libCall", fn: "dyn.toNumberCoerce", args: [boxed], type: F64, loc }; - } + if (lowerer.dynConvertible(value.type)) + return { + value: lowerer.coerceToExpected(value, DYN), + convert: dynToNumber, + runsCode: true, + }; break; } - return lowerer.coerceInto(node, value, F64); + return number(lowerer.coerceInto(node, value, F64)); } /** The stored value of an identifier whose storage holds an undefined arm diff --git a/packages/compiler/src/frontend/lowering/lower-stdlib.ts b/packages/compiler/src/frontend/lowering/lower-stdlib.ts index e968af638..2ad3657c7 100644 --- a/packages/compiler/src/frontend/lowering/lower-stdlib.ts +++ b/packages/compiler/src/frontend/lowering/lower-stdlib.ts @@ -2,7 +2,7 @@ import { dynUndefinedExpr, nodeThrowExpr } from "../../ir/build.js"; import { InternalCompilerError } from "../../errors.js"; import * as ts from "../ts7/adapter.js"; -import { lowerToNumberArgument } from "./lower-exprs.js"; +import { lowerToNumberArguments } from "./lower-exprs.js"; import type { Lowerer } from "./lowerer.js"; import { isDeclaredInAmbientNamespace } from "../type-mapper.js"; import { @@ -2415,8 +2415,14 @@ export function lowerStdlibMethodCall( const staticMath = isMath ? own(STATIC_MATH_FNS, name) : undefined; // A missing element (a hole bound by a loop, a local or field holding a // missing read, or the read itself) converts to NaN, ToNumber's answer - // for undefined, instead of trusting the checker's number type. - const mathArg = (a: ts.Expression): IrExpr => lowerToNumberArgument(lowerer, a); + // for undefined, instead of trusting the checker's number type. The + // arguments are all evaluated before any converts (lowerToNumberArguments). + const mathArgs = ( + nodes: readonly ts.Expression[], + lowerOther?: (node: ts.Expression) => IrExpr | null, + ) => lowerToNumberArguments(lowerer, nodes, lowerOther); + const afterArguments = (stmts: IrStmt[], result: IrExpr): IrExpr => + stmts.length === 0 ? result : { kind: "seqExpr", stmts, result, type: result.type, loc }; // Math.hypot, and Math.max/Math.min with a spread among other arguments // (`Math.max(...xs, 5)`): the arguments in order, spreads flattened, as // one number[] the runtime folds. @@ -2427,8 +2433,8 @@ export function lowerStdlibMethodCall( call.arguments.nodes.length > 1 && call.arguments.nodes.some(ts.isSpreadElement))) ) { - const elems = call.arguments.nodes.map((a) => - ts.isSpreadElement(a) ? lowerer.lowerExprExpecting(a.expression, arrayOf(F64)) : mathArg(a), + const { stmts, values: elems } = mathArgs(call.arguments.nodes, (a) => + ts.isSpreadElement(a) ? lowerer.lowerExprExpecting(a.expression, arrayOf(F64)) : null, ); const spreads = call.arguments.nodes.flatMap((a, i) => (ts.isSpreadElement(a) ? [i] : [])); const packed: IrExpr = { @@ -2439,7 +2445,7 @@ export function lowerStdlibMethodCall( loc, }; const fn = name === "max" ? "math.maxArr" : name === "min" ? "math.minArr" : staticMath.fn; - return { kind: "libCall", fn, args: [packed], type: F64, loc }; + return afterArguments(stmts, { kind: "libCall", fn, args: [packed], type: F64, loc }); } if (staticMath && call.arguments.nodes.every((a) => !ts.isSpreadElement(a))) { // Math.max/Math.min at ANY plain arity — Node's are variadic. The @@ -2455,16 +2461,16 @@ export function lowerStdlibMethodCall( if (call.arguments.nodes.length === 0) { return { kind: "numLit", value: name === "max" ? -Infinity : Infinity, type: F64, loc }; } - const args = call.arguments.nodes.map(mathArg); + const { stmts, values: args } = mathArgs(call.arguments.nodes); let acc = args[0]!; for (let i = 1; i < args.length; i++) { acc = { kind: "libCall", fn: staticMath.fn, args: [acc, args[i]!], type: F64, loc }; } - return acc; + return afterArguments(stmts, acc); } if (call.arguments.nodes.length === staticMath.arity) { - const args = call.arguments.nodes.map(mathArg); - return { kind: "libCall", fn: staticMath.fn, args, type: F64, loc }; + const { stmts, values: args } = mathArgs(call.arguments.nodes); + return afterArguments(stmts, { kind: "libCall", fn: staticMath.fn, args, type: F64, loc }); } } // `Math.max(...xs)` / `Math.min(...xs)` over a number[]: a STATIC diff --git a/tests/corpus/6906-tonumber-arguments.mjs b/tests/corpus/6906-tonumber-arguments.mjs index 23b150eb8..55da6bb1f 100644 --- a/tests/corpus/6906-tonumber-arguments.mjs +++ b/tests/corpus/6906-tonumber-arguments.mjs @@ -30,3 +30,48 @@ for (const value of [Symbol("s"), 10n]) { console.log(e instanceof TypeError, e.message); } } + +// A conditional argument converts whichever branch it takes. +for (const flag of [true, false]) { + console.log(isFinite(flag ? "12" : "abc"), isNaN(flag ? null : "x"), Math.floor(flag ? "3.7" : true)); +} + +// Every argument is evaluated before any converts, as in Node: valueOf runs +// after the last argument, and an argument that throws stops the call +// before any conversion. +let order = ""; +const valued = (name, n) => ({ valueOf: () => ((order += "v" + name), n) }); +const arg = (name, n) => ((order += "e" + name), valued(name, n)); +console.log(Math.max(arg("a", 1), arg("b", 5), arg("c", 3)), order); +order = ""; +console.log(Math.pow(arg("a", 2), arg("b", 10)), order); +order = ""; +console.log(Math.hypot(arg("a", 3), ...[4], arg("c", 12)), order); +order = ""; +console.log(String.fromCharCode(arg("a", 72), arg("b", 105)), order); +order = ""; +try { + Math.max( + arg("a", 1), + (() => { + order += "throw"; + throw new Error("boom"); + })(), + ); +} catch (e) { + console.log(e.message, order); +} +order = ""; +try { + Math.min( + { + valueOf() { + order += "x"; + throw new TypeError("first"); + }, + }, + arg("b", 2), + ); +} catch (e) { + console.log(e.message, order); +} From bbb37ae03ba82335a66da0b848ec2ff4682f3473 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Wed, 21 Oct 2026 16:34:37 -0500 Subject: [PATCH 3/3] Read a spread's elements before an earlier argument converts When the Math functions evaluate their arguments into temporaries before converting (an earlier argument whose ToNumber can run valueOf), a spread argument's array is now copied where it is evaluated. Before, the temporary held the array itself and its elements were read after the conversions, so a valueOf that changed the array changed the values passed, while JavaScript reads a spread's elements during argument evaluation. --- .../src/frontend/lowering/lower-exprs.ts | 30 +++++++++++++++---- tests/corpus/6906-tonumber-arguments.mjs | 20 +++++++++++++ 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/packages/compiler/src/frontend/lowering/lower-exprs.ts b/packages/compiler/src/frontend/lowering/lower-exprs.ts index 23892d43e..e7a1b79a8 100644 --- a/packages/compiler/src/frontend/lowering/lower-exprs.ts +++ b/packages/compiler/src/frontend/lowering/lower-exprs.ts @@ -1429,9 +1429,12 @@ export function lowerToNumberArgument(lowerer: Lowerer, node: ts.Expression): Ir * any. When an argument's conversion can run program code (valueOf or * toString on a dynamic value) and a later argument follows, the arguments * evaluate into temporaries first and convert in order after. `lowerOther` - * lowers an argument that takes no conversion (a spread), or returns null - * for a ToNumber argument. Returns the temporaries' statements (empty when - * none are needed) and the converted values. */ + * lowers an argument that takes no conversion (a spread's array), or returns + * null for a ToNumber argument; with temporaries, such an array is copied + * where it is evaluated, since JavaScript reads a spread's elements then, + * before a later conversion's valueOf could change them. Returns the + * temporaries' statements (empty when none are needed) and the converted + * values. */ export function lowerToNumberArguments( lowerer: Lowerer, nodes: readonly ts.Expression[], @@ -1439,7 +1442,9 @@ export function lowerToNumberArguments( ): { stmts: IrStmt[]; values: IrExpr[] } { const operands = nodes.map((node): NumberOperand => { const other = lowerOther?.(node); - return other ? { value: other, convert: null, runsCode: false } : numberOperand(lowerer, node); + return other + ? { value: other, convert: null, runsCode: false, spread: true } + : numberOperand(lowerer, node); }); const late = operands.slice(0, -1).some((operand) => operand.runsCode); if (!late) { @@ -1451,7 +1456,18 @@ export function lowerToNumberArguments( if (operand.value.kind === "numLit") return operand.value; const temp = lowerer.declareHiddenLocal("%numberArgument", operand.value.type); const loc = operand.value.loc; - stmts.push({ kind: "varDecl", localId: temp.id, init: operand.value, loc }); + const init: IrExpr = + operand.spread && operand.value.type.kind === "array" + ? { + kind: "arrIntrinsic", + method: "slice", + receiver: operand.value, + args: [numLit(0, loc), numLit(Infinity, loc)], + type: operand.value.type, + loc, + } + : operand.value; + stmts.push({ kind: "varDecl", localId: temp.id, init, loc }); const read = varRef(temp.id, operand.value.type, loc); return operand.convert ? operand.convert(read) : read; }); @@ -1460,11 +1476,13 @@ export function lowerToNumberArguments( /** An argument a ToNumber site lowers: its evaluated value and the * conversion that makes it a number (null when the value already is one). - * `runsCode`: the conversion can call program code. */ + * `runsCode`: the conversion can call program code. `spread`: the value is + * a spread's array (lowerToNumberArguments' `lowerOther`). */ interface NumberOperand { value: IrExpr; convert: ((value: IrExpr) => IrExpr) | null; runsCode: boolean; + spread?: boolean; } function numberOperand(lowerer: Lowerer, node: ts.Expression): NumberOperand { diff --git a/tests/corpus/6906-tonumber-arguments.mjs b/tests/corpus/6906-tonumber-arguments.mjs index 55da6bb1f..35589d07f 100644 --- a/tests/corpus/6906-tonumber-arguments.mjs +++ b/tests/corpus/6906-tonumber-arguments.mjs @@ -75,3 +75,23 @@ try { } catch (e) { console.log(e.message, order); } + +// A spread reads its elements where it is evaluated, before an earlier +// argument's valueOf runs. +const values = [1, 2]; +const mutator = { + valueOf() { + values[0] = 100; + values.push(50); + return 3; + }, +}; +console.log(Math.max(mutator, ...values), values.join(",")); +const more = [4]; +const resetter = { + valueOf() { + more[0] = -1; + return 0; + }, +}; +console.log(Math.hypot(resetter, ...more, 3), more.join(","));