fix(runtime): inherit Array-subclass fill

changelog.d/8987-array-subclass-enumeration.md

@@ -0,0 +1,5 @@
+### Fixed
+
+- Array-subclass enumeration now matches Node: `Object.keys` and `for...in`
+  report only enumerable indices, while `Object.getOwnPropertyNames` also
+  reports `length`; inherited `Array.prototype.fill` no longer leaks as an own key.

crates/perry-runtime/src/array/subclass.rs

@@ -543,11 +543,11 @@ pub(crate) unsafe fn array_subclass_named_prefix_token_for_slot(
         }
     }
 
-    // `js_array_subclass_init` installs two canonical own properties that are
-    // absent from most class allocation shapes: `length` and the generic
-    // `fill` method. If a class declared either name, init overwrites its
-    // existing slot; otherwise the exact missing names must follow the
-    // declared prefix in that order. Anything else is instance-specific.
+    // The legacy shape-carried representation installs `length` and its
+    // compatibility `fill` closure after the declared prefix. The default
+    // elements-backed representation inherits `fill` from `Array.prototype`
+    // and has no runtime names in its shape. Anything else is
+    // instance-specific.
     let declared_count = declared_count as u32;
     let mut expected_runtime_names: [&[u8]; 2] = [&[]; 2];
     let mut expected_runtime_count = 0usize;
@@ -568,7 +568,7 @@ pub(crate) unsafe fn array_subclass_named_prefix_token_for_slot(
         // elements store (the store owns `length`); keep it off this token.
         return 0;
     }
-    if !declared_fill {
+    if !elements_backed && !declared_fill {
         expected_runtime_names[expected_runtime_count] = b"fill";
         expected_runtime_count += 1;
     }

crates/perry-runtime/src/node_stream_constructors/builders.rs

@@ -162,15 +162,11 @@ pub extern "C" fn js_event_emitter_async_resource_subclass_init(this: f64, optio
 /// `super(n)` for a source-compiled `class X extends Array` (e.g. lru-cache's
 /// `ZeroArray`: `class ZeroArray extends Array { constructor(n){ super(n);
 /// this.fill(0) } }`). Perry models the subclass instance as a plain object,
-/// not a real exotic Array, so `super(n)` otherwise left it length-less with no
-/// Array methods. Size it (`length = ToLength(n)`, a visible own property the
-/// generic array-like helpers read) and install the Array surface the instance
-/// relies on — currently `fill`, which delegates to `js_array_fill_generic`
-/// (it operates on the receiver's own `length` + indexed properties, exactly
-/// what an array-like object exposes). Indexed get/set already work as ordinary
-/// object properties. Mirrors `js_event_emitter_subclass_init` (#5494); the
-/// codegen `super()` lowering for an `Array` parent calls this. Additional
-/// Array methods can be added to `array_subclass_methods` as bundles need them.
+/// not a real exotic Array, so `super(n)` initializes its elements store. In
+/// the default representation, inherited methods resolve through
+/// `Array.prototype` and are not stamped as enumerable own properties. The
+/// legacy shape-carried kill switch retains its old compatibility closure.
+/// The codegen `super()` lowering calls this entry point.
 #[no_mangle]
 pub extern "C" fn js_array_subclass_init(this: f64, n: f64) -> f64 {
     let raw = raw_ptr_from_value(this);
@@ -202,18 +198,6 @@ pub extern "C" fn js_array_subclass_init(this: f64, n: f64) -> f64 {
         unsafe {
             crate::array::subclass_elements::install_elements(obj, len.min(u32::MAX as f64) as u32)
         };
-        // The Array surface the instance relies on, installed exactly as in
-        // the shape-carried form. It must NOT be hidden behind a property
-        // descriptor: that sets `OBJ_FLAG_HAS_DESCRIPTORS` on every instance,
-        // which the codegen class-field inline guard rejects — every field
-        // read then takes the IC miss (measured: 6x on the wolf-ecs twins).
-        // `fill` showing up in `getOwnPropertyNames` is the pre-existing
-        // divergence tracked in #8953, unchanged by the elements store.
-        let this = this_root.get_nanbox_f64();
-        let obj = raw_ptr_from_value(this) as *mut ObjectHeader;
-        crate::closure::js_register_closure_arity(ns_array_fill as *const u8, 3);
-        let methods: [(&str, StubFn); 1] = [("fill", super::cast3(ns_array_fill))];
-        install_methods_on_existing_object(obj, this, &methods, &[]);
         return this_root.get_nanbox_f64();
     }
     let length_key = crate::string::js_string_from_bytes(b"length".as_ptr(), 6);
@@ -258,9 +242,7 @@ pub unsafe extern "C" fn js_array_subclass_init_args(
     this.get_nanbox_f64()
 }
 
-/// `Array.prototype.fill`-equivalent installed on an Array-subclass instance:
-/// fills the receiver's own indexed slots `0..length` with `value`. Delegates
-/// to the generic array-like fill (which reads `length` off the receiver).
+/// Legacy shape-carried compatibility closure for `Array.prototype.fill`.
 pub(super) extern "C" fn ns_array_fill(
     closure: *const ClosureHeader,
     value: f64,

crates/perry-runtime/src/object/field_get_set/accessors.rs

@@ -599,6 +599,29 @@ pub(crate) unsafe fn string_index_value(
     }
 }
 
+/// Resolve an inherited `Array.prototype` property for an Array-subclass
+/// instance after its own fields and class-declared methods have missed.
+/// An explicit per-instance prototype replaces the ordinary class chain and
+/// therefore suppresses this implicit fallback.
+pub(crate) unsafe fn array_subclass_prototype_field(
+    obj: *const ObjectHeader,
+    key: *const crate::StringHeader,
+) -> Option<JSValue> {
+    if obj.is_null()
+        || key.is_null()
+        || super::super::prototype_chain::object_static_prototype(obj as usize).is_some()
+        || !crate::array::is_array_subclass_class_id((*obj).class_id)
+    {
+        return None;
+    }
+    let key_ptr = crate::object::string_header_payload(key);
+    let key_len = (*key).byte_len as usize;
+    let name = std::str::from_utf8(std::slice::from_raw_parts(key_ptr, key_len)).ok()?;
+    // `array_prototype_property_value` copies `name` before its first
+    // allocation and roots the receiver across the prototype lookup.
+    array_prototype_property_value(name, obj as usize)
+}
+
 pub(crate) unsafe fn array_prototype_property_value(
     name: &str,
     receiver_addr: usize,
@@ -625,6 +648,8 @@ pub(crate) unsafe fn array_prototype_property_value(
     let name_copy = super::HeapKeyBytes::copy_of(name.as_bytes());
     let name: &str = std::str::from_utf8_unchecked(name_copy.as_bytes());
 
+    let scope = crate::gc::RuntimeHandleScope::new();
+    let receiver_h = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(receiver_addr as i64));
     let ctor = super::super::js_get_global_this_builtin_value(b"Array".as_ptr(), 5);
     let ctor_value = JSValue::from_bits(ctor.to_bits());
     if !ctor_value.is_pointer() {
@@ -636,15 +661,11 @@ pub(crate) unsafe fn array_prototype_property_value(
     if !proto_value.is_pointer() {
         return None;
     }
-    // #7498: `js_string_from_bytes` ALLOCATES, so `Array.prototype` and the
-    // receiver cannot be carried across it as bare `usize`s — and the key it
-    // produces is itself a fresh heap string this function then hands to two
-    // more calls that can collect (`js_object_get_field_by_name` runs getters;
-    // `default_object_prototype_property_value` interns another key). Root all
-    // three and read each back at its point of use.
-    let scope = crate::gc::RuntimeHandleScope::new();
+    // #7498: the receiver is rooted before the allocating global lookup above;
+    // `Array.prototype` and the fresh key are rooted before the calls below,
+    // which can collect (`js_object_get_field_by_name` runs getters and
+    // `default_object_prototype_property_value` interns another key).
     let proto_h = scope.root_nanbox_f64(proto);
-    let receiver_h = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(receiver_addr as i64));
     let key_h = scope.root_nanbox_f64(crate::value::nanbox_string_key(
         crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32),
     ));

crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs

@@ -1,12 +1,9 @@
-//! Object-deref tail of `js_object_get_field_by_name`: pointer-strip,
-//! handle dispatch, and the full ObjectHeader property walk. Extracted
-//! verbatim from field_get_set.rs (issue #1103 split) so neither half
-//! exceeds the file-size budget. Pure relocation — no logic change.
+//! Object-deref tail of `js_object_get_field_by_name`: pointer stripping,
+//! handle dispatch, and the full ObjectHeader property walk (#1103 split).
 
 use super::*;
 
-/// Tail of `js_object_get_field_by_name` (everything after the leading
-/// primitive/handle/Date receiver guards). Body moved verbatim.
+/// Object-deref tail of `js_object_get_field_by_name`.
 pub(crate) fn get_field_by_name_object_tail(
     obj: *const ObjectHeader,
     key: *const crate::StringHeader,
@@ -1470,6 +1467,9 @@ pub(crate) fn get_field_by_name_object_tail(
                 {
                     return v;
                 }
+                if let Some(v) = super::accessors::array_subclass_prototype_field(obj, key) {
+                    return v;
+                }
                 if let Some(v) = ordinary_object_prototype_property_value(obj, key) {
                     return v;
                 }
@@ -1873,6 +1873,9 @@ pub(crate) fn get_field_by_name_object_tail(
             {
                 return v;
             }
+            if let Some(v) = super::accessors::array_subclass_prototype_field(obj, key) {
+                return v;
+            }
             if let Some(v) = ordinary_object_prototype_property_value(obj, key) {
                 return v;
             }

crates/perry-runtime/src/object/native_call_method.rs

@@ -2102,19 +2102,38 @@ pub unsafe extern "C-unwind" fn js_native_call_method(
         let method_key =
             crate::string::js_string_from_bytes(method_name.as_ptr(), method_name.len() as u32);
         if !method_key.is_null() {
-            let inherited = super::prototype_chain::resolve_inherited_field(
-                obj as usize,
-                method_key,
-            )
-            .or_else(|| unsafe {
-                // A plain object's implicit Object.prototype is not stored in
-                // the recorded-prototype table. Property reads already use
-                // this guarded fallback, so direct `obj.method()` dispatch
-                // must consult it too (including user-added methods such as a
-                // borrowed Array.prototype.join). The helper rejects arrays,
-                // exotic/null-prototype objects, and explicit overrides.
-                super::field_get_set::ordinary_object_prototype_property_value(obj, method_key)
-            });
+            let inherited =
+                super::prototype_chain::resolve_inherited_field(obj as usize, method_key)
+                    .or_else(|| unsafe {
+                        // A plain object's implicit Object.prototype is not stored in
+                        // the recorded-prototype table. Property reads already use
+                        // this guarded fallback, so direct `obj.method()` dispatch
+                        // must consult it too (including user-added methods such as a
+                        // borrowed Array.prototype.join). The helper rejects arrays,
+                        // exotic/null-prototype objects, and explicit overrides.
+                        super::field_get_set::ordinary_object_prototype_property_value(
+                            obj, method_key,
+                        )
+                    })
+                    .or_else(|| unsafe {
+                        // Elements-backed Array-subclass instances inherit `fill`
+                        // instead of carrying a bound enumerable own closure (#8953).
+                        // A class method or explicit per-instance prototype wins; only
+                        // the ordinary class chain reaches Array.prototype here.
+                        let class_id = (*obj).class_id;
+                        if method_name != "fill"
+                            || super::prototype_chain::object_static_prototype(obj as usize)
+                                .is_some()
+                            || !crate::array::is_array_subclass_class_id(class_id)
+                            || lookup_class_method_in_chain(class_id, method_name).is_some()
+                        {
+                            return None;
+                        }
+                        super::field_get_set::array_prototype_property_value(
+                            method_name,
+                            obj as usize,
+                        )
+                    });
             if let Some(field_val) = inherited {
                 if !field_val.is_undefined() && !field_val.is_null() {
                     let bound = crate::closure::clone_closure_rebind_this(

crates/perry/tests/issue_8953_array_subclass_enumeration.rs

@@ -0,0 +1,84 @@
+//! #8953: Array-subclass instances use an elements store for indices and
+//! `length`, while inherited Array methods stay off the instance shape.
+use std::path::PathBuf;
+use std::process::{Command, Output};
+
+fn perry_bin() -> PathBuf {
+    PathBuf::from(env!("CARGO_BIN_EXE_perry"))
+}
+
+fn compile_and_run(source: &str) -> Output {
+    let dir = tempfile::tempdir().expect("tempdir");
+    let entry = dir.path().join("main.js");
+    let output = dir.path().join("main_bin");
+    std::fs::write(&entry, source).expect("write entry");
+    let compile = Command::new(perry_bin())
+        .current_dir(dir.path())
+        .arg("compile")
+        .arg(&entry)
+        .arg("-o")
+        .arg(&output)
+        .arg("--no-auto-optimize")
+        .output()
+        .expect("run perry compile");
+    assert!(
+        compile.status.success(),
+        "perry compile failed\nstdout:\n{}\nstderr:\n{}",
+        String::from_utf8_lossy(&compile.stdout),
+        String::from_utf8_lossy(&compile.stderr)
+    );
+    Command::new(&output).output().expect("run compiled binary")
+}
+
+#[test]
+fn array_subclass_enumeration_matches_node_and_fill_is_inherited() {
+    let run = compile_and_run(
+        r#"
+class A extends Array {}
+
+const empty = new A();
+console.log("empty keys:", Object.keys(empty).join(","));
+let emptyForIn = [];
+for (const key in empty) emptyForIn.push(key);
+console.log("empty for-in:", emptyForIn.join(","));
+console.log("empty names:", Object.getOwnPropertyNames(empty).join(","));
+console.log("fill:", Object.hasOwn(empty, "fill"), typeof empty.fill);
+
+const a = new A();
+a.push(1, 2, 3);
+console.log("keys:", Object.keys(a).join(","));
+let keys = [];
+for (const key in a) keys.push(key);
+console.log("for-in:", keys.join(","));
+console.log("names:", Object.getOwnPropertyNames(a).join(","));
+a.fill(7, 1);
+console.log("filled:", a.join(","));
+const inheritedFill = a.fill;
+inheritedFill.call(a, 9, 2);
+console.log("extracted fill:", a.join(","));
+
+class B extends Array { fill(value) { return "override:" + value; } }
+const b = new B();
+console.log("override:", Object.hasOwn(b, "fill"), b.fill(5));
+"#,
+    );
+    assert!(
+        run.status.success(),
+        "the #8953 fixture must not crash\nstdout:\n{}\nstderr:\n{}",
+        String::from_utf8_lossy(&run.stdout),
+        String::from_utf8_lossy(&run.stderr)
+    );
+    assert_eq!(
+        String::from_utf8_lossy(&run.stdout),
+        "empty keys: \n\
+empty for-in: \n\
+empty names: length\n\
+fill: false function\n\
+keys: 0,1,2\n\
+for-in: 0,1,2\n\
+names: 0,1,2,length\n\
+filled: 1,7,7\n\
+extracted fill: 1,7,9\n\
+override: false override:5\n"
+    );
+}