From ada4ca895fb3859025edeec3aad22d6b1036ebd5 Mon Sep 17 00:00:00 2001 From: Halid Odat Date: Sun, 27 Feb 2022 17:08:52 +0000 Subject: [PATCH] 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_engine/src/bytecompiler.rs | 5 +++++ .../primary/array_initializer/mod.rs | 4 ++-- .../primary/array_initializer/tests.rs | 10 ++++----- boa_engine/src/value/display.rs | 22 ++++++++++--------- boa_engine/src/vm/code_block.rs | 1 + boa_engine/src/vm/mod.rs | 11 ++++++++++ boa_engine/src/vm/opcode.rs | 8 +++++++ 7 files changed, 44 insertions(+), 17 deletions(-) diff --git a/boa_engine/src/bytecompiler.rs b/boa_engine/src/bytecompiler.rs index 1f4550a98d..e50a4173b9 100644 --- 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 index b017f779fa..b668370fb2 100644 --- 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 index 554cb6e901..ed1063cd31 100644 --- 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 index 97f2754bbc..2e6d8855b7 100644 --- 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("") + } }) .collect::>() .join(", "); diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs index 58ec989037..44e5646f4c 100644 --- 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 index 6205e37c5a..ad3658228f 100644 --- 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 index e051811a83..b6248b2454 100644 --- 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",