diff --git a/boa_engine/src/builtins/error/type.rs b/boa_engine/src/builtins/error/type.rs index 4c82327955e..1f39077784e 100644 --- a/boa_engine/src/builtins/error/type.rs +++ b/boa_engine/src/builtins/error/type.rs @@ -103,7 +103,7 @@ pub(crate) fn create_throw_type_error(context: &mut Context) -> JsObject { context.intrinsics().constructors().function().prototype(), ObjectData::function(Function::Native { function: throw_type_error, - constructor: false, + constructor: None, }), ); diff --git a/boa_engine/src/builtins/function/mod.rs b/boa_engine/src/builtins/function/mod.rs index 19bd4a8ceac..39866af9199 100644 --- a/boa_engine/src/builtins/function/mod.rs +++ b/boa_engine/src/builtins/function/mod.rs @@ -26,7 +26,7 @@ use crate::{ value::IntegerOrInfinity, Context, JsResult, JsString, JsValue, }; -use boa_gc::{self, Finalize, Gc, Trace}; +use boa_gc::{self, unsafe_empty_trace, Finalize, Gc, Trace}; use boa_interner::Sym; use boa_profiler::Profiler; use dyn_clone::DynClone; @@ -108,12 +108,17 @@ impl ThisMode { } } -#[derive(Debug, Trace, Finalize, PartialEq, Clone)] +#[derive(Debug, Finalize, PartialEq, Clone, Copy)] pub enum ConstructorKind { Base, Derived, } +// SAFETY: `Copy` types are trivially not `Trace`. +unsafe impl Trace for ConstructorKind { + unsafe_empty_trace!(); +} + impl ConstructorKind { /// Returns `true` if the constructor kind is `Base`. pub fn is_base(&self) -> bool { @@ -178,12 +183,12 @@ pub enum Function { Native { #[unsafe_ignore_trace] function: NativeFunctionSignature, - constructor: bool, + constructor: Option, }, Closure { #[unsafe_ignore_trace] function: Box, - constructor: bool, + constructor: Option, captures: Captures, }, Ordinary { @@ -205,6 +210,11 @@ impl fmt::Debug for Function { impl Function { /// Returns true if the function object is a constructor. pub fn is_constructor(&self) -> bool { + self.constructor().is_some() + } + + /// Returns the constructor kind if the function is constructable, or `None` otherwise. + pub fn constructor(&self) -> Option { match self { Self::Native { constructor, .. } | Self::Closure { constructor, .. } => *constructor, Self::Ordinary { code, .. } | Self::Generator { code, .. } => code.constructor, @@ -251,7 +261,7 @@ pub(crate) fn make_builtin_fn( .prototype(), ObjectData::function(Function::Native { function, - constructor: false, + constructor: None, }), ); let attribute = PropertyDescriptor::builder() @@ -417,7 +427,7 @@ impl BuiltInFunctionObject { prototype, ObjectData::function(Function::Native { function: |_, _, _| Ok(JsValue::undefined()), - constructor: true, + constructor: Some(ConstructorKind::Base), }), ); diff --git a/boa_engine/src/builtins/generator_function/mod.rs b/boa_engine/src/builtins/generator_function/mod.rs index 19d761c5f8d..70161a144a3 100644 --- a/boa_engine/src/builtins/generator_function/mod.rs +++ b/boa_engine/src/builtins/generator_function/mod.rs @@ -21,6 +21,8 @@ use crate::{ }; use boa_profiler::Profiler; +use super::function::ConstructorKind; + /// The internal representation on a `Generator` object. #[derive(Debug, Clone, Copy)] pub struct GeneratorFunction; @@ -71,7 +73,7 @@ impl BuiltIn for GeneratorFunction { constructor.borrow_mut().insert("prototype", property); constructor.borrow_mut().data = ObjectData::function(Function::Native { function: Self::constructor, - constructor: true, + constructor: Some(ConstructorKind::Base), }); prototype.set_prototype(Some( @@ -124,7 +126,7 @@ impl GeneratorFunction { prototype, ObjectData::function(Function::Native { function: |_, _, _| Ok(JsValue::undefined()), - constructor: true, + constructor: Some(ConstructorKind::Base), }), ); diff --git a/boa_engine/src/bytecompiler.rs b/boa_engine/src/bytecompiler.rs index b43e888c0c5..da44bf011f5 100644 --- a/boa_engine/src/bytecompiler.rs +++ b/boa_engine/src/bytecompiler.rs @@ -1,5 +1,5 @@ use crate::{ - builtins::function::ThisMode, + builtins::function::{ConstructorKind, ThisMode}, environments::{BindingLocator, CompileTimeEnvironment}, syntax::ast::{ node::{ @@ -81,7 +81,7 @@ impl<'b> ByteCompiler<'b> { #[inline] pub fn new(name: Sym, strict: bool, context: &'b mut Context) -> Self { Self { - code_block: CodeBlock::new(name, 0, strict, false), + code_block: CodeBlock::new(name, 0, strict, None), literals_map: FxHashMap::default(), names_map: FxHashMap::default(), bindings_map: FxHashMap::default(), @@ -1885,15 +1885,20 @@ impl<'b> ByteCompiler<'b> { ) -> JsResult> { let strict = strict || body.strict(); let length = parameters.length(); - let mut code = CodeBlock::new(name.unwrap_or(Sym::EMPTY_STRING), length, strict, true); + let mut code = CodeBlock::new( + name.unwrap_or(Sym::EMPTY_STRING), + length, + strict, + Some(ConstructorKind::Base), + ); if let FunctionKind::Arrow = kind { - code.constructor = false; + code.constructor = None; code.this_mode = ThisMode::Lexical; } if generator { - code.constructor = false; + code.constructor = None; } let mut compiler = ByteCompiler { @@ -2494,7 +2499,16 @@ impl<'b> ByteCompiler<'b> { /// A class declaration binds the resulting class object to it's identifier. /// A class expression leaves the resulting class object on the stack for following operations. fn class(&mut self, class: &Class, expression: bool) -> JsResult<()> { - let mut code = CodeBlock::new(class.name(), 0, true, true); + let mut code = CodeBlock::new( + class.name(), + 0, + true, + Some(if class.super_ref().is_some() { + ConstructorKind::Derived + } else { + ConstructorKind::Base + }), + ); code.computed_field_names = Some(gc::GcCell::new(vec![])); let mut compiler = ByteCompiler { code_block: code, diff --git a/boa_engine/src/object/mod.rs b/boa_engine/src/object/mod.rs index 2bd8d95c37d..31e10166a10 100644 --- a/boa_engine/src/object/mod.rs +++ b/boa_engine/src/object/mod.rs @@ -28,7 +28,8 @@ use crate::{ array_buffer::ArrayBuffer, function::arguments::Arguments, function::{ - arguments::ParameterMap, BoundFunction, Captures, Function, NativeFunctionSignature, + arguments::ParameterMap, BoundFunction, Captures, ConstructorKind, Function, + NativeFunctionSignature, }, generator::Generator, map::map_iterator::MapIterator, @@ -1460,7 +1461,7 @@ impl<'context> FunctionBuilder<'context> { context, function: Function::Native { function, - constructor: false, + constructor: None, }, name: JsString::default(), length: 0, @@ -1477,7 +1478,7 @@ impl<'context> FunctionBuilder<'context> { context, function: Function::Closure { function: Box::new(move |this, args, _, context| function(this, args, context)), - constructor: false, + constructor: None, captures: Captures::new(()), }, name: JsString::default(), @@ -1511,7 +1512,7 @@ impl<'context> FunctionBuilder<'context> { })?; function(this, args, captures, context) }), - constructor: false, + constructor: None, captures: Captures::new(captures), }, name: JsString::default(), @@ -1559,7 +1560,7 @@ impl<'context> FunctionBuilder<'context> { ref mut constructor, .. } => { - *constructor = yes; + *constructor = yes.then(|| ConstructorKind::Base); } Function::Ordinary { .. } | Function::Generator { .. } => { unreachable!("function must be native or closure"); @@ -1716,7 +1717,7 @@ pub struct ConstructorBuilder<'context> { name: JsString, length: usize, callable: bool, - constructor: bool, + constructor: Option, inherit: Option, custom_prototype: Option, } @@ -1748,7 +1749,7 @@ impl<'context> ConstructorBuilder<'context> { length: 0, name: JsString::default(), callable: true, - constructor: true, + constructor: Some(ConstructorKind::Base), inherit: None, custom_prototype: None, has_prototype_property: true, @@ -1770,7 +1771,7 @@ impl<'context> ConstructorBuilder<'context> { length: 0, name: JsString::default(), callable: true, - constructor: true, + constructor: Some(ConstructorKind::Base), inherit: None, custom_prototype: None, } @@ -1967,7 +1968,7 @@ impl<'context> ConstructorBuilder<'context> { /// Default is `true` #[inline] pub fn constructor(&mut self, constructor: bool) -> &mut Self { - self.constructor = constructor; + self.constructor = constructor.then(|| ConstructorKind::Base); self } diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs index 47281a46cc3..7b6cc569920 100644 --- a/boa_engine/src/vm/code_block.rs +++ b/boa_engine/src/vm/code_block.rs @@ -5,7 +5,7 @@ use crate::{ builtins::{ function::{ - arguments::Arguments, Captures, ClosureFunctionSignature, Function, + arguments::Arguments, Captures, ClosureFunctionSignature, ConstructorKind, Function, NativeFunctionSignature, ThisMode, }, generator::{Generator, GeneratorContext, GeneratorState}, @@ -23,7 +23,6 @@ use boa_gc::{Cell, Finalize, Gc, Trace}; use boa_interner::{Interner, Sym, ToInternedString}; use boa_profiler::Profiler; use std::{convert::TryInto, mem::size_of}; -use tap::Pipe; /// This represents whether a value can be read from [`CodeBlock`] code. /// @@ -63,10 +62,10 @@ pub struct CodeBlock { /// Is this function in strict mode. pub(crate) strict: bool, - /// Is this function a constructor. - pub(crate) constructor: bool, + /// Constructor type of this function, or `None` if the function is not constructable. + pub(crate) constructor: Option, - /// [[ThisMode]] + /// \[\[ThisMode\]\] pub(crate) this_mode: ThisMode, /// Parameters passed to this function. @@ -109,7 +108,7 @@ pub struct CodeBlock { impl CodeBlock { /// Constructs a new `CodeBlock`. - pub fn new(name: Sym, length: u32, strict: bool, constructor: bool) -> Self { + pub fn new(name: Sym, length: u32, strict: bool, constructor: Option) -> Self { Self { code: Vec::new(), literals: Vec::new(), @@ -589,7 +588,7 @@ impl JsObject { function, constructor, } => { - if *constructor { + if constructor.is_some() { construct = true; } @@ -873,13 +872,24 @@ impl JsObject { let this_function_object = self.clone(); // let mut has_parameter_expressions = false; + let create_this = |context| { + let prototype = + get_prototype_from_constructor(this_target, StandardConstructors::object, context)?; + Ok(Self::from_proto_and_data(prototype, ObjectData::ordinary())) + }; + if !self.is_constructor() { return context.throw_type_error("not a constructor function"); } + let constructor; + let body = { let object = self.borrow(); let function = object.as_function().expect("not a function"); + constructor = function + .constructor() + .expect("Already checked that the function was a constructor"); match function { Function::Native { function, .. } => FunctionBody::Native { @@ -902,17 +912,31 @@ impl JsObject { }; match body { - FunctionBody::Native { function, .. } => function(this_target, args, context)? - .as_object() - .cloned() - .expect("Constructor must always return a `JsObject` value") - .pipe(Ok), + FunctionBody::Native { function, .. } => match function(this_target, args, context)? { + JsValue::Object(ref o) => Ok(o.clone()), + val => { + if constructor.is_base() || val.is_undefined() { + create_this(context) + } else { + context.throw_type_error( + "Derived constructor can only return an Object or undefined", + ) + } + } + }, FunctionBody::Closure { function, captures } => { - (function)(this_target, args, captures, context)? - .as_object() - .cloned() - .expect("Constructor must always return a `JsObject` value") - .pipe(Ok) + match (function)(this_target, args, captures, context)? { + JsValue::Object(ref o) => Ok(o.clone()), + val => { + if constructor.is_base() || val.is_undefined() { + create_this(context) + } else { + context.throw_type_error( + "Derived constructor can only return an Object or undefined", + ) + } + } + } } FunctionBody::Ordinary { code, @@ -920,27 +944,14 @@ impl JsObject { } => { std::mem::swap(&mut environments, &mut context.realm.environments); - let this = { - // If the prototype of the constructor is not an object, then use the default object - // prototype as prototype for the new object - // see - // see - let prototype = get_prototype_from_constructor( - this_target, - StandardConstructors::object, - context, - )?; - let this = Self::from_proto_and_data(prototype, ObjectData::ordinary()); - - // Set computed class field names if they exist. - if let Some(fields) = &code.computed_field_names { - for key in fields.borrow().iter().rev() { - context.vm.push(key); - } - } + let this = create_this(context)?; - this - }; + // Set computed class field names if they exist. + if let Some(fields) = &code.computed_field_names { + for key in fields.borrow().iter().rev() { + context.vm.push(key); + } + } if code.params.has_expressions() { context.realm.environments.push_function( @@ -1045,12 +1056,19 @@ impl JsObject { match result { JsValue::Object(ref obj) => Ok(obj.clone()), - JsValue::Undefined => Ok(frame - .this - .as_object() - .cloned() - .expect("13. Assert: Type(thisBinding) is Object.")), - _ => context.throw_type_error("Constructor must return an Object or undefined"), + val => { + if constructor.is_base() || val.is_undefined() { + Ok(frame + .this + .as_object() + .cloned() + .expect("13. Assert: Type(thisBinding) is Object.")) + } else { + context.throw_type_error( + "Derived constructor can only return an Object or undefined", + ) + } + } } } FunctionBody::Generator { .. } => { diff --git a/boa_engine/src/vm/mod.rs b/boa_engine/src/vm/mod.rs index 7ceee547497..84692c9185a 100644 --- a/boa_engine/src/vm/mod.rs +++ b/boa_engine/src/vm/mod.rs @@ -1481,9 +1481,9 @@ impl Context { [compile_environments_index as usize] .clone(); - let is_constructor = self.vm.frame().code.constructor; + let constructor = self.vm.frame().code.constructor; let is_lexical = self.vm.frame().code.this_mode.is_lexical(); - let this = if is_constructor || !is_lexical { + let this = if constructor.is_some() || !is_lexical { self.vm.frame().this.clone() } else { JsValue::undefined()