Keep every field when an object converts to a union with an empty-record arm (#1992)

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

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.

packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7447-union-record-arm-width.mjs.json

@@ -0,0 +1,6 @@
+{
+ "order": [
+  "<repo>/tests/corpus/7447-union-record-arm-width.mjs"
+ ],
+ "diags": []
+}

packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7448-union-record-arm-width.ts.json

@@ -0,0 +1,6 @@
+{
+ "order": [
+  "<repo>/tests/corpus/7448-union-record-arm-width.ts"
+ ],
+ "diags": []
+}

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

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

tests/harness/corpus-costs/7447-union-record-arm-width.mjs.json

@@ -0,0 +1 @@
+{ "plain": 5785, "san": 5722 }

tests/harness/corpus-costs/7448-union-record-arm-width.ts.json

@@ -0,0 +1 @@
+{ "plain": 3204, "san": 4052 }