From c947d448da027c66bc7ba1494cbcd7af90f8161f Mon Sep 17 00:00:00 2001 From: Rod Vagg Date: Tue, 18 Oct 2016 15:08:28 +1100 Subject: [PATCH] deps: cherry-pick 0e14baf712 from V8 upstream Original commit message: 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 Review-Url: https://codereview.chromium.org/2077283004 Cr-Commit-Position: refs/heads/master@{#37198} PR-URL: https://github.com/nodejs/node-private/pull/80 Reviewed-By: Ben Noordhuis --- .../parsing/parameter-initializer-rewriter.cc | 4 ++++ deps/v8/src/parsing/parser.cc | 6 +++++ deps/v8/src/parsing/pattern-rewriter.cc | 22 ++++++++++++++++++- .../v8/test/mjsunit/regress/regress-616386.js | 10 +++++++++ 4 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 deps/v8/test/mjsunit/regress/regress-616386.js diff --git a/deps/v8/src/parsing/parameter-initializer-rewriter.cc b/deps/v8/src/parsing/parameter-initializer-rewriter.cc index 3e3587b2bd..9e65b1b53f 100644 --- a/deps/v8/src/parsing/parameter-initializer-rewriter.cc +++ b/deps/v8/src/parsing/parameter-initializer-rewriter.cc @@ -63,6 +63,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; if (old_scope_->RemoveTemporary(var)) { var->set_scope(new_scope_); new_scope_->AddTemporary(var); diff --git a/deps/v8/src/parsing/parser.cc b/deps/v8/src/parsing/parser.cc index c9897cdd92..7fd6d5b302 100644 --- a/deps/v8/src/parsing/parser.cc +++ b/deps/v8/src/parsing/parser.cc @@ -4479,6 +4479,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); } { diff --git a/deps/v8/src/parsing/pattern-rewriter.cc b/deps/v8/src/parsing/pattern-rewriter.cc index e699255cdb..26d166d619 100644 --- a/deps/v8/src/parsing/pattern-rewriter.cc +++ b/deps/v8/src/parsing/pattern-rewriter.cc @@ -668,9 +668,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()); } diff --git a/deps/v8/test/mjsunit/regress/regress-616386.js b/deps/v8/test/mjsunit/regress/regress-616386.js new file mode 100644 index 0000000000..d462ab7509 --- /dev/null +++ b/deps/v8/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})())