fix(engine): run derived class field initializers after super() - #992
Draft
simonyang08 wants to merge 1 commit into
Draft
fix(engine): run derived class field initializers after super()#992simonyang08 wants to merge 1 commit into
simonyang08 wants to merge 1 commit into
Conversation
…ova#948) A user-written derived class constructor that declared instance fields used to throw ReferenceError: Uninitialized this binding because the field initializer prelude was emitted at the start of the constructor body, before super() had bound this. For derived classes, the prelude is now built as a separate executable and stored on the function. It is invoked from step 11 of EvaluateSuper after super() has bound this, mirroring the behaviour of default constructors. Base-class constructors keep the existing prelude-inside-body path because OrdinaryCallBindThis runs before the user body and so this is already initialized. Includes a regression script under tests/ that covers the original issue, single/multi-field cases, grand-child fields, and a base-class no-regression check. Signed-off-by: simonyang08 <ppt5928@gmail.com>
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.
Fixes #948
A user-written derived class constructor that declared instance fields used to throw
ReferenceError: Uninitialized this bindingbecause the field-initializer prelude was emitted at the start of the constructor body, beforesuper()had boundthis.Change
For derived classes, the field-initializer prelude is now built as a separate executable stored on the function and invoked from
EvaluateSuperaftersuper()has boundthis, mirroring how default constructors already behave. Base-class constructors keep the existing prelude-inside-body path becauseOrdinaryCallBindThisruns before the user body.The new helper
initialize_ecmascript_function_class_field_initializersmirrorsinitialize_instance_elements(used by default constructors) so future field-init environment changes should apply to both paths.Regression test
tests/class-field-init-in-derived.jscovers the original issue repro, single/multi-field cases, grand-child fields, a base-class no-regression check, and field visibility aftersuper()inside the constructor.Verification
cargo build --bin nova_cli --profile dev-fast— cleancargo clippy --bin nova_cli --profile dev-fast— 0 warningscargo fmt --check— cleanReferenceErrorbefore the change → completes normally afterScope note
This fix inherently threads new state through the bytecode compiler, VM, executable, and function data structures, so the diff spans 10 files (+233/-30) — larger than a typical focused patch, but it is a single semantic change. The GC mark/sweep handling for the new
class_field_initializer_bytecodefield is included.Signed-off-by: simonyang08 ppt5928@gmail.com