Merge pull request #772 from vercel-labs/fix/method-census-data-writes

Fix compiling tsc-ts: keep data writes out of the method replacement census

packages/compiler/src/frontend/lowering/class-method-mutations.ts

@@ -3,6 +3,26 @@ import { literalValues } from "../literal-values.js";
 import { isJsSourceFile } from "../program.js";
 import type { Lowerer } from "./lowerer.js";
 
+/** Members every function value has (Function.prototype and
+ * Object.prototype, plus own `length`/`name`/`prototype`). */
+const FUNCTION_MEMBER_NAMES = new Set([
+  "apply",
+  "arguments",
+  "bind",
+  "call",
+  "caller",
+  "constructor",
+  "hasOwnProperty",
+  "isPrototypeOf",
+  "length",
+  "name",
+  "propertyIsEnumerable",
+  "prototype",
+  "toLocaleString",
+  "toString",
+  "valueOf",
+]);
+
 /** Census observable slots before choosing native calls. Names are shared
  * conservatively across classes: aliases cannot hide a named replacement. */
 export function collectClassMethodMutations(
@@ -17,6 +37,7 @@ export function collectClassMethodMutations(
     receiver: ts.Expression | undefined;
     site: ts.Node;
   }[] = [];
+  const propertyWrites: { target: ts.PropertyAccessExpression; site: ts.Node }[] = [];
   const reflection: ts.PropertyAccessExpression[] = [];
   const globals: ts.Identifier[] = [];
   const jsAssignments: (ts.PropertyAccessExpression | ts.ElementAccessExpression)[] = [];
@@ -88,26 +109,53 @@ export function collectClassMethodMutations(
       lowerer.stdlibGlobalMember(value.expression, "Object") === "create"
     );
   };
-  const primitiveDictionary = (value: ts.Expression): boolean => {
-    const primitive = (type: ts.Type): boolean =>
-      type.isUnionType()
-        ? ts.constituentTypes(type).every(primitive)
-        : (type.flags &
-            (ts.TypeFlags.StringLike |
-              ts.TypeFlags.NumberLike |
-              ts.TypeFlags.BooleanLike |
-              ts.TypeFlags.BigIntLike |
-              ts.TypeFlags.Null |
-              ts.TypeFlags.Undefined |
-              ts.TypeFlags.ESSymbolLike)) !==
-          0;
-    return lowerer.checker
+  // Whether no function value can inhabit `type`. Primitives qualify, and so
+  // do non-callable object types a function is not assignable to: one that
+  // requires a property functions lack, or a weak (all-optional) type that
+  // shares no property with functions (TypeScript's weak-type rule rejects
+  // callable sources too).
+  const functionFree = (type: ts.Type): boolean => {
+    if (type.isUnionType()) return ts.constituentTypes(type).every(functionFree);
+    if (
+      (type.flags &
+        (ts.TypeFlags.StringLike |
+          ts.TypeFlags.NumberLike |
+          ts.TypeFlags.BooleanLike |
+          ts.TypeFlags.BigIntLike |
+          ts.TypeFlags.Null |
+          ts.TypeFlags.Undefined |
+          ts.TypeFlags.ESSymbolLike)) !==
+      0
+    )
+      return true;
+    if (
+      !(type.flags & ts.TypeFlags.Object) ||
+      lowerer.checker.getCallSignatures(type).length > 0 ||
+      lowerer.checker.getConstructSignatures(type).length > 0
+    )
+      return false;
+    const properties = lowerer.checker.getPropertiesOfType(type);
+    if (properties.length === 0) return false;
+    const required = properties.filter(
+      (property) => (property.flags & ts.SymbolFlags.Optional) === 0,
+    );
+    if (required.some((property) => !FUNCTION_MEMBER_NAMES.has(property.name))) return true;
+    return (
+      required.length === 0 &&
+      properties.every((property) => !FUNCTION_MEMBER_NAMES.has(property.name))
+    );
+  };
+  // A string index whose values cannot be functions excludes callable
+  // method slots. Class instances have no implicit index signature, so such
+  // a dictionary is only a class view when the class declares that index
+  // signature, and then every method must be assignable to its value type.
+  const functionFreeDictionary = (value: ts.Expression): boolean =>
+    lowerer.checker
       .getIndexInfosOfType(lowerer.typeOf(value))
       .some(
         (index) =>
-          (index.keyType.flags & ts.TypeFlags.StringLike) !== 0 && primitive(index.valueType),
+          (index.keyType.flags & ts.TypeFlags.StringLike) !== 0 && functionFree(index.valueType),
       );
-  };
   const literalLoopKeys = (key: ts.Expression): (string | number)[] | null => {
     if (!ts.isIdentifier(key)) return null;
     const symbol = lowerer.checker.getSymbolAtLocation(key);
@@ -201,8 +249,15 @@ export function collectClassMethodMutations(
   };
   const markTarget = (target: ts.Expression, site: ts.Node, arrow = false): void => {
     target = unwrap(target);
-    if (ts.isPropertyAccessExpression(target)) markName(target.name.text, site, arrow);
-    else if (ts.isElementAccessExpression(target))
+    if (ts.isPropertyAccessExpression(target)) {
+      const name = target.name.text;
+      // A named write to a method name is judged by the written property's
+      // type once types are prefetched (see propertyWrites below).
+      if (methods.has(name) && name !== "prototype" && name !== "__proto__") {
+        if (!arrow) receiverFields.add(name);
+        propertyWrites.push({ target, site });
+      } else markName(name, site, arrow);
+    } else if (ts.isElementAccessExpression(target))
       markKey(target.argumentExpression, site, target.expression);
     else if (ts.isArrayLiteralExpression(target)) {
       for (const element of target.elements) {
@@ -392,6 +447,20 @@ export function collectClassMethodMutations(
       ...(write.receiver ? [write.receiver] : []),
     ]),
   );
+  // A named write can replace a method only when the written property can
+  // hold a function: a class instance seen through the receiver's type must
+  // have its method assignable to that property's type. Data fields (an AST
+  // node, a string, a JSON record) cannot shadow a same-named method of an
+  // unrelated class, so they stay out of the observed-slot census.
+  lowerer.checker.prefetchClassCollection(
+    propertyWrites.map((write) => write.target),
+    [],
+  );
+  for (const { target, site } of propertyWrites) {
+    const name = target.name.text;
+    if (lowerer.prototypeMethodAccesses.has(name) || functionFree(lowerer.typeOf(target))) continue;
+    lowerer.prototypeMethodAccesses.set(name, site);
+  }
   for (const { key, receiver, site } of computedWrites) {
     const type = key ? lowerer.typeOf(key) : undefined;
     const names = type && (literalValues(type) ?? (key ? literalLoopKeys(key) : null));
@@ -406,9 +475,10 @@ export function collectClassMethodMutations(
       unknownOwnField = true;
       continue;
     }
-    // A primitive-only string index excludes callable method slots. Unlike
-    // structural interfaces, it cannot be a live class-method view.
-    if (receiver && (freshReceiver(receiver) || primitiveDictionary(receiver))) continue;
+    // A string index whose values cannot be functions excludes callable
+    // method slots. Unlike structural interfaces, it cannot be a live
+    // class-method view.
+    if (receiver && (freshReceiver(receiver) || functionFreeDictionary(receiver))) continue;
     const flags = type?.flags ?? ts.TypeFlags.Unknown;
     if (flags & ts.TypeFlags.NumberLike) {
       // Numeric writes can replace numeric names but not ordinary names.

packages/compiler/src/frontend/lowering/class-prototypes.ts

@@ -174,9 +174,17 @@ export function classPrototypeData(
     const helper = lowerer.liftedFns.find((fn) => fn.name === info.def.prototypeDataHelper)!;
     const value = varRef(`%g.${info.def.prototypeDataHelper}`, DYN, loc);
     // Materialize only observed method slots. Own declarations stop lookup
-    // at the correct prototype even if an ancestor is replaced later.
+    // at the correct prototype even if an ancestor is replaced later. An
+    // abstract declaration emits no prototype member in JavaScript, so it
+    // gets no slot: lookup continues to the ancestors, and the concrete
+    // descendants' prototypes carry the implementations.
     for (const [method, access] of lowerer.prototypeMethodAccesses) {
-      if (!info.methods.has(method) || method.startsWith("get:") || method.startsWith("set:"))
+      if (
+        !info.methods.has(method) ||
+        info.methods.get(method)?.abstract ||
+        method.startsWith("get:") ||
+        method.startsWith("set:")
+      )
         continue;
       const compiled = classMethodValue(lowerer, access, info, method, locOf(access));
       if (!compiled) continue;

packages/compiler/test/ts7/baselines/order-parity.json

@@ -14867,6 +14867,12 @@
    ],
    "diags": []
   },
+  "<repo>/tests/corpus/3171-method-census-data-writes.ts": {
+   "order": [
+    "<repo>/tests/corpus/3171-method-census-data-writes.ts"
+   ],
+   "diags": []
+  },
   "<repo>/tests/corpus/4032-inline-rc-fast-paths.ts": {
    "order": [
     "<repo>/tests/corpus/4032-inline-rc-fast-paths.ts"

tests/corpus/3171-method-census-data-writes.ts

@@ -0,0 +1,90 @@
+// Writes that cannot hold a function never replace a class method, so they
+// must not turn same-named method calls elsewhere into dynamic dispatch:
+// a string-keyed dictionary of data records, and constructor writes to data
+// fields that share a name with an unrelated class's method.
+interface Entry {
+  name: string;
+  weight: number;
+  aliases: Map<string, number> | undefined;
+}
+interface Changes {
+  changed?: string[];
+  deleted?: string[];
+}
+type Json =
+  | { type: "null" }
+  | { type: "boolean"; value: boolean }
+  | { type: "object"; entries: Map<string, Json> };
+
+class Registry {
+  private readonly byName = new Map<string, Entry>();
+  constructor(entries: readonly Entry[]) {
+    for (const entry of entries) this.byName.set(entry.name.toLowerCase(), entry);
+  }
+  get(name: string): Entry | undefined {
+    return this.byName.get(name.toLowerCase());
+  }
+}
+
+class Scanner {
+  private pos = 0;
+  private readonly text: string;
+  constructor(text: string) {
+    this.text = text;
+  }
+  literal(word: string, value: Json): Json | undefined {
+    if (!this.text.startsWith(word, this.pos)) return undefined;
+    this.pos += word.length;
+    return value;
+  }
+}
+
+class Token {
+  readonly text: string;
+  constructor(text: string) {
+    this.text = text;
+  }
+}
+class LiteralNode {
+  literal: Token;
+  value: string;
+  constructor(literal: Token) {
+    this.literal = literal;
+    this.value = literal.text;
+  }
+}
+
+function collect(ids: readonly string[]): Record<string, Changes> | undefined {
+  let changes: Record<string, Changes> | undefined;
+  for (const id of ids) {
+    changes ??= {};
+    changes[id] = { changed: [id] };
+  }
+  return changes;
+}
+
+function weightOf(entry: Entry | undefined): number {
+  return entry === undefined ? -1 : entry.weight;
+}
+function show(json: Json | undefined): string {
+  if (json === undefined) return "none";
+  if (json.type === "object") return `object ${json.entries.size}`;
+  return json.type === "boolean" ? `boolean ${json.value}` : "null";
+}
+
+const registry = new Registry([
+  { name: "Alpha", weight: 3, aliases: new Map([["a", 1]]) },
+  { name: "beta", weight: 5, aliases: undefined },
+]);
+console.log(weightOf(registry.get("alpha")), weightOf(registry.get("BETA")), weightOf(registry.get("gamma")));
+const scanner = new Scanner("truenull");
+console.log(show(scanner.literal("true", { type: "boolean", value: true })));
+console.log(show(scanner.literal("true", { type: "boolean", value: true })));
+console.log(show(scanner.literal("null", { type: "null" })));
+console.log(show(new Scanner("{}").literal("{}", { type: "object", entries: new Map() })));
+console.log(registry.get("alpha")?.aliases?.get("a"), registry.get("beta")?.aliases);
+const node = new LiteralNode(new Token("42"));
+console.log(node.literal.text, node.value);
+const changes = collect(["p1", "p2"]);
+for (const [id, change] of Object.entries(changes ?? {})) console.log(id, change.changed?.join(","), change.deleted);
+console.log(collect([]));