Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Array.prototype.concat spec compliant #1353

Merged
merged 11 commits into from
Jul 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
102 changes: 81 additions & 21 deletions boa/src/builtins/array/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,30 @@ impl Array {
Ok(array_obj_ptr)
}

/// Utility function for concatenating array objects.
///
/// Returns a Boolean valued property that if `true` indicates that
/// an object should be flattened to its array elements
/// by `Array.prototype.concat`.
fn is_concat_spreadable(this: &Value, context: &mut Context) -> Result<bool> {
// 1. If Type(O) is not Object, return false.
if !this.is_object() {
neeldug marked this conversation as resolved.
Show resolved Hide resolved
return Ok(false);
}
// 2. Let spreadable be ? Get(O, @@isConcatSpreadable).
let spreadable = this.get_field(WellKnownSymbols::is_concat_spreadable(), context)?;

// 3. If spreadable is not undefined, return ! ToBoolean(spreadable).
if !spreadable.is_undefined() {
return Ok(spreadable.to_boolean());
}
// 4. Return ? IsArray(O).
match this.as_object() {
Some(obj) => Ok(obj.is_array()),
_ => Ok(false),
}
}

/// `get Array [ @@species ]`
///
/// The `Array [ @@species ]` accessor property returns the Array constructor.
Expand Down Expand Up @@ -461,30 +485,66 @@ impl Array {
/// [spec]: https://tc39.es/ecma262/#sec-array.prototype.concat
/// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/concat
pub(crate) fn concat(this: &Value, args: &[Value], context: &mut Context) -> Result<Value> {
if args.is_empty() {
// If concat is called with no arguments, it returns the original array
return Ok(this.clone());
}

// Make a new array (using this object as the prototype basis for the new
// one)
let mut new_values: Vec<Value> = Vec::new();

let this_length = this.get_field("length", context)?.to_length(context)?;
for n in 0..this_length {
new_values.push(this.get_field(n, context)?);
}

for concat_array in args {
let concat_length = concat_array
.get_field("length", context)?
.to_length(context)?;
for n in 0..concat_length {
new_values.push(concat_array.get_field(n, context)?);
// 1. Let O be ? ToObject(this value).
let obj = this.to_object(context)?;
// 2. Let A be ? ArraySpeciesCreate(O, 0).
let arr = Self::array_species_create(&obj, 0, context)?;
// 3. Let n be 0.
let mut n = 0;
// 4. Prepend O to items.
// 5. For each element E of items, do
for item in [Value::from(obj)].iter().chain(args.iter()) {
// a. Let spreadable be ? IsConcatSpreadable(E).
let spreadable = Self::is_concat_spreadable(&item, context)?;
// b. If spreadable is true, then
if spreadable {
// item is guaranteed to be an object since is_concat_spreadable checks it,
// so we can call `.unwrap()`
let item = item.as_object().unwrap();
// i. Let k be 0.
// ii. Let len be ? LengthOfArrayLike(E).
let len = item.length_of_array_like(context)?;
// iii. If n + len > 2^53 - 1, throw a TypeError exception.
if n + len > Number::MAX_SAFE_INTEGER as usize {
return context.throw_type_error(
"length + number of arguments exceeds the max safe integer limit",
);
}
// iv. Repeat, while k < len,
for k in 0..len {
// 1. Let P be ! ToString(𝔽(k)).
// 2. Let exists be ? HasProperty(E, P).
let exists = item.has_property(k, context)?;
// 3. If exists is true, then
if exists {
// a. Let subElement be ? Get(E, P).
let sub_element = item.get(k, context)?;
// b. Perform ? CreateDataPropertyOrThrow(A, ! ToString(𝔽(n)), subElement).
arr.create_data_property_or_throw(n, sub_element, context)?;
neeldug marked this conversation as resolved.
Show resolved Hide resolved
}
// 4. Set n to n + 1.
n += 1;
// 5. Set k to k + 1.
}
}
// c. Else,
else {
// i. NOTE: E is added as a single item rather than spread.
// ii. If n ≥ 253 - 1, throw a TypeError exception.
if n >= Number::MAX_SAFE_INTEGER as usize {
return context.throw_type_error("length exceeds the max safe integer limit");
}
// iii. Perform ? CreateDataPropertyOrThrow(A, ! ToString(𝔽(n)), E).
arr.create_data_property_or_throw(n, item, context)?;
// iv. Set n to n + 1.
n += 1
}
}
// 6. Perform ? Set(A, "length", 𝔽(n), true).
arr.set("length", n, true, context)?;

Self::construct_array(this, &new_values, context)
// 7. Return A.
Ok(Value::from(arr))
}

/// `Array.prototype.push( ...items )`
Expand Down
1 change: 0 additions & 1 deletion test_ignore.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ feature:json-modules
//feature:class

// These seem to run forever:
arg-length-exceeding-integer-limit

// These generate a stack overflow
tco-call
Expand Down