Skip to content

fix(hir): bare-factory parent's field initializers run on subclass instances#827

Merged
proggeramlug merged 2 commits into
mainfrom
worktree-fix-bare-factory-fields
May 16, 2026
Merged

fix(hir): bare-factory parent's field initializers run on subclass instances#827
proggeramlug merged 2 commits into
mainfrom
worktree-fix-bare-factory-fields

Conversation

@proggeramlug
Copy link
Copy Markdown
Contributor

Summary

Fixes the bare-factory field-init drop surfaced by PR #822's #806 harness (section 1). class Sub extends makeFactory() {} had extends_name = None and only extends_expr set, so the codegen-time chain walks (apply_field_initializers_recursive, the keys-array generator) walked by extends_name only, saw None, and skipped the factory class's field initializers entirely — new Sub().kind read undefined instead of the parent's kind = \"bare\" literal.

Minimal repro

function makeBare() {
  return class { kind = "bare"; extra = 42; hello() { return "hi"; } };
}
class Sub extends makeBare() {}
new Sub().kind;   // undefined before; "bare" after
new Sub().extra;  // undefined before; 42 after
new Sub().hello() // "hi" both before and after (methods already dispatched via CLASS_REGISTRY)

Fix

New HIR post-pass infer_dynamic_extends_names in crates/perry-hir/src/lower.rs. 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 PR fix(hir): mixin factories must register grandparent edge at runtime #826's 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. Without the guard, the tagged-factory section of the harness regresses. The guard conservatively skips those — only literal-initialized parents propagate. Method inheritance is unaffected: dispatch goes through the runtime CLASS_REGISTRY which the RegisterClassParentDynamic side effect populates regardless of this static fixup.

Test plan

  • Bare-factory repro — Sub instances now report the parent's kind: bare, extra: 42 matching Node.
  • tests: harness for dynamic class extension / mixins #806 harness — sections 1, 2 (tagged), 3 (captured-factory generic) now PASS. (Section 4 chained mixin still needs PR fix(hir): mixin factories must register grandparent edge at runtime #826; sections 5+ are 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.

…stances

`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.
@proggeramlug proggeramlug merged commit 4bab6c9 into main May 16, 2026
9 checks passed
@proggeramlug proggeramlug deleted the worktree-fix-bare-factory-fields branch May 16, 2026 06:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant