Let units without a record change the helpers they name in place

The serial mode's wave-end check threw when a unit changed a held function in place, even when the unit had no record and its change was its own lowering's: two functions that define computed-key properties on one class both rebuild the class's descriptor guard. Each unit's in-place changes are now checked against the names its IR and new entries quote: a unit with a record that changed such an entry has no record, and a unit without one keeps its changes. The wave-end check throws only for a change to an entry no unit names.

packages/compiler/src/frontend/lowering/unit-journal.test.ts

@@ -297,6 +297,48 @@ test("a recorded wave throws when a unit changes a held function in place", () =
   };
   lowerWaveRecorded(subject, ["a"], waveStats());
   expect(() => lowerWaveRecorded(subject, ["a", "b"], waveStats())).toThrow(
-    "a unit changed lifted entry %shared in place",
+    "a unit changed lifted entry %shared in place without naming it",
   );
 });
+
+/** A wave host over one function table holding `%shared` and one checked
+ * list, whose units change `%shared` in place and name it in their IR. */
+function sharedHost(grows: (name: string) => boolean): { host: WaveHost; checked: string[] } {
+  const functions = [fn("%shared")];
+  const checked: string[] = [];
+  return {
+    checked,
+    host: {
+      tables: [
+        functionTable("lifted", functions, (name) => functions.some((f) => f.name === name)),
+        checkedTable("checked", () => checked.length),
+      ],
+      scalars: [],
+      lowerRecorded: (name) => {
+        functions[0]!.body = [{ kind: "exprStmt", note: name } as never];
+        if (grows(name)) checked.push(name);
+        return {
+          fn: { ...fn(`%${name}`), body: [{ kind: "call", callee: "%shared" } as never] },
+          poisoned: false,
+        };
+      },
+      store: () => {},
+      statementCounts: () => ({ total: 0, failed: 0, other: 0 }),
+      resolves: () => true,
+    },
+  };
+}
+
+test("units without a record may change a function they name in place", () => {
+  const { host: subject } = sharedHost(() => true);
+  const stats = waveStats();
+  lowerWaveRecorded(subject, ["a", "b"], stats);
+  expect(stats).toMatchObject({ recorded: 0, unrecorded: 2, reasons: { checked: 2 } });
+});
+
+test("a unit that changes a function it names in place has no record", () => {
+  const { host: subject } = sharedHost(() => false);
+  const stats = waveStats();
+  lowerWaveRecorded(subject, ["a"], stats);
+  expect(stats).toMatchObject({ recorded: 0, unrecorded: 1, reasons: { "lifted:changed": 1 } });
+});

packages/compiler/src/frontend/lowering/unit-journal.ts

@@ -102,6 +102,31 @@ export interface HeldContents {
   extend(): void;
   /** The key of an entry this holds whose content has changed, or null. */
   changed(): string | null;
+  /** The key of an entry among `keys` whose content has changed, or null. */
+  changedAmong(keys: ReadonlySet<string>): string | null;
+  /** Takes the current content of the entries among `keys`. */
+  refresh(keys: ReadonlySet<string>): void;
+}
+
+/** The strings quoted in JSON `texts`, unescaped only as far as their
+ * delimiting quotes: the names a unit's encoded IR and entries refer to. */
+export function quotedStrings(texts: readonly string[]): Set<string> {
+  const out = new Set<string>();
+  for (const text of texts) {
+    let start = -1;
+    for (let index = 0; index < text.length; index++) {
+      const char = text.charCodeAt(index);
+      if (char === 92 /* \\ */) index++;
+      else if (char === 34 /* " */) {
+        if (start < 0) start = index + 1;
+        else {
+          out.add(text.slice(start, index));
+          start = -1;
+        }
+      }
+    }
+  }
+  return out;
 }
 
 /** A table's entries at one point (JournalTable.hold). */
@@ -237,7 +262,11 @@ export interface RecordedUnit {
  * entries it held before, every field is as it was or a flag that was set,
  * statement totals grew by the recorded statements alone, no diagnostic was
  * emitted, every event resolves and the function encodes. */
-export function recordUnit(host: WaveHost, name: string): RecordedUnit {
+export function recordUnit(
+  host: WaveHost,
+  name: string,
+  contents: readonly (HeldContents | null)[] | null = null,
+): RecordedUnit {
   const tables = host.tables;
   const before = tables.map((table) => table.size());
   const held = tables.map((table) => (table.hold === undefined ? null : table.hold()));
@@ -281,6 +310,26 @@ export function recordUnit(host: WaveHost, name: string): RecordedUnit {
       if (entries === null) reasons.push(`${table.name}:encoding`);
       else added[table.name] = entries;
     }
+  if (contents !== null) {
+    // A unit changes in place the entries it uses, which its IR and new
+    // entries name: a unit with a record must leave them as they were, and
+    // a unit without one keeps its changes.
+    const texts: string[] = [];
+    if (text !== null) texts.push(text);
+    for (const index of grown) {
+      const table = tables[index]!;
+      const entries = added[table.name] ?? table.entries!(before[index]!);
+      if (entries !== null) for (const entry of entries) texts.push(entry);
+    }
+    const names = quotedStrings(texts);
+    if (reasons.length === 0)
+      tables.forEach((table, index) => {
+        if ((contents[index] ?? null)?.changedAmong(names) != null)
+          reasons.push(`${table.name}:changed`);
+      });
+    if (reasons.length > 0) for (const held of contents) held?.refresh(names);
+    for (const held of contents) held?.extend();
+  }
   if (reasons.length > 0) return { record: null, reasons, fn, before };
   return { record: { name, fn: text, poisoned, events, tables: added }, reasons, fn, before };
 }
@@ -305,8 +354,7 @@ export function lowerWaveRecorded(
   // started: a record carries a unit's new entries, not changes to others.
   const contents = tables.map((table) => (table.contents === undefined ? null : table.contents()));
   for (const name of wave) {
-    const unit = recordUnit(host, name);
-    for (const held of contents) held?.extend();
+    const unit = recordUnit(host, name, contents);
     stats.units++;
     const record = unit.record;
     if (record === null) {
@@ -330,7 +378,7 @@ export function lowerWaveRecorded(
     const key = contents[index]?.changed() ?? null;
     if (key !== null)
       throw new InternalCompilerError(
-        `a unit changed ${table.name} entry ${key} in place, which no unit record carries`,
+        `a unit changed ${table.name} entry ${key} in place without naming it, which no unit record carries`,
       );
   });
 }
@@ -367,25 +415,30 @@ export function irDataTable<V>(name: string, map: Map<string, V>): JournalTable
   return {
     ...codecTable(name, map, encode, (text) => JSON.parse(text) as V),
     contents: () => {
-      const keys: string[] = [];
-      const texts: (string | null)[] = [];
+      const texts = new Map<string, string | null>();
+      const differs = (key: string): boolean => {
+        const value = map.get(key);
+        return value === undefined || encode(value) !== texts.get(key);
+      };
       const held: HeldContents = {
         extend: () => {
-          let index = 0;
-          for (const [key, value] of map) {
-            if (index++ < keys.length) continue;
-            keys.push(key);
-            texts.push(encode(value));
-          }
+          if (texts.size === map.size) return;
+          for (const [key, value] of map) if (!texts.has(key)) texts.set(key, encode(value));
         },
         changed: () => {
-          for (let index = 0; index < keys.length; index++) {
-            const key = keys[index]!;
-            const value = map.get(key);
-            if (value === undefined || encode(value) !== texts[index]) return key;
-          }
+          for (const key of texts.keys()) if (differs(key)) return key;
           return null;
         },
+        changedAmong: (keys) => {
+          for (const key of keys) if (texts.has(key) && differs(key)) return key;
+          return null;
+        },
+        refresh: (keys) => {
+          for (const key of keys) {
+            const value = map.get(key);
+            if (texts.has(key) && value !== undefined) texts.set(key, encode(value));
+          }
+        },
       };
       held.extend();
       return held;
@@ -515,17 +568,36 @@ export function functionTable(
     },
     contents: () => {
       const texts: (string | null)[] = [];
+      const indexes = new Map<string, number>();
+      const differs = (index: number): boolean =>
+        index < functions.length && encodeUnitFunction(functions[index]!) !== texts[index];
       const held: HeldContents = {
         extend: () => {
-          for (let index = texts.length; index < functions.length; index++)
-            texts.push(encodeUnitFunction(functions[index]!));
+          for (let index = texts.length; index < functions.length; index++) {
+            const fn = functions[index]!;
+            texts.push(encodeUnitFunction(fn));
+            indexes.set(fn.name, index);
+          }
         },
         changed: () => {
-          for (let index = 0; index < texts.length && index < functions.length; index++)
-            if (encodeUnitFunction(functions[index]!) !== texts[index])
-              return functions[index]!.name;
+          for (let index = 0; index < texts.length; index++)
+            if (differs(index)) return functions[index]!.name;
           return null;
         },
+        changedAmong: (keys) => {
+          for (const key of keys) {
+            const index = indexes.get(key);
+            if (index !== undefined && differs(index)) return key;
+          }
+          return null;
+        },
+        refresh: (keys) => {
+          for (const key of keys) {
+            const index = indexes.get(key);
+            if (index !== undefined && index < functions.length)
+              texts[index] = encodeUnitFunction(functions[index]!);
+          }
+        },
       };
       held.extend();
       return held;

packages/compiler/test/recorded-lowering.test.ts

@@ -20,6 +20,8 @@ const PROGRAMS = [
   "6496-key-order-built-and-parsed.ts",
   "6916-index-record-spread-overrides.ts",
   "6951-unknown-cast-class-methods.ts",
+  // Units without records rebuild a shared helper in place.
+  "class-computed-key-descriptors.ts",
 ];
 
 async function lower(program: string, dir: string, label: string): Promise<string> {

packages/compiler/test/ts7/baselines/order-parity/tests/corpus/class-computed-key-descriptors.ts.json

@@ -0,0 +1,6 @@
+{
+ "order": [
+  "<repo>/tests/corpus/class-computed-key-descriptors.ts"
+ ],
+ "diags": []
+}

tests/corpus/class-computed-key-descriptors.ts

@@ -0,0 +1,19 @@
+// Two functions define computed-key properties on instances of one class.
+class Point {
+  x = 1;
+  y = 2;
+}
+
+function defineValue(p: Point, key: string, value: number): void {
+  Object.defineProperty(p, key, { value, writable: true, enumerable: true, configurable: true });
+}
+
+function defineHidden(p: Point, key: string): void {
+  Object.defineProperty(p, key, { value: "hidden", writable: false, enumerable: false, configurable: true });
+}
+
+const p = new Point();
+defineValue(p, "z", 3);
+defineHidden(p, "label");
+console.log(JSON.stringify(p));
+console.log(Object.keys(p).join(","));

tests/harness/corpus-costs/class-computed-key-descriptors.ts.json

@@ -0,0 +1 @@
+{ "plain": 5600 }