diff --git a/changelog.d/9320-train16-followup.md b/changelog.d/9320-train16-followup.md new file mode 100644 index 0000000000..7ba724cc42 --- /dev/null +++ b/changelog.d/9320-train16-followup.md @@ -0,0 +1,14 @@ +### Internal + +- **Keeps `class_decl.rs` under the 2000-line file gate.** #9315 landed on a + file already at 1989 lines. The non-computed member registration and + `Symbol.iterator` wrapper helpers move to a sibling module unchanged. + +- **Classifies two order side-tables in the root-holder inventory.** + `CLASS_SYMBOL_MEMBER_ORDERS` is keyed by `SymbolHeader::id` — a stable id read + once at registration, not a heap address, so it needs no re-key when a Symbol + is evacuated — with a `u32` order value. `CLASS_DYNAMIC_PROP_ORDER` holds + owned Rust strings. Neither stores a JSValue, so neither is a GC root. + +- **Drops unnecessary parens in the numeric-range header read**, which + `-D warnings` treats as an error. diff --git a/crates/perry-hir/src/lower_decl/class_decl.rs b/crates/perry-hir/src/lower_decl/class_decl.rs index 3b883aa819..520711adbb 100644 --- a/crates/perry-hir/src/lower_decl/class_decl.rs +++ b/crates/perry-hir/src/lower_decl/class_decl.rs @@ -41,7 +41,9 @@ fn is_genuine_node_stream_parent(ctx: &LoweringContext, name: &str) -> bool { } mod class_heritage; +mod member_registration; use class_heritage::*; +use member_registration::*; use super::*; @@ -183,128 +185,6 @@ fn noncomputed_member_registration_name( format!("{}_{}_{}", base, method.span.lo.0, method.span.hi.0) } -fn lower_noncomputed_class_member_registration( - ctx: &mut LoweringContext, - method: &ast::ClassMethod, - prop_name: &str, - source_order: usize, -) -> Result { - let function_name = noncomputed_member_registration_name(method.kind, method); - let (kind, function) = match method.kind { - ast::MethodKind::Method => ( - ClassComputedMemberKind::Method, - with_static_member_context(ctx, method.is_static, |ctx| { - lower_class_method_with_name(ctx, method, function_name) - })?, - ), - ast::MethodKind::Getter => ( - ClassComputedMemberKind::Getter, - with_static_member_context(ctx, method.is_static, |ctx| { - lower_getter_method_with_name(ctx, method, function_name) - })?, - ), - ast::MethodKind::Setter => ( - ClassComputedMemberKind::Setter, - with_static_member_context(ctx, method.is_static, |ctx| { - lower_setter_method_with_name(ctx, method, function_name) - })?, - ), - }; - Ok(ClassComputedMember { - key_expr: Expr::String(prop_name.to_string()), - function, - is_static: method.is_static, - kind, - source_order, - }) -} - -/// Lower a generator `*[Symbol.iterator]()` class method (already lowered into -/// `func`, named `@@iterator`) into the runtime `@@iterator` vtable entry. -/// -/// The body is lifted to a top-level `__perry_iter_` generator with -/// `this` as an explicit first parameter — the generator transform (which only -/// visits `module.functions`) then rewrites it to the `{next, return, throw}` -/// closure triple, and the syntactic `for…of` fast path dispatches to it -/// directly via `iterator_func_for_class`. -/// -/// But every *runtime*-dispatched iterator consumer (spread `[...x]`, -/// `Math.max(...x)`, destructuring, `x[Symbol.iterator]()`, `Array.from`) -/// resolves `@@iterator` through the class registry instead. So this also -/// returns a synthetic NON-generator `@@iterator` wrapper method that forwards -/// to the lifted generator (`return __perry_iter_X(this)`) for the caller to -/// append to the instance vtable. Without it the class carries no `@@iterator` -/// for those consumers to find and they throw "value is not iterable" (#5128). -/// (The runtime maps the well-known `Symbol.iterator` to this `@@iterator` -/// method name in `js_object_get_symbol_property`.) -/// -/// Shared by `lower_class_decl` and `lower_class_from_ast` so class -/// declarations and class expressions behave identically. -fn synthesize_symbol_iterator_wrapper( - ctx: &mut LoweringContext, - class_name: &str, - func: &mut Function, -) -> Function { - let this_id = ctx.fresh_local(); - let mut new_params = Vec::with_capacity(func.params.len() + 1); - new_params.push(Param { - id: this_id, - name: "this".to_string(), - ty: Type::Named(class_name.to_string()), - default: None, - decorators: Vec::new(), - is_rest: false, - arguments_object: None, - }); - new_params.append(&mut func.params); - - let mut body = std::mem::take(&mut func.body); - crate::analysis::replace_this_in_stmts(&mut body, this_id); - - let top_fn_id = ctx.fresh_func(); - let top_fn = Function { - id: top_fn_id, - name: format!("__perry_iter_{}", class_name), - type_params: Vec::new(), - params: new_params, - return_type: Type::Any, - body, - is_async: false, - is_generator: true, - is_strict: true, - was_plain_async: false, - was_unrolled: false, - is_exported: false, - captures: Vec::new(), - decorators: Vec::new(), - }; - ctx.pending_functions.push(top_fn); - ctx.iterator_func_for_class - .insert(class_name.to_string(), top_fn_id); - - Function { - id: ctx.fresh_func(), - name: "@@iterator".to_string(), - type_params: Vec::new(), - params: Vec::new(), - return_type: Type::Any, - body: vec![Stmt::Return(Some(Expr::Call { - callee: Box::new(Expr::FuncRef(top_fn_id)), - args: vec![Expr::This], - type_args: Vec::new(), - byte_offset: 0, - }))], - is_async: false, - is_generator: false, - is_strict: true, - was_plain_async: false, - was_unrolled: false, - is_exported: false, - captures: Vec::new(), - decorators: Vec::new(), - } -} - pub fn lower_class_decl( ctx: &mut LoweringContext, class_decl: &ast::ClassDecl, diff --git a/crates/perry-hir/src/lower_decl/class_decl/member_registration.rs b/crates/perry-hir/src/lower_decl/class_decl/member_registration.rs new file mode 100644 index 0000000000..3151157c84 --- /dev/null +++ b/crates/perry-hir/src/lower_decl/class_decl/member_registration.rs @@ -0,0 +1,127 @@ +//! Computed and non-computed class-member helpers, split out of +//! `class_decl.rs` to keep it under the 2000-line file gate. +//! Behaviour is unchanged; `use super::*` reaches the shared imports. + +use super::*; + +pub(super) fn lower_noncomputed_class_member_registration( + ctx: &mut LoweringContext, + method: &ast::ClassMethod, + prop_name: &str, + source_order: usize, +) -> Result { + let function_name = noncomputed_member_registration_name(method.kind, method); + let (kind, function) = match method.kind { + ast::MethodKind::Method => ( + ClassComputedMemberKind::Method, + with_static_member_context(ctx, method.is_static, |ctx| { + lower_class_method_with_name(ctx, method, function_name) + })?, + ), + ast::MethodKind::Getter => ( + ClassComputedMemberKind::Getter, + with_static_member_context(ctx, method.is_static, |ctx| { + lower_getter_method_with_name(ctx, method, function_name) + })?, + ), + ast::MethodKind::Setter => ( + ClassComputedMemberKind::Setter, + with_static_member_context(ctx, method.is_static, |ctx| { + lower_setter_method_with_name(ctx, method, function_name) + })?, + ), + }; + Ok(ClassComputedMember { + key_expr: Expr::String(prop_name.to_string()), + function, + is_static: method.is_static, + kind, + source_order, + }) +} + +/// Lower a generator `*[Symbol.iterator]()` class method (already lowered into +/// `func`, named `@@iterator`) into the runtime `@@iterator` vtable entry. +/// +/// The body is lifted to a top-level `__perry_iter_` generator with +/// `this` as an explicit first parameter — the generator transform (which only +/// visits `module.functions`) then rewrites it to the `{next, return, throw}` +/// closure triple, and the syntactic `for…of` fast path dispatches to it +/// directly via `iterator_func_for_class`. +/// +/// But every *runtime*-dispatched iterator consumer (spread `[...x]`, +/// `Math.max(...x)`, destructuring, `x[Symbol.iterator]()`, `Array.from`) +/// resolves `@@iterator` through the class registry instead. So this also +/// returns a synthetic NON-generator `@@iterator` wrapper method that forwards +/// to the lifted generator (`return __perry_iter_X(this)`) for the caller to +/// append to the instance vtable. Without it the class carries no `@@iterator` +/// for those consumers to find and they throw "value is not iterable" (#5128). +/// (The runtime maps the well-known `Symbol.iterator` to this `@@iterator` +/// method name in `js_object_get_symbol_property`.) +/// +/// Shared by `lower_class_decl` and `lower_class_from_ast` so class +/// declarations and class expressions behave identically. +pub(super) fn synthesize_symbol_iterator_wrapper( + ctx: &mut LoweringContext, + class_name: &str, + func: &mut Function, +) -> Function { + let this_id = ctx.fresh_local(); + let mut new_params = Vec::with_capacity(func.params.len() + 1); + new_params.push(Param { + id: this_id, + name: "this".to_string(), + ty: Type::Named(class_name.to_string()), + default: None, + decorators: Vec::new(), + is_rest: false, + arguments_object: None, + }); + new_params.append(&mut func.params); + + let mut body = std::mem::take(&mut func.body); + crate::analysis::replace_this_in_stmts(&mut body, this_id); + + let top_fn_id = ctx.fresh_func(); + let top_fn = Function { + id: top_fn_id, + name: format!("__perry_iter_{}", class_name), + type_params: Vec::new(), + params: new_params, + return_type: Type::Any, + body, + is_async: false, + is_generator: true, + is_strict: true, + was_plain_async: false, + was_unrolled: false, + is_exported: false, + captures: Vec::new(), + decorators: Vec::new(), + }; + ctx.pending_functions.push(top_fn); + ctx.iterator_func_for_class + .insert(class_name.to_string(), top_fn_id); + + Function { + id: ctx.fresh_func(), + name: "@@iterator".to_string(), + type_params: Vec::new(), + params: Vec::new(), + return_type: Type::Any, + body: vec![Stmt::Return(Some(Expr::Call { + callee: Box::new(Expr::FuncRef(top_fn_id)), + args: vec![Expr::This], + type_args: Vec::new(), + byte_offset: 0, + }))], + is_async: false, + is_generator: false, + is_strict: true, + was_plain_async: false, + was_unrolled: false, + is_exported: false, + captures: Vec::new(), + decorators: Vec::new(), + } +} diff --git a/crates/perry-runtime/src/array/numeric_range.rs b/crates/perry-runtime/src/array/numeric_range.rs index 7d69c1fd00..d18ef23575 100644 --- a/crates/perry-runtime/src/array/numeric_range.rs +++ b/crates/perry-runtime/src/array/numeric_range.rs @@ -170,7 +170,7 @@ pub unsafe extern "C" fn js_array_fill_range_strided_tagged( return -1; } let raw = receiver_value.as_pointer::() as usize; - let Some(header) = (crate::value::addr_class::try_read_gc_header(raw)) else { + let Some(header) = crate::value::addr_class::try_read_gc_header(raw) else { return -1; }; if header.obj_type != crate::gc::GC_TYPE_ARRAY { diff --git a/scripts/gc_runtime_root_holders.json b/scripts/gc_runtime_root_holders.json index ef4d9c1db3..cce1a95909 100644 --- a/scripts/gc_runtime_root_holders.json +++ b/scripts/gc_runtime_root_holders.json @@ -275,6 +275,25 @@ "scanner": "object::scan_class_side_table_roots_mut and its budgeted step twin (class_registry/gc_roots.rs:138 and :256)", "why": "The class side tables are declared in state.rs and scanned from gc_roots.rs. Both twins visit it — #7239 diffed all eight budgeted (FULL, STEP) pairs and found no drift." }, + { + "file": "crates/perry-runtime/src/object/class_registry/state.rs", + "name": "CLASS_STATIC_PROTOTYPES", + "verdict": "covered_elsewhere", + "scanner": "object::class_registry::gc_roots::scan_class_side_table_roots_mut and its budgeted step twin (class_side_table_root_snapshot enumerates ClassSideTableRootSlot::StaticPrototype; scan_class_side_table_root_slot visits that slot)", + "why": "Constructor-side [[Prototype]] recorded by Object.setPrototypeOf(Ctor, obj) on a declared class. Holds a real heap ObjectHeader address as usize, so it is visited with visit_usize_slot in BOTH the full and budgeted class-side-table walks, exactly like the CLASS_DECL_PROTOTYPE_OBJECTS entries beside it, and class_static_prototype_root_store fires runtime_write_barrier_root_raw_ptr on the stored pointer." + }, + { + "file": "crates/perry-runtime/src/object/class_registry/state.rs", + "name": "CLASS_STATIC_PROTOTYPE_NULLED", + "verdict": "not_a_gc_pointer", + "why": "Set of class ids whose constructor [[Prototype]] was explicitly set to null, so Object.getPrototypeOf answers null rather than the default Function.prototype. Stores u32 class ids only — no heap address, nothing to trace or forward." + }, + { + "file": "crates/perry-runtime/src/object/class_registry/state.rs", + "name": "CLASS_SYMBOL_MEMBER_ORDERS", + "verdict": "not_a_gc_pointer", + "why": "Source order for Symbol-keyed class members: HashMap<(class_id, SymbolHeader::id, is_static), u32>. The u64 is the symbol's stable id (read once at registration in record_class_symbol_member_order), NOT its address, so the table needs no re-key when a Symbol is evacuated; the value is an order index. The member values themselves live in CLASS_SYMBOL_METHODS/CLASS_SYMBOL_ACCESSORS, which scan_class_symbol_member_keys_mut visits and rewrite_class_symbol_method_key_if_forwarded re-keys." + }, { "file": "crates/perry-runtime/src/object/global_this/fetch_globals.rs", "name": "TEST_COLLECT_BEFORE_GLOBAL_THIS_ALLOC", @@ -295,6 +314,12 @@ "scanner": "gc::roots GLOBAL_ROOTS — the cell's address is registered with js_gc_register_global_root (fetch_globals.rs, js_module_top_this)", "why": "Same shape as THREAD_GLOBAL_THIS: a NaN-boxed cache slot registered as a mutable global root at first population." }, + { + "file": "crates/perry-runtime/src/object/mod.rs", + "name": "CLASS_DYNAMIC_PROP_ORDER", + "verdict": "not_a_gc_pointer", + "why": "First-insertion order for CLASS_DYNAMIC_PROPS: HashMap> of owned Rust strings, needed because the value table is a HashMap while [[OwnPropertyKeys]] needs order. Holds no JSValues; the f64 values live in CLASS_DYNAMIC_PROPS, which is already a scanned root." + }, { "file": "crates/perry-runtime/src/object/native_module.rs", "name": "TEST_BOUND_METHOD_MOVE", @@ -1832,19 +1857,6 @@ "name": "WINDOW_ROOTS", "verdict": "not_a_gc_pointer", "why": "Window-root registry maps numeric window handles to numeric root-widget handles; neither value is a JavaScript heap pointer." - }, - { - "file": "crates/perry-runtime/src/object/class_registry/state.rs", - "name": "CLASS_STATIC_PROTOTYPES", - "verdict": "covered_elsewhere", - "scanner": "object::class_registry::gc_roots::scan_class_side_table_roots_mut and its budgeted step twin (class_side_table_root_snapshot enumerates ClassSideTableRootSlot::StaticPrototype; scan_class_side_table_root_slot visits that slot)", - "why": "Constructor-side [[Prototype]] recorded by Object.setPrototypeOf(Ctor, obj) on a declared class. Holds a real heap ObjectHeader address as usize, so it is visited with visit_usize_slot in BOTH the full and budgeted class-side-table walks, exactly like the CLASS_DECL_PROTOTYPE_OBJECTS entries beside it, and class_static_prototype_root_store fires runtime_write_barrier_root_raw_ptr on the stored pointer." - }, - { - "file": "crates/perry-runtime/src/object/class_registry/state.rs", - "name": "CLASS_STATIC_PROTOTYPE_NULLED", - "verdict": "not_a_gc_pointer", - "why": "Set of class ids whose constructor [[Prototype]] was explicitly set to null, so Object.getPrototypeOf answers null rather than the default Function.prototype. Stores u32 class ids only \u2014 no heap address, nothing to trace or forward." } ], "_FRONTIER_README": "Identity-pinned debt ratchet over new perry-ui* candidates and otherwise-unclassified core perry_thread_local! declarations (see the census docstring, “The identity-pinned frontier”). A new uncovered holder fails until it is scanned, receives a researched holders verdict, or is deliberately pinned as debt. Moving a researched false positive to holders graduates it from this list. A fixed or classified holder makes its old frontier pin stale, so the receipt must be deleted.",