[es6/cleanup]: re-implement ES6 array spreads in BytecodeGenerator

This gets rid of all the RewriteNonPattern gunk in the parser and
expression classifier, and removes one use of RewritableExpression.

This borrows pieces from several other CLs of mine which are currently
open, and includes a new and modernized abstraction for dealing with
iterators in BytecodeGenerator (so, this CL adds that, moves code from
BuildGetIterator around, and makes some minor changes to yield* which
should maintain compatability with the old behaviour).

This also implements a portion of the changes to the iteration protocol
(implemented fully in
https://chromium-review.googlesource.com/c/v8/v8/+/687997), but only for
the spread operator in Array Literals (the rest will follow).

BUG=v8:5940, v8:3018
R=rmcilroy@chromium.org, marja@chromium.org, adamk@chromium.org
TBR=adamk@chromium.org

Change-Id: Ifc494d663d8e46066a439c3541c33f0243726234
Reviewed-on: https://chromium-review.googlesource.com/804396
Commit-Queue: Caitlin Potter <caitp@igalia.com>
Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Mythri Alle <mythria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50138}

src/ast/ast.cc

@@ -514,7 +514,6 @@ bool ArrayLiteral::is_empty() const {
 }
 
 int ArrayLiteral::InitDepthAndFlags() {
-  DCHECK_LT(first_spread_index_, 0);
   if (is_initialized()) return depth();
 
   int constants_length = values()->length();
@@ -525,7 +524,6 @@ int ArrayLiteral::InitDepthAndFlags() {
   int array_index = 0;
   for (; array_index < constants_length; array_index++) {
     Expression* element = values()->at(array_index);
-    DCHECK(!element->IsSpread());
     MaterializedLiteral* literal = element->AsMaterializedLiteral();
     if (literal != nullptr) {
       int subliteral_depth = literal->InitDepthAndFlags() + 1;
@@ -546,11 +544,10 @@ int ArrayLiteral::InitDepthAndFlags() {
 }
 
 void ArrayLiteral::BuildConstantElements(Isolate* isolate) {
-  DCHECK_LT(first_spread_index_, 0);
-
   if (!constant_elements_.is_null()) return;
 
-  int constants_length = values()->length();
+  int constants_length =
+      first_spread_index_ >= 0 ? first_spread_index_ : values()->length();
   ElementsKind kind = FIRST_FAST_ELEMENTS_KIND;
   Handle<FixedArray> fixed_array =
       isolate->factory()->NewFixedArrayWithHoles(constants_length);
@@ -614,11 +611,6 @@ bool ArrayLiteral::IsFastCloningSupported() const {
              ConstructorBuiltins::kMaximumClonedShallowArrayElements;
 }
 
-void ArrayLiteral::RewindSpreads() {
-  values_->Rewind(first_spread_index_);
-  first_spread_index_ = -1;
-}
-
 bool MaterializedLiteral::IsSimple() const {
   if (IsArrayLiteral()) return AsArrayLiteral()->is_simple();
   if (IsObjectLiteral()) return AsObjectLiteral()->is_simple();

src/ast/ast.h

@@ -1450,15 +1450,15 @@ class ArrayLiteral final : public AggregateLiteral {
   }
 
   // Provide a mechanism for iterating through values to rewrite spreads.
-  ZoneList<Expression*>::iterator FirstSpread() const {
+  ZoneList<Expression*>::iterator FirstSpreadOrEndValue() const {
     return (first_spread_index_ >= 0) ? values_->begin() + first_spread_index_
                                       : values_->end();
   }
+  ZoneList<Expression*>::iterator BeginValue() const {
+    return values_->begin();
+  }
   ZoneList<Expression*>::iterator EndValue() const { return values_->end(); }
 
-  // Rewind an array literal omitting everything from the first spread on.
-  void RewindSpreads();
-
  private:
   friend class AstNodeFactory;
 

src/ast/prettyprinter.cc

@@ -26,6 +26,7 @@ CallPrinter::CallPrinter(Isolate* isolate, bool is_user_js)
   is_iterator_error_ = false;
   is_async_iterator_error_ = false;
   is_user_js_ = is_user_js;
+  function_kind_ = kNormalFunction;
   InitializeAstVisitor(isolate);
 }
 
@@ -187,7 +188,10 @@ void CallPrinter::VisitDebuggerStatement(DebuggerStatement* node) {}
 
 
 void CallPrinter::VisitFunctionLiteral(FunctionLiteral* node) {
+  FunctionKind last_function_kind = function_kind_;
+  function_kind_ = node->kind();
   FindStatements(node->body());
+  function_kind_ = last_function_kind;
 }
 
 
@@ -250,7 +254,17 @@ void CallPrinter::VisitArrayLiteral(ArrayLiteral* node) {
   Print("[");
   for (int i = 0; i < node->values()->length(); i++) {
     if (i != 0) Print(",");
-    Find(node->values()->at(i), true);
+    Expression* subexpr = node->values()->at(i);
+    Spread* spread = subexpr->AsSpread();
+    if (spread != nullptr && !found_ &&
+        position_ == spread->expression()->position()) {
+      found_ = true;
+      is_iterator_error_ = true;
+      Find(spread->expression(), true);
+      done_ = true;
+      return;
+    }
+    Find(subexpr, true);
   }
   Print("]");
 }
@@ -277,7 +291,17 @@ void CallPrinter::VisitCompoundAssignment(CompoundAssignment* node) {
 
 void CallPrinter::VisitYield(Yield* node) { Find(node->expression()); }
 
-void CallPrinter::VisitYieldStar(YieldStar* node) { Find(node->expression()); }
+void CallPrinter::VisitYieldStar(YieldStar* node) {
+  if (!found_ && position_ == node->expression()->position()) {
+    found_ = true;
+    if (IsAsyncFunction(function_kind_))
+      is_async_iterator_error_ = true;
+    else
+      is_iterator_error_ = true;
+    Print("yield* ");
+  }
+  Find(node->expression());
+}
 
 void CallPrinter::VisitAwait(Await* node) { Find(node->expression()); }
 

src/ast/prettyprinter.h

@@ -50,6 +50,7 @@ class CallPrinter final : public AstVisitor<CallPrinter> {
   bool is_iterator_error_;
   bool is_async_iterator_error_;
   bool is_call_error_;
+  FunctionKind function_kind_;
   DEFINE_AST_VISITOR_SUBCLASS_MEMBERS();
 
  protected:

src/interpreter/bytecode-generator.h

@@ -56,6 +56,7 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
   class EffectResultScope;
   class FeedbackSlotCache;
   class GlobalDeclarationsBuilder;
+  class IteratorRecord;
   class NaryCodeCoverageSlots;
   class RegisterAllocationScope;
   class TestResultScope;
@@ -150,6 +151,11 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
   void BuildAwait(int suspend_id);
 
   void BuildGetIterator(Expression* iterable, IteratorType hint);
+  IteratorRecord BuildGetIteratorRecord(Expression* iterable,
+                                        IteratorType hint);
+  void BuildIteratorNext(const IteratorRecord& iterator, Register next_result);
+
+  void BuildArrayLiteralSpread(Spread* spread, Register array);
 
   void AllocateTopLevelRegisters();
   void VisitArgumentsObject(Variable* variable);

src/parsing/expression-classifier.h

@@ -97,14 +97,12 @@ class ExpressionClassifier {
       : base_(base),
         previous_(base->classifier_),
         zone_(base->impl()->zone()),
-        non_patterns_to_rewrite_(base->impl()->GetNonPatternList()),
         reported_errors_(base->impl()->GetReportedErrorList()),
         duplicate_finder_(duplicate_finder),
         invalid_productions_(0),
         function_properties_(0) {
     base->classifier_ = this;
     reported_errors_begin_ = reported_errors_end_ = reported_errors_->length();
-    non_pattern_begin_ = non_patterns_to_rewrite_->length();
   }
 
   V8_INLINE ~ExpressionClassifier() {
@@ -291,19 +289,10 @@ class ExpressionClassifier {
     Add(Error(loc, message, kLetPatternProduction, arg));
   }
 
-  void Accumulate(ExpressionClassifier* inner, unsigned productions,
-                  bool merge_non_patterns = true) {
+  void Accumulate(ExpressionClassifier* inner, unsigned productions) {
     DCHECK_EQ(inner->reported_errors_, reported_errors_);
     DCHECK_EQ(inner->reported_errors_begin_, reported_errors_end_);
     DCHECK_EQ(inner->reported_errors_end_, reported_errors_->length());
-    DCHECK_EQ(inner->non_patterns_to_rewrite_, non_patterns_to_rewrite_);
-    DCHECK_LE(non_pattern_begin_, inner->non_pattern_begin_);
-    DCHECK_LE(inner->non_pattern_begin_, non_patterns_to_rewrite_->length());
-    // Merge non-patterns from the inner classifier, or discard them.
-    if (merge_non_patterns)
-      inner->non_pattern_begin_ = non_patterns_to_rewrite_->length();
-    else
-      non_patterns_to_rewrite_->Rewind(inner->non_pattern_begin_);
     // Propagate errors from inner, but don't overwrite already recorded
     // errors.
     unsigned non_arrow_inner_invalid_productions =
@@ -368,16 +357,12 @@ class ExpressionClassifier {
         reported_errors_end_;
   }
 
-  V8_INLINE int GetNonPatternBegin() const { return non_pattern_begin_; }
-
   V8_INLINE void Discard() {
     if (reported_errors_end_ == reported_errors_->length()) {
       reported_errors_->Rewind(reported_errors_begin_);
       reported_errors_end_ = reported_errors_begin_;
     }
     DCHECK_EQ(reported_errors_begin_, reported_errors_end_);
-    DCHECK_LE(non_pattern_begin_, non_patterns_to_rewrite_->length());
-    non_patterns_to_rewrite_->Rewind(non_pattern_begin_);
   }
 
   ExpressionClassifier* previous() const { return previous_; }
@@ -424,16 +409,8 @@ class ExpressionClassifier {
   typename Types::Base* base_;
   ExpressionClassifier* previous_;
   Zone* zone_;
-  ZoneList<typename Types::RewritableExpression>* non_patterns_to_rewrite_;
   ZoneList<Error>* reported_errors_;
   DuplicateFinder* duplicate_finder_;
-  // The uint16_t for non_pattern_begin_ will not be enough in the case,
-  // e.g., of an array literal containing more than 64K inner array
-  // literals with spreads, as in:
-  // var N=65536; eval("var x=[];" + "[" + "[...x],".repeat(N) + "].length");
-  // An implementation limit error in ParserBase::AddNonPatternForRewriting
-  // will be triggered in this case.
-  uint16_t non_pattern_begin_;
   unsigned invalid_productions_ : 14;
   unsigned function_properties_ : 2;
   // The uint16_t for reported_errors_begin_ and reported_errors_end_ will

src/parsing/parser.cc

@@ -1251,7 +1251,6 @@ Statement* Parser::ParseExportDefault(bool* ok) {
       int pos = position();
       ExpressionClassifier classifier(this);
       Expression* value = ParseAssignmentExpression(true, CHECK_OK);
-      RewriteNonPattern(CHECK_OK);
       SetFunctionName(value, ast_value_factory()->default_string());
 
       const AstRawString* local_name =
@@ -3810,24 +3809,6 @@ void Parser::RewriteAsyncFunctionBody(ZoneList<Statement*>* body, Block* block,
   body->Add(block, zone());
 }
 
-void Parser::RewriteNonPattern(bool* ok) {
-  ValidateExpression(CHECK_OK_VOID);
-  auto non_patterns_to_rewrite = function_state_->non_patterns_to_rewrite();
-  int begin = classifier()->GetNonPatternBegin();
-  int end = non_patterns_to_rewrite->length();
-  if (begin < end) {
-    for (int i = begin; i < end; i++) {
-      RewritableExpression* expr = non_patterns_to_rewrite->at(i);
-      // TODO(adamk): Make this more typesafe.
-      DCHECK(expr->expression()->IsArrayLiteral());
-      ArrayLiteral* lit = expr->expression()->AsArrayLiteral();
-      expr->Rewrite(RewriteSpreads(lit));
-    }
-    non_patterns_to_rewrite->Rewind(begin);
-  }
-}
-
-
 void Parser::RewriteDestructuringAssignments() {
   const auto& assignments =
       function_state_->destructuring_assignments_to_rewrite();
@@ -3847,102 +3828,11 @@ void Parser::RewriteDestructuringAssignments() {
   }
 }
 
-Expression* Parser::RewriteSpreads(ArrayLiteral* lit) {
-  // Array literals containing spreads are rewritten using do expressions, e.g.
-  //    [1, 2, 3, ...x, 4, ...y, 5]
-  // is roughly rewritten as:
-  //    do {
-  //      $R = [1, 2, 3];
-  //      for ($i of x) %AppendElement($R, $i);
-  //      %AppendElement($R, 4);
-  //      for ($j of y) %AppendElement($R, $j);
-  //      %AppendElement($R, 5);
-  //      $R
-  //    }
-  // where $R, $i and $j are fresh temporary variables.
-  ZoneList<Expression*>::iterator s = lit->FirstSpread();
-  if (s == lit->EndValue()) return nullptr;  // no spread, no rewriting...
-  Variable* result = NewTemporary(ast_value_factory()->dot_result_string());
-  // NOTE: The value assigned to R is the whole original array literal,
-  // spreads included. This will be fixed before the rewritten AST is returned.
-  // $R = lit
-  Expression* init_result = factory()->NewAssignment(
-      Token::INIT, factory()->NewVariableProxy(result), lit, kNoSourcePosition);
-  Block* do_block = factory()->NewBlock(16, false);
-  do_block->statements()->Add(
-      factory()->NewExpressionStatement(init_result, kNoSourcePosition),
-      zone());
-  // Traverse the array literal starting from the first spread.
-  while (s != lit->EndValue()) {
-    Expression* value = *s++;
-    Spread* spread = value->AsSpread();
-    if (spread == nullptr) {
-      // If the element is not a spread, we're adding a single:
-      // %AppendElement($R, value)
-      // or, in case of a hole,
-      // ++($R.length)
-      if (!value->IsTheHoleLiteral()) {
-        ZoneList<Expression*>* append_element_args = NewExpressionList(2);
-        append_element_args->Add(factory()->NewVariableProxy(result), zone());
-        append_element_args->Add(value, zone());
-        do_block->statements()->Add(
-            factory()->NewExpressionStatement(
-                factory()->NewCallRuntime(Runtime::kAppendElement,
-                                          append_element_args,
-                                          kNoSourcePosition),
-                kNoSourcePosition),
-            zone());
-      } else {
-        Property* length_property = factory()->NewProperty(
-            factory()->NewVariableProxy(result),
-            factory()->NewStringLiteral(ast_value_factory()->length_string(),
-                                        kNoSourcePosition),
-            kNoSourcePosition);
-        CountOperation* count_op = factory()->NewCountOperation(
-            Token::INC, true /* prefix */, length_property, kNoSourcePosition);
-        do_block->statements()->Add(
-            factory()->NewExpressionStatement(count_op, kNoSourcePosition),
-            zone());
-      }
-    } else {
-      // If it's a spread, we're adding a for/of loop iterating through it.
-      Variable* each = NewTemporary(ast_value_factory()->dot_for_string());
-      Expression* subject = spread->expression();
-      // %AppendElement($R, each)
-      Statement* append_body;
-      {
-        ZoneList<Expression*>* append_element_args = NewExpressionList(2);
-        append_element_args->Add(factory()->NewVariableProxy(result), zone());
-        append_element_args->Add(factory()->NewVariableProxy(each), zone());
-        append_body = factory()->NewExpressionStatement(
-            factory()->NewCallRuntime(Runtime::kAppendElement,
-                                      append_element_args, kNoSourcePosition),
-            kNoSourcePosition);
-      }
-      // for (each of spread) %AppendElement($R, each)
-      ForOfStatement* loop =
-          factory()->NewForOfStatement(nullptr, kNoSourcePosition);
-      const bool finalize = false;
-      InitializeForOfStatement(loop, factory()->NewVariableProxy(each), subject,
-                               append_body, finalize, IteratorType::kNormal);
-      do_block->statements()->Add(loop, zone());
-    }
-  }
-  // Now, rewind the original array literal to truncate everything from the
-  // first spread (included) until the end. This fixes $R's initialization.
-  lit->RewindSpreads();
-  return factory()->NewDoExpression(do_block, result, lit->position());
-}
-
 void Parser::QueueDestructuringAssignmentForRewriting(
     RewritableExpression* expr) {
   function_state_->AddDestructuringAssignment(expr);
 }
 
-void Parser::QueueNonPatternForRewriting(RewritableExpression* expr, bool* ok) {
-  function_state_->AddNonPatternForRewriting(expr, ok);
-}
-
 void Parser::SetFunctionNameFromPropertyName(LiteralProperty* property,
                                              const AstRawString* name,
                                              const AstRawString* prefix) {

src/parsing/parser.h

@@ -553,13 +553,8 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
 
   Expression* RewriteSpreads(ArrayLiteral* lit);
 
-  // Rewrite expressions that are not used as patterns
-  V8_INLINE void RewriteNonPattern(bool* ok);
-
   V8_INLINE void QueueDestructuringAssignmentForRewriting(
       RewritableExpression* assignment);
-  V8_INLINE void QueueNonPatternForRewriting(RewritableExpression* expr,
-                                             bool* ok);
 
   friend class InitializerRewriter;
   void RewriteParameterInitializer(Expression* expr);
@@ -984,10 +979,6 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
     return function_state_->GetReportedErrorList();
   }
 
-  V8_INLINE ZoneList<RewritableExpression*>* GetNonPatternList() const {
-    return function_state_->non_patterns_to_rewrite();
-  }
-
   V8_INLINE void CountUsage(v8::Isolate::UseCounterFeature feature) {
     ++use_counts_[feature];
   }

src/parsing/preparser.h

@@ -999,7 +999,6 @@ class PreParser : public ParserBase<PreParser> {
   V8_INLINE void RewriteAsyncFunctionBody(
       PreParserStatementList body, PreParserStatement block,
       const PreParserExpression& return_value, bool* ok) {}
-  V8_INLINE void RewriteNonPattern(bool* ok) { ValidateExpression(ok); }
 
   void DeclareAndInitializeVariables(
       PreParserStatement block,
@@ -1186,8 +1185,6 @@ class PreParser : public ParserBase<PreParser> {
 
   V8_INLINE void QueueDestructuringAssignmentForRewriting(
       PreParserExpression assignment) {}
-  V8_INLINE void QueueNonPatternForRewriting(const PreParserExpression& expr,
-                                             bool* ok) {}
 
   // Helper functions for recursive descent.
   V8_INLINE bool IsEval(const PreParserIdentifier& identifier) const {
@@ -1665,10 +1662,6 @@ class PreParser : public ParserBase<PreParser> {
     return function_state_->GetReportedErrorList();
   }
 
-  V8_INLINE ZoneList<PreParserExpression>* GetNonPatternList() const {
-    return function_state_->non_patterns_to_rewrite();
-  }
-
   V8_INLINE void CountUsage(v8::Isolate::UseCounterFeature feature) {
     if (use_counts_ != nullptr) ++use_counts_[feature];
   }