From 71bc7c9a886b8267fb530a3d16d19b69ed6f334f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 28 Aug 2026 22:04:23 +0000 Subject: [PATCH 1/2] fix(runtime): inherit Array-subclass fill (#8953) --- .../8953-array-subclass-enumeration.md | 5 ++ crates/perry-runtime/src/array/subclass.rs | 12 +-- .../src/node_stream_constructors/builders.rs | 30 ++----- .../src/object/field_get_set/accessors.rs | 37 ++++++-- .../field_get_set/get_field_by_name_tail.rs | 15 ++-- .../src/object/native_call_method.rs | 45 +++++++--- .../issue_8953_array_subclass_enumeration.rs | 84 +++++++++++++++++++ 7 files changed, 171 insertions(+), 57 deletions(-) create mode 100644 changelog.d/8953-array-subclass-enumeration.md create mode 100644 crates/perry/tests/issue_8953_array_subclass_enumeration.rs diff --git a/changelog.d/8953-array-subclass-enumeration.md b/changelog.d/8953-array-subclass-enumeration.md new file mode 100644 index 0000000000..c8cb62a190 --- /dev/null +++ b/changelog.d/8953-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. diff --git a/crates/perry-runtime/src/array/subclass.rs b/crates/perry-runtime/src/array/subclass.rs index 094f174442..3bbd35361a 100644 --- a/crates/perry-runtime/src/array/subclass.rs +++ b/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; } diff --git a/crates/perry-runtime/src/node_stream_constructors/builders.rs b/crates/perry-runtime/src/node_stream_constructors/builders.rs index c1b8956409..abfbef223e 100644 --- a/crates/perry-runtime/src/node_stream_constructors/builders.rs +++ b/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, diff --git a/crates/perry-runtime/src/object/field_get_set/accessors.rs b/crates/perry-runtime/src/object/field_get_set/accessors.rs index 46b190cc6d..ea3309b172 100644 --- a/crates/perry-runtime/src/object/field_get_set/accessors.rs +++ b/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 { + 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 = (key as *const u8).add(std::mem::size_of::()); + 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), )); diff --git a/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs b/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs index f56ff584e1..4b2605db38 100644 --- a/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs +++ b/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; } diff --git a/crates/perry-runtime/src/object/native_call_method.rs b/crates/perry-runtime/src/object/native_call_method.rs index 06a5393925..eb038fb691 100644 --- a/crates/perry-runtime/src/object/native_call_method.rs +++ b/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( diff --git a/crates/perry/tests/issue_8953_array_subclass_enumeration.rs b/crates/perry/tests/issue_8953_array_subclass_enumeration.rs new file mode 100644 index 0000000000..5ce97c7fcc --- /dev/null +++ b/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" + ); +} From 427a216295177c9730fc2b76c5a467ad48fba03c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 29 Aug 2026 00:23:04 +0200 Subject: [PATCH 2/2] chore: PR-key the fragment; reuse the shared StringHeader payload helper --- ...bclass-enumeration.md => 8987-array-subclass-enumeration.md} | 0 crates/perry-runtime/src/object/field_get_set/accessors.rs | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename changelog.d/{8953-array-subclass-enumeration.md => 8987-array-subclass-enumeration.md} (100%) diff --git a/changelog.d/8953-array-subclass-enumeration.md b/changelog.d/8987-array-subclass-enumeration.md similarity index 100% rename from changelog.d/8953-array-subclass-enumeration.md rename to changelog.d/8987-array-subclass-enumeration.md diff --git a/crates/perry-runtime/src/object/field_get_set/accessors.rs b/crates/perry-runtime/src/object/field_get_set/accessors.rs index ea3309b172..738cfed07b 100644 --- a/crates/perry-runtime/src/object/field_get_set/accessors.rs +++ b/crates/perry-runtime/src/object/field_get_set/accessors.rs @@ -614,7 +614,7 @@ pub(crate) unsafe fn array_subclass_prototype_field( { return None; } - let key_ptr = (key as *const u8).add(std::mem::size_of::()); + 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