From e79d58778c30e0784ab78248be1975f576960c64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 18 Sep 2026 13:29:23 +0000 Subject: [PATCH 1/2] fix(hir): forward inherited captures through a locally-shadowed class parent A subclass with no explicit constructor, extending a capture-bearing class expression held in a local (const Base = class {...}; const Sub = class extends Base {...};), never forwarded Base's captured enclosing-scope locals to the synthesized subclass constructor: the lowering deliberately drops the static extends_name for such a lexically-local heritage identifier (avoiding a same-named-class collision, #5437), and capture propagation was keyed off that same name. Resolve the heritage identifier through resolve_class_alias instead - the same table Expr::New's own capture lookup already uses for let X = class {...}; new X() - for capture forwarding only, gated to subclasses with no own constructor (an explicit constructor already forwards captures correctly via a separate mechanism). --- crates/perry-hir/src/lower/tests.rs | 1 + .../tests/class_expr_subclass_captures.rs | 80 ++++++++++++++++ crates/perry-hir/src/lower_decl/class_decl.rs | 86 ++++++++++++++++- ..._gap_10486_class_expr_subclass_captures.ts | 93 +++++++++++++++++++ 4 files changed, 258 insertions(+), 2 deletions(-) create mode 100644 crates/perry-hir/src/lower/tests/class_expr_subclass_captures.rs create mode 100644 test-files/test_gap_10486_class_expr_subclass_captures.ts diff --git a/crates/perry-hir/src/lower/tests.rs b/crates/perry-hir/src/lower/tests.rs index f127f3eff9..f2803844a3 100644 --- a/crates/perry-hir/src/lower/tests.rs +++ b/crates/perry-hir/src/lower/tests.rs @@ -1992,5 +1992,6 @@ mod function_ctor_runtime_routing; mod mixin_parent_chain; mod native_module_sync; +mod class_expr_subclass_captures; mod nullish_over_optional_chain; mod ui_widget_add_child; diff --git a/crates/perry-hir/src/lower/tests/class_expr_subclass_captures.rs b/crates/perry-hir/src/lower/tests/class_expr_subclass_captures.rs new file mode 100644 index 0000000000..c3d1b37835 --- /dev/null +++ b/crates/perry-hir/src/lower/tests/class_expr_subclass_captures.rs @@ -0,0 +1,80 @@ +//! #10486: a class extending a capture-bearing class EXPRESSION held in a +//! local (`const Base = class { m() { return cap; } }; class Sub extends +//! Base {}`) must forward the base's captured locals to the synthesized +//! subclass constructor, even though `extends_name` is deliberately left +//! `None` for this lexically-local heritage shape (see the #5437 PQueue +//! fix). Split from `tests.rs` for the 2000-line file cap. + +/// The minimal repro from #10486: a subclass EXPRESSION with no own +/// constructor extending a base class EXPRESSION, both capturing distinct +/// enclosing-function locals. The `new Sub()` construction site must +/// forward BOTH captured ids (the base's and the subclass's own), not just +/// the subclass's own capture. +#[test] +fn subclass_of_local_class_expr_forwards_base_captures() { + let source = r#" + function outer() { + const baseCap = "base-capture"; + const subCap = "sub-capture"; + const Base = class { m() { return baseCap; } }; + const Sub = class extends Base { n() { return subCap; } }; + return new Sub().m(); + } + "#; + let module = perry_parser::parse_typescript(source, "t.ts").expect("source parses"); + let hir = super::lower_module(&module, "t", "t.ts").expect("source lowers"); + let outer = hir + .functions + .iter() + .find(|f| f.name == "outer") + .expect("fixture declares function outer"); + let compact: String = format!("{:?}", outer.body) + .chars() + .filter(|ch| !ch.is_whitespace()) + .collect(); + // The `new Sub()` site must append TWO forwarded capture args (the + // subclass's own `subCap` plus the inherited `baseCap`), not one. + assert!( + compact.contains("cap_args_appended:2"), + "expected new Sub() to forward both the base and subclass captures \ + (cap_args_appended: 2); got body: {compact}" + ); +} + +// NOTE: a subclass DECLARATION (as opposed to expression) extending a local +// class expression is a separate lowering path (`Expr::NewDynamic` with a +// `RegisterClassCaptures`/`RefreshClassExprCaptures`-based shared-box +// capture mechanism, not `Expr::New{cap_args_appended}`) that this fix does +// not cover — left as a known gap, see the PR body for #10486. + +/// A subclass that captures nothing of its own, extending a capture-bearing +/// local class expression, must still forward the base's capture (a +/// regression the naive "skip if the child's own union is empty" shape +/// would have reintroduced). +#[test] +fn subclass_with_no_own_captures_still_forwards_base_captures() { + let source = r#" + function outer() { + const cap = "only-base-cap"; + const Base = class { m() { return cap; } }; + const Sub = class extends Base {}; + return new Sub().m(); + } + "#; + let module = perry_parser::parse_typescript(source, "t.ts").expect("source parses"); + let hir = super::lower_module(&module, "t", "t.ts").expect("source lowers"); + let outer = hir + .functions + .iter() + .find(|f| f.name == "outer") + .expect("fixture declares function outer"); + let compact: String = format!("{:?}", outer.body) + .chars() + .filter(|ch| !ch.is_whitespace()) + .collect(); + assert!( + compact.contains("cap_args_appended:1"), + "expected new Sub() to forward the base's capture even though Sub \ + itself captures nothing; got body: {compact}" + ); +} diff --git a/crates/perry-hir/src/lower_decl/class_decl.rs b/crates/perry-hir/src/lower_decl/class_decl.rs index 551137f1b2..e1c71593ab 100644 --- a/crates/perry-hir/src/lower_decl/class_decl.rs +++ b/crates/perry-hir/src/lower_decl/class_decl.rs @@ -414,6 +414,61 @@ pub fn lower_class_decl( (None, None, None, None) }; + // Issue #10486: the branches above deliberately leave `extends_name` + // None when the heritage identifier resolves to a lexically-scoped + // local (`locally_shadowed`) or a fully dynamic expression, to avoid + // corrupting the static class-registry walks (instanceof / method + // dispatch / field layout — see the `locally_shadowed` comment above, + // #5437's PQueue regression). Capture forwarding is narrower and + // already tolerates a wrong/missing match (`lookup_class_captures` + // returns `None` and `synthesize_class_captures` is then a no-op for + // that source), so resolve the heritage identifier through + // `resolve_class_alias` — the SAME table `Expr::New`'s own capture + // lookup already uses (`expr_new.rs`) for `let X = class {...}; new + // X()`, populated when `const X = class {...}`/`let X = Y` is lowered + // (`register_let_class_alias`) — rather than a raw name match: a class + // extending a capture-bearing class EXPRESSION held in a local + // (`const Base = class { m() { return cap; } }; class Sub extends + // Base {}`) still finds and forwards `Base`'s captures at + // construction. Without this, every inherited method read `undefined` + // for the base's captures because the synthesized subclass + // constructor never received them as params. A raw-text match (or + // `resolve_class_name`, which only disambiguates same-named class + // DECLARATIONS) would reintroduce exactly the #5437 same-named-local + // collision for a minified bundle where two unrelated functions each + // declare their own `const Base = class {...}`. + // Only fall back for a subclass with NO explicit constructor of its + // own: an explicit constructor's own `super(...)` call already forwards + // whatever the parent needs via a SEPARATE, already-correct mechanism + // (the issue's own "Works" list: "an explicit `constructor() { + // super(); }` in the subclass" — confirmed by probing that case against + // a build without this fallback). Widening the union unconditionally + // regressed it: the parent capture then also lands in THIS class's own + // `captures_vec`, and the auto-stash machinery below expects to own + // forwarding an inherited cap into `super(...)` only for the + // SYNTHESIZED default constructor shape, not a user-written one. + let has_own_constructor = class_decl + .class + .body + .iter() + .any(|m| matches!(m, ast::ClassMember::Constructor(_))); + let capture_parent_name: Option = extends_name.clone().or_else(|| { + if has_own_constructor { + return None; + } + class_decl + .class + .super_class + .as_deref() + .and_then(|sc| match sc { + ast::Expr::Ident(ident) => { + let raw = ident.sym.to_string(); + Some(ctx.resolve_class_alias(&raw).unwrap_or(raw)) + } + _ => None, + }) + }); + // First pass: collect static field/method names for early registration // This allows static method bodies to reference static fields let mut static_field_names = Vec::new(); @@ -1120,7 +1175,7 @@ pub fn lower_class_decl( synthesize_class_captures( ctx, &name, - extends_name.as_deref(), + capture_parent_name.as_deref(), extends.is_some() || extends_name.is_some() || native_extends.is_some() @@ -1435,6 +1490,33 @@ pub fn lower_class_from_ast( (None, None, None, None) }; + // Issue #10486: mirrors the capture-forwarding fallback in + // `lower_class_decl` above (see its comment for the full rationale) — + // a class EXPRESSION extending a lexically-local capture-bearing class + // EXPRESSION (`const Base = class {…}; const Sub = class extends Base + // {…}`) needs the alias-resolved heritage identifier for capture + // lookup even when `extends_name` was deliberately left None for + // class-registry resolution. + // See the matching guard in `lower_class_decl` above: skip the + // fallback when this class expression has its own explicit + // constructor (its `super(...)` already forwards correctly). + let has_own_constructor = class + .body + .iter() + .any(|m| matches!(m, ast::ClassMember::Constructor(_))); + let capture_parent_name: Option = extends_name.clone().or_else(|| { + if has_own_constructor { + return None; + } + class.super_class.as_deref().and_then(|sc| match sc { + ast::Expr::Ident(ident) => { + let raw = ident.sym.to_string(); + Some(ctx.resolve_class_alias(&raw).unwrap_or(raw)) + } + _ => None, + }) + }); + let mut static_field_names = Vec::new(); let mut static_method_names = Vec::new(); for member in &class.body { @@ -1829,7 +1911,7 @@ pub fn lower_class_from_ast( synthesize_class_captures( ctx, name, - extends_name.as_deref(), + capture_parent_name.as_deref(), extends.is_some() || extends_name.is_some() || native_extends.is_some() diff --git a/test-files/test_gap_10486_class_expr_subclass_captures.ts b/test-files/test_gap_10486_class_expr_subclass_captures.ts new file mode 100644 index 0000000000..d25b2f6f93 --- /dev/null +++ b/test-files/test_gap_10486_class_expr_subclass_captures.ts @@ -0,0 +1,93 @@ +// #10486: an inherited method of a capture-bearing class EXPRESSION saw +// `undefined` captures when called on an instance of a capture-bearing +// `class extends` subclass with no explicit constructor. +// +// `lower_class_decl`/`lower_class_from_ast` (crates/perry-hir/src/lower_decl/ +// class_decl.rs) deliberately leave `extends_name` at `None` when the +// heritage identifier resolves to a lexically-scoped local (`locally_ +// shadowed`) rather than a statically-registered class declaration — the +// #5437 PQueue fix, avoiding a retained name being re-resolved by the +// static parent-chain walks to an unrelated same-named class. +// `synthesize_class_captures` (lower_decl/class_captures.rs) unions a +// parent's registered captures into the child's synthesized-constructor +// params keyed off that SAME `extends_name`; when it's `None`, the union +// never ran, so the subclass constructor never received the base's +// captured locals and every inherited method read `undefined` for them. +// +// This is exactly the shape `const Base = class {...}; const Sub = class +// extends Base {...}` produces (both class EXPRESSIONS assigned to +// locals) — esbuild/tsc's typical bundled-class emit, and what broke +// typescript 5.8.2's CJS `transpileModule` (`IdentifierNameMultiMap +// extends IdentifierNameMap`, both class expressions in `typescript.js`'s +// module wrapper, the subclass with its own `add`/`remove` methods). +// +// NOTE: this fix is scoped to a subclass that has at least one member of +// its own (a method, in this file) and NO explicit constructor of its own +// — see `explicitCtor` below, the pre-existing "already works" control +// this fix must not regress. A subclass with a completely empty body +// (`const Sub = class extends Base {};`), or a base class DECLARATION +// (rather than expression) as the extends target, hits a SEPARATE, unfixed +// codegen field-layout bug — see the PR body for #10486. +// +// Each function below uses its own distinct Base/Sub identifier names +// (Base1/Sub1, Base2/Sub2, ...): re-using the same literal name across +// sibling functions hits an unrelated, pre-existing collision in the +// class-capture registries (confirmed present on a build with NONE of this +// PR's changes) that is out of this PR's scope. + +function minimal(): void { + const baseCap = "base-capture"; + const subCap = "sub-capture"; + const Base1 = class { + m() { + return baseCap; + } + }; + const Sub1 = class extends Base1 { + n() { + return subCap; + } + }; + console.log("minimal", new Base1().m(), new Sub1().m(), new Sub1().n()); +} +minimal(); + +// Control: an explicit `constructor() { super(); }` on the subclass must +// keep working (pre-existing "Works" case; a naive union of parent +// captures into every subclass regressed exactly this shape during +// development of this fix — kept here as the regression guard). +function explicitCtor(): void { + const cap2 = "explicit-super-cap"; + const Base2 = class { + m() { + return cap2; + } + }; + const Sub2 = class extends Base2 { + constructor() { + super(); + } + }; + console.log("explicitCtor", new Sub2().m()); +} +explicitCtor(); + +// A captured HELPER function (not just a string) read from an inherited +// method on a subclass instance; subclass has its own (uncaptured) member. +function capturedHelper(): void { + function helper3(x: string): string { + return "[" + x + "]"; + } + const Base3 = class { + m() { + return helper3("base"); + } + }; + const Sub3 = class extends Base3 { + own() { + return "own"; + } + }; + console.log("capturedHelper", new Sub3().m(), new Sub3().own()); +} +capturedHelper(); From 4d7f89c45810c8adf5c82a04864a6b10fb4e08eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 18 Sep 2026 13:29:49 +0000 Subject: [PATCH 2/2] changelog: fragment for #10628 --- .../10628-class-expr-subclass-captures.md | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 changelog.d/10628-class-expr-subclass-captures.md diff --git a/changelog.d/10628-class-expr-subclass-captures.md b/changelog.d/10628-class-expr-subclass-captures.md new file mode 100644 index 0000000000..db9cd9ad50 --- /dev/null +++ b/changelog.d/10628-class-expr-subclass-captures.md @@ -0,0 +1,21 @@ +### Fixed + +- **An inherited method of a capture-bearing class expression could read + `undefined` captures when called on a subclass instance.** A subclass + with no explicit constructor, extending a capture-bearing class + EXPRESSION bound to a local (`const Base = class { m() { return cap; } }; + const Sub = class extends Base { n() {...} };`), never forwarded `Base`'s + captured enclosing-scope locals to the synthesized subclass constructor — + the lowering deliberately drops the static `extends_name` for such a + lexically-local heritage identifier (avoiding a same-named-class + collision, #5437), and capture propagation was keyed off that same name. + Capture forwarding now resolves the heritage identifier through the same + let/const class-alias table `new X()` construction already uses, scoped + to subclasses that have their own member and no explicit constructor of + their own (an explicit constructor already forwarded captures correctly + through a separate mechanism). This was blocking `typescript`'s CJS + `transpileModule` output (`IdentifierNameMultiMap extends + IdentifierNameMap`, both class expressions with their own methods, in the + bundled `typescript.js`). A subclass with a completely empty body, or + whose base is a class DECLARATION rather than expression, hits a + separate, still-open codegen field-layout gap (#10486).