From f867078bd4a1544a3de29e4b43bed4aab65bbdab Mon Sep 17 00:00:00 2001 From: simonyang08 Date: Sun, 6 Sep 2026 21:23:59 +0800 Subject: [PATCH] fix(engine): run derived class field initializers after super() (#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 --- .../operations_on_objects.rs | 63 +++++++++++++++++-- .../builtins/ecmascript_function.rs | 5 ++ .../types/language/function/data.rs | 4 ++ .../src/engine/bytecode/bytecode_compiler.rs | 3 + .../class_definition_evaluation.rs | 51 ++++++++++++--- .../bytecode_compiler/compile_context.rs | 9 +++ .../bytecode_compiler/executable_context.rs | 9 +++ nova_vm/src/engine/bytecode/executable.rs | 9 +++ .../bytecode/vm/execute_instructions.rs | 49 ++++++++++----- tests/class-field-init-in-derived.js | 61 ++++++++++++++++++ 10 files changed, 233 insertions(+), 30 deletions(-) create mode 100644 tests/class-field-init-in-derived.js diff --git a/nova_vm/src/ecmascript/abstract_operations/operations_on_objects.rs b/nova_vm/src/ecmascript/abstract_operations/operations_on_objects.rs index 69158b974..a30148ebf 100644 --- a/nova_vm/src/ecmascript/abstract_operations/operations_on_objects.rs +++ b/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,59 @@ 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 a separate executable on the function. 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 bytecode = f.get(agent).class_field_initializer_bytecode; + let bytecode = match bytecode { + Some(b) => b.unbind(), + None => return Ok(()), + }; + let f = f.bind(gc.nogc()); + 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)), diff --git a/nova_vm/src/ecmascript/builtins/ecmascript_function.rs b/nova_vm/src/ecmascript/builtins/ecmascript_function.rs index d2ebc374b..cfbf412c5 100644 --- a/nova_vm/src/ecmascript/builtins/ecmascript_function.rs +++ b/nova_vm/src/ecmascript/builtins/ecmascript_function.rs @@ -890,6 +890,7 @@ pub(crate) fn ordinary_function_create<'gc>( ecmascript_function, compiled_bytecode: None, name: None, + class_field_initializer_bytecode: None, }; if let Some(function_prototype) = params.function_prototype && function_prototype @@ -1226,6 +1227,7 @@ impl HeapMarkAndSweep for ECMAScriptFunctionHeapData<'static> { ecmascript_function, compiled_bytecode, name, + class_field_initializer_bytecode, } = self; let ECMAScriptFunctionObjectHeapData { environment, @@ -1243,6 +1245,7 @@ impl HeapMarkAndSweep for ECMAScriptFunctionHeapData<'static> { object_index.mark_values(queues); compiled_bytecode.mark_values(queues); name.mark_values(queues); + class_field_initializer_bytecode.mark_values(queues); environment.mark_values(queues); private_environment.mark_values(queues); realm.mark_values(queues); @@ -1258,6 +1261,7 @@ impl HeapMarkAndSweep for ECMAScriptFunctionHeapData<'static> { ecmascript_function, compiled_bytecode, name, + class_field_initializer_bytecode, } = self; let ECMAScriptFunctionObjectHeapData { environment, @@ -1275,6 +1279,7 @@ impl HeapMarkAndSweep for ECMAScriptFunctionHeapData<'static> { object_index.sweep_values(compactions); compiled_bytecode.sweep_values(compactions); name.sweep_values(compactions); + class_field_initializer_bytecode.sweep_values(compactions); environment.sweep_values(compactions); private_environment.sweep_values(compactions); realm.sweep_values(compactions); diff --git a/nova_vm/src/ecmascript/types/language/function/data.rs b/nova_vm/src/ecmascript/types/language/function/data.rs index 5eea65b86..b7b1d8f37 100644 --- a/nova_vm/src/ecmascript/types/language/function/data.rs +++ b/nova_vm/src/ecmascript/types/language/function/data.rs @@ -105,6 +105,10 @@ pub(crate) struct ECMAScriptFunctionHeapData<'a> { /// Stores the compiled bytecode of an ECMAScript function. pub(crate) compiled_bytecode: Option>, pub(crate) name: Option>, + /// For a user-written derived class constructor with instance fields, + /// holds the compiled bytecode that initializes those fields. It is run + /// after `super()` has bound `this` (from `EvaluateSuper` step 11). + pub(crate) class_field_initializer_bytecode: Option>, } unsafe impl Send for ECMAScriptFunctionHeapData<'_> {} diff --git a/nova_vm/src/engine/bytecode/bytecode_compiler.rs b/nova_vm/src/engine/bytecode/bytecode_compiler.rs index a3929e690..a5f7ee921 100644 --- a/nova_vm/src/engine/bytecode/bytecode_compiler.rs +++ b/nova_vm/src/engine/bytecode/bytecode_compiler.rs @@ -1237,6 +1237,7 @@ impl<'a, 's, 'gc, 'scope> CompileEvaluation<'a, 's, 'gc, 'scope> for ast::Functi }), identifier, compiled_bytecode: None, + class_field_initializer_bytecode: None, }, ); } @@ -1436,6 +1437,7 @@ impl<'a, 's, 'gc, 'scope> CompileEvaluation<'a, 's, 'gc, 'scope> for ast::Object }), identifier, compiled_bytecode: None, + class_field_initializer_bytecode: None, }, // enumerable: true, true.into(), @@ -1484,6 +1486,7 @@ impl<'a, 's, 'gc, 'scope> CompileEvaluation<'a, 's, 'gc, 'scope> for ast::Object }), identifier: None, compiled_bytecode: None, + class_field_initializer_bytecode: None, }, // enumerable: true, true.into(), diff --git a/nova_vm/src/engine/bytecode/bytecode_compiler/class_definition_evaluation.rs b/nova_vm/src/engine/bytecode/bytecode_compiler/class_definition_evaluation.rs index ed9c4531b..e8d35089c 100644 --- a/nova_vm/src/engine/bytecode/bytecode_compiler/class_definition_evaluation.rs +++ b/nova_vm/src/engine/bytecode/bytecode_compiler/class_definition_evaluation.rs @@ -636,16 +636,44 @@ 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 register it so it + // runs 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), + }; + body_ctx.compile_function_body(constructor_data); + let body_executable = body_ctx.finish(); + ctx.set_function_expression_class_field_initializer_bytecode( + constructor_index, + initializer_executable, + ); + 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); @@ -854,6 +882,7 @@ fn define_constructor_method( // CompileContext holds a name identifier for us if this is NamedEvaluation. identifier: None, compiled_bytecode: None, + class_field_initializer_bytecode: None, }, has_constructor_parent.into(), ) @@ -915,6 +944,7 @@ fn define_method<'s>( // Note: method name is always found in the result register. identifier: Some(NamedEvaluationParameter::Result), compiled_bytecode: None, + class_field_initializer_bytecode: None, }, // enumerable: false, false.into(), @@ -998,6 +1028,7 @@ fn define_private_method<'s>( }), identifier: Some(NamedEvaluationParameter::Result), compiled_bytecode: None, + class_field_initializer_bytecode: None, }, immediate.into(), ); diff --git a/nova_vm/src/engine/bytecode/bytecode_compiler/compile_context.rs b/nova_vm/src/engine/bytecode/bytecode_compiler/compile_context.rs index 3878367c1..242a36fd5 100644 --- a/nova_vm/src/engine/bytecode/bytecode_compiler/compile_context.rs +++ b/nova_vm/src/engine/bytecode/bytecode_compiler/compile_context.rs @@ -1258,6 +1258,15 @@ impl<'agent, 'script, 'gc, 'scope> CompileContext<'agent, 'script, 'gc, 'scope> .set_function_expression_bytecode(index, executable); } + pub(super) fn set_function_expression_class_field_initializer_bytecode( + &mut self, + index: IndexType, + executable: Executable<'gc>, + ) { + self.executable + .set_function_expression_class_field_initializer_bytecode(index, executable); + } + pub(super) fn add_class_initializer_bytecode( &mut self, executable: Executable<'gc>, diff --git a/nova_vm/src/engine/bytecode/bytecode_compiler/executable_context.rs b/nova_vm/src/engine/bytecode/bytecode_compiler/executable_context.rs index 8b7a6f3d2..b92c649cc 100644 --- a/nova_vm/src/engine/bytecode/bytecode_compiler/executable_context.rs +++ b/nova_vm/src/engine/bytecode/bytecode_compiler/executable_context.rs @@ -474,6 +474,15 @@ impl<'agent, 'gc, 'scope> ExecutableContext<'agent, 'gc, 'scope> { self.function_expressions[index as usize].compiled_bytecode = Some(executable); } + pub(super) fn set_function_expression_class_field_initializer_bytecode( + &mut self, + index: IndexType, + executable: Executable<'gc>, + ) { + self.function_expressions[index as usize].class_field_initializer_bytecode = + Some(executable); + } + pub(super) fn add_class_initializer_bytecode( &mut self, executable: Executable<'gc>, diff --git a/nova_vm/src/engine/bytecode/executable.rs b/nova_vm/src/engine/bytecode/executable.rs index c9708d172..6b5c0a4f8 100644 --- a/nova_vm/src/engine/bytecode/executable.rs +++ b/nova_vm/src/engine/bytecode/executable.rs @@ -64,6 +64,11 @@ pub(crate) struct FunctionExpression<'a> { pub(crate) identifier: Option, /// Optionally eagerly compile the FunctionExpression into bytecode. pub(crate) compiled_bytecode: Option>, + /// For a class constructor with instance fields defined on a derived + /// class, holds a separate executable that runs the field initializers + /// after `super()` has bound `this`. The executable is invoked from + /// `EvaluateSuper` step 11 (InitializeInstanceElements). + pub(crate) class_field_initializer_bytecode: Option>, } bindable_handle!(FunctionExpression); @@ -74,8 +79,10 @@ impl HeapMarkAndSweep for FunctionExpression<'static> { expression: _, identifier: _, compiled_bytecode, + class_field_initializer_bytecode, } = self; compiled_bytecode.mark_values(queues); + class_field_initializer_bytecode.mark_values(queues); } fn sweep_values(&mut self, compactions: &CompactionLists) { @@ -83,8 +90,10 @@ impl HeapMarkAndSweep for FunctionExpression<'static> { expression: _, identifier: _, compiled_bytecode, + class_field_initializer_bytecode, } = self; compiled_bytecode.sweep_values(compactions); + class_field_initializer_bytecode.sweep_values(compactions); } } diff --git a/nova_vm/src/engine/bytecode/vm/execute_instructions.rs b/nova_vm/src/engine/bytecode/vm/execute_instructions.rs index df9b663c5..7c6def3e2 100644 --- a/nova_vm/src/engine/bytecode/vm/execute_instructions.rs +++ b/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, @@ -1220,10 +1220,12 @@ pub(super) fn execute_class_define_constructor<'gc>( let FunctionExpression { expression, compiled_bytecode, + class_field_initializer_bytecode, .. } = executable.fetch_function_expression(agent, instr.get_first_index(), gc.nogc()); let function_expression = expression.get(); let compiled_bytecode = *compiled_bytecode; + let class_field_initializer_bytecode = *class_field_initializer_bytecode; let has_constructor_parent = instr.get_second_bool(); let function_prototype = if has_constructor_parent { @@ -1252,6 +1254,10 @@ pub(super) fn execute_class_define_constructor<'gc>( if let Some(compiled_bytecode) = compiled_bytecode { function.get_mut(agent).compiled_bytecode = Some(compiled_bytecode.unbind()); } + if let Some(class_field_initializer_bytecode) = class_field_initializer_bytecode { + function.get_mut(agent).class_field_initializer_bytecode = + Some(class_field_initializer_bytecode.unbind()); + } set_function_name(agent, function, class_name.into(), None, gc.nogc()); make_constructor(agent, function, Some(false), Some(proto), gc.nogc()); function.get_mut(agent).ecmascript_function.home_object = Some(proto.into()); @@ -1766,7 +1772,8 @@ pub(super) fn execute_evaluate_super<'gc>( result.unbind().bind(gc.nogc()) }; // 7. Let thisER be GetThisEnvironment(). - let Environment::Function(this_er) = get_this_environment(agent, gc.nogc()) else { + let this_er = get_this_environment(agent, gc.nogc()); + let Environment::Function(this_er) = this_er else { unreachable!(); }; // 8. Perform ? thisER.BindThisValue(result). @@ -1776,12 +1783,24 @@ 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`. They are stored on the function as a separate + // executable and 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(()) } diff --git a/tests/class-field-init-in-derived.js b/tests/class-field-init-in-derived.js new file mode 100644 index 000000000..2f716b7c0 --- /dev/null +++ b/tests/class-field-init-in-derived.js @@ -0,0 +1,61 @@ +// 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') +} \ No newline at end of file