diff --git a/packages/compiler/src/backend/llvm/expr-primitives.ts b/packages/compiler/src/backend/llvm/expr-primitives.ts index 44db911859..439f49919b 100644 --- a/packages/compiler/src/backend/llvm/expr-primitives.ts +++ b/packages/compiler/src/backend/llvm/expr-primitives.ts @@ -1531,10 +1531,12 @@ export function emitRecordExpr( const absent = host.fieldAbsentTest(v, unionId); const present = B.tmp(); B.line(`${present} = xor i1 ${absent}, true`); + // The field load can end in a later block (a view's slot read). + const checked = B.currentLabel(); B.br(join); B.startBlock(join); const answer = B.tmp(); - B.line(`${answer} = phi i1 [ false, %${from} ], [ ${present}, %${check} ]`); + B.line(`${answer} = phi i1 [ false, %${from} ], [ ${present}, %${checked} ]`); return { name: answer, type: e.type }; } case "fieldAbsent": diff --git a/packages/compiler/src/frontend/lowering/expressions/updates.ts b/packages/compiler/src/frontend/lowering/expressions/updates.ts index 1b151fa9f9..30ea81ae29 100644 --- a/packages/compiler/src/frontend/lowering/expressions/updates.ts +++ b/packages/compiler/src/frontend/lowering/expressions/updates.ts @@ -902,7 +902,9 @@ export function fieldSetStmt( // A field some partial record assertion leaves unwritten (a partial // record view, partial-assertions.ts): a view cannot grow a slot the // asserted literal never wrote, so the write throws the catchable - // TypeError (Node would add the property — a documented divergence). + // TypeError (Node would add the property — a documented divergence). The + // receiver and the value are evaluated first, as JavaScript does, and the + // presence test runs where the write would. const refused = partialFieldWriteError(lowerer, target.shapeId, target.field, loc); if (refused !== null) { const obj = target.obj; @@ -910,10 +912,15 @@ export function fieldSetStmt( ? null : lowerer.declareHiddenLocal("%partialWrite", obj.type); const ref = stable ? varRef(stable.id, obj.type, loc) : obj; + const saved = isSafeToRepeat(value) + ? null + : lowerer.declareHiddenLocal("%partialWriteValue", value.type); + const written = saved ? varRef(saved.id, value.type, loc) : value; return { kind: "block", body: [ ...(stable ? [{ kind: "varDecl" as const, localId: stable.id, init: obj, loc }] : []), + ...(saved ? [{ kind: "varDecl" as const, localId: saved.id, init: value, loc }] : []), { kind: "if", cond: { @@ -928,7 +935,14 @@ export function fieldSetStmt( else_: [{ kind: "exprStmt", expr: refused, loc }], loc, }, - { kind: "recordSet", obj: ref, shapeId: target.shapeId, field: target.field, value, loc }, + { + kind: "recordSet", + obj: ref, + shapeId: target.shapeId, + field: target.field, + value: written, + loc, + }, ], loc, }; diff --git a/packages/compiler/src/frontend/lowering/lowerer.test.ts b/packages/compiler/src/frontend/lowering/lowerer.test.ts index 6f1c1c555c..da2f7da7fa 100644 --- a/packages/compiler/src/frontend/lowering/lowerer.test.ts +++ b/packages/compiler/src/frontend/lowering/lowerer.test.ts @@ -17,7 +17,11 @@ import { classMethodValue } from "./class-method-values.js"; const loc = { file: "island.ts", start: 0, end: 1 }; const record = (shapeId: string): IrType => ({ kind: "record", shapeId }); const context = () => - new Lowerer({ getTypeChecker: () => ({}) } as Program, { fileName: loc.file } as SourceFile, []); + new Lowerer( + { getTypeChecker: () => ({}), getImplementationSourceFiles: () => [] } as unknown as Program, + { fileName: loc.file } as SourceFile, + [], + ); test("reuses complete copy routes without repeating discriminator planning", () => { const lowerer = context(); diff --git a/packages/compiler/src/frontend/lowering/partial-assertions.ts b/packages/compiler/src/frontend/lowering/partial-assertions.ts index 12d3c32b7e..5f8ecc2533 100644 --- a/packages/compiler/src/frontend/lowering/partial-assertions.ts +++ b/packages/compiler/src/frontend/lowering/partial-assertions.ts @@ -10,12 +10,13 @@ * not declare are absent. Reflection through the view is exact. * * This module finds those sites and answers, per target shape, which - * required fields some asserted literal leaves unwritten. The lowering - * consults it for member reads and writes, destructuring and `in`, so an - * absent field's read throws the catchable TypeError (Node yields - * undefined — a documented divergence) instead of reaching a slot the view - * does not have. Computed once for the whole program, on first use, like - * the untyped-collection inference (js-collection-types.ts). */ + * fields some asserted literal leaves unwritten. The lowering consults it + * for member reads and writes, destructuring and `in`, so a read of an + * absent optional field yields undefined, as in Node, and a read of an + * absent required field throws the catchable TypeError (Node yields + * undefined — a documented divergence), instead of reaching a slot the + * view does not have. Computed once for the whole program, on first use, + * like the untyped-collection inference (js-collection-types.ts). */ import * as ts from "../ts7/adapter.js"; import type { Lowerer } from "./lowerer.js"; @@ -32,7 +33,7 @@ import { } from "../../ir/ir.js"; /** One site's facts: the asserted record type, the literal's keys in - * source order, and the required B fields the literal does not write. */ + * source order, and the B fields the literal does not write. */ export interface PartialAssertionSite { readonly target: IrType & { kind: "record" }; readonly keys: readonly string[]; @@ -48,7 +49,7 @@ interface PartialTarget { const NO_FIELDS: ReadonlySet = new Set(); -/** The required fields some asserted literal leaves unwritten on shapes of +/** The fields some asserted literal leaves unwritten on shapes of * `shapeId`; empty when no partial assertion produces that shape. */ export function partialAssertionFields(lowerer: Lowerer, shapeId: string): ReadonlySet { return partialTargetOf(lowerer, shapeId)?.missing ?? NO_FIELDS; @@ -108,22 +109,26 @@ export function partialAssertionSite( // map, which a view has no slots for. if (!shape.fields.some((f) => f.name === key)) return null; } + let required = false; for (const field of shape.fields) { if (keys.includes(field.name)) continue; - // Only a REQUIRED field can be missing: an undefined-armed slot holds - // the field's absent state, which the view keeps. - if (field.type.kind === "union" && lowerer.armTag(field.type.unionId, UNDEFINED_T) >= 0) - continue; + // The view has no slot for any field the literal does not write, an + // optional one included. missing.add(field.name); + if (!(field.type.kind === "union" && lowerer.armTag(field.type.unionId, UNDEFINED_T) >= 0)) + required = true; } - if (missing.size === 0) return null; + // Omitting only optional fields needs no view: the literal at the + // asserted type's layout holds their absent state. + if (!required) return null; return { target, keys, missing, text: lowerer.checker.typeToString(targetTs) }; } /** The checked read of `field` through a value of `shapeId` that a partial - * assertion may have produced, or null when the field is always present. - * `obj` is read twice (the presence test and the read), so it must be safe - * to repeat. */ + * assertion may have produced (undefined for an absent optional field, a + * TypeError for an absent required one), or null when the field is always + * present. `obj` is read twice (the presence test and the read), so it must + * be safe to repeat. */ export function partialFieldRead( lowerer: Lowerer, obj: IrExpr, @@ -134,17 +139,25 @@ export function partialFieldRead( ): IrExpr | null { const target = partialTargetOf(lowerer, shapeId); if (target === null || !target.missing.has(field)) return null; + // An optional field the literal omits reads as undefined, as in Node. + const optional = + fieldType.kind === "union" && lowerer.armTag(fieldType.unionId, UNDEFINED_T) >= 0; return { kind: "ternary", cond: { kind: "recordHas", obj, shapeId, field, type: BOOL, loc }, then: { kind: "recordGet", obj, shapeId, field, type: fieldType, loc }, - else_: nodeThrowExpr( - 1, - "", - `Cannot read property '${field}': the object asserted as '${target.text}' has no such property`, - fieldType, - loc, - ), + else_: optional + ? lowerer.coerceToExpected( + { kind: "unitLit", unit: "undefined", type: UNDEFINED_T, loc }, + fieldType, + ) + : nodeThrowExpr( + 1, + "", + `Cannot read property '${field}': the object asserted as '${target.text}' has no such property`, + fieldType, + loc, + ), type: fieldType, loc, }; diff --git a/packages/compiler/test/record-partial-assertion-lowering.test.ts b/packages/compiler/test/record-partial-assertion-lowering.test.ts index e7bad67b62..0ba5fb70d5 100644 --- a/packages/compiler/test/record-partial-assertion-lowering.test.ts +++ b/packages/compiler/test/record-partial-assertion-lowering.test.ts @@ -114,3 +114,52 @@ test("a write of a field a partial assertion never wrote throws before storing", }, ]); }); + +test("a read of an optional field a partial assertion omits yields undefined", async () => { + const nodes = await moduleNodes<{ + kind: string; + cond?: { kind: string; field?: string }; + else_?: { kind: string; fn?: string }; + }>( + `interface N { content: string; note?: string } + function read(value: N): string | undefined { return value.note; } + read({ content: "c" } as N); + read({} as N); + export {}; + `, + ); + const guard = nodes.find( + (n) => n.kind === "ternary" && n.cond?.kind === "recordHas" && n.cond.field === "note", + ); + expect(guard).toBeDefined(); + expect(guard!.else_?.fn).not.toBe("error.nodeThrow"); +}); + +test("a write of a field a partial assertion never wrote evaluates the value before the check", async () => { + const nodes = await moduleNodes<{ + kind: string; + localId?: string; + body?: { kind: string; localId?: string; cond?: { kind: string; field?: string } }[]; + }>( + `${TR}function value(): string { return "x"; } + function write(target: TR): void { target.content = value(); } + write({ truncated: true } as TR); + export {}; + `, + ); + const body = nodes + .filter((n) => n.kind === "block") + .map((n) => n.body ?? []) + .find((stmts) => + stmts.some( + (s) => s.kind === "if" && s.cond?.kind === "recordHas" && s.cond.field === "content", + ), + ); + expect(body).toBeDefined(); + const saved = body!.findIndex( + (s) => s.kind === "varDecl" && s.localId?.includes("partialWriteValue") === true, + ); + const check = body!.findIndex((s) => s.kind === "if"); + expect(saved).toBeGreaterThanOrEqual(0); + expect(saved).toBeLessThan(check); +}); diff --git a/tests/corpus/record-partial-assertion.ts b/tests/corpus/record-partial-assertion.ts index 6a9b2c93b5..e991299c43 100644 --- a/tests/corpus/record-partial-assertion.ts +++ b/tests/corpus/record-partial-assertion.ts @@ -22,4 +22,30 @@ console.log(render(false, "out")); const full: TruncationResult = { content: "c", truncated: false, truncatedBy: null, totalLines: 3, firstLineExceedsLimit: false }; const p = new Panel(); p.setComplete(full); console.log(p.summary(), p.dump(), p.keys(), p.hasContent(), full.totalLines + 1); + +// An optional field the literal omits reads as undefined, and `in` answers +// false for it. +interface Note { content: string; truncated: boolean; note?: string; count?: number } +function note(flag: boolean): Note { + return flag ? ({ truncated: true } as Note) : { content: "c", truncated: false, note: "n" }; +} +const omitted = note(true); +console.log(omitted.note, omitted.note ?? "none", omitted.count === undefined, typeof omitted.note); +console.log("note" in omitted, "truncated" in omitted, "content" in omitted); +const written = note(false); +console.log(written.note, "note" in written, "count" in written); + +// A write of a field the literal omits evaluates its right side first. +const order: string[] = []; +function rhs(): string { + order.push("rhs"); + return "v"; +} +const target = note(true); +try { + target.content = rhs(); +} catch { + order.push("caught"); +} +console.log(order[0]); export {};