diff --git a/changelog.d/8984-private-field-updates.md b/changelog.d/8984-private-field-updates.md new file mode 100644 index 0000000000..81d0c5f2a7 --- /dev/null +++ b/changelog.d/8984-private-field-updates.md @@ -0,0 +1,7 @@ +Private class fields now preserve their value under compound and logical +assignments instead of reading `undefined` and storing `NaN`. + +Private fields no longer occupy public class-shape keys, so they stay absent +from `Object.keys`, `Object.getOwnPropertyNames`, `for...in`, spread, and JSON +serialization. An ordinary property whose name matches Perry's transient +private-member routing spelling is now retained as ordinary user data. diff --git a/changelog.d/8985-elements-lean-push-pop.md b/changelog.d/8985-elements-lean-push-pop.md new file mode 100644 index 0000000000..a6efab2b17 --- /dev/null +++ b/changelog.d/8985-elements-lean-push-pop.md @@ -0,0 +1,4 @@ +### Changed + +- Appending to and popping from a `class X extends Array` instance no longer re-classifies its own elements store: an in-capacity append and a non-hole tail pop run without a handle scope, a proxy probe, forwarding-stub cleaning or flag resolution, keeping only the element bookkeeping the read tiers and the loop guard consume. Growth, holes, an empty store and every exotic flag keep the complete runtime entry. +- `sub.pop()` on such an instance now pops inline as well: the codegen tier resolves the payload through the meta record and runs the same length/read/take blocks on it, instead of calling the runtime entry that only re-derives the store. diff --git a/crates/perry-codegen/src/codegen/mod.rs b/crates/perry-codegen/src/codegen/mod.rs index 85d4a803d7..0e8ca294ff 100644 --- a/crates/perry-codegen/src/codegen/mod.rs +++ b/crates/perry-codegen/src/codegen/mod.rs @@ -1164,15 +1164,17 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> // first (walking from deepest ancestor down) so the slot // order matches `class_field_global_index`'s assumption. let mut packed_keys = String::new(); - // Skip computed-key fields (`[Symbol.for("k")] = …`): their key is an - // expression evaluated at runtime, not a stable string, so they don't - // get an inline slot. Including their synthetic `__computed_field_*` - // names in the packed keys would surface them as enumerable own - // properties via Object.keys() and inflate the inline-slot count. - // Their values are stored via `apply_field_initializers_recursive`'s - // IndexSet path → js_object_set_field / js_object_set_symbol_property. + // Skip computed-key fields (`[Symbol.for("k")] = …`) and private + // fields. Computed keys are evaluated at construction time; private + // fields live in class-id-qualified runtime storage installed by + // `js_private_field_add`. Neither is a public inline shape key. + // Including either synthetic/source spelling in packed keys leaks it + // through reflection and inflates/misaligns the inline-slot layout. let count_keyable = |fields: &[perry_hir::ClassField]| -> u32 { - fields.iter().filter(|f| f.key_expr.is_none()).count() as u32 + fields + .iter() + .filter(|f| f.key_expr.is_none() && !f.is_private) + .count() as u32 }; let mut total_field_count = count_keyable(&c.fields); // (parent_name, resolved_fields) captured during the chain walk so we @@ -1251,7 +1253,7 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> // (which would risk re-picking the wrong same-named stub). for (_parent_name, parent_fields) in parent_chain.iter().rev() { for f in parent_fields { - if f.key_expr.is_some() { + if f.key_expr.is_some() || f.is_private { continue; } packed_keys.push_str(&f.name); @@ -1259,7 +1261,7 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> } } for f in &c.fields { - if f.key_expr.is_some() { + if f.key_expr.is_some() || f.is_private { continue; } packed_keys.push_str(&f.name); @@ -1351,7 +1353,13 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> ); class_keys_globals_map.insert(c.name.clone(), global_name.clone()); let mut packed_keys = String::new(); - let mut total_field_count = c.fields.len() as u32; + let keyable_count = |fields: &[perry_hir::ClassField]| -> u32 { + fields + .iter() + .filter(|f| f.key_expr.is_none() && !f.is_private) + .count() as u32 + }; + let mut total_field_count = keyable_count(&c.fields); // Issue #485: imported subclass stubs also need their parent's // fields prepended to the packed-keys, so allocations on this // importing side reserve enough inline slots for parent + @@ -1379,12 +1387,12 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> resolve_parent(&parent_name, child_prefix.as_deref()) { parent_chain.push((parent_name.clone(), parent_fields.clone())); - total_field_count += parent_fields.len() as u32; + total_field_count += keyable_count(&parent_fields); p = parent_extends; child_prefix = Some(parent_prefix); } else if let Some(parent) = hir.classes.iter().find(|cls| cls.name == parent_name) { parent_chain.push((parent_name.clone(), parent.fields.clone())); - total_field_count += parent.fields.len() as u32; + total_field_count += keyable_count(&parent.fields); p = parent.extends_name.clone(); child_prefix = Some(module_prefix.clone()); } else { @@ -1393,11 +1401,17 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> } for (_parent_name, parent_fields) in parent_chain.iter().rev() { for f in parent_fields { + if f.key_expr.is_some() || f.is_private { + continue; + } packed_keys.push_str(&f.name); packed_keys.push('\0'); } } for f in &c.fields { + if f.key_expr.is_some() || f.is_private { + continue; + } packed_keys.push_str(&f.name); packed_keys.push('\0'); } diff --git a/crates/perry-codegen/src/expr/array_pop.rs b/crates/perry-codegen/src/expr/array_pop.rs index a881375033..843a19eab3 100644 --- a/crates/perry-codegen/src/expr/array_pop.rs +++ b/crates/perry-codegen/src/expr/array_pop.rs @@ -36,19 +36,30 @@ const POINTER_TAG_HI16: &str = "32765"; // 0x7FFD const HANDLE_BAND_TOP: &str = "1048575"; // 0x0FFFFF — heap objects are above const HEAP_LIMIT: &str = "140737488355328"; // 2^47 const GC_TYPE_ARRAY_I8: &str = "1"; +const GC_TYPE_OBJECT_I8: &str = "2"; const GC_FLAG_FORWARDED_I8: &str = "-128"; // 0x80 as i8 const MAX_FAST_LENGTH_I32: &str = "100000000"; /// Lower `recv.pop()` for an Array-admitted receiver: the inline tier above /// with `js_array_pop_f64` behind it. Returns the popped element (boxed). pub(crate) fn lower_array_pop_inline(ctx: &mut FnCtx<'_>, recv_box: &str) -> String { + let meta_offset = + crate::target_layout::object_meta_slot_offset_bytes(ctx.target_triple).to_string(); let hdr_idx = ctx.new_block("apop.hdr"); + // An elements-backed Array subclass (`ObjectMeta.elements`, word 12) pops + // from its store: the header gate resolves the payload and the same + // length/read/take blocks run on it, so `sub.pop()` stops paying a runtime + // entry that only re-derives the store (5.1% of the wolf-ecs entity cycle). + let elem_idx = ctx.new_block("apop.elements"); + let elem_check_idx = ctx.new_block("apop.elements.check"); let len_idx = ctx.new_block("apop.len"); let read_idx = ctx.new_block("apop.read"); let take_idx = ctx.new_block("apop.take"); let slow_idx = ctx.new_block("apop.slow"); let merge_idx = ctx.new_block("apop.merge"); let hdr_label = ctx.block_label(hdr_idx); + let elem_label = ctx.block_label(elem_idx); + let elem_check_label = ctx.block_label(elem_check_idx); let len_label = ctx.block_label(len_idx); let read_label = ctx.block_label(read_idx); let take_label = ctx.block_label(take_idx); @@ -90,18 +101,72 @@ pub(crate) fn lower_array_pop_inline(ctx: &mut FnCtx<'_>, recv_box: &str) -> Str let plain = blk.icmp_eq(I16, &blocking, "0"); let invalidated = blk.load_volatile(I8, "@PERRY_ARRAY_INDEX_FAST_PATH_INVALIDATED"); let prototype_clean = blk.icmp_eq(I8, &invalidated, "0"); + let mut ok = blk.and(I1, ¬_fwd, &plain); + ok = blk.and(I1, &ok, &prototype_clean); + let array_ok = blk.and(I1, &ok, &is_array); + // A `GC_TYPE_OBJECT` receiver may be an elements-backed Array + // subclass; anything else keeps the runtime entry. + let is_object = blk.icmp_eq(I8, &gc_type, GC_TYPE_OBJECT_I8); + blk.cond_br(&array_ok, &len_label, &elem_label); + let _ = is_object; + } + let hdr_end = ctx.block_label(hdr_idx); + + ctx.current_block = elem_idx; + let store = { + let blk = ctx.block(); + let gc_type_addr = blk.sub(I64, &handle, "8"); + let gc_type_ptr = blk.inttoptr(I64, &gc_type_addr); + let gc_type = blk.load(I8, &gc_type_ptr); + let is_object = blk.icmp_eq(I8, &gc_type, GC_TYPE_OBJECT_I8); + let meta_addr = blk.add(I64, &handle, &meta_offset); + let meta_slot = blk.inttoptr(I64, &meta_addr); + let meta = blk.load(I64, &meta_slot); + let has_meta = blk.icmp_ne(I64, &meta, "0"); + let can_read_meta = blk.and(I1, &is_object, &has_meta); + // `select` keeps the load of word 12 off a null meta pointer. + let safe_meta = blk.select(I1, &can_read_meta, I64, &meta, &handle); + let meta_ptr = blk.inttoptr(I64, &safe_meta); + let store_slot = blk.gep(I64, &meta_ptr, &[(I64, "12")]); + let store = blk.load(I64, &store_slot); + let has_store = blk.icmp_ne(I64, &store, "0"); + let ok = blk.and(I1, &can_read_meta, &has_store); + blk.cond_br(&ok, &elem_check_label, &slow_label); + store + }; + + ctx.current_block = elem_check_idx; + { + let blk = ctx.block(); + let gc_type_addr = blk.sub(I64, &store, "8"); + let gc_type_ptr = blk.inttoptr(I64, &gc_type_addr); + let gc_type = blk.load(I8, &gc_type_ptr); + let is_array = blk.icmp_eq(I8, &gc_type, GC_TYPE_ARRAY_I8); + let gc_flags_addr = blk.sub(I64, &store, "7"); + let gc_flags_ptr = blk.inttoptr(I64, &gc_flags_addr); + let gc_flags = blk.load(I8, &gc_flags_ptr); + let fwd_bits = blk.and(I8, &gc_flags, GC_FLAG_FORWARDED_I8); + let not_fwd = blk.icmp_eq(I8, &fwd_bits, "0"); + let reserved_addr = blk.sub(I64, &store, "6"); + let reserved_ptr = blk.inttoptr(I64, &reserved_addr); + let reserved = blk.load(I16, &reserved_ptr); + let blocking = blk.and(I16, &reserved, POP_BLOCKING_FLAGS_I16); + let plain = blk.icmp_eq(I16, &blocking, "0"); let mut ok = blk.and(I1, &is_array, ¬_fwd); ok = blk.and(I1, &ok, &plain); - ok = blk.and(I1, &ok, &prototype_clean); blk.cond_br(&ok, &len_label, &slow_label); } + let elem_end = ctx.block_label(elem_check_idx); ctx.current_block = len_idx; + let payload = ctx + .block() + .phi(I64, &[(&handle, &hdr_end), (&store, &elem_end)]); let new_length = { let blk = ctx.block(); // ArrayHeader: length @0 (i32), capacity @4 (i32), elements @8. - let length = blk.safe_load_i32_from_ptr(&handle); - let cap_addr = blk.add(I64, &handle, "4"); + let length = blk.safe_load_i32_from_ptr(&payload); + let cap_addr = blk.add(I64, &payload, "4"); let cap_ptr = blk.inttoptr(I64, &cap_addr); let capacity = blk.load(I32, &cap_ptr); let nonempty = blk.icmp_ne(I32, &length, "0"); @@ -119,7 +184,7 @@ pub(crate) fn lower_array_pop_inline(ctx: &mut FnCtx<'_>, recv_box: &str) -> Str let blk = ctx.block(); let new_length_i64 = blk.zext(I32, &new_length, I64); let elem_off = blk.shl(I64, &new_length_i64, "3"); - let elements_addr = blk.add(I64, &handle, "8"); + let elements_addr = blk.add(I64, &payload, "8"); let elem_addr = blk.add(I64, &elements_addr, &elem_off); let elem_ptr = blk.inttoptr(I64, &elem_addr); let elem = blk.load(DOUBLE, &elem_ptr); @@ -132,7 +197,7 @@ pub(crate) fn lower_array_pop_inline(ctx: &mut FnCtx<'_>, recv_box: &str) -> Str ctx.current_block = take_idx; { let blk = ctx.block(); - let len_ptr = blk.inttoptr(I64, &handle); + let len_ptr = blk.inttoptr(I64, &payload); // `length` is a plain i32 word: no pointer, no barrier, no layout // note — exactly the runtime fast path's single store. blk.store(I32, &new_length, &len_ptr); @@ -337,6 +402,21 @@ mod tests { slow.contains("call double @js_array_pop_f64("), "{what}: the runtime pop must remain the fallback:\n{slow}" ); + // An elements-backed Array subclass resolves its payload through the + // meta record (`ObjectMeta.elements`, word 12) and pops from it with + // the same length/read/take blocks — no runtime entry for that case. + let elements = super::super::class_field_barrier_tests::block_body(ir, "apop.elements.") + .expect("the elements probe block exists"); + assert!( + elements.contains("getelementptr i64, ptr %") && elements.contains(", i64 12"), + "{what}: the probe must load ObjectMeta.elements at word 12:\n{elements}" + ); + let check = super::super::class_field_barrier_tests::block_body(ir, "apop.elements.check.") + .expect("the store validation block exists"); + assert!( + check.contains(", 1031") && check.contains("icmp eq i8"), + "{what}: the store must clear the same integrity mask as a plain array:\n{check}" + ); } /// Both `pop` routes — the erased class-field receiver diff --git a/crates/perry-codegen/src/expr/index_get/inline_dyn_typed_array.rs b/crates/perry-codegen/src/expr/index_get/inline_dyn_typed_array.rs index 8968594c52..5f1f8b93e8 100644 --- a/crates/perry-codegen/src/expr/index_get/inline_dyn_typed_array.rs +++ b/crates/perry-codegen/src/expr/index_get/inline_dyn_typed_array.rs @@ -387,9 +387,8 @@ pub(super) fn lower_inline_dyn_typed_array_get( } else { 8 }; - let meta_offset = (crate::target_layout::object_header_size_bytes(ctx.target_triple) - - meta_ptr_size) - .to_string(); + let meta_offset = + crate::target_layout::object_meta_slot_offset_bytes(ctx.target_triple).to_string(); // Reject every non-pointer / handle-band / noncanonical-index case before // touching a managed header. The miss helper retains full ToPropertyKey, diff --git a/crates/perry-codegen/src/expr/property_get/composed_ics.rs b/crates/perry-codegen/src/expr/property_get/composed_ics.rs index 041ab6f050..3397a4a170 100644 --- a/crates/perry-codegen/src/expr/property_get/composed_ics.rs +++ b/crates/perry-codegen/src/expr/property_get/composed_ics.rs @@ -228,9 +228,8 @@ pub(super) fn emit_array_subclass_length_ic( } else { 8 }; - let meta_offset = (crate::target_layout::object_header_size_bytes(ctx.target_triple) - - meta_ptr_size) - .to_string(); + let meta_offset = + crate::target_layout::object_meta_slot_offset_bytes(ctx.target_triple).to_string(); // An elements-backed Array-subclass instance (`ObjectMeta.elements`): // `length` is the inner Array's length word — no shape IC. A probe miss // (no meta, no store) is the shape-carried form and keeps the IC below. diff --git a/crates/perry-codegen/src/expr/property_get/generic_dispatch.rs b/crates/perry-codegen/src/expr/property_get/generic_dispatch.rs index 89c1a4f49a..60335e3060 100644 --- a/crates/perry-codegen/src/expr/property_get/generic_dispatch.rs +++ b/crates/perry-codegen/src/expr/property_get/generic_dispatch.rs @@ -597,9 +597,8 @@ pub(crate) fn lower_generic_property_get( } else { 8 }; - let meta_offset = (crate::target_layout::object_header_size_bytes(ctx.target_triple) - - meta_ptr_size) - .to_string(); + let meta_offset = + crate::target_layout::object_meta_slot_offset_bytes(ctx.target_triple).to_string(); let meta_addr = ctx.block().add(I64, &obj_handle, &meta_offset); let meta_slot = ctx.block().inttoptr(I64, &meta_addr); let meta_load_ty = if meta_ptr_size == 4 { I32 } else { I64 }; diff --git a/crates/perry-codegen/src/lower_call/new_alloc.rs b/crates/perry-codegen/src/lower_call/new_alloc.rs index f62d2b1301..2a43b655fa 100644 --- a/crates/perry-codegen/src/lower_call/new_alloc.rs +++ b/crates/perry-codegen/src/lower_call/new_alloc.rs @@ -173,7 +173,13 @@ fn emit_instance_alloc_inner( // Compute total field count including inherited parent fields. // The runtime allocates at least 8 inline slots regardless, so this // mostly matters for shapes >8 fields. - let mut field_count = class.fields.len() as u32; + let public_keyable_count = |fields: &[perry_hir::ClassField]| -> u32 { + fields + .iter() + .filter(|field| field.key_expr.is_none() && !field.is_private) + .count() as u32 + }; + let mut field_count = public_keyable_count(&class.fields); // Imported classes now carry their real field_names from the source // module. If the field count is still 0 (no fields info available), // use a generous default as a safety net. @@ -183,7 +189,7 @@ fn emit_instance_alloc_inner( let mut parent = class.extends_name.as_deref(); while let Some(parent_name) = parent { if let Some(p) = ctx.classes.get(parent_name).copied() { - field_count += p.fields.len() as u32; + field_count += public_keyable_count(&p.fields); parent = p.extends_name.as_deref(); } else { break; @@ -306,7 +312,7 @@ fn emit_instance_alloc_inner( // inline bump-alloc fast path (which would bake the wrong layout). let mut packed_keys = String::new(); for f in &class.fields { - if f.key_expr.is_some() { + if f.key_expr.is_some() || f.is_private { continue; } packed_keys.push_str(&f.name); @@ -688,15 +694,12 @@ fn emit_instance_alloc_inner( break; } } - // Skip computed-key fields: their key is an expression evaluated at - // construction time, not a stable string, so they don't get an inline - // slot. The runtime stores them via IndexSet → js_object_set_field / - // js_object_set_symbol_property paths in `apply_field_initializers_recursive`. - // Including their synthetic `__computed_field_*` names in packed_keys - // would surface them as enumerable own properties on Object.keys(). + // Skip computed and private fields: both are initialized through + // dedicated runtime paths and neither belongs in the public inline + // shape exposed by Object.keys/getOwnPropertyNames. for pc in parent_chain.iter().rev() { for f in &pc.fields { - if f.key_expr.is_some() { + if f.key_expr.is_some() || f.is_private { continue; } packed_keys.push_str(&f.name); @@ -704,7 +707,7 @@ fn emit_instance_alloc_inner( } } for f in &class.fields { - if f.key_expr.is_some() { + if f.key_expr.is_some() || f.is_private { continue; } packed_keys.push_str(&f.name); diff --git a/crates/perry-codegen/src/stmt/stable_packed_loop.rs b/crates/perry-codegen/src/stmt/stable_packed_loop.rs index e9617580d6..31967e4477 100644 --- a/crates/perry-codegen/src/stmt/stable_packed_loop.rs +++ b/crates/perry-codegen/src/stmt/stable_packed_loop.rs @@ -696,9 +696,8 @@ fn build_numeric_access( } else { 8 }; - let meta_offset = (crate::target_layout::object_header_size_bytes(ctx.target_triple) - - pointer_size) - .to_string(); + let meta_offset = + crate::target_layout::object_meta_slot_offset_bytes(ctx.target_triple).to_string(); let meta_addr = ctx.block().add(I64, live_raw, &meta_offset); let meta_slot = ctx.block().inttoptr(I64, &meta_addr); let meta_native = ctx @@ -1071,9 +1070,8 @@ pub(crate) fn try_lower_index_get( } else { 8 }; - let meta_offset = (crate::target_layout::object_header_size_bytes(ctx.target_triple) - - pointer_size) - .to_string(); + let meta_offset = + crate::target_layout::object_meta_slot_offset_bytes(ctx.target_triple).to_string(); let meta_addr = ctx.block().add(I64, &raw, &meta_offset); let meta_slot = ctx.block().inttoptr(I64, &meta_addr); let meta_native = ctx diff --git a/crates/perry-codegen/src/target_layout.rs b/crates/perry-codegen/src/target_layout.rs index 897c3b13ae..38b2e397fa 100644 --- a/crates/perry-codegen/src/target_layout.rs +++ b/crates/perry-codegen/src/target_layout.rs @@ -88,6 +88,18 @@ pub fn object_header_size_bytes(_target_triple: &str) -> u64 { 16 } +/// Byte offset of `ObjectHeader::meta` — the last word of the header — for +/// the target. +/// +/// The metadata pointer is the entry point to `ObjectMeta` (the prototype +/// override, the spill buffer, the Array-subclass elements store), and several +/// inline tiers load it. Keeping the derivation in one place also keeps the +/// object-header-size callsite census stable as tiers are added. +pub fn object_meta_slot_offset_bytes(target_triple: &str) -> u64 { + let pointer_size = if target_is_ilp32(target_triple) { 4 } else { 8 }; + object_header_size_bytes(target_triple) - pointer_size +} + /// `std::mem::size_of::()` for the /// target. /// diff --git a/crates/perry-codegen/src/typed_shape.rs b/crates/perry-codegen/src/typed_shape.rs index 17d89af661..d45b6b2cfa 100644 --- a/crates/perry-codegen/src/typed_shape.rs +++ b/crates/perry-codegen/src/typed_shape.rs @@ -298,7 +298,10 @@ fn typed_layout_from_fields<'a>( let mut pointer_mask_words = Vec::new(); let mut slot_count = 0u32; for field in fields { - if field.key_expr.is_some() { + // Computed fields and private fields are initialized through runtime + // storage, not the public inline slots represented by this descriptor. + // Keep the mask indices in lockstep with the packed class keys. + if field.key_expr.is_some() || field.is_private { continue; } let slot = slot_count as usize; diff --git a/crates/perry-hir/src/lower_patterns.rs b/crates/perry-hir/src/lower_patterns.rs index 204f860179..b7f9e4bd39 100644 --- a/crates/perry-hir/src/lower_patterns.rs +++ b/crates/perry-hir/src/lower_patterns.rs @@ -246,9 +246,12 @@ pub(crate) fn lower_assign_target_to_expr( Ok(Expr::IndexGet { object, index }) } ast::MemberProp::PrivateName(private) => { - // Compound and logical assignments lower the read and write - // halves separately. Match ordinary private-member reads: - // guard the receiver and use the class-mangled storage key. + // A compound/logical assignment reads the target before + // writing it back. Private fields do not live under their + // source spelling (`#n`): use the same guarded, class-id- + // qualified storage lookup as an ordinary `this.#n` read. + // Reading `#n` as a public property returns `undefined`, + // which made `this.#n += 1` store NaN in the real slot. let private_name = format!("#{}", private.name); let object = wrap_private_guard(ctx, object, &private_name, PRIV_OP_READ); let property = private_storage_property(ctx, &private_name); diff --git a/crates/perry-runtime/src/array/subclass_elements.rs b/crates/perry-runtime/src/array/subclass_elements.rs index cc5c34f3e1..6c8c98bc4a 100644 --- a/crates/perry-runtime/src/array/subclass_elements.rs +++ b/crates/perry-runtime/src/array/subclass_elements.rs @@ -666,6 +666,17 @@ pub(super) fn elements_push(receiver: &ValidatedObjectReceiver, value: f64) -> O if !mutation_receiver_allows_plain_tail(receiver.object_flags) { return None; } + // In-capacity append: no allocation, therefore no rooting, no head + // write-back, and none of the receiver re-classification + // `js_array_push_f64` owes an arbitrary caller-supplied pointer (proxy + // probe, forwarding-stub cleaning, flag resolution) — this store is ours, + // reached through the meta slot, and its header is one read away. Only the + // element bookkeeping (`store_array_slot_resolved`) is kept: it maintains + // the numeric/element-shape proofs the read tiers and the loop guard + // consume. + if let Some(length) = unsafe { elements_push_in_capacity(elements, value) } { + return Some(length); + } let obj = receiver.object as *mut ObjectHeader; unsafe { let scope = crate::gc::RuntimeHandleScope::new(); @@ -689,5 +700,73 @@ pub(super) fn elements_pop(receiver: &ValidatedObjectReceiver) -> Option { if !mutation_receiver_allows_plain_tail(receiver.object_flags) { return None; } + // The mirror of `elements_push_in_capacity`: removing the tail stores + // nothing, so a non-hole element is a length decrement and a load. An + // empty store, a hole (which reads through the prototype chain) and every + // exotic flag keep the complete runtime entry. + if let Some(value) = unsafe { elements_pop_tail(elements) } { + return Some(value); + } Some(crate::array::js_array_pop_f64(elements)) } + +/// The store's header when it is an ordinary, non-forwarded, unrestricted +/// Array — the only shape the lean append/pop below may touch. +/// +/// # Safety +/// `elements` must be a live store address read from the meta slot. +#[inline] +unsafe fn plain_store_flags(elements: *mut ArrayHeader) -> Option { + let header = crate::value::addr_class::try_read_gc_header(elements as usize)?; + if header.obj_type != crate::gc::GC_TYPE_ARRAY + || header.gc_flags & crate::gc::GC_FLAG_FORWARDED != 0 + { + return None; + } + let blocking = crate::gc::OBJ_FLAG_FROZEN + | crate::gc::OBJ_FLAG_SEALED + | crate::gc::OBJ_FLAG_NO_EXTEND + | crate::gc::OBJ_FLAG_ARRAY_DESCRIPTORS; + (header._reserved & blocking == 0).then_some(header._reserved) +} + +/// Append without allocating: `Some(new_length)` when the store had spare +/// capacity, `None` when the caller must take the growing path. +/// +/// # Safety +/// As [`plain_store_flags`]. +#[inline] +unsafe fn elements_push_in_capacity(elements: *mut ArrayHeader, value: f64) -> Option { + let flags = plain_store_flags(elements)?; + let length = (*elements).length; + let capacity = (*elements).capacity; + if length >= capacity { + return None; + } + crate::string::js_string_addref_if_heap_string(value); + crate::array::store_array_slot_resolved(elements, length as usize, value, flags); + (*elements).length = length + 1; + Some(f64::from(length + 1)) +} + +/// Remove and return the tail element, or `None` when the complete runtime +/// entry has to run (empty store, a hole, an exotic flag). +/// +/// # Safety +/// As [`plain_store_flags`]. +#[inline] +unsafe fn elements_pop_tail(elements: *mut ArrayHeader) -> Option { + plain_store_flags(elements)?; + let length = (*elements).length; + let capacity = (*elements).capacity; + if length == 0 || length > capacity { + return None; + } + let index = length - 1; + let bits = slot_bits(elements, index); + if bits == crate::value::TAG_HOLE { + return None; + } + (*elements).length = index; + Some(f64::from_bits(bits)) +} diff --git a/crates/perry-runtime/src/array/subclass_elements_tests.rs b/crates/perry-runtime/src/array/subclass_elements_tests.rs index c58ba484ce..b93cc97b2d 100644 --- a/crates/perry-runtime/src/array/subclass_elements_tests.rs +++ b/crates/perry-runtime/src/array/subclass_elements_tests.rs @@ -427,3 +427,94 @@ fn the_counted_loop_guard_admits_an_elements_backed_receiver_as_kind_three() { ); assert_eq!(facts[6], 64); } + +/// The allocation-free append and tail-pop paths agree with the complete +/// runtime entries: values, `length`, holes, an empty store, and the growth +/// edge (which must still publish the re-allocated head). +#[test] +fn the_lean_append_and_pop_paths_match_the_runtime_entries() { + let _representation = ArraySubclassRepresentationGuard::elements(); + let _triggers = crate::gc::GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + crate::gc::register_runtime_handle_root_scanner_for_tests(); + let class_id = 0x0074_869a; + crate::object::js_register_class_parent(class_id, CLASS_ID_ARRAY); + let obj = js_object_alloc(class_id, 2); + let scope = crate::gc::RuntimeHandleScope::new(); + let recv_h = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(obj as i64)); + unsafe { install_elements(live_obj(recv_h.get_nanbox_f64()), 0) }; + let recv = || recv_h.get_nanbox_f64(); + let store = || unsafe { elements_of(live_obj(recv())) }; + + // Empty: pop takes the runtime entry and answers `undefined`. + let empty = super::subclass::array_subclass_fast_pop(recv()).expect("pop is handled"); + assert_eq!(empty.to_bits(), crate::value::TAG_UNDEFINED); + assert_eq!( + super::subclass::array_subclass_fast_length(recv()), + Some(0.0) + ); + + // 64 appends: the first of each capacity class grows (head write-back), + // the rest take the in-capacity path. + let mut heads = std::collections::HashSet::new(); + for i in 0..64u32 { + assert_eq!( + super::subclass::array_subclass_fast_push_one(recv(), f64::from(i)), + Some(f64::from(i + 1)) + ); + heads.insert(store() as usize); + assert_eq!(unsafe { (*store()).length }, i + 1); + } + assert!(heads.len() > 1, "the store re-allocated at least once"); + assert_eq!( + super::subclass::array_subclass_fast_length(recv()), + Some(64.0) + ); + for i in 0..64u32 { + assert_eq!( + super::subclass::array_subclass_fast_index_get(recv(), i), + Some(f64::from(i)) + ); + } + + // Tail pops walk back down, and the values come out in order. + for i in (32..64u32).rev() { + assert_eq!( + super::subclass::array_subclass_fast_pop(recv()), + Some(f64::from(i)) + ); + } + assert_eq!( + super::subclass::array_subclass_fast_length(recv()), + Some(32.0) + ); + + // A hole at the tail keeps the complete entry (it reads through the + // prototype chain), and `length` still drops by one. + assert_eq!( + crate::object::js_object_delete_dynamic(live_obj(recv()), 31.0), + 1 + ); + let popped = super::subclass::array_subclass_fast_pop(recv()).expect("pop is handled"); + assert!( + popped.to_bits() == crate::value::TAG_UNDEFINED || popped.is_nan(), + "a hole pops as undefined: {popped:?}" + ); + assert_eq!( + super::subclass::array_subclass_fast_length(recv()), + Some(31.0) + ); + + // A pointer value still gets its bookkeeping: store a string and read it + // back through the funnel. + let text = crate::string::js_string_from_bytes(b"hello".as_ptr(), 5); + let text_value = crate::value::js_nanbox_string(text as i64); + assert!(super::subclass::array_subclass_fast_push_one(recv(), text_value).is_some()); + assert_eq!( + super::subclass::array_subclass_fast_index_get(recv(), 31).map(|v| v.to_bits()), + Some(text_value.to_bits()) + ); + assert_eq!( + super::subclass::array_subclass_fast_pop(recv()).map(|v| v.to_bits()), + Some(text_value.to_bits()) + ); +} diff --git a/crates/perry-runtime/src/object/field_get_set/enumeration.rs b/crates/perry-runtime/src/object/field_get_set/enumeration.rs index b86c326272..001b9ddd0a 100644 --- a/crates/perry-runtime/src/object/field_get_set/enumeration.rs +++ b/crates/perry-runtime/src/object/field_get_set/enumeration.rs @@ -1368,6 +1368,12 @@ pub(crate) unsafe fn instance_private_key_hidden( /// prefix test would wrongly hide legitimate user properties whose name happens /// to begin with `__perry_` (e.g. `this.__perry_user = 1`). /// +/// `#` is deliberately NOT in this list. It is a +/// transient compiler routing key for private method/accessor operations; the +/// runtime consumes it only when a matching private-access hint is pending and +/// never installs it as private object storage. Without a hint, that spelling +/// is ordinary user data and must remain visible to reflection. +/// /// The one prefix family is `__perry_native_super__` (#6316): the native /// base method a subclass override displaced. Its key set is parameterized by /// method name, so an exact allowlist cannot enumerate it. The prefix is a @@ -1386,7 +1392,6 @@ pub(crate) fn is_internal_runtime_key_bytes(b: &[u8]) -> bool { || b == b"#" || b == b"#" || b.starts_with(b"# u64 {": 1 }, @@ -55,7 +52,7 @@ "crates/perry-runtime/src/object/object_ops.rs|keys_array|declaration|pub(crate) use keys_array::{": 1 }, "summary": { - "codegen_object_header_size_sites": 46, + "codegen_object_header_size_sites": 42, "raw_member_files": 7, "raw_member_sites": { "keys_array": 24 diff --git a/test-files/test_gap_8969_private_field_compound_update.ts b/test-files/test_gap_8969_private_field_compound_update.ts new file mode 100644 index 0000000000..a006edd32a --- /dev/null +++ b/test-files/test_gap_8969_private_field_compound_update.ts @@ -0,0 +1,119 @@ +class Counter { + #count = 0; + + explicit(): number { + this.#count = this.#count + 1; + return this.#count; + } + + readInExpr(): number { + const value = this.#count; + return value + 1; + } + + compound(): number { + this.#count += 1; + return this.#count; + } + + logical(): number { + this.#count ||= 10; + this.#count &&= 4; + this.#count ??= 20; + return this.#count; + } + + postfix(): number { + return this.#count++; + } + + prefix(): number { + return ++this.#count; + } + + read(): number { + return this.#count; + } +} + +const compound = new Counter(); +console.log("explicit", compound.explicit()); +console.log("readInExpr", compound.readInExpr()); +console.log("compound", compound.compound()); +console.log("logical", compound.logical()); + +// Exercise update lowering on a fresh instance so a failed compound write +// cannot poison the observation. +const update = new Counter(); +console.log("postfix-result", update.postfix()); +console.log("postfix-value", update.read()); +console.log("prefix-result", update.prefix()); +console.log("prefix-value", update.read()); + +class Hidden { + #value = 5; + + read(): number { + return this.#value; + } +} + +const hidden = new Hidden(); +console.log("hidden-read", hidden.read()); +console.log("own-names", JSON.stringify(Object.getOwnPropertyNames(hidden))); +console.log("keys", JSON.stringify(Object.keys(hidden))); +console.log("json", JSON.stringify(hidden)); +console.log("spread", JSON.stringify({ ...hidden })); +let forIn = ""; +for (const key in hidden) { + forIn += key; +} +console.log("for-in", forIn); + +// A compiler routing spelling used as ordinary user data must remain an +// ordinary property when no private-access hint accompanies it. +const collisionKey = "#"; +const collision: Record = {}; +collision[collisionKey] = 8; +console.log("collision-json", JSON.stringify(collision)); +console.log("collision-keys", JSON.stringify(Object.keys(collision))); +console.log("collision-names", JSON.stringify(Object.getOwnPropertyNames(collision))); +console.log("collision-read", collision[collisionKey]); +console.log("collision-in", collisionKey in collision); +console.log("collision-own", Object.hasOwn(collision, collisionKey)); + +// Private fields must not consume or shift public shape slots, including +// across an inheritance chain. +class Parent { + parent = 1; + #parentSecret = 2; + + parentTotal(): number { + return this.parent + this.#parentSecret; + } +} + +class Child extends Parent { + child = 3; + #childSecret = 4; + + total(): number { + return this.parentTotal() + this.child + this.#childSecret; + } +} + +const mixed = new Child(); +console.log("mixed-total", mixed.total()); +console.log("mixed-keys", JSON.stringify(Object.keys(mixed))); +console.log("mixed-names", JSON.stringify(Object.getOwnPropertyNames(mixed))); + +class StaticCounter { + static #count = 0; + + static increment(): number { + this.#count += 1; + return this.#count; + } +} + +console.log("static-compound", StaticCounter.increment());