Throw FallThroughError when we fall through a case in a switch statement. Fixes

gen.dart

Index: gen.dart
===================================================================
--- gen.dart	(revision 917)
+++ gen.dart	(working copy)
@@ -163,7 +163,7 @@
       Member standardConstructor = type.constructors[''];
       if (standardConstructor == null ||
           standardConstructor.generator == null) {
-        if (!type.isNativeType) {
+        if (!type.isNativeType) { 
           writer.writeln('function ${type.jsname}() {}');
         }
       } else {
@@ -865,7 +865,7 @@
   }
 
   /** Visits [body] without creating a new block for a [BlockStatement]. */
-  void visitStatementsInBlock(Statement body) {
+  bool visitStatementsInBlock(Statement body) {
     if (body is BlockStatement) {
       for (var stmt in body.body) {
         stmt.visit(this);
@@ -873,6 +873,7 @@
     } else {
       if (body != null) body.visit(this);
     }
+    return false;
   }
 
   _pushBlock([bool reentrant = false]) {
@@ -922,12 +923,13 @@
 
   // ******************* Statements *******************
 
-  void visitDietStatement(DietStatement node) {
+  bool visitDietStatement(DietStatement node) {
     var parser = new Parser(node.span.file, /*diet:*/false, node.span.start);
     visitStatementsInBlock(parser.block());
+    return false;
   }
 
-  void visitVariableDefinition(VariableDefinition node) {
+  bool visitVariableDefinition(VariableDefinition node) {
     var isFinal = false;
     // TODO(jimhug): Clean this up and share modifier parsing somewhere.
     if (node.modifiers != null && node.modifiers[0].kind == TokenKind.FINAL) {
@@ -960,9 +962,11 @@
       }
     }
     writer.writeln(';');
+    return false;
+
   }
 
-  void visitFunctionDefinition(FunctionDefinition node) {
+  bool visitFunctionDefinition(FunctionDefinition node) {
     var name = world.toJsIdentifier(node.name.name);
 
     var meth = _makeLambdaMethod(name, node);
@@ -970,9 +974,14 @@
     // TODO(jimhug): Pass js name into writeDefinition?
     var funcValue = _scope.create(name, meth.functionType, method.definition);
     meth.generator.writeDefinition(writer, null);
+    return false;
   }
 
-  void visitReturnStatement(ReturnStatement node) {
+  /**
+   * Returns true indicating that normal control-flow is interrupted by 
+   * this statement. (This could be a return, break, throw, or continue.)
+   */
+  bool visitReturnStatement(ReturnStatement node) {
     if (node.value == null) {
       writer.writeln('return;');
     } else {
@@ -981,9 +990,10 @@
       }
       writer.writeln('return ${visitValue(node.value).code};');
     }
+    return true;
   }
 
-  void visitThrowStatement(ThrowStatement node) {
+  bool visitThrowStatement(ThrowStatement node) {
     // Dart allows throwing anything, just like JS
     if (node.value != null) {
       var value = visitValue(node.value);
@@ -999,9 +1009,10 @@
         writer.writeln('throw ${rethrow.code};');
       }
     }
+    return true;
   }
 
-  void visitAssertStatement(AssertStatement node) {
+  bool visitAssertStatement(AssertStatement node) {
     // be sure to walk test for static checking even is asserts disabled
     var test = visitValue(node.test); // TODO(jimhug): check bool or callable.
     if (options.enableAsserts) {
@@ -1016,53 +1027,61 @@
       writer.writeln('\$assert(${test.code}, "${_escapeString(span.text)}",' +
         ' "${span.file.filename}", ${line + 1}, ${column + 1});');
     }
+    return false;
   }
 
-  void visitBreakStatement(BreakStatement node) {
+  bool visitBreakStatement(BreakStatement node) {
     // TODO(jimhug): Lots of flow error checking here and below.
     if (node.label == null) {
       writer.writeln('break;');
     } else {
       writer.writeln('break ${node.label.name};');
     }
+    return true;
   }
 
-  void visitContinueStatement(ContinueStatement node) {
+  bool visitContinueStatement(ContinueStatement node) {
     if (node.label == null) {
       writer.writeln('continue;');
     } else {
       writer.writeln('continue ${node.label.name};');
     }
+    return true;
   }
 
-  void visitIfStatement(IfStatement node) {
+  bool visitIfStatement(IfStatement node) {
     var test = visitBool(node.test);
     writer.write('if (${test.code}) ');
-    node.trueBranch.visit(this);
+    var exit1 = node.trueBranch.visit(this);
     if (node.falseBranch != null) {
       writer.write('else ');
-      node.falseBranch.visit(this);
+      if (node.falseBranch.visit(this) && exit1) {
+        return true;
+      }
     }
+    return false;
   }
 
-  void visitWhileStatement(WhileStatement node) {
+  bool visitWhileStatement(WhileStatement node) {
     var test = visitBool(node.test);
     writer.write('while (${test.code}) ');
     _pushBlock(/*reentrant:*/true);
     node.body.visit(this);
     _popBlock();
+    return false;
   }
 
-  void visitDoStatement(DoStatement node) {
+  bool visitDoStatement(DoStatement node) {
     writer.write('do ');
     _pushBlock(/*reentrant:*/true);
     node.body.visit(this);
     _popBlock();
     var test = visitBool(node.test);
     writer.writeln('while (${test.code})');
+    return false;
   }
 
-  void visitForStatement(ForStatement node) {
+  bool visitForStatement(ForStatement node) {
     _pushBlock();
     writer.write('for (');
     if (node.init != null) node.init.visit(this);
@@ -1086,9 +1105,10 @@
     node.body.visit(this);
     _popBlock();
     _popBlock();
+    return false;
   }
 
-  void visitForInStatement(ForInStatement node) {
+  bool visitForInStatement(ForInStatement node) {
     // TODO(jimhug): visitValue and other cleanups here.
     var itemType = method.resolveType(node.item.type);
     var itemName = node.item.name.name;
@@ -1132,9 +1152,10 @@
     visitStatementsInBlock(node.body);
     writer.exitBlock('}');
     _popBlock();
+    return false;
   }
 
-  void visitTryStatement(TryStatement node) {
+  bool visitTryStatement(TryStatement node) {
     writer.enterBlock('try {');
     _pushBlock();
     visitStatementsInBlock(node.body);
@@ -1229,9 +1250,14 @@
 
     // Close the try-catch-finally
     writer.exitBlock('}');
+    // TODO(efortuna): This could be more precise by combining all the different
+    // paths here.  -i.e. if there is a finally block with a return at the end 
+    // then this can return true, similarly if all blocks have a return at the 
+    // end then the same holds.
+    return false;
   }
 
-  void visitSwitchStatement(SwitchStatement node) {
+  bool visitSwitchStatement(SwitchStatement node) {
     // TODO(jimhug): Lots of negative tests to check for.
     var test = visitValue(node.test);
     writer.enterBlock('switch (${test.code}) {');
@@ -1254,34 +1280,56 @@
         }
       }
       writer.enterBlock('');
-      for (var stmt in case_.statements) {
-        stmt.visit(this);
-      }
-      if (case_ != node.cases[node.cases.length - 1]) {
+      bool caseExits = _visitAllStatements(case_.statements, false);
+
+      if (case_ != node.cases[node.cases.length - 1] && !caseExits) {
         var span = case_.statements[case_.statements.length - 1].span;
+        //TODO(efortuna): This error message isn't 100% correct because 
+        // constructors are not being built correctly. Check back for correct
+        // output when constructors are fixed. 
+        writer.writeln('\$throw(new FallThroughError("${span.file.filename}",' +
+            ' ${span.file.getLine(span.start)}))');
       }
       writer.exitBlock('');
       _popBlock();
     }
     writer.exitBlock('}');
+    // TODO(efortuna): When we are passing more information back about
+    // control flow by returning something other than bool, return true for the
+    // cases where every branch of the switch statement ends with a return 
+    // statement.
+    return false;
   }
+  
+  bool _visitAllStatements(statementList, exits) {
+    for (int i = 0; i < statementList.length; i++) {
+      var stmt = statementList[i];
+      exits = stmt.visit(this);
+      //TODO(efortuna): fix this so you only get one error if you have "return;
+      //a; b; c;"
+      if (stmt != statementList[statementList.length - 1] && exits) {
+        world.warning('unreachable code', statementList[i + 1].span);
+      }
+    }
+    return exits;
+  }
 
-  void visitBlockStatement(BlockStatement node) {
+  bool visitBlockStatement(BlockStatement node) {
     _pushBlock();
     writer.enterBlock('{');
-    for (var stmt in node.body) {
-      stmt.visit(this);
-    }
+    var exits = _visitAllStatements(node.body, false);
     writer.exitBlock('}');
     _popBlock();
+    return exits;
   }
 
-  void visitLabeledStatement(LabeledStatement node) {
+  bool visitLabeledStatement(LabeledStatement node) {
     writer.writeln('${node.name.name}:');
     node.body.visit(this);
+    return false;
   }
 
-  void visitExpressionStatement(ExpressionStatement node) {
+  bool visitExpressionStatement(ExpressionStatement node) {
     if (node.body is VarExpression || node.body is ThisExpression) {
       // TODO(jmesserly): this is a "warning" but not a "type warning",
       // Is that okay? We have a similar issue around unreachable code warnings.
@@ -1289,10 +1337,12 @@
     }
     var value = visitVoid(node.body);
     writer.writeln('${value.code};');
+    return false;
   }
 
-  void visitEmptyStatement(EmptyStatement node) {
+  bool visitEmptyStatement(EmptyStatement node) {
     writer.writeln(';');
+    return false;
   }
 
   _checkNonStatic(Node node) {
@@ -1783,7 +1833,7 @@
     return m.invoke(parent, node, null, _makeArgs(node.arguments));
   }
 
-  void visitListExpression(ListExpression node) {
+  visitListExpression(ListExpression node) {
     // TODO(jimhug): Use node.type or other type inference here.
     var argsCode = [];
     var argValues = [];
@@ -1811,7 +1861,7 @@
     return new Value(world.listType, code);
   }
 
-  void visitMapExpression(MapExpression node) {
+  visitMapExpression(MapExpression node) {
     // Ensure that Map factory is intialized - pseudo-call needed members.
     var mapImplType = parent.useMapFactory();