Rewrite scopes in computed properties in destructured parameters

While we properly handled scopes of initializers in destructured parameters,
we never did the right thing for computed properties. This patch fixes that
by factoring out PatternRewriter's scope rewriting logic and calls it
for the computed property case.

BUG=chromium:620119

Review-Url: https://codereview.chromium.org/2084103002
Cr-Commit-Position: refs/heads/master@{#37228}

src/parsing/parser.h

@@ -899,6 +899,8 @@ class Parser : public ParserBase<ParserTraits> {
     PatternContext SetAssignmentContextIfNeeded(Expression* node);
     PatternContext SetInitializerContextIfNeeded(Expression* node);
 
+    void RewriteParameterScopes(Expression* expr);
+
     Variable* CreateTempVar(Expression* value = nullptr);
 
     AstNodeFactory* factory() const { return parser_->factory(); }

src/parsing/pattern-rewriter.cc

@@ -381,6 +381,37 @@ void Parser::PatternRewriter::VisitRewritableExpression(
   return set_context(old_context);
 }
 
+// Two cases for scope rewriting the scope of default parameters:
+// - Eagerly parsed arrow functions are initially parsed as having
+//   expressions in the enclosing scope, but when the arrow is encountered,
+//   need to be in the scope of the function.
+// - When an extra declaration scope needs to be inserted to account for
+//   a sloppy eval in a default parameter or function body, the expressions
+//   needs to be in that new inner scope which was added after initial
+//   parsing.
+// Each of these cases can be handled by rewriting the contents of the
+// expression to the current scope. The source scope is typically the outer
+// scope when one case occurs; when both cases occur, both scopes need to
+// be included as the outer scope. (Both rewritings still need to be done
+// to account for lazily parsed arrow functions which hit the second case.)
+// TODO(littledan): Remove the outer_scope parameter of
+//                  RewriteParameterInitializerScope
+void Parser::PatternRewriter::RewriteParameterScopes(Expression* expr) {
+  if (!IsBindingContext()) return;
+  if (descriptor_->declaration_kind != DeclarationDescriptor::PARAMETER) return;
+  if (!scope()->is_arrow_scope() && !scope()->is_block_scope()) return;
+
+  // Either this scope is an arrow scope or a declaration block scope.
+  DCHECK(scope()->is_declaration_scope());
+
+  if (scope()->outer_scope()->is_arrow_scope() && scope()->is_block_scope()) {
+    RewriteParameterInitializerScope(parser_->stack_limit(), expr,
+                                     scope()->outer_scope()->outer_scope(),
+                                     scope());
+  }
+  RewriteParameterInitializerScope(parser_->stack_limit(), expr,
+                                   scope()->outer_scope(), scope());
+}
 
 void Parser::PatternRewriter::VisitObjectLiteral(ObjectLiteral* pattern,
                                                  Variable** temp_var) {
@@ -390,6 +421,11 @@ void Parser::PatternRewriter::VisitObjectLiteral(ObjectLiteral* pattern,
 
   for (ObjectLiteralProperty* property : *pattern->properties()) {
     PatternContext context = SetInitializerContextIfNeeded(property->value());
+
+    // Computed property names contain expressions which might require
+    // scope rewriting.
+    if (!property->key()->IsLiteral()) RewriteParameterScopes(property->key());
+
     RecurseIntoSubpattern(
         property->value(),
         factory()->NewProperty(factory()->NewVariableProxy(temp),
@@ -677,32 +713,8 @@ void Parser::PatternRewriter::VisitAssignment(Assignment* node) {
                                       RelocInfo::kNoPosition);
   }
 
-  // Two cases for scope rewriting the scope of default parameters:
-  // - Eagerly parsed arrow functions are initially parsed as having
-  //   initializers in the enclosing scope, but when the arrow is encountered,
-  //   need to be in the scope of the function.
-  // - When an extra declaration scope needs to be inserted to account for
-  //   a sloppy eval in a default parameter or function body, the initializer
-  //   needs to be in that new inner scope which was added after initial
-  //   parsing.
-  // Each of these cases can be handled by rewriting the contents of the
-  // initializer to the current scope. The source scope is typically the outer
-  // scope when one case occurs; when both cases occur, both scopes need to
-  // be included as the outer scope. (Both rewritings still need to be done
-  // to account for lazily parsed arrow functions which hit the second case.)
-  // TODO(littledan): Remove the outer_scope parameter of
-  //                  RewriteParameterInitializerScope
-  if (IsBindingContext() &&
-      descriptor_->declaration_kind == DeclarationDescriptor::PARAMETER &&
-      (scope()->is_arrow_scope() || scope()->is_block_scope())) {
-    if (scope()->outer_scope()->is_arrow_scope() && scope()->is_block_scope()) {
-      RewriteParameterInitializerScope(parser_->stack_limit(), initializer,
-                                       scope()->outer_scope()->outer_scope(),
-                                       scope());
-    }
-    RewriteParameterInitializerScope(parser_->stack_limit(), initializer,
-                                     scope()->outer_scope(), scope());
-  }
+  // Initializer may have been parsed in the wrong scope.
+  RewriteParameterScopes(initializer);
 
   PatternContext old_context = SetAssignmentContextIfNeeded(initializer);
   RecurseIntoSubpattern(node->target(), value);

test/mjsunit/regress/regress-crbug-620119.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.
+
+// Flags: --no-lazy
+
+assertEquals(0, ((x, {[(x = function() { y = 0 }, "foo")]: y = eval(1)}) => { x(); return y })(42, {}));
+assertEquals(0, (function (x, {[(x = function() { y = 0 }, "foo")]: y = eval(1)}) { x(); return y })(42, {}));