Skip to content

Commit

Permalink
deps: backport f795a79 from upstream V8
Browse files Browse the repository at this point in the history
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: #10347
PR-URL: #10386
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
targos committed Dec 26, 2016
1 parent 272a971 commit e60cafd
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 26 deletions.
2 changes: 2 additions & 0 deletions deps/v8/src/parsing/parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -880,6 +880,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(); }
Expand Down
64 changes: 38 additions & 26 deletions deps/v8/src/parsing/pattern-rewriter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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),
Expand Down Expand Up @@ -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);
Expand Down
8 changes: 8 additions & 0 deletions deps/v8/test/mjsunit/regress/regress-crbug-620119.js
Original file line number Diff line number Diff line change
@@ -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, {}));

0 comments on commit e60cafd

Please sign in to comment.