Feature arrays with empty elements (#1870)

This PR adds support for arrays with empty elements e.g. `["", false, , , ]`. Before we were filling the empty places with `undefined`, but this is wrong according to [spec](https://tc39.es/ecma262/#sec-runtime-semantics-arrayaccumulation) there shouldn't be undefined with a index at that place, instead only `length` is incremented. So `[,,,].length == 3` and operations like `[,,,,].indexOf(undefined) == -1`, `[,,,,].lastIndexOf(undefined) == -1` etc.
boa-dev · Feb 27, 2022 · ada4ca8 · ada4ca8
1 parent 6b2ca30
commit ada4ca8
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 17 deletions.
diff --git a/boa_engine/src/bytecompiler.rs b/boa_engine/src/bytecompiler.rs
@@ -933,6 +933,11 @@ impl<'b> ByteCompiler<'b> {
                 self.emit_opcode(Opcode::PopOnReturnAdd);
 
                 for element in array.as_ref() {
+                    if let Node::Empty = element {
+                        self.emit_opcode(Opcode::PushElisionToArray);
+                        continue;
+                    }
+
                     self.compile_expr(element, true)?;
                     if let Node::Spread(_) = element {
                         self.emit_opcode(Opcode::InitIterator);

diff --git a/boa_engine/src/syntax/parser/expression/primary/array_initializer/mod.rs b/boa_engine/src/syntax/parser/expression/primary/array_initializer/mod.rs
@@ -13,7 +13,7 @@ mod tests;
 use crate::syntax::{
     ast::{
         node::{ArrayDecl, Node, Spread},
-        Const, Punctuator,
+        Punctuator,
     },
     parser::{
         expression::AssignmentExpression, AllowAwait, AllowYield, Cursor, ParseError, TokenParser,
@@ -68,7 +68,7 @@ where
         loop {
             // TODO: Support all features.
             while cursor.next_if(Punctuator::Comma, interner)?.is_some() {
-                elements.push(Node::Const(Const::Undefined));
+                elements.push(Node::Empty);
             }
 
             if cursor

diff --git a/boa_engine/src/syntax/parser/expression/primary/array_initializer/tests.rs b/boa_engine/src/syntax/parser/expression/primary/array_initializer/tests.rs
@@ -1,7 +1,7 @@
 // ! Tests for array initializer parsing.
 
 use crate::syntax::{
-    ast::{node::ArrayDecl, Const},
+    ast::{node::ArrayDecl, Const, Node},
     parser::tests::check_parser,
 };
 use boa_interner::{Interner, Sym};
@@ -19,7 +19,7 @@ fn check_empty_slot() {
     let mut interner = Interner::default();
     check_parser(
         "[,]",
-        vec![ArrayDecl::from(vec![Const::Undefined.into()]).into()],
+        vec![ArrayDecl::from(vec![Node::Empty]).into()],
         &mut interner,
     );
 }
@@ -65,7 +65,7 @@ fn check_numeric_array_elision() {
         vec![ArrayDecl::from(vec![
             Const::from(1).into(),
             Const::from(2).into(),
-            Const::Undefined.into(),
+            Node::Empty,
             Const::from(3).into(),
         ])
         .into()],
@@ -82,8 +82,8 @@ fn check_numeric_array_repeated_elision() {
         vec![ArrayDecl::from(vec![
             Const::from(1).into(),
             Const::from(2).into(),
-            Const::Undefined.into(),
-            Const::Undefined.into(),
+            Node::Empty,
+            Node::Empty,
             Const::from(3).into(),
         ])
         .into()],

diff --git a/boa_engine/src/value/display.rs b/boa_engine/src/value/display.rs
@@ -134,16 +134,18 @@ pub(crate) fn log_string_from(x: &JsValue, print_internals: bool, print_children
                             .map(|i| {
                                 // Introduce recursive call to stringify any objects
                                 // which are part of the Array
-                                log_string_from(
-                                    v.borrow()
-                                        .properties()
-                                        .get(&i.into())
-                                        // FIXME: handle accessor descriptors
-                                        .and_then(PropertyDescriptor::value)
-                                        .unwrap_or(&JsValue::Undefined),
-                                    print_internals,
-                                    false,
-                                )
+
+                                // FIXME: handle accessor descriptors
+                                if let Some(value) = v
+                                    .borrow()
+                                    .properties()
+                                    .get(&i.into())
+                                    .and_then(PropertyDescriptor::value)
+                                {
+                                    log_string_from(value, print_internals, false)
+                                } else {
+                                    String::from("<empty>")
+                                }
                             })
                             .collect::<Vec<String>>()
                             .join(", ");

diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs
@@ -318,6 +318,7 @@ impl CodeBlock {
             | Opcode::RestParameterInit
             | Opcode::RestParameterPop
             | Opcode::PushValueToArray
+            | Opcode::PushElisionToArray
             | Opcode::PushIteratorToArray
             | Opcode::PushNewArray
             | Opcode::PopOnReturnAdd

diff --git a/boa_engine/src/vm/mod.rs b/boa_engine/src/vm/mod.rs
@@ -203,6 +203,17 @@ impl Context {
                     .expect("should be able to create new data property");
                 self.vm.push(array);
             }
+            Opcode::PushElisionToArray => {
+                let array = self.vm.pop();
+                let o = array.as_object().expect("should always be an object");
+
+                let len = o
+                    .length_of_array_like(self)
+                    .expect("arrays should always have a 'length' property");
+
+                o.set("length", len + 1, true, self)?;
+                self.vm.push(array);
+            }
             Opcode::PushIteratorToArray => {
                 let next_function = self.vm.pop();
                 let iterator = self.vm.pop();

diff --git a/boa_engine/src/vm/opcode.rs b/boa_engine/src/vm/opcode.rs
@@ -145,6 +145,13 @@ pub enum Opcode {
     /// Stack: array, value **=>** array
     PushValueToArray,
 
+    /// Push an empty element/hole to an array.
+    ///
+    /// Operands:
+    ///
+    /// Stack: array **=>** array
+    PushElisionToArray,
+
     /// Push all iterator values to an array.
     ///
     /// Operands:
@@ -937,6 +944,7 @@ impl Opcode {
             Opcode::PushEmptyObject => "PushEmptyObject",
             Opcode::PushNewArray => "PushNewArray",
             Opcode::PushValueToArray => "PushValueToArray",
+            Opcode::PushElisionToArray => "PushElisionToArray",
             Opcode::PushIteratorToArray => "PushIteratorToArray",
             Opcode::Add => "Add",
             Opcode::Sub => "Sub",