fix(codegen): keep a string or symbol key off #7890's declared-array claim (#7891)

changelog.d/7891-declared-array-claim-string-key.md

@@ -0,0 +1,63 @@
+### fix(codegen): keep a string or symbol key off #7890's declared-array claim, and cover the shape #7890 added
+
+Two follow-ups to #7890, both found by writing the coverage #7890 was missing.
+
+#### A. A string/symbol key must not ride the claim (#7891)
+
+#7890 lets a property read whose receiver's *declared* property type is an array
+(`e.vals[i]`, `p.toks[p.pos]`) reach `expr/index_get.rs`'s array arm. That is a
+CLAIM, not a proof, and it was admitted on the grounds that the array arm
+re-checks `GC_TYPE_ARRAY` on the receiver and falls back.
+
+That is true of the arm **as a whole** and false of one route inside it. The two
+key routes have different receiver-validation strength:
+
+* **numeric** — `js_array_get_f64`, which classifies the receiver through
+  `clean_arr_ptr` / `array_object_receiver` and answers correctly for a string,
+  an array-like object, a typed array or a number.
+* **static string / symbol** — `js_array_get_index_or_string` →
+  `array_get_property_by_key` → `js_object_get_field_by_name`, which has **no
+  string-receiver index arm** and answers `undefined` for `s["0"]` where JS
+  answers the character.
+
+So only the numeric route is claim-safe. The claim now requires a non-string,
+non-symbol key; a string or symbol key keeps exactly the generic path it had
+before #7890. `interp`'s and `iso_miss`'s reads are all numeric, so the measured
+result is unchanged — every one of the 19 corpus binaries is byte-identical to
+the ones timed for #7890.
+
+The `undefined` answer itself is **pre-existing on `main`** and reachable without
+any of this, through a plain non-union declared receiver:
+
+```ts
+type Bag = { items: string[] };
+function mk(v: any): Bag { return { items: v }; }
+function viaDeclared(b: Bag): string { return "" + b.items["0"] + "/" + b.items[0]; }
+const s: any = "ss";
+console.log(viaDeclared(mk(s)));                 // node: s/s   perry: undefined/s
+const direct: any = "ss";
+console.log("" + direct["0"] + "/" + direct[0]); // node: s/s   perry: s/s
+```
+
+The same read on a bare `any` is correct, which is the tell: the wrong answer is
+selected by the ANNOTATION, not by the value. Tracked as **#7891**; not checked in
+as a gap test, because it would be red by construction and
+`test-parity/gap_snapshot.json` is generated on Linux and must not be hand-edited.
+
+#### B. Coverage for the shape #7890 actually added
+
+`test-files/test_gap_7890_declared_array_receiver_element_read.ts`. #7854's own
+test always routes through an intermediate local (`const items = e.items`), so
+nothing covered a `PropertyGet` used **directly** as the receiver — which is
+exactly what #7890 added. The new file reads `e.items[i]` / `e.items.length`
+through a `type` alias, an `interface`, a class, a nullable reassigned cursor and
+a nested chain, handed an array, a string, a number, an array-like object with
+numeric and with non-numeric `length`, a typed array, a function, `null` and
+`undefined`, plus negative / fractional / out-of-range indexes, a store through
+the same shape, and static string keys (which A leaves on the generic path).
+Byte-identical to node on every row.
+
+Live rather than decorative: on that file the guarded-read `arr.fast` blocks go
+**11 → 15** and the `js_dyn_index_get` calls go **5 → 1**.
+
+Writing it is what found #7891.

crates/perry-codegen/src/expr/index_get.rs

@@ -1126,6 +1126,17 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
                 recv_ty,
                 None | Some(perry_hir::types::Type::Any) | Some(perry_hir::types::Type::Unknown)
             );
+            // #5525: route every non-static-string/symbol read on an unknown
+            // receiver through `js_dyn_index_get` (numeric, runtime-string, and
+            // runtime-symbol are all triaged in the runtime). The earlier
+            // `is_numeric_expr(index)` gate missed `lr[off]`/`lr[off + 1]`
+            // (bcryptjs `_encipher`'s `off` is an `any` param, so `off + 1` is
+            // not provably numeric); statically-known string-literal / symbol
+            // keys keep their dedicated interned-handle / symbol routes below.
+            let index_is_static_string_or_symbol = matches!(
+                index.as_ref(),
+                Expr::String(_) | Expr::WtfString(_) | Expr::SymbolFor(_)
+            ) || is_string_expr(ctx, index);
             // #7854 recovered a receiver's declared array type for a LOCAL
             // (`const names = e.names`), never for the read used directly as a
             // receiver (`e.vals[i]`, `p.toks[p.pos]`) — the HIR types a
@@ -1141,20 +1152,22 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
             // fallback. A violated claim costs a branch, not an answer. (#6132
             // records that the same guard is what makes a typed-array-valued
             // member receiver safe on this path.)
-            let claimed_array =
-                recv_unknown && crate::type_analysis::declared_array_property_claim(ctx, object);
+            //
+            // Restricted to a NON-string, NON-symbol key, and that restriction is
+            // load-bearing rather than tidy. The string-key arm of the array
+            // branch is `js_array_get_index_or_string`, whose string half calls
+            // `js_object_get_field_by_name` on the receiver — and that answers
+            // `undefined` for `s["0"]` on a heap STRING receiver, where JS
+            // answers the character. That is a pre-existing wrong answer on
+            // `main` — reachable today through a plain non-union declared
+            // receiver, filed as #7891 with a minimal repro — and a claim must
+            // not widen the set of shapes that reach it. With
+            // the restriction, a string or symbol key takes exactly the generic
+            // path it takes today; only the numeric read moves.
+            let claimed_array = recv_unknown
+                && !index_is_static_string_or_symbol
+                && crate::type_analysis::declared_array_property_claim(ctx, object);
             let recv_unknown = recv_unknown && !claimed_array;
-            // #5525: route every non-static-string/symbol read on an unknown
-            // receiver through `js_dyn_index_get` (numeric, runtime-string, and
-            // runtime-symbol are all triaged in the runtime). The earlier
-            // `is_numeric_expr(index)` gate missed `lr[off]`/`lr[off + 1]`
-            // (bcryptjs `_encipher`'s `off` is an `any` param, so `off + 1` is
-            // not provably numeric); statically-known string-literal / symbol
-            // keys keep their dedicated interned-handle / symbol routes below.
-            let index_is_static_string_or_symbol = matches!(
-                index.as_ref(),
-                Expr::String(_) | Expr::WtfString(_) | Expr::SymbolFor(_)
-            ) || is_string_expr(ctx, index);
             if recv_unknown && !index_is_static_string_or_symbol {
                 // #7640 section B: receiver live across an unconstrained index.
                 return rooting::with_operands_rooted(ctx, &[object, index], |ctx, vals| {

test-files/test_gap_7890_declared_array_receiver_element_read.ts

@@ -162,3 +162,15 @@ function scan(b: Bag, needle: string): string {
 console.log(scan(mkAlias(["a", "b"]), "a"));
 console.log(scan(mkAlias([1, 2, 3] as any), "a"));
 console.log(scan(mkAlias({ length: 3 }), "a"));
+
+// A STRING-literal key on the same receiver shape is deliberately NOT admitted
+// to the array arm by #7890 — see `index_get.rs`. It stays on the generic path,
+// which is what these rows pin.
+function stringKey(b: Bag): string {
+  return (
+    "" + b.items["length"] + "/" + b.items["nope"] + "/" + typeof b.items["constructor"]
+  );
+}
+console.log(stringKey(mkAlias(["k"])));
+console.log(stringKey(mkAlias({ length: 2, 0: "obj0", nope: "here" })));
+console.log(stringKey(mkAlias(9)));