From 6f373b983b4e4b26a144078b57b4be59b13aeaec Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Sun, 25 Oct 2026 22:26:16 -0500 Subject: [PATCH] Keep every field of a record converted into a union arm A record that converts into several arms of a union by a width conversion preferred the unique arm whose fields the record has or that are undefined-only, even when that arm holds the record only by dropping fields and another arm keeps every field but declares an optional field the record lacks (`adapts?: true`). The record was wrapped as the dropping arm: - In a discriminated union the value kept its discriminant but had the other arm's tag, so narrowing on the discriminant stopped the program with "a narrowed value does not have the type its narrowing proved". - Without a discriminant the dropped fields were lost: `"fnName" in value` was false and JSON.stringify left them out. The native compiler hit the first case on tests/corpus/2910-overloaded-callable-alias.ts: the closure that a value read of an overloaded function alias builds is a record that is not typed by IrExpr, and it converted into the `selfRef` arm. The undefined-only preference no longer selects an arm that drops fields when another candidate keeps every field, so a discriminated union selects the arm by the record's discriminant. The lossless-arm preference still prefers the unique arm with exactly the record's fields; when there is none, it accepts the unique arm that keeps every field and declares optional fields the record lacks, which complete as absent. --- .../lowering/coercions/structural-plans.ts | 53 +++++++++++++----- .../union-record-kept-fields-arm.ts.json | 6 ++ ...record-kept-fields-no-discriminant.ts.json | 6 ++ tests/corpus/union-record-kept-fields-arm.ts | 56 +++++++++++++++++++ ...nion-record-kept-fields-no-discriminant.ts | 39 +++++++++++++ .../union-record-kept-fields-arm.ts.json | 1 + ...record-kept-fields-no-discriminant.ts.json | 1 + 7 files changed, 147 insertions(+), 15 deletions(-) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/union-record-kept-fields-arm.ts.json create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/union-record-kept-fields-no-discriminant.ts.json create mode 100644 tests/corpus/union-record-kept-fields-arm.ts create mode 100644 tests/corpus/union-record-kept-fields-no-discriminant.ts create mode 100644 tests/harness/corpus-costs/union-record-kept-fields-arm.ts.json create mode 100644 tests/harness/corpus-costs/union-record-kept-fields-no-discriminant.ts.json diff --git a/packages/compiler/src/frontend/lowering/coercions/structural-plans.ts b/packages/compiler/src/frontend/lowering/coercions/structural-plans.ts index bff419d48..2a394c4d1 100644 --- a/packages/compiler/src/frontend/lowering/coercions/structural-plans.ts +++ b/packages/compiler/src/frontend/lowering/coercions/structural-plans.ts @@ -1,6 +1,6 @@ import { planRecordUnionLift, planRecordUnionWrap } from "../union-retag.js"; import type { WidthLift } from "../width-lift.js"; -import type { IrType } from "../../../ir/ir.js"; +import type { IrRecordShape, IrType } from "../../../ir/ir.js"; import { canDynCheckTo, isUnitType, typeEquals, UNDEFINED_T } from "../../../ir/ir.js"; import { type ClassInfo, findMethodOn, findGenericMethodOn } from "../lower-classes.js"; import type { Lowerer } from "../lowerer.js"; @@ -104,7 +104,15 @@ export function widthLiftPlan(lowerer: Lowerer, src: IrType, dst: IrType): Width }) ); }); - if (absentOnly.length === 1) selected = absentOnly; + // That layout does not win when it holds the record only by dropping + // fields and another arm keeps every field (`{ kind; type }` beside + // `{ kind; name; adapts?: true; type }`): the value is not its variant. + const dropsFields = (arm: IrType): boolean => !keepsEveryField(lowerer, source, arm); + if ( + absentOnly.length === 1 && + !(dropsFields(absentOnly[0]!.arm) && selected.some(({ arm }) => !dropsFields(arm))) + ) + selected = absentOnly; } // A record that fits several arms of a discriminated union selects one // by its discriminant at run time. Without this, an arm that a nested @@ -131,21 +139,19 @@ export function widthLiftPlan(lowerer: Lowerer, src: IrType, dst: IrType): Width // A record whose fields some arm keeps EVERY one of, each lifting // (`{ id; limit; opts?: A | B }` into `{ id; limit; opts?: A | B | C } | // { id; opts?: C }`): that arm is the value's own layout, widened; the - // others hold it only by dropping fields. Prefer the unique lossless arm. + // others hold it only by dropping fields. Prefer the unique lossless arm + // with the record's own fields, then the unique one that also declares + // optional fields the record lacks, which complete as absent. if (selected.length > 1 && src.kind === "record") { const source = lowerer.shapes.get(src.shapeId); - const lossless = selected.filter(({ arm }) => { - const target = arm.kind === "record" ? lowerer.shapes.get(arm.shapeId) : undefined; - return ( - source !== undefined && - target !== undefined && - !source.tuple && - !target.tuple && - target.fields.length === source.fields.length && - source.fields.every((field) => target.fields.some((kept) => kept.name === field.name)) - ); - }); - if (lossless.length === 1) selected = lossless; + const lossless = selected.filter(({ arm }) => keepsEveryField(lowerer, source, arm)); + const exact = lossless.filter( + ({ arm }) => + arm.kind === "record" && + lowerer.shapes.get(arm.shapeId)?.fields.length === source?.fields.length, + ); + if (exact.length === 1) selected = exact; + else if (exact.length === 0 && lossless.length === 1) selected = lossless; } if (selected.length !== 1) return null; return { how: "liftWrap", tag: selected[0]!.tag, arm: selected[0]!.arm }; @@ -241,6 +247,23 @@ export function widthLiftPlan(lowerer: Lowerer, src: IrType, dst: IrType): Width return null; } +/** True when the record arm `arm` has a field of every name the record + * `source` has. Tuples keep their own positional layouts. */ +function keepsEveryField( + lowerer: Lowerer, + source: IrRecordShape | undefined, + arm: IrType, +): boolean { + const target = arm.kind === "record" ? lowerer.shapes.get(arm.shapeId) : undefined; + return ( + source !== undefined && + target !== undefined && + !source.tuple && + !target.tuple && + source.fields.every((field) => target.fields.some((kept) => kept.name === field.name)) + ); +} + /** Two record arms a source record fits, where one arm declares a field * the other lacks and the source types that field `T | undefined` * (`{ count, errors: errs.length ? errs : undefined }` into `{ count; diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/union-record-kept-fields-arm.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/union-record-kept-fields-arm.ts.json new file mode 100644 index 000000000..a29a9d99a --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/union-record-kept-fields-arm.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/union-record-kept-fields-arm.ts" + ], + "diags": [] +} diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/union-record-kept-fields-no-discriminant.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/union-record-kept-fields-no-discriminant.ts.json new file mode 100644 index 000000000..f40acc997 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/union-record-kept-fields-no-discriminant.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/union-record-kept-fields-no-discriminant.ts" + ], + "diags": [] +} diff --git a/tests/corpus/union-record-kept-fields-arm.ts b/tests/corpus/union-record-kept-fields-arm.ts new file mode 100644 index 000000000..69887cf49 --- /dev/null +++ b/tests/corpus/union-record-kept-fields-arm.ts @@ -0,0 +1,56 @@ +// A record flowing into a discriminated union where one record arm holds it +// by dropping fields and another keeps every field but declares an optional +// field the record lacks. The record's discriminant selects the arm that keeps +// its fields. + +type Loc = { file: string; start: number }; + +type Expr = + | { kind: "ref"; type: number; loc: Loc } + | { + kind: "closure"; + fnName: string; + captures: string[]; + adapts?: true; + type: number; + loc: Loc; + } + | { kind: "call"; callee: string; args: Expr[]; type: number; loc: Loc }; + +function closureOf(name: string, loc: Loc): Expr { + const closure = { + kind: "closure" as const, + fnName: name, + captures: [], + type: 1, + loc, + }; + return closure; +} + +function describe(expr: Expr): string { + if (expr.kind === "closure") + return `closure ${expr.fnName}/${expr.captures.length} ${expr.adapts ?? "-"}`; + if (expr.kind === "call") + return `call ${expr.callee}(${expr.args.map(describe).join(", ")})`; + return `ref ${expr.type}`; +} + +const loc: Loc = { file: "a.ts", start: 3 }; +const first = closureOf("pick", loc); +console.log(first.kind, describe(first), JSON.stringify(first)); + +const exprs: Expr[] = [first]; +const captured = { + kind: "closure" as const, + fnName: "inner", + captures: ["x"], + type: 2, + loc, +}; +exprs.push(captured); +exprs.push({ kind: "ref", type: 7, loc }); +console.log( + describe({ kind: "call", callee: "run", args: exprs, type: 0, loc }), +); +console.log(exprs.map((e) => e.kind).join(",")); diff --git a/tests/corpus/union-record-kept-fields-no-discriminant.ts b/tests/corpus/union-record-kept-fields-no-discriminant.ts new file mode 100644 index 000000000..45370f152 --- /dev/null +++ b/tests/corpus/union-record-kept-fields-no-discriminant.ts @@ -0,0 +1,39 @@ +// A record flowing into a union without a discriminant where one record arm +// holds it by dropping fields and another keeps every field but declares an +// optional field the record lacks. The record converts into the arm that +// keeps its fields, and the optional field stays absent. + +type Loc = { file: string; start: number }; + +type Slot = + | { type: number; loc: Loc } + | { + fnName: string; + captures: string[]; + adapts?: true; + type: number; + loc: Loc; + }; + +const loc: Loc = { file: "a.ts", start: 3 }; + +function slotOf(name: string): Slot { + const slot = { fnName: name, captures: [], type: 1, loc }; + return slot; +} + +const slot = slotOf("stored"); +console.log("fnName" in slot, "adapts" in slot, JSON.stringify(slot)); +if ("fnName" in slot) + console.log(slot.fnName, slot.adapts, slot.captures.length); + +function describe(value: Slot): string { + return "fnName" in value + ? `${value.fnName}/${value.captures.length}` + : `type ${value.type}`; +} + +const inner = { fnName: "inner", captures: ["x"], type: 2, loc }; +const slots: Slot[] = [slot, { type: 7, loc }]; +slots.push(inner); +console.log(slots.map(describe).join(", "), JSON.stringify(slots[2])); diff --git a/tests/harness/corpus-costs/union-record-kept-fields-arm.ts.json b/tests/harness/corpus-costs/union-record-kept-fields-arm.ts.json new file mode 100644 index 000000000..3c750a2dc --- /dev/null +++ b/tests/harness/corpus-costs/union-record-kept-fields-arm.ts.json @@ -0,0 +1 @@ +{ "plain": 4793, "san": 7135 } diff --git a/tests/harness/corpus-costs/union-record-kept-fields-no-discriminant.ts.json b/tests/harness/corpus-costs/union-record-kept-fields-no-discriminant.ts.json new file mode 100644 index 000000000..5e891c40e --- /dev/null +++ b/tests/harness/corpus-costs/union-record-kept-fields-no-discriminant.ts.json @@ -0,0 +1 @@ +{ "plain": 4466, "san": 5253 }