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 ca36ac8..e9daf4a 100644 (file)
--- 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 72967d6..e338155 100644 (file)
--- 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<const AstRawString*>* labels, bool* ok) {
+Statement* Parser::ParseSwitchStatement(ZoneList<const AstRawString*>* 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<CaseClause*>* cases = new(zone()) ZoneList<CaseClause*>(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<CaseClause*>* cases =
+        new (zone()) ZoneList<CaseClause*>(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 602b527..dced211 100644 (file)
--- a/src/parser.h
+++ b/src/parser.h
@@ -1070,8 +1070,8 @@ class Parser : public ParserBase<ParserTraits> {
   Statement* ParseWithStatement(ZoneList<const AstRawString*>* labels,
                                 bool* ok);
   CaseClause* ParseCaseClause(bool* default_seen_ptr, bool* ok);
-  SwitchStatement* ParseSwitchStatement(ZoneList<const AstRawString*>* labels,
-                                        bool* ok);
+  Statement* ParseSwitchStatement(ZoneList<const AstRawString*>* labels,
+                                  bool* ok);
   DoWhileStatement* ParseDoWhileStatement(ZoneList<const AstRawString*>* labels,
                                           bool* ok);
   WhileStatement* ParseWhileStatement(ZoneList<const AstRawString*>* labels,

diff --git a/src/scopes.h b/src/scopes.h
index 4990fab..97606a7 100644 (file)
--- 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 (file)
index 0000000..4646992
--- /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 6a61fe5..4722e9e 100644 (file)
--- 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);
+})();