Reland of write scopes of non-simple default arguments (patchset #1 i…

…d:1 of https://codereview.chromium.org/2081323006/ )

Reason for revert:
Infra issue appears to be over

TBR=adamk@chromium.org

Original issue's description:
> Revert of Rewrite scopes of non-simple default arguments (patchset v8#5 id:80001 of https://codereview.chromium.org/2077283004/ )
>
> Reason for revert:
> Seems to close tree (but it could be an infra issue)
>
> Original issue's description:
> > Rewrite scopes of non-simple default arguments
> >
> > Default parameters have additional declaration block scopes inserted
> > around them when something in the function scope calls eval. This
> > patch sets the parent scope of the expressions introduced due to
> > those defaults to the new block scope.
> >
> > R=adamk
> > BUG=chromium:616386
> >
> > Committed: https://crrev.com/0e14baf712955a1993f742647bb2adc293702b80
> > Cr-Commit-Position: refs/heads/master@{#37198}
>
> TBR=adamk@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=chromium:616386
>
> Committed: https://crrev.com/dd50262933d2ac087da32be887a7c18385fd998e
> Cr-Commit-Position: refs/heads/master@{#37201}

TBR=adamk@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:616386

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

src/parsing/parameter-initializer-rewriter.cc

@@ -90,6 +90,10 @@ void Rewriter::VisitVariableProxy(VariableProxy* proxy) {
   if (proxy->is_resolved()) {
     Variable* var = proxy->var();
     if (var->mode() != TEMPORARY) return;
+    // For rewriting inside the same ClosureScope (e.g., putting default
+    // parameter values in their own inner scope in certain cases), refrain
+    // from invalidly moving temporaries to a block scope.
+    if (var->scope()->ClosureScope() == new_scope_->ClosureScope()) return;
     int index = old_scope_->RemoveTemporary(var);
     if (index >= 0) {
       temps_.push_back(std::make_pair(var, index));

src/parsing/parser.cc

@@ -4614,6 +4614,12 @@ Block* Parser::BuildParameterInitializationBlock(
       param_block = factory()->NewBlock(NULL, 8, true, RelocInfo::kNoPosition);
       param_block->set_scope(param_scope);
       descriptor.hoist_scope = scope_;
+      // Pass the appropriate scope in so that PatternRewriter can appropriately
+      // rewrite inner initializers of the pattern to param_scope
+      descriptor.scope = param_scope;
+      // Rewrite the outer initializer to point to param_scope
+      RewriteParameterInitializerScope(stack_limit(), initial_value, scope_,
+                                       param_scope);
     }
 
     {

src/parsing/pattern-rewriter.cc

@@ -677,9 +677,29 @@ 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_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());
   }

test/mjsunit/regress/regress-616386.js

@@ -0,0 +1,10 @@
+// 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, ((y = (function(a2) { bbbb = a2 }), bbbb = eval('1')) => {y(0); return bbbb})())
+assertEquals(0, (({y = (function(a2) { bbbb = a2 }), bbbb = eval('1')} = {}) => {y(0); return bbbb})())
+assertEquals(0, (function (y = (function(a2) { bbbb = a2 }), bbbb = eval('1')) {y(0); return bbbb})())
+assertEquals(0, (function ({y = (function(a2) { bbbb = a2 }), bbbb = eval('1')} = {}) {y(0); return bbbb})())