From c3d5e22bbec7079fcdedf1c303d0065268ff411c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 16 May 2026 07:34:03 +0200 Subject: [PATCH 1/2] fix(hir): bare-factory parent's field initializers run on subclass instances MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `class Sub extends makeFactory() {}` had `extends_name = None` and only `extends_expr` set, because the parent expression is a Call to a runtime function. The codegen-time chain walks (`apply_field_initializers_recursive`, the keys-array generator) walk by `extends_name` only, so the factory class's field initializers were skipped entirely and the keys array had no slot for them either. Surfaced by the #806 mixin harness — section 1: - Node: `bare.kind: bare`, `bare.extra: 7`. - Perry pre-fix: `bare.kind: undefined`, `bare.extra: undefined`. Adds a HIR post-pass `infer_dynamic_extends_names`. Runs after every function and class is in `module` (so forward-references work despite function hoisting). For each class with `extends_name = None` and `extends_expr = Call(FuncRef(N))`: 1. Look up function `N`'s body. 2. Match `Return(Some(ClassRef(name)))` or `Return(Some(Sequence([…, ClassRef(name)])))` — the latter is what the #826 mixin-grandparent fix emits when the factory's inner class itself has a dynamic parent. 3. If matched AND the returned class's field initializers are closure-free (no `LocalGet` anywhere — bare literals only), set `extends_name = Some(name)`. The closure-free guard is load-bearing. `apply_field_initializers_recursive` inlines each chained class's init expressions directly into the subclass's constructor IR, so an init like `_tag = tag` (where `tag` is the factory's parameter) would re-resolve `LocalGet(tag)` in the subclass's scope and read garbage. Conservatively skip those — only literal-initialized parents like `kind = "bare"` propagate. Method inheritance is unaffected: dispatch goes through the runtime `CLASS_REGISTRY` which the #826 RegisterClassParentDynamic side effect populates regardless of this static fixup. Validation: - `class Sub extends makeBare() {}` repro — Sub instances now report the parent's `kind: bare`, `extra: 42` matching Node. - #806 harness sections 1 (bare factory), 2 (tagged factory), 3 (captured-factory generic) now PASS. Sections 4+ still need #826 (chained mixin) and follow-up work (super-args, static method dispatch, double-call factory `_tag` binding). - Class smoke set (test_simple_class, test_get_prototype_of_instance, test_issue_711_function_prototype, test_gap_class_advanced, test_class_static_iife) — all PASS byte-for-byte vs Node. - `cargo test --release -p perry-hir -p perry-codegen` — all green. Refs #321. --- crates/perry-hir/src/lower.rs | 127 ++++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) diff --git a/crates/perry-hir/src/lower.rs b/crates/perry-hir/src/lower.rs index 9078af17..c34d3193 100644 --- a/crates/perry-hir/src/lower.rs +++ b/crates/perry-hir/src/lower.rs @@ -2319,9 +2319,136 @@ pub fn lower_module_full( } } + // Post-pass: infer `extends_name` from `extends_expr` for the bare-factory + // shape `class Sub extends makeFactory() {}` where `makeFactory` is a + // top-level function whose body trivially returns a static `ClassRef`. + // Without this, the codegen chain walks + // (`apply_field_initializers_recursive` + the keys-array generator) walk + // by `extends_name` only, see `None`, and skip the factory class's + // field initializers entirely — `new Sub().kind` reads `undefined` + // instead of the parent's `kind = "bare"` literal. Surfaced by the + // #806 mixin harness (bare-factory section). + infer_dynamic_extends_names(&mut module); + Ok((module, ctx.next_class_id)) } +/// Fill in `Class::extends_name` for classes whose parent is the result of +/// calling a statically-resolvable factory function — but ONLY when the +/// parent's field initializers are closure-free (no `LocalGet` reads). The +/// post-pass runs after every function and class is in `module`, so +/// forward-references work (e.g. `class Sub extends makeBare() {}` ahead of +/// `function makeBare() …` hoisting). +/// +/// The closure-free guard exists because the field-init pass at codegen +/// (`apply_field_initializers_recursive`) inlines each chained class's +/// init expressions directly into the subclass's constructor. That's +/// correct for pure-literal initializers like `kind = "bare"` but wrong +/// for `_tag = tag` where `tag` is the factory's parameter — the inlined +/// `LocalGet(tag)` would re-resolve in the subclass's scope (where `tag` +/// doesn't exist) and produce garbage. Conservatively skip those: the +/// subclass's static parent stays None and field-init inheritance only +/// works for the literal-initialized parents that #806's bare-factory +/// section needs. +fn infer_dynamic_extends_names(module: &mut Module) { + use std::collections::HashMap; + // Build a map of `function_id → returned ClassRef name` for every + // function whose body returns a static ClassRef. Only the LAST `Return` + // is examined — bodies with multiple Returns to different classes + // don't resolve uniquely, and the canonical factory shape has exactly + // one Return as its last statement. + let mut factory_returns: HashMap = HashMap::new(); + for func in &module.functions { + if let Some(name) = trailing_return_classref(&func.body) { + factory_returns.insert(func.id, name); + } + } + // Index classes by name so we can re-resolve transitively (a chain like + // `Sub extends A() {}` where `A` returns `__anon_N` and `__anon_N` is + // a class we own — we only set `extends_name` for `Sub` here; chain + // walks at codegen step through `__anon_N.extends_name` normally). + let class_field_inits_pure: HashMap = module + .classes + .iter() + .map(|c| (c.name.clone(), fields_are_pure(c))) + .collect(); + for class in &mut module.classes { + if class.extends_name.is_some() { + continue; + } + let Some(expr) = class.extends_expr.as_deref() else { + continue; + }; + let Expr::Call { callee, .. } = expr else { + continue; + }; + let Expr::FuncRef(func_id) = callee.as_ref() else { + continue; + }; + let Some(parent_name) = factory_returns.get(func_id) else { + continue; + }; + // Only inherit field-init machinery when the parent's fields are + // pure (no `LocalGet`). Methods on the parent are unaffected — + // those dispatch through the runtime CLASS_REGISTRY which is + // populated by the #826 RegisterClassParentDynamic side effect. + if class_field_inits_pure.get(parent_name).copied().unwrap_or(false) { + class.extends_name = Some(parent_name.clone()); + } + } +} + +/// True when none of the class's field initializers contain a `LocalGet` +/// (the canonical sign that an initializer closes over its surrounding +/// scope — function parameters, outer-block lets, etc.). +fn fields_are_pure(class: &Class) -> bool { + for field in &class.fields { + if let Some(init) = &field.init { + if expr_reads_local(init) { + return false; + } + } + if let Some(key) = &field.key_expr { + if expr_reads_local(key) { + return false; + } + } + } + true +} + +fn expr_reads_local(expr: &Expr) -> bool { + if matches!(expr, Expr::LocalGet(_)) { + return true; + } + let mut found = false; + crate::walker::walk_expr_children(expr, &mut |child| { + if !found && expr_reads_local(child) { + found = true; + } + }); + found +} + +/// Return `Some(name)` if `body`'s last `Return` statement yields a static +/// `Expr::ClassRef` (directly or as the last element of an `Expr::Sequence`). +fn trailing_return_classref(body: &[Stmt]) -> Option { + for stmt in body.iter().rev() { + if let Stmt::Return(Some(expr)) = stmt { + return classref_name(expr); + } + } + None +} + +fn classref_name(expr: &Expr) -> Option { + match expr { + Expr::ClassRef(name) => Some(name.clone()), + Expr::Sequence(parts) => parts.last().and_then(classref_name), + _ => None, + } +} + /// Post-lowering pass that widens every `Expr::Closure`'s `mutable_captures` /// to include any capture that is assigned to inside a sibling closure in the /// same lexical scope. Then recurses into each closure body so nested scopes From d71592b644047aaf3e41eb6e437465759e4b5e37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 16 May 2026 07:36:21 +0200 Subject: [PATCH 2/2] style: cargo fmt (rustfmt chained-method-call formatting) --- crates/perry-hir/src/lower.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/perry-hir/src/lower.rs b/crates/perry-hir/src/lower.rs index c34d3193..f44c0f81 100644 --- a/crates/perry-hir/src/lower.rs +++ b/crates/perry-hir/src/lower.rs @@ -2392,7 +2392,11 @@ fn infer_dynamic_extends_names(module: &mut Module) { // pure (no `LocalGet`). Methods on the parent are unaffected — // those dispatch through the runtime CLASS_REGISTRY which is // populated by the #826 RegisterClassParentDynamic side effect. - if class_field_inits_pure.get(parent_name).copied().unwrap_or(false) { + if class_field_inits_pure + .get(parent_name) + .copied() + .unwrap_or(false) + { class.extends_name = Some(parent_name.clone()); } }