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

gen.dart

 // Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 /**
  * Top level generator object for writing code and keeping track of
  * dependencies.
  *
  * Should have two compilation models, but only one implemented so far.
  *
@@ skipping 847 matching lines @@
    * Escapes a string so it can be inserted into JS code as a double-quoted
    * JS string.
    */
   static String _escapeString(String text) {
     // TODO(jimhug): Use a regex for performance here.
     return text.replaceAll('\\', '\\\\').replaceAll('"', '\\"').replaceAll(
         '\n', '\\n').replaceAll('\r', '\\r');
   }
 
   /** 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);
       }
     } else {
       if (body != null) body.visit(this);
     }
+    return false;
   }
 
   _pushBlock([bool reentrant = false]) {
     _scope = new BlockScope(this, _scope, reentrant);
   }
 
   _popBlock() {
     _scope = _scope.parent;
   }
 
@@ skipping 29 matching lines @@
       var value = visitPostfixExpression(node, /*isVoid:*/true);
       value.checkFirstClass(node.span);
       return value;
     }
     // TODO(jimhug): Some level of warnings for non-void things here?
     return visitValue(node);
   }
 
   // ******************* 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) {
       isFinal = true;
     }
     writer.write('var ');
     var type = method.resolveType(node.type);
     for (int i=0; i < node.names.length; i++) {
       var thisType = type;
       if (i > 0) {
@@ skipping 12 matching lines @@
 
       var val = _scope.create(name, thisType, node.names[i]);
 
       if (value == null) {
         writer.write('${val.code}');
       } else {
         writer.write('${val.code} = ${value.code}');
       }
     }
     writer.writeln(';');
+    return false;
+
   }
 
-  void visitFunctionDefinition(FunctionDefinition node) {
+  bool visitFunctionDefinition(FunctionDefinition node) {
     var name = world.toJsIdentifier(node.name.name);
 
     var meth = _makeLambdaMethod(name, node);
 
     // 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 {
       if (method.isConstructor) {
         world.error('return of value not allowed from constructor', node.span);
       }
       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);
       // Ensure that we generate a toString() method for things that we throw
       value.invoke(parent, 'toString', node, Arguments.EMPTY);
       writer.writeln('\$throw(${value.code});');
     } else {
       var rethrow = _scope.getRethrow();
       if (rethrow == null) {
         world.error('rethrow outside of catch', node.span);
       } else {
         // Use a normal throw instead of $throw so we don't capture a new stack
         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) {
       var err = world.corelib.types['AssertError'];
       err.markUsed();
       parent.genMethod(err.getConstructor(''));
       parent.genMethod(err.members['toString']);
       var span = node.test.span;
 
       var line = span.file.getLine(span.start);
       var column = span.file.getColumn(line, span.start);
       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);
+    var exit1;
     writer.write('if (${test.code}) ');
-    node.trueBranch.visit(this);
+    exit1 = node.trueBranch.visit(this);

[jimhug, 2011/10/28 20:02:44]

I'd move the var decl to this line as I really like declaring and initializing
variables in one line.

     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);
     else writer.write(';');
     if (node.test != null) {
       var test = visitBool(node.test);
       writer.write(' ${test.code}; ');
     } else {
       writer.write('; ');
     }
 
     bool needsComma = false;
     for (var s in node.step) {
       if (needsComma) writer.write(', ');
       var sv = visitVoid(s);
       writer.write(sv.code);
       needsComma = true;
     }
     writer.write(') ');
     _pushBlock(/*reentrant:*/true);
     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;
     var list = node.list.visit(this);
     _pushBlock(/*reentrant:*/true);
     // TODO(jimhug): Check that itemType matches list members...
     var item = _scope.create(itemName, itemType, node.item.name);
     Value listVar = list;
     if (list.needsTemp) {
       listVar = _scope.create('\$list', list.type, null);
@@ skipping 23 matching lines @@
       var next = tmpi.invoke(parent, 'next', node.list, Arguments.EMPTY);
 
       writer.enterBlock(
         'for (var ${tmpi.code} = ${iterator.code}; ${hasNext.code}; ) {');
       writer.writeln('var ${item.code} = ${next.code};');
     }
 
     visitStatementsInBlock(node.body);
     writer.exitBlock('}');
     _popBlock();
+    return false;
   }
 
-  void visitTryStatement(TryStatement node) {
+  bool visitTryStatement(TryStatement node) {
     writer.enterBlock('try {');
     _pushBlock();
     visitStatementsInBlock(node.body);
     _popBlock();
 
     if (node.catches.length == 1) {
       // Handle a single catch. We can generate simple code here compared to the
       // multiple catch, such as no extra temp or if-else-if chain.
       var catch_ = node.catches[0];
       _pushBlock();
@@ skipping 73 matching lines @@
     }
 
     if (node.finallyBlock != null) {
       writer.nextBlock('} finally {');
       _pushBlock();
       visitStatementsInBlock(node.finallyBlock);
       _popBlock();
     }
 
     // Close the try-catch-finally
     writer.exitBlock('}');

[jimhug, 2011/10/28 20:02:44]

Add a TODO: 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}) {');
     for (var case_ in node.cases) {
       if (case_.label != null) {
         world.error('unimplemented: labeled case statement', case_.span);
       }
       _pushBlock();
       for (int i=0; i < case_.cases.length; i++) {
         var expr = case_.cases[i];
         if (expr == null) {
           // Default can only be the last case.
           if (i < case_.cases.length - 1) {
             world.error('default clause must be the last case', case_.span);
           }
           writer.writeln('default:');
         } else {
           var value = visitValue(expr);
           writer.writeln('case ${value.code}:');
         }
       }
       writer.enterBlock('');
-      for (var stmt in case_.statements) {
-        stmt.visit(this);
+      bool caseExits = false;
+      for (int i = 0; i < case_.statements.length; i++) {
+        var stmt = case_.statements[i];
+        caseExits = stmt.visit(this);
+        if (stmt != case_.statements[case_.statements.length - 1] && caseExits) {
+          world.warning('unreachable code', case_.statements[i + 1].span);

[jimhug, 2011/10/28 20:02:44]

Don't worry about it in this checkin - but for a future TODO - this will
generate a lot of errors if I have:
  case 0:
    return 1;
    a;
    b;
    c;

As written, this will give me three errors - and I'd prefer to just get the
error on a.

+        }
       }
-      if (case_ != node.cases[node.cases.length - 1]) {
+      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.

[jimhug, 2011/10/28 20:02:44]

Nice TODO.

+    return false;
   }
-
-  void visitBlockStatement(BlockStatement node) {
+  
+  bool visitBlockStatement(BlockStatement node) {
     _pushBlock();
     writer.enterBlock('{');
-    for (var stmt in node.body) {
-      stmt.visit(this);
+    var exits = false;
+    for (int i = 0; i < node.body.length; i++) {
+      var stmt = node.body[i];
+      exits = stmt.visit(this);
+      if (exits && stmt != node.body[node.body.length - 1]) {

[jimhug, 2011/10/28 20:02:44]

TODO: Again, not for this checkin - but the same statement as above applies here
for getting only one error.  Now that I think about it, this code should
probably be factored out to be shared there - as well as by
visitStatementsInBlock as well - where you are currently missing the error
message.

+        world.warning('unreachable code', node.body[i + 1].span);
+      }
     }
     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.
       world.warning('variable used as statement', node.span);
     }
     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) {
     if (method.isStatic) {
       world.warning('not allowed in static method', node.span);
     }
   }
 
   _makeSuperValue(Node node) {
     var parentType = method.declaringType.parent;
@@ skipping 470 matching lines @@
       }
       for (var arg in node.arguments) {
         if (!visitValue(arg.value).isConst) {
           world.error('const constructor expects const arguments', arg.span);
         }
       }
     }
     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 = [];
     for (var item in node.values) {
       var arg = visitValue(item);
       argValues.add(arg);
       if (node.isConst) {
         if (!arg.isConst) {
           world.error('const list can only contain const values', item.span);
           argsCode.add(arg.code);
         } else {
           argsCode.add(arg.canonicalCode);
         }
       } else {
         argsCode.add(arg.code);
       }
     }
 
     var code = '[${Strings.join(argsCode, ", ")}]';
     if (node.isConst) {
       final value = new EvaluatedValue(world.listType,
           'const $code', code, node.span);
       return parent.globalForConst(value, argValues);
     }
     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();
 
     var argValues = [];
     var argsCode = [];
     for (int i=0; i < node.items.length; i += 2) {
       // TODO(jimhug): Use node.type or other type inference here.
       // TODO(jimhug): Would be nice to allow arbitrary keys here.
       var key = visitTypedValue(node.items[i], world.stringType);
       final valueItem = node.items[i+1];
@@ skipping 264 matching lines @@
       result.add(new Value(world.varType, '\$$i', false, /*needsTemp:*/false));
     }
     for (int i = bareCount; i < length; i++) {
       var name = getName(i);
       if (name == null) name = '\$$i';
       result.add(new Value(world.varType, name, false, /*needsTemp:*/false));
     }
     return new Arguments(nodes, result);
   }
 }