From 4b8ca7652cf6ce6581ff30e18a1e1d44f46ef160 Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Mon, 30 Oct 2023 18:59:47 +0100 Subject: [PATCH] Move ordinary function [[ConstructorKind]] to CodeBlock (#3439) --- boa_engine/src/builtins/function/mod.rs | 20 +++-------- boa_engine/src/bytecompiler/class.rs | 6 ++++ .../src/object/internal_methods/function.rs | 35 +++++++++---------- boa_engine/src/vm/code_block.rs | 14 ++++++-- boa_engine/src/vm/opcode/push/class/mod.rs | 16 --------- 5 files changed, 39 insertions(+), 52 deletions(-) diff --git a/boa_engine/src/builtins/function/mod.rs b/boa_engine/src/builtins/function/mod.rs index 74e8ce61c2d..0d9575c6773 100644 --- a/boa_engine/src/builtins/function/mod.rs +++ b/boa_engine/src/builtins/function/mod.rs @@ -147,9 +147,6 @@ unsafe impl Trace for ClassFieldDefinition { pub(crate) enum FunctionKind { /// A bytecode function. Ordinary { - /// The `[[ConstructorKind]]` internal slot. - constructor_kind: ConstructorKind, - /// The `[[Fields]]` internal slot. fields: ThinVec, @@ -248,15 +245,8 @@ impl OrdinaryFunction { } /// Returns true if the function object is a derived constructor. - pub(crate) const fn is_derived_constructor(&self) -> bool { - if let FunctionKind::Ordinary { - constructor_kind, .. - } = self.kind - { - constructor_kind.is_derived() - } else { - false - } + pub(crate) fn is_derived_constructor(&self) -> bool { + self.code.is_derived_constructor() } /// Does this function have the `[[ClassFieldInitializerName]]` internal slot set to non-empty value. @@ -330,9 +320,9 @@ impl OrdinaryFunction { &self.kind } - /// Gets a mutable reference to the [`FunctionKind`] of the `Function`. - pub(crate) fn kind_mut(&mut self) -> &mut FunctionKind { - &mut self.kind + /// Check if function is [`FunctionKind::Ordinary`]. + pub(crate) const fn is_ordinary(&self) -> bool { + matches!(self.kind(), FunctionKind::Ordinary { .. }) } } diff --git a/boa_engine/src/bytecompiler/class.rs b/boa_engine/src/bytecompiler/class.rs index bae59003bbf..e95707e1599 100644 --- a/boa_engine/src/bytecompiler/class.rs +++ b/boa_engine/src/bytecompiler/class.rs @@ -80,6 +80,12 @@ impl ByteCompiler<'_, '_> { } compiler.emit_opcode(Opcode::SetReturnValue); + // 17. If ClassHeritageopt is present, set F.[[ConstructorKind]] to derived. + compiler.code_block_flags.set( + CodeBlockFlags::IS_DERIVED_CONSTRUCTOR, + class.super_ref().is_some(), + ); + let code = Gc::new(compiler.finish()); let index = self.push_function_to_constants(code); self.emit( diff --git a/boa_engine/src/object/internal_methods/function.rs b/boa_engine/src/object/internal_methods/function.rs index fcc21fcb85f..d5061b4365b 100644 --- a/boa_engine/src/object/internal_methods/function.rs +++ b/boa_engine/src/object/internal_methods/function.rs @@ -1,5 +1,5 @@ use crate::{ - builtins::function::{arguments::Arguments, FunctionKind, ThisMode}, + builtins::function::{arguments::Arguments, ThisMode}, context::intrinsics::StandardConstructors, environments::{FunctionSlots, ThisBindingStatus}, native_function::NativeFunctionObject, @@ -48,13 +48,15 @@ pub(crate) fn function_call( let function = object.as_function().expect("not a function"); let realm = function.realm().clone(); - if let FunctionKind::Ordinary { .. } = function.kind() { - if function.code.is_class_constructor() { - return Err(JsNativeError::typ() - .with_message("class constructor cannot be invoked without 'new'") - .with_realm(realm) - .into()); - } + if function.code.is_class_constructor() { + debug_assert!( + function.is_ordinary(), + "only ordinary functions can be classes" + ); + return Err(JsNativeError::typ() + .with_message("class constructor cannot be invoked without 'new'") + .with_realm(realm) + .into()); } let code = function.code.clone(); @@ -172,17 +174,14 @@ fn function_construct( let function = object.as_function().expect("not a function"); let realm = function.realm().clone(); - let FunctionKind::Ordinary { - constructor_kind, .. - } = function.kind() - else { - unreachable!("not a constructor") - }; + debug_assert!( + function.is_ordinary(), + "only ordinary functions can be constructed" + ); let code = function.code.clone(); let environments = function.environments.clone(); let script_or_module = function.script_or_module.clone(); - let constructor_kind = *constructor_kind; drop(object); let env_fp = environments.len() as u32; @@ -191,7 +190,9 @@ fn function_construct( let at = context.vm.stack.len() - argument_count; - let this = if constructor_kind.is_base() { + let this = if code.is_derived_constructor() { + None + } else { // If the prototype of the constructor is not an object, then use the default object // prototype as prototype for the new object // see @@ -207,8 +208,6 @@ fn function_construct( this.initialize_instance_elements(this_function_object, context)?; Some(this) - } else { - None }; let frame = CallFrame::new(code.clone(), script_or_module, environments, realm) diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs index 3ea3cbc0979..0b760747144 100644 --- a/boa_engine/src/vm/code_block.rs +++ b/boa_engine/src/vm/code_block.rs @@ -3,7 +3,7 @@ //! This module is for the `CodeBlock` which implements a function representation in the VM use crate::{ - builtins::function::{ConstructorKind, FunctionKind, OrdinaryFunction, ThisMode}, + builtins::function::{FunctionKind, OrdinaryFunction, ThisMode}, environments::{BindingLocator, CompileTimeEnvironment}, object::{JsObject, ObjectData, PROTOTYPE}, property::PropertyDescriptor, @@ -69,6 +69,9 @@ bitflags! { /// The `[[ClassFieldInitializerName]]` internal slot. const IN_CLASS_FIELD_INITIALIZER = 0b0010_0000; + /// `[[ConstructorKind]]` + const IS_DERIVED_CONSTRUCTOR = 0b0100_0000; + /// Trace instruction execution to `stdout`. #[cfg(feature = "trace")] const TRACEABLE = 0b1000_0000; @@ -241,6 +244,13 @@ impl CodeBlock { .contains(CodeBlockFlags::IN_CLASS_FIELD_INITIALIZER) } + /// Returns true if this function is a derived constructor. + pub(crate) fn is_derived_constructor(&self) -> bool { + self.flags + .get() + .contains(CodeBlockFlags::IS_DERIVED_CONSTRUCTOR) + } + /// Find exception [`Handler`] in the code block given the current program counter (`pc`). #[inline] pub(crate) fn find_handler(&self, pc: u32) -> Option<(usize, &Handler)> { @@ -838,7 +848,6 @@ pub(crate) fn create_function_object( home_object: None, script_or_module, kind: FunctionKind::Ordinary { - constructor_kind: ConstructorKind::Base, fields: ThinVec::new(), private_methods: ThinVec::new(), }, @@ -903,7 +912,6 @@ pub(crate) fn create_function_object_fast( FunctionKind::Async } else { FunctionKind::Ordinary { - constructor_kind: ConstructorKind::Base, fields: ThinVec::new(), private_methods: ThinVec::new(), } diff --git a/boa_engine/src/vm/opcode/push/class/mod.rs b/boa_engine/src/vm/opcode/push/class/mod.rs index a7c59e8b486..4a6d1d77a42 100644 --- a/boa_engine/src/vm/opcode/push/class/mod.rs +++ b/boa_engine/src/vm/opcode/push/class/mod.rs @@ -1,5 +1,4 @@ use crate::{ - builtins::function::{ConstructorKind, FunctionKind}, error::JsNativeError, object::PROTOTYPE, vm::{opcode::Operation, CompletionType}, @@ -69,21 +68,6 @@ impl Operation for PushClassPrototype { class_object.set_prototype(Some(constructor_parent)); } - let mut class_object_mut = class_object.borrow_mut(); - let class_function = class_object_mut - .as_function_mut() - .expect("class must be function object"); - - // 17. If ClassHeritageopt is present, set F.[[ConstructorKind]] to derived. - if let FunctionKind::Ordinary { - constructor_kind, .. - } = class_function.kind_mut() - { - *constructor_kind = ConstructorKind::Derived; - } - - drop(class_object_mut); - context.vm.push(class); context.vm.push(proto_parent);