From ad2eefe4cef5c0259abf6dd5b0ad830a9aca13ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 18 Sep 2026 13:18:06 +0000 Subject: [PATCH 1/2] fix(transform): give an exported dynamic-heritage class factory real per-evaluation identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A class expression returned from a function (a mixin/factory — function withCommands(Base) { return class extends Base {}; }) had no per-evaluation identity when the function lived in a non-entry module: every call returned the SAME shared-template class object, re-parented to the most recently passed Base. specialize_captured_class_factories already fixes this for same-module callers by cloning a distinct class per call site, but it only ever sees call sites in the SAME module as the factory -- a caller in another module reaches the factory through an ordinary cross-module call that pass never visits, so an exported factory's own template stayed shared and got silently re-parented on each call. Give an exported factory real per-evaluation identity directly: when its body is nothing but the single-statement return class extends {} shape, upgrade the class's own ClassRef to ClassExprFresh, exactly what the same class expression would already lower to had it needed per-evaluation statics/captures/a private brand. This closes the gap for every caller, local or cross-module, without touching the existing same-module specialization (a locally-cloned call site never calls the factory at runtime at all, so it is unaffected). --- .../src/inline/factory_specialize.rs | 76 +++++++++++++++++ crates/perry-transform/src/inline/mod.rs | 12 ++- ...0455_class_expr_factory_identity_helper.ts | 11 +++ ...t_gap_10455_class_expr_factory_identity.ts | 82 +++++++++++++++++++ 4 files changed, 180 insertions(+), 1 deletion(-) create mode 100644 test-files/gap_10455_class_expr_factory_identity_helper.ts create mode 100644 test-files/test_gap_10455_class_expr_factory_identity.ts diff --git a/crates/perry-transform/src/inline/factory_specialize.rs b/crates/perry-transform/src/inline/factory_specialize.rs index 4eb17a0e16..9c3d8c2574 100644 --- a/crates/perry-transform/src/inline/factory_specialize.rs +++ b/crates/perry-transform/src/inline/factory_specialize.rs @@ -1078,3 +1078,79 @@ pub fn specialize_captured_class_factories(module: &mut Module) { // Flush new specialized classes. module.classes.extend(new_classes); } + +/// #10455: `specialize_captured_class_factories` above only rewrites a +/// factory's CALL SITES, and only visits the module the Call expression +/// appears in — a caller in ANOTHER module reaches the factory through an +/// ordinary cross-module call this pass never sees. An exported factory +/// (`export function withCommands(Base) { return class extends Base {}; }`, +/// redis's `commander.js` `attachConfig` mixin shape) then keeps returning +/// the ONE shared-template `ClassRef` every local call above would +/// otherwise have cloned, and each call's `RegisterClassParentDynamic` +/// silently re-parents that single shared class in place: `withCommands(A) +/// === withCommands(B)` where the spec requires two distinct classes, and +/// an earlier caller's result is corrupted by a later call. +/// +/// Give an exported factory real per-EVALUATION identity directly, instead +/// of relying on caller-side cloning that cannot reach outside the module: +/// when its body is nothing but `return class extends {…}` — lowered +/// to `Sequence([RegisterClassParentDynamic { class_name, parent_expr }, +/// ClassRef(class_name)])` by `crates/perry-hir/src/lower/lower_expr/ +/// arm_class.rs` — upgrade the trailing `ClassRef` to `ClassExprFresh`, +/// exactly what the same class expression would have lowered to had it +/// needed per-evaluation statics/captures/a private brand. +/// `named_statics`/`computed_keys`/`captured_args`/static blocks/private +/// elements/self-binding are all empty here by construction: any of those +/// would already have forced `arm_class.rs` onto the `ClassExprFresh` route +/// at lowering time, so a class that still presents as a bare `ClassRef` +/// never had them. +/// +/// Scoped to this exact single-statement direct-return shape — the filed +/// repro's own pattern. A Let-bound intermediate variable, computed-name +/// evaluations, or Effect's object-literal wrapper shape are left to the +/// same-module handling above; those still work correctly for a caller in +/// the SAME module as the factory (the specialization this file already +/// performs), just not yet for a caller reached only across modules. +pub fn fresh_export_dynamic_heritage_factories(module: &mut Module) { + if module.exported_functions.is_empty() { + return; + } + let exported: HashSet = module + .exported_functions + .iter() + .map(|(_, id)| *id) + .collect(); + for f in &mut module.functions { + if !exported.contains(&f.id) { + continue; + } + let [Stmt::Return(Some(Expr::Sequence(parts)))] = f.body.as_mut_slice() else { + continue; + }; + if parts.len() != 2 { + continue; + } + let is_dynamic_heritage_classref = matches!( + (&parts[0], &parts[1]), + ( + Expr::RegisterClassParentDynamic { class_name: reg, .. }, + Expr::ClassRef(rf), + ) if reg == rf + ); + if !is_dynamic_heritage_classref { + continue; + } + let Expr::ClassRef(template) = parts.remove(1) else { + unreachable!("matched Expr::ClassRef(_) above"); + }; + parts.push(Expr::ClassExprFresh { + template, + evaluation_owner: None, + named_statics: Vec::new(), + computed_keys: Vec::new(), + computed_statics: Vec::new(), + static_init_order: Vec::new(), + captured_args: Vec::new(), + }); + } +} diff --git a/crates/perry-transform/src/inline/mod.rs b/crates/perry-transform/src/inline/mod.rs index 219d36c3d2..11a313a1bb 100644 --- a/crates/perry-transform/src/inline/mod.rs +++ b/crates/perry-transform/src/inline/mod.rs @@ -44,7 +44,9 @@ pub(crate) use exact_receivers::{ collect_module_prototype_facts, intersect_exact_receiver_facts, invalidate_exact_receivers_for_expr, kill_referenced_exact_receivers, }; -pub(crate) use factory_specialize::specialize_captured_class_factories; +pub(crate) use factory_specialize::{ + fresh_export_dynamic_heritage_factories, specialize_captured_class_factories, +}; pub(crate) use imul::{detect_math_imul_polyfill, rewrite_imul_calls_in_stmts}; pub(crate) use substitute::{ collect_body_local_ids, substitute_locals, substitute_locals_in_stmts, substitute_this, @@ -497,6 +499,14 @@ fn inline_functions_inner( // no-op for the rewritten sites. specialize_captured_class_factories(module); + // #10455: same-module call sites above are cloned per call site; + // an EXPORTED factory also needs real per-evaluation identity for + // callers outside this module, which no per-module call-site pass + // can see. See `fresh_export_dynamic_heritage_factories`'s own doc + // comment for the exact shape and why it's safe to run after the + // pass above. + fresh_export_dynamic_heritage_factories(module); + // Phases 0 + 1 fused (Tier 4.1, v0.5.335): single iteration over // module.functions collects both Math.imul polyfill ids AND // inlinable-function candidates. Pre-Tier-4 these were two separate diff --git a/test-files/gap_10455_class_expr_factory_identity_helper.ts b/test-files/gap_10455_class_expr_factory_identity_helper.ts new file mode 100644 index 0000000000..38d01cd9f9 --- /dev/null +++ b/test-files/gap_10455_class_expr_factory_identity_helper.ts @@ -0,0 +1,11 @@ +// Non-entry module half of test_gap_10455_class_expr_factory_identity.ts — +// the exact shape redis's `commander.js` `attachConfig` uses +// (`Class = class extends BaseClass {}`), reached through an ordinary +// cross-module call. +export function withCommands(Base: any) { + return class extends Base {}; +} + +export function withCommandsB(Base: any) { + return class extends Base {}; +} diff --git a/test-files/test_gap_10455_class_expr_factory_identity.ts b/test-files/test_gap_10455_class_expr_factory_identity.ts new file mode 100644 index 0000000000..f34bb45184 --- /dev/null +++ b/test-files/test_gap_10455_class_expr_factory_identity.ts @@ -0,0 +1,82 @@ +// #10455: a class expression returned from a function had no per-evaluation +// identity when the function lived in a NON-ENTRY module — every call +// returned the SAME class object, re-parented to the most recently passed +// `Base`. A mixin/factory declared and called in the entry module (or +// specialized per call site by `specialize_captured_class_factories`) always +// worked; the same shape reached through an ordinary cross-module call did +// not, because the per-module specialization pass never sees callers outside +// its own module. redis's `commander.js` `attachConfig` (`Class = class +// extends BaseClass {}`) hits exactly this from `RedisClient.factory` and +// `Client.prototype.Multi = MultiCommand.extend(config)`. +import { withCommands, withCommandsB } from "./gap_10455_class_expr_factory_identity_helper.ts"; + +class A { + constructor() { + console.log(" A constructor"); + } + hello() { + return "A.hello"; + } +} +class B { + constructor() { + console.log(" B constructor"); + } + world() { + return "B.world"; + } +} + +function localWithCommands(Base: any) { + return class extends Base {}; +} + +// ── entry-module factory: two calls, two evaluations ── +const LocalA = localWithCommands(A); +const LocalB = localWithCommands(B); +const la = new LocalA(); +console.log( + "entry module: LocalA !== LocalB:", + LocalA !== LocalB, + "| instanceof A:", + la instanceof A, + "| typeof hello:", + typeof (la as any).hello, +); + +// ── non-entry-module factory: two calls, two evaluations ── +const ClientA = withCommands(A); +const ClientB = withCommands(B); +const ca = new ClientA(); +const cb = new ClientB(); +console.log( + "non-entry module: ClientA !== ClientB:", + ClientA !== ClientB, + "| instanceof A:", + ca instanceof A, + "| typeof hello:", + typeof (ca as any).hello, +); +console.log( + "non-entry module: cb instanceof B:", + cb instanceof B, + "| cb instanceof A:", + cb instanceof A, + "| typeof world:", + typeof (cb as any).world, +); +// ── three sequential calls at the SAME call site (loop) also stay distinct ── +const made: any[] = []; +for (let i = 0; i < 3; i++) { + made.push(withCommandsB(i % 2 === 0 ? A : B)); +} +console.log( + "loop calls pairwise distinct:", + made[0] !== made[1] && made[1] !== made[2] && made[0] !== made[2], +); +console.log( + "loop instances match their own parent:", + new made[0]() instanceof A, + new made[1]() instanceof B, + new made[2]() instanceof A, +); From 324ec2feefd40affea111cd6452ff996d6230d05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 18 Sep 2026 13:20:12 +0000 Subject: [PATCH 2/2] docs(changelog): record the exported class-factory identity fix (#10622) --- .../10622-exported-class-factory-identity.md | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 changelog.d/10622-exported-class-factory-identity.md diff --git a/changelog.d/10622-exported-class-factory-identity.md b/changelog.d/10622-exported-class-factory-identity.md new file mode 100644 index 0000000000..19a45f1078 --- /dev/null +++ b/changelog.d/10622-exported-class-factory-identity.md @@ -0,0 +1,26 @@ +### Fixed + +- **A class expression returned from a factory function had no + per-evaluation identity when the function lived in a non-entry module** + (#10455). `function withCommands(Base) { return class extends Base {}; }` + called twice with different `Base`s returned the SAME class object, + re-parented to the most recently passed `Base` — declared and called in + the entry module, the identical shape already worked because + `specialize_captured_class_factories` (`crates/perry-transform/src/inline/factory_specialize.rs`) + clones a distinct class per call site, but that pass only ever sees call + sites in the SAME module the factory Call expression appears in; a caller + in another module reaches the factory through an ordinary cross-module + call the pass never visits. redis's `commander.js` `attachConfig` (`Class + = class extends BaseClass {}`) hits this every time it's called with a + second `BaseClass`, from `RedisClient.factory` and + `Client.prototype.Multi`, and `new Client(options)` ran the wrong parent + constructor. New pass `fresh_export_dynamic_heritage_factories`: for an + **exported** factory whose body is exactly `return class extends + {};` (no other members), upgrades the class's shared-template `ClassRef` + to a fresh-per-evaluation `ClassExprFresh` object directly — exactly what + the same class expression would already lower to had it needed + per-evaluation statics/captures/a private brand — so every caller, local + or cross-module, gets a genuinely distinct class per call. Scoped to + exported functions and this single-statement shape only; a `Let`-bound + intermediate variable or Effect's object-literal wrapper factory shape + still rely on same-module specialization alone.