diff --git a/packages/compiler/src/backend/llvm/expr-stream-bridges.ts b/packages/compiler/src/backend/llvm/expr-stream-bridges.ts index b642041227..398384e757 100644 --- a/packages/compiler/src/backend/llvm/expr-stream-bridges.ts +++ b/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]!)); 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 56b65a3d30..3b64d65cee 100644 --- a/packages/compiler/src/frontend/lowering/key-order-witness.test.ts +++ b/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" }, + }); +}); diff --git a/packages/compiler/src/frontend/lowering/key-order-witness.ts b/packages/compiler/src/frontend/lowering/key-order-witness.ts index c35860d435..4eef5dc545 100644 --- a/packages/compiler/src/frontend/lowering/key-order-witness.ts +++ 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>(); /** 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[] => { 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>(); 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; } } diff --git a/packages/compiler/src/ir/ir.ts b/packages/compiler/src/ir/ir.ts index 89a5ee73ad..a9f5dda24a 100644 --- a/packages/compiler/src/ir/ir.ts +++ b/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 diff --git a/tests/corpus/7136-key-order-copies.ts b/tests/corpus/7136-key-order-copies.ts index daafba23b3..5d754bced5 100644 --- a/tests/corpus/7136-key-order-copies.ts +++ b/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)); +} diff --git a/tests/corpus/7138-key-order-late-writes.ts b/tests/corpus/7138-key-order-late-writes.ts index b1afd8761c..d0adef3da7 100644 --- a/tests/corpus/7138-key-order-late-writes.ts +++ b/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)); diff --git a/tests/harness/corpus-costs/7136-key-order-copies.ts.json b/tests/harness/corpus-costs/7136-key-order-copies.ts.json index ba3e270eed..9d76a548c8 100644 --- a/tests/harness/corpus-costs/7136-key-order-copies.ts.json +++ b/tests/harness/corpus-costs/7136-key-order-copies.ts.json @@ -1 +1 @@ -{ "plain": 1700, "san": 2400 } +{ "plain": 1900, "san": 3500 } diff --git a/tests/harness/corpus-costs/7138-key-order-late-writes.ts.json b/tests/harness/corpus-costs/7138-key-order-late-writes.ts.json index 0b945a1c9c..2b1aad28db 100644 --- a/tests/harness/corpus-costs/7138-key-order-late-writes.ts.json +++ b/tests/harness/corpus-costs/7138-key-order-late-writes.ts.json @@ -1 +1 @@ -{ "plain": 1700, "san": 2500 } +{ "plain": 1700, "san": 2800 }