Handle switch effects

src/typing.cc

Index: src/typing.cc
diff --git a/src/typing.cc b/src/typing.cc
index dd4abcfd7d1718a0224086e35a649f91ad464e9a..804d6b79ac4ad06b6b8cb84c95d73d82d94495f3 100644
--- a/src/typing.cc
+++ b/src/typing.cc
@@ -80,6 +80,7 @@ void AstTyper::VisitStatements(ZoneList<Statement*>* stmts) {
   for (int i = 0; i < stmts->length(); ++i) {
     Statement* stmt = stmts->at(i);
     RECURSE(Visit(stmt));
+    if (stmt->IsJump()) break;
   }
 }
 
@@ -151,8 +152,13 @@ void AstTyper::VisitSwitchStatement(SwitchStatement* stmt) {
 
   ZoneList<CaseClause*>* clauses = stmt->cases();
   SwitchStatement::SwitchType switch_type = stmt->switch_type();
+  Effects local_effects(zone());
+  bool complex_effects = false;  // True for label effects or fall-through.
+
   for (int i = 0; i < clauses->length(); ++i) {
     CaseClause* clause = clauses->at(i);
+    Effects clause_effects = EnterEffects();
+
     if (!clause->is_default()) {
       Expression* label = clause->label();
       SwitchStatement::SwitchType label_switch_type =
@@ -165,12 +171,24 @@ void AstTyper::VisitSwitchStatement(SwitchStatement* stmt) {
         switch_type = SwitchStatement::GENERIC_SWITCH;
 
       RECURSE(Visit(label));
+      if (!clause_effects.IsEmpty()) complex_effects = true;
+    }
+
+    ZoneList<Statement*>* stmts = clause->statements();
+    RECURSE(VisitStatements(stmts));
+    ExitEffects();
+    if (stmts->is_empty() || stmts->last()->IsJump()) {
+      local_effects.Alt(clause_effects);

[titzer, 2013/07/15 09:18:18]

I'm not sure this is quite right. Did you define IsJump() to be true for both
break and continue?

[rossberg, 2013/07/15 09:44:59]

Yes, see ast.h. The idea is to give up on non-trivial fall-through, at least for
the time being. What precisely worries you?

[titzer, 2013/07/15 13:30:42]

So you actually have 5 different types of control transfer:

1. statement has no control flow => 
2. statement is break, inside a case => goes to end of switch
3. statement is break, inside a loop => goes to end of loop
4. statement is continue => goes to start of loop
5. statement is throw => goes out of method or to catch

Compositionally, a compound statement (if-then-else, loop, switch) could do any
set of these.

I'm not sure which of the behaviors you want to handle here, or what you want to
do for each.

[rossberg, 2013/07/15 14:21:56]

There's also 'return', and a 'continue' in a 'for' loop, which behaves
differently from other loops. But I don't think any of that matters.
Where the control goes does not matter, as long as it's out of the switch (which
is true for all of the above). All you want to ensure is that control from one
case cannot flow into the next case (a.k.a. fall-through). That is, I want a
well-behaved switch that is just a multiple branch, and no random mess. Because
only then all the branch effects are mutually exclusive.

+    } else {
+      complex_effects = true;
     }
-    RECURSE(VisitStatements(clause->statements()));
   }
 
-  // TODO(rossberg): handle switch effects
-  store_.Forget();
+  if (complex_effects) {
+    store_.Forget();  // Reached this in unknown state.
+  } else {
+    store_.Seq(local_effects);
+  }
 
   if (switch_type == SwitchStatement::UNKNOWN_SWITCH)
     switch_type = SwitchStatement::GENERIC_SWITCH;
@@ -379,7 +397,7 @@ void AstTyper::VisitAssignment(Assignment* expr) {
     NarrowType(expr, expr->binary_operation()->bounds());
   } else {
     // Collect type feedback.
-    if (expr->target()->AsProperty()) {
+    if (expr->target()->IsProperty()) {
       expr->RecordTypeFeedback(oracle(), zone());
     }
 
@@ -389,11 +407,9 @@ void AstTyper::VisitAssignment(Assignment* expr) {
     NarrowType(expr, expr->value()->bounds());
   }
 
-  if (expr->target()->AsVariableProxy()) {
-    Variable* var = expr->target()->AsVariableProxy()->var();
-    if (var->IsStackAllocated()) {
-      store_.Seq(variable_index(var), Effect(expr->bounds()));
-    }
+  VariableProxy* proxy = expr->target()->AsVariableProxy();
+  if (proxy != NULL && proxy->var()->IsStackAllocated()) {
+    store_.Seq(variable_index(proxy->var()), Effect(expr->bounds()));
   }
 }
 
@@ -528,11 +544,9 @@ void AstTyper::VisitCountOperation(CountOperation* expr) {
 
   NarrowType(expr, Bounds(Type::Smi(), Type::Number(), isolate_));
 
-  if (expr->expression()->AsVariableProxy()) {
-    Variable* var = expr->expression()->AsVariableProxy()->var();
-    if (var->IsStackAllocated()) {
-      store_.Seq(variable_index(var), Effect(expr->bounds()));
-    }
+  VariableProxy* proxy = expr->expression()->AsVariableProxy();
+  if (proxy != NULL && proxy->var()->IsStackAllocated()) {
+    store_.Seq(variable_index(proxy->var()), Effect(expr->bounds()));
   }
 }