Make Array.prototype.concat spec compliant

boa/benches/bench_scripts/array_concat.js

@@ -0,0 +1,6 @@
+(function () {
+  let arr1 = [1, 2, 3, 4, 5, 6, 7, 8, 9, 0];
+  let arr2 = [1, 2, 3, 4, 5, 6, 7, 8, 9, 0];
+
+  let arr3 = arr1.concat(arr2);
+})();

boa/benches/exec.rs

@@ -330,6 +330,18 @@ fn mini_js(c: &mut Criterion) {
     });
 }
 
+static ARRAY_CONCAT: &str = include_str!("bench_scripts/array_concat.js");
+
+fn array_concat(c: &mut Criterion) {
+    let mut context = Context::new();
+    let nodes = Parser::new(ARRAY_CONCAT.as_bytes(), false)
+        .parse_all()
+        .unwrap();
+    c.bench_function("Array concat (Execution)", move |b| {
+        b.iter(|| black_box(&nodes).run(&mut context).unwrap())
+    });
+}
+
 criterion_group!(
     execution,
     create_realm,
@@ -355,5 +367,6 @@ criterion_group!(
     arithmetic_operations,
     clean_js,
     mini_js,
+    array_concat,
 );
 criterion_main!(execution);

boa/benches/full.rs

@@ -196,6 +196,14 @@ fn mini_js(c: &mut Criterion) {
     });
 }
 
+static ARRAY_CONCAT: &str = include_str!("bench_scripts/array_concat.js");
+
+fn array_concat(c: &mut Criterion) {
+    c.bench_function("Array concat (Full)", move |b| {
+        b.iter(|| Context::new().eval(black_box(ARRAY_CONCAT)))
+    });
+}
+
 criterion_group!(
     full,
     symbol_creation,
@@ -220,5 +228,6 @@ criterion_group!(
     arithmetic_operations,
     clean_js,
     mini_js,
+    array_concat,
 );
 criterion_main!(full);

boa/benches/parser.rs

@@ -86,6 +86,14 @@ fn mini_js(c: &mut Criterion) {
     });
 }
 
+static ARRAY_CONCAT: &str = include_str!("bench_scripts/array_concat.js");
+
+fn array_concat(c: &mut Criterion) {
+    c.bench_function("Array concat (Parser)", move |b| {
+        b.iter(|| Parser::new(black_box(ARRAY_CONCAT.as_bytes()), false).parse_all())
+    });
+}
+
 criterion_group!(
     parser,
     expression_parser,
@@ -95,5 +103,6 @@ criterion_group!(
     goal_symbol_switch,
     clean_js,
     mini_js,
+    array_concat,
 );
 criterion_main!(parser);

boa/src/builtins/array/mod.rs

@@ -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() {
+            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.
@@ -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)?;
+                    }
+                    // 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 )`

test_ignore.txt

@@ -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