From: littledan Date: Fri, 21 Aug 2015 23:54:21 +0000 (-0700) Subject: Add a separate scope for switch X-Git-Tag: upstream/4.7.83~726 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=9edbc1f21eb1050cabbe3b8bc9aebf89ada7ebd7;p=platform%2Fupstream%2Fv8.git Add a separate scope for switch The ES2015 specification for switch statements 13.12.11 specifies that they get their own lexical scope. This patch introduces such a scope through a complex desugaring in terms of blocks, done so that Crankshaft does not have to be updated to support multiple constructs providing scopes. BUG=v8:4377 LOG=Y R=adamk Review URL: https://codereview.chromium.org/1293283002 Cr-Commit-Position: refs/heads/master@{#30314} --- diff --git a/src/ast-value-factory.h b/src/ast-value-factory.h index ca36ac8ea..e9daf4a2f 100644 --- a/src/ast-value-factory.h +++ b/src/ast-value-factory.h @@ -255,6 +255,7 @@ class AstValue : public ZoneObject { F(dot_iterator, ".iterator") \ F(dot_module, ".module") \ F(dot_result, ".result") \ + F(dot_switch_tag, ".switch_tag") \ F(empty, "") \ F(eval, "eval") \ F(get_template_callsite, "$getTemplateCallSite") \ diff --git a/src/parser.cc b/src/parser.cc index 72967d6df..e33815528 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -2967,31 +2967,72 @@ CaseClause* Parser::ParseCaseClause(bool* default_seen_ptr, bool* ok) { } -SwitchStatement* Parser::ParseSwitchStatement( - ZoneList* labels, bool* ok) { +Statement* Parser::ParseSwitchStatement(ZoneList* labels, + bool* ok) { // SwitchStatement :: // 'switch' '(' Expression ')' '{' CaseClause* '}' + // In order to get the CaseClauses to execute in their own lexical scope, + // but without requiring downstream code to have special scope handling + // code for switch statements, desugar into blocks as follows: + // { // To group the statements--harmless to evaluate Expression in scope + // .tag_variable = Expression; + // { // To give CaseClauses a scope + // switch (.tag_variable) { CaseClause* } + // } + // } - SwitchStatement* statement = - factory()->NewSwitchStatement(labels, peek_position()); - Target target(&this->target_stack_, statement); + Block* switch_block = + factory()->NewBlock(NULL, 2, true, RelocInfo::kNoPosition); + int switch_pos = peek_position(); Expect(Token::SWITCH, CHECK_OK); Expect(Token::LPAREN, CHECK_OK); Expression* tag = ParseExpression(true, CHECK_OK); Expect(Token::RPAREN, CHECK_OK); - bool default_seen = false; - ZoneList* cases = new(zone()) ZoneList(4, zone()); - Expect(Token::LBRACE, CHECK_OK); - while (peek() != Token::RBRACE) { - CaseClause* clause = ParseCaseClause(&default_seen, CHECK_OK); - cases->Add(clause, zone()); + Variable* tag_variable = + scope_->NewTemporary(ast_value_factory()->dot_switch_tag_string()); + Assignment* tag_assign = factory()->NewAssignment( + Token::ASSIGN, factory()->NewVariableProxy(tag_variable), tag, + tag->position()); + Statement* tag_statement = + factory()->NewExpressionStatement(tag_assign, RelocInfo::kNoPosition); + switch_block->AddStatement(tag_statement, zone()); + + Block* cases_block = + factory()->NewBlock(NULL, 1, true, RelocInfo::kNoPosition); + Scope* cases_scope = NewScope(scope_, BLOCK_SCOPE); + + SwitchStatement* switch_statement = + factory()->NewSwitchStatement(labels, switch_pos); + + cases_scope->set_start_position(scanner()->location().beg_pos); + { + BlockState cases_block_state(&scope_, cases_scope); + Target target(&this->target_stack_, switch_statement); + + Expression* tag_read = factory()->NewVariableProxy(tag_variable); + + bool default_seen = false; + ZoneList* cases = + new (zone()) ZoneList(4, zone()); + Expect(Token::LBRACE, CHECK_OK); + while (peek() != Token::RBRACE) { + CaseClause* clause = ParseCaseClause(&default_seen, CHECK_OK); + cases->Add(clause, zone()); + } + switch_statement->Initialize(tag_read, cases); + cases_block->AddStatement(switch_statement, zone()); } Expect(Token::RBRACE, CHECK_OK); - if (statement) statement->Initialize(tag, cases); - return statement; + cases_scope->set_end_position(scanner()->location().end_pos); + cases_scope = cases_scope->FinalizeBlockScope(); + cases_block->set_scope(cases_scope); + + switch_block->AddStatement(cases_block, zone()); + + return switch_block; } diff --git a/src/parser.h b/src/parser.h index 602b5278e..dced2117b 100644 --- a/src/parser.h +++ b/src/parser.h @@ -1070,8 +1070,8 @@ class Parser : public ParserBase { Statement* ParseWithStatement(ZoneList* labels, bool* ok); CaseClause* ParseCaseClause(bool* default_seen_ptr, bool* ok); - SwitchStatement* ParseSwitchStatement(ZoneList* labels, - bool* ok); + Statement* ParseSwitchStatement(ZoneList* labels, + bool* ok); DoWhileStatement* ParseDoWhileStatement(ZoneList* labels, bool* ok); WhileStatement* ParseWhileStatement(ZoneList* labels, diff --git a/src/scopes.h b/src/scopes.h index 4990fab99..97606a745 100644 --- a/src/scopes.h +++ b/src/scopes.h @@ -246,6 +246,10 @@ class Scope: public ZoneObject { // for (let x ...) stmt // start position: start position of '(' // end position: end position of last token of 'stmt' + // * For the scope of a switch statement + // switch (tag) { cases } + // start position: start position of '{' + // end position: end position of '}' int start_position() const { return start_position_; } void set_start_position(int statement_pos) { start_position_ = statement_pos; diff --git a/test/mjsunit/regress/regress-4377.js b/test/mjsunit/regress/regress-4377.js new file mode 100644 index 000000000..4646992d4 --- /dev/null +++ b/test/mjsunit/regress/regress-4377.js @@ -0,0 +1,45 @@ +// Copyright 2015 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. + +// See: http://code.google.com/p/v8/issues/detail?id=4377 + +// Switch statements should introduce their own lexical scope + +'use strict'; + +switch (1) { case 1: let x = 2; } + +assertThrows(() => x, ReferenceError); + +{ + let result; + let x = 1; + switch (x) { + case 1: + let x = 2; + result = x; + break; + default: + result = 0; + break; + } + assertEquals(1, x); + assertEquals(2, result); +} + +{ + let result; + let x = 1; + switch (eval('x')) { + case 1: + let x = 2; + result = x; + break; + default: + result = 0; + break; + } + assertEquals(1, x); + assertEquals(2, result); +} diff --git a/test/mjsunit/switch.js b/test/mjsunit/switch.js index 6a61fe594..4722e9e5d 100644 --- a/test/mjsunit/switch.js +++ b/test/mjsunit/switch.js @@ -460,3 +460,58 @@ function test_switches(opt) { test_switches(false); test_switches(true); + + +// Test labeled and anonymous breaks in switch statements +(function test_switch_break() { + A: for (var i = 1; i < 10; i++) { + switch (i) { + case 1: + break A; + } + } + assertEquals(1, i); + + for (var i = 1; i < 10; i++) { + B: switch (i) { + case 1: + break B; + } + } + assertEquals(10, i); + + for (var i = 1; i < 10; i++) { + switch (i) { + case 1: + break; + } + } + assertEquals(10, i); + + switch (1) { + case 1: + C: for (var i = 1; i < 10; i++) { + break C; + } + i = 2; + } + assertEquals(2, i); + + switch (1) { + case 1: + for (var i = 1; i < 10; i++) { + break; + } + i = 2; + } + assertEquals(2, i); + + D: switch (1) { + case 1: + for (var i = 1; i < 10; i++) { + break D; + } + i = 2; + } + assertEquals(1, i); +})();