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.

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.

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" }],
+      },
     });
   }
 });

packages/compiler/test/ts7/baselines/order-parity/tests/corpus/timers-optional-delay-reads.ts.json

@@ -0,0 +1,6 @@
+{
+ "order": [
+  "<repo>/tests/corpus/timers-optional-delay-reads.ts"
+ ],
+ "diags": []
+}

packages/compiler/test/ts7/baselines/order-parity/tests/corpus/timers-untyped-delay-reads.cjs.json

@@ -0,0 +1,6 @@
+{
+ "order": [
+  "<repo>/tests/corpus/timers-untyped-delay-reads.cjs"
+ ],
+ "diags": []
+}

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))

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));

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);

tests/harness/corpus-costs/timers-optional-delay-reads.ts.json

@@ -0,0 +1 @@
+{ "plain": 3200, "san": 5300 }

tests/harness/corpus-costs/timers-untyped-delay-reads.cjs.json

@@ -0,0 +1 @@
+{ "plain": 3300, "san": 5400 }