diff --git a/packages/compiler/src/frontend/lowering/unit-journal.test.ts b/packages/compiler/src/frontend/lowering/unit-journal.test.ts index ff47e5c0a..439e18fd7 100644 --- a/packages/compiler/src/frontend/lowering/unit-journal.test.ts +++ b/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 } }); +}); diff --git a/packages/compiler/src/frontend/lowering/unit-journal.ts b/packages/compiler/src/frontend/lowering/unit-journal.ts index abb9d90b4..0992e7544 100644 --- a/packages/compiler/src/frontend/lowering/unit-journal.ts +++ b/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 | null; + /** Takes the current content of the entries among `keys`. */ + refresh(keys: ReadonlySet): 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 { + const out = new Set(); + 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(name: string, map: Map): JournalTable return { ...codecTable(name, map, encode, (text) => JSON.parse(text) as V), contents: () => { - const keys: string[] = []; - const texts: (string | null)[] = []; + const texts = new Map(); + 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(); + 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; diff --git a/packages/compiler/test/recorded-lowering.test.ts b/packages/compiler/test/recorded-lowering.test.ts index a40f33964..6c9b176f2 100644 --- a/packages/compiler/test/recorded-lowering.test.ts +++ b/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 { diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/class-computed-key-descriptors.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/class-computed-key-descriptors.ts.json new file mode 100644 index 000000000..898e483df --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/class-computed-key-descriptors.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/class-computed-key-descriptors.ts" + ], + "diags": [] +} diff --git a/tests/corpus/class-computed-key-descriptors.ts b/tests/corpus/class-computed-key-descriptors.ts new file mode 100644 index 000000000..d86c035c4 --- /dev/null +++ b/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(",")); diff --git a/tests/harness/corpus-costs/class-computed-key-descriptors.ts.json b/tests/harness/corpus-costs/class-computed-key-descriptors.ts.json new file mode 100644 index 000000000..1c61ba33a --- /dev/null +++ b/tests/harness/corpus-costs/class-computed-key-descriptors.ts.json @@ -0,0 +1 @@ +{ "plain": 5600 }