From 6f7d1960677efbe0835b85b20c766cda166d8f7a Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Fri, 23 Oct 2026 06:47:08 -0500 Subject: [PATCH] Count records built for callbacks and replacements as built at run time The key-order witness treats records JavaScript builds as having a run-time order. Two cases this change narrowed are counted again: - A native callback that a checked JavaScript function finds inside a record or array it receives is called with records JavaScript builds. The walk of a checked function's parameters reset to the built direction at the fields and elements of the records it passes out, so the callback's own parameters counted as passed out. Children of a container passed out now count in both directions (written by JavaScript, and read out of it), and class instances keep the direction too. - A replaceable method's checked result builds a record when a replacement returns one. It is left out only while every site the census found for the method name reads it or assigns a function whose return type is primitive, as the handlers a router class installs through a computed key do. The census now records every such site (Lowerer.methodAccessSites). --- .../src/frontend/lowering/class-callbacks.ts | 8 +- .../lowering/class-method-mutations.ts | 18 +-- .../lowering/key-order-witness.test.ts | 34 ++++-- .../frontend/lowering/key-order-witness.ts | 108 ++++++++++++++---- .../src/frontend/lowering/lower-classes.ts | 4 +- .../compiler/src/frontend/lowering/lowerer.ts | 17 +++ .../main.ts.json | 7 ++ .../lib.js | 11 ++ .../main.ts | 50 ++++++++ .../main.ts.json | 1 + 10 files changed, 215 insertions(+), 43 deletions(-) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7623-key-order-javascript-built-records/main.ts.json create mode 100644 tests/corpus/7623-key-order-javascript-built-records/lib.js create mode 100644 tests/corpus/7623-key-order-javascript-built-records/main.ts create mode 100644 tests/harness/corpus-costs/7623-key-order-javascript-built-records/main.ts.json diff --git a/packages/compiler/src/frontend/lowering/class-callbacks.ts b/packages/compiler/src/frontend/lowering/class-callbacks.ts index f92f42a80..3a1c7975d 100644 --- a/packages/compiler/src/frontend/lowering/class-callbacks.ts +++ b/packages/compiler/src/frontend/lowering/class-callbacks.ts @@ -159,8 +159,8 @@ export function classCallbackValue( /** Notes a checked conversion of a replaceable method's result for the * key-order witness: the result is a value the method or its replacement * returns, not a record built from a dynamic value. */ -function methodResult(lowerer: Lowerer, checked: IrExpr): IrExpr { - return checked.kind === "dynCheck" ? lowerer.keyOrder.methodResult(checked) : checked; +function methodResult(lowerer: Lowerer, checked: IrExpr, name: string): IrExpr { + return checked.kind === "dynCheck" ? lowerer.keyOrder.methodResult(checked, name) : checked; } export function classCallbackCall( @@ -204,7 +204,7 @@ export function classCallbackCall( // generator arm does not box) keeps its native type: the unchanged // method's result stays as it is, and a replacement's dynamic result // takes the checked conversion into that type. - const converted = methodResult(lowerer, lowerer.coerceToExpected(call, fallback.type)); + const converted = methodResult(lowerer, lowerer.coerceToExpected(call, fallback.type), name); if (!typeEquals(converted.type, fallback.type)) { lowerer.pushDiag( unsupportedDiag( @@ -243,7 +243,7 @@ export function classCallbackCall( answer = fallback.type.kind === "void" ? result - : methodResult(lowerer, lowerer.coerceToExpected(result, fallback.type)); + : methodResult(lowerer, lowerer.coerceToExpected(result, fallback.type), name); } // Read after observedSlot built the prototype data and its originals. const originalId = observed ? original?.originalId() : undefined; diff --git a/packages/compiler/src/frontend/lowering/class-method-mutations.ts b/packages/compiler/src/frontend/lowering/class-method-mutations.ts index fd23b6db8..035c2c2a5 100644 --- a/packages/compiler/src/frontend/lowering/class-method-mutations.ts +++ b/packages/compiler/src/frontend/lowering/class-method-mutations.ts @@ -84,7 +84,7 @@ export function collectClassMethodMutations( for (const name of literalValues(lowerer.typeOf(key)) ?? []) methods.add(String(name)); const markName = (name: string, site: ts.Node, arrow = false): void => { if (name === "prototype" || name === "__proto__") unknownKey = site; - if (methods.has(name)) lowerer.prototypeMethodAccesses.set(name, site); + if (methods.has(name)) lowerer.noteMethodAccess(name, site); if (!arrow) receiverFields.add(name); }; const unwrap = (value: ts.Expression): ts.Expression => { @@ -395,10 +395,9 @@ export function collectClassMethodMutations( ts.isStringLiteralLike(base.argumentExpression) && base.argumentExpression.text === "prototype"); if (prototype) { - if (ts.isPropertyAccessExpression(node)) - lowerer.prototypeMethodAccesses.set(node.name.text, node); + if (ts.isPropertyAccessExpression(node)) lowerer.noteMethodAccess(node.name.text, node); else if (ts.isStringLiteralLike(node.argumentExpression)) - lowerer.prototypeMethodAccesses.set(node.argumentExpression.text, node); + lowerer.noteMethodAccess(node.argumentExpression.text, node); else markKey(node.argumentExpression, node); } } @@ -435,7 +434,7 @@ export function collectClassMethodMutations( ? lowerer.checker.getSymbolAtLocation(access.name) : lowerer.checker.getPropertyOfType(lowerer.typeOf(access.expression), name); if (symbol && symbol.declarations.some(ts.isMethodDeclaration)) - lowerer.prototypeMethodAccesses.set(name, access); + lowerer.noteMethodAccess(name, access); } lowerer.checker.prefetchSymbolRoots([...globals, ...reflection]); for (const node of globals) @@ -571,9 +570,10 @@ export function collectClassMethodMutations( ); for (const { target, site } of propertyWrites) { const name = target.name.text; - if (lowerer.prototypeMethodAccesses.has(name) || functionFree(lowerer.typeOf(target))) continue; - if (!methodOwnerCanInhabit(target.expression, name)) continue; - lowerer.prototypeMethodAccesses.set(name, site); + if (functionFree(lowerer.typeOf(target)) || !methodOwnerCanInhabit(target.expression, name)) + continue; + if (lowerer.prototypeMethodAccesses.has(name)) lowerer.noteMethodSite(name, site); + else lowerer.noteMethodAccess(name, site); } for (const { key, receiver, site } of computedWrites) { // A class method declared with a well-known symbol key is not a write; @@ -620,7 +620,7 @@ export function collectClassMethodMutations( } } if (unknownKey) { - for (const method of methods) lowerer.prototypeMethodAccesses.set(method, unknownKey); + for (const method of methods) lowerer.noteMethodAccess(method, unknownKey); for (const method of symbolMethods) lowerer.replacedSymbolMethods.add(method); } if (!unknownKey && !unknownOwnField) 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 dc9149119..8058d0066 100644 --- a/packages/compiler/src/frontend/lowering/key-order-witness.test.ts +++ b/packages/compiler/src/frontend/lowering/key-order-witness.test.ts @@ -15,6 +15,7 @@ import { type KeyOrderSegment, } from "./key-order-witness.js"; import type { Lowerer } from "./lowerer.js"; +import * as ts from "../ts7/adapter.js"; const loc = { file: "order.ts", start: 0, end: 1 }; /** `number | undefined`: the optional-field slot type. */ @@ -40,6 +41,7 @@ function fixture(): Fixture { keyOrder: notes, unions: new Map([["u0", { id: "u0", arms: [F64, { kind: "undefinedT" }] }]]), classes: new Map(), + methodAccessSites: new Map(), armTag: (unionId: string, arm: IrType): number => unionId === "u0" && arm.kind === "undefinedT" ? 1 : -1, recordFieldPresent: (obj: IrExpr, shapeId: string, field: string): IrExpr => ({ @@ -739,20 +741,38 @@ test("a replaceable method's checked result builds no record", () => { expect( selfCopy((f, ab) => { const result: IrExpr = { kind: "dynCheck", value: ref("r", { kind: "dyn" }), type: ab, loc }; - return [observe(f.notes.methodResult(result))]; + return [observe(f.notes.methodResult(result, "load"))]; }), ).toEqual(["b", "a"]); }); +test("a replaceable method's checked result builds records when a replacement may return one", () => { + expect( + selfCopy((f, ab) => { + // A site the census cannot see through (a computed write of some + // value, a reflection call) may install a replacement returning a + // record built in any order. + (f.lowerer.methodAccessSites as Map).set("load", [ + { kind: ts.SyntaxKind.CallExpression } as unknown as ts.Node, + ]); + const result: IrExpr = { kind: "dynCheck", value: ref("r", { kind: "dyn" }), type: ab, loc }; + return [observe(f.notes.methodResult(result, "load"))]; + }), + ).toEqual(["a", "b"]); +}); + test("a replaceable method's result keeps its witness through an IR rewrite", () => { expect( selfCopy((f, ab) => { - const result = f.notes.methodResult({ - kind: "dynCheck", - value: ref("r", { kind: "dyn" }), - type: ab, - loc, - }); + const result = f.notes.methodResult( + { + kind: "dynCheck", + value: ref("r", { kind: "dyn" }), + type: ab, + loc, + }, + "load", + ); const identity = { expr: (e: IrExpr): IrExpr => e, stmt: (st: IrStmt): IrStmt => st }; observeRecordLiteralRebuilds((from, to) => f.notes.follow(from, to)); try { diff --git a/packages/compiler/src/frontend/lowering/key-order-witness.ts b/packages/compiler/src/frontend/lowering/key-order-witness.ts index 2f573e676..933c9efcd 100644 --- a/packages/compiler/src/frontend/lowering/key-order-witness.ts +++ b/packages/compiler/src/frontend/lowering/key-order-witness.ts @@ -69,6 +69,7 @@ import { transformStmtList, } from "../../ir/traverse.js"; import type { Lowerer } from "./lowerer.js"; +import * as ts from "../ts7/adapter.js"; import { ALWAYS_THROW_LIB_FNS } from "../../ir/builtin-effects.js"; /** The statements a key-order helper runs for record `ref`: `walks[0]`, @@ -124,7 +125,7 @@ export type KeyOrderSegment = export class KeyOrderNotes { private readonly literals = new Map(); private readonly copies = new Map(); - private readonly methodResults = new Set(); + private readonly methodResults = new Map(); /** Shapes a frontend order surface (keys, values/entries, inspect, * index-signature merges) walks. */ readonly observed = new Set(); @@ -141,10 +142,10 @@ export class KeyOrderNotes { return node; } - /** The checked result of a typed call of a method the program may - * replace on its instance (class-callbacks.ts classCallbackCall). */ - methodResult(node: IrExpr): IrExpr { - this.methodResults.add(node); + /** The checked result of a typed call of method `name`, which the program + * may replace on its instance (class-callbacks.ts classCallbackCall). */ + methodResult(node: IrExpr, name: string): IrExpr { + this.methodResults.set(node, name); return node; } @@ -155,7 +156,8 @@ export class KeyOrderNotes { if (segments !== undefined) this.literals.set(to, segments); const source = this.copies.get(from); if (source !== undefined) this.copies.set(to, source); - if (this.methodResults.has(from)) this.methodResults.add(to); + const method = this.methodResults.get(from); + if (method !== undefined) this.methodResults.set(to, method); } observe(type: IrType): void { @@ -170,8 +172,9 @@ export class KeyOrderNotes { return this.copies.get(node); } - isMethodResult(node: IrExpr): boolean { - return this.methodResults.has(node); + /** The method whose checked result `node` is, if it is one. */ + methodResultOf(node: IrExpr): string | undefined { + return this.methodResults.get(node); } } @@ -243,6 +246,54 @@ function primitiveSource(node: IrExpr & { kind: "dynCheck" }): boolean { return node.value.kind === "dynFrom" && PRIMITIVE_KINDS.has(node.value.value.type.kind); } +/** Whether a census site for a method name can only read the method or + * install a replacement whose result holds no object: a read + * (`C.prototype.m`, `obj.m`) outside an assignment target, or an assignment + * of a function expression whose return type is primitive. Other sites (a + * computed write of another value, a reflection call, a property bag) may + * install a replacement that returns a record. */ +function replacesWithoutRecords(lowerer: Lowerer, site: ts.Node): boolean { + const parent = site.parent; + if (ts.isPropertyAccessExpression(site) || ts.isElementAccessExpression(site)) + return !( + parent !== undefined && + ts.isBinaryExpression(parent) && + parent.left === site && + parent.operatorToken.kind >= ts.SyntaxKind.FirstAssignment && + parent.operatorToken.kind <= ts.SyntaxKind.LastAssignment + ); + if (!ts.isBinaryExpression(site) || site.operatorToken.kind !== ts.SyntaxKind.EqualsToken) + return false; + let value = site.right; + while (ts.isParenthesizedExpression(value)) value = value.expression; + if (!ts.isArrowFunction(value) && !ts.isFunctionExpression(value)) return false; + const signatures = lowerer.checker.getCallSignatures(lowerer.typeOf(value)); + return ( + signatures.length > 0 && + signatures.every((signature) => + primitiveOnly(lowerer.checker.getReturnTypeOfSignature(signature)), + ) + ); +} + +/** A type whose values are primitives (or void): it holds no record. */ +function primitiveOnly(type: ts.Type): boolean { + if (type.isUnionType()) return ts.constituentTypes(type).every(primitiveOnly); + return ( + (type.flags & + (ts.TypeFlags.StringLike | + ts.TypeFlags.NumberLike | + ts.TypeFlags.BooleanLike | + ts.TypeFlags.BigIntLike | + ts.TypeFlags.ESSymbolLike | + ts.TypeFlags.Null | + ts.TypeFlags.Undefined | + ts.TypeFlags.Void | + ts.TypeFlags.Never)) !== + 0 + ); +} + /** Chooses each in-scope record shape's enumeration table from its * witnesses. Runs once retained reachability has settled the historical * tables and derived metadata, before the order-snapshotting helper bodies @@ -294,16 +345,24 @@ export function applyKeyOrderWitnesses(lowerer: Lowerer, fns: readonly IrFunctio const shapesIn = (type: IrType, out: Set, seen: Set, produced = false): void => { const visit = (t: IrType, built: boolean): boolean => { const mark = built ? "" : "-"; + // A container JavaScript builds converts into fresh native storage, so + // its children are built too. One passed out stays shared: JavaScript + // writes into it (built children) and reads native values out of it, + // such as callbacks whose parameters it then builds (children passed + // out). Those children count in both directions. + const child = (type: IrType): boolean => { + visit(type, true); + if (!built) visit(type, false); + return true; + }; if (t.kind === "record") { if (seen.has(mark + "r:" + t.shapeId)) return true; seen.add(mark + "r:" + t.shapeId); if (built) out.add(t.shapeId); const shape = shapes.get(t.shapeId); if (shape) { - // A record passed out can be written through: its fields take - // values JavaScript builds. - for (const f of shape.fields) visit(f.type, true); - if (shape.indexValue) visit(shape.indexValue, true); + for (const f of shape.fields) child(f.type); + if (shape.indexValue) child(shape.indexValue); } return true; } @@ -315,17 +374,16 @@ export function applyKeyOrderWitnesses(lowerer: Lowerer, fns: readonly IrFunctio } if (t.kind === "object") { if (produced && t.className !== "%Error") return true; - if (seen.has("c:" + t.className)) return true; - seen.add("c:" + t.className); - for (const f of lowerer.classes.get(t.className)?.def.fields ?? []) visit(f.type, true); + if (seen.has(mark + "c:" + t.className)) return true; + seen.add(mark + "c:" + t.className); + for (const f of lowerer.classes.get(t.className)?.def.fields ?? []) child(f.type); return true; } if (produced && t.kind === "func") { for (const param of t.params) visit(param, !built); return visit(t.ret, built); } - // Containers passed out stay writable: their elements count as built. - return everyTypeChild(t, (child) => visit(child, true)); + return everyTypeChild(t, child); }; visit(type, true); }; @@ -338,6 +396,14 @@ export function applyKeyOrderWitnesses(lowerer: Lowerer, fns: readonly IrFunctio * declaration's order, the order their table holds. */ const declaredShapes = new Set(); const declaredSeen = new Set(); + /** The checked result of a replaceable method call while every site that + * can replace the method installs functions that return no record. */ + const unchangedMethodResult = (node: IrExpr): boolean => { + const name = notes.methodResultOf(node); + if (name === undefined) return false; + const sites = lowerer.methodAccessSites.get(name) ?? []; + return sites.every((site) => replacesWithoutRecords(lowerer, site)); + }; const producedBy = (type: IrType, opaque: boolean): void => { if (opaque) shapesIn(type, opaqueShapes, opaqueSeen); else shapesIn(type, runtimeShapes, runtimeSeen, true); @@ -449,10 +515,10 @@ export function applyKeyOrderWitnesses(lowerer: Lowerer, fns: readonly IrFunctio case "dynCheck": if (e.adopt === true) shapesIn(e.type, declaredShapes, declaredSeen); // Conversions that build no record: a boxed primitive, and a - // replaceable method's result, which is the value the method - // returns (boxed and checked back unchanged) or what a replacement - // installed on the instance returns. - else if (!orderPreservingClone(e) && !primitiveSource(e) && !notes.isMethodResult(e)) + // replaceable method's result while no replacement the program can + // install returns a record: then it is the method's own value, + // boxed and checked back unchanged. + else if (!orderPreservingClone(e) && !primitiveSource(e) && !unchangedMethodResult(e)) producedBy(e.type, false); return; case "dynDestrCheck": diff --git a/packages/compiler/src/frontend/lowering/lower-classes.ts b/packages/compiler/src/frontend/lowering/lower-classes.ts index 29afbb4fe..56c70c5bd 100644 --- a/packages/compiler/src/frontend/lowering/lower-classes.ts +++ b/packages/compiler/src/frontend/lowering/lower-classes.ts @@ -4147,7 +4147,7 @@ export function collectClassShapeInner( // An own JS callable shadows its prototype declaration. // Keep the method in the prototype and store the assigned // value in the instance property bag. - lowerer.prototypeMethodAccesses.set(name, assign); + lowerer.noteMethodAccess(name, assign); continue; } const sym = lowerer.checker.getSymbolAtLocation(assign); @@ -4229,7 +4229,7 @@ export function collectClassShapeInner( } if (!key && (fields.has(fieldName) || hasAccessor(fieldName))) continue; if (!key && (methods.has(fieldName) || lowerer.findMethodOn(base, fieldName))) { - lowerer.prototypeMethodAccesses.set(fieldName, lhs); + lowerer.noteMethodAccess(fieldName, lhs); continue; } // A key already declared (own or inherited) makes later diff --git a/packages/compiler/src/frontend/lowering/lowerer.ts b/packages/compiler/src/frontend/lowering/lowerer.ts index e36ffd09f..0922f8928 100644 --- a/packages/compiler/src/frontend/lowering/lowerer.ts +++ b/packages/compiler/src/frontend/lowering/lowerer.ts @@ -3021,6 +3021,23 @@ export class Lowerer { /** Named method slots observed before class collection need a callable * ABI and shared prototype descriptors instead of call-site specialization. */ readonly prototypeMethodAccesses = new Map(); + /** Every site the census found for a name in prototypeMethodAccesses (which + * keeps one): reads of the method and writes that may replace it. The + * key-order witness asks which replacements can return records. */ + readonly methodAccessSites = new Map(); + + /** Marks `name` as a method the program may read or replace at `site`. */ + noteMethodAccess(name: string, site: ts.Node): void { + this.prototypeMethodAccesses.set(name, site); + this.noteMethodSite(name, site); + } + + /** Records a site for `name` without changing its prototypeMethodAccesses entry. */ + noteMethodSite(name: string, site: ts.Node): void { + const sites = this.methodAccessSites.get(name); + if (sites === undefined) this.methodAccessSites.set(name, [site]); + else sites.push(site); + } /** Whether the program reads `Error.prototype`, the checked-dynamic * object an ES5 constructor function can inherit from, or has a class * whose heritage is evaluated at run time (`extends (Base ?? Error)`): diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7623-key-order-javascript-built-records/main.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7623-key-order-javascript-built-records/main.ts.json new file mode 100644 index 000000000..c3c2aa5b0 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7623-key-order-javascript-built-records/main.ts.json @@ -0,0 +1,7 @@ +{ + "order": [ + "/tests/corpus/7623-key-order-javascript-built-records/lib.js", + "/tests/corpus/7623-key-order-javascript-built-records/main.ts" + ], + "diags": [] +} diff --git a/tests/corpus/7623-key-order-javascript-built-records/lib.js b/tests/corpus/7623-key-order-javascript-built-records/lib.js new file mode 100644 index 000000000..4ff312846 --- /dev/null +++ b/tests/corpus/7623-key-order-javascript-built-records/lib.js @@ -0,0 +1,11 @@ +// Records built in JavaScript in another key order than the TypeScript +// literals of the same type: a replacement method installed through a +// computed key, and a function stored on the global object, read through a +// typed view, that calls a callback inside its options record. +export function install(target, name) { + target[name] = () => ({ a: 1, b: 2 }); +} + +globalThis.__eachRow = function (options) { + options.visit({ x: 5, y: 6 }); +}; diff --git a/tests/corpus/7623-key-order-javascript-built-records/main.ts b/tests/corpus/7623-key-order-javascript-built-records/main.ts new file mode 100644 index 000000000..ae4876131 --- /dev/null +++ b/tests/corpus/7623-key-order-javascript-built-records/main.ts @@ -0,0 +1,50 @@ +// Copies and enumerations of records JavaScript builds follow the order +// JavaScript wrote, also when the TypeScript literals of the type write +// another order and spread their own type. Each case uses its own type. +import { install } from "./lib.js"; + +interface Pair { + a: number; + b: number; +} + +interface Row { + x: number; + y: number; +} + +interface Options { + visit: (row: Row) => void; +} + +class Store { + get(): Pair { + return { b: 2, a: 1 }; + } +} + +function showPair(label: string, value: Pair): void { + const copy = { ...value }; + console.log(label, Object.keys(value).join(), JSON.stringify(copy), Object.keys(copy).join()); +} + +function showRow(label: string, value: Row): void { + const copy = { ...value }; + console.log(label, Object.keys(value).join(), JSON.stringify(copy), Object.keys(copy).join()); +} + +// A replaceable method whose replacement returns a record JavaScript builds. +const store = new Store(); +showPair("method", store.get()); +showPair("self copy", { ...store.get(), b: 3 }); +install(store, "get"); +showPair("replaced", store.get()); + +// A JavaScript function, checked into a typed slot, calls a native callback +// it receives inside its options record with a record it builds. +const literal: Row = { y: 20, x: 10 }; +showRow("literal", literal); +showRow("self copy", { ...literal, y: 21 }); +const eachRow = (globalThis as typeof globalThis & { __eachRow: (options: Options) => void }) + .__eachRow; +eachRow({ visit: (row: Row) => showRow("checked function", row) }); diff --git a/tests/harness/corpus-costs/7623-key-order-javascript-built-records/main.ts.json b/tests/harness/corpus-costs/7623-key-order-javascript-built-records/main.ts.json new file mode 100644 index 000000000..4ffc07c5a --- /dev/null +++ b/tests/harness/corpus-costs/7623-key-order-javascript-built-records/main.ts.json @@ -0,0 +1 @@ +{ "plain": 6182, "san": 10202 }