diff --git a/packages/compiler/src/frontend/lowering/lower-timers.ts b/packages/compiler/src/frontend/lowering/lower-timers.ts index a5d2ca90c0..445b49d3ae 100644 --- a/packages/compiler/src/frontend/lowering/lower-timers.ts +++ b/packages/compiler/src/frontend/lowering/lower-timers.ts @@ -8,8 +8,10 @@ import { DYN, F64, isUnitType, + STRING, VOID, type IrExpr, + type IrStmt, type IrType, type SrcLoc, typeKey, @@ -396,6 +398,7 @@ function lowerTimerCall( // An omitted delay is NaN, Node's undefined: the loop clamps it to // 1ms, and mock.timers queues it to run at the next tick. let warned = false; + let throwingDelay = false; // An element read past the end is undefined in Node, which warns // nothing; here it reads as NaN, so the runtime skips the NaN warning. let delayNode = expr.arguments.nodes[1]; @@ -408,34 +411,42 @@ function lowerTimerCall( expr.arguments.nodes.length >= 2 ? lowerer.lowerExpr(expr.arguments.nodes[1]!) : { kind: "numLit", value: NaN, type: F64, loc }; - // A delay that may be a boolean (execa's `forceKillAfterTimeout = true` - // default flowing on to setTimeout): Node converts it with ToNumber - // (`after *= 1`). - if (ms.type.kind === "bool" || hasBoolArm(lowerer, ms.type)) { - ms = { - kind: "libCall", - fn: "dyn.numberConstructor", - args: [lowerer.coerceToExpected(ms, DYN)], - type: F64, - loc, - }; - } else if (ms.type.kind === "union") { + if (ms.type.kind === "union" && !hasBoolArm(lowerer, ms.type)) { // A delay that may be undefined: an optional `ms?: number` passed on, // or an element read (`delays[0]`) whose storage keeps the // out-of-range undefined. Node converts the delay with ToNumber - // (`after *= 1`), so undefined is NaN, the omitted delay. Other - // unions stay as they are, and the lib boundary refuses them. + // (`after *= 1`), so undefined is NaN, the omitted delay. ms = lowerOptionalNumber(lowerer, ms, loc, expr.arguments.nodes[1]); - warned = warnOnDefinedDelay(ms, delayKind, loc); - } else if (ms.type.kind === "dyn") { - // An untyped delay converts with ToNumber; undefined stays the - // omitted delay (Node's `after === undefined` branch). + // The defined arm is never the undefined, so an element read's NaN + // there warns. + if (ms.type.kind === "f64") warned = warnOnDefinedDelay(ms, delayKind & 3, loc); + } + // Any other delay that is not a number (a boolean such as execa's + // `forceKillAfterTimeout = true` default, a BigInt, a string, another + // union) converts through the checked-dynamic path below. + if (ms.type.kind !== "f64" && ms.type.kind !== "dyn" && lowerer.dynConvertible(ms.type)) { + ms = lowerer.coerceToExpected(ms, DYN); + } + if (ms.type.kind === "dyn") { + // An untyped delay converts as Node's `after *= 1` does: ToNumber, + // with the TypeError a BigInt throws there. Undefined stays the + // omitted delay (Node's `after === undefined` branch), tested before + // the conversion, so a NaN after it warns, element reads included. const tmp = lowerer.declareHiddenLocal("%timerDelay", DYN); const stable: IrExpr = { kind: "varRef", localId: tmp.id, type: DYN, loc }; const converted: IrExpr = { - kind: "libCall", - fn: "dyn.numberConstructor", - args: [stable], + kind: "dynCheck", + value: { + kind: "libCall", + fn: "dyn.arithmetic", + args: [ + stable, + { kind: "dynFrom", value: numLit(1, loc), type: DYN, loc }, + { kind: "strLit", value: "*", type: STRING, loc }, + ], + type: DYN, + loc, + }, type: F64, loc, }; @@ -446,7 +457,7 @@ function lowerTimerCall( kind: "ternary", cond: { kind: "dynTest", test: "undefined", value: stable, type: BOOL, loc }, then: { kind: "numLit", value: NaN, type: F64, loc }, - else_: userDelay(converted, delayKind, loc), + else_: userDelay(converted, delayKind & 3, loc), type: F64, loc, }, @@ -454,6 +465,7 @@ function lowerTimerCall( loc, }; warned = true; + throwingDelay = true; } // A delay Node sets to 1 emits its Timeout warning; a literal delay in // range needs no check, and an omitted delay is undefined, which warns @@ -466,8 +478,34 @@ function lowerTimerCall( ) { ms = userDelay(ms, delayKind, loc); } + // A delay conversion that can throw (a BigInt) runs after the callback + // is built: the callback and the converted delay wait in hidden locals, + // so a throw leaves only the callback's local, which the frame + // releases. + let callback = cb; + const callbackFirst: IrStmt[] = []; + if (throwingDelay) { + const held = lowerer.declareHiddenLocal("%timerCallback", cb.type); + const delay = lowerer.declareHiddenLocal("%timerDelayMs", F64); + callbackFirst.push( + { kind: "varDecl", localId: held.id, init: cb, loc }, + { kind: "varDecl", localId: delay.id, init: ms, loc }, + ); + callback = varRef(held.id, cb.type, loc); + ms = varRef(delay.id, F64, loc); + } + const scheduled = (call: IrExpr): IrExpr => + callbackFirst.length === 0 + ? call + : { kind: "seqExpr", stmts: callbackFirst, result: call, type: call.type, loc }; if (member === "setInterval") { - return { kind: "libCall", fn: "timers.setInterval", args: [cb, ms], type: F64, loc }; + return scheduled({ + kind: "libCall", + fn: "timers.setInterval", + args: [callback, ms], + type: F64, + loc, + }); } // The use position decides the shape: a Timeout handle (mapped to // f64) when the call is USED (assigned, `.unref()`d, cleared) — the @@ -476,9 +514,21 @@ function lowerTimerCall( // the same heap; only the handle form can be unref'd/cleared. const resultT = lowerer.mapTypeOf(lowerer.typeOf(expr)); if (resultT?.kind === "f64" && !ts.isExpressionStatement(expr.parent)) { - return { kind: "libCall", fn: "timers.setTimeoutHandle", args: [cb, ms], type: F64, loc }; + return scheduled({ + kind: "libCall", + fn: "timers.setTimeoutHandle", + args: [callback, ms], + type: F64, + loc, + }); } - return { kind: "libCall", fn: "timers.setTimeout", args: [cb, ms], type: VOID, loc }; + return scheduled({ + kind: "libCall", + fn: "timers.setTimeout", + args: [callback, ms], + type: VOID, + loc, + }); } // clearTimeout(handle): shares the interval clear (the handle ids // share one space). An optional handle clears nothing when absent. diff --git a/packages/compiler/test/timer-delay-lowering.test.ts b/packages/compiler/test/timer-delay-lowering.test.ts index e4219c3c2b..deff90e25d 100644 --- a/packages/compiler/test/timer-delay-lowering.test.ts +++ b/packages/compiler/test/timer-delay-lowering.test.ts @@ -50,8 +50,20 @@ test("number delays pass to the timer unchanged", async () => { export {}; `, ); - expect(delays.map((delay) => delay.kind)).toEqual(["varRef", "numLit", "varRef"]); + // A literal delay in range passes as it is. A number variable passes + // through timers.userDelay, which emits Node's Timeout warning when the + // timer sets the delay to 1; its second argument is 0 for setTimeout and 1 + // for setInterval. + expect(delays.map((delay) => delay.kind)).toEqual(["libCall", "numLit", "libCall"]); for (const delay of delays) expect(delay.type).toEqual({ kind: "f64" }); + expect(delays[0]).toMatchObject({ + fn: "timers.userDelay", + args: [{ kind: "varRef" }, { kind: "numLit", value: 0 }], + }); + expect(delays[2]).toMatchObject({ + fn: "timers.userDelay", + args: [{ kind: "varRef" }, { kind: "numLit", value: 1 }], + }); }); test("a number | undefined delay converts with ToNumber", async () => { @@ -67,13 +79,18 @@ test("a number | undefined delay converts with ToNumber", async () => { ); expect(delays).toHaveLength(2); for (const delay of delays) { - // undefined is NaN, the omitted delay; a number passes through. + // undefined is NaN, the omitted delay, and warns nothing; a number + // passes through timers.userDelay. expect(delay).toMatchObject({ kind: "ternary", type: { kind: "f64" }, cond: { kind: "unionIsTag" }, then: { kind: "bin", op: "/" }, - else_: { kind: "unionNarrow", type: { kind: "f64" } }, + else_: { + kind: "libCall", + fn: "timers.userDelay", + args: [{ kind: "unionNarrow", type: { kind: "f64" } }, { kind: "numLit" }], + }, }); } }); diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/timers-optional-delay-reads.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/timers-optional-delay-reads.ts.json new file mode 100644 index 0000000000..a0aa195489 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/timers-optional-delay-reads.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/timers-optional-delay-reads.ts" + ], + "diags": [] +} diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/timers-untyped-delay-reads.cjs.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/timers-untyped-delay-reads.cjs.json new file mode 100644 index 0000000000..cb1ca3aa69 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/timers-untyped-delay-reads.cjs.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/timers-untyped-delay-reads.cjs" + ], + "diags": [] +} diff --git a/packages/runtime/src/scr_dyn_invoke.c b/packages/runtime/src/scr_dyn_invoke.c index 8ac5bfc2de..dc499bf336 100644 --- a/packages/runtime/src/scr_dyn_invoke.c +++ b/packages/runtime/src/scr_dyn_invoke.c @@ -2337,10 +2337,29 @@ static ScrDyn *dyn_builtin_fn_call(ScrClosure *clo, ScrDyn *const *args, size_t ScrDyn *result = NULL; bool timeout = strcmp(name->data, dyn_s16) == 0; if (timeout || strcmp(name->data, dyn_s15) == 0) { - double ms = 0; - if (dyn_builtin_fn_callback(args, argc) && (argc < 2 || scr_dyn_number_coerce_js(args[1], &ms))) { - ScrClosure *c = dyn_builtin_fn_deferred(args, argc, 2); - result = scr_dyn_new_num(timeout ? scr_set_timeout_handle(c, ms) : scr_set_interval(c, ms)); + /* Node validates the callback, then its Timeout constructor takes an + * undefined delay as the default 1ms, converts another as `after *= 1` + * does (a BigInt throws its TypeError) and warns when it sets the + * delay to 1. */ + if (dyn_builtin_fn_callback(args, argc)) { + double ms = NAN; + bool converted = true; + if (argc >= 2 && args[1]->kind != SCR_DYN_UNDEF) { + ScrDyn *one = scr_dyn_new_num(1); + ScrStr *times = scr_str_new("*", 1); + ScrDyn *product = scr_dyn_arithmetic(args[1], one, times); + scr_str_release(times); + scr_dyn_release(one); + converted = product != NULL; + if (product) { + ms = scr_timer_user_delay(product->v.num, timeout ? 0 : 1); + scr_dyn_release(product); + } + } + if (converted) { + ScrClosure *c = dyn_builtin_fn_deferred(args, argc, 2); + result = scr_dyn_new_num(timeout ? scr_set_timeout_handle(c, ms) : scr_set_interval(c, ms)); + } } } else if (strcmp(name->data, dyn_s13) == 0) { if (dyn_builtin_fn_callback(args, argc)) diff --git a/tests/corpus/timers-optional-delay-reads.ts b/tests/corpus/timers-optional-delay-reads.ts new file mode 100644 index 0000000000..ba510a856e --- /dev/null +++ b/tests/corpus/timers-optional-delay-reads.ts @@ -0,0 +1,14 @@ +// @normalize-pid +// A delay typed `number | undefined` read from an array: an element past +// the end is undefined, the default delay, and warns nothing, and an +// element that holds NaN warns once, as Node's Timeout constructor does. +const delays: (number | undefined)[] = [NaN, undefined, 1]; +const plain: number[] = [5]; +function at(index: number): number | undefined { + return delays[index]; +} +setTimeout(() => console.log("undefined element"), delays[1]); +setTimeout(() => console.log("past the end"), delays[9]); +setTimeout(() => console.log("typed past the end"), plain[3]); +setTimeout(() => console.log("nan element"), delays[0]); +setTimeout(() => console.log("nan through a call"), at(0)); diff --git a/tests/corpus/timers-untyped-delay-reads.cjs b/tests/corpus/timers-untyped-delay-reads.cjs new file mode 100644 index 0000000000..0acbee5b9b --- /dev/null +++ b/tests/corpus/timers-untyped-delay-reads.cjs @@ -0,0 +1,28 @@ +// @normalize-pid +'use strict'; +// An untyped delay converts as Node's `after *= 1` does: a BigInt delay +// throws its TypeError from setTimeout and setInterval, called directly or +// as stored function values, and a delay read from an untyped array that +// converts to NaN warns, while an element read past the end is undefined, +// the default delay, and warns nothing. A stored function value warns as +// the direct call does. +const big = [1n]; +try { + setTimeout(() => console.log('not reached'), big[0]); +} catch (error) { + console.log('direct', error.name, error.message); +} +for (const schedule of [setTimeout, setInterval]) { + try { + schedule(() => console.log('not reached'), 1n); + console.log('no throw'); + } catch (error) { + console.log(error.name, error.message); + } +} +const delays = [{}, 'x', 1]; +setTimeout(() => console.log('past the end'), delays[9]); +setTimeout(() => console.log('object element'), delays[0]); +setTimeout(() => console.log('string element'), delays[1]); +const stored = [setTimeout][0]; +stored(() => console.log('stored negative'), -5); diff --git a/tests/harness/corpus-costs/timers-optional-delay-reads.ts.json b/tests/harness/corpus-costs/timers-optional-delay-reads.ts.json new file mode 100644 index 0000000000..cbfa5de811 --- /dev/null +++ b/tests/harness/corpus-costs/timers-optional-delay-reads.ts.json @@ -0,0 +1 @@ +{ "plain": 3200, "san": 5300 } diff --git a/tests/harness/corpus-costs/timers-untyped-delay-reads.cjs.json b/tests/harness/corpus-costs/timers-untyped-delay-reads.cjs.json new file mode 100644 index 0000000000..d5fcc27e98 --- /dev/null +++ b/tests/harness/corpus-costs/timers-untyped-delay-reads.cjs.json @@ -0,0 +1 @@ +{ "plain": 3300, "san": 5400 }