Address review feedback on array named properties

packages/compiler/src/backend/llvm/lib-shared.ts

@@ -1520,6 +1520,7 @@ export const LIB_FN_SYMS: Record<string, string> = {
   "regex.groupParseInt": "scr_regex_group_parse_int",
   "regex.resultProp": "scr_regex_result_prop",
   "array.namedKeys": "scr_arr_named_keys",
+  "array.hasNamedKey": "scr_arr_has_named_key",
   // queueMicrotask's checked-dynamic form (borrowed dyn; a non-function
   // throws synchronously), the minted setImmediate value, and the
   // timers/promises immediate — all mark the loop live.

packages/compiler/src/backend/llvm/reference-writes.ts

@@ -106,6 +106,7 @@ const INERT_LIB_CALLS = new Set([
   "regex.groupParseInt",
   "regex.resultProp",
   "array.namedKeys",
+  "array.hasNamedKey",
   "process.cwd",
   "process.argv",
   "process.execArgv",

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

@@ -11620,21 +11620,23 @@ function lowerForInArray(
       loc,
     });
     // Indices first, then the named keys the array had when the loop began.
+    const namedKey = (): IrExpr => ({
+      kind: "arrayGet",
+      arr: keysRef(),
+      index: { kind: "bin", op: "-", left: iRef(), right: lenRef(), type: F64, loc },
+      type: STRING,
+      loc,
+    });
     const keyInit: IrExpr = {
       kind: "ternary",
       cond: isIndex(),
       then: { kind: "toString", operand: iRef(), type: STRING, loc },
-      else_: {
-        kind: "arrayGet",
-        arr: keysRef(),
-        index: { kind: "bin", op: "-", left: iRef(), right: lenRef(), type: F64, loc },
-        type: STRING,
-        loc,
-      },
+      else_: namedKey(),
       type: STRING,
       loc,
     };
-    // An index is visited while it is below the live length and not a hole.
+    // An index is visited while it is below the live length and not a hole,
+    // a named key while the array still has it.
     const visits: IrExpr = {
       kind: "ternary",
       cond: isIndex(),
@@ -11653,7 +11655,13 @@ function lowerForInArray(
         type: BOOL,
         loc,
       },
-      else_: { kind: "boolLit", value: true, type: BOOL, loc },
+      else_: {
+        kind: "libCall",
+        fn: "array.hasNamedKey",
+        args: [arrRef(), namedKey()],
+        type: BOOL,
+        loc,
+      },
       type: BOOL,
       loc,
     };

packages/compiler/src/ir/builtin-signatures.ts

@@ -3727,6 +3727,9 @@ export const LIB_FN_SIGS = defineLibFnSignatures({
   // The string keys `for...in` visits on an array (args[0], any element
   // type) after its indices. Borrowed; never throws.
   "array.namedKeys": { argTypes: [null], result: arrayOf(STRING) },
+  // Whether the array (args[0]) still has the key (args[1]) from its
+  // namedKeys list. Borrowed; never throws.
+  "array.hasNamedKey": { argTypes: [null, STRING], result: BOOL },
   // node:events EventEmitter: receivers are emitter-hierarchy objects and
   // the chaining forms (on/off/removeAll/setMax) return the receiver's
   // own class — program-dependent object types, checked in the libCall

packages/compiler/src/ir/validate.ts

@@ -4262,9 +4262,9 @@ function validateFunction(
       if (!ok) err(`libCall dyn.objEntryPairs must return a [string, any] tuple array`, e.loc);
       return true;
     }
-    if (e.fn === "array.namedKeys") {
+    if (e.fn === "array.namedKeys" || e.fn === "array.hasNamedKey") {
       if (e.args[0]?.type.kind !== "array") {
-        err(`libCall array.namedKeys arg 0: expected an array, got ${e.args[0]?.type.kind}`, e.loc);
+        err(`libCall ${e.fn} arg 0: expected an array, got ${e.args[0]?.type.kind}`, e.loc);
       }
       return true;
     }

packages/runtime/src/scr_array.c

@@ -4790,6 +4790,22 @@ ScrStr *scr_str_raw(ScrArr *raw, ScrArr *subs) {
   return out;
 }
 
+bool scr_arr_numeric_key(const char *key, size_t len, double *out) {
+  char text[64];
+  if (len == 0 || len >= sizeof text) return false;
+  memcpy(text, key, len);
+  text[len] = '\0';
+  char *end;
+  double number = strtod(text, &end);
+  if (end != text + len) return false;
+  char spelled[64];
+  if (scr_f64_to_str(number, spelled) != len || memcmp(spelled, key, len) != 0) return false;
+  size_t index;
+  if (scr_arr_valid_index(number, &index)) return false;
+  *out = number;
+  return true;
+}
+
 void scr_arr_copy_metadata(ScrArr *a, ScrDyn *target) {
   const ScrDyn *metadata = SCR_AX(a)->metadata;
   if (metadata && metadata->kind == SCR_DYN_HANDLE && metadata->v.handle.tag == SCR_DYNH_MATCH_SOURCE &&
@@ -4838,6 +4854,21 @@ ScrArr *scr_arr_named_keys(ScrArr *a) {
   return keys;
 }
 
+bool scr_arr_has_named_key(ScrArr *a, ScrStr *key) {
+  const ScrArrExt *x = scr_arr_ext_of(a);
+  if (!x) return false;
+  if (scr_arr_prop_index(a, key->data, key->len) != SIZE_MAX) return true;
+  const ScrDyn *metadata = x->metadata;
+  if (metadata && metadata->kind == SCR_DYN_HANDLE && metadata->v.handle.tag == SCR_DYNH_MATCH_SOURCE)
+    return true; /* index, input and groups, the only keys a pending result lists */
+  if (!metadata || metadata->kind != SCR_DYN_OBJ) return false;
+  for (size_t i = 0; i < metadata->v.obj.len; i++) {
+    const ScrDynEntry *entry = &metadata->v.obj.entries[i];
+    if (entry->key_len == key->len && memcmp(entry->key, key->data, key->len) == 0) return true;
+  }
+  return false;
+}
+
 void scr_arr_adopt_view_properties(ScrArr *a, ScrDyn *view, ScrArr *target) {
   ScrArrExt *from = scr_arr_ext_of(target);
   if (from && from->prop_len) {

packages/runtime/src/scr_array.h

@@ -322,6 +322,13 @@ void scr_arr_copy_metadata(ScrArr *a, struct ScrDyn *target);
  * (non-index) properties as its metadata and the noncanonical numeric
  * properties of `target`, so neither is lost when the storage is swapped. */
 void scr_arr_adopt_view_properties(ScrArr *a, struct ScrDyn *view, ScrArr *target);
+/* True when `key` is the string spelling of a number that is not an array
+ * index ("-1", "1.5", "NaN", "Infinity"), with the number in *out. Arrays
+ * keep such properties in their noncanonical property table. */
+bool scr_arr_numeric_key(const char *key, size_t len, double *out);
+/* Whether the array still has the named or noncanonical numeric property
+ * `key`: `for...in` skips a key deleted before its turn. Borrows both. */
+bool scr_arr_has_named_key(ScrArr *a, ScrStr *key);
 /* The enumerable string keys `for...in` visits after the indices: the
  * noncanonical numeric properties, then the named properties in insertion
  * order. +1. */

packages/runtime/src/scr_json.c

@@ -10270,9 +10270,17 @@ void scr_dyn_key_delete(ScrDyn *recv, const ScrStr *key, bool strict) {
     return;
   }
   /* A live native array keeps its named properties in its metadata, so
-   * deleting one goes through the view and its commit. Elements and
-   * `length` keep the boundary below. */
+   * deleting one goes through the view and its commit. Numeric properties
+   * are deleted natively. Elements and `length` keep the boundary below. */
   size_t element;
+  double number;
+  if (recv->kind == SCR_DYN_TYPED_REF && scr_dyn_typed_ref_array_ops(recv) &&
+      scr_arr_numeric_key(key->data, key->len, &number)) {
+    /* Noncanonical numeric properties ("-1", "1.5") live in the native
+     * array's property table. */
+    scr_arr_delete(recv->v.typed_ref.ptr, number);
+    return;
+  }
   if (recv->kind == SCR_DYN_TYPED_REF && recv->v.typed_ref.commit && scr_dyn_typed_ref_array_ops(recv) &&
       !scr_typed_ref_is_length(key) && !scr_typed_ref_canonical_index(key->data, key->len, &element)) {
     ScrDyn *view = scr_dyn_typed_ref_materialize(recv);

tests/corpus/6987-array-view-named-properties.ts

@@ -77,3 +77,20 @@ for (const key in shrinking) {
 
 const match = /b(?<rest>c)?/.exec("abc");
 if (match) for (const key in match) console.log("match key", key);
+
+// A numeric property deleted through a view is gone from the array.
+const numeric: number[] = [1, 2];
+numeric[-1] = 5;
+delete (numeric as unknown as Bag)["-1"];
+console.log(numeric[-1], "-1" in (numeric as unknown as Bag));
+for (const key in numeric) console.log("numeric key", key);
+
+// A named key deleted before its turn is not visited.
+const visited: number[] = [1];
+(visited as unknown as Bag).first = 1;
+(visited as unknown as Bag).second = 2;
+(visited as unknown as Bag).third = 3;
+for (const key in visited) {
+  console.log("visited key", key);
+  if (key === "first") delete (visited as unknown as Bag).second;
+}