Lower Object.getOwnPropertyNames over records and Array.prototype.join with a dynamic separator (#2475)

packages/compiler/src/frontend/lowering/containers/array-methods.ts

@@ -242,50 +242,200 @@ function lowerArrayPosition(
   };
 }
 
-function lowerArrayJoinSeparator(
+/** `receiver.join(node)`: the separator defaults to "," for an absent or
+ * undefined argument and is ToString of anything else. */
+function lowerArrayJoin(
   lowerer: Lowerer,
+  receiver: IrExpr,
   node: ts.Expression | undefined,
   loc: SrcLoc,
 ): IrExpr {
+  const join = (sep: IrExpr): IrExpr => arrayJoin(receiver, sep, loc);
   const comma = strLit(",", loc);
-  if (!node) return comma;
+  if (!node) return join(comma);
   const undefinedValue = lowerStaticallyUndefinedArgument(lowerer, node);
-  if (undefinedValue) return defaultAfterUndefined(undefinedValue, comma);
+  if (undefinedValue) return join(defaultAfterUndefined(undefinedValue, comma));
   const value = lowerer.lowerExpr(node);
-  if (value.type.kind === "nullT") return defaultAfterUndefined(value, strLit("null", loc));
+  if (value.type.kind === "nullT") return join(defaultAfterUndefined(value, strLit("null", loc)));
+  const ternary = (cond: IrExpr, else_: IrExpr): IrExpr => ({
+    kind: "ternary",
+    cond,
+    then: comma,
+    else_,
+    type: STRING,
+    loc,
+  });
+  // The separator from its value: "," for the undefined value, ToString of
+  // anything else (null prints "null", a symbol throws the catchable
+  // TypeError).
+  let separatorOf = (ref: IrExpr): IrExpr => lowerer.ensureString(ref, node);
+  let held = false;
   if (value.type.kind === "union") {
-    const undefinedTag = lowerer.armTag(value.type.unionId, UNDEFINED_T);
+    const unionId = value.type.unionId;
+    const undefinedTag = lowerer.armTag(unionId, UNDEFINED_T);
     if (undefinedTag >= 0) {
-      const local = lowerer.declareHiddenLocal("%joinSeparator", value.type);
-      const ref = varRef(local.id, value.type, loc);
-      return {
-        kind: "seqExpr",
-        stmts: [{ kind: "varDecl", localId: local.id, init: value, loc }],
-        result: {
-          kind: "ternary",
-          cond: {
+      held = true;
+      separatorOf = (ref) =>
+        ternary(
+          {
             kind: "unionIsTag",
-            unionId: value.type.unionId,
+            unionId,
             tag: undefinedTag,
             value: ref,
             negated: false,
             type: BOOL,
             loc,
           },
-          then: comma,
-          else_: lowerer.ensureString(ref, node),
-          type: STRING,
-          loc,
-        },
+          lowerer.ensureString(ref, node),
+        );
+    }
+  } else if (value.type.kind === "dyn") {
+    held = true;
+    separatorOf = (ref) =>
+      ternary(
+        { kind: "dynTest", test: "undefined", value: ref, type: BOOL, loc },
+        { kind: "libCall", fn: "dyn.toStringCoerce", args: [ref], type: STRING, loc },
+      );
+  }
+  if (!convertsWithoutCode(lowerer, value.type))
+    return orderedSeparatorJoin(lowerer, receiver, value, separatorOf, loc);
+  if (!held) return join(separatorOf(value));
+  const local = lowerer.declareHiddenLocal("%joinSeparator", value.type);
+  return join({
+    kind: "seqExpr",
+    stmts: [{ kind: "varDecl", localId: local.id, init: value, loc }],
+    result: separatorOf(varRef(local.id, value.type, loc)),
+    type: STRING,
+    loc,
+  });
+}
+
+/** Whether ToString of a `type` value runs no code of the program: primitives
+ * and unions of them. */
+function convertsWithoutCode(lowerer: Lowerer, type: IrType): boolean {
+  if (type.kind === "union")
+    return (
+      lowerer.unions.get(type.unionId)?.arms.every((arm) => convertsWithoutCode(lowerer, arm)) ??
+      false
+    );
+  return (
+    type.kind === "string" ||
+    type.kind === "f64" ||
+    type.kind === "bool" ||
+    type.kind === "nullT" ||
+    type.kind === "undefinedT"
+  );
+}
+
+function arrayJoin(receiver: IrExpr, sep: IrExpr, loc: SrcLoc): IrExpr {
+  return { kind: "arrIntrinsic", method: "join", receiver, args: [sep], type: STRING, loc };
+}
+
+/** `receiver.join(value)` where ToString of the separator value can run code
+ * of the program (an object's toString, a checked-dynamic value).
+ * Array.prototype.join reads the length before it converts the separator,
+ * and that code can change the array, so the join covers exactly the
+ * elements below that length: a longer array joins its first ones, and the
+ * missing elements of a shorter one join as empty strings. */
+function orderedSeparatorJoin(
+  lowerer: Lowerer,
+  receiver: IrExpr,
+  value: IrExpr,
+  separatorOf: (ref: IrExpr) => IrExpr,
+  loc: SrcLoc,
+): IrExpr {
+  const array = lowerer.declareHiddenLocal("%joinArray", receiver.type);
+  const separatorValue = lowerer.declareHiddenLocal("%joinSeparatorValue", value.type);
+  const length = lowerer.declareHiddenLocal("%joinLength", F64);
+  const separator = lowerer.declareHiddenLocal("%joinSeparator", STRING);
+  const arrayRef = varRef(array.id, receiver.type, loc);
+  const separatorValueRef = varRef(separatorValue.id, value.type, loc);
+  const lengthRef = varRef(length.id, F64, loc);
+  const separatorRef = varRef(separator.id, STRING, loc);
+  const currentLength = (): IrExpr => ({
+    kind: "arrIntrinsic",
+    method: "length",
+    receiver: arrayRef,
+    args: [],
+    type: F64,
+    loc,
+  });
+  const compare = (op: "===" | ">", left: IrExpr, right: IrExpr): IrExpr => ({
+    kind: "bin",
+    op,
+    left,
+    right,
+    type: BOOL,
+    loc,
+  });
+  const minus = (left: IrExpr, right: IrExpr): IrExpr => ({
+    kind: "bin",
+    op: "-",
+    left,
+    right,
+    type: F64,
+    loc,
+  });
+  const repeat = (count: IrExpr): IrExpr => ({
+    kind: "strIntrinsic",
+    method: "repeat",
+    receiver: separatorRef,
+    args: [count],
+    type: STRING,
+    loc,
+  });
+  const ternary = (cond: IrExpr, then: IrExpr, else_: IrExpr): IrExpr => ({
+    kind: "ternary",
+    cond,
+    then,
+    else_,
+    type: STRING,
+    loc,
+  });
+  const first: IrExpr = {
+    kind: "arrIntrinsic",
+    method: "slice",
+    receiver: arrayRef,
+    args: [numLit(0, loc), lengthRef],
+    type: receiver.type,
+    loc,
+  };
+  const changed = ternary(
+    compare(">", currentLength(), lengthRef),
+    arrayJoin(first, separatorRef, loc),
+    ternary(
+      compare("===", currentLength(), numLit(0, loc)),
+      repeat(minus(lengthRef, numLit(1, loc))),
+      {
+        kind: "strConcat",
+        left: arrayJoin(arrayRef, separatorRef, loc),
+        right: repeat(minus(lengthRef, currentLength())),
         type: STRING,
         loc,
-      };
-    }
-  }
-  if (value.type.kind === "dyn") {
-    lowerer.noLowering(".join separator with a runtime-dependent undefined value", node);
-  }
-  return lowerer.ensureString(value, node);
+      },
+    ),
+  );
+  return {
+    kind: "seqExpr",
+    stmts: [
+      { kind: "varDecl", localId: array.id, init: receiver, loc },
+      { kind: "varDecl", localId: separatorValue.id, init: value, loc },
+      { kind: "varDecl", localId: length.id, init: currentLength(), loc },
+      {
+        kind: "varDecl",
+        localId: separator.id,
+        init: separatorOf(separatorValueRef),
+        loc,
+      },
+    ],
+    result: ternary(
+      compare("===", currentLength(), lengthRef),
+      arrayJoin(arrayRef, separatorRef, loc),
+      changed,
+    ),
+    type: STRING,
+    loc,
+  };
 }
 
 /** Ambient array method calls. `push`/`unshift`/`pop`/`reverse`/
@@ -1190,8 +1340,7 @@ export function lowerArrayMethodCall(
       );
     }
     const receiver = lowerer.lowerExpr(access.expression);
-    const sep = lowerArrayJoinSeparator(lowerer, call.arguments.nodes[0], loc);
-    return { kind: "arrIntrinsic", method: "join", receiver, args: [sep], type: STRING, loc };
+    return lowerArrayJoin(lowerer, receiver, call.arguments.nodes[0], loc);
   }
   if (name === "map" || name === "filter" || name === "forEach") {
     return lowerArrayHofCall(lowerer, call, access, name, elem);

packages/compiler/src/frontend/lowering/lower-calls.ts

@@ -8910,6 +8910,56 @@ function lowerObjectStaticCall(
     const helper = recordHasOwnHelper(lowerer, receiver.type.shapeId, loc);
     return { kind: "call", callee: helper, args: [receiver, key], type: BOOL, loc };
   }
+  // Object.getOwnPropertyNames over a fixed record: every own property a
+  // scriptc record carries is an enumerable declared field (or an index
+  // signature's overflow), so the answer is exactly Object.keys' walk —
+  // same construction, same interned helper. Tuples keep the fence (an
+  // array's own names include "length" and its indices), and so do
+  // accessor-carrying shapes (the walk cannot list accessor names).
+  if (member === "getOwnPropertyNames") {
+    if (call.arguments.nodes.length !== 1 || ts.isSpreadElement(call.arguments.nodes[0]!))
+      return null;
+    const node = call.arguments.nodes[0]!;
+    let recordT = lowerer.mapTypeOf(lowerer.typeOf(node));
+    const represented = tryLowerExpression(lowerer, node);
+    if (represented?.type.kind === "record") recordT = represented.type;
+    if (recordT?.kind !== "record") return null; // arrays, Maps, classes → the fence
+    const shape = lowerer.shapes.get(recordT.shapeId);
+    if (!shape || shape.tuple) return null; // tuple → the fence
+    fencePerfEntryKeyWalk(lowerer, recordT, call, "Object.getOwnPropertyNames");
+    fenceX509KeyWalk(lowerer, recordT, call, "Object.getOwnPropertyNames");
+    if (shapeHasAccessorSlots(shape)) {
+      lowerer.unsupported(
+        "SC1090",
+        call,
+        "Object.getOwnPropertyNames over a shape carrying get/set accessor properties (Node lists the accessor names — the static key walk cannot; read the properties explicitly)",
+      );
+    }
+    if (shape.indexValue) {
+      return lowerObjectIterOverIndexShape(lowerer, call, "keys", recordT, shape);
+    }
+    const loc = locOf(call);
+    const enumerated = objectEnumerationReceiver(lowerer, lowerer.lowerExpr(node), recordT, loc);
+    const answer = (receiver: IrExpr): IrExpr =>
+      recordKeysArrayCall(lowerer, receiver, recordT, shape, loc);
+    // A record that kept undeclared properties of its checked source
+    // answers from its live checked view (the array checks back into
+    // string[]).
+    return recordMayKeepSource(lowerer, recordT)
+      ? throughRecordSource(lowerer, enumerated, loc, answer, (view, type) => ({
+          kind: "dynCheck",
+          value: {
+            kind: "libCall",
+            fn: "dyn.getOwnPropertyNames",
+            args: [view],
+            type: DYN,
+            loc,
+          },
+          type,
+          loc,
+        }))
+      : answer(enumerated);
+  }
   if (member !== "keys" && member !== "values" && member !== "entries") return null;
   if (call.arguments.nodes.length !== 1 || ts.isSpreadElement(call.arguments.nodes[0]!))
     return null;

packages/compiler/test/ts7/baselines/order-parity/tests/corpus/array-join-dynamic-separator.ts.json

@@ -0,0 +1,6 @@
+{
+ "order": [
+  "<repo>/tests/corpus/array-join-dynamic-separator.ts"
+ ],
+ "diags": []
+}

packages/compiler/test/ts7/baselines/order-parity/tests/corpus/array-join-separator-reads-length-first.js.json

@@ -0,0 +1,6 @@
+{
+ "order": [
+  "<repo>/tests/corpus/array-join-separator-reads-length-first.js"
+ ],
+ "diags": []
+}

packages/compiler/test/ts7/baselines/order-parity/tests/corpus/object-get-own-property-names-records.ts.json

@@ -0,0 +1,6 @@
+{
+ "order": [
+  "<repo>/tests/corpus/object-get-own-property-names-records.ts"
+ ],
+ "diags": []
+}

tests/corpus/array-join-dynamic-separator.ts

@@ -0,0 +1,32 @@
+// Array.prototype.join over a checked-dynamic separator: only the
+// undefined VALUE means the default comma; every other value is ToString
+// (null prints "null", numbers and booleans their text), and the
+// separator expression evaluates once.
+
+function joinWith(sep: any): string {
+  return ["a", "b", "c"].join(sep);
+}
+console.log(joinWith(undefined));
+console.log(joinWith(null));
+console.log(joinWith("-"));
+console.log(joinWith(0));
+console.log(joinWith(false));
+console.log(joinWith(""));
+
+const held: any = "+";
+console.log(["x", "y"].join(held), ["x", "y"].join(undefined));
+const numbers = [1, 2, 3];
+console.log(numbers.join(held), numbers.join(null as any));
+
+let reads = 0;
+function separator(): any {
+  reads += 1;
+  return "/";
+}
+console.log(["p", "q"].join(separator()), reads);
+
+// A dynamic separator that turns out to be undefined keeps the default.
+function maybe(flagValue: boolean): any {
+  return flagValue ? "-" : undefined;
+}
+console.log(["m", "n"].join(maybe(true)), ["m", "n"].join(maybe(false)));

tests/corpus/array-join-separator-reads-length-first.js

@@ -0,0 +1,28 @@
+// Array.prototype.join reads the length before it converts the separator:
+// a separator whose toString changes the array joins exactly that many
+// elements, and elements it removed join as empty strings. Object literal,
+// class instance and untyped separators.
+function joinChanging(change) {
+  const values = ["a", "b", "c"];
+  const sep = {
+    toString() {
+      change(values);
+      return "-";
+    },
+  };
+  return values.join(sep);
+}
+console.log(joinChanging((values) => { values.push("d"); }));
+console.log(joinChanging((values) => { values.length = 1; }));
+console.log(joinChanging((values) => { values.length = 0; }));
+console.log(joinChanging((values) => { values[1] = "B"; }));
+console.log(joinChanging(() => {}));
+class Sep {
+  constructor(values) { this.values = values; }
+  toString() { this.values.length = 2; return "+"; }
+}
+const shrinking = ["x", "y", "z"];
+console.log(shrinking.join(new Sep(shrinking)));
+function untyped(values, sep) { return values.join(sep); }
+const grow = ["p", "q"];
+console.log(untyped(grow, { toString() { grow.push("r"); return "/"; } }));

tests/corpus/object-get-own-property-names-records.ts

@@ -0,0 +1,45 @@
+// Object.getOwnPropertyNames over a statically typed record answers the
+// record's own keys: the declared fields in declaration order (an absent
+// optional field stays out), and for an index signature the declared
+// fields then the overflow in own-key order. The answer is the same
+// string[] Object.keys builds, and a record that kept undeclared
+// properties of its checked source answers Node's full name list.
+
+type Schema = { type: string; minimum?: number; maximum?: number };
+
+const partial: Schema = { type: "number", maximum: 10 };
+const partialNames = Object.getOwnPropertyNames(partial);
+console.log(partialNames.join(","), partialNames.length);
+console.log(Array.isArray(partialNames), typeof partialNames[0], partialNames[0] === "type");
+
+const full: Schema = { type: "string", minimum: 1, maximum: 2 };
+console.log(Object.getOwnPropertyNames(full).join(","));
+const onlyType: Schema = { type: "boolean" };
+console.log(JSON.stringify(Object.getOwnPropertyNames(onlyType)));
+console.log(Object.getOwnPropertyNames(full).join("|") === Object.keys(full).join("|"));
+console.log(Object.getOwnPropertyNames(full).sort().join(","));
+console.log(Object.getOwnPropertyNames(full).map((name) => name.length).join(","));
+
+// Declaration order, not literal order.
+type Point = { y: number; x: number; label: string };
+const point: Point = { x: 1, y: 2, label: "p" };
+const walked: string[] = [];
+for (const name of Object.getOwnPropertyNames(point)) walked.push(name);
+console.log(walked.join(","), Object.getOwnPropertyNames(point).includes("x"));
+
+// An index signature: declared fields, then the overflow.
+const counters: Record<string, number> = { alpha: 1 };
+counters["beta"] = 2;
+counters["gamma"] = 3;
+console.log(Object.getOwnPropertyNames(counters).join(","), Object.getOwnPropertyNames(counters).length);
+
+// A record built by a checked conversion keeps undeclared own properties.
+const kept = JSON.parse('{"a":1,"extra":true,"b":2}') as { a: number };
+console.log(Object.getOwnPropertyNames(kept).join(","), Object.getOwnPropertyNames(kept).length);
+const closed = JSON.parse('{"a":1}') as { a: number };
+console.log(Object.getOwnPropertyNames(closed).join(","));
+
+// The array is a fresh, writable string[].
+const fresh = Object.getOwnPropertyNames(full);
+fresh.push("extra");
+console.log(fresh.length, Object.getOwnPropertyNames(full).length);

tests/harness/corpus-costs/array-join-dynamic-separator.ts.json

@@ -0,0 +1 @@
+{ "plain": 3300, "san": 6750 }

tests/harness/corpus-costs/array-join-separator-reads-length-first.js.json

@@ -0,0 +1 @@
+{ "plain": 1900, "san": 3000 }

tests/harness/corpus-costs/object-get-own-property-names-records.ts.json

@@ -0,0 +1 @@
+{ "plain": 3250, "san": 4700 }