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 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 43 additions & 3 deletions boa/src/builtins/array/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,31 @@ 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`.
pub(crate) 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 @@ -468,19 +493,34 @@ impl Array {

// Make a new array (using this object as the prototype basis for the new
// one)
let mut new_values: Vec<Value> = Vec::new();
let mut new_values = 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)?);
}

let mut n = this_length;

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)?);
let spreadable = Self::is_concat_spreadable(concat_array, context)?;
if spreadable {
if n + concat_length > Number::MAX_SAFE_INTEGER as usize {
return context.throw_type_error("Invalid array length");
}
for k in 0..concat_length {
new_values.push(concat_array.get_field(k, context)?);
n += 1;
}
} else {
if n >= Number::MAX_SAFE_INTEGER as usize {
return context.throw_type_error("Invalid array length");
}
new_values.push(concat_array.get_field(0, context)?);
n += 1;
}
}

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