From e60cafdb3bd681335fc1efa76d36e1b33e61ea95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Wed, 21 Dec 2016 16:20:19 +0100 Subject: [PATCH] deps: backport f795a79 from upstream V8 Original commit message: 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} Fixes: https://github.com/nodejs/node/issues/10347 PR-URL: https://github.com/nodejs/node/pull/10386 Reviewed-By: Ben Noordhuis Reviewed-By: James M Snell --- deps/v8/src/parsing/parser.h | 2 + deps/v8/src/parsing/pattern-rewriter.cc | 64 +++++++++++-------- .../mjsunit/regress/regress-crbug-620119.js | 8 +++ 3 files changed, 48 insertions(+), 26 deletions(-) create mode 100644 deps/v8/test/mjsunit/regress/regress-crbug-620119.js diff --git a/deps/v8/src/parsing/parser.h b/deps/v8/src/parsing/parser.h index c82682e323..5df11e77a0 100644 --- a/deps/v8/src/parsing/parser.h +++ b/deps/v8/src/parsing/parser.h @@ -880,6 +880,8 @@ class Parser : public ParserBase { PatternContext SetAssignmentContextIfNeeded(Expression* node); PatternContext SetInitializerContextIfNeeded(Expression* node); + void RewriteParameterScopes(Expression* expr); + Variable* CreateTempVar(Expression* value = nullptr); AstNodeFactory* factory() const { return parser_->factory(); } diff --git a/deps/v8/src/parsing/pattern-rewriter.cc b/deps/v8/src/parsing/pattern-rewriter.cc index 26d166d619..0c1153aea1 100644 --- a/deps/v8/src/parsing/pattern-rewriter.cc +++ b/deps/v8/src/parsing/pattern-rewriter.cc @@ -387,6 +387,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) { @@ -396,6 +427,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), @@ -668,32 +704,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); diff --git a/deps/v8/test/mjsunit/regress/regress-crbug-620119.js b/deps/v8/test/mjsunit/regress/regress-crbug-620119.js new file mode 100644 index 0000000000..cbe5a78713 --- /dev/null +++ b/deps/v8/test/mjsunit/regress/regress-crbug-620119.js @@ -0,0 +1,8 @@ +// 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, {}));