fix(hir): bare-factory parent's field initializers run on subclass instances#827
Merged
Conversation
…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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the bare-factory field-init drop surfaced by PR #822's #806 harness (section 1).
class Sub extends makeFactory() {}hadextends_name = Noneand onlyextends_exprset, so the codegen-time chain walks (apply_field_initializers_recursive, the keys-array generator) walked byextends_nameonly, sawNone, and skipped the factory class's field initializers entirely —new Sub().kindreadundefinedinstead of the parent'skind = \"bare\"literal.Minimal repro
Fix
New HIR post-pass
infer_dynamic_extends_namesincrates/perry-hir/src/lower.rs. Runs after every function and class is inmodule(so forward-references work despite function hoisting). For each class withextends_name = Noneandextends_expr = Call(FuncRef(N)):N's body.Return(Some(ClassRef(name)))orReturn(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).LocalGetanywhere — bare literals only), setextends_name = Some(name).The closure-free guard is load-bearing.
apply_field_initializers_recursiveinlines each chained class's init expressions directly into the subclass's constructor IR, so an init like_tag = tag(wheretagis the factory's parameter) would re-resolveLocalGet(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 runtimeCLASS_REGISTRYwhich the RegisterClassParentDynamic side effect populates regardless of this static fixup.Test plan
Subinstances now report the parent'skind: bare,extra: 42matching Node._tagbinding.)cargo test --release -p perry-hir -p perry-codegen— all green.Refs #321.