From c43d4002e71f3c87a8b7bee79c12c02793bf1c2b Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Thu, 22 Oct 2026 09:59:57 -0500 Subject: [PATCH] Skip only always-throwing calls as record producers in the key-order witness The witness skipped every libCall whose signature result is VOID. VOID is also the signature of runtime calls that take their record type from the use site and build the record at run time, such as net.serverAddress (`server.address()`). A literal of the same shape then set the shape's key order, and the runtime record enumerated in the literal's order instead of Node's (`port, family, address` for `address, family, port`). The witness now skips only the calls in ALWAYS_THROW_LIB_FNS, which never produce a value. The set moves from validate.ts's result-type check to builtin-effects.ts, and both use it. --- .../lowering/key-order-witness.test.ts | 30 +++++++++++++++++++ .../frontend/lowering/key-order-witness.ts | 11 +++---- packages/compiler/src/ir/builtin-effects.ts | 22 ++++++++++++++ packages/compiler/src/ir/validate.ts | 29 ++---------------- 4 files changed, 61 insertions(+), 31 deletions(-) diff --git a/packages/compiler/src/frontend/lowering/key-order-witness.test.ts b/packages/compiler/src/frontend/lowering/key-order-witness.test.ts index de16e5d61..8632bad47 100644 --- a/packages/compiler/src/frontend/lowering/key-order-witness.test.ts +++ b/packages/compiler/src/frontend/lowering/key-order-witness.test.ts @@ -157,6 +157,36 @@ test("a synthesized construction pins today's table", () => { expect(f.order(ab)).toEqual(["a", "b"]); }); +test("a runtime builder typed by its use site pins today's table", () => { + const f = fixture(); + const ab = f.record( + [ + ["a", F64], + ["b", F64], + ], + ["a", "b"], + ); + // net.serverAddress's signature result is VOID: the call takes its record + // type from its use site, and the runtime builds the record in that order. + const built: IrExpr = { kind: "libCall", fn: "net.serverAddress", args: [], type: ab, loc }; + f.run([observe(f.literal(ab, ["b", "a"])), observe(built)]); + expect(f.order(ab)).toEqual(["a", "b"]); +}); + +test("an always-throwing call typed as the record leaves its literals' order", () => { + const f = fixture(); + const ab = f.record( + [ + ["a", F64], + ["b", F64], + ], + ["a", "b"], + ); + const thrown: IrExpr = { kind: "libCall", fn: "error.nodeThrow", args: [], type: ab, loc }; + f.run([observe(f.literal(ab, ["b", "a"])), observe(thrown)]); + expect(f.order(ab)).toEqual(["b", "a"]); +}); + test("a checked conversion alone pins today's table", () => { const f = fixture(); const ab = f.record( diff --git a/packages/compiler/src/frontend/lowering/key-order-witness.ts b/packages/compiler/src/frontend/lowering/key-order-witness.ts index 7ce8ae074..4adfb87ab 100644 --- a/packages/compiler/src/frontend/lowering/key-order-witness.ts +++ b/packages/compiler/src/frontend/lowering/key-order-witness.ts @@ -45,7 +45,7 @@ import type { IrExpr, IrFunction, IrRecordShape, IrStmt, IrType } from "../../ir import { shapeHasAccessorSlots, typeEquals, UNDEFINED_T } from "../../ir/ir.js"; import { everyExprChild, everyStmtChild, everyTypeChild } from "../../ir/traverse.js"; import type { Lowerer } from "./lowerer.js"; -import { LIB_FN_SIGS } from "../../ir/builtin-signatures.js"; +import { ALWAYS_THROW_LIB_FNS } from "../../ir/builtin-effects.js"; /** One property of an object literal, in source order. */ export type KeyOrderSegment = @@ -313,10 +313,11 @@ export function applyKeyOrderWitnesses(lowerer: Lowerer, fns: readonly IrFunctio // Runtime surfaces (console, util.format, workers) that take a // record may walk its keys; runtime builders make new records. for (const arg of e.args) observedRoots.push(arg.type); - // A void runtime call typed by its use site is a throw standing in - // for a value (`error.nodeThrow` in a narrowing ternary): it never - // produces one. - if (LIB_FN_SIGS[e.fn].result.kind !== "void") producedBy(e.type, true); + // An always-throwing call stands in for a value (`error.nodeThrow` + // in a narrowing ternary) and never produces one. Other calls + // whose signature result is VOID are typed by their use site and + // build the record at run time (`server.address()`). + if (!ALWAYS_THROW_LIB_FNS.has(e.fn)) producedBy(e.type, true); return; case "intrinsic": // Intrinsics hand back values built elsewhere, except allSettled's diff --git a/packages/compiler/src/ir/builtin-effects.ts b/packages/compiler/src/ir/builtin-effects.ts index 16db45b6c..c2194a13c 100644 --- a/packages/compiler/src/ir/builtin-effects.ts +++ b/packages/compiler/src/ir/builtin-effects.ts @@ -1001,3 +1001,25 @@ export const MAY_THROW_LIB_FNS: ReadonlySet = new Set([ "crypto.createSign", "crypto.createVerify", ]); + +/** libCall members that always throw. Their result type is the replaced + * expression's own and is never materialized: global.undefRead's is the + * declared type of the undefined global; the others stand in for a value + * of the expression they replace. The fs Chk ladders qualify: every + * validation failure throws Node's typed error, and a full pass throws the + * trailing compiler-rendered fence. */ +export const ALWAYS_THROW_LIB_FNS: ReadonlySet = new Set([ + "global.undefRead", + "error.nodeThrow", + "error.argTypeThrow", + "error.propTypeThrow", + "fs.mkdtempChk", + "fs.readFileChk", + "fs.opendirChk", + "fs.watchFileChk", + "fs.lchmodChk", + "fs.readChk", + "fs.streamOptsChk", + "net.connectOptsChk", + "tls.caCertsChk", +]); diff --git a/packages/compiler/src/ir/validate.ts b/packages/compiler/src/ir/validate.ts index bb36c7007..163c2a41c 100644 --- a/packages/compiler/src/ir/validate.ts +++ b/packages/compiler/src/ir/validate.ts @@ -61,6 +61,7 @@ import { unionWideningTags } from "./analysis.js"; import { alwaysReturns } from "./control-flow.js"; import { dynamicOperationOf, functionTier } from "./tiers.js"; import { LIB_FN_SIGS, pipelinePumpedRuns } from "./builtin-signatures.js"; +import { ALWAYS_THROW_LIB_FNS } from "./builtin-effects.js"; /** The promisified crypto calls' value kinds besides checked-dynamic. */ const CRYPTO_PROMISE_VALUES: Readonly> = { @@ -5345,32 +5346,8 @@ function validateFunction( const callback = e.args[e.args.length - 1]?.type; if (callback?.kind !== "func") err(`libCall ${e.fn} callback must be a function`, e.loc); } - if (e.fn === "global.undefRead") { - // Always throws — the result type is whatever the declared type - // of the undefined global mapped to (never materialized). - return true; - } - if ( - e.fn === "error.nodeThrow" || - e.fn === "error.argTypeThrow" || - e.fn === "error.propTypeThrow" || - e.fn === "fs.mkdtempChk" || - e.fn === "fs.readFileChk" || - e.fn === "fs.opendirChk" || - e.fn === "fs.watchFileChk" || - e.fn === "fs.lchmodChk" || - e.fn === "fs.readChk" || - e.fn === "fs.streamOptsChk" || - e.fn === "net.connectOptsChk" || - e.fn === "tls.caCertsChk" - ) { - // Always throws — the result type is the replaced expression's - // own (never materialized; the global.undefRead pattern). The - // fs Chk ladders qualify: every validation failure throws - // Node's typed error, and a full pass throws the trailing - // compiler-rendered fence. - return true; - } + // Always throws: the result type is never materialized. + if (ALWAYS_THROW_LIB_FNS.has(e.fn)) return true; if (e.fn === "error.new" || e.fn === "error.newOptions") { // Which builtin the runtime constructs is named by the result type. if (!isBuiltinErrorObject(e.type)) {