From 35f2d8f791022b779972e54a55d2b369ab657fef Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Thu, 22 Oct 2026 13:07:47 -0500 Subject: [PATCH 1/4] Report dictionary copies of `{}` views, and keep method calls direct after writes into dictionaries of dictionaries Fixes #1408 Co-authored-by: Chris Ainsley --- .../record-identity-and-property-order.mdx | 1 + .../compiler/src/diagnostics/diagnostic.ts | 10 +- .../lowering/class-method-mutations.ts | 16 ++- .../src/frontend/lowering/divergences.ts | 110 +++++++++++++++++- .../test/class-method-mutations.test.ts | 42 +++++++ ...onary-of-dictionaries-keep-methods.ts.json | 6 + ...dictionary-of-dictionaries-keep-methods.ts | 38 ++++++ .../dictionary-view-copies.ts | 29 +++++ tests/coverage-fixtures/no-divergences.ts | 11 +- .../coverage-dictionary-view-copies.txt | 21 ++++ ...onary-of-dictionaries-keep-methods.ts.json | 1 + tests/harness/coverage.test.ts | 14 +++ 12 files changed, 288 insertions(+), 11 deletions(-) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7010-dictionary-of-dictionaries-keep-methods.ts.json create mode 100644 tests/corpus/7010-dictionary-of-dictionaries-keep-methods.ts create mode 100644 tests/coverage-fixtures/dictionary-view-copies.ts create mode 100644 tests/harness/__snapshots__/coverage-dictionary-view-copies.txt create mode 100644 tests/harness/corpus-costs/7010-dictionary-of-dictionaries-keep-methods.ts.json diff --git a/docs/content/limitations/record-identity-and-property-order.mdx b/docs/content/limitations/record-identity-and-property-order.mdx index 0381803f6a..224757d526 100644 --- a/docs/content/limitations/record-identity-and-property-order.mdx +++ b/docs/content/limitations/record-identity-and-property-order.mdx @@ -1,6 +1,7 @@ A record converted to a record type with a different set of properties, such as a narrower shape or one that adds optional properties, is copied. Mutations through the converted reference do not affect the original, and the copy is not `===` to it. Empty record layouts are also snapshots because they have no property storage. +A class instance or record held as `{}`, `object` or `unknown` and then converted to a dictionary type such as `Record` is copied as well; a dictionary of `unknown` values keeps the original object. `scriptc coverage` and `scriptc build` report `SC6001` where a copied value is then written or compared, whether the record was passed as an argument, stored in a typed binding, asserted with `as`, assigned to a typed binding later, or stored as an element of a typed array literal. Typed records with declared fields, indexed records, and arrays retain identity and mutations through native checked storage. diff --git a/packages/compiler/src/diagnostics/diagnostic.ts b/packages/compiler/src/diagnostics/diagnostic.ts index 17cb2efc09..47db2193b3 100644 --- a/packages/compiler/src/diagnostics/diagnostic.ts +++ b/packages/compiler/src/diagnostics/diagnostic.ts @@ -129,6 +129,7 @@ export function widthCopyDivergenceDiag( useText: string, site: "call" | "declaration" | "cast" | "element", loc: SrcLoc, + dictionary = false, ): ScrDiagnostic { const how = site === "call" @@ -142,11 +143,16 @@ export function widthCopyDivergenceDiag( use === "write" ? `the write to '${useText}' updates only the copy, while under Node it updates the original object` : `'${useText}' compares the copy, which is never the original object, while under Node it is the same object`; + const cause = dictionary + ? "converting an object held as '{}', 'object' or 'unknown' to a dictionary type" + : "converting to a record type with other properties"; return { code: "SC6001", - message: `${how} copies the object (converting to a record type with other properties compiles as a field copy): ${effect}`, + message: `${how} copies the object (${cause} compiles as a field copy): ${effect}`, loc, - hint: `declare the parameter or binding with the value's full type ('${sourceType}'), or make the shape a class — class instances keep their identity when passed as a base class`, + hint: dictionary + ? `keep the value at its own type ('${sourceType}'), or declare the dictionary's values as 'unknown', which keeps the original object` + : `declare the parameter or binding with the value's full type ('${sourceType}'), or make the shape a class — class instances keep their identity when passed as a base class`, }; } diff --git a/packages/compiler/src/frontend/lowering/class-method-mutations.ts b/packages/compiler/src/frontend/lowering/class-method-mutations.ts index abbaf3a754..7629377d47 100644 --- a/packages/compiler/src/frontend/lowering/class-method-mutations.ts +++ b/packages/compiler/src/frontend/lowering/class-method-mutations.ts @@ -125,9 +125,12 @@ export function collectClassMethodMutations( }; // 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 + // requires a property functions lack, a weak (all-optional) type that // shares no property with functions (TypeScript's weak-type rule rejects - // callable sources too). + // callable sources too), or a dictionary whose string index values are + // neither `any` nor a type parameter: a function type has no index + // signature, and only an `any` index admits a source without one (an + // instantiation can make a type parameter `any`). const functionFree = (type: ts.Type): boolean => { if (type.isUnionType()) return ts.constituentTypes(type).every(functionFree); if ( @@ -149,7 +152,14 @@ export function collectClassMethodMutations( ) return false; const properties = lowerer.checker.getPropertiesOfType(type); - if (properties.length === 0) return false; + if (properties.length === 0) + return lowerer.checker + .getIndexInfosOfType(type) + .some( + (index) => + (index.keyType.flags & ts.TypeFlags.StringLike) !== 0 && + (index.type.flags & (ts.TypeFlags.Any | ts.TypeFlags.Instantiable)) === 0, + ); const required = properties.filter( (property) => (property.flags & ts.SymbolFlags.Optional) === 0, ); diff --git a/packages/compiler/src/frontend/lowering/divergences.ts b/packages/compiler/src/frontend/lowering/divergences.ts index 1c362cb09f..f766aba14a 100644 --- a/packages/compiler/src/frontend/lowering/divergences.ts +++ b/packages/compiler/src/frontend/lowering/divergences.ts @@ -180,7 +180,15 @@ export function scanDivergences( const copy = safely(() => widthCopyBetween(lowerer, arg, param.type!)); if (copy === undefined || copy === null) return; out.push( - widthCopyDivergenceDiag(copy.source, copy.target, use.kind, use.text, "call", locOf(arg)), + widthCopyDivergenceDiag( + copy.source, + copy.target, + use.kind, + use.text, + "call", + locOf(arg), + copy.dictionary, + ), ); }); } @@ -200,6 +208,7 @@ export function scanDivergences( use.text, "declaration", locOf(decl.initializer!), + copy.dictionary, ), ); } @@ -215,7 +224,15 @@ export function scanDivergences( const copy = safely(() => widthCopyBetween(lowerer, source, type)); if (copy === undefined || copy === null) continue; out.push( - widthCopyDivergenceDiag(copy.source, copy.target, use.kind, use.text, "cast", locOf(source)), + widthCopyDivergenceDiag( + copy.source, + copy.target, + use.kind, + use.text, + "cast", + locOf(source), + copy.dictionary, + ), ); } for (const assignment of assignments) { @@ -240,6 +257,7 @@ export function scanDivergences( use.text, "declaration", locOf(assignment.right), + copy.dictionary, ), ); } @@ -254,7 +272,15 @@ export function scanDivergences( const copy = safely(() => widthCopyBetween(lowerer, item, element)); if (copy === undefined || copy === null) continue; out.push( - widthCopyDivergenceDiag(copy.source, copy.target, "write", use, "element", locOf(item)), + widthCopyDivergenceDiag( + copy.source, + copy.target, + "write", + use, + "element", + locOf(item), + copy.dictionary, + ), ); break; } @@ -653,14 +679,23 @@ function enclosingScope(node: ts.Node): ts.Node | undefined { * instance flowing into a plain record type that does not keep class * identity (ClassProtocols marks the targets a class instance visibly * flows into; a value reaching another record type is a field copy). */ +/** The types a width copy converts between, and whether the target is a + * dictionary. */ +interface WidthCopy { + source: string; + target: string; + dictionary?: boolean; +} + function widthCopyBetween( lowerer: Lowerer, source: ts.Expression, targetNode: ts.TypeNode, -): { source: string; target: string } | null { +): WidthCopy | null { const checker = lowerer.checker; const s = checker.getTypeAtLocation(source); const t = checker.getTypeFromTypeNode(targetNode); + if (isDictionaryType(checker, t)) return dictionaryViewCopy(lowerer, source, s, t); if (!isPlainRecordType(checker, t)) return null; const classSource = isProgramClassType(checker, s); if (!classSource && !isPlainRecordType(checker, s)) return null; @@ -680,6 +715,73 @@ function widthCopyBetween( return { source: checker.typeToString(s), target: checker.typeToString(t) }; } +/** A copy made by converting a checked-dynamic view (`{}`, `object`, + * `unknown`) to a dictionary type that compiles to a native record (a + * dictionary of `unknown` values stays checked-dynamic and keeps the + * object). A dictionary of that type comes back out of the view as itself, + * so a copy is reported only when the view is a constant binding + * initialized with a class instance or a record: that object is converted + * field by field into a new dictionary. */ +function dictionaryViewCopy( + lowerer: Lowerer, + source: ts.Expression, + s: ts.Type, + t: ts.Type, +): WidthCopy | null { + const checker = lowerer.checker; + if (!isDynamicViewType(checker, s)) return null; + if (lowerer.mapTypeOf(t)?.kind !== "record") return null; + while (ts.isParenthesizedExpression(source) || ts.isNonNullExpression(source)) + source = source.expression; + if (!ts.isIdentifier(source)) return null; + const decl = checker.getSymbolAtLocation(source)?.valueDeclaration; + if ( + decl === undefined || + !ts.isVariableDeclaration(decl) || + decl.initializer === undefined || + !ts.isVariableDeclarationList(decl.parent) || + !(decl.parent.flags & ts.NodeFlags.Const) + ) + return null; + let origin = decl.initializer; + while ( + ts.isParenthesizedExpression(origin) || + ts.isNonNullExpression(origin) || + ts.isAsExpression(origin) || + ts.isTypeAssertion(origin) + ) + origin = origin.expression; + const o = checker.getTypeAtLocation(origin); + if (!isProgramClassType(checker, o) && !isPlainRecordType(checker, o)) return null; + return { source: checker.typeToString(o), target: checker.typeToString(t), dictionary: true }; +} + +/** `{}`, `object` and `unknown`: types whose values lower to checked + * dynamic storage that keeps the original object. */ +function isDynamicViewType(checker: ts.TypeChecker, type: ts.Type): boolean { + if (type.flags & (ts.TypeFlags.Unknown | ts.TypeFlags.NonPrimitive)) return true; + return ( + (type.flags & ts.TypeFlags.Object) !== 0 && + checker.getPropertiesOfType(type).length === 0 && + checker.getIndexInfosOfType(type).length === 0 && + checker.getCallSignatures(type).length === 0 && + checker.getConstructSignatures(type).length === 0 + ); +} + +/** A string-keyed dictionary with no declared properties + * (`Record`, `{ [key: string]: number }`). */ +function isDictionaryType(checker: ts.TypeChecker, type: ts.Type): boolean { + if ((type.flags & ts.TypeFlags.Object) === 0) return false; + if (checker.isArrayType(type) || checker.isTupleType(type)) return false; + if (checker.getCallSignatures(type).length > 0) return false; + if (checker.getConstructSignatures(type).length > 0) return false; + if (checker.getPropertiesOfType(type).length > 0) return false; + return checker + .getIndexInfosOfType(type) + .some((index) => (index.keyType.flags & ts.TypeFlags.String) !== 0); +} + function isProgramClassType(checker: ts.TypeChecker, type: ts.Type): boolean { if ((type.flags & ts.TypeFlags.Object) === 0) return false; const symbol = type.getSymbol(); diff --git a/packages/compiler/test/class-method-mutations.test.ts b/packages/compiler/test/class-method-mutations.test.ts index b508a39495..3221e44ee6 100644 --- a/packages/compiler/test/class-method-mutations.test.ts +++ b/packages/compiler/test/class-method-mutations.test.ts @@ -135,3 +135,45 @@ test("a reflection call with a well-known symbol key keeps checked dispatch", as const callers = await callersOf(`${range}${write}`, "%Range.sym:iterator"); expect(callers.filter((name) => !name.startsWith("%method.value:"))).toEqual([]); }); + +const groups = ` +class Tally { + n = 0; + add(): void { this.n++; } +} +interface Settings { groups: VALUE } +function addGroup(settings: Settings, name: string): void { + settings.groups[name] = {}; +} +const settings: Settings = { groups: {} }; +const tally = new Tally(); +addGroup(settings, String(tally.n)); +tally.add(); +console.log(tally.n, Object.keys(settings.groups)); +`; + +test("a computed write into a dictionary of dictionaries leaves method calls direct", async () => { + const source = groups.replace("VALUE", "Record>"); + expect(await callersOf(source, "%dyn.class.properties")).toEqual([]); +}); + +test("a computed write into a dictionary that can hold functions keeps checked dispatch", async () => { + for (const value of [ + "Record | (() => void)>", + "Record>", + ]) { + const source = groups.replace("VALUE", value); + expect(await callersOf(source, "%dyn.class.properties")).not.toEqual([]); + } +}); + +test("a computed write into a generic dictionary of dictionaries keeps checked dispatch", async () => { + const source = groups + .replace( + "interface Settings { groups: VALUE }", + "interface Settings { groups: Record> }", + ) + .replace("function addGroup(settings: Settings,", "function addGroup(settings: Settings,") + .replace("const settings: Settings =", "const settings: Settings ="); + expect(await callersOf(source, "%dyn.class.properties")).not.toEqual([]); +}); diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7010-dictionary-of-dictionaries-keep-methods.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7010-dictionary-of-dictionaries-keep-methods.ts.json new file mode 100644 index 0000000000..cc3c9dabe6 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7010-dictionary-of-dictionaries-keep-methods.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/7010-dictionary-of-dictionaries-keep-methods.ts" + ], + "diags": [] +} diff --git a/tests/corpus/7010-dictionary-of-dictionaries-keep-methods.ts b/tests/corpus/7010-dictionary-of-dictionaries-keep-methods.ts new file mode 100644 index 0000000000..a53e82f839 --- /dev/null +++ b/tests/corpus/7010-dictionary-of-dictionaries-keep-methods.ts @@ -0,0 +1,38 @@ +// A string-keyed dictionary whose values are dictionaries cannot hold a +// function, so storing into it under a key known only at runtime leaves +// class methods in place: their calls stay direct, and a record a method +// returns is the record the instance holds, so writes through it are kept. +interface Data { + items: string[]; +} +class Store { + data: Data = { items: [] }; + get(): Data { + return this.data; + } +} +class Counter { + count = 0; + bump(): number { + return ++this.count; + } +} +interface Settings { + groups: Record>; +} +function addGroup(settings: Settings, name: string): void { + settings.groups[name] = {}; +} + +const settings: Settings = { groups: {} }; +const name = process.argv.length > 99 ? "never" : "colours"; +addGroup(settings, name); +const group = settings.groups[name]; +if (group !== undefined) group["sky"] = "blue"; +const store = new Store(); +store.get().items.push("a"); +store.get().items.push("b"); +const counter = new Counter(); +let total = 0; +for (let i = 0; i < 1000; i++) total += counter.bump(); +console.log(JSON.stringify(settings.groups), store.data.items.join(","), total); diff --git a/tests/coverage-fixtures/dictionary-view-copies.ts b/tests/coverage-fixtures/dictionary-view-copies.ts new file mode 100644 index 0000000000..bfcfa61f44 --- /dev/null +++ b/tests/coverage-fixtures/dictionary-view-copies.ts @@ -0,0 +1,29 @@ +// A class instance or record held as `{}`, `object` or `unknown` keeps the +// original object, and converting it to a dictionary type copies it field +// by field. Each site below then writes through or compares the copy. +class Counter { + label = "c"; + add(): number { + return this.label.length; + } +} +const key = process.argv.length > 5 ? "z" : "b"; + +const counter = new Counter(); +const view: {} = counter; +const table: Record = view; +table[key] = "y"; + +const point = { a: "x" }; +const loose: object = point; +const dict = loose as Record; +dict[key] = "y"; + +const held: unknown = { a: "x" }; +const fromUnknown = held as Record; +console.log(fromUnknown === held); + +const empty: {} = {}; +const filled: Record = empty; +filled[key] = "y"; +console.log(JSON.stringify(counter), JSON.stringify(point), JSON.stringify(empty)); diff --git a/tests/coverage-fixtures/no-divergences.ts b/tests/coverage-fixtures/no-divergences.ts index 3739336e3c..ca76bcc324 100644 --- a/tests/coverage-fixtures/no-divergences.ts +++ b/tests/coverage-fixtures/no-divergences.ts @@ -1,6 +1,6 @@ // Ordinary code: identity-preserving calls, declaration-ordered literals, -// Latin-script comparisons, related-class casts, ISO dates and views of the -// global object report no divergence. +// Latin-script comparisons, related-class casts, ISO dates, views of the +// global object and a dictionary held as `{}` report no divergence. interface Item { name: string; qty: number; @@ -38,3 +38,10 @@ const COUNT = Symbol.for("no-divergences.count"); const globalState = globalThis as { [COUNT]?: { n: number } }; globalState[COUNT] = { n: 1 }; console.log(globalState[COUNT]?.n); + +// A dictionary held as `{}` comes back out as itself. +const counts: Record = { a: "x" }; +const opaque: {} = counts; +const again: Record = opaque; +again[counts["a"] ?? "a"] = "y"; +console.log(JSON.stringify(counts)); diff --git a/tests/harness/__snapshots__/coverage-dictionary-view-copies.txt b/tests/harness/__snapshots__/coverage-dictionary-view-copies.txt new file mode 100644 index 0000000000..2359a4ffa8 --- /dev/null +++ b/tests/harness/__snapshots__/coverage-dictionary-view-copies.txt @@ -0,0 +1,21 @@ +scriptc coverage tests/coverage-fixtures/dictionary-view-copies.ts + + statements analyzed 18 + compile statically 18 (100%) + in dynamic tier 3 (operate on values typed only at runtime; --explain-dynamic lists them) + + fully static — all analyzed statements compile to native code. + + differs from Node 4 sites (compiles; the native program can behave differently) + ×1 SC6001 storing a 'Counter' in a 'Record<…>' binding copies the object (converting an object held as '{}', 'object' or 'unknown' to a dictionary type compiles as a field copy): the write to 'table[key]' upda… + at tests/coverage-fixtures/dictionary-view-copies.ts:14:39 + hint: keep the value at its own type ('Counter'), or declare the dictionary's values as 'unknown', which keeps the original object + ×1 SC6001 storing a '{}' in a 'Record<…>' binding copies the object (converting an object held as '{}', 'object' or 'unknown' to a dictionary type compiles as a field copy): the write to 'filled[key]' updates … + at tests/coverage-fixtures/dictionary-view-copies.ts:27:40 + hint: keep the value at its own type ('{}'), or declare the dictionary's values as 'unknown', which keeps the original object + ×1 SC6001 asserting a '{ a: string; }' as 'Record<…>' copies the object (converting an object held as '{}', 'object' or 'unknown' to a dictionary type compiles as a field copy): the write to 'dict[key]' update… + at tests/coverage-fixtures/dictionary-view-copies.ts:19:14 + hint: keep the value at its own type ('{ a: string; }'), or declare the dictionary's values as 'unknown', which keeps the original object + ×1 SC6001 asserting a '{ a: string; }' as 'Record<…>' copies the object (converting an object held as '{}', 'object' or 'unknown' to a dictionary type compiles as a field copy): 'fromUnknown === held' compares… + at tests/coverage-fixtures/dictionary-view-copies.ts:23:21 + hint: keep the value at its own type ('{ a: string; }'), or declare the dictionary's values as 'unknown', which keeps the original object \ No newline at end of file diff --git a/tests/harness/corpus-costs/7010-dictionary-of-dictionaries-keep-methods.ts.json b/tests/harness/corpus-costs/7010-dictionary-of-dictionaries-keep-methods.ts.json new file mode 100644 index 0000000000..cc07613e75 --- /dev/null +++ b/tests/harness/corpus-costs/7010-dictionary-of-dictionaries-keep-methods.ts.json @@ -0,0 +1 @@ +{ "plain": 2184, "san": 1198 } diff --git a/tests/harness/coverage.test.ts b/tests/harness/coverage.test.ts index 86d202b461..f3958c5fe7 100644 --- a/tests/harness/coverage.test.ts +++ b/tests/harness/coverage.test.ts @@ -107,6 +107,20 @@ test("record width copies report at every storing site", async () => { ); }); +test("dictionary conversions of checked-dynamic views report copies", async () => { + const { coverage } = analyze(fixture("dictionary-view-copies.ts")); + expect(coverage.diagnostics).toEqual([]); + expect((coverage.divergences ?? []).map((d) => d.code)).toEqual([ + "SC6001", + "SC6001", + "SC6001", + "SC6001", + ]); + await expect(report(fixture("dictionary-view-copies.ts"))).toMatchFileSnapshot( + "__snapshots__/coverage-dictionary-view-copies.txt", + ); +}); + test("ordinary code reports no divergence", () => { const { coverage } = analyze(fixture("no-divergences.ts")); expect(coverage.diagnostics).toEqual([]); From 961aad6ac99b9b3cf8d101c24415cb5b60d9a616 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Thu, 22 Oct 2026 20:37:10 -0500 Subject: [PATCH 2/4] Keep the identity of records and class instances converted to dictionaries A record or class instance converted to a string-keyed dictionary type (`Record`, `{ [key: string]: V }`) was copied field by field into a native dictionary record, directly, through a dictionary parameter, or through a `{}`, `object` or `unknown` binding or parameter. Writes through the dictionary missed the original object, and the dictionary was never `===` to it, where under Node it is the same object. ClassProtocols now marks a dictionary value type when a dictionary of that value type receives an existing object (a binding, member, element or `this` whose type is a program class instance, a record with declared properties, or a `{}`, `object` or `unknown` binding or parameter that holds one), and the program writes through, compares or passes to a whole-object observer a dictionary of that value type. usesCheckedIdentity answers true for those dictionaries, so they lower to checked-dynamic storage, which holds the original object. Dictionaries of `unknown` or `any` values are checked-dynamic already and are not marked. The dictionary case of SC6001 is removed, since these conversions now keep the object. --- .../record-identity-and-property-order.mdx | 3 +- .../compiler/src/diagnostics/diagnostic.ts | 10 +- .../src/frontend/lowering/class-protocols.ts | 205 +++++++++++++++++- .../src/frontend/lowering/divergences.ts | 110 +--------- .../compiler/test/dictionary-views.test.ts | 77 +++++++ ...7012-dictionary-views-keep-objects.ts.json | 6 + .../7013-dictionary-view-parameter.ts.json | 6 + .../7012-dictionary-views-keep-objects.ts | 61 ++++++ .../corpus/7013-dictionary-view-parameter.ts | 11 + .../dictionary-view-copies.ts | 29 --- tests/coverage-fixtures/no-divergences.ts | 7 + .../coverage-dictionary-view-copies.txt | 21 -- ...7012-dictionary-views-keep-objects.ts.json | 1 + .../7013-dictionary-view-parameter.ts.json | 1 + tests/harness/coverage.test.ts | 14 -- 15 files changed, 382 insertions(+), 180 deletions(-) create mode 100644 packages/compiler/test/dictionary-views.test.ts create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7012-dictionary-views-keep-objects.ts.json create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7013-dictionary-view-parameter.ts.json create mode 100644 tests/corpus/7012-dictionary-views-keep-objects.ts create mode 100644 tests/corpus/7013-dictionary-view-parameter.ts delete mode 100644 tests/coverage-fixtures/dictionary-view-copies.ts delete mode 100644 tests/harness/__snapshots__/coverage-dictionary-view-copies.txt create mode 100644 tests/harness/corpus-costs/7012-dictionary-views-keep-objects.ts.json create mode 100644 tests/harness/corpus-costs/7013-dictionary-view-parameter.ts.json diff --git a/docs/content/limitations/record-identity-and-property-order.mdx b/docs/content/limitations/record-identity-and-property-order.mdx index 224757d526..2299122a4c 100644 --- a/docs/content/limitations/record-identity-and-property-order.mdx +++ b/docs/content/limitations/record-identity-and-property-order.mdx @@ -1,10 +1,11 @@ A record converted to a record type with a different set of properties, such as a narrower shape or one that adds optional properties, is copied. Mutations through the converted reference do not affect the original, and the copy is not `===` to it. Empty record layouts are also snapshots because they have no property storage. -A class instance or record held as `{}`, `object` or `unknown` and then converted to a dictionary type such as `Record` is copied as well; a dictionary of `unknown` values keeps the original object. `scriptc coverage` and `scriptc build` report `SC6001` where a copied value is then written or compared, whether the record was passed as an argument, stored in a typed binding, asserted with `as`, assigned to a typed binding later, or stored as an element of a typed array literal. Typed records with declared fields, indexed records, and arrays retain identity and mutations through native checked storage. +A record or class instance converted to a string-keyed dictionary type such as `Record`, directly or through a `{}`, `object` or `unknown` binding or parameter, keeps its identity when the program writes through, compares or inspects values of that dictionary type. +Every dictionary type with the same value type then uses checked dynamic storage, which is slower than a native dictionary. Class instances retain identity at their exact type or a base class, and at any interface or object type that a class instance is passed, stored, returned, or asserted as when the program could tell a copy from the instance: values of such a type are checked views of the original object, so writes, identity comparisons, Set and Map keys, `util.inspect`, and `JSON.stringify` through them reach the instance, as in Node.js. These types use checked dynamic storage, which is slower than native record fields, for all of their values, including plain objects. A type whose values are only read field by field, from fields the class does not write after construction and does not implement as accessors, stays a native record and holds a copy. diff --git a/packages/compiler/src/diagnostics/diagnostic.ts b/packages/compiler/src/diagnostics/diagnostic.ts index 47db2193b3..17cb2efc09 100644 --- a/packages/compiler/src/diagnostics/diagnostic.ts +++ b/packages/compiler/src/diagnostics/diagnostic.ts @@ -129,7 +129,6 @@ export function widthCopyDivergenceDiag( useText: string, site: "call" | "declaration" | "cast" | "element", loc: SrcLoc, - dictionary = false, ): ScrDiagnostic { const how = site === "call" @@ -143,16 +142,11 @@ export function widthCopyDivergenceDiag( use === "write" ? `the write to '${useText}' updates only the copy, while under Node it updates the original object` : `'${useText}' compares the copy, which is never the original object, while under Node it is the same object`; - const cause = dictionary - ? "converting an object held as '{}', 'object' or 'unknown' to a dictionary type" - : "converting to a record type with other properties"; return { code: "SC6001", - message: `${how} copies the object (${cause} compiles as a field copy): ${effect}`, + message: `${how} copies the object (converting to a record type with other properties compiles as a field copy): ${effect}`, loc, - hint: dictionary - ? `keep the value at its own type ('${sourceType}'), or declare the dictionary's values as 'unknown', which keeps the original object` - : `declare the parameter or binding with the value's full type ('${sourceType}'), or make the shape a class — class instances keep their identity when passed as a base class`, + hint: `declare the parameter or binding with the value's full type ('${sourceType}'), or make the shape a class — class instances keep their identity when passed as a base class`, }; } diff --git a/packages/compiler/src/frontend/lowering/class-protocols.ts b/packages/compiler/src/frontend/lowering/class-protocols.ts index 76091b13e5..4edf1ab7e8 100644 --- a/packages/compiler/src/frontend/lowering/class-protocols.ts +++ b/packages/compiler/src/frontend/lowering/class-protocols.ts @@ -10,6 +10,9 @@ export class ClassProtocols { private readonly accepted = new Map(); /** What markIdentityFlows learned about the program, while it runs. */ private facts: FlowFacts | null = null; + /** The value types of the string-keyed dictionaries that keep checked + * identity (see markDictionaryViews). */ + private readonly dictionaryViews = new Set(); constructor( private readonly checker: ts.TypeChecker, @@ -323,6 +326,7 @@ export class ClassProtocols { else if (names !== "all") names.add(name); } } + this.markDictionaryViews(sites, sourceNodes, observedNodes, writeNodes); for (let i = 0; i < looseArguments.length; i++) { const type = typeOf(looseNodes[i]!); if (type === undefined) continue; @@ -697,6 +701,163 @@ export class ClassProtocols { return source !== undefined && this.isProgramClassInstance(source); } + /** String-keyed dictionaries (`Record`, `{ [key: string]: V }`) + * that receive an existing record or class instance. Natively a + * dictionary is a key/value record, and converting another object into + * one copies its fields, so writes through the dictionary and identity + * checks would miss the original. When the program writes through, or + * observes, a value of such a dictionary type, every dictionary with that + * value type keeps checked identity instead (records with the same value + * type share one native layout, so values pass between them). A source + * counts when it is an existing object (a binding, member, element or + * `this`, not a fresh literal or call result) whose type is a program + * class instance or a record with declared properties, or a constant + * `{}`, `object` or `unknown` binding initialized with one. */ + private markDictionaryViews( + sites: readonly { + source: ts.Expression; + target: () => ts.Type | undefined; + contextual?: ts.Expression; + }[], + sourceNodes: readonly ts.Expression[], + observedNodes: readonly ts.Expression[], + writeNodes: readonly ts.Expression[], + ): void { + // The calls the sites' arguments belong to, by callee name, built on + // first use. + let calls: Map | null = null; + const callsNamed = (name: string): readonly (ts.CallExpression | ts.NewExpression)[] => { + if (calls === null) { + calls = new Map(); + for (const site of sites) { + const call = site.contextual?.parent; + if (call === undefined || (!ts.isCallExpression(call) && !ts.isNewExpression(call))) + continue; + const callee = call.expression; + const named = ts.isIdentifier(callee) + ? callee.text + : ts.isPropertyAccessExpression(callee) + ? callee.name.text + : undefined; + if (named === undefined) continue; + const list = calls.get(named) ?? []; + if (!list.includes(call)) list.push(call); + calls.set(named, list); + } + } + return calls.get(name) ?? []; + }; + const candidates: { index: number; sourceType: ts.Type }[] = []; + for (let i = 0; i < sites.length; i++) { + const source = sourceNodes[i]!; + if (!isExistingObject(source)) continue; + const sourceType = attempt(() => this.checker.getTypeAtLocation(source)); + if (sourceType !== undefined && this.dictionarySource(source, sourceType, callsNamed)) + candidates.push({ index: i, sourceType }); + } + if (candidates.length === 0) return; + // One batch for the contextual targets of the candidate sites. + attempt(() => + this.checker.prefetchContextualTypes( + candidates.flatMap(({ index }) => { + const contextual = sites[index]!.contextual; + return contextual === undefined ? [] : [contextual]; + }), + ), + ); + const received = new Set(); + for (const { index, sourceType } of candidates) { + const target = attempt(sites[index]!.target); + const parts = + target === undefined ? [] : target.isUnionType() ? ts.constituentTypes(target) : [target]; + for (const part of parts) { + const value = this.dictionaryValueType(part); + // A dictionary of `unknown` or `any` values is checked-dynamic already. + if ( + value !== undefined && + (value.flags & (ts.TypeFlags.Unknown | ts.TypeFlags.Any)) === 0 && + this.dictionaryValueType(sourceType) !== value + ) + received.add(value); + } + } + if (received.size === 0) return; + // The dictionary values the program writes through or observes. + const seen = new Set(); + for (const node of [...writeNodes, ...observedNodes]) { + const type = attempt(() => this.checker.getTypeAtLocation(node)); + const parts = + type === undefined ? [] : type.isUnionType() ? ts.constituentTypes(type) : [type]; + for (const part of parts) { + const value = this.dictionaryValueType(part); + if (value !== undefined) seen.add(value); + } + } + for (const value of received) if (seen.has(value)) this.dictionaryViews.add(value); + } + + /** Whether `source` holds an existing object that a dictionary + * conversion would copy. */ + private dictionarySource( + source: ts.Expression, + type: ts.Type, + callsNamed: (name: string) => readonly (ts.CallExpression | ts.NewExpression)[], + ): boolean { + if (this.isProgramClassInstance(type) || this.isRecordWithFields(type)) return true; + if (!isDynamicView(this.checker, type) || !ts.isIdentifier(source)) return false; + const decl = attempt(() => this.checker.getSymbolAtLocation(source))?.valueDeclaration; + if (decl === undefined) return false; + const holdsObject = (expr: ts.Expression): boolean => { + const held = attempt(() => this.checker.getTypeAtLocation(this.flowSourceNode(expr))); + return ( + held !== undefined && (this.isProgramClassInstance(held) || this.isRecordWithFields(held)) + ); + }; + if (ts.isParameter(decl)) { + // A parameter holds what its callers pass: the calls of the function + // by name whose signature resolves to it. + const fn = decl.parent; + if (fn === undefined || !ts.isFunctionLike(fn)) return false; + const index = fn.parameters.nodes.indexOf(decl); + const name = functionName(fn); + if (name === undefined || index < 0) return false; + return callsNamed(name).some((call) => { + const argument = call.arguments?.nodes[index]; + return ( + argument !== undefined && + !ts.isSpreadElement(argument) && + attempt(() => this.checker.getResolvedSignature(call)?.declaration) === fn && + holdsObject(argument) + ); + }); + } + return ( + ts.isVariableDeclaration(decl) && + decl.initializer !== undefined && + ts.isVariableDeclarationList(decl.parent) && + (decl.parent.flags & ts.NodeFlags.Const) !== 0 && + holdsObject(decl.initializer) + ); + } + + /** A program object type with declared properties that is not a class + * instance, array, tuple, function or library type. */ + private isRecordWithFields(type: ts.Type): boolean { + return this.isStructuralTarget(type) && this.checker.getPropertiesOfType(type).length > 0; + } + + /** The value type of a string-keyed dictionary with no declared + * properties, or undefined for any other type. */ + private dictionaryValueType(type: ts.Type): ts.Type | undefined { + if ((type.flags & ts.TypeFlags.Object) === 0) return undefined; + if (this.checker.isArrayType(type) || this.checker.isTupleType(type)) return undefined; + if (this.checker.getCallSignatures(type).length > 0) return undefined; + if (this.checker.getPropertiesOfType(type).length > 0) return undefined; + return this.checker + .getIndexInfosOfType(type) + .find((index) => (index.keyType.flags & ts.TypeFlags.String) !== 0)?.type; + } + private isProgramClassInstance(type: ts.Type): boolean { const symbol = type.getSymbol(); return ( @@ -822,7 +983,11 @@ export class ClassProtocols { if ((type.flags & ts.TypeFlags.Object) === 0) return false; const previous = this.answers.get(type); if (previous !== undefined) return previous; - const answer = this.protocol(type); + const dictionaryValue = this.dictionaryValueType(type); + const answer = + dictionaryValue !== undefined + ? (this.index(), this.dictionaryViews.has(dictionaryValue)) + : this.protocol(type); this.answers.set(type, answer); return answer; } @@ -922,6 +1087,44 @@ const IDENTITY_METHODS = new Set([ "includes", ]); +/** An expression that denotes an object that already exists, not a fresh + * literal or call result. */ +function isExistingObject(node: ts.Expression): boolean { + return ( + ts.isIdentifier(node) || + ts.isPropertyAccessExpression(node) || + ts.isElementAccessExpression(node) || + node.kind === ts.SyntaxKind.ThisKeyword + ); +} + +/** `{}`, `object` and `unknown`: types whose values are checked-dynamic + * and keep the original object. */ +function isDynamicView(checker: ts.TypeChecker, type: ts.Type): boolean { + if (type.flags & (ts.TypeFlags.Unknown | ts.TypeFlags.NonPrimitive)) return true; + return ( + (type.flags & ts.TypeFlags.Object) !== 0 && + checker.getPropertiesOfType(type).length === 0 && + checker.getIndexInfosOfType(type).length === 0 && + checker.getCallSignatures(type).length === 0 && + checker.getConstructSignatures(type).length === 0 + ); +} + +/** The name calls use for a function, method or function-valued binding. */ +function functionName(fn: ts.Node): string | undefined { + if ((ts.isFunctionDeclaration(fn) || ts.isMethodDeclaration(fn)) && fn.name !== undefined) + return ts.isIdentifier(fn.name) ? fn.name.text : undefined; + if ( + (ts.isArrowFunction(fn) || ts.isFunctionExpression(fn)) && + fn.parent !== undefined && + ts.isVariableDeclaration(fn.parent) && + ts.isIdentifier(fn.parent.name) + ) + return fn.parent.name.text; + return undefined; +} + function isNullishLiteral(node: ts.Expression): boolean { while (ts.isParenthesizedExpression(node)) node = node.expression; return ( diff --git a/packages/compiler/src/frontend/lowering/divergences.ts b/packages/compiler/src/frontend/lowering/divergences.ts index f766aba14a..1c362cb09f 100644 --- a/packages/compiler/src/frontend/lowering/divergences.ts +++ b/packages/compiler/src/frontend/lowering/divergences.ts @@ -180,15 +180,7 @@ export function scanDivergences( const copy = safely(() => widthCopyBetween(lowerer, arg, param.type!)); if (copy === undefined || copy === null) return; out.push( - widthCopyDivergenceDiag( - copy.source, - copy.target, - use.kind, - use.text, - "call", - locOf(arg), - copy.dictionary, - ), + widthCopyDivergenceDiag(copy.source, copy.target, use.kind, use.text, "call", locOf(arg)), ); }); } @@ -208,7 +200,6 @@ export function scanDivergences( use.text, "declaration", locOf(decl.initializer!), - copy.dictionary, ), ); } @@ -224,15 +215,7 @@ export function scanDivergences( const copy = safely(() => widthCopyBetween(lowerer, source, type)); if (copy === undefined || copy === null) continue; out.push( - widthCopyDivergenceDiag( - copy.source, - copy.target, - use.kind, - use.text, - "cast", - locOf(source), - copy.dictionary, - ), + widthCopyDivergenceDiag(copy.source, copy.target, use.kind, use.text, "cast", locOf(source)), ); } for (const assignment of assignments) { @@ -257,7 +240,6 @@ export function scanDivergences( use.text, "declaration", locOf(assignment.right), - copy.dictionary, ), ); } @@ -272,15 +254,7 @@ export function scanDivergences( const copy = safely(() => widthCopyBetween(lowerer, item, element)); if (copy === undefined || copy === null) continue; out.push( - widthCopyDivergenceDiag( - copy.source, - copy.target, - "write", - use, - "element", - locOf(item), - copy.dictionary, - ), + widthCopyDivergenceDiag(copy.source, copy.target, "write", use, "element", locOf(item)), ); break; } @@ -679,23 +653,14 @@ function enclosingScope(node: ts.Node): ts.Node | undefined { * instance flowing into a plain record type that does not keep class * identity (ClassProtocols marks the targets a class instance visibly * flows into; a value reaching another record type is a field copy). */ -/** The types a width copy converts between, and whether the target is a - * dictionary. */ -interface WidthCopy { - source: string; - target: string; - dictionary?: boolean; -} - function widthCopyBetween( lowerer: Lowerer, source: ts.Expression, targetNode: ts.TypeNode, -): WidthCopy | null { +): { source: string; target: string } | null { const checker = lowerer.checker; const s = checker.getTypeAtLocation(source); const t = checker.getTypeFromTypeNode(targetNode); - if (isDictionaryType(checker, t)) return dictionaryViewCopy(lowerer, source, s, t); if (!isPlainRecordType(checker, t)) return null; const classSource = isProgramClassType(checker, s); if (!classSource && !isPlainRecordType(checker, s)) return null; @@ -715,73 +680,6 @@ function widthCopyBetween( return { source: checker.typeToString(s), target: checker.typeToString(t) }; } -/** A copy made by converting a checked-dynamic view (`{}`, `object`, - * `unknown`) to a dictionary type that compiles to a native record (a - * dictionary of `unknown` values stays checked-dynamic and keeps the - * object). A dictionary of that type comes back out of the view as itself, - * so a copy is reported only when the view is a constant binding - * initialized with a class instance or a record: that object is converted - * field by field into a new dictionary. */ -function dictionaryViewCopy( - lowerer: Lowerer, - source: ts.Expression, - s: ts.Type, - t: ts.Type, -): WidthCopy | null { - const checker = lowerer.checker; - if (!isDynamicViewType(checker, s)) return null; - if (lowerer.mapTypeOf(t)?.kind !== "record") return null; - while (ts.isParenthesizedExpression(source) || ts.isNonNullExpression(source)) - source = source.expression; - if (!ts.isIdentifier(source)) return null; - const decl = checker.getSymbolAtLocation(source)?.valueDeclaration; - if ( - decl === undefined || - !ts.isVariableDeclaration(decl) || - decl.initializer === undefined || - !ts.isVariableDeclarationList(decl.parent) || - !(decl.parent.flags & ts.NodeFlags.Const) - ) - return null; - let origin = decl.initializer; - while ( - ts.isParenthesizedExpression(origin) || - ts.isNonNullExpression(origin) || - ts.isAsExpression(origin) || - ts.isTypeAssertion(origin) - ) - origin = origin.expression; - const o = checker.getTypeAtLocation(origin); - if (!isProgramClassType(checker, o) && !isPlainRecordType(checker, o)) return null; - return { source: checker.typeToString(o), target: checker.typeToString(t), dictionary: true }; -} - -/** `{}`, `object` and `unknown`: types whose values lower to checked - * dynamic storage that keeps the original object. */ -function isDynamicViewType(checker: ts.TypeChecker, type: ts.Type): boolean { - if (type.flags & (ts.TypeFlags.Unknown | ts.TypeFlags.NonPrimitive)) return true; - return ( - (type.flags & ts.TypeFlags.Object) !== 0 && - checker.getPropertiesOfType(type).length === 0 && - checker.getIndexInfosOfType(type).length === 0 && - checker.getCallSignatures(type).length === 0 && - checker.getConstructSignatures(type).length === 0 - ); -} - -/** A string-keyed dictionary with no declared properties - * (`Record`, `{ [key: string]: number }`). */ -function isDictionaryType(checker: ts.TypeChecker, type: ts.Type): boolean { - if ((type.flags & ts.TypeFlags.Object) === 0) return false; - if (checker.isArrayType(type) || checker.isTupleType(type)) return false; - if (checker.getCallSignatures(type).length > 0) return false; - if (checker.getConstructSignatures(type).length > 0) return false; - if (checker.getPropertiesOfType(type).length > 0) return false; - return checker - .getIndexInfosOfType(type) - .some((index) => (index.keyType.flags & ts.TypeFlags.String) !== 0); -} - function isProgramClassType(checker: ts.TypeChecker, type: ts.Type): boolean { if ((type.flags & ts.TypeFlags.Object) === 0) return false; const symbol = type.getSymbol(); diff --git a/packages/compiler/test/dictionary-views.test.ts b/packages/compiler/test/dictionary-views.test.ts new file mode 100644 index 0000000000..b841447cd4 --- /dev/null +++ b/packages/compiler/test/dictionary-views.test.ts @@ -0,0 +1,77 @@ +import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { expect, test } from "vitest"; +import { compile, deserializeModule } from "../src/index.js"; + +/** The IR type kind of each named module-level binding. */ +async function globalKinds(source: string): Promise> { + const dir = await mkdtemp(join(tmpdir(), "scriptc-dictionary-views-")); + try { + const entry = join(dir, "main.ts"); + const outPath = join(dir, "main.ir.json"); + await writeFile(entry, source); + const result = await compile(entry, { outDir: dir, outPath, outputKind: "ir" }); + if (!result.ok) + throw new Error(result.diagnostics.map((d) => `${d.code}: ${d.message}`).join("\n")); + const mod = deserializeModule(await readFile(outPath, "utf8")); + return Object.fromEntries((mod.globals ?? []).map((g) => [g.name, g.type.kind])); + } finally { + await rm(dir, { recursive: true, force: true }); + } +} + +const program = (body: string): string => ` +const key = String(process.argv.length); +const counts: Record = { a: 1 }; +counts[key] = 2; +${body} +console.log(JSON.stringify(counts)); +`; + +test("a record written through a dictionary keeps that dictionary type checked", async () => { + const kinds = await globalKinds( + program(`const point: { a: string } = { a: "x" }; +const table: Record = point; +table[key] = "y"; +console.log(JSON.stringify(point));`), + ); + expect(kinds["table"]).toBe("dyn"); + // Dictionaries of other value types keep their native layout. + expect(kinds["counts"]).toBe("record"); +}); + +test("a record reaching a dictionary through a {} parameter keeps it checked", async () => { + const kinds = await globalKinds( + program(`function stamp(value: {}): void { + const inner: Record = value; + inner[key] = "y"; +} +const point: { a: string } = { a: "x" }; +stamp(point); +const other: Record = { b: "z" }; +console.log(JSON.stringify(point), other["b"]);`), + ); + expect(kinds["other"]).toBe("dyn"); +}); + +test("dictionaries keep their native layout without a record flowing in", async () => { + const kinds = await globalKinds( + program(`const table: Record = { a: "x" }; +table[key] = "y"; +const copy: Record = table; +copy[key] = "z"; +console.log(JSON.stringify(table));`), + ); + expect(kinds["table"]).toBe("record"); + expect(kinds["copy"]).toBe("record"); +}); + +test("a record converted to a dictionary that is only read keeps the native layout", async () => { + const kinds = await globalKinds( + program(`const point: { a: string } = { a: "x" }; +const table: Record = point; +console.log(table["a"]);`), + ); + expect(kinds["table"]).toBe("record"); +}); diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7012-dictionary-views-keep-objects.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7012-dictionary-views-keep-objects.ts.json new file mode 100644 index 0000000000..6d7944cf7e --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7012-dictionary-views-keep-objects.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/7012-dictionary-views-keep-objects.ts" + ], + "diags": [] +} diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7013-dictionary-view-parameter.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7013-dictionary-view-parameter.ts.json new file mode 100644 index 0000000000..41f80c41e0 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7013-dictionary-view-parameter.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/7013-dictionary-view-parameter.ts" + ], + "diags": [] +} diff --git a/tests/corpus/7012-dictionary-views-keep-objects.ts b/tests/corpus/7012-dictionary-views-keep-objects.ts new file mode 100644 index 0000000000..8ede809267 --- /dev/null +++ b/tests/corpus/7012-dictionary-views-keep-objects.ts @@ -0,0 +1,61 @@ +// A record or class instance converted to a string-keyed dictionary is +// the same object: writes through the dictionary reach it, and the +// dictionary is === to it. +const key = process.argv.length > 99 ? "never" : "added"; +type Point = { a: string }; + +// A type-literal record stored straight in a dictionary binding. +const direct: Point = { a: "x" }; +const directTable: Record = direct; +directTable[key] = "y"; +console.log(JSON.stringify(direct), (directTable as unknown) === direct); + +// The same record through a `{}` view. +const viewed: Point = { a: "x" }; +const view: {} = viewed; +const viewTable: Record = view; +viewTable[key] = "y"; +console.log(JSON.stringify(viewed)); + +// A declared index type, and a parameter. +interface Labels { + [name: string]: string; +} +function label(table: Labels, name: string): void { + table[name] = "set"; +} +const passed: Point = { a: "x" }; +label(passed, key); +console.log(JSON.stringify(passed)); + +// A `{}` parameter converted inside the function, called with a record +// and with a dictionary. +function viaView(value: {}): void { + const table: Record = value; + table[key] = "y"; +} +const record: Point = { a: "x" }; +viaView(record); +const dictionary: Record = { a: "x" }; +viaView(dictionary); +console.log(JSON.stringify(record), JSON.stringify(dictionary)); + +// A class instance through a `{}` view: a computed write replaces a method. +class Greeter { + name = "g"; + greet(): string { + return "hello " + this.name; + } +} +const greeter = new Greeter(); +const greeterView: {} = greeter; +const slots: Record = greeterView; +slots[process.argv.length > 99 ? "never" : "greet"] = "replaced"; +slots["name"] = "renamed"; +console.log(typeof greeter.greet, greeter.name, JSON.stringify(greeter)); + +// Deleting through the dictionary removes the key from the object. +const pruned: { a: string; b?: string } = { a: "x", b: "y" }; +const prunedTable: Record = pruned; +delete prunedTable[process.argv.length > 99 ? "never" : "b"]; +console.log(JSON.stringify(pruned), "b" in pruned); diff --git a/tests/corpus/7013-dictionary-view-parameter.ts b/tests/corpus/7013-dictionary-view-parameter.ts new file mode 100644 index 0000000000..859f22c235 --- /dev/null +++ b/tests/corpus/7013-dictionary-view-parameter.ts @@ -0,0 +1,11 @@ +// A record passed to a `{}` parameter and converted to a dictionary inside +// the function is the same object: the write through the dictionary +// reaches the caller's record. +const key = process.argv.length > 99 ? "never" : "added"; +function stamp(value: {}): void { + const table: Record = value; + table[key] = "y"; +} +const record: { a: string } = { a: "x" }; +stamp(record); +console.log(JSON.stringify(record), key in record); diff --git a/tests/coverage-fixtures/dictionary-view-copies.ts b/tests/coverage-fixtures/dictionary-view-copies.ts deleted file mode 100644 index bfcfa61f44..0000000000 --- a/tests/coverage-fixtures/dictionary-view-copies.ts +++ /dev/null @@ -1,29 +0,0 @@ -// A class instance or record held as `{}`, `object` or `unknown` keeps the -// original object, and converting it to a dictionary type copies it field -// by field. Each site below then writes through or compares the copy. -class Counter { - label = "c"; - add(): number { - return this.label.length; - } -} -const key = process.argv.length > 5 ? "z" : "b"; - -const counter = new Counter(); -const view: {} = counter; -const table: Record = view; -table[key] = "y"; - -const point = { a: "x" }; -const loose: object = point; -const dict = loose as Record; -dict[key] = "y"; - -const held: unknown = { a: "x" }; -const fromUnknown = held as Record; -console.log(fromUnknown === held); - -const empty: {} = {}; -const filled: Record = empty; -filled[key] = "y"; -console.log(JSON.stringify(counter), JSON.stringify(point), JSON.stringify(empty)); diff --git a/tests/coverage-fixtures/no-divergences.ts b/tests/coverage-fixtures/no-divergences.ts index ca76bcc324..70409d3192 100644 --- a/tests/coverage-fixtures/no-divergences.ts +++ b/tests/coverage-fixtures/no-divergences.ts @@ -45,3 +45,10 @@ const opaque: {} = counts; const again: Record = opaque; again[counts["a"] ?? "a"] = "y"; console.log(JSON.stringify(counts)); + +// A record stored in a dictionary binding and written through it stays +// the same object. +const named: { first: string } = { first: "a" }; +const byName: Record = named; +byName[named.first] = "b"; +console.log(JSON.stringify(named)); diff --git a/tests/harness/__snapshots__/coverage-dictionary-view-copies.txt b/tests/harness/__snapshots__/coverage-dictionary-view-copies.txt deleted file mode 100644 index 2359a4ffa8..0000000000 --- a/tests/harness/__snapshots__/coverage-dictionary-view-copies.txt +++ /dev/null @@ -1,21 +0,0 @@ -scriptc coverage tests/coverage-fixtures/dictionary-view-copies.ts - - statements analyzed 18 - compile statically 18 (100%) - in dynamic tier 3 (operate on values typed only at runtime; --explain-dynamic lists them) - - fully static — all analyzed statements compile to native code. - - differs from Node 4 sites (compiles; the native program can behave differently) - ×1 SC6001 storing a 'Counter' in a 'Record<…>' binding copies the object (converting an object held as '{}', 'object' or 'unknown' to a dictionary type compiles as a field copy): the write to 'table[key]' upda… - at tests/coverage-fixtures/dictionary-view-copies.ts:14:39 - hint: keep the value at its own type ('Counter'), or declare the dictionary's values as 'unknown', which keeps the original object - ×1 SC6001 storing a '{}' in a 'Record<…>' binding copies the object (converting an object held as '{}', 'object' or 'unknown' to a dictionary type compiles as a field copy): the write to 'filled[key]' updates … - at tests/coverage-fixtures/dictionary-view-copies.ts:27:40 - hint: keep the value at its own type ('{}'), or declare the dictionary's values as 'unknown', which keeps the original object - ×1 SC6001 asserting a '{ a: string; }' as 'Record<…>' copies the object (converting an object held as '{}', 'object' or 'unknown' to a dictionary type compiles as a field copy): the write to 'dict[key]' update… - at tests/coverage-fixtures/dictionary-view-copies.ts:19:14 - hint: keep the value at its own type ('{ a: string; }'), or declare the dictionary's values as 'unknown', which keeps the original object - ×1 SC6001 asserting a '{ a: string; }' as 'Record<…>' copies the object (converting an object held as '{}', 'object' or 'unknown' to a dictionary type compiles as a field copy): 'fromUnknown === held' compares… - at tests/coverage-fixtures/dictionary-view-copies.ts:23:21 - hint: keep the value at its own type ('{ a: string; }'), or declare the dictionary's values as 'unknown', which keeps the original object \ No newline at end of file diff --git a/tests/harness/corpus-costs/7012-dictionary-views-keep-objects.ts.json b/tests/harness/corpus-costs/7012-dictionary-views-keep-objects.ts.json new file mode 100644 index 0000000000..dcf40a9fae --- /dev/null +++ b/tests/harness/corpus-costs/7012-dictionary-views-keep-objects.ts.json @@ -0,0 +1 @@ +{ "plain": 2368, "san": 3286 } diff --git a/tests/harness/corpus-costs/7013-dictionary-view-parameter.ts.json b/tests/harness/corpus-costs/7013-dictionary-view-parameter.ts.json new file mode 100644 index 0000000000..9118817686 --- /dev/null +++ b/tests/harness/corpus-costs/7013-dictionary-view-parameter.ts.json @@ -0,0 +1 @@ +{ "plain": 1466, "san": 2392 } diff --git a/tests/harness/coverage.test.ts b/tests/harness/coverage.test.ts index f3958c5fe7..86d202b461 100644 --- a/tests/harness/coverage.test.ts +++ b/tests/harness/coverage.test.ts @@ -107,20 +107,6 @@ test("record width copies report at every storing site", async () => { ); }); -test("dictionary conversions of checked-dynamic views report copies", async () => { - const { coverage } = analyze(fixture("dictionary-view-copies.ts")); - expect(coverage.diagnostics).toEqual([]); - expect((coverage.divergences ?? []).map((d) => d.code)).toEqual([ - "SC6001", - "SC6001", - "SC6001", - "SC6001", - ]); - await expect(report(fixture("dictionary-view-copies.ts"))).toMatchFileSnapshot( - "__snapshots__/coverage-dictionary-view-copies.txt", - ); -}); - test("ordinary code reports no divergence", () => { const { coverage } = analyze(fixture("no-divergences.ts")); expect(coverage.diagnostics).toEqual([]); From 6dfde9919c4cc2f9ef29adf4a77bd1fc2eb3bfca Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Fri, 23 Oct 2026 20:45:37 -0500 Subject: [PATCH 3/4] Keep checked dictionary protocols and invalidate direct calls after method writes --- .../lowering/class-method-mutations.ts | 13 +++++-- .../src/frontend/lowering/class-protocols.ts | 8 ++-- .../test/class-method-mutations.test.ts | 17 +++++++++ .../compiler/test/dictionary-views.test.ts | 12 ++++++ .../7012-dictionary-views-keep-objects.ts | 37 +++++++++++++++++++ 5 files changed, 80 insertions(+), 7 deletions(-) diff --git a/packages/compiler/src/frontend/lowering/class-method-mutations.ts b/packages/compiler/src/frontend/lowering/class-method-mutations.ts index a07a4d3e12..789ee7a8cb 100644 --- a/packages/compiler/src/frontend/lowering/class-method-mutations.ts +++ b/packages/compiler/src/frontend/lowering/class-method-mutations.ts @@ -170,14 +170,19 @@ export function collectClassMethodMutations( // 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)) + // signature, and then every method must be assignable to its value type, + // or when the dictionary keeps checked identity: it then holds the + // existing objects it receives, class instances included. + const functionFreeDictionary = (value: ts.Expression): boolean => { + const type = lowerer.typeOf(value); + if (lowerer.classImplementedProtocol(type)) return false; + return lowerer.checker + .getIndexInfosOfType(type) .some( (index) => (index.keyType.flags & ts.TypeFlags.StringLike) !== 0 && functionFree(index.type), ); + }; const literalLoopKeys = (key: ts.Expression): (string | number)[] | null => { if (!ts.isIdentifier(key)) return null; const symbol = lowerer.checker.getSymbolAtLocation(key); diff --git a/packages/compiler/src/frontend/lowering/class-protocols.ts b/packages/compiler/src/frontend/lowering/class-protocols.ts index 4edf1ab7e8..00eedb6c03 100644 --- a/packages/compiler/src/frontend/lowering/class-protocols.ts +++ b/packages/compiler/src/frontend/lowering/class-protocols.ts @@ -983,11 +983,13 @@ export class ClassProtocols { if ((type.flags & ts.TypeFlags.Object) === 0) return false; const previous = this.answers.get(type); if (previous !== undefined) return previous; + // A dictionary keeps checked identity when the protocols already mark + // it (a literal whose methods read `this`) or when it receives existing + // objects (markDictionaryViews). const dictionaryValue = this.dictionaryValueType(type); const answer = - dictionaryValue !== undefined - ? (this.index(), this.dictionaryViews.has(dictionaryValue)) - : this.protocol(type); + this.protocol(type) || + (dictionaryValue !== undefined && this.dictionaryViews.has(dictionaryValue)); this.answers.set(type, answer); return answer; } diff --git a/packages/compiler/test/class-method-mutations.test.ts b/packages/compiler/test/class-method-mutations.test.ts index b63e47619a..3e29a1da2c 100644 --- a/packages/compiler/test/class-method-mutations.test.ts +++ b/packages/compiler/test/class-method-mutations.test.ts @@ -178,3 +178,20 @@ test("a store into a generic dictionary of dictionaries keeps checked dispatch", .replace("const settings: Settings =", "const settings: Settings ="); expect(await callersOf(source, "%dyn.class.properties")).not.toEqual([]); }); + +test("a runtime-key store into a dictionary view of a class instance keeps checked dispatch", async () => { + const source = ` +class Counter { + n = 0; + tick(): number { return ++this.n; } +} +const counter = new Counter(); +const view: {} = counter; +const slots: Record = view; +const key: string = String(process.argv.length); +slots[key] = "replaced"; +console.log(counter.tick()); +`; + // The method has a slot the write can replace, which calls consult. + expect(await callersOf(source, "%Counter.tick")).toContain("%method.value:%Counter.tick"); +}); diff --git a/packages/compiler/test/dictionary-views.test.ts b/packages/compiler/test/dictionary-views.test.ts index b841447cd4..a9cfd1056e 100644 --- a/packages/compiler/test/dictionary-views.test.ts +++ b/packages/compiler/test/dictionary-views.test.ts @@ -75,3 +75,15 @@ console.log(table["a"]);`), ); expect(kinds["table"]).toBe("record"); }); + +test("a dictionary literal whose methods read this keeps checked identity", async () => { + const kinds = await globalKinds( + program(`interface Table { [key: string]: () => number } +const methods: Table = { + base() { return 2; }, + twice() { return this.base() * 2; }, +}; +console.log(methods.twice());`), + ); + expect(kinds["methods"]).toBe("dyn"); +}); diff --git a/tests/corpus/7012-dictionary-views-keep-objects.ts b/tests/corpus/7012-dictionary-views-keep-objects.ts index 8ede809267..0df47cbb8c 100644 --- a/tests/corpus/7012-dictionary-views-keep-objects.ts +++ b/tests/corpus/7012-dictionary-views-keep-objects.ts @@ -53,9 +53,46 @@ const slots: Record = greeterView; slots[process.argv.length > 99 ? "never" : "greet"] = "replaced"; slots["name"] = "renamed"; console.log(typeof greeter.greet, greeter.name, JSON.stringify(greeter)); +// A write under a runtime string key replaces a method too, and a direct +// call reaches the replacement, which is not a function. +class Counter { + n = 0; + tick(): number { + return ++this.n; + } +} +const counter = new Counter(); +const counterView: {} = counter; +const counterSlots: Record = counterView; +const methodKey: string = process.argv.length > 99 ? "never" : "tick"; +counterSlots[methodKey] = "replaced"; +try { + console.log(counter.tick()); +} catch (error) { + console.log((error as Error).name, (error as Error).message); +} +try { + console.log(greeter.greet()); +} catch (error) { + console.log((error as Error).name, (error as Error).message); +} // Deleting through the dictionary removes the key from the object. const pruned: { a: string; b?: string } = { a: "x", b: "y" }; const prunedTable: Record = pruned; delete prunedTable[process.argv.length > 99 ? "never" : "b"]; console.log(JSON.stringify(pruned), "b" in pruned); + +// A dictionary literal whose methods read `this` keeps its live object. +interface Table { + [key: string]: () => number; +} +const table: Table = { + base() { + return 2; + }, + twice() { + return this.base() * 2; + }, +}; +console.log(table.twice(), Object.keys(table).join(",")); From 43b0f290119f12b92ed0a021efc1a5adc04aa91f Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Fri, 23 Oct 2026 21:34:14 -0500 Subject: [PATCH 4/4] Guard direct calls after dot-property method writes through dictionary views --- .../lowering/class-method-mutations.ts | 18 +++++++++++++++- .../test/class-method-mutations.test.ts | 16 ++++++++++++++ .../corpus/7013-dictionary-view-parameter.ts | 21 ++++++++++++++++++- 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/packages/compiler/src/frontend/lowering/class-method-mutations.ts b/packages/compiler/src/frontend/lowering/class-method-mutations.ts index 789ee7a8cb..2c792c4d2c 100644 --- a/packages/compiler/src/frontend/lowering/class-method-mutations.ts +++ b/packages/compiler/src/frontend/lowering/class-method-mutations.ts @@ -173,6 +173,19 @@ export function collectClassMethodMutations( // signature, and then every method must be assignable to its value type, // or when the dictionary keeps checked identity: it then holds the // existing objects it receives, class instances included. + // A string-keyed dictionary that keeps checked identity holds the + // existing objects it receives, class instances included, so any write + // through it can replace a method. + const dictionaryView = (value: ts.Expression): boolean => { + const type = lowerer.typeOf(value); + return ( + lowerer.checker.getPropertiesOfType(type).length === 0 && + lowerer.checker + .getIndexInfosOfType(type) + .some((index) => (index.keyType.flags & ts.TypeFlags.StringLike) !== 0) && + lowerer.classImplementedProtocol(type) + ); + }; const functionFreeDictionary = (value: ts.Expression): boolean => { const type = lowerer.typeOf(value); if (lowerer.classImplementedProtocol(type)) return false; @@ -582,7 +595,10 @@ export function collectClassMethodMutations( ); for (const { target, site } of propertyWrites) { const name = target.name.text; - if (functionFree(lowerer.typeOf(target)) || !methodOwnerCanInhabit(target.expression, name)) + if ( + !dictionaryView(target.expression) && + (functionFree(lowerer.typeOf(target)) || !methodOwnerCanInhabit(target.expression, name)) + ) continue; if (lowerer.prototypeMethodAccesses.has(name)) lowerer.noteMethodSite(name, site); else lowerer.noteMethodAccess(name, site); diff --git a/packages/compiler/test/class-method-mutations.test.ts b/packages/compiler/test/class-method-mutations.test.ts index 3e29a1da2c..166b9d5560 100644 --- a/packages/compiler/test/class-method-mutations.test.ts +++ b/packages/compiler/test/class-method-mutations.test.ts @@ -195,3 +195,19 @@ console.log(counter.tick()); // The method has a slot the write can replace, which calls consult. expect(await callersOf(source, "%Counter.tick")).toContain("%method.value:%Counter.tick"); }); + +test("a dot store into a dictionary view of a class instance keeps checked dispatch", async () => { + const source = ` +class Lamp { + on = false; + toggle(): boolean { this.on = !this.on; return this.on; } +} +const lamp = new Lamp(); +const view: {} = lamp; +const slots: Record = view; +slots.toggle = "replaced"; +console.log(lamp.toggle()); +`; + // The method has a slot the write can replace, which calls consult. + expect(await callersOf(source, "%Lamp.toggle")).toContain("%method.value:%Lamp.toggle"); +}); diff --git a/tests/corpus/7013-dictionary-view-parameter.ts b/tests/corpus/7013-dictionary-view-parameter.ts index 859f22c235..2bfc34a760 100644 --- a/tests/corpus/7013-dictionary-view-parameter.ts +++ b/tests/corpus/7013-dictionary-view-parameter.ts @@ -1,6 +1,7 @@ // A record passed to a `{}` parameter and converted to a dictionary inside // the function is the same object: the write through the dictionary -// reaches the caller's record. +// reaches the caller's record. A dot write through a dictionary view of a +// class instance replaces its method. const key = process.argv.length > 99 ? "never" : "added"; function stamp(value: {}): void { const table: Record = value; @@ -9,3 +10,21 @@ function stamp(value: {}): void { const record: { a: string } = { a: "x" }; stamp(record); console.log(JSON.stringify(record), key in record); + +// A dot write through the dictionary view replaces a method too. +class Lamp { + on = false; + toggle(): boolean { + this.on = !this.on; + return this.on; + } +} +const lamp = new Lamp(); +const lampView: {} = lamp; +const lampSlots: Record = lampView; +lampSlots.toggle = "replaced"; +try { + console.log(lamp.toggle()); +} catch (error) { + console.log((error as Error).name, (error as Error).message); +}