fix(engine): run derived class field initializers after super()

nova_vm/src/ecmascript/abstract_operations/operations_on_objects.rs

@@ -9,11 +9,11 @@ use core::ops::ControlFlow;
 use crate::{
     ecmascript::{
         Agent, ArgumentsList, Array, BUILTIN_STRING_MEMORY, BuiltinConstructorFunction,
-        ECMAScriptCodeEvaluationState, Environment, ExceptionType, ExecutionContext, Function,
-        InternalMethods, InternalSlots, IteratorRecord, JsError, JsResult, KeyedGroup, Number,
-        Object, OrdinaryObject, PrivateName, PropertyDescriptor, PropertyKey, PropertyKeySet,
-        PropertyLookupCache, ProtoIntrinsics, Realm, SetResult, SmallInteger, String, TryError,
-        TryGetResult, TryHasResult, TryResult, Value, array_create,
+        ECMAScriptCodeEvaluationState, ECMAScriptFunction, Environment, ExceptionType,
+        ExecutionContext, Function, InternalMethods, InternalSlots, IteratorRecord, JsError,
+        JsResult, KeyedGroup, Number, Object, OrdinaryObject, PrivateName, PropertyDescriptor,
+        PropertyKey, PropertyKeySet, PropertyLookupCache, ProtoIntrinsics, Realm, SetResult,
+        SmallInteger, String, TryError, TryGetResult, TryHasResult, TryResult, Value, array_create,
         canonicalize_keyed_collection_key, get_iterator, if_abrupt_close_iterator, is_callable,
         is_constructor, iterator_close_with_error, iterator_step_value, js_result_into_try,
         new_class_field_initializer_environment, require_object_coercible, to_length, to_object,
@@ -2747,6 +2747,72 @@ pub(crate) fn initialize_instance_elements<'a>(
     Ok(())
 }
 
+/// Runs the deferred class field initializer bytecode associated with a
+/// user-written ECMAScript function constructor.
+///
+/// For a user-written derived class constructor that has instance fields
+/// declared on the class, the field initializers must not run before
+/// `super()` (because `this` is uninitialized at that point). The compiler
+/// stores them as the body executable's `class_initializer_bytecodes[0]`
+/// entry (a slot already present on `ExecutableHeapData`). This helper runs
+/// that executable in a new function environment where `this` is bound to
+/// the constructed instance, mirroring the behaviour of
+/// [`initialize_instance_elements`] for built-in default constructors.
+pub(crate) fn initialize_ecmascript_function_class_field_initializers<'a>(
+    agent: &mut Agent,
+    f: ECMAScriptFunction,
+    instance: Object,
+    gc: GcScope<'a, '_>,
+) -> JsResult<'a, ()> {
+    // Read everything we need before mutating the agent.
+    let f = f.bind(gc.nogc());
+    // The body executable's `class_initializer_bytecodes` always has the
+    // deferred field initializer at index 0 when one was emitted at compile
+    // time. For constructors without deferred initializers the slot is
+    // empty.
+    let body_executable = f.get(agent).compiled_bytecode;
+    let bytecode = body_executable
+        .and_then(|body_exe| {
+            body_exe
+                .get(agent)
+                .class_initializer_bytecodes
+                .first()
+                .copied()
+        })
+        .and_then(|(init, _)| init);
+    let Some(bytecode) = bytecode else {
+        return Ok(());
+    };
+    let outer_env = f.get(agent).ecmascript_function.environment;
+    let outer_priv_env = f.get(agent).ecmascript_function.private_environment;
+    let source_code = f.get(agent).ecmascript_function.source_code;
+    let realm = f.get(agent).ecmascript_function.realm;
+    let instance = instance.bind(gc.nogc());
+    let decl_env = new_class_field_initializer_environment(
+        agent,
+        Function::ECMAScriptFunction(f),
+        instance,
+        outer_env,
+        gc.nogc(),
+    );
+    agent.push_execution_context(ExecutionContext {
+        ecmascript_code: Some(ECMAScriptCodeEvaluationState {
+            lexical_environment: Environment::Function(decl_env.unbind()),
+            variable_environment: Environment::Function(decl_env.unbind()),
+            private_environment: outer_priv_env.unbind(),
+            is_strict_mode: true,
+            source_code: source_code.unbind(),
+        }),
+        function: Some(Function::ECMAScriptFunction(f.unbind())),
+        realm: realm.unbind(),
+        script_or_module: None,
+    });
+    let bytecode = bytecode.scope(agent, gc.nogc());
+    let result = Vm::execute(agent, bytecode, None, gc).into_js_result();
+    agent.pop_execution_context();
+    result.map(|_| ())
+}
+
 /// ### [7.3.34 AddValueToKeyedGroup ( groups, key, value )](https://tc39.es/ecma262/#sec-add-value-to-keyed-group)
 /// The abstract operation AddValueToKeyedGroup takes arguments groups (a List of Records with fields
 /// [[Key]] (an ECMAScript language value) and [[Elements]] (a List of ECMAScript language values)),

nova_vm/src/engine/bytecode/bytecode_compiler/class_definition_evaluation.rs

@@ -636,16 +636,51 @@ impl<'a, 's, 'gc, 'scope> CompileEvaluation<'a, 's, 'gc, 'scope> for ast::Class<
             constructor_ctx.add_instruction(Instruction::Store);
             let source_code = constructor_ctx.get_source_code();
             if let Some(constructor) = constructor {
-                let constructor_data = CompileFunctionBodyData {
-                    source_code,
-                    is_lexical: false,
-                    // Class code is always strict.
-                    is_strict: true,
-                    ast: FunctionAstRef::ClassConstructor(&constructor.value),
-                };
-                constructor_ctx.compile_function_body(constructor_data);
-                let executable = constructor_ctx.finish();
-                ctx.set_function_expression_bytecode(constructor_index, executable);
+                // For a user-written constructor on a derived class, the
+                // instance field initializers cannot run before `super()`
+                // because `this` is uninitialized at that point. Build the
+                // prelude as a separate executable and attach it to the
+                // body executable's `class_initializer_bytecodes`. It is then
+                // invoked from `EvaluateSuper` step 11 after `super()` has
+                // bound `this`. For base classes the existing
+                // prelude-inside-body approach is preserved because
+                // `OrdinaryCallBindThis` runs before the user body and so
+                // `this` is already initialized.
+                if has_constructor_parent {
+                    let initializer_executable = constructor_ctx.finish();
+                    let mut body_ctx = CompileContext::new(agent, source_code, gc);
+                    let constructor_data = CompileFunctionBodyData {
+                        source_code,
+                        is_lexical: false,
+                        // Class code is always strict.
+                        is_strict: true,
+                        ast: FunctionAstRef::ClassConstructor(&constructor.value),
+                    };
+                    // The slot must be reserved before the body is compiled:
+                    // class definitions nested inside the constructor push
+                    // their own default-constructor initializer entries into
+                    // the same `class_initializer_bytecodes` vec while the
+                    // body compiles, so index 0 is only ours if we claim it
+                    // first. `EvaluateSuper` reads the entry at index 0.
+                    body_ctx.add_class_initializer_bytecode(
+                        initializer_executable,
+                        has_constructor_parent,
+                    );
+                    body_ctx.compile_function_body(constructor_data);
+                    let body_executable = body_ctx.finish();
+                    ctx.set_function_expression_bytecode(constructor_index, body_executable);
+                } else {
+                    let constructor_data = CompileFunctionBodyData {
+                        source_code,
+                        is_lexical: false,
+                        // Class code is always strict.
+                        is_strict: true,
+                        ast: FunctionAstRef::ClassConstructor(&constructor.value),
+                    };
+                    constructor_ctx.compile_function_body(constructor_data);
+                    let executable = constructor_ctx.finish();
+                    ctx.set_function_expression_bytecode(constructor_index, executable);
+                }
             } else {
                 let executable = constructor_ctx.finish();
                 ctx.add_class_initializer_bytecode(executable, has_constructor_parent);

nova_vm/src/engine/bytecode/vm/execute_instructions.rs

@@ -17,17 +17,17 @@ use crate::{
         copy_data_properties, copy_data_properties_into_object, create_builtin_constructor,
         create_data_property_or_throw, create_unmapped_arguments_object, define_property_or_throw,
         evaluate_import_call, get_this_environment, get_this_value, get_value, has_property,
-        is_constructor, is_less_than, is_loosely_equal, is_private_reference,
-        is_property_reference, is_strictly_equal, is_super_reference, is_unresolvable_reference,
-        iterator_complete, iterator_value, make_constructor, make_method,
-        new_class_static_element_environment, new_declarative_environment, new_private_environment,
-        ordinary_function_create, ordinary_object_create_with_intrinsics, perform_eval,
-        private_element_find, put_value, resolve_binding, resolve_private_identifier,
-        resolve_this_binding, set, set_function_name, throw_no_proxy_private_names,
-        throw_read_undefined_or_null_error, to_boolean, to_number, to_number_primitive, to_numeric,
-        to_numeric_primitive, to_object, to_property_key, to_property_key_complex,
-        to_property_key_primitive, to_property_key_simple, to_string, to_string_primitive,
-        try_copy_data_properties_into_object, try_create_data_property,
+        initialize_ecmascript_function_class_field_initializers, is_constructor, is_less_than,
+        is_loosely_equal, is_private_reference, is_property_reference, is_strictly_equal,
+        is_super_reference, is_unresolvable_reference, iterator_complete, iterator_value,
+        make_constructor, make_method, new_class_static_element_environment,
+        new_declarative_environment, new_private_environment, ordinary_function_create,
+        ordinary_object_create_with_intrinsics, perform_eval, private_element_find, put_value,
+        resolve_binding, resolve_private_identifier, resolve_this_binding, set, set_function_name,
+        throw_no_proxy_private_names, throw_read_undefined_or_null_error, to_boolean, to_number,
+        to_number_primitive, to_numeric, to_numeric_primitive, to_object, to_property_key,
+        to_property_key_complex, to_property_key_primitive, to_property_key_simple, to_string,
+        to_string_primitive, try_copy_data_properties_into_object, try_create_data_property,
         try_define_property_or_throw, try_get_value, try_has_property,
         try_initialize_referenced_binding, try_put_value, try_resolve_binding, try_result_into_js,
         try_result_into_option_js, unwrap_try,
@@ -1776,12 +1776,26 @@ pub(super) fn execute_evaluate_super<'gc>(
         .bind(gc.nogc());
     // 9. Let F be thisER.[[FunctionObject]].
     // 10. Assert: F is an ECMAScript function object.
-    let Function::ECMAScriptFunction(_f) = this_er.get_function_object(agent) else {
-        unreachable!();
+    let f_unbound = match this_er.get_function_object(agent) {
+        Function::ECMAScriptFunction(f) => f.unbind(),
+        _ => unreachable!(),
     };
     // 11. Perform ? InitializeInstanceElements(result, F).
+    // For a user-written derived class constructor with instance fields
+    // declared on the class, the field initializers must run after `super()`
+    // has bound `this`. The compiler attaches the deferred initializer
+    // bytecode to the constructor body's `compiled_bytecode` Executable via
+    // its `class_initializer_bytecodes` slot (a slot already present on
+    // `ExecutableHeapData`), and it is invoked here.
+    let result_object_unbound = result.unbind();
+    initialize_ecmascript_function_class_field_initializers(
+        agent,
+        f_unbound,
+        result_object_unbound,
+        gc,
+    )?;
     // 12. Return result.
-    vm.result = Some(result.unbind().into());
+    vm.result = Some(result_object_unbound.into());
     Ok(())
 }
 

tests/class-field-init-in-derived.js

@@ -0,0 +1,108 @@
+// Regression test for https://github.com/trynova/nova/issues/948
+// "class field initializers are broken in subclasses"
+//
+// A user-written derived class constructor used to throw
+// `ReferenceError: Uninitialized this binding` because the instance field
+// initializer prelude was emitted at the start of the constructor body,
+// before `super()` had bound `this`. The fix defers the field initializer
+// to a separate executable that runs after `super()` returns.
+
+class A {}
+class B extends A {
+  b = 2
+  constructor() { super() }
+}
+
+const b = new B()
+if (b.b !== 2) {
+  throw new Error('expected b.b === 2, got ' + b.b)
+}
+
+// Field visible to the constructor body after super() returns.
+class C extends A {
+  c = 3
+  constructor() {
+    super()
+    if (this.c !== 3) {
+      throw new Error('expected this.c === 3 inside constructor')
+    }
+  }
+}
+new C()
+
+// Multiple instance fields.
+class E extends A {
+  e1 = 1
+  e2 = 2
+  constructor() { super() }
+}
+const e = new E()
+if (e.e1 !== 1 || e.e2 !== 2) {
+  throw new Error('expected e.e1 === 1 and e.e2 === 2')
+}
+
+// Grand-child still inherits fields from both levels.
+class I extends B {
+  i = 'i-field'
+  constructor() { super() }
+}
+const i = new I()
+if (i.b !== 2 || i.i !== 'i-field') {
+  throw new Error('expected i.b === 2 and i.i === "i-field"')
+}
+
+// Base class with fields must remain unchanged (no regression).
+class G {
+  g = 42
+  constructor() {}
+}
+if (new G().g !== 42) {
+  throw new Error('expected new G().g === 42')
+}
+// Nested classes inside a derived constructor must not displace the
+// deferred field-initializer entry: the compiler reserves slot 0 of the
+// body executable's class_initializer_bytecodes for it before compiling
+// the body, and EvaluateSuper reads slot 0.
+
+// Variant 1: nested base class with fields and an implicit constructor
+// (its default-constructor initializer entry is appended after ours).
+class P extends A {
+  p = 'p-field'
+  constructor() {
+    class InnerWithDefault {
+      inner = 7
+    }
+    if (new InnerWithDefault().inner !== 7) {
+      throw new Error('expected inner === 7')
+    }
+    super()
+  }
+}
+if (new P().p !== 'p-field') {
+  throw new Error('expected p === "p-field"')
+}
+
+// Variant 2: nested derived class with its own user constructor and
+// fields (its deferred initializer lives at slot 0 of its own body
+// executable, one level down).
+class Q extends A {
+  q = 'q-field'
+  constructor() {
+    const Nested = class extends A {
+      nested = 'nested-field'
+      constructor() { super() }
+    }
+    const inner = new Nested()
+    if (inner.nested !== 'nested-field') {
+      throw new Error('expected nested === "nested-field"')
+    }
+    super()
+    if (this.q !== 'q-field') {
+      throw new Error('expected this.q === "q-field" inside constructor')
+    }
+  }
+}
+const q = new Q()
+if (q.q !== 'q-field') {
+  throw new Error('expected q.q === "q-field"')
+}