dart2js: Avoid empty else clauses in generators.

pkg/compiler/lib/src/js/rewrite_async.dart

-// Copyright (c) 2015, the Dart project authors.  Please see the AUTHORS file
+tra// Copyright (c) 2015, the Dart project authors.  Please see the AUTHORS file

[sigurdm, 2016/04/26 05:15:05]

This looks like a mistake

 // 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.
 
 library rewrite_async;
 
 import "dart:math" show max;
 import 'dart:collection';
 
 import 'package:js_runtime/shared/async_await_error_codes.dart' as error_codes;
 
@@ skipping 948 matching lines @@
 
   @override
   void visitDefault(js.Default node) => unreachable(node);
 
   @override
   void visitDo(js.Do node) {
     if (!shouldTransform(node)) {
       bool oldInsideUntranslatedBreakable = insideUntranslatedBreakable;
       insideUntranslatedBreakable = true;
       addStatement(js.js.statement('do {#} while (#)',
-          [translateInBlock(node.body), visitExpression(node.condition)]));
+          [translateToStatement(node.body), visitExpression(node.condition)]));
       insideUntranslatedBreakable = oldInsideUntranslatedBreakable;
       return;
     }
     int startLabel = newLabel("do body");
 
     int continueLabel = newLabel("do condition");
     continueLabels[node] = continueLabel;
 
     int afterLabel = newLabel("after do");
     breakLabels[node] = afterLabel;
@@ skipping 25 matching lines @@
   @override
   void visitFor(js.For node) {
     if (!shouldTransform(node)) {
       bool oldInsideUntranslated = insideUntranslatedBreakable;
       insideUntranslatedBreakable = true;
       // Note that node.init, node.condition, node.update all can be null, but
       // withExpressions handles that.
       withExpressions([node.init, node.condition, node.update],
           (List<js.Expression> transformed) {
         addStatement(new js.For(transformed[0], transformed[1], transformed[2],
-            translateInBlock(node.body)));
+            translateToStatement(node.body)));
       });
       insideUntranslatedBreakable = oldInsideUntranslated;
       return;
     }
 
     if (node.init != null) {
       visitExpressionIgnoreResult(node.init);
     }
     int startLabel = newLabel("for condition");
     // If there is no update, continuing the loop is the same as going to the
@@ skipping 30 matching lines @@
     // The dart output currently never uses for-in loops.
     throw "Javascript for-in not implemented yet in the await transformation";
   }
 
   @override
   void visitFunctionDeclaration(js.FunctionDeclaration node) {
     unsupported(node);
   }
 
   // Only used for code where `!shouldTransform(node)`.
-  js.Block translateInBlock(js.Statement node) {
+  js.Block translateToStatement(js.Statement node) {

[sigurdm, 2016/04/26 05:15:04]

I think the return type should be `js.Statement` now.

     assert(!shouldTransform(node));
     List<js.Statement> oldBuffer = currentStatementBuffer;
     currentStatementBuffer = new List();
     List<js.Statement> resultBuffer = currentStatementBuffer;
     visitStatement(node);
     currentStatementBuffer = oldBuffer;
+    if (resultBuffer.length == 1)
+      return resultBuffer.single;

[sigurdm, 2016/04/26 05:15:04]

Either put the return on the same  line as the if, or surround it with braces.

[sigurdm, 2016/04/26 05:15:05]

For the case where `node` is an empty block (can that even happen?), Should you
not also do
```
if (resultBuffer.isEmpty) {
  return new EmptyStatement;
}
```
?

     return new js.Block(resultBuffer);
   }
 
   @override
   void visitIf(js.If node) {
     if (!shouldTransform(node.then) && !shouldTransform(node.otherwise)) {
       withExpression(node.condition, (js.Expression condition) {
-        addStatement(new js.If(condition, translateInBlock(node.then),
-            translateInBlock(node.otherwise)));
+        js.Ast translatedThen = translateToStatement(node.then);
+        js.Ast translatedElse = translateToStatement(node.otherwise);
+        addStatement(new js.If(condition, translatedThen, translatedElse));
       }, store: false);
       return;
     }
     int thenLabel = newLabel("then");
     int joinLabel = newLabel("join");
     int elseLabel =
         (node.otherwise is js.EmptyStatement) ? joinLabel : newLabel("else");
 
     withExpression(node.condition, (js.Expression condition) {
       addExpressionStatement(new js.Assignment(
@@ skipping 37 matching lines @@
 
   @override
   visitInterpolatedStatement(js.InterpolatedStatement node) {
     return unsupported(node);
   }
 
   @override
   void visitLabeledStatement(js.LabeledStatement node) {
     if (!shouldTransform(node)) {
       addStatement(
-          new js.LabeledStatement(node.label, translateInBlock(node.body)));
+          new js.LabeledStatement(node.label, translateToStatement(node.body)));
       return;
     }
     // `continue label` is really continuing the nested loop.
     // This is set up in [PreTranslationAnalysis.visitContinue].
     // Here we only need a breakLabel:
     int breakLabel = newLabel("break ${node.label}");
     breakLabels[node] = breakLabel;
 
     jumpTargets.add(node);
     visitStatement(node.body);
@@ skipping 144 matching lines @@
   @override
   void visitSwitch(js.Switch node) {
     if (!node.cases.any(shouldTransform)) {
       // If only the key has an await, translation can be simplified.
       bool oldInsideUntranslated = insideUntranslatedBreakable;
       insideUntranslatedBreakable = true;
       withExpression(node.key, (js.Expression key) {
         List<js.SwitchClause> cases = node.cases.map((js.SwitchClause clause) {
           if (clause is js.Case) {
             return new js.Case(
-                clause.expression, translateInBlock(clause.body));
+                clause.expression, translateToStatement(clause.body));
           } else if (clause is js.Default) {
-            return new js.Default(translateInBlock(clause.body));
+            return new js.Default(translateToStatement(clause.body));
           }
         }).toList();
         addStatement(new js.Switch(key, cases));
       }, store: false);
       insideUntranslatedBreakable = oldInsideUntranslated;
       return;
     }
     int before = newLabel("switch");
     int after = newLabel("after switch");
     breakLabels[node] = after;
@@ skipping 101 matching lines @@
       if (finallyLabel != null) {
         result.add(finallyLabel);
       }
     }
     return result.reversed.toList();
   }
 
   /// See the comments of [rewriteFunction] for more explanation.
   void visitTry(js.Try node) {
     if (!shouldTransform(node)) {
-      js.Block body = translateInBlock(node.body);
+      js.Block body = translateToStatement(node.body);

[sigurdm, 2016/04/26 05:15:04]

This would also become a `js.Statement`

       js.Catch catchPart = (node.catchPart == null)
           ? null
           : new js.Catch(node.catchPart.declaration,
-              translateInBlock(node.catchPart.body));
+              translateToStatement(node.catchPart.body));
       js.Block finallyPart = (node.finallyPart == null)
           ? null
-          : translateInBlock(node.finallyPart);
+          : translateToStatement(node.finallyPart);
       addStatement(new js.Try(body, catchPart, finallyPart));
       return;
     }
 
     hasTryBlocks = true;
     int uncaughtLabel = newLabel("uncaught");
     int handlerLabel =
         (node.catchPart == null) ? uncaughtLabel : newLabel("catch");
 
     int finallyLabel = newLabel("finally");
@@ skipping 122 matching lines @@
     if (renaming == null) return node;
     return new js.VariableUse(renaming.b);
   }
 
   @override
   void visitWhile(js.While node) {
     if (!shouldTransform(node)) {
       bool oldInsideUntranslated = insideUntranslatedBreakable;
       insideUntranslatedBreakable = true;
       withExpression(node.condition, (js.Expression condition) {
-        addStatement(new js.While(condition, translateInBlock(node.body)));
+        addStatement(new js.While(condition, translateToStatement(node.body)));
       }, store: false);
       insideUntranslatedBreakable = oldInsideUntranslated;
       return;
     }
     int continueLabel = newLabel("while condition");
     continueLabels[node] = continueLabel;
     beginLabel(continueLabel);
 
     int afterLabel = newLabel("after while");
     breakLabels[node] = afterLabel;
@@ skipping 987 matching lines @@
     return condition || body;
   }
 
   @override
   bool visitDartYield(js.DartYield node) {
     hasYield = true;
     visit(node.expression);
     return true;
   }
 }