Skip to content

Commit

Permalink
Ensure construct always returns an Object
Browse files Browse the repository at this point in the history
  • Loading branch information
jedel1043 committed Jun 21, 2022
1 parent 3f4232b commit 4247ae8
Show file tree
Hide file tree
Showing 18 changed files with 170 additions and 144 deletions.
29 changes: 4 additions & 25 deletions boa_engine/src/builtins/array/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down Expand Up @@ -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)?,
};

Expand Down Expand Up @@ -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)?,
};

Expand Down Expand Up @@ -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)?,
};

Expand Down
66 changes: 32 additions & 34 deletions boa_engine/src/builtins/array_buffer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})?;
Expand All @@ -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 )`
Expand Down
2 changes: 1 addition & 1 deletion boa_engine/src/builtins/error/type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}),
);

Expand Down
22 changes: 16 additions & 6 deletions boa_engine/src/builtins/function/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -417,7 +427,7 @@ impl BuiltInFunctionObject {
prototype,
ObjectData::function(Function::Native {
function: |_, _, _| Ok(JsValue::undefined()),
constructor: true,
constructor: Some(ConstructorKind::Base),
}),
);

Expand Down
6 changes: 4 additions & 2 deletions boa_engine/src/builtins/generator_function/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -124,7 +126,7 @@ impl GeneratorFunction {
prototype,
ObjectData::function(Function::Native {
function: |_, _, _| Ok(JsValue::undefined()),
constructor: true,
constructor: Some(ConstructorKind::Base),
}),
);

Expand Down
2 changes: 1 addition & 1 deletion boa_engine/src/builtins/promise/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
4 changes: 3 additions & 1 deletion boa_engine/src/builtins/reflect/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
12 changes: 2 additions & 10 deletions boa_engine/src/builtins/regexp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand Down Expand Up @@ -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");
Expand All @@ -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() {
Expand All @@ -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,
Expand Down
7 changes: 2 additions & 5 deletions boa_engine/src/builtins/typed_array/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))?;
Expand All @@ -2994,7 +2991,7 @@ impl TypedArray {
}

// 4. Return newTypedArray.
Ok(obj.clone())
Ok(new_typed_array.clone())
}

/// <https://tc39.es/ecma262/#sec-allocatetypedarraybuffer>
Expand Down
26 changes: 20 additions & 6 deletions boa_engine/src/bytecompiler.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{
builtins::function::ThisMode,
builtins::function::{ConstructorKind, ThisMode},
environments::{BindingLocator, CompileTimeEnvironment},
syntax::ast::{
node::{
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down
Loading

0 comments on commit 4247ae8

Please sign in to comment.