Compare generic declarations by identity across pinned signatures (#2565)

packages/compiler/src/frontend/lowering/expressions/binary.ts

@@ -59,6 +59,7 @@ import { tryLowerExpression } from "./try-lower-expression.js";
 import { fenceNodeModuleMutation, lowerRequireCacheHas } from "../lower-node-module.js";
 import { errorToStringCall } from "../error-methods.js";
 import { classToStringConversion, classUnionToString } from "../class-to-primitive.js";
+import { noteDynamicIdentityComparison } from "../generic-functions.js";
 import { isPerfEntryType, perfEntryTag } from "../builtins/performance.js";
 import { isX509Type, x509ToString } from "../builtins/x509.js";
 import { guardNullProto, nullProtoRecordShape } from "../null-prototype.js";
@@ -1706,6 +1707,8 @@ export function lowerBinary(lowerer: Lowerer, expr: ts.BinaryExpression): IrExpr
             ? { liveRef: true as const }
             : {}),
         };
+        if (reference.kind === "func")
+          noteDynamicIdentityComparison(lowerer, expr, expr.left, expr.right);
         return {
           kind: "dynScalarEq",
           left: scalarSide === left ? boxed : left,
@@ -1761,6 +1764,12 @@ export function lowerBinary(lowerer: Lowerer, expr: ts.BinaryExpression): IrExpr
           // reference kinds by identity (scr_dyn_strict_eq).
           scalarSide.type.kind === "dyn")
       ) {
+        // A generic declaration read through `unknown` compares as the
+        // function object it names, which is one object in JavaScript but
+        // one closed instance per signature here: the whole-program check
+        // refuses it when the program holds the declaration at more than
+        // one signature.
+        noteDynamicIdentityComparison(lowerer, expr, expr.left, expr.right);
         return {
           kind: "dynScalarEq",
           left,
@@ -2722,6 +2731,11 @@ export function lowerStrictEquality(
     };
   }
   if (lowerer.dynConvertible(idLeft.type) && lowerer.dynConvertible(idRight.type)) {
+    // A generic declaration read through `unknown` compares as the function
+    // object it names, which is one object in JavaScript but one closed
+    // instance per signature here: the whole-program check refuses it when
+    // the program holds the declaration at more than one signature.
+    noteDynamicIdentityComparison(lowerer, node, leftNode, rightNode);
     return {
       kind: "dynScalarEq",
       left: lowerer.coerceToExpected(idLeft, DYN),

packages/compiler/src/frontend/lowering/generic-functions.ts

@@ -22,6 +22,7 @@ import {
   typeEquals,
 } from "../../ir/ir.js";
 import { helperLoc, nodeThrowExpr } from "../../ir/build.js";
+import { unsupportedDiag } from "../../diagnostics/diagnostic.js";
 import { isJsSourceFile, locOf } from "../program.js";
 import { typeKey } from "../type-mapper.js";
 import { functionCanReturnUndefined } from "../function-completion.js";
@@ -145,6 +146,11 @@ export interface GenericFnInfo {
   /** Parameter index → symbol for the function parameters whose `.name`
    * the body reads (static-function-names.ts); computed on first use. */
   nameReadParams?: Map<number, ts.Symbol>;
+  /** Names of the instances value references produced (pinnedGenericClosure,
+   * erasedGenericInstance, the implicit-any default): more than one means the
+   * program holds this declaration as several function objects, while
+   * JavaScript has one. */
+  valueInstances?: Set<string>;
 }
 
 /** One enclosing-frame value a generic arrow/function-expression binding
@@ -1440,6 +1446,19 @@ export function lowerGenericInstance(
   }
 }
 
+/** Records that `instance` is one of `info`'s value instances (see
+ * `valueInstances`). Written as its own statements rather than
+ * `(x ??= new Set()).add(...)`: the self-hosted frontend fences a Set method
+ * call on a non-Set receiver, and this compiler compiles its own source. */
+function noteValueInstance(info: GenericFnInfo, instance: string): void {
+  let instances = info.valueInstances;
+  if (instances === undefined) {
+    instances = new Set<string>();
+    info.valueInstances = instances;
+  }
+  instances.add(instance);
+}
+
 /** A generic function taken as a VALUE, monomorphized by flow. A function
  * value needs ONE concrete signature; tsc pins one at exactly two
  * reference shapes — an instantiation EXPRESSION (`identity<number>`,
@@ -1464,6 +1483,7 @@ export function lowerGenericFnValue(
   // closure per function, so `f === f` holds like any declaration).
   if (info.implicitParams) {
     const inst = implicitDefaultInstance(lowerer, ref, info);
+    noteValueInstance(info, inst.name);
     const funcType: IrType = funcTypeFromParamShapes(inst.params, inst.returnType);
     lowerer.requireExactArityValue(ref, ref, inst.params, funcType);
     lowerer.noteEdge(inst.name);
@@ -1513,6 +1533,10 @@ export function lowerGenericFnValue(
       if (callable.length === 1) target = callable[0]![0]!;
     }
   }
+  if (!target) {
+    target = identityComparisonPin(lowerer, ref);
+    if (target) lowerer.genericIdentityPins.push({ info, node: ref });
+  }
   if (!target) fenceUnpinned();
   return guardEarlyBindingValue(
     lowerer,
@@ -1522,6 +1546,41 @@ export function lowerGenericFnValue(
   );
 }
 
+/** The pinning signature of a generic declaration compared by identity
+ * (`held === ident`, `held !== ident`, and the loose forms): the other
+ * operand's one concrete call signature. A held value of that type that is
+ * this declaration is its instance at that signature, whose interned
+ * closure is the one this reference names, so the comparison answers as
+ * in JavaScript, provided the program holds the declaration at that one
+ * signature only (checkGenericIdentityPins). Null when the other operand
+ * has no single concrete call signature (another generic value, a union of
+ * callables, a non-function). */
+function identityComparisonPin(lowerer: Lowerer, ref: ts.Expression): ts.Signature | null {
+  let operand: ts.Node = ref;
+  if (
+    ref.parent !== undefined &&
+    ts.isPropertyAccessExpression(ref.parent) &&
+    ref.parent.name === ref
+  )
+    operand = ref.parent;
+  while (operand.parent !== undefined && ts.isParenthesizedExpression(operand.parent))
+    operand = operand.parent;
+  const bin = operand.parent;
+  if (bin === undefined || !ts.isBinaryExpression(bin)) return null;
+  const op = bin.operatorToken.kind;
+  if (
+    op !== ts.SyntaxKind.EqualsEqualsEqualsToken &&
+    op !== ts.SyntaxKind.ExclamationEqualsEqualsToken &&
+    op !== ts.SyntaxKind.EqualsEqualsToken &&
+    op !== ts.SyntaxKind.ExclamationEqualsToken
+  )
+    return null;
+  const other = bin.left === operand ? bin.right : bin.left;
+  const sigs = lowerer.checker.getCallSignatures(lowerer.typeOf(other));
+  if (sigs.length !== 1 || sigs[0]!.getTypeParameters()?.length) return null;
+  return sigs[0]!;
+}
+
 /** The value a generic binding declared inside a function stores when a
  * nested function read it before the declaration lowered (`const read =
  * () => id; const id: (x: number) => number = <T>(x: T): T => x`): such
@@ -1628,6 +1687,7 @@ function pinnedGenericClosure(
   // nothing: mapType answers null for an unsubstituted parameter.
   if (info.typeParams.some((tp) => !bindings.get(tp))) fenceUnpinned();
   const inst = genericValueInstance(lowerer, ref, info, bindings);
+  noteValueInstance(info, inst.name);
   // The value's type is the completed ABI signature. Typed rest keeps its
   // packed-array slot; dynRest stays hidden behind the rest marker.
   const funcType: IrType = funcTypeFromParamShapes(inst.params, inst.returnType);
@@ -1677,12 +1737,153 @@ export function erasedGenericInstance(
       new Map(info.typeParams.map((tp) => [tp, DYN])),
     );
     instance.erased = true;
+    noteValueInstance(info, instance.name);
     return instance;
   } finally {
     lowerer.erasingDepth--;
   }
 }
 
+/** After every body lowered: an identity comparison of a generic
+ * declaration is exact only when the program holds the declaration as one
+ * function object. Otherwise a held value may be another instance's
+ * closure, where JavaScript has one function, so the comparison is
+ * refused instead of answering false. */
+export function checkGenericIdentityPins(lowerer: Lowerer): void {
+  if (lowerer.genericIdentityCheckDone) return;
+  lowerer.genericIdentityCheckDone = true;
+  const seen = new Map<ts.Node, Set<GenericFnInfo>>();
+  const report = (info: GenericFnInfo, node: ts.Node, dynamic: boolean): void => {
+    let infos = seen.get(node);
+    if (infos === undefined) seen.set(node, (infos = new Set()));
+    if (infos.has(info)) return;
+    infos.add(info);
+    const held = info.valueInstances?.size ?? 0;
+    if (held <= 1) return;
+    const through = dynamic ? " through a dynamic value" : "";
+    lowerer.pushDiag(
+      unsupportedDiag(
+        "SC1090",
+        locOf(node),
+        `comparing the generic function '${info.baseName}' by identity${through} while the program holds it at ${held} different signatures (each is a separate compiled function object; JavaScript has one) — compare against a value pinned once, or hold '${info.baseName}' at a single signature`,
+      ),
+    );
+  };
+  for (const { info, node } of lowerer.genericIdentityPins) report(info, node, false);
+  // Dynamic comparisons resolve their provenance only when some declaration
+  // is held at more than one signature: the symbol lookups are the
+  // expensive part, and every other program answers them all at one
+  // instance. Recorded at lowering, resolved here so the pins' instance
+  // sets are complete.
+  let ambiguous = false;
+  for (const info of lowerer.genericFnsBySymbol.values()) {
+    if ((info.valueInstances?.size ?? 0) > 1) {
+      ambiguous = true;
+      break;
+    }
+  }
+  if (!ambiguous) return;
+  for (const { node, left, right } of lowerer.dynIdentityComparisons) {
+    for (const operand of [left, right]) {
+      const info = genericDeclarationBehindValue(lowerer, operand);
+      if (info) report(info, node, true);
+    }
+  }
+}
+
+/** The generic declaration whose value instance an expression names, when
+ * the expression is a binding initialized (directly, or through casts,
+ * aliasing bindings and property initializers) from that declaration. Used
+ * where a comparison reads the value through `unknown`/`any`, so the
+ * declaration is not syntactically at the compared site. */
+export function genericDeclarationBehindValue(
+  lowerer: Lowerer,
+  node: ts.Node,
+): GenericFnInfo | null {
+  let current: ts.Node | undefined = node;
+  const seen = new Set<ts.Node>();
+  while (current !== undefined && !seen.has(current)) {
+    seen.add(current);
+    if (
+      ts.isParenthesizedExpression(current) ||
+      ts.isAsExpression(current) ||
+      ts.isSatisfiesExpression(current) ||
+      ts.isNonNullExpression(current)
+    ) {
+      current = current.expression;
+      continue;
+    }
+    if (!ts.isIdentifier(current) && !ts.isPropertyAccessExpression(current)) return null;
+    const symbol = lowerer.checker.getSymbolAtLocation(
+      ts.isPropertyAccessExpression(current) ? current.name : current,
+    );
+    if (symbol === undefined) return null;
+    const info = lowerer.genericFnsBySymbol.get(symbol);
+    if (info !== undefined) return info;
+    const decl = symbol.valueDeclaration ?? symbol.declarations?.[0];
+    if (decl === undefined) return null;
+    if (!ts.isVariableDeclaration(decl) && !ts.isPropertyDeclaration(decl) && !ts.isParameter(decl))
+      return null;
+    current = decl.initializer;
+  }
+  return null;
+}
+
+/** An identity comparison at the dynamic boundary (`unknown`/`any`
+ * operands): when either operand reads a generic declaration's value
+ * instance, the comparison is exact only while the program holds that
+ * declaration at one signature (checkGenericIdentityPins). */
+export function noteDynamicIdentityComparison(
+  lowerer: Lowerer,
+  node: ts.Node,
+  left: ts.Node,
+  right: ts.Node,
+): void {
+  // A function compared with a string, number, boolean, bigint or symbol
+  // is unequal whichever instance it is, as in JavaScript.
+  if (isPrimitiveTyped(lowerer, left) || isPrimitiveTyped(lowerer, right)) return;
+  // The only work done while lowering. No instance count can gate this:
+  // a value reference's instance is interned when its binding settles,
+  // which can be later than the comparison that reads it, so the notes
+  // are resolved at the end of lowering, when every instance is known.
+  if (mightNameBinding(left) || mightNameBinding(right))
+    lowerer.dynIdentityComparisons.push({ node, left, right });
+}
+
+const PRIMITIVE_TYPE_FLAGS =
+  ts.TypeFlags.StringLike |
+  ts.TypeFlags.NumberLike |
+  ts.TypeFlags.BooleanLike |
+  ts.TypeFlags.BigIntLike |
+  ts.TypeFlags.ESSymbolLike;
+
+/** Whether the checker types `node` as a string, number, boolean, bigint or
+ * symbol (a union with any other member is not). */
+function isPrimitiveTyped(lowerer: Lowerer, node: ts.Node): boolean {
+  return (lowerer.typeOf(node).flags & PRIMITIVE_TYPE_FLAGS) !== 0;
+}
+
+/** Whether an expression could name a binding at all — an identifier or a
+ * property access, through parentheses and casts. A literal, a call, an
+ * operator expression: nothing to resolve. */
+function mightNameBinding(node: ts.Node): boolean {
+  let current: ts.Node | undefined = node;
+  for (let hops = 0; hops < 16 && current !== undefined; hops++) {
+    if (ts.isIdentifier(current) || ts.isPropertyAccessExpression(current)) return true;
+    if (
+      ts.isParenthesizedExpression(current) ||
+      ts.isAsExpression(current) ||
+      ts.isNonNullExpression(current) ||
+      ts.isSatisfiesExpression(current)
+    ) {
+      current = current.expression;
+      continue;
+    }
+    return false;
+  }
+  return false;
+}
+
 /** The instance a pinned VALUE reference names: the declaration's modes
  * over the DECLARED types mapped under the bindings (mapType's resolver
  * substitutes — the instance-body trick), which is the same result the

packages/compiler/src/frontend/lowering/lowerer.ts

@@ -538,6 +538,7 @@ import {
   inferTypeParamBindings,
   lowerGenericInstance,
   genericLocalClassBinding,
+  checkGenericIdentityPins,
 } from "./generic-functions.js";
 import { bindingNeverReassigned } from "./binding-analysis.js";
 import { isMessageListenerCall } from "./expressions/message-listeners.js";
@@ -2597,6 +2598,17 @@ export class Lowerer {
    * type parameters are bound to `unknown`, so lambdaSignature admits
    * them. */
   readonly erasingLambdas = new Set<ts.Node>();
+  /** Identity comparisons of a generic declaration met while lowering
+   * (directly against the declaration, or through a dynamic value): exact
+   * only while the program holds that declaration as one function object
+   * (checkGenericIdentityPins). */
+  readonly genericIdentityPins: { info: GenericFnInfo; node: ts.Node }[] = [];
+  /** Identity comparisons whose operands are dynamic values, recorded while
+   * lowering and resolved by checkGenericIdentityPins only when some
+   * generic declaration is held at more than one signature. */
+  readonly dynIdentityComparisons: { node: ts.Node; left: ts.Node; right: ts.Node }[] = [];
+  /** The identity comparison check reports once, after all lowering. */
+  genericIdentityCheckDone = false;
   /** Function expressions lowered against a fixed target signature with a
    * tuple-typed `arguments` (typed-arguments.ts). */
   readonly typedArgumentsLambdas = new Set<ts.Node>();
@@ -4308,6 +4320,8 @@ export class Lowerer {
       checkedTable("genericFunctionDemandOwner", () => this.genericFunctionDemandOwner.size),
       checkedTable("genericClassDemandOwner", () => this.genericClassDemandOwner.size),
       checkedTable("erasingLambdas", () => this.erasingLambdas.size),
+      checkedTable("genericIdentityPins", () => this.genericIdentityPins.length),
+      checkedTable("dynIdentityComparisons", () => this.dynIdentityComparisons.length),
       checkedTable("typedArgumentsLambdas", () => this.typedArgumentsLambdas.size),
       checkedTable("tupleAliasSymbols", () => this.tupleAliasSymbols.size),
       checkedTable("nullishFuncResults", () => this.nullishFuncResults.size),
@@ -4736,6 +4750,9 @@ export class Lowerer {
       fixed("errorAccessorReflection", () => this.errorAccessorReflection),
       fixed("subclassLinks", () => this.subclassLinks),
       fixed("errorPathCopies", () => this.errorPathCopies),
+      // A latch of the end-of-lowering audit (checkGenericIdentityPins): a
+      // unit never touches it.
+      fixed("genericIdentityCheckDone", () => this.genericIdentityCheckDone),
       fixed("errorMethodDispatchRevision", () => this.errorMethodDispatchRevision),
       fixed("errorPropertyDispatchRevision", () => this.errorPropertyDispatchRevision),
       fixed("childAdditions", () => this.childAdditions),
@@ -9754,6 +9771,9 @@ export class Lowerer {
         if (this.alreadyFlushed.has(symbol)) continue;
         for (const d of [...diags]) this.pushDiag(d);
       }
+      // Coverage visits more bodies; more value instances can only add
+      // refusals to the identity comparison check.
+      checkGenericIdentityPins(this);
       return {
         module: null,
         diagnostics: this.diags,
@@ -9770,6 +9790,10 @@ export class Lowerer {
   /** Final retention, pruning, and module assembly shared by ordinary emit
    * and the retained reachability worklist. */
   finishModule(functions: IrFunction[]): LowerResult {
+    // Every instance of every generic value is known now (the
+    // monomorphization worklists drained): an identity comparison against
+    // one is exact only if the declaration is held at a single signature.
+    checkGenericIdentityPins(this);
     boxBuiltinValuesChecked(this, functions);
     if (this.diags.length === 0) fuseArrayPipelines(this, functions);
     finalizeClassMethodValues(this, functions);

packages/compiler/test/ts7/baselines/order-parity/tests/corpus/generic-identity-pin.ts.json

@@ -0,0 +1,6 @@
+{
+ "order": [
+  "<repo>/tests/corpus/generic-identity-pin.ts"
+ ],
+ "diags": []
+}

packages/compiler/test/ts7/baselines/order-parity/tests/corpus/generic-identity-unknown-scalars.ts.json

@@ -0,0 +1,6 @@
+{
+ "order": [
+  "<repo>/tests/corpus/generic-identity-unknown-scalars.ts"
+ ],
+ "diags": []
+}

packages/compiler/test/ts7/baselines/order-parity/tests/corpus/generic-identity-unknown.ts.json

@@ -0,0 +1,6 @@
+{
+ "order": [
+  "<repo>/tests/corpus/generic-identity-unknown.ts"
+ ],
+ "diags": []
+}

packages/compiler/test/ts7/baselines/order-parity/tests/diagnostics/generic-identity-multiple-pins.ts.json

@@ -0,0 +1,6 @@
+{
+ "order": [
+  "<repo>/tests/diagnostics/generic-identity-multiple-pins.ts"
+ ],
+ "diags": []
+}

packages/compiler/test/ts7/baselines/order-parity/tests/diagnostics/generic-identity-unknown.ts.json

@@ -0,0 +1,6 @@
+{
+ "order": [
+  "<repo>/tests/diagnostics/generic-identity-unknown.ts"
+ ],
+ "diags": []
+}

tests/corpus/generic-identity-pin.ts

@@ -0,0 +1,17 @@
+// An unpinned generic declaration compared by identity against a held
+// function: the comparison pins the instance the other operand's type
+// names, so `h.fn === ident` answers as JavaScript does.
+type Fn = (x: number) => number;
+function ident<T>(x: T): T {
+  return x;
+}
+class Holder {
+  fn: Fn = ident;
+}
+const h = new Holder();
+console.log(h.fn === ident, h.fn(3));
+h.fn = (x) => x + 1;
+console.log(h.fn === ident, h.fn(3), ident !== h.fn);
+const fns: Fn[] = [ident, (x) => x * 2];
+console.log(fns.map((f) => (f === ident ? "ident" : "other")).join(","));
+export {};

tests/corpus/generic-identity-unknown-scalars.ts

@@ -0,0 +1,16 @@
+// A generic declaration held at two signatures, read through `unknown`,
+// compares unequal to strings, numbers and booleans whichever instance the
+// value holds, as in JavaScript.
+type Fn = (x: number) => number;
+function ident<T>(x: T): T {
+  return x;
+}
+const a: Fn = ident;
+const b: (x: string) => string = ident;
+const held: unknown = a;
+const label = "ident";
+const count = 0;
+const flag = true;
+console.log(held === label, held !== count, held === flag, label === held);
+console.log(held === "ident", held !== 1, false === held);
+console.log(a(2), b("s"));

tests/corpus/generic-identity-unknown.ts

@@ -0,0 +1,11 @@
+// A generic declaration held at one signature compares equal to itself
+// through `unknown`, and further calls at other signatures (which do not
+// hold the declaration as a second function object) stay compilable.
+type Fn = (x: number) => number;
+function ident<T>(x: T): T {
+  return x;
+}
+const f: Fn = ident;
+const u: unknown = f;
+console.log(u === f, u !== f, f(4));
+console.log(ident<number>(5), ident("s").toUpperCase());

tests/diagnostics/generic-identity-multiple-pins.ts

@@ -0,0 +1,11 @@
+// The program holds `ident` at two signatures and compares a value of one
+// against the declaration by identity. JavaScript has one function object;
+// the compiled program has one closure per signature, so the comparison is
+// refused rather than answered `false`.
+type Fn = (x: number) => number;
+function ident<T>(x: T): T {
+  return x;
+}
+const num: Fn = ident;
+const str: (x: string) => string = ident;
+console.log(num === ident, str("a"));

tests/diagnostics/generic-identity-unknown.ts

@@ -0,0 +1,12 @@
+// One generic declaration held at two signatures compares unequal through
+// `unknown` today (one closure per signature, one function object in
+// JavaScript). Refuse it instead of answering silently wrong.
+type Fn = (x: number) => number;
+function ident<T>(x: T): T {
+  return x;
+}
+const a: Fn = ident;
+const b: (x: string) => string = ident;
+const ua = a as unknown;
+const ub = b as unknown;
+console.log(ua === ub);

tests/harness/__snapshots__/generic-identity-multiple-pins.ts.txt

@@ -0,0 +1,6 @@
+generic-identity-multiple-pins.ts:11:21 - error SC1090: comparing the generic function 'ident' by identity while the program holds it at 2 different signatures (each is a separate compiled function object; JavaScript has one) — compare against a value pinned once, or hold 'ident' at a single signature are not supported yet
+
+  10 | const str: (x: string) => string = ident;
+  11 | console.log(num === ident, str("a"));
+     |                     ^~~~~
+  12 | 

tests/harness/__snapshots__/generic-identity-unknown.ts.txt

@@ -0,0 +1,6 @@
+generic-identity-unknown.ts:12:13 - error SC1090: comparing the generic function 'ident' by identity through a dynamic value while the program holds it at 2 different signatures (each is a separate compiled function object; JavaScript has one) — compare against a value pinned once, or hold 'ident' at a single signature are not supported yet
+
+  11 | const ub = b as unknown;
+  12 | console.log(ua === ub);
+     |             ^~~~~~~~~
+  13 | 

tests/harness/corpus-costs/generic-identity-pin.ts.json

@@ -0,0 +1 @@
+{ "plain": 2213, "san": 49301 }

tests/harness/corpus-costs/generic-identity-unknown-scalars.ts.json

@@ -0,0 +1 @@
+{ "plain": 8425, "san": 6439 }

tests/harness/corpus-costs/generic-identity-unknown.ts.json

@@ -0,0 +1 @@
+{ "plain": 1172, "san": 59112 }