From 84b3d1f475fa17bb3f28365afdd142d7df6cbe61 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Fri, 30 Oct 2026 10:32:40 -0500 Subject: [PATCH] Warn for NaN dynamic and optional element delays and throw on BigInt delays A delay read through bracket access skipped TimeoutNaNWarning, so that an element read past the end, undefined in Node and NaN here, warns nothing. That only holds for a delay typed number: an untyped (checked-dynamic) delay tests undefined before it converts, and a `number | undefined` delay warns only in its defined arm, so a NaN in either now warns, as Node's Timeout constructor does. An untyped delay converted with Number(), so a BigInt delay scheduled a timer. It now converts as Node's `after *= 1` does, which throws "Cannot mix BigInt and other types, use explicit conversions" from setTimeout and setInterval. setTimeout and setInterval called as stored function values convert their delay the same way, and now also emit the Timeout warnings. Corpus timers-untyped-delay-reads.cjs (BigInt delays, direct and through function values; NaN and past-the-end element reads of an untyped array; a negative delay through a stored setTimeout) and timers-optional-delay-reads.ts (number | undefined element reads). timer-delay-lowering.test.ts pinned the delay as the plain number the call reads; it now expects the timers.userDelay check around a number variable and around the defined arm of a `number | undefined` delay. --- .../src/frontend/lowering/lower-timers.ts | 100 +++++++++++++----- .../test/timer-delay-lowering.test.ts | 23 +++- .../timers-optional-delay-reads.ts.json | 6 ++ .../timers-untyped-delay-reads.cjs.json | 6 ++ packages/runtime/src/scr_dyn_invoke.c | 27 ++++- tests/corpus/timers-optional-delay-reads.ts | 14 +++ tests/corpus/timers-untyped-delay-reads.cjs | 28 +++++ .../timers-optional-delay-reads.ts.json | 1 + .../timers-untyped-delay-reads.cjs.json | 1 + 9 files changed, 174 insertions(+), 32 deletions(-) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/timers-optional-delay-reads.ts.json create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/timers-untyped-delay-reads.cjs.json create mode 100644 tests/corpus/timers-optional-delay-reads.ts create mode 100644 tests/corpus/timers-untyped-delay-reads.cjs create mode 100644 tests/harness/corpus-costs/timers-optional-delay-reads.ts.json create mode 100644 tests/harness/corpus-costs/timers-untyped-delay-reads.cjs.json 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 }