fix(class): new.target reflects the actual constructed class, not the enclosing one#5061
Merged
Merged
Conversation
… enclosing one
HIR hardcoded `new.target` inside a class constructor to a literal
`{ name: <enclosing-class> }` (expr_misc.rs / expr_member.rs /
lower_expr.rs). So `new.target` was always the class whose BODY runs —
for `new Derived()` that inlines `Base`'s constructor via super(), it
was `Base`, not `Derived` — and `new.target === SomeClass` was always
false (a fresh object never equals the class ref), breaking the
abstract-base-class guard idiom `if (new.target === Abstract) throw`.
Lower `new.target` (and `new.target.name` / `.prototype` / `?.` member
reads) to the real `Expr::NewTarget`. Codegen resolves it to the active
constructor's LEAF class ref (`INT32_TAG | class_id`, the same value
`Expr::ClassRef` produces) via a `new_target_stack` slot pushed around
the inlined constructor body — in both the ordinary inline-`new` path
(lower_call/new.rs) and the scalar-replacement path for a non-escaping
`const c = new C()` (stmt/let_stmt.rs). Using the codegen slot rather
than the runtime cell keeps a non-constructor method called from the
ctor body — compiled separately, empty new_target_stack — correctly
reading `undefined`.
Now matches Node: `new Derived()` new.target is the leaf class across
no-own-ctor / explicit-super / field-init-bearing / multi-level chains;
`new.target === C`, `.name`, `.prototype` all correct; abstract guards
fire; nested and scalar-replaced construction restore the outer target.
Surfaced while verifying #2768 (whose functional acceptance criteria —
newTarget honored, array-like args, constructor/proxy validation — all
pass on current main).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Surfaced while verifying #2768.
Bug
HIR hardcoded
new.targetinside a class constructor to a literal{ name: <enclosing-class> }(lower/expr_misc.rs, plus the.namemember folds inexpr_member.rs/lower_expr.rs). Two consequences:new.targetwas always the class whose body runs. Fornew Derived()whereDerived extends BaseandBase's constructor is inlined viasuper(),new.targetwasBase, notDerived.new.target === SomeClasswas alwaysfalse— a fresh object literal never equals the class reference — which silently breaks the abstract-base-class guard idiomif (new.target === Abstract) throw ….new.target.prototypewas also alwaysundefined.Fix
Lower
new.target(and its.name/.prototype/?.member reads) to the realExpr::NewTarget. Codegen resolves it to the leaf class ref (INT32_TAG | class_id— the same valueExpr::ClassRefproduces, so identity /.name/.prototypeall work) via anew_target_stackslot pushed around the inlined constructor body, in both construction paths:newpath (lower_call/new.rs), andconst c = new C()(stmt/let_stmt.rs).Using the codegen slot (not the runtime
js_new_target_*cell) keeps a non-constructor method called from the constructor body — compiled as a separate function with an emptynew_target_stack— correctly readingundefined.Validation (all byte-identical to
node --experimental-strip-types)new Derived()new.target is the leaf class across no-own-ctor, explicit-super(), field-bearing, and 3-level chains.new.target === Cidentity,.name,.prototype === C.prototypeall correct.if (new.target === Abstract) throwfires for the directnew Abstract()and not for a subclass.new Outer()whose ctor doesnew Inner()) and scalar-replacedconst c = new C()both restore the correct target;const t = new.targetbinding works.new.target === undefined(no leak).perry-codegen+perry-hirunit tests pass.Known remaining limitation
Reflect.construct(KnownClass, args, differentNewTarget)where the explicitnewTargetdiffers from the target still folds to inlinenew KnownClass(args)(HIRReflect.constructfold), sonew.targetinside isKnownClassrather thandifferentNewTarget. Threading a differing newTarget into a registered class's constructor needs the runtime class-ref construct path to honor the new.target cell (it already does for plain-function targets —Reflect.construct(fn, args, nt)is correct). Left as-is to avoid a runtime regression (the prior fold behavior is preserved, non-throwing); tracked under #2768.Code-only PR — version bump + changelog left for merge time.