fix(transform): specialized mixin clones keep parent edge + a unique id (#6356)

[proggeramlug]

Fixes #6356.

Symptom

A class whose parent is wired at runtime (a mixin/factory class that declines the HIR mixin fast path) loses the base's inheritance through a two-level dynamic chain:

class Base { value = "base value"; hello() { return "base hello"; } }
function Greetable(B: any)    { return class extends B { greet() { return "hi"; } }; }
function Serializable(B: any) { return class extends B { ser()   { return "ser"; } }; }

const Mid = Greetable(Base);
const Top = Serializable(Mid as any);

(new Top() as any).value       // node: "base value"   perry: undefined      ❌
(new Top() as any).greet()     // node: "hi"           perry: TypeError      ❌
new Top() instanceof Mid       // node: true           perry: false          ❌

On current main most of the #6356 matrix already passes (field-init through a single dynamic parent was fixed by later work); the residual failure is the two-level Serializable(Mid) chain, where Top inherits nothing from Mid.

Root cause

These shapes route through specialize_captured_class_factories (crates/perry-transform/src/inline/factory_specialize.rs), which monomorphizes the returned class per call site. Two defects there broke the chain:

1. Colliding class id. class.clone() copied the template's id, and the pass renamed the clone but never reassigned it. Codegen keys the runtime class registry (methods, constructor, parent edges) by c.id, so a clone collided with its template and with sibling clones of the same template (Serializable(Greetable(Base)) and Serializable(Mid) both clone __anon_class_N) — the distinct classes shared one registry slot, last-writer-wins on the constructor/parent edge.

2. Dropped parent edge for a runtime-value parent. When the parent arg specialized to a runtime value (Serializable(Mid), where Mid is a local binding) rather than a static ClassRef, no static extends_name could be set, and the factory Call — including the RegisterClassParentDynamic its lowered Sequence carried — was replaced by a bare ClassRef, dropping the parent edge entirely.

Confirmed via --trace llvm: the clone registered under the template's id (js_register_class_method/js_register_class_constructor emitted twice for the same cid), and no parent edge (js_register_class_parent[_dynamic]) was emitted for it.

Fix

• Assign each specialized clone a fresh, unique class id (past the current max). Codegen resolves static parent edges by name and derives its imported-class fallback id range from the post-specialization max, so only numeric uniqueness matters here. Later clones that reference this one as a static parent read the id assigned here (so it's assigned before pushing).
• When the specialized parent is a runtime value, sequence a RegisterClassParentDynamic ahead of the ClassRef so the parent edge is wired when the init runs — mirroring the class-expression lowering in perry-hir's arm_class.rs. The Sequence still yields the ClassRef as its value.

Validation

• New fixture test-files/test_gap_6356_dynamic_parent_mixin_chain.ts (return-via-const, composed mixins, the two-level Serializable(Mid) chain, and sibling-clone independence) is byte-identical to node --experimental-strip-types.
• After the fix, the clone gets a distinct cid with a (Top → Mid) dynamic edge, so Top → Mid → Base resolves: instanceof Mid, inherited greet, and the value field all match node.
• No regressions across the class/mixin/factory gap tests, incl. test_gap_5952_mixin_factory_binding, test_gap_class_expr_*, test_gap_class_ref_new_dynamic, test_gap_class_extends_function_static. The two pre-existing triaged failures in that neighbourhood (test_gap_2159_defineproperty_class_prototype, test_gap_class_expr_extends_static_call_this) produce identical output with and without this change (A/B'd against a stashed baseline).
• cargo test -p perry-transform green (48 tests); cargo fmt --all --check and the 2000-line file-size gate pass.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved class mixin and factory behavior when the parent is resolved dynamically at runtime.
  • Preserved dynamic parent wiring so extends relationships and inherited fields/methods remain correct through multi-level chains.
  • Prevented ID collisions between specialized class variants created from the same template.
  • Updated module initialization so dynamic parent expressions are correctly hoisted and applied to child classes.
• Tests
  • Added a regression test covering dynamic parent chains, composed mixins, instanceof correctness, and runtime-value extends scenarios.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 76be1367-c69c-4920-9d48-d480b0e639a4

📥 Commits

Reviewing files that changed from the base of the PR and between 4c0db6a and 116ae80.

📒 Files selected for processing (2)

• crates/perry-transform/src/inline/factory_specialize.rs
• test-files/test_gap_6356_dynamic_parent_mixin_chain.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• crates/perry-transform/src/inline/factory_specialize.rs

---

📝 Walkthrough

Walkthrough

Factory-specialized classes now preserve runtime parent expressions, register dynamic parent edges at factory call sites, receive unique class IDs, and support hoisting through rewritten sequences. A regression test covers dynamic mixin chains, inherited fields and methods, instanceof relationships, sibling specializations, and runtime-value inheritance.

Changes

Dynamic parent specialization

Layer / File(s)	Summary
Capture dynamic parent expressions crates/perry-transform/src/inline/factory_specialize.rs	Non-static extends expressions are captured for later dynamic parent registration.
Rewrite and hoist specialized factory calls crates/perry-transform/src/inline/factory_specialize.rs	Specialized clones receive fresh IDs; rewritten calls register dynamic parents before yielding class references, and hoisting resolves class references from sequence expressions.
Validate dynamic mixin chains test-files/test_gap_6356_dynamic_parent_mixin_chain.ts	Five scenarios verify inherited fields and methods, dynamic parent chains, instanceof, independent sibling clones, and runtime-value inheritance.

Estimated code review effort: 4 (Complex) | ~35 minutes

Sequence Diagram(s)

sequenceDiagram
  participant rewrite_to_specialized_class
  participant FactoryCallSite
  participant RegisterClassParentDynamic
  participant module_init
  rewrite_to_specialized_class->>FactoryCallSite: replace factory Call
  FactoryCallSite->>RegisterClassParentDynamic: register cloned class with runtime parent
  RegisterClassParentDynamic-->>FactoryCallSite: yield specialized ClassRef
  module_init->>RegisterClassParentDynamic: hoist parent edge from rewritten sequence

Loading

Possibly related PRs

• PerryTS/perry#5613: Related dynamic-parent handling during derived-class construction.
• PerryTS/perry#6379: Related constructor behavior for classes with dynamic parents.
• PerryTS/perry#6449: Related dynamic heritage registration and parent resolution.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description has useful detail, but it does not follow the required template sections like Summary, Changes, Related issue, and Test plan.	Reformat the PR description to match the template and add the missing sections, especially Summary, Changes, Related issue, Test plan, and Checklist.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately summarizes the main fix: specialized mixin clones retain parent edges and unique IDs.
Linked Issues check	✅ Passed	The changes address #6356 by preserving dynamic parent edges, assigning unique clone IDs, and adding regression coverage for the dynamic chain cases.
Out of Scope Changes check	✅ Passed	The diff appears focused on the #6356 fix and its regression test, with no obvious unrelated or out-of-scope changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/perry-transform/src/inline/factory_specialize.rs (1)

991-996: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Use classref_name to correctly hoist parents from sequence expressions.

With the newly introduced dynamic parent behavior, factory calls can be rewritten into Expr::Sequence blocks containing RegisterClassParentDynamic and ClassRef. If a user writes a class declaration like class Child extends DynamicFactory(Var) {}, the parent_expr inside module.init's pre-existing registration will be an Expr::Sequence.

The strict if let Expr::ClassRef(...) = parent_expr.as_ref() match will fail on sequences, preventing the static parent from being hoisted and causing the child class to lose inherited fields.

Using the existing classref_name helper ensures the name is extracted correctly even if it's wrapped in a sequence.

🐛 Proposed fix

-            if let Expr::ClassRef(parent_name) = parent_expr.as_ref() {
+            if let Some(ref parent_name) = classref_name(parent_expr.as_ref()) {
                 if let Some(child) = module.classes.iter_mut().find(|c| &c.name == class_name) {
                     child.extends_name = Some(parent_name.clone());
                     if let Some(parent_cls) = new_classes
                         .iter()
                         .find(|c| &c.name == parent_name)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-transform/src/inline/factory_specialize.rs` around lines 991 -
996, Update the parent-name extraction in the class specialization logic around
the existing Expr::ClassRef match to use the classref_name helper instead.
Ensure sequence-wrapped ClassRef expressions produced by dynamic parent
registration resolve their parent name, so the existing extends_name assignment
and parent hoisting continue to apply.

🧹 Nitpick comments (1)

crates/perry-transform/src/inline/factory_specialize.rs (1)

928-944: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Extract the base class ID calculation to avoid O(N) recomputation.

base_class_id is recalculated by iterating over all original module classes every time a factory call is rewritten. While functionally correct and guaranteed to produce unique IDs, this results in O(N) work per rewrite. You could calculate this once before the visit_stmts traversal and either pass it down or keep a mutable max ID counter to eliminate the redundant loops.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-transform/src/inline/factory_specialize.rs` around lines 928 -
944, Move the initial class-ID calculation out of the per-rewrite logic and
compute it once before the visit_stmts traversal. Pass that base value into the
factory specialization flow or maintain a mutable next-ID counter, then update
the cloned.id assignment to allocate IDs without iterating over classes for each
rewrite while preserving uniqueness across all new clones.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@crates/perry-transform/src/inline/factory_specialize.rs`:
- Around line 991-996: Update the parent-name extraction in the class
specialization logic around the existing Expr::ClassRef match to use the
classref_name helper instead. Ensure sequence-wrapped ClassRef expressions
produced by dynamic parent registration resolve their parent name, so the
existing extends_name assignment and parent hoisting continue to apply.

---

Nitpick comments:
In `@crates/perry-transform/src/inline/factory_specialize.rs`:
- Around line 928-944: Move the initial class-ID calculation out of the
per-rewrite logic and compute it once before the visit_stmts traversal. Pass
that base value into the factory specialization flow or maintain a mutable
next-ID counter, then update the cloned.id assignment to allocate IDs without
iterating over classes for each rewrite while preserving uniqueness across all
new clones.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 633a3215-ccee-4eda-bf7e-52d722647de9

📥 Commits

Reviewing files that changed from the base of the PR and between 520894f and 4c0db6a.

📒 Files selected for processing (2)

• crates/perry-transform/src/inline/factory_specialize.rs
• test-files/test_gap_6356_dynamic_parent_mixin_chain.ts

[proggeramlug]

Thanks @coderabbitai — good catch on the Sequence parent_expr. Confirmed: class Child extends Factory(runtimeVar) {} rewrites the inner factory call into Sequence([RegisterClassParentDynamic, ClassRef(clone)]), so the strict Expr::ClassRef match in the hoisting block failed to set the child's static extends_name.

Fixed in 116ae80 by resolving through the existing classref_name helper (which unwraps a Sequence to its trailing ClassRef). Verified via --trace llvm that the static js_register_class_parent(Child, clone) edge is now emitted, and added scenario (5) to the fixture covering the class-declaration-extends-factory shape (byte-identical to node --experimental-strip-types).

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@proggeramlug Great—thanks for the thorough verification. Unwrapping the trailing ClassRef through classref_name is the right fit for the Sequence([RegisterClassParentDynamic, ClassRef(clone)]) rewrite shape, and scenario (5) directly protects the class-declaration hoisting regression.

🐇✨

---

✏️ Learnings added

Learnt from: proggeramlug
URL: https://github.com/PerryTS/perry/pull/6548

Timestamp: 2026-07-17T23:43:22.914Z
Learning: In `crates/perry-transform/src/inline/factory_specialize.rs`, factory-call specialization can rewrite an expression to `Sequence([RegisterClassParentDynamic, ClassRef(clone)])`. Any later class-parent hoisting logic that needs the specialized class reference must use the existing `classref_name` helper rather than matching only `Expr::ClassRef`, so declarations such as `class Child extends Factory(runtimeVar) {}` retain their static `extends_name` and emit `js_register_class_parent(Child, clone)`.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

---

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!