Fix spread array inside array literal

During parsing, we now keep track of the first spread seen in an array
literal (if any), and make use of that information when creating the
FixedArray backing store representing the constant elements for array
literal materialization.

The old code tried to do this by setting the generated JSArray's length
in ArrayLiteral::BuildConstantElements(), but that Array length is never
read by the rest of the literal materialization code (it always uses
the length of the FixedArray backing store).

BUG=v8:4298
LOG=n

Review URL: https://codereview.chromium.org/1225223004

Cr-Commit-Position: refs/heads/master@{#29684}

diff --git a/src/ast.cc b/src/ast.cc
index f7eab6ec710dd438fbe3d9095e90f5e00e086d85..93cb6c09b99358f6a118f58e83442ef01f58e832 100644 (file)
--- a/src/ast.cc
+++ b/src/ast.cc
@@ -505,9 +505,12 @@ void ObjectLiteral::BuildConstantProperties(Isolate* isolate) {
 void ArrayLiteral::BuildConstantElements(Isolate* isolate) {
   if (!constant_elements_.is_null()) return;
 
+  int constants_length =
+      first_spread_index_ >= 0 ? first_spread_index_ : values()->length();
+
   // Allocate a fixed array to hold all the object literals.
   Handle<JSArray> array = isolate->factory()->NewJSArray(
-      FAST_HOLEY_SMI_ELEMENTS, values()->length(), values()->length(),
+      FAST_HOLEY_SMI_ELEMENTS, constants_length, constants_length,
       Strength::WEAK, INITIALIZE_ARRAY_ELEMENTS_WITH_HOLE);
 
   // Fill in the literals.
@@ -515,9 +518,9 @@ void ArrayLiteral::BuildConstantElements(Isolate* isolate) {
   int depth_acc = 1;
   bool is_holey = false;
   int array_index = 0;
-  for (int n = values()->length(); array_index < n; array_index++) {
+  for (; array_index < constants_length; array_index++) {
     Expression* element = values()->at(array_index);
-    if (element->IsSpread()) break;
+    DCHECK(!element->IsSpread());
     MaterializedLiteral* m_literal = element->AsMaterializedLiteral();
     if (m_literal != NULL) {
       m_literal->BuildConstants(isolate);
@@ -543,9 +546,6 @@ void ArrayLiteral::BuildConstantElements(Isolate* isolate) {
         .Assert();
   }
 
-  if (array_index != values()->length()) {
-    JSArray::SetLength(array, array_index);
-  }
   JSObject::ValidateElements(array);
   Handle<FixedArrayBase> element_values(array->elements());
 

diff --git a/src/ast.h b/src/ast.h
index 3a51982ec8bd2014ded90c86d8e13c2f27137823..b1a634edb4c7f02f4dc96658e3e9dba6c5cb9908 100644 (file)
--- a/src/ast.h
+++ b/src/ast.h
@@ -1612,10 +1612,12 @@ class ArrayLiteral final : public MaterializedLiteral {
   };
 
  protected:
-  ArrayLiteral(Zone* zone, ZoneList<Expression*>* values, int literal_index,
-               bool is_strong, int pos)
+  ArrayLiteral(Zone* zone, ZoneList<Expression*>* values,
+               int first_spread_index, int literal_index, bool is_strong,
+               int pos)
       : MaterializedLiteral(zone, literal_index, is_strong, pos),
-        values_(values) {}
+        values_(values),
+        first_spread_index_(first_spread_index) {}
   static int parent_num_ids() { return MaterializedLiteral::num_ids(); }
 
  private:
@@ -1623,6 +1625,7 @@ class ArrayLiteral final : public MaterializedLiteral {
 
   Handle<FixedArray> constant_elements_;
   ZoneList<Expression*>* values_;
+  int first_spread_index_;
 };
 
 
@@ -3452,8 +3455,15 @@ class AstNodeFactory final BASE_EMBEDDED {
                                 int literal_index,
                                 bool is_strong,
                                 int pos) {
-    return new (zone_) ArrayLiteral(zone_, values, literal_index, is_strong,
-                                    pos);
+    return new (zone_)
+        ArrayLiteral(zone_, values, -1, literal_index, is_strong, pos);
+  }
+
+  ArrayLiteral* NewArrayLiteral(ZoneList<Expression*>* values,
+                                int first_spread_index, int literal_index,
+                                bool is_strong, int pos) {
+    return new (zone_) ArrayLiteral(zone_, values, first_spread_index,
+                                    literal_index, is_strong, pos);
   }
 
   VariableProxy* NewVariableProxy(Variable* var,

diff --git a/src/preparser.h b/src/preparser.h
index 14951e6f919529a1c8c6d2631ec0cef12e8beb30..921cd875a1816abaee53eed6db15e6222465bc5a 100644 (file)
--- a/src/preparser.h
+++ b/src/preparser.h
@@ -1175,6 +1175,11 @@ class PreParserFactory {
                                       int pos) {
     return PreParserExpression::Default();
   }
+  PreParserExpression NewArrayLiteral(PreParserExpressionList values,
+                                      int first_spread_index, int literal_index,
+                                      bool is_strong, int pos) {
+    return PreParserExpression::Default();
+  }
   PreParserExpression NewObjectLiteralProperty(PreParserExpression key,
                                                PreParserExpression value,
                                                ObjectLiteralProperty::Kind kind,
@@ -2397,6 +2402,7 @@ typename ParserBase<Traits>::ExpressionT ParserBase<Traits>::ParseArrayLiteral(
   int pos = peek_position();
   typename Traits::Type::ExpressionList values =
       this->NewExpressionList(4, zone_);
+  int first_spread_index = -1;
   Expect(Token::LBRACK, CHECK_OK);
   while (peek() != Token::RBRACK) {
     bool seen_spread = false;
@@ -2419,6 +2425,9 @@ typename ParserBase<Traits>::ExpressionT ParserBase<Traits>::ParseArrayLiteral(
           this->ParseAssignmentExpression(true, classifier, CHECK_OK);
       elem = factory()->NewSpread(argument, start_pos);
       seen_spread = true;
+      if (first_spread_index < 0) {
+        first_spread_index = values->length();
+      }
     } else {
       elem = this->ParseAssignmentExpression(true, classifier, CHECK_OK);
     }
@@ -2435,7 +2444,7 @@ typename ParserBase<Traits>::ExpressionT ParserBase<Traits>::ParseArrayLiteral(
   // Update the scope information before the pre-parsing bailout.
   int literal_index = function_state_->NextMaterializedLiteralIndex();
 
-  return factory()->NewArrayLiteral(values, literal_index,
+  return factory()->NewArrayLiteral(values, first_spread_index, literal_index,
                                     is_strong(language_mode()), pos);
 }
 

diff --git a/test/mjsunit/harmony/regress/regress-4298.js b/test/mjsunit/harmony/regress/regress-4298.js
new file mode 100644 (file)
index 0000000..98e69d1
--- /dev/null
+++ b/test/mjsunit/harmony/regress/regress-4298.js
@@ -0,0 +1,8 @@
+// Copyright 2015 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// Flags: --harmony-spread-arrays
+
+var arr = [1, 2, ...[3]];
+assertEquals([1, 2, 3], arr);