[Merged by Bors] - Refactor construct and PromiseCapability to preserve JsObject invariants

boa_engine/src/builtins/array/mod.rs

@@ -366,10 +366,7 @@ impl Array {
         // 7. If IsConstructor(C) is false, throw a TypeError exception.
         if let Some(c) = c.as_constructor() {
             // 8. Return ? Construct(C, « 𝔽(length) »).
-            Ok(c.construct(&[JsValue::new(length)], Some(c), context)?
-                .as_object()
-                .expect("constructing an object should always return an object")
-                .clone())
+            c.construct(&[JsValue::new(length)], Some(c), context)
         } else {
             context.throw_type_error("Symbol.species must be a constructor")
         }
@@ -418,13 +415,7 @@ impl Array {
             // b. Else,
             //     i. Let A be ? ArrayCreate(0en).
             let a = match this.as_constructor() {
-                Some(constructor) => constructor
-                    .construct(&[], None, context)?
-                    .as_object()
-                    .cloned()
-                    .ok_or_else(|| {
-                        context.construct_type_error("Object constructor didn't return an object")
-                    })?,
+                Some(constructor) => constructor.construct(&[], None, context)?,
                 _ => Self::array_create(0, None, context)?,
             };
 
@@ -497,13 +488,7 @@ impl Array {
             // 10. Else,
             //     a. Let A be ? ArrayCreate(len).
             let a = match this.as_constructor() {
-                Some(constructor) => constructor
-                    .construct(&[len.into()], None, context)?
-                    .as_object()
-                    .cloned()
-                    .ok_or_else(|| {
-                        context.construct_type_error("Object constructor didn't return an object")
-                    })?,
+                Some(constructor) => constructor.construct(&[len.into()], None, context)?,
                 _ => Self::array_create(len, None, context)?,
             };
 
@@ -579,13 +564,7 @@ impl Array {
         // 5. Else,
         //     a. Let A be ? ArrayCreate(len).
         let a = match this.as_constructor() {
-            Some(constructor) => constructor
-                .construct(&[len.into()], None, context)?
-                .as_object()
-                .cloned()
-                .ok_or_else(|| {
-                    context.construct_type_error("object constructor didn't return an object")
-                })?,
+            Some(constructor) => constructor.construct(&[len.into()], None, context)?,
             _ => Self::array_create(len, None, context)?,
         };
 

boa_engine/src/builtins/array_buffer/mod.rs

@@ -243,13 +243,9 @@ impl ArrayBuffer {
         // 16. Let new be ? Construct(ctor, « 𝔽(newLen) »).
         let new = ctor.construct(&[new_len.into()], Some(&ctor), context)?;
 
-        // 17. Perform ? RequireInternalSlot(new, [[ArrayBufferData]]).
-        let new_obj = new.as_object().cloned().ok_or_else(|| {
-            context.construct_type_error("ArrayBuffer constructor returned non-object value")
-        })?;
-
         {
-            let new_obj = new_obj.borrow();
+            let new_obj = new.borrow();
+            // 17. Perform ? RequireInternalSlot(new, [[ArrayBufferData]]).
             let new_array_buffer = new_obj.as_array_buffer().ok_or_else(|| {
                 context.construct_type_error("ArrayBuffer constructor returned invalid object")
             })?;
@@ -264,44 +260,46 @@ impl ArrayBuffer {
             }
         }
         // 20. If SameValue(new, O) is true, throw a TypeError exception.
-        if JsValue::same_value(&new, this) {
+        if this.as_object().map(|obj| obj == &new).unwrap_or_default() {
             return context.throw_type_error("New ArrayBuffer is the same as this ArrayBuffer");
         }
 
-        let mut new_obj_borrow = new_obj.borrow_mut();
-        let new_array_buffer = new_obj_borrow
-            .as_array_buffer_mut()
-            .expect("Already checked that `new_obj` was an `ArrayBuffer`");
+        {
+            let mut new_obj_borrow = new.borrow_mut();
+            let new_array_buffer = new_obj_borrow
+                .as_array_buffer_mut()
+                .expect("Already checked that `new_obj` was an `ArrayBuffer`");
 
-        // 21. If new.[[ArrayBufferByteLength]] < newLen, throw a TypeError exception.
-        if new_array_buffer.array_buffer_byte_length < new_len {
-            return context.throw_type_error("New ArrayBuffer length too small");
-        }
+            // 21. If new.[[ArrayBufferByteLength]] < newLen, throw a TypeError exception.
+            if new_array_buffer.array_buffer_byte_length < new_len {
+                return context.throw_type_error("New ArrayBuffer length too small");
+            }
 
-        // 22. NOTE: Side-effects of the above steps may have detached O.
-        // 23. If IsDetachedBuffer(O) is true, throw a TypeError exception.
-        if Self::is_detached_buffer(o) {
-            return context
-                .throw_type_error("ArrayBuffer detached while ArrayBuffer.slice was running");
-        }
+            // 22. NOTE: Side-effects of the above steps may have detached O.
+            // 23. If IsDetachedBuffer(O) is true, throw a TypeError exception.
+            if Self::is_detached_buffer(o) {
+                return context
+                    .throw_type_error("ArrayBuffer detached while ArrayBuffer.slice was running");
+            }
 
-        // 24. Let fromBuf be O.[[ArrayBufferData]].
-        let from_buf = o
-            .array_buffer_data
-            .as_ref()
-            .expect("ArrayBuffer cannot be detached here");
+            // 24. Let fromBuf be O.[[ArrayBufferData]].
+            let from_buf = o
+                .array_buffer_data
+                .as_ref()
+                .expect("ArrayBuffer cannot be detached here");
 
-        // 25. Let toBuf be new.[[ArrayBufferData]].
-        let to_buf = new_array_buffer
-            .array_buffer_data
-            .as_mut()
-            .expect("ArrayBuffer cannot be detached here");
+            // 25. Let toBuf be new.[[ArrayBufferData]].
+            let to_buf = new_array_buffer
+                .array_buffer_data
+                .as_mut()
+                .expect("ArrayBuffer cannot be detached here");
 
-        // 26. Perform CopyDataBlockBytes(toBuf, 0, fromBuf, first, newLen).
-        copy_data_block_bytes(to_buf, 0, from_buf, first as usize, new_len);
+            // 26. Perform CopyDataBlockBytes(toBuf, 0, fromBuf, first, newLen).
+            copy_data_block_bytes(to_buf, 0, from_buf, first as usize, new_len);
+        }
 
         // 27. Return new.
-        Ok(new)
+        Ok(new.into())
     }
 
     /// `25.1.2.1 AllocateArrayBuffer ( constructor, byteLength )`

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,
         }),
     );
 

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<ConstructorKind>,
     },
     Closure {
         #[unsafe_ignore_trace]
         function: Box<dyn ClosureFunctionSignature>,
-        constructor: bool,
+        constructor: Option<ConstructorKind>,
         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<ConstructorKind> {
         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<N>(
             .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),
                 }),
             );
 

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),
             }),
         );
 

boa_engine/src/builtins/promise/mod.rs

@@ -185,7 +185,7 @@ impl PromiseCapability {
                 }
 
                 // 9. Set promiseCapability.[[Promise]] to promise.
-                promise_capability.reject = promise;
+                promise_capability.reject = promise.into();
 
                 // 10. Return promiseCapability.
                 Ok(promise_capability.clone())

boa_engine/src/builtins/reflect/mod.rs

@@ -126,7 +126,9 @@ impl Reflect {
             .create_list_from_array_like(&[], context)?;
 
         // 5. Return ? Construct(target, args, newTarget).
-        target.construct(&args, Some(new_target), context)
+        target
+            .construct(&args, Some(new_target), context)
+            .map(JsValue::from)
     }
 
     /// Defines a property on an object.

boa_engine/src/builtins/regexp/mod.rs

@@ -1184,9 +1184,6 @@ impl RegExp {
 
         // 6. Let matcher be ? Construct(C, « R, flags »).
         let matcher = c.construct(&[this.clone(), flags.clone().into()], Some(&c), context)?;
-        let matcher = matcher
-            .as_object()
-            .expect("construct must always return an Object");
 
         // 7. Let lastIndex be ? ToLength(? Get(R, "lastIndex")).
         let last_index = regexp.get("lastIndex", context)?.to_length(context)?;
@@ -1579,11 +1576,6 @@ impl RegExp {
             Some(&constructor),
             context,
         )?;
-        let splitter = splitter
-            .as_object()
-            // TODO: remove when we handle realms on `get_prototype_from_constructor` and make
-            // `construct` always return a `JsObject`
-            .ok_or_else(|| context.construct_type_error("constructor did not return an object"))?;
 
         // 11. Let A be ! ArrayCreate(0).
         let a = Array::array_create(0, None, context).expect("this ArrayCreate call must not fail");
@@ -1610,7 +1602,7 @@ impl RegExp {
         // 16. If size is 0, then
         if size == 0 {
             // a. Let z be ? RegExpExec(splitter, S).
-            let result = Self::abstract_exec(splitter, arg_str.clone(), context)?;
+            let result = Self::abstract_exec(&splitter, arg_str.clone(), context)?;
 
             // b. If z is not null, return A.
             if result.is_some() {
@@ -1636,7 +1628,7 @@ impl RegExp {
             splitter.set("lastIndex", JsValue::new(q), true, context)?;
 
             // b. Let z be ? RegExpExec(splitter, S).
-            let result = Self::abstract_exec(splitter, arg_str.clone(), context)?;
+            let result = Self::abstract_exec(&splitter, arg_str.clone(), context)?;
 
             // c. If z is null, set q to AdvanceStringIndex(S, q, unicodeMatching).
             // d. Else,

boa_engine/src/builtins/typed_array/mod.rs

@@ -2970,11 +2970,8 @@ impl TypedArray {
         // 1. Let newTypedArray be ? Construct(constructor, argumentList).
         let new_typed_array = constructor.construct(args, Some(constructor), context)?;
 
+        let obj_borrow = new_typed_array.borrow();
         // 2. Perform ? ValidateTypedArray(newTypedArray).
-        let obj = new_typed_array
-            .as_object()
-            .ok_or_else(|| context.construct_type_error("Value is not a typed array object"))?;
-        let obj_borrow = obj.borrow();
         let o = obj_borrow
             .as_typed_array()
             .ok_or_else(|| context.construct_type_error("Value is not a typed array object"))?;
@@ -2994,7 +2991,7 @@ impl TypedArray {
         }
 
         // 4. Return newTypedArray.
-        Ok(obj.clone())
+        Ok(new_typed_array.clone())
     }
 
     /// <https://tc39.es/ecma262/#sec-allocatetypedarraybuffer>

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<Gc<CodeBlock>> {
         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,