[parser] Use ScopedPtrList in ParseExpressionCoverGrammar

This additionally optimizes ExpressionListToExpression in the parser to
allocate Nary if possible.  This allows us to drop unnecessary intermediate
objects in the parser, and avoids all the work altogether in the preparser.

Change-Id: I4a7d0ec3a28624c94ed85959d291e54eb81ffce3
Reviewed-on: https://chromium-review.googlesource.com/c/1297952
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56943}

src/parsing/parser-base.h

@@ -1912,11 +1912,10 @@ ParserBase<Impl>::ParseExpressionCoverGrammar(bool accept_IN, bool* ok) {
   //   AssignmentExpression
   //   Expression ',' AssignmentExpression
 
-  ExpressionT result = impl()->NullExpression();
+  ScopedExpressionListT list(zone_, expression_buffer_);
+  ExpressionT right;
   while (true) {
-    int comma_pos = position();
     ExpressionClassifier binding_classifier(this);
-    ExpressionT right;
     if (Check(Token::ELLIPSIS)) {
       // 'x, y, ...z' in CoverParenthesizedExpressionAndArrowParameterList only
       // as the formal parameters of'(x, y, ...z) => foo', and is not itself a
@@ -1930,7 +1929,7 @@ ParserBase<Impl>::ParseExpressionCoverGrammar(bool accept_IN, bool* ok) {
       if (peek() == Token::ASSIGN) {
         ReportMessage(MessageTemplate::kRestDefaultInitializer);
         *ok = false;
-        return result;
+        return impl()->NullExpression();
       }
       right = factory()->NewSpread(pattern, ellipsis_pos, pattern_pos);
     } else {
@@ -1940,17 +1939,7 @@ ParserBase<Impl>::ParseExpressionCoverGrammar(bool accept_IN, bool* ok) {
     // an Expression can't be a binding pattern anyway.
     AccumulateNonBindingPatternErrors();
     if (!impl()->IsIdentifier(right)) classifier()->RecordNonSimpleParameter();
-    if (impl()->IsNull(result)) {
-      // First time through the loop.
-      result = right;
-    } else if (impl()->CollapseNaryExpression(&result, right, Token::COMMA,
-                                              comma_pos,
-                                              SourceRange::Empty())) {
-      // Do nothing, "result" is already updated.
-    } else {
-      result =
-          factory()->NewBinaryOperation(Token::COMMA, result, right, comma_pos);
-    }
+    list.Add(right);
 
     if (!Check(Token::COMMA)) break;
 
@@ -1972,7 +1961,12 @@ ParserBase<Impl>::ParseExpressionCoverGrammar(bool accept_IN, bool* ok) {
     }
   }
 
-  return result;
+  // Return the single element if the list is empty. We need to do this because
+  // callers of this function care about the type of the result if there was
+  // only a single assignment expression. The preparser would lose this
+  // information otherwise.
+  if (list.length() == 1) return right;
+  return impl()->ExpressionListToExpression(list);
 }
 
 template <typename Impl>

src/parsing/parser.cc

@@ -3568,11 +3568,17 @@ void Parser::SetAsmModule() {
 Expression* Parser::ExpressionListToExpression(
     const ScopedPtrList<Expression>& args) {
   Expression* expr = args.at(0);
-  for (int i = 1; i < args.length(); ++i) {
-    expr = factory()->NewBinaryOperation(Token::COMMA, expr, args.at(i),
+  if (args.length() == 1) return expr;
+  if (args.length() == 2) {
+    return factory()->NewBinaryOperation(Token::COMMA, expr, args.at(1),
                                          expr->position());
   }
-  return expr;
+  NaryOperation* result =
+      factory()->NewNaryOperation(Token::COMMA, expr, args.length() - 1);
+  for (int i = 1; i < args.length(); i++) {
+    result->AddSubsequent(args.at(i), expr->position());
+  }
+  return result;
 }
 
 // This method completes the desugaring of the body of async_function.

src/parsing/preparser.h

@@ -119,18 +119,6 @@ class PreParserExpression {
                                              Token::Value op,
                                              const PreParserExpression& right,
                                              Zone* zone) {
-    if (op == Token::COMMA) {
-      // Possibly an arrow function parameter list.
-      if (left.variables_ == nullptr) {
-        return PreParserExpression(TypeField::encode(kExpression),
-                                   right.variables_);
-      }
-      if (right.variables_ != nullptr) {
-        left.variables_->Append(std::move(*right.variables_));
-      }
-      return PreParserExpression(TypeField::encode(kExpression),
-                                 left.variables_);
-    }
     return PreParserExpression(TypeField::encode(kExpression));
   }