From 2cc8a60712d9596d4396b12ec7b8c36a2d173ac3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 7 Jul 2026 19:22:07 +0200 Subject: [PATCH] fix(hir): drop static extends_name for lexically-shadowed class heritage `class A extends Base` where `Base` is an in-scope lexical local (a let/const/param), not a class, is heritage-shadowed: the parent is a runtime value resolved dynamically via `extends_expr`. Lowering already captured `extends_expr` and set `extends = None`, but still left `extends_name = Some("Base")`. The many STATIC parent-chain walks in codegen (packed-keys field layout, `js_register_class_parent` edge, inherited-method / vtable install, type-facts) re-resolve that bare name through the module-wide name->class map, binding to an UNRELATED same-named class elsewhere in the module -- e.g. a function-local `class Base` that leaked into the global map. In a large minified program this mis-bound `let Y = _?.Parent ?? Object; class A extends Y {}` to a captured function-local iterator class also named `Y` (declaring a private `#q`), so `A` instances inherited that class's layout/methods and a `this.#q` access threw "Cannot access private member from an object whose class did not declare it" on a legal receiver. Fix: for a lexically-shadowed heritage, leave both `extends` and `extends_name` `None` (matching the fully-dynamic `class X extends ` shape). The parent edge is wired at runtime via `js_register_class_parent_dynamic` and `super()` runs through `extends_expr` + `heritage_lexically_shadowed`. The super-call codegen gate is updated to proceed via `extends_expr` when `extends_name` is absent, so a shadowed subclass's `super()` still runs its (dynamic) parent constructor. Adds HIR lowering regression tests: a lexically-shadowed heritage lowers to `extends_name = None` + `extends_expr = Some`, while a plain class-to-class heritage keeps its static `extends_name`. --- .../perry-codegen/src/expr/this_super_call.rs | 22 ++++-- crates/perry-hir/src/lower/tests.rs | 75 +++++++++++++++++++ crates/perry-hir/src/lower_decl/class_decl.rs | 31 +++++--- 3 files changed, 111 insertions(+), 17 deletions(-) diff --git a/crates/perry-codegen/src/expr/this_super_call.rs b/crates/perry-codegen/src/expr/this_super_call.rs index ba46f6f212..76a8ceb8da 100644 --- a/crates/perry-codegen/src/expr/this_super_call.rs +++ b/crates/perry-codegen/src/expr/this_super_call.rs @@ -261,12 +261,24 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { return Ok(double_literal(0.0)); } }; - let Some(parent_name) = current_class.extends_name.as_deref().map(|s| s.to_string()) - else { - for a in super_args { - let _ = lower_expr(ctx, a)?; + let parent_name = match current_class.extends_name.as_deref() { + Some(s) => s.to_string(), + // A lexically-shadowed / fully-dynamic parent carries no + // `extends_name` (the parent is a runtime value, not a named + // class) but DOES carry `extends_expr`. Proceed with an empty + // placeholder name — for this shape `static_parent_lookup` below + // is forced to `None` (extends_expr present) and the builtin-name + // gate is disabled by `heritage_lexically_shadowed`, so the name + // is never consulted; `super()` dispatches via `extends_expr`. + // Without this, `super()` in such a subclass silently no-ops and + // the (dynamic) parent constructor never runs. + None if current_class.extends_expr.is_some() => String::new(), + None => { + for a in super_args { + let _ = lower_expr(ctx, a)?; + } + return Ok(double_literal(0.0)); } - return Ok(double_literal(0.0)); }; // #5437 (Next.js p-queue `PQueue`): when HIR captured a dynamic // `extends_expr` for this class, the parent is a LEXICAL runtime diff --git a/crates/perry-hir/src/lower/tests.rs b/crates/perry-hir/src/lower/tests.rs index 5e9a3f5bc8..58226a70d3 100644 --- a/crates/perry-hir/src/lower/tests.rs +++ b/crates/perry-hir/src/lower/tests.rs @@ -430,3 +430,78 @@ fn test_lower_accepts_chain_under_limit() { ); }); } + +/// A `class A extends Base` whose parent Ident is an in-scope LEXICAL LOCAL +/// (a `let`/`const`/param), not a class, must be lowered with NO static +/// `extends_name` — the parent is resolved purely dynamically via +/// `extends_expr`. Retaining a static `extends_name` lets the codegen +/// parent-chain walks (packed-keys field layout, `js_register_class_parent` +/// edge, inherited-method / vtable install, type-facts) re-resolve the bare +/// name through the module-wide name→class map to an UNRELATED same-named class +/// — e.g. a function-local `class Base` that leaked into that map — corrupting +/// the subclass's field layout and inheritance. (Regression: a large minified +/// program's zod `let Y=_?.Parent??Object; class A extends Y{}` wrongly +/// inherited a captured iterator class `Y`'s private `#q`, throwing "Cannot +/// access private member from an object whose class did not declare it".) +#[test] +fn test_lexically_shadowed_heritage_drops_static_extends_name() { + let source = r#" + function make(spec) { + let Base = (spec && spec.Parent) || Object; + class A extends Base {} + return A; + } + "#; + 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 a = hir + .classes + .iter() + .find(|c| c.name == "A") + .expect("class A is lowered"); + assert!( + a.heritage_lexically_shadowed, + "`Base` is a lexical local, so `class A extends Base` is lexically shadowed" + ); + assert_eq!( + a.extends_name, None, + "a lexically-shadowed heritage must NOT retain a static extends_name — \ + it would re-resolve to an unrelated same-named class" + ); + assert_eq!( + a.extends, None, + "no static parent class id for a dynamically-resolved parent" + ); + assert!( + a.extends_expr.is_some(), + "the parent is resolved dynamically via extends_expr" + ); +} + +/// A normal subclass whose parent is a CLASS DECLARATION (not a local) is +/// unaffected by the shadowed-heritage handling: class declarations are not in +/// `ctx.locals`, so the heritage is NOT lexically shadowed and static parent +/// resolution (field/method inheritance) is preserved. +#[test] +fn test_plain_class_to_class_heritage_keeps_static_extends_name() { + let source = r#" + class Base { x = 1; } + class Sub extends Base { y = 2; } + "#; + 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 sub = hir + .classes + .iter() + .find(|c| c.name == "Sub") + .expect("class Sub is lowered"); + assert!( + !sub.heritage_lexically_shadowed, + "a class-declaration parent is not a lexical local" + ); + assert_eq!( + sub.extends_name.as_deref(), + Some("Base"), + "static class-to-class heritage keeps its extends_name" + ); +} diff --git a/crates/perry-hir/src/lower_decl/class_decl.rs b/crates/perry-hir/src/lower_decl/class_decl.rs index 6b6d842254..7ae25796d4 100644 --- a/crates/perry-hir/src/lower_decl/class_decl.rs +++ b/crates/perry-hir/src/lower_decl/class_decl.rs @@ -450,10 +450,16 @@ pub fn lower_class_decl( // Lexical local shadow → dynamic parent via `extends_expr` (the // in-scope local value), invoked by `super()` through // `js_fetch_or_value_super`. See the class-expression arm below - // for the full rationale (Next.js p-queue `PQueue`). + // for the full rationale (Next.js p-queue `PQueue`). Leave + // `extends_name` None too: the parent Ident is a lexical LOCAL, + // not a class; a retained name is re-resolved by the static + // parent-chain walks (layout / parent-edge / inherited-method / + // type-facts) to an UNRELATED same-named class, corrupting the + // subclass. Matches the fully-dynamic `class X extends + // ` shape (`extends`+`extends_name` both None). match lower_expr(ctx, super_class) { - Ok(expr) => (None, Some(parent_name), None, Some(Box::new(expr))), - Err(_) => (None, Some(parent_name), None, None), + Ok(expr) => (None, None, None, Some(Box::new(expr))), + Err(_) => (None, None, None, None), } } else { // #5437 (Next.js NodeNextRequest cross-module heritage): a @@ -1548,16 +1554,17 @@ pub fn lower_class_from_ast( // rename exists (that disambiguation is exact). Pure-Ident // module-global heritage (no shadowing local) is unaffected — // `ctx.locals.lookup` returns `None` for a class name. - // Do NOT set a static `extends` (parent_cid) here: the only - // candidate would be `lookup_class(parent_name)`, which is the - // wrong same-named module-global class we are deliberately - // avoiding (wiring it would mis-route inherited-method / vtable - // dispatch to that class's members). The dynamic `extends_expr` - // path registers the correct parent edge at runtime via - // `RegisterClassParentDynamic` + `function_class_id`. + // Do NOT set a static `extends` (parent_cid) OR `extends_name` + // here: the only candidate is `lookup_class(parent_name)`, the + // wrong same-named module-global class we deliberately avoid — and + // a retained `extends_name` is re-resolved back to it by the + // static parent-chain walks (layout / parent-edge / inherited- + // method / vtable / type-facts), corrupting the subclass. The + // dynamic `extends_expr` path registers the correct parent edge at + // runtime via `RegisterClassParentDynamic` + `function_class_id`. match lower_expr(ctx, super_class) { - Ok(expr) => (None, Some(parent_name), None, Some(Box::new(expr))), - Err(_) => (None, Some(parent_name), None, None), + Ok(expr) => (None, None, None, Some(Box::new(expr))), + Err(_) => (None, None, None, None), } } else { // #5437: resolve the parent through active scope-local class