Fix treatment of rest pattern in array destructuring.

When seeing a rest pattern, we used to get the remaining elements from the
iterator by calling %concat_iterable_to_array on it.  This was wrong because it
caused an observable [[Get]] for @@iterator (which the iterator may not even
provide).

This CL gets rid of the call to %concat_iterable_to_array and does the iteration
manually in a simple while-loop.  It also gets rid of %concat_iterable_to_array
itself because there aren't any other uses of it.

BUG=v8:4759
LOG=n
R=adamk@chromium.org

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

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

src/contexts.h

@@ -79,7 +79,6 @@ enum BindingFlags {
 
 #define NATIVE_CONTEXT_INTRINSIC_FUNCTIONS(V)                             \
   V(IS_ARRAYLIKE, JSFunction, is_arraylike)                               \
-  V(CONCAT_ITERABLE_TO_ARRAY_INDEX, JSFunction, concat_iterable_to_array) \
   V(GET_TEMPLATE_CALL_SITE_INDEX, JSFunction, get_template_call_site)     \
   V(MAKE_RANGE_ERROR_INDEX, JSFunction, make_range_error)                 \
   V(MAKE_TYPE_ERROR_INDEX, JSFunction, make_type_error)                   \

src/js/runtime.js

@@ -42,14 +42,6 @@ utils.ImportFromExperimental(function(from) {
    ---------------------------------
 */
 
-function ConcatIterableToArray(target, iterable) {
-   var index = target.length;
-   for (var element of iterable) {
-     AddIndexedProperty(target, index++, element);
-   }
-   return target;
-}
-
 
 // This function should be called rather than %AddElement in contexts where the
 // argument might not be less than 2**32-1. ES2015 ToLength semantics mean that
@@ -137,8 +129,4 @@ utils.Export(function(to) {
   to.SpeciesConstructor = SpeciesConstructor;
 });
 
-%InstallToContext([
-  "concat_iterable_to_array", ConcatIterableToArray,
-]);
-
 })

src/parsing/pattern-rewriter.cc

@@ -547,39 +547,82 @@ void Parser::PatternRewriter::VisitArrayLiteral(ArrayLiteral* node,
     // RecurseIntoSubpattern above.
 
     // let array = [];
-    // if (!done) %concat_iterable_to_array(array, iterator);
+    // while (!done) {
+    //   result = IteratorNext(iterator);
+    //   if (result.done) {
+    //     done = true;
+    //   } else {
+    //     %AppendElement(array, result.value);
+    //   }
+    // }
+
+    // let array = [];
+    Variable* array;
+    {
+      auto empty_exprs = new (zone()) ZoneList<Expression*>(0, zone());
+      array = CreateTempVar(factory()->NewArrayLiteral(
+          empty_exprs,
+          // Reuse pattern's literal index - it is unused since there is no
+          // actual literal allocated.
+          node->literal_index(), RelocInfo::kNoPosition));
+    }
+
+    // result = IteratorNext(iterator);
+    Statement* get_next = factory()->NewExpressionStatement(
+        parser_->BuildIteratorNextResult(factory()->NewVariableProxy(iterator),
+                                         result, nopos),
+        nopos);
+
     // done = true;
+    Statement* set_done = factory()->NewExpressionStatement(
+        factory()->NewAssignment(
+            Token::ASSIGN, factory()->NewVariableProxy(done),
+            factory()->NewBooleanLiteral(true, nopos), nopos),
+        nopos);
+
+    // %AppendElement(array, result.value);
+    Statement* append_element;
+    {
+      auto args = new (zone()) ZoneList<Expression*>(2, zone());
+      args->Add(factory()->NewVariableProxy(array), zone());
+      args->Add(factory()->NewProperty(
+                    factory()->NewVariableProxy(result),
+                    factory()->NewStringLiteral(
+                        ast_value_factory()->value_string(), nopos),
+                    nopos),
+                zone());
+      append_element = factory()->NewExpressionStatement(
+          factory()->NewCallRuntime(Runtime::kAppendElement, args, nopos),
+          nopos);
+    }
 
-    auto empty_exprs = new (zone()) ZoneList<Expression*>(0, zone());
-    auto array = CreateTempVar(factory()->NewArrayLiteral(
-        empty_exprs,
-        // Reuse pattern's literal index - it is unused since there is no
-        // actual literal allocated.
-        node->literal_index(), RelocInfo::kNoPosition));
-
-    auto arguments = new (zone()) ZoneList<Expression*>(2, zone());
-    arguments->Add(factory()->NewVariableProxy(array), zone());
-    arguments->Add(factory()->NewVariableProxy(iterator), zone());
-    auto spread_into_array_call =
-        factory()->NewCallRuntime(Context::CONCAT_ITERABLE_TO_ARRAY_INDEX,
-                                  arguments, RelocInfo::kNoPosition);
-
-    auto if_statement = factory()->NewIfStatement(
-        factory()->NewUnaryOperation(Token::NOT,
-                                     factory()->NewVariableProxy(done),
-                                     RelocInfo::kNoPosition),
-        factory()->NewExpressionStatement(spread_into_array_call,
-                                          RelocInfo::kNoPosition),
-        factory()->NewEmptyStatement(RelocInfo::kNoPosition),
-        RelocInfo::kNoPosition);
-    block_->statements()->Add(if_statement, zone());
+    // if (result.done) { #set_done } else { #append_element }
+    Statement* set_done_or_append;
+    {
+      Expression* result_done =
+          factory()->NewProperty(factory()->NewVariableProxy(result),
+                                 factory()->NewStringLiteral(
+                                     ast_value_factory()->done_string(), nopos),
+                                 nopos);
+      set_done_or_append = factory()->NewIfStatement(result_done, set_done,
+                                                     append_element, nopos);
+    }
 
-    auto set_done = factory()->NewAssignment(
-        Token::ASSIGN, factory()->NewVariableProxy(done),
-        factory()->NewBooleanLiteral(true, nopos), nopos);
-    block_->statements()->Add(
-        factory()->NewExpressionStatement(set_done, nopos), zone());
+    // while (!done) {
+    //   #get_next;
+    //   #set_done_or_append;
+    // }
+    WhileStatement* loop = factory()->NewWhileStatement(nullptr, nopos);
+    {
+      Expression* condition = factory()->NewUnaryOperation(
+          Token::NOT, factory()->NewVariableProxy(done), nopos);
+      Block* body = factory()->NewBlock(nullptr, 2, true, nopos);
+      body->statements()->Add(get_next, zone());
+      body->statements()->Add(set_done_or_append, zone());
+      loop->Initialize(condition, body);
+    }
 
+    block_->statements()->Add(loop, zone());
     RecurseIntoSubpattern(spread->expression(),
                           factory()->NewVariableProxy(array));
   }

test/cctest/interpreter/bytecode_expectations/CallRuntime.golden

@@ -83,7 +83,7 @@ bytecodes: [
   B(Star), R(0),
   B(CreateArrayLiteral), U8(0), U8(0), U8(3),
   B(Star), R(1),
-  B(CallJSRuntime), U8(116), R(0), U8(2),
+  B(CallJSRuntime), U8(115), R(0), U8(2),
   B(Return),
 ]
 constant pool: [

test/mjsunit/es6/regress/regress-4759.js

+// Copyright 2016 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.
+
+function iterable(done) {
+  return {
+    [Symbol.iterator]: function() {
+      return {
+        next: function() {
+          if (done) return { done: true };
+          done = true;
+          return { value: 42, done: false };
+        }
+      }
+    }
+  }
+}
+
+var [...result] = iterable(true);
+assertEquals([], result);
+
+var [...result] = iterable(false);
+assertEquals([42], result);