Keep key orders exact through write values, live views and spreads

Three cases where a value's key order could differ from Node:

- A write of an optional key moved the key before its value was
  evaluated. A value that adds another key to the same record, as in
  `o.b = ((o.c = 1), 2)`, then put the keys in the wrong order. The write
  now evaluates its value first, then tests and moves the key.
- A value seen as a JavaScript value bound the field hooks of its order
  when it was boxed, and a later write that moved a key did not change
  them. Shapes whose values can change order after they are built
  (IrRecordShape.orderMoves) now read through the value's view, which
  follows the current order.
- A literal that spreads a value with orders and then sets a key the
  value may lack, as in `{ ...value, tag: "x" }`, put that key where the
  source's order has it. Node puts it there only when the value has it,
  and after the value's keys otherwise. The literal now picks its order
  from the source's order and which of those keys the source has.

Corpus 7136 and 7138 cover each case.

packages/compiler/src/backend/llvm/expr-stream-bridges.ts

@@ -1071,9 +1071,15 @@ export function streamTypedRefMaterializeAdapter(
       };
       // Published before the field converters, like the array hooks below.
       // A Dirent row reads through its finished view (the prototype and
-      // the non-enumerable %dtype).
+      // the non-enumerable %dtype). A capsule binds the ops of its value's
+      // order when it is built, so a shape whose values can change order
+      // later (orderMoves) reads through its view.
       const dirent = isDirentShape(shape);
-      if (!dirent && orders.every((order) => liveRecordOpsEligible(fieldsOf(order)))) {
+      if (
+        !dirent &&
+        shape.orderMoves === undefined &&
+        orders.every((order) => liveRecordOpsEligible(fieldsOf(order)))
+      ) {
         if (orders.length === 1) {
           adapter.recordOps = `@${snapshot}_rops`;
           liveRecordOps(host, t, snapshot, fieldsOf(orders[0]!));

packages/compiler/src/frontend/lowering/key-order-witness.test.ts

@@ -468,3 +468,111 @@ test("re-adding a key a literal creates earlier moves it to the end of that valu
     ["b", "a"],
   ]);
 });
+
+test("a moving write evaluates its value before it tests and moves the key", () => {
+  const f = fixture();
+  const o = f.record(
+    [
+      ["id", F64],
+      ["x", OPT],
+      ["y", OPT],
+    ],
+    ["id", "x", "y"],
+  );
+  // `o.x = ((o.y = 2), 3)`: y is created first.
+  const value: IrExpr = {
+    kind: "seqExpr",
+    stmts: [set(ref("o", o), "y")],
+    result: num(3),
+    type: F64,
+    loc,
+  };
+  const write: IrStmt = {
+    kind: "recordSet",
+    obj: ref("o", o),
+    shapeId: o.shapeId,
+    field: "x",
+    value,
+    loc,
+  };
+  const fn = f.run([
+    decl("o", f.literal(o, ["id"])),
+    observe(f.literal(o, ["x", "id"])),
+    write,
+    observe(ref("o", o)),
+  ]);
+  const shape = f.shapes.get(o.shapeId)!;
+  expect(shape.orderVariants).toBeDefined();
+  expect(shape.orderMoves).toBe(true);
+  const block = fn.body[2] as IrStmt & { kind: "block" };
+  expect(block.kind).toBe("block");
+  const [first, guard, written] = block.body;
+  expect(first).toMatchObject({ kind: "varDecl", init: { kind: "seqExpr" } });
+  const local = (first as IrStmt & { kind: "varDecl" }).localId;
+  expect(guard).toMatchObject({ kind: "if", cond: { kind: "unary", op: "!" } });
+  expect(written).toMatchObject({
+    kind: "recordSet",
+    field: "x",
+    value: { kind: "varRef", localId: local },
+  });
+  // The write inside the value moves y in its own block.
+  const init = (first as IrStmt & { kind: "varDecl" }).init as IrExpr & { kind: "seqExpr" };
+  expect(init.stmts[0]).toMatchObject({ kind: "block" });
+});
+
+test("a key after a spread that the source may lack picks its order from the source's keys", () => {
+  const f = fixture();
+  const s = f.record(
+    [
+      ["a", F64],
+      ["b", OPT],
+      ["c", OPT],
+    ],
+    ["a", "b", "c"],
+  );
+  const t = f.record(
+    [
+      ["a", F64],
+      ["b", F64],
+      ["c", OPT],
+    ],
+    ["a", "b", "c"],
+  );
+  const read = (field: string): IrExpr => ({
+    kind: "recordGet",
+    obj: ref("x", s),
+    shapeId: s.shapeId,
+    field,
+    type: OPT,
+    loc,
+  });
+  // `{ ...x, b: 1 }`
+  const merged = f.notes.literal(
+    {
+      kind: "recordLit",
+      fields: [
+        { name: "a", value: read("a") },
+        { name: "c", value: read("c") },
+        { name: "b", value: num(1) },
+      ],
+      type: t,
+      loc,
+    },
+    [
+      { kind: "spread", shapeId: s.shapeId, maybeAbsent: false },
+      { kind: "key", name: "b", definite: true },
+    ],
+  );
+  f.run([decl("x", f.literal(s, ["c", "a"])), observe(f.literal(s, ["a", "c"])), observe(merged)]);
+  // b keeps its place in the source's order when the source has it, and
+  // follows the source's keys when it does not.
+  expect(f.shapes.get(t.shapeId)!.orderVariants).toEqual([
+    ["a", "c", "b"],
+    ["c", "a", "b"],
+    ["a", "b", "c"],
+  ]);
+  expect(orderIndexOf(merged)).toMatchObject({
+    kind: "ternary",
+    cond: { kind: "recordHas", obj: { kind: "varRef", localId: "x" }, field: "b" },
+  });
+});

packages/compiler/src/frontend/lowering/key-order-witness.ts

@@ -768,7 +768,8 @@ export function applyKeyOrderWitnesses(lowerer: Lowerer, fns: readonly IrFunctio
   //    table, as before.
   // A write of an optional key a value lacks moves the key to the end of
   // the value's order, as in Node: the orders include every order such
-  // writes reach, and each write site updates the index first.
+  // writes reach, and each write site updates the index after it evaluates
+  // the value and before it writes the key.
   const MAX_ORDERS = 16;
   const unresolved = (id: string): boolean => result.get(id) === "unknown";
   /** `order` over every key of `table`: keys the order lacks follow it. */
@@ -809,6 +810,30 @@ export function applyKeyOrderWitnesses(lowerer: Lowerer, fns: readonly IrFunctio
     }
     return [...moving];
   };
+  /** `values[i]` for the order index `current` holds, as a chain of
+   * tests. */
+  const indexChain = (current: IrExpr, values: readonly number[], loc: SrcLoc): IrExpr => {
+    let out: IrExpr = { kind: "numLit", value: values[values.length - 1]!, type: F64, loc };
+    for (let i = values.length - 2; i >= 0; i--) {
+      if (out.kind === "numLit" && out.value === values[i]) continue;
+      out = {
+        kind: "ternary",
+        cond: {
+          kind: "bin",
+          op: "===",
+          left: current,
+          right: { kind: "numLit", value: i, type: F64, loc },
+          type: BOOL,
+          loc,
+        },
+        then: { kind: "numLit", value: values[i]!, type: F64, loc },
+        else_: out,
+        type: F64,
+        loc,
+      };
+    }
+    return out;
+  };
   const moves = new Map<string, Map<string, number[]>>();
   /** Adds to `orders` every order writes of `moving` keys reach from them
    * and records each key's transitions; false past MAX_ORDERS. */
@@ -887,8 +912,17 @@ export function applyKeyOrderWitnesses(lowerer: Lowerer, fns: readonly IrFunctio
   // way: copies (rest bindings, width copies), or literals spreading the
   // value between the same explicit keys. Order i is the source's order i
   // over the shape's keys, after the keys before the spread and before
-  // the keys after it.
-  type Template = { source: string; prefix: string[]; suffix: string[] };
+  // the keys after it. A key after the spread that the source value may
+  // lack goes where the source has it, or after the source's keys when it
+  // has not: such a literal's orders are one per source order and set of
+  // those keys the source has, and `select.index[present][i]` is the order
+  // for source order i with the keys of bit set `present`.
+  type Template = {
+    source: string;
+    prefix: string[];
+    suffix: string[];
+    select?: { keys: string[]; index: number[][] };
+  };
   const templateOf = (segments: readonly KeyOrderSegment[], id: string): Template | null => {
     const prefix: string[] = [];
     const suffix: string[] = [];
@@ -924,11 +958,39 @@ export function applyKeyOrderWitnesses(lowerer: Lowerer, fns: readonly IrFunctio
     const orders = variants.get(first.source);
     if (orders === undefined || mirrors.has(first.source)) continue;
     const table = tableOf(candidate.shape);
-    const mirrored = orders.map((o) => {
+    const tested = first.suffix.filter((k) => maybeAbsent(first.source, k));
+    if (tested.length > 4) continue;
+    const mirrorOf = (o: readonly string[], lacks: ReadonlySet<string>): string[] => {
       const order = [...first.prefix];
-      for (const k of [...o, ...first.suffix]) if (!order.includes(k)) order.push(k);
+      for (const k of o) if (!lacks.has(k) && !order.includes(k)) order.push(k);
+      for (const k of first.suffix) if (!order.includes(k)) order.push(k);
       return complete(order, table);
-    });
+    };
+    let mirrored: string[][];
+    let template: Template = first;
+    if (tested.length === 0) mirrored = orders.map((o) => mirrorOf(o, new Set()));
+    else {
+      mirrored = [];
+      const index: number[][] = [];
+      for (let present = 0; present < 1 << tested.length; present++) {
+        const lacks = new Set(tested.filter((_, j) => (present & (1 << j)) === 0));
+        const row: number[] = [];
+        for (const o of orders) {
+          const order = mirrorOf(o, lacks);
+          let i = indexOfOrder(mirrored, order);
+          if (i < 0) i = mirrored.push(order) - 1;
+          row.push(i);
+        }
+        index.push(row);
+      }
+      if (mirrored.length > MAX_ORDERS) continue;
+      template = {
+        source: first.source,
+        prefix: first.prefix,
+        suffix: first.suffix,
+        select: { keys: tested, index },
+      };
+    }
     if (!closeUnderMoves(id, mirrored, candidate.moving)) continue;
     if (mirrored.every((o) => sameOrder(o, mirrored[0]!))) {
       // Every order of the source agrees over these keys: one table.
@@ -940,7 +1002,7 @@ export function applyKeyOrderWitnesses(lowerer: Lowerer, fns: readonly IrFunctio
       continue;
     }
     variants.set(id, mirrored);
-    mirrors.set(id, first);
+    mirrors.set(id, template);
   }
   if (variants.size === 0) return;
 
@@ -995,7 +1057,7 @@ export function applyKeyOrderWitnesses(lowerer: Lowerer, fns: readonly IrFunctio
     if (segments !== undefined && mirror !== undefined) {
       const obj = copySource(lit, mirror.source);
       if (obj === null) return null;
-      return {
+      const sourceIndex: IrExpr = {
         kind: "recordGet",
         obj,
         shapeId: mirror.source,
@@ -1003,6 +1065,22 @@ export function applyKeyOrderWitnesses(lowerer: Lowerer, fns: readonly IrFunctio
         type: F64,
         loc: lit.loc,
       };
+      const select = mirror.select;
+      if (select === undefined) return sourceIndex;
+      // Tests the source for each key in turn, then picks by its index.
+      const pick = (j: number, present: number): IrExpr => {
+        if (j === select.keys.length)
+          return indexChain(sourceIndex, select.index[present]!, lit.loc);
+        return {
+          kind: "ternary",
+          cond: lowerer.recordFieldPresent(obj, mirror.source, select.keys[j]!, lit.loc),
+          then: pick(j + 1, present | (1 << j)),
+          else_: pick(j + 1, present),
+          type: F64,
+          loc: lit.loc,
+        };
+      };
+      return pick(0, 0);
     }
     if (segments !== undefined) {
       const order = literalOrder(segments);
@@ -1072,7 +1150,7 @@ export function applyKeyOrderWitnesses(lowerer: Lowerer, fns: readonly IrFunctio
       typeof index === "number" ? { kind: "numLit", value: index, type: F64, loc: lit.loc } : index;
     lit.fields.push({ name: KEY_ORDER_FIELD, value });
   }
-  let receivers = 0;
+  let temporaries = 0;
   const replacements = new Map<IrFunction, Map<IrStmt, IrStmt>>();
   for (const { st, fn } of writeSites) {
     if (dropped.has(st.shapeId)) continue;
@@ -1084,7 +1162,7 @@ export function applyKeyOrderWitnesses(lowerer: Lowerer, fns: readonly IrFunctio
     const prologue: IrStmt[] = [];
     let obj = st.obj;
     if (plain.kind !== "varRef") {
-      const localId = `%keyOrderReceiver.${receivers++}`;
+      const localId = `%keyOrderReceiver.${temporaries++}`;
       fn.locals.push({ id: localId, name: "%keyOrderReceiver", type: st.obj.type, mutable: false });
       prologue.push({ kind: "varDecl", localId, init: st.obj, loc });
       obj = { kind: "varRef", localId, type: st.obj.type, loc };
@@ -1097,29 +1175,20 @@ export function applyKeyOrderWitnesses(lowerer: Lowerer, fns: readonly IrFunctio
       type: F64,
       loc,
     };
-    // transitions[i] for the current index i, as a chain of tests.
-    let next: IrExpr = {
-      kind: "numLit",
-      value: transitions[transitions.length - 1]!,
-      type: F64,
-      loc,
-    };
-    for (let i = transitions.length - 2; i >= 0; i--) {
-      next = {
-        kind: "ternary",
-        cond: {
-          kind: "bin",
-          op: "===",
-          left: current,
-          right: { kind: "numLit", value: i, type: F64, loc },
-          type: BOOL,
-          loc,
-        },
-        then: { kind: "numLit", value: transitions[i]!, type: F64, loc },
-        else_: next,
-        type: F64,
-        loc,
-      };
+    const next = indexChain(current, transitions, loc);
+    // The value evaluates before the key is tested and moved: it may add
+    // other keys to the same value, which then come first.
+    let value = st.value;
+    const pure =
+      value.kind === "numLit" ||
+      value.kind === "strLit" ||
+      value.kind === "boolLit" ||
+      value.kind === "varRef";
+    if (!pure) {
+      const localId = `%keyOrderValue.${temporaries++}`;
+      fn.locals.push({ id: localId, name: "%keyOrderValue", type: value.type, mutable: false });
+      prologue.push({ kind: "varDecl", localId, init: value, loc });
+      value = { kind: "varRef", localId, type: value.type, loc };
     }
     const absent: IrExpr = {
       kind: "unary",
@@ -1142,7 +1211,7 @@ export function applyKeyOrderWitnesses(lowerer: Lowerer, fns: readonly IrFunctio
       obj,
       shapeId: st.shapeId,
       field: st.field,
-      value: st.value,
+      value,
       loc,
     };
     const guard: IrStmt = { kind: "if", cond: absent, then: [update], else_: null, loc };
@@ -1172,5 +1241,6 @@ export function applyKeyOrderWitnesses(lowerer: Lowerer, fns: readonly IrFunctio
       tables.push(table);
     }
     shape.orderVariants = tables;
+    if (moves.has(id)) shape.orderMoves = true;
   }
 }

packages/compiler/src/ir/ir.ts

@@ -1673,6 +1673,10 @@ export interface IrRecordShape {
    * and every other construction leaves 0. Each key-order surface
    * dispatches on that field. */
   orderVariants?: string[][];
+  /** A shape with orderVariants whose values can change order after they
+   * are built: a write of an optional key a value lacks updates its
+   * KEY_ORDER_FIELD. */
+  orderMoves?: true;
 }
 
 /** The hidden field of a shape with orderVariants: the index of the value's

tests/corpus/7136-key-order-copies.ts

@@ -17,3 +17,16 @@ for (const value of [a, b, c]) {
   console.log(name, JSON.stringify(rest), Object.keys(rest).join(","));
   console.log(JSON.stringify(assigned), JSON.stringify(cloned));
 }
+// Keys after a spread that the spread value may lack: a key the value has
+// keeps its place, and a key it lacks goes after the value's keys.
+type Options = { id: number; label?: string; size?: number; tag?: string };
+const options: Options[] = [
+  { id: 1, size: 2 },
+  { size: 3, id: 2 },
+  { label: "c", id: 3, tag: "t" },
+  { tag: "u", size: 4, id: 4 },
+];
+for (const value of options) {
+  const merged = { ...value, tag: "x", label: "y", done: true };
+  console.log(Object.keys(merged).join(","), JSON.stringify(merged));
+}

tests/corpus/7138-key-order-late-writes.ts

@@ -23,5 +23,15 @@ for (let i = 0; i < 2; i++) {
   console.log(Object.keys(built).join(","));
 }
 console.log(built);
-// d
-// e
+// A write's value runs first, so a key it adds comes before the written
+// key.
+const nested: Item = { count: 3, id: 4 };
+nested.name = ((nested.note = "first"), "second");
+console.log(Object.keys(nested).join(","), JSON.stringify(nested));
+// A value seen as a JavaScript value enumerates later writes in order.
+const seen: Item = { id: 5, count: 4 };
+const view: unknown = seen;
+console.log(Object.keys(view as object).join(","));
+seen.note = "n";
+seen.name = "m";
+console.log(Object.keys(view as object).join(","), JSON.stringify(view));

tests/harness/corpus-costs/7136-key-order-copies.ts.json

@@ -1 +1 @@
-{ "plain": 1700, "san": 2400 }
+{ "plain": 1900, "san": 3500 }

tests/harness/corpus-costs/7138-key-order-late-writes.ts.json

@@ -1 +1 @@
-{ "plain": 1700, "san": 2500 }
+{ "plain": 1700, "san": 2800 }