Skip to content

Commit

Permalink
deps: cherry-pick 0e14baf712 from V8 upstream
Browse files Browse the repository at this point in the history
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: nodejs-private/node-private#80
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
rvagg committed Oct 18, 2016
1 parent cae9eb3 commit c947d44
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 1 deletion.
4 changes: 4 additions & 0 deletions deps/v8/src/parsing/parameter-initializer-rewriter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 6 additions & 0 deletions deps/v8/src/parsing/parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

{
Expand Down
22 changes: 21 additions & 1 deletion deps/v8/src/parsing/pattern-rewriter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
10 changes: 10 additions & 0 deletions deps/v8/test/mjsunit/regress/regress-616386.js
Original file line number Diff line number Diff line change
@@ -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})())

0 comments on commit c947d44

Please sign in to comment.