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).

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;

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)

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<string, ts.Node[]>).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 {

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<IrExpr, KeyOrderSegment[]>();
   private readonly copies = new Map<IrExpr, string>();
-  private readonly methodResults = new Set<IrExpr>();
+  private readonly methodResults = new Map<IrExpr, string>();
   /** Shapes a frontend order surface (keys, values/entries, inspect,
    * index-signature merges) walks. */
   readonly observed = new Set<string>();
@@ -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<string>, seen: Set<string>, 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<string>();
   const declaredSeen = new Set<string>();
+  /** 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":

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

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<string, ts.Node>();
+  /** 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<string, ts.Node[]>();
+
+  /** 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)`):

packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7623-key-order-javascript-built-records/main.ts.json

@@ -0,0 +1,7 @@
+{
+ "order": [
+  "<repo>/tests/corpus/7623-key-order-javascript-built-records/lib.js",
+  "<repo>/tests/corpus/7623-key-order-javascript-built-records/main.ts"
+ ],
+ "diags": []
+}

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 });
+};

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) });

tests/harness/corpus-costs/7623-key-order-javascript-built-records/main.ts.json

@@ -0,0 +1 @@
+{ "plain": 6182, "san": 10202 }