diff --git a/changelog.d/9466-scope-aware-class-disambiguation.md b/changelog.d/9466-scope-aware-class-disambiguation.md new file mode 100644 index 0000000000..69e6525a2b --- /dev/null +++ b/changelog.d/9466-scope-aware-class-disambiguation.md @@ -0,0 +1,59 @@ +### Fixed + +- **Same-name `class` declarations at different lexical depths are distinct + classes again — the inner body is no longer silently dropped.** Perry keeps + two same-named classes apart by registering the second under a uniquified key + (`M$0`); the key was minted once per **name** instead of once per **scope**, so + the third and every later `class M` aliased onto the second's ClassId. Whichever + body registered first won, and the program ran the wrong methods with no + diagnostic: + + ```ts + class M { v() { return "top"; } } + function h() { + class M { v() { return "outer"; } } + function h2() { class M { v() { return "inner"; } } return new M().v(); } + return [new M().v(), h2()].join(","); + } + console.log(new M().v(), h()); // node: top outer,inner perry: top outer,outer + ``` + + Two defects, one symptom: + + 1. **The "already renamed" guard was per name, not per scope.** + `maybe_rename_colliding_class` returned early on + `class_renames.contains_key(name)` — but `class_renames` is inherited by + nested bodies (it is saved and restored per body, so an enclosing body's + alias is live while the nested one lowers). A nested body declaring the same + name therefore took the early return and registered its `class X` under the + **outer** body's key. The map now carries the source span of the scope that + minted each alias, so the guard means "this scope already renamed it" — the + idempotence the guard existed for — and every nested scope mints its own. + + 2. **Block scopes never ran the disambiguation scan at all.** Only function + bodies did, so two sibling `{ class Blk { … } }` blocks shared one ClassId + and the second ran the first's body: + + ```ts + { class Blk { v() { return "b1"; } } console.log(new Blk().v()); } // b1 + { class Blk { v() { return "b2"; } } console.log(new Blk().v()); } // node b2, perry b1 + ``` + + `class` is block-scoped, so the scan is now bracketed at every `{ … }`-shaped + scope — bare block, `if` / `else` branch, loop body, `try` / `catch` / + `finally`, and `switch` — mirroring `register_block_forward_lexicals` + (#6062), which brackets the same boundary for TDZ names: record only what the + scope changed, undo exactly that, so an alias owned by an enclosing scope + survives. + + This is an **identity** fix, not a naming one: each declaration now gets its own + ClassId, so `instanceof` across the shadowing boundary is correct in both + directions (an inner instance is not `instanceof` the outer class, and vice + versa), `Object.getPrototypeOf(inner) !== Outer.prototype`, and `class Sub + extends M` inside the inner scope extends the **inner** `M`. `.name` keeps + reporting the source name for all of them — the display-name override #9413 + (PR #9465) installed on this exact path is what carries it, and every newly + minted alias goes through the same `lower_class_decl` site that records it. + + Validated by `test-files/test_gap_9466_shadowed_class_identity.ts`, byte-identical + to `node --experimental-strip-types`. diff --git a/crates/perry-hir/src/lower/context.rs b/crates/perry-hir/src/lower/context.rs index 532ead4347..4c045db809 100644 --- a/crates/perry-hir/src/lower/context.rs +++ b/crates/perry-hir/src/lower/context.rs @@ -466,20 +466,53 @@ impl LoweringContext { pub(crate) fn resolve_class_name(&self, name: &str) -> String { self.class_renames .get(name) - .cloned() + .map(|(registration_key, _)| registration_key.clone()) .unwrap_or_else(|| name.to_string()) } - /// Register a scope-local rename for `class X` when an outer/prior `class X` - /// is already registered (a distinct class that the name-keyed dedup would - /// otherwise skip). Returns immediately if no collision or already aliased. - /// Call from each body's Phase-1.5 class scan. - pub(crate) fn maybe_rename_colliding_class(&mut self, name: &str) { - if self.lookup_class(name).is_some() && !self.class_renames.contains_key(name) { - let unique = format!("{}${}", name, self.next_class_rename_id); - self.next_class_rename_id += 1; - self.class_renames.insert(name.to_string(), unique); + /// Mint a scope-local rename for `class X` when an outer/prior `class X` is + /// already registered (a distinct class that the name-keyed dedup would + /// otherwise skip). Returns `Some(displaced entry)` when it minted — so the + /// caller can restore exactly what it replaced — and `None` when no rename + /// was needed. + /// + /// #9466: the "already renamed" guard is per SCOPE, not per name. It used + /// to be `!class_renames.contains_key(name)`, but `class_renames` inherits + /// every enclosing body's aliases, so a NESTED body declaring the same name + /// took that branch and registered its class under the OUTER body's key. + /// The two source classes then shared one ClassId, whichever body lowered + /// first won, and the other's members silently vanished — no diagnostic. + /// Keying on `scope_key` (the declaring scope's source span) keeps the + /// idempotence the old guard existed for — one alias per scope, so a scope + /// scanned twice (a function body: Phase-1.5, then `lower_block_stmt`) + /// mints exactly once — while letting each nested scope mint its own. + pub(crate) fn mint_class_rename( + &mut self, + name: &str, + scope_key: u32, + ) -> Option> { + if self.lookup_class(name).is_none() { + return None; + } + if self + .class_renames + .get(name) + .is_some_and(|(_, key)| *key == scope_key) + { + return None; } + let unique = format!("{}${}", name, self.next_class_rename_id); + self.next_class_rename_id += 1; + Some( + self.class_renames + .insert(name.to_string(), (unique, scope_key)), + ) + } + + /// Single-name entry point for the function-body Phase-1.5 class scans, + /// which snapshot and restore the whole `class_renames` map themselves. + pub(crate) fn maybe_rename_colliding_class(&mut self, name: &str, scope_key: u32) { + let _ = self.mint_class_rename(name, scope_key); } /// Is `name` a user-declared `interface`? Interfaces are not classes, so diff --git a/crates/perry-hir/src/lower/expr_function.rs b/crates/perry-hir/src/lower/expr_function.rs index cbbbf6953e..16e1c3d0c5 100644 --- a/crates/perry-hir/src/lower/expr_function.rs +++ b/crates/perry-hir/src/lower/expr_function.rs @@ -1151,7 +1151,7 @@ fn lower_fn_expr_anon(ctx: &mut LoweringContext, fn_expr: &ast::FnExpr) -> Resul // shape `(function(e){…class s{…}…})(t)` declares superstruct's // `Struct` = `class s`, which collided with other `class s` in // the bundle and was dedup-skipped). See `class_renames`. - ctx.maybe_rename_colliding_class(class_decl.ident.sym.as_str()); + ctx.maybe_rename_colliding_class(class_decl.ident.sym.as_str(), block.span.lo.0); let cname = class_decl.ident.sym.to_string(); ctx.forward_class_decl_depth .entry(cname.clone()) diff --git a/crates/perry-hir/src/lower/lowering_context.rs b/crates/perry-hir/src/lower/lowering_context.rs index f744350041..ef37621c37 100644 --- a/crates/perry-hir/src/lower/lowering_context.rs +++ b/crates/perry-hir/src/lower/lowering_context.rs @@ -820,9 +820,21 @@ pub struct LoweringContext { /// name-keyed: `Expr::New { class_name }` / `ClassRef(name)`). When a body /// declares `class X` while an outer/prior `class X` is already registered, /// the body's X is renamed `X$` and `X -> X$` recorded so every - /// reference in that body binds to the lexically-correct class. Saved/ - /// restored per body in both `lower_fn_body_block_stmt` and `lower_fn_expr`. - pub(crate) class_renames: std::collections::HashMap, + /// reference in that body binds to the lexically-correct class. + /// + /// The value is `(registration key, scope key)`, where the scope key is the + /// source span of the LEXICAL SCOPE that minted the alias. #9466: without + /// it there was one alias per NAME — a nested body inherited the enclosing + /// body's alias, saw "already renamed", and registered its own `class X` + /// under the OUTER body's key, so the two classes shared one ClassId and + /// the inner body was silently dropped. With it there is one alias per + /// (name, scope), which is what a lexical declaration actually is. + /// + /// Bracketed at every scope that can declare a class: the function-body + /// Phase-1.5 scans in `lower_fn_body_block_stmt` / `lower_fn_expr` + /// (whole-map snapshot + restore), and `enter_class_rename_scope` / + /// `exit_class_rename_scope` for `{ … }`-shaped block scopes. + pub(crate) class_renames: std::collections::HashMap, /// Monotonic suffix source for `class_renames` unique names. pub(crate) next_class_rename_id: u32, /// Names of TOP-LEVEL `class X { … }` declarations in the module being diff --git a/crates/perry-hir/src/lower/stmt.rs b/crates/perry-hir/src/lower/stmt.rs index e0a0802be2..5acbea4c5a 100644 --- a/crates/perry-hir/src/lower/stmt.rs +++ b/crates/perry-hir/src/lower/stmt.rs @@ -1822,9 +1822,19 @@ pub(crate) fn lower_stmt( // Case statement-lists share the switch's block scope without // being a `BlockStmt`, so they don't pass through // `lower_block_stmt` — re-bind their pre-registered - // forward-captured lets here (all cases up front: one scope). + // forward-captured lets here (all cases up front: one scope), and + // (#9466) disambiguate the `class` declarations they hold for the + // same reason. Every case shares ONE lexical scope, so they take + // one shared scope key: a second case re-declaring the name is a + // redeclaration, not a shadow. + let mut saved_class_renames = Vec::new(); for case in &switch_stmt.cases { rebind_nested_forward_scope_lets(ctx, &case.cons); + saved_class_renames.extend(enter_class_rename_scope( + ctx, + switch_stmt.span.lo.0, + &case.cons, + )); } for case in &switch_stmt.cases { @@ -1838,6 +1848,7 @@ pub(crate) fn lower_stmt( cases.push(SwitchCase { test, body }); } + exit_class_rename_scope(ctx, saved_class_renames); ctx.pop_block_scope(switch_scope_mark); module.init.push(Stmt::Switch { diff --git a/crates/perry-hir/src/lower_decl/block.rs b/crates/perry-hir/src/lower_decl/block.rs index f9cae5ccc9..c2315806a2 100644 --- a/crates/perry-hir/src/lower_decl/block.rs +++ b/crates/perry-hir/src/lower_decl/block.rs @@ -20,7 +20,22 @@ pub(crate) use var_names::{ pub fn lower_block_stmt(ctx: &mut LoweringContext, block: &ast::BlockStmt) -> Result> { rebind_nested_forward_scope_lets(ctx, &block.stmts); - lower_stmts_using_aware(ctx, &block.stmts) + // #9466: `class` is block-scoped, so a `class X` here is a DISTINCT class + // from any enclosing/prior `class X` and needs its own registration key. + // This is the funnel every `{}`-shaped scope shares — bare block, `if` / + // `else` branch, loop body, `try` / `catch` / `finally` — the same set + // `rebind_nested_forward_scope_lets` documents. Bracketed so the alias dies + // with the block. + // + // Keyed on the block's span: a FUNCTION body arrives here after + // `lower_fn_body_block_stmt`'s Phase-1.5 scan already aliased this same + // block, and the matching key makes this call a no-op rather than a second + // alias — which would strand that function's end-of-body capture + // re-registration on the now-stale key. + let saved_class_renames = enter_class_rename_scope(ctx, block.span.lo.0, &block.stmts); + let lowered = lower_stmts_using_aware(ctx, &block.stmts); + exit_class_rename_scope(ctx, saved_class_renames); + lowered } /// Make the forward-captured `let`/`const` bindings that @@ -524,7 +539,7 @@ pub fn lower_fn_body_block_stmt( // Disambiguate a distinct same-named class declared in this body so // its references don't bind to a colliding `class X` elsewhere in // the bundled module (see `class_renames`). - ctx.maybe_rename_colliding_class(class_decl.ident.sym.as_str()); + ctx.maybe_rename_colliding_class(class_decl.ident.sym.as_str(), block.span.lo.0); let cname = class_decl.ident.sym.to_string(); // Record the (shallowest) scope depth this class is declared at so a // later bare-ident reference can compare it against a same-named @@ -1025,13 +1040,22 @@ pub fn lower_block_stmt_scoped( block: &ast::BlockStmt, ) -> Result> { let mark = ctx.push_block_scope(); + // #9466: the strict-mode branch does NOT route through `lower_block_stmt`, + // so the block-scoped class disambiguation is bracketed here, around both + // branches. On the non-strict path `lower_block_stmt`'s own bracket sees + // this same span key and is a no-op. + let saved_class_renames = enter_class_rename_scope(ctx, block.span.lo.0, &block.stmts); // Via `lower_block_stmt` so this scope's pre-registered forward-captured // lets are re-bound at entry (`rebind_nested_forward_scope_lets`). let stmts = if ctx.current_strict { - lower_strict_block_fn_decls(ctx, block)? + lower_strict_block_fn_decls(ctx, block) } else { - lower_block_stmt(ctx, block)? + lower_block_stmt(ctx, block) }; + exit_class_rename_scope(ctx, saved_class_renames); + // `?` deliberately AFTER the rename restore but BEFORE `pop_block_scope`, + // preserving this function's original error control flow exactly. + let stmts = stmts?; ctx.pop_block_scope(mark); Ok(stmts) } @@ -1160,6 +1184,58 @@ fn register_block_forward_lexicals(ctx: &mut LoweringContext, stmts: &[ast::Stmt newly } +/// What one [`enter_class_rename_scope`] bracket displaced, keyed by source +/// name: `None` = the name had no active alias, `Some(entry)` = the enclosing +/// scope's alias to put back. See `LoweringContext::class_renames`. +pub(crate) type ClassRenameScopeSave = Vec<(String, Option<(String, u32)>)>; + +/// #9466: scope-entry hook for a `{ … }`-shaped lexical scope — disambiguate +/// every `class X` declared DIRECTLY in `stmts` and return what to hand to +/// [`exit_class_rename_scope`] on the way out. +/// +/// A `class` declaration is block-scoped, so a bare block, an `if`/`else` +/// branch, a loop body, a `try` / `catch` / `finally` block and a `switch` body +/// each shadow an enclosing same-named class exactly as a `let` does. Before +/// #9466 only FUNCTION bodies ran the disambiguation scan, so two sibling +/// `{ class X { … } }` blocks registered one ClassId between them and the +/// second block silently ran the first's body. +/// +/// Deliberately mirrors [`register_block_forward_lexicals`] (#6062), which +/// brackets this same boundary for TDZ names: record only what this call +/// changed and undo exactly that, so an alias owned by an enclosing scope +/// survives the block. +pub(crate) fn enter_class_rename_scope( + ctx: &mut LoweringContext, + scope_key: u32, + stmts: &[ast::Stmt], +) -> ClassRenameScopeSave { + let mut saved = ClassRenameScopeSave::new(); + for stmt in stmts { + let ast::Stmt::Decl(ast::Decl::Class(class_decl)) = stmt else { + continue; + }; + let name = class_decl.ident.sym.as_str(); + if let Some(displaced) = ctx.mint_class_rename(name, scope_key) { + saved.push((name.to_string(), displaced)); + } + } + saved +} + +/// Undo an [`enter_class_rename_scope`] bracket, innermost mint first. +pub(crate) fn exit_class_rename_scope(ctx: &mut LoweringContext, saved: ClassRenameScopeSave) { + for (name, displaced) in saved.into_iter().rev() { + match displaced { + Some(entry) => { + ctx.class_renames.insert(name, entry); + } + None => { + ctx.class_renames.remove(&name); + } + } + } +} + pub fn lower_stmts_using_aware( ctx: &mut LoweringContext, stmts: &[ast::Stmt], diff --git a/crates/perry-hir/src/lower_decl/body_stmt.rs b/crates/perry-hir/src/lower_decl/body_stmt.rs index 59c2b347b5..f9be1c6e73 100644 --- a/crates/perry-hir/src/lower_decl/body_stmt.rs +++ b/crates/perry-hir/src/lower_decl/body_stmt.rs @@ -1025,9 +1025,19 @@ fn lower_body_stmt_impl(ctx: &mut LoweringContext, stmt: &ast::Stmt) -> Result Result string> = []; +{ + class Cap { v() { return "cap1"; } } + closures.push(() => new Cap().v()); +} +{ + class Cap { v() { return "cap2"; } } + closures.push(() => new Cap().v()); +} +function capFn() { + class Cap { v() { return "cap-fn"; } } + return () => new Cap().v(); +} +closures.push(capFn()); +class Cap { v() { return "cap-top"; } } +closures.push(() => new Cap().v()); +console.log("closures:", closures.map((f) => f()).join(",")); + +// --- 4. instanceof across the shadowing boundary -------------------------- +class P { tag() { return "P-top"; } } +const topP = new P(); +function innerP() { + class P { tag() { return "P-inner"; } } + const p = new P(); + return { + inst: p, + ownIsInner: p instanceof P, + topIsInner: topP instanceof P, + cls: P as any, + }; +} +const r = innerP(); +console.log("io own-inner:", r.ownIsInner); +console.log("io top-is-inner:", r.topIsInner); +console.log("io inner-is-top:", r.inst instanceof P); +console.log("io top-is-top:", topP instanceof P); +console.log("io ctor-identity:", r.cls === P); +console.log("io proto-identity:", Object.getPrototypeOf(r.inst) === P.prototype); +console.log("io tags:", topP.tag(), r.inst.tag()); + +// --- 5. `.name` stays the SOURCE name for every one of them (#9413) ------- +function nameA() { class N {} return N.name; } +function nameB() { class N {} return N.name; } +function nameC() { function d() { class N {} return N.name; } return d(); } +class N {} +console.log("names:", N.name, nameA(), nameB(), nameC()); +console.log("ctor-names:", new M().constructor.name, r.inst.constructor.name); + +// --- 6. subclassing a shadowed class inside the inner scope --------------- +class B { who() { return "B-top"; } } +class SubTop extends B {} +function innerSub() { + class B { who() { return "B-inner"; } } + class Sub extends B { both() { return this.who() + "/sub"; } } + const s = new Sub(); + return [ + s.who(), + s.both(), + String(s instanceof B), + String(s instanceof Sub), + String(s instanceof SubTop), + ].join(","); +} +console.log("sub-top:", new SubTop().who(), new SubTop() instanceof B); +console.log("sub-inner:", innerSub()); + +// --- 7. switch: a bare case statement-list shares ONE switch block scope --- +class Sw { v() { return "sw-top"; } } +function switchBare(k: number) { + switch (k) { + case 1: + class Sw { v() { return "sw-case"; } } + return new Sw().v(); + default: + return "none"; + } +} +console.log("switch-bare:", switchBare(1), switchBare(2), new Sw().v()); + +// A braced case is its own block scope on top of the switch's. +class Sw2 { v() { return "sw2-top"; } } +function switchBraced(k: number) { + switch (k) { + case 1: { class Sw2 { v() { return "c1"; } } return new Sw2().v(); } + case 2: { class Sw2 { v() { return "c2"; } } return new Sw2().v(); } + default: return new Sw2().v(); + } +} +console.log("switch-braced:", switchBraced(1), switchBraced(2), switchBraced(3)); + +// --- 8. loop body: ONE declaration site, so ONE class for every iteration --- +// (the disambiguation is keyed on the declaration's source span, and every +// iteration shares that span). The closures must still hold the inner class +// after the loop exits, and the post-loop `new Lp()` must get the OUTER one. +class Lp { v() { return "lp-top"; } } +function loopSameClass() { + const fs: Array<() => string> = []; + for (let i = 0; i < 3; i++) { + class Lp { v() { return "lp-body"; } } + fs.push(() => new Lp().v()); + } + return fs.map((f) => f()).join(",") + "|" + new Lp().v(); +} +console.log("loop-same-class:", loopSameClass()); + +// NOT covered here: the same loop body where the class CAPTURES the loop +// variable (`class Cp { v() { return "cp" + i; } }`). Node gives one class +// with three environments (`cp0,cp1,cp2`); perry gives `cp3,cp3,cp3` because +// a class carries ONE `RegisterClassCaptures` snapshot, refreshed at +// assignments and returns — neither of which a loop body has. That is the +// class-capture mechanism, not class identity: it reproduces with NO name +// shadowing anywhere (`class Uniq` declared in a loop body, nothing else +// named Uniq in the program) and is byte-identical before and after this fix. +// Reported separately so this fixture keeps discriminating exactly one thing. + +// --- 9. instanceof across a BLOCK boundary, and at the THIRD depth --------- +// Arm 4's instanceof rows sit at two-scope depth, which the name-keyed +// disambiguation already handled; these two are where identity actually broke. +class Ib { tag() { return "ib-top"; } } +const ibTop = new Ib(); +let ibInnerInst: any = null; +let ibInnerCls: any = null; +{ + class Ib { tag() { return "ib-block"; } } + ibInnerInst = new Ib(); + ibInnerCls = Ib; +} +console.log("blk-io same-class:", ibInnerCls === Ib); +console.log("blk-io inner-is-top:", ibInnerInst instanceof Ib); +console.log("blk-io top-is-inner:", ibTop instanceof ibInnerCls); +console.log("blk-io proto:", Object.getPrototypeOf(ibInnerInst) === Ib.prototype); +console.log("blk-io tags:", ibTop.tag(), ibInnerInst.tag()); + +function ioDepth() { + class Id { tag() { return "id-1"; } } + function inner() { + class Id { tag() { return "id-2"; } } + return { inst: new Id(), cls: Id as any }; + } + const deep = inner(); + return [ + deep.inst.tag(), + String(deep.cls === Id), + String(deep.inst instanceof Id), + String(new Id() instanceof deep.cls), + ].join(","); +} +class Id { tag() { return "id-top"; } } +console.log("depth-io:", ioDepth(), new Id().tag());