From 5c015be6be05670a87b57d3a63b888f6f24bbae0 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Fri, 23 Oct 2026 07:47:26 -0500 Subject: [PATCH] Try wider record arms first when a dyn value converts to a union Records are width-tolerant, and a union's arms were tried in canonical order, which sorts record arms by shape id. When a `{}` arm, or an arm whose fields are all optional, came first, it took every object, so a value converted to the union lost the fields only a wider arm has: const pairs = [["a", { publicKeyEncoding: 5 }], ["b", {}]]; for (const [, v] of pairs) console.log(Object.keys(v)); printed [] twice, where Node prints [ 'publicKeyEncoding' ] and []. A checked cast to `{ p: number; q: string } | { p?: number }` dropped `q` the same way. The checked conversion now tries record arms with more required fields first, then those with more fields, and keeps the canonical order otherwise. The union's tags are unchanged. --- packages/compiler/src/backend/llvm/dyn.ts | 39 +++++++++++++++++-- packages/compiler/src/ir/ir.ts | 6 ++- .../7447-union-record-arm-width.mjs.json | 6 +++ .../7448-union-record-arm-width.ts.json | 6 +++ tests/corpus/7447-union-record-arm-width.mjs | 37 ++++++++++++++++++ tests/corpus/7448-union-record-arm-width.ts | 21 ++++++++++ .../7447-union-record-arm-width.mjs.json | 1 + .../7448-union-record-arm-width.ts.json | 1 + 8 files changed, 111 insertions(+), 6 deletions(-) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7447-union-record-arm-width.mjs.json create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7448-union-record-arm-width.ts.json create mode 100644 tests/corpus/7447-union-record-arm-width.mjs create mode 100644 tests/corpus/7448-union-record-arm-width.ts create mode 100644 tests/harness/corpus-costs/7447-union-record-arm-width.mjs.json create mode 100644 tests/harness/corpus-costs/7448-union-record-arm-width.ts.json diff --git a/packages/compiler/src/backend/llvm/dyn.ts b/packages/compiler/src/backend/llvm/dyn.ts index 94581cfa0..86626d304 100644 --- a/packages/compiler/src/backend/llvm/dyn.ts +++ b/packages/compiler/src/backend/llvm/dyn.ts @@ -370,6 +370,36 @@ export class LlDyn { B.line(`call void @scr_dyn_build_add(ptr %d, ptr @${name}, ptr ${value})`); } + /** The order a dyn value tries a union's arms in: the canonical order, + * except that among the record arms, those with more required fields go + * first, then those with more fields. Records are width-tolerant, so an + * arm with fewer fields (`{}`, or one whose fields are all optional, + * matches every object) would otherwise take an object that a wider arm + * also matches and drop the fields only the wider arm has. */ + private unionArmOrder(def: IrUnionDef): number[] { + const width = (i: number): [number, number] | null => { + const arm = def.arms[i]!; + if (arm.kind !== "record") return null; + const shape = this.host.recordsById.get(arm.shapeId); + if (!shape || shape.tuple) return null; + const required = shape.fields.filter( + (field) => + field.type.kind !== "union" || + !this.host.unionsById.get(field.type.unionId)?.arms.some((x) => x.kind === "undefinedT"), + ).length; + return [required, shape.fields.length]; + }; + const order = def.arms.map((_, i) => i); + const records = order.filter((i) => width(i) !== null); + const widest = [...records].sort((x, y) => { + const [rx, fx] = width(x)!; + const [ry, fy] = width(y)!; + return ry - rx || fy - fx || x - y; + }); + records.forEach((slot, k) => (order[slot] = widest[k]!)); + return order; + } + /** Branch to a variant only after its literal discriminator and complete * structure match. Use the same path for predicates and builders. */ private unionArmMatch( @@ -2368,9 +2398,10 @@ export class LlDyn { throw new InternalCompilerError( `llvm emitter bug: dynCheck of unknown union ${t.unionId}`, ); - // Arms in CANONICAL order, first FULL match wins. The matched - // arm's builder can no longer fail. - def.arms.forEach((arm, i) => { + // Arms in unionArmOrder, first FULL match wins. The matched arm's + // builder can no longer fail. + for (const i of this.unionArmOrder(def)) { + const arm = def.arms[i]!; const lHit = B.newLabel("dcu.h"); const lNext = B.newLabel("dcu.n"); this.unionArmMatch(B, def, i, lHit, lNext, preserveRefs); @@ -2414,7 +2445,7 @@ export class LlDyn { B.terminate(`ret ptr ${u}`); } B.startBlock(lNext); - }); + } if (preserveRefs) { B.line( `call void @scr_dyn_check_fail(ptr %path, ptr ${host.cstr("reference with its original layout")}, ptr %d)`, diff --git a/packages/compiler/src/ir/ir.ts b/packages/compiler/src/ir/ir.ts index 6d0760b81..878088d65 100644 --- a/packages/compiler/src/ir/ir.ts +++ b/packages/compiler/src/ir/ir.ts @@ -3898,8 +3898,10 @@ export type IrExpr = * not shape equality; missing or wrong-typed fields throw, EXCEPT that a * missing key for an undefined-armed union field — an optional field — * builds the interned undefined arm instead); arrays check every element; - * unions try arms in canonical order and the first FULL match wins (no - * match → throw; an undefined arm matches no dyn value); JSON null matches + * unions try arms in canonical order, except that record arms with more + * required fields and then more fields go first, and the first FULL match + * wins (no match → throw; an undefined arm matches no dyn value); JSON + * null matches * exactly the nullT arm of a union target (bare null targets cannot * exist). MAY THROW: * backends' may-throw analyses must treat it like a `throw` statement. diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7447-union-record-arm-width.mjs.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7447-union-record-arm-width.mjs.json new file mode 100644 index 000000000..68bd750d9 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7447-union-record-arm-width.mjs.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/7447-union-record-arm-width.mjs" + ], + "diags": [] +} diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7448-union-record-arm-width.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7448-union-record-arm-width.ts.json new file mode 100644 index 000000000..498ee884a --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7448-union-record-arm-width.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/7448-union-record-arm-width.ts" + ], + "diags": [] +} diff --git a/tests/corpus/7447-union-record-arm-width.mjs b/tests/corpus/7447-union-record-arm-width.mjs new file mode 100644 index 000000000..dc12f8a59 --- /dev/null +++ b/tests/corpus/7447-union-record-arm-width.mjs @@ -0,0 +1,37 @@ +// Values read back from arrays whose elements mix object literals of +// different widths, an empty object among them, keep every field: the +// record arm with more fields is tried before `{}`, which matches every +// object. +const pairs = [ + ["a", { publicKeyEncoding: 5 }], + ["b", {}], + ["c", { publicKeyEncoding: 6, extra: true }], +]; +for (const [label, value] of pairs) console.log(label, Object.keys(value), JSON.stringify(value)); + +const options = [ + ["rsa", { modulusLength: 1024 }], + ["ec", { namedCurve: "P-256", paramEncoding: "named" }], + ["none", {}], +]; +for (const [type, value] of options) { + const { modulusLength, namedCurve, paramEncoding } = value; + console.log(type, modulusLength, namedCurve, paramEncoding); +} + +const mixed = [{ a: 1 }, {}, "s", { a: 2, b: "two" }]; +for (const value of mixed) { + console.log(typeof value, JSON.stringify(value), typeof value === "object" ? Object.keys(value).length : -1); +} + +function describe(value) { + return Object.entries(value) + .map(([key, field]) => `${key}=${field}`) + .join(";"); +} +const rows = [ + [1, { x: 1, y: 2 }], + [2, {}], + [3, { x: 3 }], +]; +for (const [id, value] of rows) console.log(id, describe(value), value.x, value.y); diff --git a/tests/corpus/7448-union-record-arm-width.ts b/tests/corpus/7448-union-record-arm-width.ts new file mode 100644 index 000000000..1518dd575 --- /dev/null +++ b/tests/corpus/7448-union-record-arm-width.ts @@ -0,0 +1,21 @@ +// A checked cast to a union of record types keeps every field of the +// value: arms with more required fields, then more fields, are tried +// before an arm whose fields are all optional, which matches every object. +type Full = { p: number; q: string }; +type Bare = { p?: number }; +type Other = { r: boolean }; + +const values = JSON.parse('[{"p":5,"q":"x"},{},{"r":true},{"p":7}]') as (Full | Bare | Other)[]; +for (const value of values) { + const q = "q" in value ? value.q : "-"; + const r = "r" in value ? value.r : "-"; + console.log(JSON.stringify(value), "q" in value, "r" in value, q, r); +} + +const one = JSON.parse('{"p":1,"q":"y"}') as Bare | Full; +console.log(JSON.stringify(one), "q" in one ? one.q : "no q"); + +type Point = { x: number; y?: number }; +type Labeled = { x: number; label: string }; +const points = JSON.parse('[{"x":1,"label":"a"},{"x":2,"y":3},{"x":4}]') as (Point | Labeled)[]; +for (const point of points) console.log(point.x, "label" in point ? point.label : "none", "y" in point ? point.y : "none"); diff --git a/tests/harness/corpus-costs/7447-union-record-arm-width.mjs.json b/tests/harness/corpus-costs/7447-union-record-arm-width.mjs.json new file mode 100644 index 000000000..5648bd255 --- /dev/null +++ b/tests/harness/corpus-costs/7447-union-record-arm-width.mjs.json @@ -0,0 +1 @@ +{ "plain": 5785, "san": 5722 } diff --git a/tests/harness/corpus-costs/7448-union-record-arm-width.ts.json b/tests/harness/corpus-costs/7448-union-record-arm-width.ts.json new file mode 100644 index 000000000..c60a813c2 --- /dev/null +++ b/tests/harness/corpus-costs/7448-union-record-arm-width.ts.json @@ -0,0 +1 @@ +{ "plain": 3204, "san": 4052 }