From 709a6e0b1362577ec7faa2083ac2770e1dbe72e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 23 Aug 2026 21:53:48 +0200 Subject: [PATCH] perf(codegen): return undefined from a ctor symbol that cannot replace this #8630 gave every standalone _constructor symbol a completion block ending in js_ctor_return_override(this, , ...), so a derived super() whose base hands back a replacement object can publish it. That also changed the ORDINARY constructor's return from `undefined` to `this`, which flipped every caller's `is_undef` fast arm from never-taken to always-taken. lower_call/new.rs states the invariant that stopped holding: "the fast arm ran no constructor, which is `undefined` -- the same thing an ordinary ctor body returns". The guarded call is not cheap: constructor_return_overrides_this probes the typed-array registry, the buffer registry, callability, the Proxy registry, `arguments`, clean_arr_ptr (which walks GC forwarding chains) and the GC header, per construction, to hand back the value the caller already had -- and under RS4GC it is a statepoint, so live pointers spill around it. This is #8648's second, independent cause. It is not an inheritance story: benchmarks/issue-8289/cycles.ts has no `extends` and was 1.68x. What decides who pays is ctor_prologue_stores, which skips the constructor call entirely for a body that is nothing but `this. = ` stores; one literal initializer (`this.peer = null`) or a super() disqualifies the plan. Publish `this` only when a replacement can exist. ctor_chain_can_replace_this (now shared, in new_helpers.rs) walks the heritage chain and answers true for a value-bearing return in any constructor on it, a native base, a dynamic extends, an id-only parent edge, or a class missing from ctx.classes. field_init.rs's own copy is replaced by it: that copy looked for the ctor in class.methods under the name "constructor", but HIR keeps it in class.constructor and never puts it in methods, so its value-returning arm could not fire. The now-superseded collectors::mutation copy, which also missed try/switch/for-of bodies, is deleted. Measured (instructions retired, vs the pre-#8630 numbers in the issue): two-class `new B(x, y)` loop 998,471,071 -> 1,648,244,291 -> 1,007,905,144 cycles.ts 1,301,925,013 -> 2,182,711,384 -> 1,330,975,040 plain-class control 381,756,402 -> 372,826,080 -> 373,056,337 1.65x -> 1.01x and 1.68x -> 1.02x, with byte-identical program output. Node 26.5.1 differential over 21 constructor-semantics cases: every one is byte-identical to what main prints, 18 of 21 match Node, and the 3 that do not fail identically on main. lower_call/ctor_return_publish_tests.rs pins all four directions at the IR level, since nothing behavioural can see this. Refs #8648. --- .../8648-ctor-symbol-returns-undefined.md | 70 +++++ crates/perry-codegen/src/codegen/method.rs | 44 +-- crates/perry-codegen/src/collectors/mod.rs | 4 +- .../perry-codegen/src/collectors/mutation.rs | 38 --- .../lower_call/ctor_return_publish_tests.rs | 267 ++++++++++++++++++ .../src/lower_call/field_init.rs | 36 +-- crates/perry-codegen/src/lower_call/mod.rs | 6 +- .../src/lower_call/new_helpers.rs | 45 +++ 8 files changed, 417 insertions(+), 93 deletions(-) create mode 100644 changelog.d/8648-ctor-symbol-returns-undefined.md create mode 100644 crates/perry-codegen/src/lower_call/ctor_return_publish_tests.rs diff --git a/changelog.d/8648-ctor-symbol-returns-undefined.md b/changelog.d/8648-ctor-symbol-returns-undefined.md new file mode 100644 index 0000000000..36562dacd8 --- /dev/null +++ b/changelog.d/8648-ctor-symbol-returns-undefined.md @@ -0,0 +1,70 @@ +Restored `ret undefined` for a constructor symbol that cannot replace `this`. + +This is the **second**, independent cause of #8648's regression — the one +`field_init.rs` (#8653) did not touch. It is not about inheritance, despite the +symptom set: `benchmarks/issue-8289/cycles.ts` has no `extends` anywhere and was +1.68x. + +#8630 gave every standalone `_constructor` symbol a completion block that +ends in `js_ctor_return_override(this, , …)`, so a derived `super()` +whose base hands back a replacement object can publish it to the caller. The +motivation is right, but it also changed the ORDINARY constructor's return value +from `undefined` to `this` — and `lower_call/new.rs` says in its own comment what +that costs: *"the fast arm ran no constructor, which is `undefined` — the same +thing an ordinary ctor body returns — so `emit_ctor_return_override` below yields +the instance on both arms"*. That sentence stopped being true. The caller's +`is_undef` test went from never-taken to always-taken, and the call it guards is +not cheap: `constructor_return_overrides_this` probes the typed-array registry, +the buffer registry, callability, the Proxy registry, `arguments`, `clean_arr_ptr` +(which walks GC forwarding chains) and finally the GC header — per construction, +to answer "yes, an object" and hand back the value the caller already had. Under +RS4GC the call is also a statepoint, so the live-pointer set spills and relocates +around it. + +Why the affected set looked like an inheritance story: `ctor_prologue_stores` +skips the constructor call entirely when the whole body is a run of +`this. = ` stores, so those classes never reach the symbol. That is +exactly `micro_ctor` and `tree_wide` (1.00x). One literal initializer +(`this.peer = null` in `cycles.ts`) or a `super()` disqualifies the plan, the +call is emitted, and the class pays. + +The fix publishes `this` only when a replacement can exist. `ctor_chain_can_replace_this` +(now shared, in `new_helpers.rs`) walks the heritage chain and answers `true` for a +value-bearing `return` in any constructor on it, a native base, a dynamic +`extends`, an id-only parent edge, or a class missing from `ctx.classes` — +conservative on every edge it cannot see. Everything else returns `undefined` +exactly as before #8630; every caller (`lower_call/new.rs`, the synthesized +default-derived path in `codegen/method.rs`, and the runtime construct paths, +which discard the value outright) already maps `undefined` onto its own receiver. + +`field_init.rs`'s own copy of this predicate is replaced by the shared one. The +copy looked for the constructor in `class.methods` under the name `"constructor"`; +HIR keeps it in `class.constructor` and never puts it in `methods`, so the +value-returning arm could not fire. The shared version also uses +`ctor_body_has_value_return`, which walks `try`/`switch`/`for-of` bodies that +`collectors::mutation`'s version did not — that now-superseded copy is deleted. + +Measured (instructions retired, `/usr/bin/time -l`, vs the pre-#8630 numbers in +the issue): + +| bench | pre-#8630 | main `c2da034` | this change | | +|---|---|---|---|---| +| two-class `new B(x, y)` loop | 998,471,071 | 1,648,244,291 | **1,007,905,144** | 1.65x -> **1.01x** | +| `cycles.ts` | 1,301,925,013 | 2,182,711,384 | **1,330,975,040** | 1.68x -> **1.02x** | +| plain-class control | 381,756,402 | 372,826,080 | 373,056,337 | 0.98x (unchanged) | + +Program output is byte-identical on all three. + +Differential against Node 26.5.1 (`--experimental-strip-types`), 21 constructor +semantics cases: every one is byte-identical to what `main` prints, and 18 of 21 +match Node. The three that do not (`super()` from inside an arrow, an uncaught +`ReferenceError` after a base ctor throws inside `try`, and `class X extends +Error`) fail identically on `main` — pre-existing, untouched by this change. The +cases that pin the semantics being preserved all match Node: a base constructor +returning a replacement object seen by a subclass field initializer, a derived +constructor returning an object, a derived constructor returning a primitive +(TypeError), a conditional value return, and a value return inside `try`. + +`lower_call/ctor_return_publish_tests.rs` pins all four directions at the IR +level — this is invisible to every behavioural test, since both spellings produce +the same program output. diff --git a/crates/perry-codegen/src/codegen/method.rs b/crates/perry-codegen/src/codegen/method.rs index 70435b62e5..daa864bc27 100644 --- a/crates/perry-codegen/src/codegen/method.rs +++ b/crates/perry-codegen/src/codegen/method.rs @@ -1202,6 +1202,15 @@ pub(super) fn compile_method( })?; } + // #8648: pre-#8630 this symbol ended in `ret undefined`, and every caller + // maps `undefined` onto its own receiver. Returning `this` instead flipped + // the callers' `js_ctor_return_override` check from never-taken to + // always-taken — a cross-crate call that runs the typed-array, buffer, + // callable, Proxy, arguments and array probes before answering "yes, an + // object" and handing back the value the caller already held. Publish only + // when a replacement `this` can actually exist. + let publishes_this = standalone_ctor_return.is_some() + && crate::lower_call::ctor_chain_can_replace_this(ctx.classes, &class.name); if let Some((target, after_idx)) = standalone_ctor_return.as_ref() { let _ = ctx .inline_ctor_return @@ -1215,23 +1224,24 @@ pub(super) fn compile_method( if !ctx.block().is_terminated() { let undef = crate::nanbox::double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)); - let return_value = if let Some((target, _)) = standalone_ctor_return.as_ref() { - let raw = ctx.block().load(DOUBLE, &target.result_slot); - let this_value = ctx - .this_stack - .last() - .cloned() - .map(|slot| ctx.block().load(DOUBLE, &slot)) - .unwrap_or_else(|| undef.clone()); - crate::lower_call::emit_ctor_return_override( - &mut ctx, - &this_value, - &raw, - target.is_derived, - ) - } else { - undef.clone() - }; + let return_value = + if let Some((target, _)) = standalone_ctor_return.as_ref().filter(|_| publishes_this) { + let raw = ctx.block().load(DOUBLE, &target.result_slot); + let this_value = ctx + .this_stack + .last() + .cloned() + .map(|slot| ctx.block().load(DOUBLE, &slot)) + .unwrap_or_else(|| undef.clone()); + crate::lower_call::emit_ctor_return_override( + &mut ctx, + &this_value, + &raw, + target.is_derived, + ) + } else { + undef.clone() + }; if ctx.shared_super_scope_active { ctx.block().call_void("js_derived_super_scope_pop", &[]); } diff --git a/crates/perry-codegen/src/collectors/mod.rs b/crates/perry-codegen/src/collectors/mod.rs index ad3f4b6dbc..0ae01715ea 100644 --- a/crates/perry-codegen/src/collectors/mod.rs +++ b/crates/perry-codegen/src/collectors/mod.rs @@ -82,9 +82,7 @@ pub(crate) use integer_locals::{ collect_flat_row_aliases, is_int32_producing_expr, static_index_window, }; pub(crate) use local_refs::{expr_contains_local_get, mark_all_candidate_refs_in_expr}; -pub(crate) use mutation::{ - body_contains_call, body_contains_closure, body_returns_value, has_any_mutation, -}; +pub(crate) use mutation::{body_contains_call, body_contains_closure, has_any_mutation}; pub(crate) use number_by_construction::collect_number_by_construction_locals; pub(crate) use param_ranges::{collect_param_int_ranges, ParamIntRanges}; pub(crate) use pointer_locals::collect_pointer_typed_locals; diff --git a/crates/perry-codegen/src/collectors/mutation.rs b/crates/perry-codegen/src/collectors/mutation.rs index 9f5b4015b9..18bfdd1f4e 100644 --- a/crates/perry-codegen/src/collectors/mutation.rs +++ b/crates/perry-codegen/src/collectors/mutation.rs @@ -57,44 +57,6 @@ pub fn body_contains_closure(stmts: &[perry_hir::Stmt]) -> bool { any_top_level_expr(stmts, &mut expr_contains_closure) } -/// #8648: can this constructor body hand back a replacement `this`? -/// -/// ECMAScript lets a constructor `return` an object, which becomes the -/// construction's result (`js_ctor_return_override`). That replacement may be a -/// Proxy, and DefineField must reach a Proxy's `defineProperty` trap rather -/// than its `set`. With no value-returning `return` anywhere on the chain, the -/// instance a field initializer writes to is provably the freshly allocated -/// ordinary object, so `CreateDataProperty` and a plain own-slot store agree. -pub fn body_returns_value(stmts: &[perry_hir::Stmt]) -> bool { - stmts_have_value_return(stmts) -} - -/// `Stmt::Return(Some(_))` at any statement depth. -fn stmts_have_value_return(stmts: &[perry_hir::Stmt]) -> bool { - use perry_hir::Stmt; - for s in stmts { - let hit = match s { - Stmt::Return(Some(_)) => true, - Stmt::If { - then_branch, - else_branch, - .. - } => { - stmts_have_value_return(then_branch) - || else_branch - .as_ref() - .is_some_and(|b| stmts_have_value_return(b)) - } - Stmt::While { body, .. } | Stmt::For { body, .. } => stmts_have_value_return(body), - _ => false, - }; - if hit { - return true; - } - } - false -} - fn expr_contains_closure(expr: &perry_hir::Expr) -> bool { if matches!(expr, perry_hir::Expr::Closure { .. }) { return true; diff --git a/crates/perry-codegen/src/lower_call/ctor_return_publish_tests.rs b/crates/perry-codegen/src/lower_call/ctor_return_publish_tests.rs new file mode 100644 index 0000000000..c3e61ba3f3 --- /dev/null +++ b/crates/perry-codegen/src/lower_call/ctor_return_publish_tests.rs @@ -0,0 +1,267 @@ +//! #8648: what a standalone `_constructor` symbol RETURNS. +//! +//! An IR-census test, and the "assert the subject was live" kind (CLAUDE.md): +//! nothing behavioural can see this. Both spellings produce the same program +//! output, because every caller of the symbol maps an `undefined` return onto +//! its own receiver — which for an ordinary class IS the value the other +//! spelling returns. The difference is only ever visible in a profile. +//! +//! #8630 made every constructor symbol end in +//! `js_ctor_return_override(this, , …)` so that a derived `super()` whose +//! base hands back a replacement object could publish it. Correct, but it also +//! changed the ORDINARY constructor from `ret undefined` to `ret this`, and the +//! callers' own `is_undef` fast arm — the one `lower_call/new.rs` documents as +//! "the fast arm ran no constructor, which is `undefined` — the same thing an +//! ordinary ctor body returns" — went from never-taken to always-taken. The +//! call it guards is not cheap: `constructor_return_overrides_this` probes the +//! typed-array registry, the buffer registry, callability, the Proxy registry, +//! `arguments`, `clean_arr_ptr` (which walks GC forwarding chains) and finally +//! the GC header, per construction, to answer "yes, an object" and hand back +//! the value the caller already held. Measured on a two-class `new B(x, y)` +//! loop: 1.65x, and on `benchmarks/issue-8289/cycles.ts` 1.68x. +//! +//! So the positive test asserts the ordinary constructor is back to +//! `ret undefined` with no override call at all, and the negative asserts the +//! publish survives for the one shape that needs it — a constructor that can +//! hand back a replacement `this`. + +use super::typed_shape_bake_tests::emit; +use perry_hir::types::Type; +use perry_hir::{Class, ClassField, Expr, Function, Module, Param, Stmt}; + +/// The NaN-boxed `undefined` literal codegen prints for `TAG_UNDEFINED`. +const RET_UNDEFINED: &str = "ret double 0x7FFC000000000001"; +const OVERRIDE_CALL: &str = "@js_ctor_return_override("; + +fn field(name: &str) -> ClassField { + ClassField { + name: name.to_string(), + key_expr: None, + ty: Type::Number, + init: None, + is_private: false, + is_readonly: false, + decorators: Vec::new(), + } +} + +fn param(id: u32, name: &str) -> Param { + Param { + id, + name: name.to_string(), + ty: Type::Number, + default: None, + decorators: Vec::new(), + is_rest: false, + arguments_object: None, + } +} + +/// `constructor(a) { this.v = a; }` +fn ctor(tail: Vec) -> Function { + let mut body = vec![Stmt::Expr(Expr::PutValueSet { + target: Box::new(Expr::This), + key: Box::new(Expr::String("v".to_string())), + value: Box::new(Expr::LocalGet(1)), + receiver: Box::new(Expr::This), + strict: false, + })]; + body.extend(tail); + Function { + id: 900, + name: "constructor".to_string(), + type_params: Vec::new(), + params: vec![param(1, "a")], + return_type: Type::Void, + body, + is_async: false, + is_generator: false, + is_strict: false, + is_exported: false, + captures: Vec::new(), + decorators: Vec::new(), + was_plain_async: false, + was_unrolled: false, + } +} + +fn class(id: u32, name: &str, extends_name: Option<&str>, ctor_tail: Vec) -> Class { + Class { + id, + name: name.to_string(), + type_params: Vec::new(), + extends: None, + extends_name: extends_name.map(str::to_string), + native_extends: None, + extends_expr: None, + heritage_lexically_shadowed: false, + fields: vec![field("v")], + constructor: Some(ctor(ctor_tail)), + methods: Vec::new(), + getters: Vec::new(), + setters: Vec::new(), + static_accessor_names: Vec::new(), + static_accessor_fn_ids: Vec::new(), + computed_members: Vec::new(), + static_fields: Vec::new(), + static_methods: Vec::new(), + decorators: Vec::new(), + is_exported: false, + aliases: Vec::new(), + is_nested: false, + alloc_width_hint: 0, + specialized_from: None, + } +} + +fn module(classes: Vec, construct: &str) -> Module { + let mut m = Module::new("ctor_return_publish.ts"); + m.classes = classes; + m.init = vec![Stmt::Let { + id: 20, + name: "x".to_string(), + ty: Type::Named(construct.to_string()), + mutable: false, + init: Some(Expr::New { + class_name: construct.to_string(), + args: vec![Expr::Integer(1)], + type_args: Vec::new(), + byte_offset: 0, + cap_args_appended: 0, + }), + }]; + m +} + +/// The body of the `define` for `_constructor`, `}`-terminated. +fn constructor_body(ir: &str, name: &str) -> String { + let needle = format!("_{name}_constructor("); + let define = ir + .split("\ndefine ") + .find(|chunk| { + chunk + .split('\n') + .next() + .is_some_and(|h| h.contains(&needle)) + }) + .unwrap_or_else(|| panic!("no `{name}_constructor` in the emitted IR:\n{ir}")); + let end = define.find("\n}").expect("unterminated define"); + define[..end].to_string() +} + +/// Every `ret` instruction in `body`, trimmed. The `ret` is NOT the last line +/// of the text: codegen emits blocks in creation order, so the constructor's +/// completion block precedes the field-store diamonds it was created before. +fn returns(body: &str) -> Vec { + body.lines() + .map(str::trim) + .filter(|line| line.starts_with("ret ")) + .map(str::to_string) + .collect() +} + +#[test] +fn an_ordinary_constructor_symbol_returns_undefined() { + let ir = emit(&module( + vec![class(404, "Plain", None, Vec::new())], + "Plain", + )); + let body = constructor_body(&ir, "Plain"); + assert!( + !body.contains(OVERRIDE_CALL), + "a constructor that cannot hand back a replacement `this` still emits \ + the return-override publish, so its callers' own `is_undef` fast arm \ + is dead and every construction pays \ + `constructor_return_overrides_this`:\n{body}" + ); + assert_eq!( + returns(&body), + vec![RET_UNDEFINED.to_string()], + "the ordinary constructor symbol no longer returns `undefined`; every \ + caller maps that onto its own receiver, and returning `this` instead \ + is what #8648's second regression was:\n{body}" + ); +} + +/// The same class with `extends`, whose parent also cannot replace `this`. +/// A derived constructor is not by itself a reason to publish. +#[test] +fn a_derived_constructor_with_an_ordinary_base_returns_undefined() { + let ir = emit(&module( + vec![ + class(404, "Base", None, Vec::new()), + class( + 405, + "Sub", + Some("Base"), + vec![Stmt::Expr(Expr::SuperCall(vec![Expr::LocalGet(1)]))], + ), + ], + "Sub", + )); + let body = constructor_body(&ir, "Sub"); + // Only the RETURN is asserted here. `super()` inlines the parent body and + // emits its own return-override diamond over the parent's completion slot; + // with an ordinary parent that slot is a compile-time `undefined`, so LLVM + // folds the diamond away and the call never reaches the binary. The publish + // this ticket is about is the one over the SYMBOL's own result. + assert_eq!( + returns(&body), + vec![RET_UNDEFINED.to_string()], + "expected the only `ret` to be `undefined` for an ordinary derived \ + constructor:\n{body}" + ); +} + +/// The shape #8630 added the publish for: a constructor whose `return` can +/// hand back a different object. The publish MUST survive — this is the +/// negative control that stops the elision from being unconditional. +#[test] +fn a_value_returning_constructor_still_publishes_its_this() { + let ir = emit(&module( + vec![class( + 404, + "Swap", + None, + vec![Stmt::Return(Some(Expr::LocalGet(1)))], + )], + "Swap", + )); + let body = constructor_body(&ir, "Swap"); + assert!( + body.contains(OVERRIDE_CALL), + "a constructor with a value-bearing `return` dropped its \ + return-override publish, so `new Swap(...)` would keep the \ + provisional allocation instead of the returned object:\n{body}" + ); +} + +/// And through an ancestor: the leaf's own body is ordinary, but its base can +/// replace `this`, so the leaf must still publish what `super()` bound. +#[test] +fn a_value_returning_base_makes_its_subclass_publish() { + let ir = emit(&module( + vec![ + class( + 404, + "SwapBase", + None, + vec![Stmt::Return(Some(Expr::LocalGet(1)))], + ), + class( + 405, + "Leaf", + Some("SwapBase"), + vec![Stmt::Expr(Expr::SuperCall(vec![Expr::LocalGet(1)]))], + ), + ], + "Leaf", + )); + let body = constructor_body(&ir, "Leaf"); + assert!( + body.contains(OVERRIDE_CALL), + "the leaf stopped publishing although its base returns a value — the \ + replacement `this` bound by `super()` would never reach the \ + caller:\n{body}" + ); +} diff --git a/crates/perry-codegen/src/lower_call/field_init.rs b/crates/perry-codegen/src/lower_call/field_init.rs index 002f710568..c61826233c 100644 --- a/crates/perry-codegen/src/lower_call/field_init.rs +++ b/crates/perry-codegen/src/lower_call/field_init.rs @@ -836,7 +836,8 @@ pub(crate) fn apply_field_initializers_recursive( // `PropertySet` path (inline shape precheck -> direct slot store) // exactly as this did before #8630. Anything else keeps the full // DefineField call. - let chain_can_replace_this = chain_constructor_returns_value(ctx, &class_name_in_chain); + let chain_can_replace_this = + crate::lower_call::ctor_chain_can_replace_this(ctx.classes, &class_name_in_chain); let no_accessor_on_chain = crate::type_analysis::class_field_global_index(ctx, &class_name_in_chain, &prop) .is_some(); @@ -914,36 +915,3 @@ pub(crate) fn apply_field_initializers_recursive( #[cfg(test)] mod tests; - -/// #8648: does any constructor on `leaf`'s inheritance chain `return` a value? -/// -/// A value-returning constructor can hand back a replacement `this` -/// (`js_ctor_return_override`), and that replacement may be a Proxy — which -/// DefineField must reach through `defineProperty`, not `set`. Conservative on -/// every edge it cannot see: a native base, a dynamic `extends`, or a class -/// missing from `ctx.classes` all answer `true`. -fn chain_constructor_returns_value(ctx: &crate::expr::FnCtx<'_>, leaf: &str) -> bool { - let mut name = leaf.to_string(); - for _ in 0..32 { - let Some(class) = ctx.classes.get(&name).copied() else { - return true; - }; - if class.native_extends.is_some() || class.extends_expr.is_some() { - return true; - } - if let Some(ctor) = class.methods.iter().find(|m| m.name == "constructor") { - if crate::collectors::body_returns_value(&ctor.body) { - return true; - } - } - // `extends` is a class id; `extends_name` is the textual parent. Only - // the latter can be followed through `ctx.classes`, so an id-only edge - // is treated as unknown. - match class.extends_name.as_ref() { - Some(parent) => name = parent.clone(), - None if class.extends.is_some() => return true, - None => return false, - } - } - true -} diff --git a/crates/perry-codegen/src/lower_call/mod.rs b/crates/perry-codegen/src/lower_call/mod.rs index f7d3a34af3..4ebba36f4e 100644 --- a/crates/perry-codegen/src/lower_call/mod.rs +++ b/crates/perry-codegen/src/lower_call/mod.rs @@ -47,6 +47,10 @@ mod console_rooting_tests; #[cfg(test)] mod ctor_prologue_store_tests; mod ctor_prologue_stores; +/// #8648: what a standalone `_constructor` symbol RETURNS — see the +/// module header for why this can only be asserted on IR. +#[cfg(test)] +mod ctor_return_publish_tests; mod dataview_intrinsic; mod early_branches; mod event_target; @@ -153,7 +157,7 @@ pub(crate) use new_ctor_args::{ // super-must-be-called. pub(crate) use new_helpers::{ ctor_body_calls_super, ctor_body_closure_calls_super, ctor_body_has_value_return, - ctor_body_uses_this, + ctor_body_uses_this, ctor_chain_can_replace_this, }; // #6325 / #6326: the class-chain walk to a native base whose surface perry // stamps onto the instance, plus its init emitter. Shared by the implicit diff --git a/crates/perry-codegen/src/lower_call/new_helpers.rs b/crates/perry-codegen/src/lower_call/new_helpers.rs index 1ba9e15d68..d13d721c42 100644 --- a/crates/perry-codegen/src/lower_call/new_helpers.rs +++ b/crates/perry-codegen/src/lower_call/new_helpers.rs @@ -476,6 +476,51 @@ pub(crate) fn ctor_body_has_value_return(body: &[perry_hir::Stmt]) -> bool { ) } +/// #8648: can `new (…)` complete with a `this` other than the instance +/// the allocator just handed out? +/// +/// Exactly three things substitute the receiver, and all three are statically +/// decidable from the heritage chain: +/// +/// * a constructor anywhere on the chain with a value-bearing `return` — the +/// `js_ctor_return_override` route, and the only way a Proxy or any other +/// foreign object can become the construction result; +/// * a native base (`native_extends`) or a heritage the compiler cannot resolve +/// (`extends_expr`, an id-only `extends` edge, a class this module never +/// declared), whose `super()` materializes an exotic object. +/// +/// Conservative on every edge it cannot see: unknown answers `true`. +pub(crate) fn ctor_chain_can_replace_this( + classes: &std::collections::HashMap, + leaf: &str, +) -> bool { + let mut name = leaf.to_string(); + for _ in 0..32 { + let Some(class) = classes.get(&name).copied() else { + return true; + }; + if class.native_extends.is_some() || class.extends_expr.is_some() { + return true; + } + if class + .constructor + .as_ref() + .is_some_and(|ctor| ctor_body_has_value_return(&ctor.body)) + { + return true; + } + // `extends` is a class id; `extends_name` is the textual parent. Only + // the latter can be followed through `classes`, so an id-only edge is + // treated as unknown. + match class.extends_name.as_ref() { + Some(parent) => name = parent.clone(), + None if class.extends.is_some() => return true, + None => return false, + } + } + true +} + pub(super) fn node_stream_parent_kind( ctx: &FnCtx<'_>, class: &perry_hir::Class,