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 }