Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions changelog.d/9567-mixin-of-a-mixin-dynamic-parent.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
**A mixin applied to a mixin no longer crashes.** `const Mixed2 = mixin(Mixed)`
— the second level of a mixin chain — SIGSEGVed as soon as anything derived
from it was constructed (`exit 139` where node printed the value). Mixin
composition is normally written as a chain, so this was the common shape
rather than an edge case, and it was a crash rather than a wrong value.

The HIR mixin fast path synthesizes a real class for `const M = mixinFn(Base)`.
At the second level the base is a lexical VALUE binding, so the parent is
correctly captured as `extends_expr` — a dynamic parent — instead of a static
class link. The missing half was the registration: unlike the sibling
`const X = class {…}` path in the same function, this arm bound the
synthesized class without emitting the declaration-time
`RegisterClassParentDynamic`. Its constructor therefore asked
`js_get_dynamic_parent_value` for a class id nothing had registered, and with
an undefined parent `js_fetch_or_value_super` fell back to the most-derived
receiver, re-selected the same class, and recursed until the stack overflowed.

The registration is now emitted here too, in source order after the parent's
own value binding and before the synthesized class's. A single-level
`mixin(Root)` extends a real class, keeps `extends_expr` at `None`, and is
unchanged — which is why one level already worked (#9073) and two did not.

Pinned in both directions. The gap fixture keeps the issue's reproducer
verbatim and adds what it left open: inherited base state and the mixin method
through both synthesized levels, `instanceof` across the whole chain, a leaf
with no own constructor, the one-level case #9073 fixed, and a three-level
chain built from three distinct mixins so a dropped level shows as a missing
method rather than being masked by identical bodies. A lowering unit test
asserts the registration lands between the parent's binding and its own, and
asserts the negative for level 1; it was confirmed to fail against the
unpatched lowering. The compiled fixture's LLVM now has a matching
`js_register_class_parent_dynamic` for every `js_get_dynamic_parent_value` in
the module — zero orphans.
40 changes: 40 additions & 0 deletions crates/perry-hir/src/lower/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -913,7 +913,47 @@ pub(crate) fn lower_stmt(
.clone()
.unwrap_or_default(),
);
// Issue #9079: `const Mixed2 =
// mixin(Mixed)` — a mixin applied to a
// previous mixin's RESULT. That base is
// a lexical VALUE binding, so
// `lower_class_from_ast` captures it as
// `extends_expr` (a dynamic parent)
// instead of a static class link. This
// arm bound the synthesized class
// WITHOUT the decl-time
// `RegisterClassParentDynamic` its
// sibling `const X = class {…}` path
// above emits, so the class id got a
// `js_get_dynamic_parent_value` in its
// constructor and no registration to
// answer it: `js_fetch_or_value_super`
// fell back to the most-derived
// receiver, re-selected the same class,
// and recursed until the stack
// overflowed — a SIGSEGV, not a wrong
// value. Emit it here too, in source
// order before the value binding, and
// clone the extends expression before
// `push_class_dedup` moves the class
// out. A single-level `mixin(Root)`
// extends a real class, keeps
// `extends_expr` None, and is unchanged.
let parent_register = lowered_class
.extends_expr
.clone()
.map(|parent_expr| {
Stmt::Expr(
Expr::RegisterClassParentDynamic {
class_name: bind_name.clone(),
parent_expr,
},
)
});
push_class_dedup(module, lowered_class);
if let Some(reg) = parent_register {
module.init.push(reg);
}
ctx.class_expr_aliases.insert(
bind_name.clone(),
bind_name.clone(),
Expand Down
1 change: 1 addition & 0 deletions crates/perry-hir/src/lower/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1944,6 +1944,7 @@ fn unresolved_new_names_the_identifier_and_defers_to_a_runtime_global_lookup() {
}

mod capture_stash;
mod mixin_parent_chain;

/// `const masks = opts?.masks ?? null` must not be declared `Null`. The
/// AST-level `??` rule used to answer the right operand's type whenever the
Expand Down
74 changes: 74 additions & 0 deletions crates/perry-hir/src/lower/tests/mixin_parent_chain.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
//! Dynamic-parent registration for a mixin applied to a mixin (#9079). Split
//! from `tests.rs` for the 2000-line file cap.

use super::*;

/// Index of the `RegisterClassParentDynamic` for `class_name` in `init`.
fn register_at(init: &[Stmt], class_name: &str) -> Option<usize> {
init.iter().position(|stmt| {
matches!(
stmt,
Stmt::Expr(Expr::RegisterClassParentDynamic { class_name: n, .. }) if n == class_name
)
})
}

/// Index of the value binding (`const M = <class>`) for `name` in `init`.
fn binding_at(init: &[Stmt], name: &str) -> Option<usize> {
init.iter()
.position(|stmt| matches!(stmt, Stmt::Let { name: n, .. } if n == name))
}

/// #9079: `const Mixed2 = mixin(Mixed)` — a mixin applied to a previous
/// mixin's RESULT — synthesizes a class whose parent is a lexical value
/// binding, so it lowers with `extends_expr` (a dynamic parent). The mixin
/// fast path bound that class without the declaration-time
/// `RegisterClassParentDynamic` its sibling `const X = class …` path emits, so
/// the class had no registered parent at all: `js_fetch_or_value_super` fell
/// back to the most-derived receiver, re-selected the same class, and recursed
/// until the stack overflowed (SIGSEGV, not a wrong value).
///
/// The registration must sit between the PARENT's binding (it reads that
/// local) and the synthesized class's own binding, exactly where the sibling
/// class-expression path puts it.
#[test]
fn mixin_of_a_mixin_registers_its_dynamic_parent_before_its_own_binding() {
let source = r#"
class Root { r = 1; }
function mixin(Base: any) { return class extends Base { m() { return 1; } }; }
const Mixed = mixin(Root);
const Mixed2 = mixin(Mixed);
class Deep extends Mixed2 { d = 4; constructor() { super(); } }
console.log(new Deep().d);
"#;
let module = perry_parser::parse_typescript(source, "mixin-chain.ts").expect("source parses");
let hir = super::super::lower_module(&module, "mixin-chain", "mixin-chain.ts")
.expect("source lowers");

let mixed_binding =
binding_at(&hir.init, "Mixed").expect("`Mixed` gets a class-expression value binding");
let mixed2_binding =
binding_at(&hir.init, "Mixed2").expect("`Mixed2` gets a class-expression value binding");
let mixed2_register = register_at(&hir.init, "Mixed2").unwrap_or_else(|| {
panic!(
"`Mixed2` extends the lexical value `Mixed` and must register that parent \
at declaration time; module init was:\n{:#?}",
hir.init
)
});

assert!(
mixed_binding < mixed2_register && mixed2_register < mixed2_binding,
"the registration reads the parent local and must precede its own binding: \
Mixed@{mixed_binding} register@{mixed2_register} Mixed2@{mixed2_binding}"
);

// Level 1's parent is the real class `Root`, which resolves statically, so
// no dynamic parent is captured and none is registered. Asserting the
// negative keeps the fix scoped to the case that needs it.
assert!(
register_at(&hir.init, "Mixed").is_none(),
"`Mixed` extends the static class `Root`; it must not gain a dynamic parent:\n{:#?}",
hir.init
);
}
65 changes: 65 additions & 0 deletions test-files/test_gap_9079_dynamic_parent_chain_own_ctor.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// Issue #9079: two levels of dynamic parent — a mixin applied to a mixin —
// SIGSEGVed when the leaf class had its own constructor.
//
// `const Mixed2 = mixin(Mixed)` extends a lexical VALUE binding, so the class
// the HIR mixin fast path synthesizes carries a DYNAMIC parent (extends_expr).
// That path bound the class without emitting the declaration-time
// `RegisterClassParentDynamic` its sibling `const X = class …` path emits, so
// the synthesized class had no registered parent: `js_fetch_or_value_super`
// fell back to the most-derived receiver, re-selected the same class, and
// recursed until the stack overflowed. One level was already correct (#9073),
// which is why the failure looked arbitrary.

// --- the exact reproducer from the issue ------------------------------------
class Root { r = 1; }
function mixin(Base: any) { return class extends Base { m() { return 1; } }; }
const Mixed = mixin(Root);
const Mixed2 = mixin(Mixed);
class Deep extends Mixed2 { d = 4; constructor() { super(); } }
console.log(new Deep().d);

// --- the chain is WALKED, not merely survived -------------------------------
// Root's field initializer must have run, and the mixin method must be
// reachable through both synthesized levels.
const deep: any = new Deep();
console.log("state:", deep.r, deep.d, "method:", deep.m());
console.log(
"instanceof:",
deep instanceof Deep,
deep instanceof Mixed2,
deep instanceof Mixed,
deep instanceof Root,
);

// The issue left open whether the leaf's OWN constructor was required. It is
// not the only shape that must work: an implicit constructor over the same
// two-level chain has to reach Root too.
class DeepImplicit extends Mixed2 { d2 = 5; }
const implicit: any = new DeepImplicit();
console.log("implicit:", implicit.r, implicit.d2, implicit.m());

// One level still works — it did before this fix; keep it pinned so the new
// registration cannot regress the shape #9073 fixed.
class One extends Mixed { x = 2; constructor() { super(); } }
const one: any = new One();
console.log("one-level:", one.r, one.x, one.m(), one instanceof Mixed, one instanceof Root);

// --- every level of a longer chain contributes ------------------------------
// Distinct mixins so a missing level is visible as a missing METHOD rather
// than being masked by an identical body at each level.
function withA(Base: any) { return class extends Base { a() { return "a"; } }; }
function withB(Base: any) { return class extends Base { b() { return "b"; } }; }
function withC(Base: any) { return class extends Base { c() { return "c"; } }; }
const A = withA(Root);
const B = withB(A);
const C = withC(B);
class Leaf extends C { n = 9; constructor() { super(); } }
const leaf: any = new Leaf();
console.log("three-level:", leaf.r, leaf.n, leaf.a(), leaf.b(), leaf.c());
console.log(
"three-level instanceof:",
leaf instanceof C,
leaf instanceof B,
leaf instanceof A,
leaf instanceof Root,
);
Loading