diff --git a/changelog.d/9226-class-prototype-own-keys.md b/changelog.d/9226-class-prototype-own-keys.md new file mode 100644 index 0000000000..2ecbfa0a3a --- /dev/null +++ b/changelog.d/9226-class-prototype-own-keys.md @@ -0,0 +1,41 @@ +**Class prototypes now expose one coherent, spec-ordered own-key surface** +(#9226). `Object.getOwnPropertyNames(C.prototype)` omitted every accessor, +listed Perry's internal `@@iterator` dispatch alias as if it were a source +string key, and `Object.getOwnPropertySymbols` returned nothing — so +`Reflect.ownKeys` disagreed with `hasOwnProperty` and +`getOwnPropertyDescriptor`, both of which found the missing keys. A two-step +"list the keys, then inspect each one" walk — what decorators, DI containers, +serializers and test-framework method discovery all do — got self-contradictory +answers. + +Three separate causes, not one. Accessors were missing because the names +builder read only `vtable.methods`, never `getters`/`setters`. The +`"@@iterator"` string was a *lowering* artifact: every well-known-symbol class +member was diverted away from the computed-key path and registered under a +synthetic string name, so the real Symbol key was never installed anywhere +`getOwnPropertySymbols` could see it. And the order was whatever the dispatch +hash maps happened to yield, because those maps are keyed for lookup speed and +carry no source position. + +Only three well-known-symbol forms now need special lowering (a generator +`[Symbol.iterator]`, `static [Symbol.hasInstance]`, and a +`get [Symbol.toStringTag]`); the rest register a real Symbol key, with the +generator form registering both its dispatch wrapper and its Symbol key. The +synthetic dispatch aliases stay in the vtable for fast calls but are filtered +out of enumeration — a class with a source method literally named +`"@@iterator"` still lists it, because that one carries a definition-order +record and an alias does not. Definition order itself comes from the member +function's HIR id, allocated while walking the ClassBody, which is what lets +reflection reconstruct one source order across the separate method, getter, +setter and Symbol registries. Static fields keep first-install order, and a +class key that is deleted and recreated moves to the end, as `[[OwnPropertyKeys]]` +requires. + +`Reflect.ownKeys` is now the union in spec order — integer-index strings +ascending, remaining strings in property-creation order, then Symbols in +property-creation order — with `getOwnPropertyNames` and +`getOwnPropertySymbols` as its two halves, and `hasOwnProperty` agreeing with +both for Symbol-keyed class members. A 41-assertion gap fixture that diverges +from `node --experimental-strip-types` on 36 of its 41 lines before the change +is byte-identical after, and a 104-assertion class-prototype differential goes +from 11 divergences to zero. diff --git a/crates/perry-codegen/src/codegen/string_pool.rs b/crates/perry-codegen/src/codegen/string_pool.rs index 9c1d9bfd3b..ded5ceba4c 100644 --- a/crates/perry-codegen/src/codegen/string_pool.rs +++ b/crates/perry-codegen/src/codegen/string_pool.rs @@ -657,14 +657,15 @@ pub(super) fn emit_string_pool( // symbols for those live in the defining module's object file. // Each module's init registers its own classes; the linker // ensures all init functions run before main. - // (class_id, name, llvm_symbol, total_param_count, has_synth_args, has_rest, spec_length) - let mut method_triples: Vec<(u32, String, String, u32, bool, bool, u32)> = Vec::new(); + // (class_id, name, llvm_symbol, total_param_count, has_synth_args, + // has_rest, spec_length, definition_order) + let mut method_triples: Vec<(u32, String, String, u32, bool, bool, u32, u32)> = Vec::new(); // #1788: (cid, static-method name, perry_static_* symbol, param_count, // has_rest). Registered into the runtime CLASS_STATIC_METHODS table so a // subclass whose parent is a class-expression value inherits the parent's // static methods (`class Sub extends make(...) {}; Sub.greet()`); has_rest // tells the dispatcher to bundle trailing args for a `...rest` param. - let mut static_method_triples: Vec<(u32, String, String, u32, bool, u32)> = Vec::new(); + let mut static_method_triples: Vec<(u32, String, String, u32, bool, u32, u32)> = Vec::new(); // #1787: (cid, standalone-constructor symbol, total_param_count). // Registered into CLASS_CONSTRUCTORS so `new ()` (a // class-expression value constructed dynamically) can replay the class's @@ -759,6 +760,7 @@ pub(super) fn emit_string_pool( has_synth_args, has_rest, spec_length, + method.id, )); } // #1788: static methods are emitted as `perry_static_*` (no `this` @@ -785,6 +787,7 @@ pub(super) fn emit_string_pool( sm.params.len() as u32, has_rest, spec_length, + sm.id, )); } // #1787: the standalone constructor `___constructor` @@ -857,8 +860,16 @@ pub(super) fn emit_string_pool( ctor_triples.push((cid, ctor_symbol, ctor_params, ctor_sig_caps)); } method_triples.sort_unstable(); - for (cid, method_name, llvm_name, param_count, has_synth_args, has_rest, spec_length) in - method_triples + for ( + cid, + method_name, + llvm_name, + param_count, + has_synth_args, + has_rest, + spec_length, + definition_order, + ) in method_triples { chunker.roll_if_full(); let blk = chunker.current_block(); @@ -892,6 +903,16 @@ pub(super) fn emit_string_pool( (I64, has_rest_str), ], ); + blk.call_void( + "js_register_class_string_member_order", + &[ + (I64, &cid.to_string()), + (I64, &bytes_i64), + (I64, &len_str), + (I64, "0"), + (I64, &definition_order.to_string()), + ], + ); // Record the default-aware spec `.length` so `C.prototype.m.length` // reflects params-before-first-default, not the raw param count. blk.call_void( @@ -908,7 +929,9 @@ pub(super) fn emit_string_pool( // static methods (subclass extends a class-expression value) resolve at // runtime via the class_id parent-chain walk. static_method_triples.sort_unstable(); - for (cid, method_name, llvm_name, param_count, has_rest, spec_length) in static_method_triples { + for (cid, method_name, llvm_name, param_count, has_rest, spec_length, definition_order) in + static_method_triples + { chunker.roll_if_full(); let blk = chunker.current_block(); let entry = match strings.iter().find(|e| e.value == method_name) { @@ -932,6 +955,16 @@ pub(super) fn emit_string_pool( (I64, has_rest_str), ], ); + blk.call_void( + "js_register_class_string_member_order", + &[ + (I64, &cid.to_string()), + (I64, &bytes_i64), + (I64, &len_str), + (I64, "1"), + (I64, &definition_order.to_string()), + ], + ); // Record the default-aware spec `.length` for the static method so // `C.staticGen.length` reflects params-before-first-default rather than // the raw param count (which over-counts generator/async methods). @@ -1121,7 +1154,7 @@ pub(super) fn emit_string_pool( // `undefined`. // (class_id, prop_name, llvm_symbol, is_static) — static accessors register // onto the class constructor (CLASS_STATIC_ACCESSORS), not the instance vtable. - let mut getter_pairs: Vec<(u32, String, String, bool)> = Vec::new(); + let mut getter_pairs: Vec<(u32, String, String, bool, u32)> = Vec::new(); for (class_name, class) in classes.iter() { // Refs #486: skip alias keys (see method-emission loop above). if *class_name != class.name { @@ -1166,11 +1199,11 @@ pub(super) fn emit_string_pool( sanitize_member(&inner), ) }; - getter_pairs.push((cid, prop.clone(), llvm_name, is_static)); + getter_pairs.push((cid, prop.clone(), llvm_name, is_static, getter_fn.id)); } } getter_pairs.sort_unstable(); - for (cid, prop_name, llvm_name, is_static) in getter_pairs { + for (cid, prop_name, llvm_name, is_static, definition_order) in getter_pairs { chunker.roll_if_full(); let blk = chunker.current_block(); let entry = match strings.iter().find(|e| e.value == prop_name) { @@ -1196,6 +1229,16 @@ pub(super) fn emit_string_pool( (I64, &func_i64), ], ); + blk.call_void( + "js_register_class_string_member_order", + &[ + (I64, &cid.to_string()), + (I64, &bytes_i64), + (I64, &len_str), + (I64, if is_static { "1" } else { "0" }), + (I64, &definition_order.to_string()), + ], + ); } // Refs #486 (hono): parallel registration for class setters. Without @@ -1215,7 +1258,7 @@ pub(super) fn emit_string_pool( // class/setter-length-dflt): without a per-func-ptr length registration // the runtime fell back to the setter's ABI arity (1), over-counting the // defaulted param. - let mut setter_pairs: Vec<(u32, String, String, bool, u32)> = Vec::new(); + let mut setter_pairs: Vec<(u32, String, String, bool, u32, u32)> = Vec::new(); for (class_name, class) in classes.iter() { if *class_name != class.name { continue; @@ -1252,11 +1295,18 @@ pub(super) fn emit_string_pool( ) }; let spec_length = spec_function_length(&setter_fn.params) as u32; - setter_pairs.push((cid, prop.clone(), llvm_name, is_static, spec_length)); + setter_pairs.push(( + cid, + prop.clone(), + llvm_name, + is_static, + spec_length, + setter_fn.id, + )); } } setter_pairs.sort_unstable(); - for (cid, prop_name, llvm_name, is_static, spec_length) in setter_pairs { + for (cid, prop_name, llvm_name, is_static, spec_length, definition_order) in setter_pairs { chunker.roll_if_full(); let blk = chunker.current_block(); let entry = match strings.iter().find(|e| e.value == prop_name) { @@ -1291,6 +1341,16 @@ pub(super) fn emit_string_pool( (I64, &func_i64), ], ); + blk.call_void( + "js_register_class_string_member_order", + &[ + (I64, &cid.to_string()), + (I64, &bytes_i64), + (I64, &len_str), + (I64, if is_static { "1" } else { "0" }), + (I64, &definition_order.to_string()), + ], + ); } // Issue #493: register each rest-bearing closure body's func_ptr -> diff --git a/crates/perry-codegen/src/expr/static_field_meta.rs b/crates/perry-codegen/src/expr/static_field_meta.rs index 3a82546d3e..fdf19b161a 100644 --- a/crates/perry-codegen/src/expr/static_field_meta.rs +++ b/crates/perry-codegen/src/expr/static_field_meta.rs @@ -333,6 +333,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { is_static, param_count, has_rest, + definition_order, } => { let key_v = lower_expr(ctx, key_expr)?; if let Some(&class_id) = ctx.class_ids.get(class_name) { @@ -349,6 +350,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { let param_count_str = param_count.to_string(); let is_static_str = (*is_static as i64).to_string(); let has_rest_str = (*has_rest as i64).to_string(); + let definition_order_str = definition_order.to_string(); ctx.block().call_void( "js_register_class_computed_method", &[ @@ -358,6 +360,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { (I64, ¶m_count_str), (I64, &is_static_str), (I64, &has_rest_str), + (I64, &definition_order_str), ], ); } @@ -371,6 +374,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { getter_name, setter_name, is_static, + definition_order, } => { let key_v = lower_expr(ctx, key_expr)?; if let Some(&class_id) = ctx.class_ids.get(class_name) { @@ -407,6 +411,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { .unwrap_or_else(|| "0".to_string()); let cid_str = class_id.to_string(); let is_static_str = (*is_static as i64).to_string(); + let definition_order_str = definition_order.to_string(); ctx.block().call_void( "js_register_class_computed_accessor", &[ @@ -415,6 +420,7 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { (I64, &getter_i64), (I64, &setter_i64), (I64, &is_static_str), + (I64, &definition_order_str), ], ); } diff --git a/crates/perry-codegen/src/runtime_decls/stdlib_ffi/language_core.rs b/crates/perry-codegen/src/runtime_decls/stdlib_ffi/language_core.rs index 67b81d7aa9..725016002a 100644 --- a/crates/perry-codegen/src/runtime_decls/stdlib_ffi/language_core.rs +++ b/crates/perry-codegen/src/runtime_decls/stdlib_ffi/language_core.rs @@ -319,6 +319,11 @@ pub(crate) fn declare_core(module: &mut LlModule) { module.declare_function("js_register_class_getter", VOID, &[I64, I64, I64, I64]); // Refs #486: per-class setter dispatch — see object.rs::js_register_class_setter. module.declare_function("js_register_class_setter", VOID, &[I64, I64, I64, I64]); + module.declare_function( + "js_register_class_string_member_order", + VOID, + &[I64, I64, I64, I64, I64], + ); // Default-aware spec `.length` per class method (CLASS_METHOD_BIND_LENGTHS). module.declare_function( "js_register_class_method_bind_length", diff --git a/crates/perry-codegen/src/runtime_decls/strings_part2.rs b/crates/perry-codegen/src/runtime_decls/strings_part2.rs index 44c013b7bb..c77db22de8 100644 --- a/crates/perry-codegen/src/runtime_decls/strings_part2.rs +++ b/crates/perry-codegen/src/runtime_decls/strings_part2.rs @@ -236,12 +236,12 @@ pub(crate) fn declare_phase_b_strings_part2(module: &mut LlModule) { module.declare_function( "js_register_class_computed_method", VOID, - &[I64, DOUBLE, I64, I64, I64, I64], + &[I64, DOUBLE, I64, I64, I64, I64, I64], ); module.declare_function( "js_register_class_computed_accessor", VOID, - &[I64, DOUBLE, I64, I64, I64], + &[I64, DOUBLE, I64, I64, I64, I64], ); // v0.5.747: register a string-named static field on a class so reads // via the runtime dynamic-dispatch path (when the class ref is in an diff --git a/crates/perry-hir/src/analysis/value_types_tests.rs b/crates/perry-hir/src/analysis/value_types_tests.rs index bb5ea6431b..3380cc7448 100644 --- a/crates/perry-hir/src/analysis/value_types_tests.rs +++ b/crates/perry-hir/src/analysis/value_types_tests.rs @@ -1339,6 +1339,7 @@ fn infers_class_prototype_and_super_meta_value_shapes() { is_static: false, param_count: 0, has_rest: false, + definition_order: 1, }, Expr::RegisterClassComputedAccessor { class_name: "Widget".to_string(), @@ -1346,6 +1347,7 @@ fn infers_class_prototype_and_super_meta_value_shapes() { getter_name: Some("getValue".to_string()), setter_name: None, is_static: false, + definition_order: 2, }, ] { assert_eq!(infer_expr_type(&expr, &env), Type::Void); diff --git a/crates/perry-hir/src/ir/expr.rs b/crates/perry-hir/src/ir/expr.rs index bae425d8bb..831ead8020 100644 --- a/crates/perry-hir/src/ir/expr.rs +++ b/crates/perry-hir/src/ir/expr.rs @@ -537,6 +537,7 @@ pub enum Expr { is_static: bool, param_count: u32, has_rest: bool, + definition_order: u32, }, /// Register one side of a computed class accessor. @@ -546,6 +547,7 @@ pub enum Expr { getter_name: Option, setter_name: Option, is_static: bool, + definition_order: u32, }, /// Issue #1772: per-evaluation identity for a class EXPRESSION diff --git a/crates/perry-hir/src/lower_decl/class_computed.rs b/crates/perry-hir/src/lower_decl/class_computed.rs index 8aba1adc4d..eb6bb7fc12 100644 --- a/crates/perry-hir/src/lower_decl/class_computed.rs +++ b/crates/perry-hir/src/lower_decl/class_computed.rs @@ -30,6 +30,7 @@ pub(crate) fn class_computed_member_registration_expr( .last() .map(|p| p.is_rest) .unwrap_or(false), + definition_order: member.function.id, }, ClassComputedMemberKind::Getter => Expr::RegisterClassComputedAccessor { class_name: class_name.to_string(), @@ -37,6 +38,7 @@ pub(crate) fn class_computed_member_registration_expr( getter_name: Some(member.function.name.clone()), setter_name: None, is_static: member.is_static, + definition_order: member.function.id, }, ClassComputedMemberKind::Setter => Expr::RegisterClassComputedAccessor { class_name: class_name.to_string(), @@ -44,6 +46,7 @@ pub(crate) fn class_computed_member_registration_expr( getter_name: None, setter_name: Some(member.function.name.clone()), is_static: member.is_static, + definition_order: member.function.id, }, } } diff --git a/crates/perry-hir/src/lower_decl/class_decl.rs b/crates/perry-hir/src/lower_decl/class_decl.rs index 9ba1218b97..3b883aa819 100644 --- a/crates/perry-hir/src/lower_decl/class_decl.rs +++ b/crates/perry-hir/src/lower_decl/class_decl.rs @@ -46,16 +46,22 @@ use class_heritage::*; use super::*; fn generic_computed_member_key<'a>( - ctx: &LoweringContext, + _ctx: &LoweringContext, method: &'a ast::ClassMethod, ) -> Option<&'a ast::ComputedPropName> { let ast::PropName::Computed(computed) = &method.key else { return None; }; - if is_symbol_iterator_key(&computed.expr) - || is_inspect_custom_key(ctx, &computed.expr) - || symbol_well_known_key(&computed.expr).is_some() - { + let well_known = symbol_well_known_key(&computed.expr); + let needs_special_lowering = + (well_known == Some("iterator") && method.function.is_generator && !method.is_static) + || (well_known == Some("hasInstance") + && method.is_static + && matches!(method.kind, ast::MethodKind::Method)) + || (well_known == Some("toStringTag") + && !method.is_static + && matches!(method.kind, ast::MethodKind::Getter)); + if needs_special_lowering { return None; } Some(computed) @@ -887,7 +893,16 @@ pub fn lower_class_decl( // consumers work (#5128). See the helper for details. if prop_name == "@@iterator" && func.is_generator && !method.is_static { let wrapper = synthesize_symbol_iterator_wrapper(ctx, &name, &mut func); - methods.push(wrapper); + let ast::PropName::Computed(computed) = &method.key else { + unreachable!("@@iterator generator key must be computed"); + }; + computed_members.push(ClassComputedMember { + key_expr: lower_expr(ctx, &computed.expr)?, + function: wrapper, + is_static: false, + kind: ClassComputedMemberKind::Method, + source_order: member_index, + }); continue; } if seen_generic_computed_member && can_source_order_register { @@ -1743,7 +1758,16 @@ pub fn lower_class_from_ast( // exactly as the class-declaration path does above. if prop_name == "@@iterator" && func.is_generator && !method.is_static { let wrapper = synthesize_symbol_iterator_wrapper(ctx, name, &mut func); - methods.push(wrapper); + let ast::PropName::Computed(computed) = &method.key else { + unreachable!("@@iterator generator key must be computed"); + }; + computed_members.push(ClassComputedMember { + key_expr: lower_expr(ctx, &computed.expr)?, + function: wrapper, + is_static: false, + kind: ClassComputedMemberKind::Method, + source_order: member_index, + }); continue; } if seen_generic_computed_member && can_source_order_register { diff --git a/crates/perry-hir/src/stable_hash/expr.rs b/crates/perry-hir/src/stable_hash/expr.rs index e6a518a110..a4b1391989 100644 --- a/crates/perry-hir/src/stable_hash/expr.rs +++ b/crates/perry-hir/src/stable_hash/expr.rs @@ -647,8 +647,8 @@ impl SH for Expr { Expr::RefreshClassExprCaptures { class_value, captures } => { tag(h, 12243); class_value.as_ref().hash(h); for c in captures { c.hash(h); } } Expr::ClassCaptureValue { class_name, index, fallback, prefer_fallback } => { tag(h, 12242); class_name.hash(h); index.hash(h); fallback.hash(h); prefer_fallback.hash(h); } Expr::RegisterClassStaticSymbol { class_name, key_expr, value_expr, } => { tag(h, 12025); class_name.hash(h); key_expr.as_ref().hash(h); value_expr.as_ref().hash(h); } - Expr::RegisterClassComputedMethod { class_name, key_expr, method_name, is_static, param_count, has_rest } => { tag(h, 12233); class_name.hash(h); key_expr.as_ref().hash(h); method_name.hash(h); is_static.hash(h); param_count.hash(h); has_rest.hash(h); } - Expr::RegisterClassComputedAccessor { class_name, key_expr, getter_name, setter_name, is_static } => { tag(h, 12234); class_name.hash(h); key_expr.as_ref().hash(h); getter_name.hash(h); setter_name.hash(h); is_static.hash(h); } + Expr::RegisterClassComputedMethod { class_name, key_expr, method_name, is_static, param_count, has_rest, definition_order } => { tag(h, 12233); class_name.hash(h); key_expr.as_ref().hash(h); method_name.hash(h); is_static.hash(h); param_count.hash(h); has_rest.hash(h); definition_order.hash(h); } + Expr::RegisterClassComputedAccessor { class_name, key_expr, getter_name, setter_name, is_static, definition_order } => { tag(h, 12234); class_name.hash(h); key_expr.as_ref().hash(h); getter_name.hash(h); setter_name.hash(h); is_static.hash(h); definition_order.hash(h); } Expr::ClassExprFresh { template, evaluation_owner, named_statics, computed_keys, computed_statics, static_init_order, captured_args, } => { tag(h, 12026); template.hash(h); evaluation_owner.hash(h); for (n, v) in named_statics { n.hash(h); v.hash(h); } for (n, k) in computed_keys { n.hash(h); k.hash(h); } for (n, v) in computed_statics { n.hash(h); v.hash(h); } for step in static_init_order { match step { ClassFreshStaticInit::Named(index) => { tag(h, 0); index.hash(h); }, ClassFreshStaticInit::Computed(index) => { tag(h, 1); index.hash(h); }, ClassFreshStaticInit::Block(index) => { tag(h, 2); index.hash(h); }, } } for a in captured_args { a.hash(h); } } Expr::SetFunctionPrototype { func, proto } => { tag(h, 448); func.as_ref().hash(h); proto.as_ref().hash(h); } Expr::RegisterPrototypeMethod { class_name, method_name, value, } => { tag(h, 463); class_name.hash(h); method_name.hash(h); value.as_ref().hash(h); } diff --git a/crates/perry-runtime/src/object/class_image.rs b/crates/perry-runtime/src/object/class_image.rs index 08d4a050f1..2d14997426 100644 --- a/crates/perry-runtime/src/object/class_image.rs +++ b/crates/perry-runtime/src/object/class_image.rs @@ -93,6 +93,11 @@ pub type StaticMethodTable = PtrHashMap /// class_id -> { name -> (getter func_ptr, setter func_ptr) } for static accessors. /// Outer map fast-hashed, inner `String`-keyed map deliberately not — see above. pub type StaticAccessorTable = PtrHashMap>; +/// `(class_id, is_static, property_name) -> source-order token` for declared +/// string-keyed methods and accessors. The token is the member function's HIR +/// id, which is allocated while walking the class body and therefore orders +/// entries across the otherwise separate method/getter/setter registries. +pub type StringMemberOrderTable = HashMap<(u32, bool, String), u32>; /// class_id -> (ctor func_ptr, total param count, signature capture count). pub type ConstructorTable = PtrHashMap; /// class_id -> (has_synthetic_arguments, has_rest) for a registered constructor. @@ -122,6 +127,7 @@ pub struct ClassImageTables { pub(crate) vtables: RwLock>>, pub(crate) static_methods: RwLock>, pub(crate) static_accessors: RwLock>, + pub(crate) string_member_orders: RwLock>, pub(crate) method_bind_lengths: RwLock>>, pub(crate) static_method_bind_lengths: RwLock>>, pub(crate) registered_class_ids: RwLock>>, @@ -150,6 +156,7 @@ impl ClassImageTables { vtables: RwLock::new(None), static_methods: RwLock::new(None), static_accessors: RwLock::new(None), + string_member_orders: RwLock::new(None), method_bind_lengths: RwLock::new(None), static_method_bind_lengths: RwLock::new(None), registered_class_ids: RwLock::new(None), diff --git a/crates/perry-runtime/src/object/class_registry.rs b/crates/perry-runtime/src/object/class_registry.rs index 9ac3cbcf4b..cf0a399e62 100644 --- a/crates/perry-runtime/src/object/class_registry.rs +++ b/crates/perry-runtime/src/object/class_registry.rs @@ -65,8 +65,9 @@ pub(crate) use state::{ class_decl_prototype_value_for_instance_class, class_delete_own_dynamic_prop, class_dynamic_prop_root_store, class_has_own_dynamic_prop, class_id_for_decl_prototype_object, class_is_key_deleted, class_mark_key_deleted, class_object_value_for_cid, - class_object_value_root_store, class_own_enumerable_field_names, class_own_static_field_value, - class_parent_closure, class_parent_closure_root_store, class_prototype_method_is_enumerable, + class_object_value_root_store, class_own_dynamic_prop_names, class_own_enumerable_field_names, + class_own_static_field_value, class_own_string_member_names, class_parent_closure, + class_parent_closure_root_store, class_prototype_method_is_enumerable, class_prototype_method_set_enumerable, class_prototype_method_value_cache_root_store, class_prototype_object_root_store, class_static_defined_attrs, class_static_prototype, class_static_prototype_is_nulled, class_static_prototype_root_clear, @@ -80,8 +81,8 @@ pub use state::{ CLASS_METHOD_BIND_LENGTHS, CLASS_OBJECT_VALUES, CLASS_PARENT_CLOSURES, CLASS_PROTOTYPE_METHOD_NONENUM, CLASS_PROTOTYPE_OBJECTS, CLASS_STATIC_ACCESSORS, CLASS_STATIC_METHODS, CLASS_STATIC_METHOD_BIND_LENGTHS, CLASS_STATIC_PROTOTYPES, - CLASS_SYMBOL_ACCESSORS, CLASS_SYMBOL_METHODS, CLASS_VTABLE_REGISTRY, FUNCTION_CLASS_IDS, - REGISTERED_CLASS_IDS, + CLASS_STRING_MEMBER_ORDERS, CLASS_SYMBOL_ACCESSORS, CLASS_SYMBOL_MEMBER_ORDERS, + CLASS_SYMBOL_METHODS, CLASS_VTABLE_REGISTRY, FUNCTION_CLASS_IDS, REGISTERED_CLASS_IDS, }; // ── prototype_objects.rs ──────────────────────────────────────────────────── @@ -166,12 +167,13 @@ pub(crate) use gc_roots::{ // ── registration.rs ───────────────────────────────────────────────────────── pub(crate) use registration::{ class_accessor_function_value, class_own_accessor_ptrs, class_own_static_accessor_ptrs, + invalidate_class_string_member_order, }; pub use registration::{ is_class_id_registered, js_register_class_getter, js_register_class_method, js_register_class_method_bind_length, js_register_class_setter, js_register_class_static_getter, js_register_class_static_method_bind_length, - js_register_class_static_setter, + js_register_class_static_setter, js_register_class_string_member_order, }; // ── dispatch.rs ───────────────────────────────────────────────────────────── @@ -190,14 +192,14 @@ pub(crate) use parent_static::{ call_private_static_method_for_owner, call_registered_static_method, call_static_method, class_chain_has_instance_accessor, class_dynamic_static_accessor_descriptor, class_dynamic_static_accessor_getter_value, class_has_instance_getter, - class_has_own_static_method, class_has_symbol_member_in_chain, class_instance_setter_apply, - class_method_bind_length, class_object_own_field_bytes, class_object_pinned_parent, - class_own_symbol_accessor_ptrs, class_own_symbol_member_keys, class_own_symbol_method, - class_private_instance_getter_value, class_private_instance_setter_apply, - class_static_accessor_getter_value, class_static_accessor_setter_apply, - class_symbol_getter_value, class_symbol_setter_apply, get_parent_class_id, - lookup_class_symbol_method_in_chain, lookup_static_method_in_chain, register_class, - register_class_dynamic_static_accessor, + class_has_own_static_method, class_has_own_symbol_member, class_has_symbol_member_in_chain, + class_instance_setter_apply, class_method_bind_length, class_object_own_field_bytes, + class_object_pinned_parent, class_own_symbol_accessor_ptrs, class_own_symbol_member_keys, + class_own_symbol_method, class_private_instance_getter_value, + class_private_instance_setter_apply, class_static_accessor_getter_value, + class_static_accessor_setter_apply, class_symbol_getter_value, class_symbol_setter_apply, + get_parent_class_id, lookup_class_symbol_method_in_chain, lookup_static_method_in_chain, + register_class, register_class_dynamic_static_accessor, }; pub use parent_static::{ is_class_object_ptr, is_class_object_value, is_registered_class_prototype_object, diff --git a/crates/perry-runtime/src/object/class_registry/gc_roots.rs b/crates/perry-runtime/src/object/class_registry/gc_roots.rs index 40882a0cb7..228e6de610 100644 --- a/crates/perry-runtime/src/object/class_registry/gc_roots.rs +++ b/crates/perry-runtime/src/object/class_registry/gc_roots.rs @@ -653,6 +653,7 @@ pub(crate) fn test_clear_class_side_table_roots() { // and `use crate::object::*`; name the canonical definition explicitly. use super::state::CLASS_DELETED_KEYS; CLASS_DYNAMIC_PROPS.with(|m| m.borrow_mut().clear()); + crate::object::CLASS_DYNAMIC_PROP_ORDER.with(|order| order.borrow_mut().clear()); CLASS_DELETED_KEYS.with(|m| m.borrow_mut().clear()); CLASS_PROTOTYPE_METHOD_VALUES.with(|cache| cache.borrow_mut().clear()); CLASS_PROTOTYPE_METHODS.with(|table| { @@ -700,6 +701,11 @@ pub(crate) fn test_clear_class_side_table_roots() { *guard = None; } }); + CLASS_SYMBOL_MEMBER_ORDERS.with(|table| { + if let Ok(mut guard) = table.write() { + *guard = None; + } + }); // The static-accessor table is deliberately NOT cleared here: it holds // code addresses, not heap pointers, so it is not a root, and since #8546 // it lives in the calling thread's class image (`object/class_image.rs`) diff --git a/crates/perry-runtime/src/object/class_registry/parent_static.rs b/crates/perry-runtime/src/object/class_registry/parent_static.rs index db98d25382..2f8b0e2995 100644 --- a/crates/perry-runtime/src/object/class_registry/parent_static.rs +++ b/crates/perry-runtime/src/object/class_registry/parent_static.rs @@ -553,6 +553,7 @@ pub unsafe extern "C" fn js_register_class_computed_method( param_count: i64, is_static: i64, has_rest: i64, + definition_order: i64, ) { if class_id == 0 || func_ptr == 0 { return; @@ -565,6 +566,12 @@ pub unsafe extern "C" fn js_register_class_computed_method( return; } crate::symbol::note_symbol_key_installed(sym_key); + super::registration::record_class_symbol_member_order( + class_id, + sym_key, + is_static != 0, + definition_order as u32, + ); CLASS_SYMBOL_METHODS.with(|table| { let mut guard = table.write().unwrap(); if guard.is_none() { @@ -606,6 +613,10 @@ pub unsafe extern "C" fn js_register_class_computed_method( } else { None } + }) + .or_else(|| { + (sym_key == crate::symbol::inspect_custom_symbol_ptr()) + .then_some("__perry_inspect_custom__") }); if let Some(method_name) = alias { let mut registry = CLASS_VTABLE_REGISTRY.write().unwrap(); @@ -639,6 +650,12 @@ pub unsafe extern "C" fn js_register_class_computed_method( Some(name) => name, None => return, }; + super::registration::record_class_string_member_order( + class_id, + name.clone(), + is_static != 0, + definition_order as u32, + ); if is_static != 0 && name == "prototype" { throw_object_type_error(b"Classes may not have a static property named 'prototype'"); } @@ -694,6 +711,7 @@ pub unsafe extern "C" fn js_register_class_computed_accessor( getter_ptr: i64, setter_ptr: i64, is_static: i64, + definition_order: i64, ) { if class_id == 0 || (getter_ptr == 0 && setter_ptr == 0) { return; @@ -706,6 +724,12 @@ pub unsafe extern "C" fn js_register_class_computed_accessor( return; } crate::symbol::note_symbol_key_installed(sym_key); + super::registration::record_class_symbol_member_order( + class_id, + sym_key, + is_static != 0, + definition_order as u32, + ); CLASS_SYMBOL_ACCESSORS.with(|table| { let mut guard = table.write().unwrap(); if guard.is_none() { @@ -727,6 +751,12 @@ pub unsafe extern "C" fn js_register_class_computed_accessor( return; } if let Some(name) = property_key_string(property_key) { + super::registration::record_class_string_member_order( + class_id, + name.clone(), + is_static != 0, + definition_order as u32, + ); if is_static != 0 && name == "prototype" { throw_object_type_error(b"Classes may not have a static property named 'prototype'"); } @@ -917,10 +947,18 @@ pub(crate) fn class_own_symbol_member_keys(class_id: u32, is_static: bool) -> Ve }); keys.sort_by_key(|sym_key| unsafe { let ptr = *sym_key as *const crate::symbol::SymbolHeader; - if ptr.is_null() { - u64::MAX + let symbol_id = if ptr.is_null() { u64::MAX } else { (*ptr).id }; + let definition_order = CLASS_SYMBOL_MEMBER_ORDERS.with(|orders| { + orders.read().ok().and_then(|guard| { + guard + .as_ref() + .and_then(|map| map.get(&(class_id, symbol_id, is_static)).copied()) + }) + }); + if let Some(order) = definition_order { + (0u8, order, symbol_id) } else { - (*ptr).id + (1u8, u32::MAX, symbol_id) } }); keys diff --git a/crates/perry-runtime/src/object/class_registry/parent_static/private_and_dynamic.rs b/crates/perry-runtime/src/object/class_registry/parent_static/private_and_dynamic.rs index 057e82f073..4b74d9ed05 100644 --- a/crates/perry-runtime/src/object/class_registry/parent_static/private_and_dynamic.rs +++ b/crates/perry-runtime/src/object/class_registry/parent_static/private_and_dynamic.rs @@ -28,6 +28,11 @@ pub(crate) fn class_own_symbol_accessor_ptrs( }) } +pub(crate) fn class_has_own_symbol_member(class_id: u32, sym_key: usize, is_static: bool) -> bool { + class_own_symbol_method(class_id, sym_key, is_static).is_some() + || class_own_symbol_accessor_ptrs(class_id, sym_key, is_static).is_some() +} + fn dynamic_static_accessor_key(name: &str) -> String { let mut key = String::with_capacity(name.len() + 24); key.push('\0'); diff --git a/crates/perry-runtime/src/object/class_registry/registration.rs b/crates/perry-runtime/src/object/class_registry/registration.rs index 8ed1d8ac2d..a0158f6a92 100644 --- a/crates/perry-runtime/src/object/class_registry/registration.rs +++ b/crates/perry-runtime/src/object/class_registry/registration.rs @@ -32,6 +32,110 @@ pub fn is_class_id_registered(class_id: u32) -> bool { .unwrap_or(false) } +pub(crate) fn record_class_string_member_order( + class_id: u32, + name: String, + is_static: bool, + definition_order: u32, +) { + if class_id == 0 { + return; + } + let mut guard = match CLASS_STRING_MEMBER_ORDERS.write() { + Ok(guard) => guard, + Err(_) => return, + }; + if guard.is_none() { + *guard = Some(HashMap::new()); + } + guard + .as_mut() + .unwrap() + .entry((class_id, is_static, name)) + // A later getter/setter or duplicate method redefines the existing + // property without moving it in [[OwnPropertyKeys]]. + .and_modify(|order| *order = (*order).min(definition_order)) + .or_insert(definition_order); +} + +/// A configurable class element that is deleted and later recreated is a new +/// property and must move to the end of its string-key partition. Keep the +/// dispatch registry entry for fast calls, but retire its declaration-order +/// position until a future class evaluation explicitly registers it again. +pub(crate) fn invalidate_class_string_member_order(class_id: u32, name: &str, is_static: bool) { + if class_id == 0 { + return; + } + let mut guard = match CLASS_STRING_MEMBER_ORDERS.write() { + Ok(guard) => guard, + Err(_) => return, + }; + if guard.is_none() { + *guard = Some(HashMap::new()); + } + guard + .as_mut() + .unwrap() + .insert((class_id, is_static, name.to_string()), u32::MAX); +} + +pub(crate) unsafe fn record_class_symbol_member_order( + class_id: u32, + sym_key: usize, + is_static: bool, + definition_order: u32, +) { + let symbol_id = (*(sym_key as *const crate::symbol::SymbolHeader)).id; + CLASS_SYMBOL_MEMBER_ORDERS.with(|orders| { + let mut guard = orders.write().unwrap(); + if guard.is_none() { + *guard = Some(HashMap::new()); + } + guard + .as_mut() + .unwrap() + .entry((class_id, symbol_id, is_static)) + .and_modify(|order| *order = (*order).min(definition_order)) + .or_insert(definition_order); + }); +} + +/// Record the ClassBody position of a non-computed string-keyed method or +/// accessor. Codegen emits this beside the existing dispatch registration; +/// computed keys record the same metadata after runtime ToPropertyKey. +#[no_mangle] +pub unsafe extern "C" fn js_register_class_string_member_order( + class_id: i64, + name_ptr: *const u8, + name_len: i64, + is_static: i64, + definition_order: i64, +) { + if class_id <= 0 || name_ptr.is_null() || name_len < 0 { + return; + } + let Ok(name) = std::str::from_utf8(std::slice::from_raw_parts(name_ptr, name_len as usize)) + else { + return; + }; + record_class_string_member_order( + class_id as u32, + name.to_string(), + is_static != 0, + definition_order as u32, + ); +} + +#[cfg(feature = "keepalive-anchors")] +#[used] +static KEEP_REGISTER_CLASS_STRING_MEMBER_ORDER: unsafe extern "C" fn( + i64, + *const u8, + i64, + i64, + i64, +) = js_register_class_string_member_order; + /// Register a class method in the vtable registry. /// Called at startup from the init function for every class method/getter. #[no_mangle] diff --git a/crates/perry-runtime/src/object/class_registry/state.rs b/crates/perry-runtime/src/object/class_registry/state.rs index 73516022de..3162193b7a 100644 --- a/crates/perry-runtime/src/object/class_registry/state.rs +++ b/crates/perry-runtime/src/object/class_registry/state.rs @@ -1,6 +1,8 @@ use super::decl_prototype_table::DeclPrototypeTable; use super::*; -use crate::object::class_image::{ImageTable, StaticAccessorTable, StaticMethodTable}; +use crate::object::class_image::{ + ImageTable, StaticAccessorTable, StaticMethodTable, StringMemberOrderTable, +}; use std::collections::HashMap; use std::sync::RwLock; @@ -100,10 +102,21 @@ pub(crate) fn class_dynamic_prop_root_store(class_id: u32, name: &str, value: f6 }); } CLASS_DYNAMIC_PROPS.with(|m| { - m.borrow_mut() + let created = m + .borrow_mut() .entry(class_id) .or_default() - .insert(name.to_string(), value); + .insert(name.to_string(), value) + .is_none(); + if created { + crate::object::CLASS_DYNAMIC_PROP_ORDER.with(|order| { + order + .borrow_mut() + .entry(class_id) + .or_default() + .push(name.to_string()); + }); + } }); crate::gc::runtime_write_barrier_root_nanbox(value.to_bits()); } @@ -129,22 +142,37 @@ pub(crate) fn class_own_static_field_value(class_id: u32, name: &str) -> Option< /// here too (never reflectable). Returned unsorted; the caller applies ECMA /// ordering. (test262 class/elements static-field-declaration & friends.) pub(crate) fn class_own_enumerable_field_names(class_id: u32) -> Vec { - CLASS_DYNAMIC_PROPS.with(|m| { - m.borrow() - .get(&class_id) - .map(|props| { - props - .keys() - .filter(|k| !crate::object::is_internal_runtime_key(k)) - // #7190: a key installed by `Object.defineProperty` without - // `enumerable: true` shares this table with static fields - // but is NOT enumerable. - .filter(|k| !class_static_key_is_non_enumerable(class_id, k)) - .cloned() - .collect() - }) - .unwrap_or_default() - }) + class_own_dynamic_prop_names(class_id) + .into_iter() + // #7190: a key installed by `Object.defineProperty` without + // `enumerable: true` shares this table with static fields but is NOT + // enumerable. + .filter(|key| !class_static_key_is_non_enumerable(class_id, key)) + .collect() +} + +pub(crate) fn class_own_dynamic_prop_names(class_id: u32) -> Vec { + let mut names = crate::object::CLASS_DYNAMIC_PROP_ORDER + .with(|order| order.borrow().get(&class_id).cloned().unwrap_or_default()); + CLASS_DYNAMIC_PROPS.with(|props| { + let props = props.borrow(); + let Some(props) = props.get(&class_id) else { + names.clear(); + return; + }; + names.retain(|name| props.contains_key(name)); + // Registries populated by older/native paths may predate the order + // side table. Keep those visible with a deterministic fallback. + let mut missing: Vec = props + .keys() + .filter(|name| !names.contains(name)) + .cloned() + .collect(); + missing.sort(); + names.extend(missing); + }); + names.retain(|key| !crate::object::is_internal_runtime_key(key)); + names } /// #7190: record a `defineProperty`-installed static key's attributes. Called @@ -196,6 +224,11 @@ pub(crate) fn class_delete_own_dynamic_prop(class_id: u32, name: &str) { props.remove(name); } }); + crate::object::CLASS_DYNAMIC_PROP_ORDER.with(|order| { + if let Some(names) = order.borrow_mut().get_mut(&class_id) { + names.retain(|existing| existing != name); + } + }); } pub(crate) fn class_prototype_method_value_cache_root_store( @@ -260,6 +293,12 @@ pub static CLASS_STATIC_METHODS: ImageTable>> = pub static CLASS_STATIC_ACCESSORS: ImageTable>> = ImageTable::new(|image| &image.static_accessors); +/// Source order for public class methods/accessors. Dispatch data lives in +/// separate hash maps by member kind; keeping this metadata alongside the +/// image lets [[OwnPropertyKeys]] reconstruct the single ClassBody order. +pub static CLASS_STRING_MEMBER_ORDERS: ImageTable>> = + ImageTable::new(|image| &image.string_member_orders); + /// Spec `Function.prototype.length` per (class_id, method/accessor name) — the /// count of formal parameters before the first one with a default or a rest. /// The vtable only records the *total* param count (needed for call dispatch), @@ -284,6 +323,12 @@ crate::perry_thread_local! { pub static CLASS_SYMBOL_ACCESSORS: RwLock>> = RwLock::new(None); + + /// Source order for Symbol-keyed class methods/accessors. Symbol member + /// registries are thread-local because their keys are heap addresses, so + /// their ordering metadata follows the same ownership model. + pub static CLASS_SYMBOL_MEMBER_ORDERS: RwLock>> = + RwLock::new(None); } /// Set of all registered class ids. Populated at module init by codegen @@ -751,8 +796,72 @@ pub(crate) fn class_decl_prototype_method_names(class_id: u32) -> Vec { names.extend(vtable.methods.keys().cloned()); } } + order_class_string_member_names(class_id, false, &mut names); + names +} + +fn internal_symbol_dispatch_alias(name: &str) -> bool { + matches!( + name, + "@@iterator" + | "@@asyncIterator" + | "@@toPrimitive" + | "__perry_dispose__" + | "__perry_async_dispose__" + | "__perry_inspect_custom__" + ) +} + +fn order_class_string_member_names(class_id: u32, is_static: bool, names: &mut Vec) { names.sort(); names.dedup(); + let orders = CLASS_STRING_MEMBER_ORDERS.read().ok(); + let order_for = |name: &str| { + orders + .as_ref() + .and_then(|guard| guard.as_ref()) + .and_then(|map| map.get(&(class_id, is_static, name.to_string())).copied()) + }; + // Synthetic names exist solely to keep Perry's string-based dispatch fast. + // A source method literally named "@@iterator" has an order registration + // and remains visible; an alias created for a Symbol method does not. + names.retain(|name| { + let order = order_for(name); + order != Some(u32::MAX) && (order.is_some() || !internal_symbol_dispatch_alias(name)) + }); + names.sort_by(|left, right| match (order_for(left), order_for(right)) { + (Some(a), Some(b)) => a.cmp(&b).then_with(|| left.cmp(right)), + (Some(_), None) => std::cmp::Ordering::Less, + (None, Some(_)) => std::cmp::Ordering::Greater, + (None, None) => left.cmp(right), + }); +} + +/// Own string-keyed methods and accessors in ClassBody definition order. +/// The dispatch registries intentionally remain hash maps; reflection is the +/// only consumer that needs their cross-kind ordering. +pub(crate) fn class_own_string_member_names(class_id: u32, is_static: bool) -> Vec { + let mut names = Vec::new(); + if is_static { + if let Ok(methods) = CLASS_STATIC_METHODS.read() { + if let Some(map) = methods.as_ref().and_then(|all| all.get(&class_id)) { + names.extend(map.keys().cloned()); + } + } + if let Ok(accessors) = CLASS_STATIC_ACCESSORS.read() { + if let Some(map) = accessors.as_ref().and_then(|all| all.get(&class_id)) { + names.extend(map.keys().cloned()); + } + } + } else if let Ok(registry) = CLASS_VTABLE_REGISTRY.read() { + if let Some(vtable) = registry.as_ref().and_then(|all| all.get(&class_id)) { + names.extend(vtable.methods.keys().cloned()); + names.extend(vtable.getters.keys().cloned()); + names.extend(vtable.setters.keys().cloned()); + } + } + names.retain(|name| !name.starts_with('#')); + order_class_string_member_names(class_id, is_static, &mut names); names } diff --git a/crates/perry-runtime/src/object/delete_rest.rs b/crates/perry-runtime/src/object/delete_rest.rs index 8145bed5a8..3507a09cee 100644 --- a/crates/perry-runtime/src/object/delete_rest.rs +++ b/crates/perry-runtime/src/object/delete_rest.rs @@ -72,6 +72,9 @@ pub extern "C" fn js_object_delete_field( if super::class_registry::class_name_for_id(class_id).is_some() { super::class_registry::class_delete_own_dynamic_prop(class_id, name); super::class_registry::class_mark_key_deleted(class_id, name); + super::class_registry::invalidate_class_string_member_order( + class_id, name, true, + ); } // #6363: a native HANDLE's own properties are its user expandos. // `delete` used to unconditionally report success while LEAVING @@ -295,6 +298,9 @@ pub extern "C" fn js_object_delete_field( .is_some()) { super::class_registry::class_mark_key_deleted(cid, name); + super::class_registry::invalidate_class_string_member_order( + cid, name, false, + ); super::class_registry::invalidate_class_prototype_fast_guards_for_method( name, ); @@ -721,6 +727,7 @@ fn delete_class_prototype_key(class_id: u32, name: &str) -> i32 { return 1; } super::class_registry::class_mark_key_deleted(class_id, name); + super::class_registry::invalidate_class_string_member_order(class_id, name, false); super::class_registry::invalidate_class_prototype_fast_guards_for_method(name); crate::typed_feedback::invalidate_method_change(class_id); 1 diff --git a/crates/perry-runtime/src/object/descriptors.rs b/crates/perry-runtime/src/object/descriptors.rs index 91e0b82353..ac983a3733 100644 --- a/crates/perry-runtime/src/object/descriptors.rs +++ b/crates/perry-runtime/src/object/descriptors.rs @@ -1382,72 +1382,17 @@ fn js_object_get_own_property_names_shape(obj_value: f64) -> f64 { "prototype".to_string(), ] }; - if let Ok(registry) = CLASS_VTABLE_REGISTRY.read() { - if let Some(reg) = registry.as_ref() { - if let Some(vtable) = reg.get(&class_id) { - if is_prototype_ref { - let mut method_names: Vec = - vtable.methods.keys().cloned().collect(); - method_names.sort(); - for name in method_names { - if !name.starts_with('#') { - push_unique_name(&mut names, name); - } - } - let mut getter_names: Vec = - vtable.getters.keys().cloned().collect(); - getter_names.sort(); - for name in getter_names { - if !name.starts_with('#') { - push_unique_name(&mut names, name); - } - } - let mut setter_names: Vec = - vtable.setters.keys().cloned().collect(); - setter_names.sort(); - for name in setter_names { - if !name.starts_with('#') { - push_unique_name(&mut names, name); - } - } - } - } + for name in + super::class_registry::class_own_string_member_names(class_id, !is_prototype_ref) + { + if !super::class_registry::class_is_key_deleted(class_id, &name) { + push_unique_name(&mut names, name); } } if !is_prototype_ref { - if let Ok(static_methods) = CLASS_STATIC_METHODS.read() { - if let Some(map) = static_methods.as_ref().and_then(|m| m.get(&class_id)) { - let mut method_names: Vec = map.keys().cloned().collect(); - method_names.sort(); - for name in method_names { - if !name.starts_with('#') { - push_unique_name(&mut names, name); - } - } - } - } - if let Ok(static_accessors) = CLASS_STATIC_ACCESSORS.read() { - if let Some(map) = static_accessors.as_ref().and_then(|m| m.get(&class_id)) { - let mut accessor_names: Vec = map.keys().cloned().collect(); - accessor_names.sort(); - for name in accessor_names { - if !name.starts_with('#') { - push_unique_name(&mut names, name); - } - } - } + for name in super::class_registry::class_own_dynamic_prop_names(class_id) { + push_unique_name(&mut names, name); } - CLASS_DYNAMIC_PROPS.with(|m| { - if let Some(props) = m.borrow().get(&class_id) { - let mut prop_names: Vec = props.keys().cloned().collect(); - prop_names.sort(); - for name in prop_names { - if !super::field_get_set::is_internal_runtime_key(&name) { - push_unique_name(&mut names, name); - } - } - } - }); } names.retain(|n| !super::field_get_set::is_internal_runtime_key(n)); sort_property_names_ecma(&mut names); @@ -1622,6 +1567,62 @@ fn js_object_get_own_property_names_shape(obj_value: f64) -> f64 { || crate::wasi::is_wasi_instance(f64::from_bits( crate::value::js_nanbox_pointer(obj as i64).to_bits(), )); + + // Declared class prototypes split their own properties between the + // ordinary keys array (constructor, methods, later expandos) and the + // class vtable (accessors). Rebuild their string half from both stores + // in ClassBody order. The physical-key membership check preserves + // deletion of mirrored methods/constructor; accessors have no physical + // slot, so their class deletion marker is authoritative. + if let Some(class_id) = + super::class_registry::class_id_for_decl_prototype_object(obj as usize) + { + let mut physical = Vec::new(); + let mut sso_buf = [0u8; crate::value::SHORT_STRING_MAX_LEN]; + for i in 0..len { + let key_val = crate::array::js_array_get(keys, pos(i)); + if key_val.bits() == crate::value::TAG_HOLE + || key_val.bits() == crate::value::TAG_UNDEFINED + { + continue; + } + let Some(bytes) = crate::string::js_string_key_bytes(key_val, &mut sso_buf) else { + continue; + }; + if super::field_get_set::is_internal_runtime_key_bytes(bytes) { + continue; + } + if let Ok(name) = std::str::from_utf8(bytes) { + push_unique_name(&mut physical, name.to_string()); + } + } + + let mut names = Vec::new(); + if physical.iter().any(|name| name == "constructor") { + names.push("constructor".to_string()); + } + for name in super::class_registry::class_own_string_member_names(class_id, false) { + if super::class_registry::class_is_key_deleted(class_id, &name) { + continue; + } + let is_accessor = + super::class_registry::class_own_accessor_ptrs(class_id, &name).is_some(); + if is_accessor || physical.contains(&name) { + push_unique_name(&mut names, name); + } + } + for name in physical { + push_unique_name(&mut names, name); + } + sort_property_names_ecma(&mut names); + let result = crate::array::js_array_alloc(names.len() as u32); + for name in names { + let key = crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32); + crate::array::js_array_push(result, JSValue::string_ptr(key)); + } + return f64::from_bits((result as u64) | 0x7FFD_0000_0000_0000); + } + let result = crate::array::js_array_alloc(len as u32); let mut sso_buf = [0u8; crate::value::SHORT_STRING_MAX_LEN]; for i in 0..len { diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index 3e88d17ba6..432ccf4520 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -598,6 +598,11 @@ crate::perry_thread_local! { /// this side-table keyed by class_id. pub(crate) static CLASS_DYNAMIC_PROPS: std::cell::RefCell>> = std::cell::RefCell::new(std::collections::HashMap::new()); + /// Property-creation order for `CLASS_DYNAMIC_PROPS`. The value table is a + /// HashMap for hot lookup, while [[OwnPropertyKeys]] needs first-insertion + /// order (with delete + re-add moving a key to the end). + pub(crate) static CLASS_DYNAMIC_PROP_ORDER: std::cell::RefCell>> = + std::cell::RefCell::new(std::collections::HashMap::new()); /// #7190: `(writable, enumerable)` for static own keys installed by /// `Object.defineProperty(C, k, desc)`. They live in `CLASS_DYNAMIC_PROPS` /// next to `static x = …` fields, which are writable AND enumerable by diff --git a/crates/perry-runtime/src/object/object_ops/has_own.rs b/crates/perry-runtime/src/object/object_ops/has_own.rs index 61eb98ec69..203ac1bd10 100644 --- a/crates/perry-runtime/src/object/object_ops/has_own.rs +++ b/crates/perry-runtime/src/object/object_ops/has_own.rs @@ -148,14 +148,31 @@ pub extern "C" fn js_object_has_own(obj_value: f64, key_value: f64) -> f64 { // Symbol-keyed lookup: route through SYMBOL_PROPERTIES side table. if crate::symbol::js_is_symbol(key_value) != 0 { - // ClassRef receivers carry class_id in the low 32 bits. - let bits = obj_value.to_bits(); - if (bits >> 48) == 0x7FFE { - let class_id = (bits & 0xFFFF_FFFF) as u32; - let present = - crate::symbol::class_static_symbol_lookup(class_id, key_value).is_some(); + let sym_key = crate::symbol::sym_key_from_f64(key_value); + if let Some(class_id) = super::super::class_ref_id(obj_value) { + let is_prototype = super::super::class_prototype_ref_id(obj_value).is_some(); + let present = if is_prototype { + super::super::class_registry::class_has_own_symbol_member( + class_id, sym_key, false, + ) + } else { + crate::symbol::class_static_symbol_lookup(class_id, key_value).is_some() + || super::super::class_registry::class_has_own_symbol_member( + class_id, sym_key, true, + ) + }; return f64::from_bits(if present { TAG_TRUE } else { TAG_FALSE }); } + let obj_key = crate::symbol::obj_key_from_f64(obj_value); + if let Some(class_id) = + super::super::class_registry::class_id_for_decl_prototype_object(obj_key) + { + if super::super::class_registry::class_has_own_symbol_member( + class_id, sym_key, false, + ) { + return f64::from_bits(TAG_TRUE); + } + } let present = crate::symbol::js_object_has_own_symbol(obj_value, key_value); return f64::from_bits(if present { TAG_TRUE } else { TAG_FALSE }); } diff --git a/crates/perry-runtime/src/object/property_key.rs b/crates/perry-runtime/src/object/property_key.rs index cb655f9d23..cada3adae4 100644 --- a/crates/perry-runtime/src/object/property_key.rs +++ b/crates/perry-runtime/src/object/property_key.rs @@ -677,6 +677,7 @@ mod property_key_tests { 0, 0, 0, + 1, ); let method = crate::object::class_registry::lookup_class_symbol_method_in_chain( class_id, sym_key, false, @@ -691,6 +692,7 @@ mod property_key_tests { computed_class_getter as *const () as usize as i64, 0, 0, + 2, ); let value = crate::object::class_registry::class_symbol_getter_value( class_id, diff --git a/crates/perry-runtime/src/symbol.rs b/crates/perry-runtime/src/symbol.rs index 087ca886d2..f7f98a65e1 100644 --- a/crates/perry-runtime/src/symbol.rs +++ b/crates/perry-runtime/src/symbol.rs @@ -1055,15 +1055,30 @@ pub(crate) static CLASS_STATIC_SYMBOLS_LATCH: crate::registry_latch::RegistryLat pub(crate) fn store_class_static_symbol_root(class_id: u32, sym_key: usize, value_bits: u64) { note_symbol_key_installed(sym_key); CLASS_STATIC_SYMBOLS_LATCH.arm(); + let symbol_id = unsafe { (*(sym_key as *const SymbolHeader)).id }; + let created; { let mut guard = crate::gc::lock_gc_root_registry(&CLASS_STATIC_SYMBOLS); if guard.is_none() { *guard = Some(HashMap::new()); } - guard + created = guard .as_mut() .unwrap() - .insert((class_id, sym_key), value_bits); + .insert((class_id, sym_key), value_bits) + .is_none(); + } + if created { + let mut order = CLASS_STATIC_SYMBOL_ORDER.lock().unwrap(); + if order.is_none() { + *order = Some(HashMap::new()); + } + order + .as_mut() + .unwrap() + .entry(class_id) + .or_default() + .push(symbol_id); } publish_symbol_side_table_root_edges(sym_key, value_bits); } @@ -1076,6 +1091,10 @@ per_test_global! { /// when the receiver is a class identifier (NaN-boxed INT32_TAG). /// Refs #420. static CLASS_STATIC_SYMBOLS: Mutex>> = Mutex::new(None); + + /// Symbol-id creation order for static symbol data properties. IDs are + /// stable across moving GC, unlike the pointer keys in the value table. + static CLASS_STATIC_SYMBOL_ORDER: Mutex>>> = Mutex::new(None); } #[cfg(test)] diff --git a/crates/perry-runtime/src/symbol/gc_roots.rs b/crates/perry-runtime/src/symbol/gc_roots.rs index 018bd8dbf8..8c05be3014 100644 --- a/crates/perry-runtime/src/symbol/gc_roots.rs +++ b/crates/perry-runtime/src/symbol/gc_roots.rs @@ -356,6 +356,7 @@ pub(crate) fn test_clear_symbol_side_table_roots() { *crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTIES) = None; *crate::gc::lock_gc_root_registry(&SYMBOL_PROPERTY_ATTRS) = None; *crate::gc::lock_gc_root_registry(&CLASS_STATIC_SYMBOLS) = None; + *CLASS_STATIC_SYMBOL_ORDER.lock().unwrap() = None; accessors::test_clear_symbol_accessor_roots(); let mut persistent = Vec::new(); @@ -420,7 +421,21 @@ pub(crate) fn test_symbol_property_owner_exists(owner: usize) -> bool { #[cfg(test)] pub(crate) fn test_seed_class_static_symbol_root(class_id: u32, sym_key: usize, value_bits: u64) { if class_id != 0 && sym_key != 0 { - store_class_static_symbol_root(class_id, sym_key, value_bits); + // Root-scanner tests deliberately use synthetic addresses, including + // an unaligned sentinel. Seed only the root table they exercise; + // production registration additionally reads SymbolHeader::id for + // [[OwnPropertyKeys]] ordering and therefore requires a real Symbol. + CLASS_STATIC_SYMBOLS_LATCH.arm(); + let mut guard = crate::gc::lock_gc_root_registry(&CLASS_STATIC_SYMBOLS); + if guard.is_none() { + *guard = Some(HashMap::new()); + } + guard + .as_mut() + .unwrap() + .insert((class_id, sym_key), value_bits); + drop(guard); + publish_symbol_side_table_root_edges(sym_key, value_bits); } } diff --git a/crates/perry-runtime/src/symbol/iterator.rs b/crates/perry-runtime/src/symbol/iterator.rs index b17e9604d8..d624aa587c 100644 --- a/crates/perry-runtime/src/symbol/iterator.rs +++ b/crates/perry-runtime/src/symbol/iterator.rs @@ -35,14 +35,6 @@ pub unsafe extern "C" fn js_object_get_own_property_symbols(obj_f64: f64) -> i64 keys.push(sym_key); } } - keys.sort_by_key(|sym_key| { - let ptr = *sym_key as *const SymbolHeader; - if ptr.is_null() { - u64::MAX - } else { - (*ptr).id - } - }); keys }; let mut arr = crate::array::js_array_alloc(entries.len() as u32); diff --git a/crates/perry-runtime/src/symbol/properties.rs b/crates/perry-runtime/src/symbol/properties.rs index e58feb0188..ddcbcf4f32 100644 --- a/crates/perry-runtime/src/symbol/properties.rs +++ b/crates/perry-runtime/src/symbol/properties.rs @@ -611,14 +611,27 @@ fn class_static_symbol_lookup_slow(class_id: u32, sym_f64: f64) -> Option { pub(crate) fn class_static_symbol_keys_for_class(class_id: u32) -> Vec { let guard = crate::gc::lock_gc_root_registry(&CLASS_STATIC_SYMBOLS); - guard + let mut keys: Vec = guard .as_ref() .map(|map| { map.keys() .filter_map(|&(cid, sym_key)| (cid == class_id).then_some(sym_key)) .collect() }) - .unwrap_or_default() + .unwrap_or_default(); + drop(guard); + let order = CLASS_STATIC_SYMBOL_ORDER.lock().unwrap(); + keys.sort_by_key(|sym_key| unsafe { + let symbol_id = (*sym_key as *const SymbolHeader) + .as_ref() + .map_or(u64::MAX, |symbol| symbol.id); + let position = order + .as_ref() + .and_then(|all| all.get(&class_id)) + .and_then(|ids| ids.iter().position(|id| *id == symbol_id)); + (position.unwrap_or(usize::MAX), symbol_id) + }); + keys } /// `Object.prototype.hasOwnProperty.call(obj, sym)` for Symbol keys. diff --git a/test-files/test_gap_9226_class_prototype_own_keys.ts b/test-files/test_gap_9226_class_prototype_own_keys.ts new file mode 100644 index 0000000000..ef6f39330d --- /dev/null +++ b/test-files/test_gap_9226_class_prototype_own_keys.ts @@ -0,0 +1,123 @@ +// #9226: declared class prototypes must expose one coherent +// [[OwnPropertyKeys]] surface. Accessors used to be absent from the string +// half, the internal `@@iterator` dispatch alias leaked as a string key, and +// the real Symbol.iterator key was absent from the symbol half. +// +// Keep the order visible: integer-index strings first (ascending), then other +// strings in property-creation order, then symbols in property-creation order. + +const baseSymbol = Symbol("base"); +const ownSymbol = Symbol("own"); +const staticSymbol = Symbol("static"); + +function renderKey(key: PropertyKey): string { + return typeof key === "symbol" ? String(key) : key; +} + +function renderKeys(keys: PropertyKey[]): string { + return keys.map(renderKey).join("|"); +} + +function sameKeys(left: PropertyKey[], right: PropertyKey[]): boolean { + if (left.length !== right.length) return false; + for (let i = 0; i < left.length; i++) { + if (left[i] !== right[i]) return false; + } + return true; +} + +function dumpOwnKeys(label: string, value: object, skipFunctionIntrinsics = false): void { + const names = Object.getOwnPropertyNames(value); + const symbols = Object.getOwnPropertySymbols(value); + const reflected = Reflect.ownKeys(value); + const reflectedNames = reflected.filter((key: PropertyKey) => typeof key === "string"); + const reflectedSymbols = reflected.filter((key: PropertyKey) => typeof key === "symbol"); + + console.log(label + ".names=" + renderKeys(names)); + console.log(label + ".symbols=" + renderKeys(symbols)); + console.log(label + ".reflect=" + renderKeys(reflected)); + console.log( + label + ".partition=" + + sameKeys(names, reflectedNames) + "," + + sameKeys(symbols, reflectedSymbols) + "," + + (reflected.length === names.length + symbols.length), + ); + + // The consistency triple: every reported key must be an own property and + // must have an own descriptor. This catches a symbol-side hasOwn mismatch + // independently of list contents. Class-constructor length/name/prototype + // descriptor parity is tracked separately from #9226, so the static-member + // pass deliberately checks every user key while leaving those three alone. + for (const key of reflected) { + if ( + skipFunctionIntrinsics && + (key === "length" || key === "name" || key === "prototype") + ) continue; + console.log( + label + ".key=" + renderKey(key) + ":" + + Object.prototype.hasOwnProperty.call(value, key) + "," + + (Object.getOwnPropertyDescriptor(value, key) !== undefined), + ); + } +} + +class Base { + baseMethod(): string { return "base"; } + get baseGet(): number { return 1; } + set baseSet(_value: number) {} + [baseSymbol](): string { return "base-symbol"; } +} + +class Subject extends Base { + zed(): string { return "zed"; } + get alpha(): number { return 2; } + set charlie(_value: number) {} + get pair(): number { return 3; } + set pair(_value: number) {} + 10(): string { return "ten"; } + 2(): string { return "two"; } + [Symbol.iterator](): any { return [][Symbol.iterator](); } + [ownSymbol](): string { return "own-symbol"; } + yankee(): string { return "yankee"; } + + static staticZed(): string { return "static-zed"; } + static get staticAlpha(): number { return 4; } + static set staticCharlie(_value: number) {} + static get staticPair(): number { return 5; } + static set staticPair(_value: number) {} + static [Symbol.iterator](): any { return [][Symbol.iterator](); } + static [staticSymbol](): string { return "static-symbol"; } + static staticYankee(): string { return "static-yankee"; } +} + +dumpOwnKeys("base.prototype", Base.prototype); +dumpOwnKeys("subject.prototype", Subject.prototype); +dumpOwnKeys("subject.static", Subject, true); + +// Accessors are ordinary own properties even though their values live in +// getter/setter slots rather than in data fields. +for (const key of ["alpha", "charlie", "pair"]) { + const descriptor = Object.getOwnPropertyDescriptor(Subject.prototype, key); + console.log( + "accessor." + key + "=" + + Object.prototype.hasOwnProperty.call(Subject.prototype, key) + "," + + (descriptor !== undefined) + "," + + (descriptor !== undefined && typeof descriptor.get === "function") + "," + + (descriptor !== undefined && typeof descriptor.set === "function"), + ); +} + +// Inherited members remain discoverable through ordinary lookup, but are not +// own keys of the subclass prototype. +for (const key of ["baseMethod", "baseGet", "baseSet"]) { + console.log( + "inherited." + key + "=" + + Object.prototype.hasOwnProperty.call(Subject.prototype, key) + "," + + (Object.getOwnPropertyDescriptor(Subject.prototype, key) !== undefined), + ); +} +console.log( + "inherited.symbol=" + + Object.prototype.hasOwnProperty.call(Subject.prototype, baseSymbol) + "," + + (Object.getOwnPropertyDescriptor(Subject.prototype, baseSymbol) !== undefined), +);