fix(runtime,codegen): #2768 — honor new.target in registered-class constructors

crates/perry-codegen/src/lower_call/new.rs

@@ -13,7 +13,8 @@ use super::field_init::{apply_field_initializers_recursive, FieldInitMode};
 use super::lower_builtin_new;
 use super::new_helpers::{
     collect_decl_local_ids, ctor_body_calls_super, ctor_body_closure_calls_super,
-    ctor_body_has_value_return, ctor_body_uses_this, node_stream_parent_kind,
+    ctor_body_has_value_return, ctor_body_uses_new_target, ctor_body_uses_this,
+    node_stream_parent_kind,
 };
 use crate::expr::{lower_expr, lower_js_args_array, nanbox_pointer_inline, FnCtx};
 use crate::nanbox::{double_literal, POINTER_MASK_I64};
@@ -331,6 +332,44 @@ fn local_constructor_symbol_exists(ctx: &FnCtx<'_>, class: &perry_hir::Class) ->
         .contains_key(&(class.name.clone(), ctor_method_name))
 }
 
+/// #2768: true when the standalone `<class>_constructor` symbol's body reads
+/// `new.target` — either the class's OWN ctor body, or an ancestor ctor body
+/// it reaches through `super(...)`. The symbol is a separately compiled
+/// function whose only `new.target` source is the runtime cell, and a
+/// `super(...)` call inlines the parent ctor body into that same symbol, so an
+/// ancestor that reads `new.target` (e.g. an abstract-class guard in a base)
+/// still observes the cell. Gating the cell write on the WHOLE chain keeps
+/// `new Child()` correct when only the inherited body reads `new.target`, while
+/// a chain with no reader anywhere stays on the zero-overhead fast path. The
+/// walk follows `extends_name` through the codegen class map; an unresolved
+/// parent name just stops the walk, and a depth cap guards a cyclic graph.
+fn ctor_chain_uses_new_target(ctx: &FnCtx<'_>, class: &perry_hir::Class) -> bool {
+    let reads = |c: &perry_hir::Class| {
+        c.constructor
+            .as_ref()
+            .is_some_and(|f| ctor_body_uses_new_target(&f.body))
+    };
+    if reads(class) {
+        return true;
+    }
+    let mut parent = class.extends_name.as_deref();
+    let mut depth = 0;
+    while let Some(parent_name) = parent {
+        depth += 1;
+        if depth > 64 {
+            break;
+        }
+        let Some(pc) = ctx.classes.get(parent_name).copied() else {
+            break;
+        };
+        if reads(pc) {
+            return true;
+        }
+        parent = pc.extends_name.as_deref();
+    }
+    false
+}
+
 /// Emit a call to the shared standalone `<class>_constructor` symbol and
 /// return the raw value it produced. The standalone ctor function returns
 /// `undefined` for an ordinary constructor (implicit `return this`) or the
@@ -1095,13 +1134,46 @@ fn lower_new_impl(
         // function ("value is not a function" on `new Chalk(...).red(...)`).
         // `js_ctor_return_override` returns `obj_box` for an `undefined`/
         // primitive (base) return, so ordinary ctors are unaffected.
+        //
+        // #2768/new.target: the standalone `<class>_constructor` symbol is a
+        // separate compiled function, so its only `new.target` source is the
+        // runtime cell — which this path never set, leaving `new.target ===
+        // undefined` for a base class. Set the cell to this class's ref (the
+        // `INT32_TAG | class_id` value `Expr::ClassRef` produces) around the
+        // call and restore it after, but ONLY when the ctor actually reads
+        // `new.target`, so the common ctor keeps the zero-overhead fast path.
+        // The gate spans the WHOLE super(...) chain, not just the leaf's own
+        // body: the symbol inlines `super(...)` into itself, so an ancestor
+        // ctor that reads `new.target` (e.g. an abstract-class guard in a base)
+        // observes the same cell — `new Child()` where only `Base` reads
+        // `new.target` would otherwise see `undefined` instead of `Child`.
+        // ponytail: a throw inside the ctor skips the restore, leaving the cell
+        // set — same edge case the runtime construct paths already have; fix
+        // holistically if it bites.
+        let saved_new_target = if ctor_chain_uses_new_target(ctx, class) {
+            ctx.class_ids.get(class_name).map(|&cid| {
+                let prev = ctx.block().call(DOUBLE, "js_new_target_get", &[]);
+                let class_ref = double_literal(f64::from_bits(
+                    crate::nanbox::INT32_TAG | (cid as u64 & 0xFFFF_FFFF),
+                ));
+                ctx.block()
+                    .call(DOUBLE, "js_new_target_set", &[(DOUBLE, &class_ref)]);
+                prev
+            })
+        } else {
+            None
+        };
         if let Some(ctor_ret) = call_local_constructor_symbol(
             ctx,
             class,
             &obj_box,
             &lowered_args,
             caps_absent_from_args,
         ) {
+            if let Some(prev) = &saved_new_target {
+                ctx.block()
+                    .call(DOUBLE, "js_new_target_set", &[(DOUBLE, prev)]);
+            }
             let is_derived = class.extends.is_some()
                 || class.extends_name.is_some()
                 || class.native_extends.is_some()
@@ -1118,6 +1190,10 @@ fn lower_new_impl(
             );
             return Ok(final_box);
         }
+        if let Some(prev) = &saved_new_target {
+            ctx.block()
+                .call(DOUBLE, "js_new_target_set", &[(DOUBLE, prev)]);
+        }
         return Ok(obj_box);
     }
 

crates/perry-codegen/src/lower_call/new_helpers.rs

@@ -168,6 +168,37 @@ fn expr_uses_this_direct(expr: &Expr) -> bool {
     found
 }
 
+/// #2768: true when the constructor body reads `new.target` — directly, or
+/// lexically from an arrow/closure that captured it. The default `new C()`
+/// path calls the standalone `<class>_constructor` symbol (a separate compiled
+/// function whose only `new.target` source is the runtime cell), so the cell
+/// must be set around that call. Gating on this keeps the common ctor (no
+/// `new.target`) on the zero-overhead fast path — no per-`new`-site cell writes.
+pub(crate) fn ctor_body_uses_new_target(body: &[perry_hir::Stmt]) -> bool {
+    ctor_body_any(body, &expr_uses_new_target, NO_STMT_PRED)
+}
+
+fn expr_uses_new_target(expr: &Expr) -> bool {
+    match expr {
+        Expr::NewTarget => true,
+        // A closure's precomputed flag is authoritative; don't descend (the
+        // walk below would otherwise re-scan its body).
+        Expr::Closure {
+            captures_new_target,
+            ..
+        } => *captures_new_target,
+        _ => {
+            let mut found = false;
+            perry_hir::walker::walk_expr_children(expr, &mut |child| {
+                if !found && expr_uses_new_target(child) {
+                    found = true;
+                }
+            });
+            found
+        }
+    }
+}
+
 /// True when the constructor body contains a value-bearing `return` in its
 /// own body (closures excluded; a bare `return undefined` does NOT count —
 /// spec falls back to the uninitialized `this` and still throws). The

crates/perry-runtime/src/object/class_registry/construct.rs

@@ -852,7 +852,9 @@ pub unsafe extern "C" fn js_new_function_construct(
     // registered class id and replay the standalone constructor so field
     // initializers and `this.foo = ...` writes match static `new ClassName()`.
     if let Some(class_cid) = constructor_class_ref_id(func_value) {
-        return construct_registered_class_ref(class_cid, class_cid, args_ptr, args_len);
+        return construct_registered_class_ref(
+            class_cid, class_cid, func_value, args_ptr, args_len,
+        );
     }
     if is_arrow_function_value(func_value) {
         crate::fs::validate::throw_type_error_with_code(
@@ -1079,6 +1081,7 @@ fn new_target_class_id(new_target: f64) -> Option<u32> {
 unsafe fn construct_registered_class_ref(
     target_cid: u32,
     instance_cid: u32,
+    new_target: f64,
     args_ptr: *const f64,
     args_len: usize,
 ) -> f64 {
@@ -1087,9 +1090,32 @@ unsafe fn construct_registered_class_ref(
     } else {
         js_object_alloc(instance_cid, 0)
     };
+    // #2768: a registered-class constructor reached through this path — static
+    // `new ClassName()`, a first-class ClassRef `new`, or `Reflect.construct`
+    // with a distinct newTarget — must observe `new.target` inside its body.
+    // The function-construct paths set the NEW_TARGET cell (read by codegen's
+    // `js_new_target_get`) around the call; this path replayed the constructor
+    // without it, so `new.target` was `undefined` for a base class and the
+    // explicit `Reflect.construct` newTarget never reached the body. Mirror the
+    // other paths: set the cell to the constructor (or the Reflect newTarget)
+    // around the replay, then restore.
+    //
+    // ponytail: the cell is process-global, so a non-constructor function called
+    // synchronously from the ctor body reads it too and sees the newTarget
+    // instead of `undefined`. This matches the pre-existing plain-function
+    // construct paths (which already set the cell the same way) — the codegen
+    // `new_target_stack` slot avoids this for fully-inlined `new`, but the
+    // replayed ctor is a separate compiled function that can only read the cell.
+    // Fix holistically with the slot mechanism if it ever bites.
+    let prev_new_target = crate::object::js_new_target_get();
+    crate::object::js_new_target_set(new_target);
+    let prev_current_new_target =
+        CURRENT_NEW_TARGET.with(|value| value.replace(new_target.to_bits()));
     super::super::class_constructors::replay_registered_class_constructor(
         target_cid, inst, args_ptr, args_len,
     );
+    CURRENT_NEW_TARGET.with(|value| value.set(prev_current_new_target));
+    crate::object::js_new_target_set(prev_new_target);
     // ClassRef `new` of a Request/Response subclass — attach the native fetch
     // handle on the dynamic path (mirrors the class-expression arm above).
     if let Some(kind) = fetch_parent_kind_in_chain(target_cid) {
@@ -1172,7 +1198,7 @@ pub unsafe extern "C" fn js_new_function_construct_with_new_target(
     }
     if let Some(target_cid) = constructor_class_ref_id(func_value) {
         let instance_cid = new_target_class_id(nt).unwrap_or(target_cid);
-        return construct_registered_class_ref(target_cid, instance_cid, args_ptr, args_len);
+        return construct_registered_class_ref(target_cid, instance_cid, nt, args_ptr, args_len);
     }
     // `Reflect.construct(Int8Array, [len], newTarget)` — a typed-array
     // constructor invoked with a distinct newTarget. Build the typed array the

test-parity/node-suite/object/reflect-proxy-construct.ts

@@ -126,3 +126,90 @@ show("proxy class empty handler newTarget", () => {
   const instance: any = Reflect.construct(Wrapped, ["y"], Child);
   return [instance instanceof Child, instance instanceof Thing, instance.child()];
 });
+
+class CtorNewTarget {
+  ntName: string;
+  constructor() {
+    this.ntName = (new.target as any)?.name ?? "undefined";
+  }
+}
+class OtherNewTarget {}
+
+show("Reflect.construct new.target inside class ctor", () => {
+  const a: any = Reflect.construct(CtorNewTarget, [], OtherNewTarget);
+  return a.ntName;
+});
+
+show("ClassRef new new.target inside class ctor", () => {
+  const Ref: any = CtorNewTarget;
+  const a: any = new Ref();
+  return a.ntName;
+});
+
+show("static new new.target inside base class ctor", () => {
+  return new CtorNewTarget().ntName;
+});
+
+function freeProbe(): string {
+  return (new.target as any) === undefined ? "undef" : "leaked";
+}
+class CallsFreeProbe {
+  probed: string;
+  constructor() {
+    this.probed = freeProbe();
+  }
+}
+
+show("free fn called from static-new ctor sees undefined new.target", () => {
+  return new CallsFreeProbe().probed;
+});
+
+// #2768: a subclass whose OWN ctor body never reads `new.target` still runs the
+// inherited base ctor (via `super()`) inlined into its standalone symbol. The
+// base reads `new.target`, so `new Child()` must observe `Child`, not undefined
+// — the symbol-call new.target gate must span the whole super(...) chain.
+class NtBase {
+  ntName: string;
+  constructor() {
+    this.ntName = (new.target as any)?.name ?? "undefined";
+  }
+}
+class NtChild extends NtBase {
+  extra: number;
+  constructor() {
+    super();
+    this.extra = 1;
+  }
+}
+class NtNoCtorChild extends NtBase {}
+
+show("static new on own-ctor subclass: base ctor sees leaf new.target", () => {
+  return new NtChild().ntName;
+});
+
+show("static new on no-own-ctor subclass: base ctor sees leaf new.target", () => {
+  return new NtNoCtorChild().ntName;
+});
+
+// An abstract-class guard living in the base must still fire for `new Base()`
+// but NOT for `new Sub()` whose own ctor forwards through `super()`.
+class AbstractBase {
+  constructor() {
+    if (new.target === AbstractBase) throw new TypeError("abstract");
+  }
+}
+class ConcreteSub extends AbstractBase {
+  tag: number;
+  constructor() {
+    super();
+    this.tag = 7;
+  }
+}
+
+show("abstract-base guard: new Sub() does not trip base new.target guard", () => {
+  return new ConcreteSub().tag;
+});
+
+show("abstract-base guard: new Base() trips base new.target guard", () => {
+  return new AbstractBase();
+});