[pattern-rewriter] Handle already-rewritten RewritableExpressions as before

Before 983eec89, RewritableExpressions
which had been queued for destructuring assignment rewriting but which
turned out to be part of a binding pattern in arrow function parameters
would be silently ignored by the PatternRewriter. After that CL, they
failed with a DCHECK.

This patch reverts to the previous behavior, with a TODO to handle this
in a better way by dequeuing RewritableExpressions that turned out
to be part of an inner arrow function.

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

src/parsing/pattern-rewriter.cc

@@ -291,9 +291,13 @@ 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.
-    DCHECK(!node->is_rewritten());
     node->Rewrite(node->expression());
     return Visit(node->expression());
   }

test/mjsunit/regress/regress-crbug-756332.js

+// Copyright 2017 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.
+
+{
+  let z = ({x: {y} = {y: 42}} = {}) => y;
+  assertEquals(42, z());
+}
+
+{
+  let z = ({x: [y] = [42]} = {}) => y;
+  assertEquals(42, z());
+}