perf(codegen): interp 0.780 -> 0.675 s, iso_miss 1.061 -> 0.967 s — let a declared array type reach the guarded element read and the inline .length

changelog.d/7890-declared-array-claim-element-reads.md

@@ -0,0 +1,60 @@
+### perf(codegen): a declared array type reaches the guarded element read, and `.length` stops refusing one
+
+Round 7 of the `interp` campaign. Two changes in the same mechanism — what a
+program is allowed to do with an array type it got from an *annotation* rather
+than from an initializer that proved an array.
+
+#### A. `e.vals[i]` / `p.toks[p.pos]` — a property read used directly as a receiver
+
+#7854 taught `refine_type_from_init` to recover a receiver's declared property
+type for a **local** (`const names = e.names` on `type Env = { names: string[] }`),
+which is why `names[i]` is an inline element read today. It did nothing for the
+same read used **directly as the receiver** — `e.vals[i]`, `p.toks[p.pos]` —
+because the HIR types a `PropertyGet` off a UNION receiver as `Any`
+(`perry-hir/src/analysis/value_types.rs`, the `Union` arm), so `static_type_of`
+answers `Any` and `expr/index_get.rs` routes the read to the unknown-receiver
+dispatcher `js_dyn_index_get`.
+
+`declared_array_property_claim` answers the question for that shape, and
+`index_get.rs` consumes it in exactly two places: it suppresses the
+`recv_unknown` route, and it admits the receiver to the array arm. The tier this
+unlocks is `lower_guarded_array_index_get`, which re-checks `GC_TYPE_ARRAY`, the
+forwarding flag, per-array descriptors, the prototype latch and the bounds **on
+the receiver itself**, and routes every failure to
+`js_typed_feedback_array_index_get_fallback_boxed`. So a violated claim costs a
+predicted branch and returns the same answer — the deal #7854 already records
+for element reads, and the same guard #6132 relies on to make a
+typed-array-valued member receiver safe on this path.
+
+Measured share on `gc-handoff/apps/interp.ts` before the change (xctrace time
+profile, `PERRY_DEBUG_SYMBOLS=1` build): `js_dyn_index_get` 5.0%,
+`js_array_length` 4.6% — the latter reached from `js_dyn_index_get`'s and the
+IC-miss handler's `.length` short-circuit.
+
+#### B. `.length` no longer refuses a declared-only array local
+
+#7854 recorded these locals in `FnCtx::declared_only_array_locals` and had the
+inline `.length` arm refuse them. The reason was specific and correct at the
+time: the arm's inline half was guarded, but its FALLBACK was
+`js_value_length_f64`, which answered **0** for every value that carries no
+length where JS answers `undefined`, and continued instead of throwing for a
+nullish receiver (#7853).
+
+**#7862 replaced that fallback with `js_value_length_property_f64`** — ordinary
+property semantics: `undefined` for a missing property, the real value for a
+non-numeric one, normal object / function / native / proxy dispatch, and a
+catchable `TypeError` for a nullish receiver. It did not lift the refusal that
+existed only because of the old fallback. This lifts it, and deletes the set and
+its classifier with it: a mode that no longer gates anything is not a decision
+that has been made.
+
+`declared_only_numeric_locals` (#7773) is untouched and stays — its consumer is
+an arithmetic operator with no guarded fallback, which is a different situation.
+
+The sabotage is pre-existing and now runs the inline arm instead of the generic
+tower: `test-files/test_gap_7853_declared_array_length_runtime_value.ts` and
+`test-files/test_gap_declared_field_type_refine_guarded.ts` feed a
+`string[]`-declared local an array, a string, a number, an array-like object
+with a numeric `length`, an array-like object with a *non-numeric* `length`, a
+function, a typed array, `null` and `undefined`, through an alias, an interface
+and a class, and require node-identical output on every row.

crates/perry-codegen/src/codegen/closure.rs

@@ -979,7 +979,6 @@ pub(super) fn compile_closure(
         shadow_slot_map,
         persistent_shadow_slots: std::collections::HashSet::new(),
         declared_only_numeric_locals: std::collections::HashSet::new(),
-        declared_only_array_locals: std::collections::HashSet::new(),
         shadow_slot_clears_after_stmt,
         arena_state_slot: None,
         class_keys_slots: HashMap::new(),

crates/perry-codegen/src/codegen/entry.rs

@@ -817,7 +817,6 @@ pub(super) fn compile_module_entry(
             shadow_slot_map: main_shadow_slot_map,
             persistent_shadow_slots: std::collections::HashSet::new(),
             declared_only_numeric_locals: std::collections::HashSet::new(),
-            declared_only_array_locals: std::collections::HashSet::new(),
             shadow_slot_clears_after_stmt: main_shadow_slot_clears_after_stmt,
             arena_state_slot: None,
             class_keys_slots: HashMap::new(),
@@ -1488,7 +1487,6 @@ pub(super) fn compile_module_entry(
             shadow_slot_map: init_shadow_slot_map,
             persistent_shadow_slots: std::collections::HashSet::new(),
             declared_only_numeric_locals: std::collections::HashSet::new(),
-            declared_only_array_locals: std::collections::HashSet::new(),
             shadow_slot_clears_after_stmt: init_shadow_slot_clears_after_stmt,
             arena_state_slot: None,
             class_keys_slots: HashMap::new(),

crates/perry-codegen/src/codegen/function.rs

@@ -783,7 +783,6 @@ pub(super) fn compile_function(
         shadow_slot_map,
         persistent_shadow_slots: std::collections::HashSet::new(),
         declared_only_numeric_locals: std::collections::HashSet::new(),
-        declared_only_array_locals: std::collections::HashSet::new(),
         shadow_slot_clears_after_stmt,
         shadow_slots_bound: bound_param_slots,
         temp_roots: crate::rooting::TempRootPool::default(),

crates/perry-codegen/src/codegen/method.rs

@@ -510,7 +510,6 @@ pub(super) fn compile_method(
         shadow_slot_map,
         persistent_shadow_slots: std::collections::HashSet::new(),
         declared_only_numeric_locals: std::collections::HashSet::new(),
-        declared_only_array_locals: std::collections::HashSet::new(),
         shadow_slot_clears_after_stmt,
         arena_state_slot: None,
         class_keys_slots: HashMap::new(),
@@ -1575,7 +1574,6 @@ pub(super) fn compile_static_method(
         shadow_slot_map,
         persistent_shadow_slots: std::collections::HashSet::new(),
         declared_only_numeric_locals: std::collections::HashSet::new(),
-        declared_only_array_locals: std::collections::HashSet::new(),
         shadow_slot_clears_after_stmt,
         arena_state_slot: None,
         class_keys_slots: HashMap::new(),

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

@@ -1126,6 +1126,24 @@ 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)
             );
+            // #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
+            // `PropertyGet` off a UNION receiver as `Any`, so those land in the
+            // `recv_unknown` arm below and pay `js_dyn_index_get` plus the
+            // `js_array_length` its miss path calls.
+            //
+            // A declared property type is a CLAIM. It is admissible here and
+            // only here because the tier this unlocks —
+            // `lower_guarded_array_index_get` — re-checks `GC_TYPE_ARRAY`, the
+            // forwarding flag, descriptors, the prototype latch and the bounds
+            // on the receiver itself and routes every failure to the boxed
+            // 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);
+            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
@@ -1157,7 +1175,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
             //      generic object field access via js_object_get_field_by_name_f64
             //   3. Anything else → fall back to dynamic object field
             //      access by stringifying the index at runtime
-            if is_array_expr(ctx, object) {
+            if is_array_expr(ctx, object) || claimed_array {
                 // #321: a symbol-keyed array read (`arr[Symbol.iterator]`) must
                 // NOT take the numeric fast path below — `fptosi` on the symbol
                 // value yields a garbage index (returned a number). Route symbol

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

@@ -768,29 +768,6 @@ pub(crate) struct FnCtx<'a> {
     /// the trust into a four-instruction runtime tag test instead.
     pub declared_only_numeric_locals: std::collections::HashSet<u32>,
 
-    /// LocalIds whose `Array`/`String` type came from
-    /// `refine.rs::declared_property_type_from_annotation` — i.e. from the
-    /// RECEIVER'S ANNOTATION (`type Env = { names: string[] }`), not from an
-    /// initializer that proves an array (`[...]`, `.split()`, `Object.keys()`).
-    ///
-    /// Twin of `declared_only_numeric_locals` (#7773) and there for the same
-    /// reason: the refinement is load-bearing — without it `const names =
-    /// e.names` stays `Any` and every `names[i]` is a `js_dyn_index_get` call —
-    /// but it copies an annotation rather than proving anything, and Perry does
-    /// not enforce annotations at runtime.
-    ///
-    /// Element reads and stores are safe on a claim: they re-check
-    /// `GC_TYPE_ARRAY` on the receiver and fall back. `.length` is NOT: its
-    /// slow path (`js_value_length_f64`) answers **0** for every value that
-    /// carries no length, where JS answers `undefined` (and throws on
-    /// nullish) — a documented, pre-existing degradation
-    /// (`value/dynamic_object.rs`, "the generic PropertyGet slow path already
-    /// degrades to 0 here"). Feeding it a claim would widen a silent wrong
-    /// answer, so the `.length` arm in `expr/property_get.rs` refuses these
-    /// ids and lets them take the generic property path — exactly what the
-    /// unrefined `Any` local does today.
-    pub declared_only_array_locals: std::collections::HashSet<u32>,
-
     /// Cached pointer to this function's `InlineArenaState` slot —
     /// allocated lazily on the first `new ClassName()` site that uses
     /// the inline bump-allocator path. The slot lives in the function

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

@@ -294,15 +294,30 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
         Expr::PropertyGet {
             object, property, ..
         } if property == "length"
-            // #7854: a receiver whose Array/String type is a copied ANNOTATION
-            // rather than a proof must not reach this arm. The inline half is
-            // guarded, but the `js_value_length_f64` fallback answers 0 where
-            // JS answers `undefined` (and where a nullish receiver must throw),
-            // so a violated claim becomes a silent wrong answer instead of a
-            // slower path. These ids take the generic property route — exactly
-            // what the same local took before it was refined at all.
-            // See `FnCtx::declared_only_array_locals`.
-            && !matches!(object.as_ref(), Expr::LocalGet(id) if ctx.declared_only_array_locals.contains(id))
+            // #7854 recorded a receiver whose Array/String type is a copied
+            // ANNOTATION rather than a proof in `FnCtx::declared_only_array_locals`
+            // and refused it here, because the inline half was guarded but the
+            // fallback — `js_value_length_f64` — answered **0** for every value
+            // that carries no length where JS answers `undefined`, and continued
+            // instead of throwing for a nullish receiver. A violated claim was
+            // therefore a silent wrong answer rather than a slower path.
+            //
+            // #7862 replaced that fallback with `js_value_length_property_f64`
+            // (the slow arm below, and the string-lowering arm above), which
+            // *is* ordinary property semantics: `undefined` for a missing
+            // property, the real value for a non-numeric one, normal
+            // object/function/native/proxy dispatch, and a catchable TypeError
+            // for a nullish receiver. The reason for the refusal is gone, so the
+            // refusal is too — a claim now costs a guard branch and nothing else,
+            // which is exactly the deal element reads have always taken.
+            //
+            // `test-files/test_gap_7853_declared_array_length_runtime_value.ts`
+            // and `test_gap_declared_field_type_refine_guarded.ts` are the
+            // sabotage: both feed a `string[]`-declared local an array, a
+            // string, a number, an array-like object with numeric and
+            // non-numeric `length`, a function, a typed array, `null` and
+            // `undefined`, and require node-identical output. They run the
+            // inline arm now instead of the generic tower.
             && (is_array_expr(ctx, object)
                 || is_string_expr(ctx, object)
                 || match crate::type_analysis::static_type_of(ctx, object) {

crates/perry-codegen/src/stmt/let_stmt.rs

@@ -305,21 +305,13 @@ pub(crate) fn lower_let(
         }
     }
 
-    // Same discipline for the array/string half. When the refinement above
-    // answered `Array`/`String` only because the RECEIVER'S ANNOTATION said so
-    // (`const names = e.names` on `type Env = { names: string[] }`), record the
-    // id: element reads re-check `GC_TYPE_ARRAY` and are safe on a claim, but
-    // the `.length` fast arm's fallback answers 0 where JS answers `undefined`,
-    // so it must not consume one. See `FnCtx::declared_only_array_locals`.
-    if matches!(
-        refined_ty,
-        perry_hir::types::Type::Array(_) | perry_hir::types::Type::String
-    ) && init.is_some_and(|e| {
-        matches!(e, perry_hir::Expr::PropertyGet { .. })
-            && crate::type_analysis::refined_array_type_is_declared_only(ctx, e)
-    }) {
-        ctx.declared_only_array_locals.insert(id);
-    }
+    // (#7854 also recorded the array/string half of this — a local whose
+    // `Array`/`String` type came only from the RECEIVER'S ANNOTATION — in
+    // `declared_only_array_locals`, so the `.length` fast arm could refuse it.
+    // #7862 gave that arm a property-semantic fallback, which is what the
+    // refusal existed to avoid, so both the set and its one consumer are gone;
+    // see the `.length` arm in `expr/property_get.rs`. The NUMERIC half above
+    // stays: its consumer is an arithmetic op with no guarded fallback.)
 
     // Track closure func_id → local_id mapping so the closure
     // call site in lower_call can look up rest param info.

crates/perry-codegen/src/type_analysis.rs

@@ -52,9 +52,8 @@ pub(crate) use predicates::{
 #[cfg(test)]
 pub(crate) use predicates::tuple_index_literal;
 pub(crate) use refine::{
-    compute_auto_captures, is_crypto_digest_chain, is_global_constructor_expr,
-    is_process_namespace_version_property, refine_type_from_init,
-    refined_array_type_is_declared_only,
+    compute_auto_captures, declared_array_property_claim, is_crypto_digest_chain,
+    is_global_constructor_expr, is_process_namespace_version_property, refine_type_from_init,
 };
 pub(crate) use strings::{
     class_name_extends_url_search_params, is_declared_string_expr, is_definitely_string_expr,