From 0d15ec82e3e87437069e4511e2848892d4cf4501 Mon Sep 17 00:00:00 2001 From: Cristian Cavalli Date: Tue, 18 Oct 2016 16:21:16 -0700 Subject: [PATCH] deps: cherry pick 7166503 from upstream v8 Original Commit Message: Previously, any expressions inside destructuring patterns in a catch would be parsed in the surrounding scope, instead of in the catch's scope. This change fixes that by entering not only the catch scope, but also the block scope inside it. R=neis@chromium.org BUG=v8:5106, v8:5112 Review-Url: https://codereview.chromium.org/2110193002 Cr-Commit-Position: refs/heads/master@{#37415} PR-URL: https://github.com/nodejs/node/pull/9173 Reviewed-By: jasnell - James M Snell Reviewed-By: ofrobots - Ali Ijaz Sheikh --- deps/v8/src/parsing/parser.cc | 42 ++++++++++---------- deps/v8/test/mjsunit/regress/regress-5106.js | 29 ++++++++++++++ 2 files changed, 51 insertions(+), 20 deletions(-) create mode 100644 deps/v8/test/mjsunit/regress/regress-5106.js diff --git a/deps/v8/src/parsing/parser.cc b/deps/v8/src/parsing/parser.cc index df67d15c0ff8fb..48837f0ad62e27 100644 --- a/deps/v8/src/parsing/parser.cc +++ b/deps/v8/src/parsing/parser.cc @@ -2923,40 +2923,40 @@ TryStatement* Parser::ParseTryStatement(bool* ok) { catch_scope = NewScope(scope_, CATCH_SCOPE); catch_scope->set_start_position(scanner()->location().beg_pos); - ExpressionClassifier pattern_classifier(this); - Expression* pattern = ParsePrimaryExpression(&pattern_classifier, CHECK_OK); - ValidateBindingPattern(&pattern_classifier, CHECK_OK); - - const AstRawString* name = ast_value_factory()->dot_catch_string(); - bool is_simple = pattern->IsVariableProxy(); - if (is_simple) { - auto proxy = pattern->AsVariableProxy(); - scope_->RemoveUnresolved(proxy); - name = proxy->raw_name(); - } - - catch_variable = catch_scope->DeclareLocal(name, VAR, kCreatedInitialized, - Variable::NORMAL); - - Expect(Token::RPAREN, CHECK_OK); - { CollectExpressionsInTailPositionToListScope collect_expressions_in_tail_position_scope( function_state_, &expressions_in_tail_position_in_catch_block); BlockState block_state(&scope_, catch_scope); - // TODO(adamk): Make a version of ParseBlock that takes a scope and - // a block. catch_block = factory()->NewBlock(nullptr, 16, false, RelocInfo::kNoPosition); - Scope* block_scope = NewScope(scope_, BLOCK_SCOPE); + // Create a block scope to hold any lexical declarations created + // as part of destructuring the catch parameter. + Scope* block_scope = NewScope(scope_, BLOCK_SCOPE); block_scope->set_start_position(scanner()->location().beg_pos); { BlockState block_state(&scope_, block_scope); Target target(&this->target_stack_, catch_block); + ExpressionClassifier pattern_classifier(this); + Expression* pattern = + ParsePrimaryExpression(&pattern_classifier, CHECK_OK); + ValidateBindingPattern(&pattern_classifier, CHECK_OK); + + const AstRawString* name = ast_value_factory()->dot_catch_string(); + bool is_simple = pattern->IsVariableProxy(); + if (is_simple) { + auto proxy = pattern->AsVariableProxy(); + scope_->RemoveUnresolved(proxy); + name = proxy->raw_name(); + } + catch_variable = catch_scope->DeclareLocal( + name, VAR, kCreatedInitialized, Variable::NORMAL); + + Expect(Token::RPAREN, CHECK_OK); + if (!is_simple) { DeclarationDescriptor descriptor; descriptor.declaration_kind = DeclarationDescriptor::NORMAL; @@ -2978,6 +2978,8 @@ TryStatement* Parser::ParseTryStatement(bool* ok) { catch_block->statements()->Add(init_block, zone()); } + // TODO(adamk): This should call ParseBlock in order to properly + // add an additional block scope for the catch body. Expect(Token::LBRACE, CHECK_OK); while (peek() != Token::RBRACE) { Statement* stat = ParseStatementListItem(CHECK_OK); diff --git a/deps/v8/test/mjsunit/regress/regress-5106.js b/deps/v8/test/mjsunit/regress/regress-5106.js new file mode 100644 index 00000000000000..52d550a8784068 --- /dev/null +++ b/deps/v8/test/mjsunit/regress/regress-5106.js @@ -0,0 +1,29 @@ +// 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. + +function* g1() { + try { + throw {}; + } catch ({a = class extends (yield) {}}) { + } +} +g1().next(); // crashes without fix + +function* g2() { + let x = function(){}; + try { + throw {}; + } catch ({b = class extends x {}}) { + } +} +g2().next(); // crashes without fix + +function* g3() { + let x = 42; + try { + throw {}; + } catch ({c = (function() { return x })()}) { + } +} +g3().next(); // throws a ReferenceError without fix