Revert of Add a separate scope for switch (patchset #7 id:120001 of https://codereview.chromium.org/1293283002/ )

Reason for revert:
Breaks cctest/test-cpu-profiler/SourceLocation on nosnap

Original issue's description:
> 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
>
> Committed: https://crrev.com/9edbc1f21eb1050cabbe3b8bc9aebf89ada7ebd7
> Cr-Commit-Position: refs/heads/master@{#30314}

TBR=adamk@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:4377

Review URL: https://codereview.chromium.org/1309043004

Cr-Commit-Position: refs/heads/master@{#30316}

src/ast-value-factory.h

@@ -255,7 +255,6 @@ 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")                         \

src/parser.cc

@@ -2967,72 +2967,31 @@ CaseClause* Parser::ParseCaseClause(bool* default_seen_ptr, bool* ok) {
 }
 
 
-Statement* Parser::ParseSwitchStatement(ZoneList<const AstRawString*>* labels,
+SwitchStatement* Parser::ParseSwitchStatement(
-                                        bool* ok) {
+    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* }
-  //   }
-  // }
 
-  Block* switch_block =
+  SwitchStatement* statement =
-      factory()->NewBlock(NULL, 2, true, RelocInfo::kNoPosition);
+      factory()->NewSwitchStatement(labels, peek_position());
-  int switch_pos = peek_position();
+  Target target(&this->target_stack_, statement);
 
   Expect(Token::SWITCH, CHECK_OK);
   Expect(Token::LPAREN, CHECK_OK);
   Expression* tag = ParseExpression(true, CHECK_OK);
   Expect(Token::RPAREN, CHECK_OK);
 
-  Variable* tag_variable =
+  bool default_seen = false;
-      scope_->NewTemporary(ast_value_factory()->dot_switch_tag_string());
+  ZoneList<CaseClause*>* cases = new(zone()) ZoneList<CaseClause*>(4, zone());
-  Assignment* tag_assign = factory()->NewAssignment(
+  Expect(Token::LBRACE, CHECK_OK);
-      Token::ASSIGN, factory()->NewVariableProxy(tag_variable), tag,
+  while (peek() != Token::RBRACE) {
-      tag->position());
+    CaseClause* clause = ParseCaseClause(&default_seen, CHECK_OK);
-  Statement* tag_statement =
+    cases->Add(clause, zone());
-      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);
 
-  cases_scope->set_end_position(scanner()->location().end_pos);
+  if (statement) statement->Initialize(tag, cases);
-  cases_scope = cases_scope->FinalizeBlockScope();
+  return statement;
-  cases_block->set_scope(cases_scope);
-
-  switch_block->AddStatement(cases_block, zone());
-
-  return switch_block;
 }
 
 

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);
-  Statement* ParseSwitchStatement(ZoneList<const AstRawString*>* labels,
+  SwitchStatement* ParseSwitchStatement(ZoneList<const AstRawString*>* labels,
-                                  bool* ok);
+                                        bool* ok);
   DoWhileStatement* ParseDoWhileStatement(ZoneList<const AstRawString*>* labels,
                                           bool* ok);
   WhileStatement* ParseWhileStatement(ZoneList<const AstRawString*>* labels,

src/scopes.h

@@ -246,10 +246,6 @@ 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;

test/mjsunit/regress/regress-4377.js

@@ -1,45 +0,0 @@
-// 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);
-}

test/mjsunit/switch.js

@@ -460,58 +460,3 @@ 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);
-})();