Refactor construct and PromiseCapability to preserve JsObject i…

…nvariants (#2136) This Pull Request changes the signature of `construct` to always return `JsObject`s, and refactors `PromiseCapability` to store only `JsObject`/`JsFunction`s. This preserves the following invariants specified in the spec: https://tc39.es/ecma262/multipage/ordinary-and-exotic-objects-behaviours.html#sec-ecmascript-function-objects-construct-argumentslist-newtarget > The [[Construct]] internal method of an ECMAScript function object ... returns either a normal completion containing an Object or a throw completion ... https://tc39.es/ecma262/multipage/control-abstraction-objects.html#sec-promisecapability-records Table 82: [PromiseCapability Record](https://tc39.es/ecma262/multipage/control-abstraction-objects.html#sec-promisecapability-records) Fields Field Name | Value | Meaning -- | -- | -- [[Promise]] | an Object | An object that is usable as a promise. [[Resolve]] | a function object | The function that is used to resolve the given promise. [[Reject]] | a function object | The function that is used to reject the given promise. Co-authored-by: Iban Eguia <razican@protonmail.ch>
boa-dev · Jun 22, 2022 · 0ebb3cf · 0ebb3cf
1 parent 613f4c3
commit 0ebb3cf
Show file tree

Hide file tree

Showing 20 changed files with 249 additions and 211 deletions.
diff --git a/boa_engine/src/builtins/array/mod.rs b/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)?,
         };
 

diff --git a/boa_engine/src/builtins/array_buffer/mod.rs b/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,50 @@ 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| JsObject::equals(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 )`

diff --git 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
@@ -108,7 +108,7 @@ impl ThisMode {
     }
 }
 
-#[derive(Debug, Trace, Finalize, PartialEq, Clone)]
+#[derive(Debug, PartialEq, Clone, Copy)]
 pub enum ConstructorKind {
     Base,
     Derived,
@@ -178,12 +178,14 @@ pub enum Function {
     Native {
         #[unsafe_ignore_trace]
         function: NativeFunctionSignature,
-        constructor: bool,
+        #[unsafe_ignore_trace]
+        constructor: Option<ConstructorKind>,
     },
     Closure {
         #[unsafe_ignore_trace]
         function: Box<dyn ClosureFunctionSignature>,
-        constructor: bool,
+        #[unsafe_ignore_trace]
+        constructor: Option<ConstructorKind>,
         captures: Captures,
     },
     Ordinary {
@@ -205,6 +207,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 +258,7 @@ pub(crate) fn make_builtin_fn<N>(
             .prototype(),
         ObjectData::function(Function::Native {
             function,
-            constructor: false,
+            constructor: None,
         }),
     );
     let attribute = PropertyDescriptor::builder()
@@ -417,7 +424,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
@@ -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),
             }),
         );