[parser] Avoid redundant rewriting of arrow params

This fixed a TODO from cec289ea by marking RewritableExpressions as
rewritten in AddArrowFunctionFormalParameters when decomposing
Assignments into pattern/initializer.

Also added a set_rewritten() helper method to RewritableExpression
to simplify callsites.

Change-Id: Ifa36c9fb6c79193cbbcb168eedf7f782dc73a77b
Reviewed-on: https://chromium-review.googlesource.com/622353Reviewed-by: Marja Hölttä <marja@chromium.org>
Commit-Queue: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47524}

src/ast/ast.h

@@ -2244,13 +2244,16 @@ class RewritableExpression final : public Expression {
  public:
   Expression* expression() const { return expr_; }
   bool is_rewritten() const { return IsRewrittenField::decode(bit_field_); }
+  void set_rewritten() {
+    bit_field_ = IsRewrittenField::update(bit_field_, true);
+  }
 
   void Rewrite(Expression* new_expression) {
     DCHECK(!is_rewritten());
     DCHECK_NOT_NULL(new_expression);
     DCHECK(!new_expression->IsRewritableExpression());
     expr_ = new_expression;
-    bit_field_ = IsRewrittenField::update(bit_field_, true);
+    set_rewritten();
   }
 
  private:

src/parsing/parser.cc

@@ -2427,6 +2427,11 @@ void Parser::AddArrowFunctionFormalParameters(
 
   Expression* initializer = nullptr;
   if (expr->IsAssignment()) {
+    if (expr->IsRewritableExpression()) {
+      // This expression was parsed as a possible destructuring assignment.
+      // Mark it as already-rewritten to avoid an unnecessary visit later.
+      expr->AsRewritableExpression()->set_rewritten();
+    }
     Assignment* assignment = expr->AsAssignment();
     DCHECK(!assignment->IsCompoundAssignment());
     initializer = assignment->value();

src/parsing/pattern-rewriter.cc

@@ -299,14 +299,10 @@ void PatternRewriter::VisitRewritableExpression(RewritableExpression* node) {
     DCHECK_EQ(AstNode::kArrayLiteral, node->expression()->node_type());
     return Visit(node->expression());
   } else if (context() != ASSIGNMENT) {
-    // TODO(adamk): This early return should be a DCHECK, but in some cases we
-    // try to rewrite the same assignment twice: https://crbug.com/756332
-    // DCHECK(!node->is_rewritten());
-    if (node->is_rewritten()) return;
-
     // This is not a destructuring assignment. Mark the node as rewritten to
     // prevent redundant rewriting and visit the underlying expression.
-    node->Rewrite(node->expression());
+    DCHECK(!node->is_rewritten());
+    node->set_rewritten();
     return Visit(node->expression());
   }