From 3449b7a1f301483747566989893ba26f745a6792 Mon Sep 17 00:00:00 2001 From: simonyang08 Date: Mon, 19 Oct 2026 09:11:29 +0800 Subject: [PATCH] Fix derived class field initializer evaluation order A user-written derived constructor's instance field initializers used to run before super() and hit the uninitialized this binding. They are now deferred into the body executable's class_initializer_bytecodes[0] slot and run from EvaluateSuper after super() binds this. Slot 0 is reserved before the body compiles so nested class definitions don't displace it. Fixes #948 Signed-off-by: simonyang08 --- .../operations_on_objects.rs | 64 ++++++++++++- .../class_definition_evaluation.rs | 43 +++++++-- .../bytecode/vm/execute_instructions.rs | 41 ++++---- tests/class-field-init-in-derived.js | 95 +++++++++++++++++++ 4 files changed, 212 insertions(+), 31 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..ec20bf210 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,60 @@ pub(crate) fn initialize_instance_elements<'a>( Ok(()) } +/// Runs a user-written derived constructor's deferred class field +/// initializer, mirroring [`initialize_instance_elements`]. +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()); + // Slot 0 holds the deferred field initializer, if any. + 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)), 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..22e7ea3eb 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,39 @@ 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); + // A derived constructor's field initializers can't run + // before `super()`; defer them to EvaluateSuper step 11. + 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), + }; + // Slot 0 is ours; nested class definitions compiled in + // the body push their entries after it. + 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); diff --git a/nova_vm/src/engine/bytecode/vm/execute_instructions.rs b/nova_vm/src/engine/bytecode/vm/execute_instructions.rs index df9b663c5..8bd3d8e3b 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, @@ -1776,12 +1776,21 @@ 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!(); - }; - // 11. Perform ? InitializeInstanceElements(result, F). + let f_unbound = match this_er.get_function_object(agent) { + Function::ECMAScriptFunction(f) => f.unbind(), + _ => unreachable!(), + }; + // 11. Perform ? InitializeInstanceElements(result, F): run the deferred + // class field initializer from slot 0 of the body executable. + 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..a564f3161 --- /dev/null +++ b/tests/class-field-init-in-derived.js @@ -0,0 +1,95 @@ +// Regression test for https://github.com/trynova/nova/issues/948 + +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 must not displace the deferred field-initializer slot. +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"') +}