Skip to content

fix(hir): mixin factories must register grandparent edge at runtime#826

Merged
proggeramlug merged 1 commit into
mainfrom
worktree-fix-mixin-grandparent
May 16, 2026
Merged

fix(hir): mixin factories must register grandparent edge at runtime#826
proggeramlug merged 1 commit into
mainfrom
worktree-fix-mixin-grandparent

Conversation

@proggeramlug
Copy link
Copy Markdown
Contributor

Summary

Fixes the chained-mixin TypeError: value is not a function that the #806 harness surfaced on PR #822. Root cause: class expressions inside a function body (the canonical mixin shape) had their extends_expr captured at HIR level but never evaluated — only top-level class declarations had a RegisterClassParentDynamic statement emitted for them. So in

function WithA<TBase extends Ctor>(B: TBase) {
  return class extends B { a() { return "a"; } };
}
class Chained extends WithA(WithB(WithC(CoreBase))) {}

Chained → Anon3 got wired (via the top-level class-decl arm), but Anon3 → Anon2 → Anon1 → CoreBase was never registered. chained.a() worked; chained.core() walked one level and threw.

Minimal repro

class Base { m() { return "base"; } }
function Mix<T extends new(...a:any[])=>{}>(B: T) {
  return class extends B { x() { return "x"; } };
}
class Sub extends Mix(Base) {}
new Sub().m();  // TypeError before; "base" after

The const M = Mix(Base); class Sub extends M {} form already worked because the const path takes a different lowering branch that synthesizes a named class.

Fix

One arm in crates/perry-hir/src/lower.rs (the ast::Expr::Class lowering). When the lowered class has extends_expr, return

Expr::Sequence([
  Expr::RegisterClassParentDynamic { class_name, parent_expr },
  Expr::ClassRef(class_name),
])

instead of a bare Expr::ClassRef. Sequence is the existing comma-operator primitive whose codegen evaluates each element in order and returns the last — so the side effect runs every time the factory function executes, and the value the call site sees is unchanged. No new HIR variant, no codegen changes, no boilerplate through walker/monomorph/analysis/stable_hash/js_transform.

Test plan

  • 1-deep mixin (class X extends Mix(Base) {}) — new X().m() returns "base" (was TypeError).
  • 3-deep chained mixin (extends WithA(WithB(WithC(Base)))) — all 4 methods (core/a/b/c) dispatch correctly.
  • cargo test --release -p perry-hir -p perry-codegen — all green.
  • 4-test class smoke (test_simple_class, test_get_prototype_of_instance, test_issue_711_function_prototype, test_gap_class_advanced) — all PASS byte-for-byte vs Node.

What the #806 harness still surfaces

Four unrelated gaps stay visible (no longer cascading from the TypeError):

  • Bare-factory base-class field initializer is dropped (bare.kind: undefined).
  • super(arg) in a mixin constructor doesn't propagate the arg.
  • Static method dispatch on a factory-returned class returns garbage.
  • Effect double-call factory (TaggedError<X>()("tag", schema)) doesn't bind _tag.

Each is now independently visible — they'd been masked by the cascading TypeError.

Refs #321, closes the chained-mixin sub-case of #806's harness.

Class expressions inside a function body — the canonical mixin shape
`function WithA(B) { return class extends B {}; }` — pushed the
synthetic class into `pending_classes` but never emitted a runtime
parent-registration call. The class-decl arm at the top of `lower.rs`
emits `Stmt::Expr(Expr::RegisterClassParentDynamic { … })` ONLY for
top-level class declarations; an anonymous class produced by a factory
got no equivalent side effect, so its `extends_expr` (a `LocalGet` of
the factory's parameter `B`) was captured in `Class.extends_expr` but
never evaluated.

Surfaced by the #806 harness on this branch's parent. `class Chained
extends WithA(WithB(WithC(CoreBase))) {}` — `Chained → Anon3` was
wired (so `chained.a()` worked), but `Anon3 → Anon2 → Anon1 → CoreBase`
was never registered. `chained.core()` walked Chained → Anon3 and
stopped, throwing `TypeError: value is not a function`. The minimal
repro is one level deep: `class X extends WithA(Base) {}` —
`new X().baseMethod()` crashes; `const M = WithA(Base); class X extends M {}`
works because the const path takes a different lowering branch that
synthesizes a named class.

Fix sits in the `ast::Expr::Class` arm of `lower_expr` (lower.rs:8570).
When the lowered class has `extends_expr`, the arm now returns

  Expr::Sequence([
    Expr::RegisterClassParentDynamic { class_name, parent_expr },
    Expr::ClassRef(class_name),
  ])

instead of a bare `Expr::ClassRef`. Sequence is the existing
comma-operator primitive — its codegen evaluates each element in order
and returns the last, so the side effect runs every time the factory
function executes and the value the call site sees is unchanged. No
new HIR variant, no codegen changes, no boilerplate through walker/
monomorph/analysis/stable_hash/js_transform — the Sequence machinery
already covers all of those.

Validation:
- `class Chained extends WithA(WithB(WithC(Base))) {}` now prints
  `core`/`a`/`b`/`c` matching Node.
- `class X extends WithA(Base) {}` 1-deep variant prints `a`/`core`
  matching Node (was crashing before).
- 4-test class smoke set (test_simple_class, test_get_prototype_of_instance,
  test_issue_711_function_prototype, test_gap_class_advanced) — all
  PASS byte-for-byte vs Node.
- `cargo test --release -p perry-hir -p perry-codegen` — all green.

The #806 harness still surfaces four unrelated gaps (bare-factory
field initializer, super-arg propagation, static method dispatch,
Effect double-call factory tag binding) — none cascade from this
TypeError anymore, so they're each independently visible.
@proggeramlug proggeramlug merged commit c3100ba into main May 16, 2026
9 checks passed
@proggeramlug proggeramlug deleted the worktree-fix-mixin-grandparent branch May 16, 2026 05:21
proggeramlug added a commit that referenced this pull request May 16, 2026
…stances (#827)

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

`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.

* style: cargo fmt (rustfmt chained-method-call formatting)
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