Fix error handling in dart2js async-await

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

 // Copyright (c) 2015, 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.
 
 library rewrite_async;
 
-// TODO(sigurdm): Throws in catch-handlers are handled wrong.
 // TODO(sigurdm): Avoid using variables in templates. It could blow up memory
 // use.
 
 import "dart:math" show max;
 import 'dart:collection';
 
+import 'package:_internal/compiler/js_lib/shared/async_await_error_codes.dart'
+    as error_codes;
+
 import "js.dart" as js;
 
 import '../util/util.dart';
 import '../dart2jslib.dart' show DiagnosticListener;
 
 import "../helpers/helpers.dart";
 
 /// Rewrites a [js.Fun] with async/sync*/async* functions and await and yield
 /// (with dart-like semantics) to an equivalent function without these.
 /// await-for is not handled and must be rewritten before. (Currently handled
 /// in ssa/builder.dart).
 ///
 /// When generating the input to this, special care must be taken that
 /// parameters to sync* functions that are mutated in the body must be boxed.
 /// (Currently handled in closure.dart).
 ///
 /// Look at [visitFun], [visitDartYield] and [visitAwait] for more explanation.
 class AsyncRewriter extends js.NodeVisitor {
 
   // Local variables are hoisted to the top of the function, so they are
   // collected here.
   List<js.VariableDeclaration> localVariables =
       new List<js.VariableDeclaration>();
 
   Map<js.Node, int> continueLabels = new Map<js.Node, int>();
   Map<js.Node, int> breakLabels = new Map<js.Node, int>();
-  Map<js.Node, int> finallyLabels = new Map<js.Node, int>();
+
+  /// The label of a finally part.
+  Map<js.Block, int> finallyLabels = new Map<js.Block, int>();
+
+  /// The label of the catch handler of a [js.Try] or a [js.Fun] or [js.Catch].

[floitsch, 2015/02/16 18:26:36]

Not sure I understand the comment.
is it ([js.Try] or a [js.Fun] or [js.Catch]), or
(the catch handler of a [js.Try]), or a [js.Fun] or [js.Catch].?

In any case it's not clear.

[sigurdm, 2015/02/17 10:29:05]

I try to explain more in depth.

+  /// The catch handler of a catch block will redirect to the enclosing handler
+  /// after running all the finally blocks inbetween.
+  Map<js.Node, int> handlerLabels = new Map<js.Node, int>();
+
   int exitLabel;
+  int rethrowLabel;
 
-  // A stack of all enclosing jump targets (including the function for
-  // representing the target of a return, and all enclosing try-blocks that have
-  // finally part, this way ensuring all the finally blocks between a jump and
-  // its target are run before the jump.
-  List<js.Node> targetsAndTries = new List<js.Node>();
+  /// A stack of all enclosing jump targets, pending finally blocks and
+  /// catch handlers.
+  ///
+  /// The jump-targets include the function for

[floitsch, 2015/02/16 18:26:36]

the function, representing the target of a return.

[sigurdm, 2015/02/17 10:29:06]

Done.

+  /// representing the target of a return,
+  ///
+  /// The enclosing catch-handlers include the enclosing function for rethrowing
+  /// an uncaught error.

[floitsch, 2015/02/16 18:26:36]

That sounds like the function is used either for the "return" or for rethrowing
uncaught errors. correct?

[sigurdm, 2015/02/17 10:29:06]

Yes, it is used for both.

+  ///
+  /// This is used to ensure all the finally blocks between a jump and
+  /// its target are run before the jump.
+  List<js.Node> targetsCatchesAndFinallies = new List<js.Node>();

[floitsch, 2015/02/16 18:26:36]

/// A stack of all (surrounding) jump targets.
///
/// Jump targets are:
///
/// * The function, signalling a return or uncaught throw.
/// * Loops.
/// * LabeledStatements (also used for 'continue' when attached to loops).
/// * Try statements, for catch and finally handlers.
///
/// When jumping to a target it is necessary to visit all finallies that
/// are on the way to target (i.e. more nested than the jump target).
List<js.Node> jumpTargetStack = new List<js.Node>();

[sigurdm, 2015/02/17 10:29:06]

This is not very precise. But might be easier to understand.

 
   List<int> continueStack = new List<int>();
   List<int> breakStack = new List<int>();
   List<int> returnStack = new List<int>();
 
   List<Pair<String, String>> variableRenamings =
       new List<Pair<String, String>>();
 
   PreTranslationAnalysis analysis;
 
-  List<int> errorHandlerLabels = new List<int>();
-
   final Function safeVariableName;
 
   // All the <x>Name variables are names of Javascript variables used in the
   // transformed code.
 
   /// Contains the result of an awaited expression, or a conditional or
   /// lazy boolean operator.
   ///
   /// For example a conditional expression is roughly translated like:
   /// [[cond ? a : b]]
   ///
   /// Becomes:
   ///
   /// while true { // outer while loop
   ///   switch (goto) { // Simulates goto
   ///     ...
   ///       goto = [[cond]] ? thenLabel : elseLabel
   ///       break;
   ///     case thenLabel:
   ///       result = [[a]];
   ///       goto = joinLabel;
   ///     case elseLabel:
   ///       result = [[b]];
   ///     case joinLabel:
   ///       // Now the result of computing the condition is in result.
   ///     ....
   ///   }
   /// }
   ///
-  /// It is a parameter to the [helperName] function, so that [thenHelper] and
-  /// [streamHelper] can call [helperName] with the result of an awaited Future.
+  /// It is a parameter to the [bodyName] function, so that [asyncHelper] and
+  /// [streamHelper] can call [bodyName] with the result of an awaited Future.
   String resultName;
 
+  /// A parameter to the [bodyName] function. Indicating if we are in success
+  /// or error case.
+  String errorCodeName;
+
   /// The name of the inner function that is scheduled to do each await/yield,
   /// and called to do a new iteration for sync*.
-  String helperName;
+  String bodyName;
 
   /// The Completer that will finish an async function.
   ///
   /// Not used for sync* or async* functions.
   String completerName;
 
   /// The StreamController that controls an async* function.
   ///
   /// Not used for async and sync* functions
   String controllerName;
 
   /// Used to simulate a goto.
   ///
   /// To "goto" a label, the label is assigned to this
   /// variable, and break out of the switch to take another iteration in the
   /// while loop. See [addGoto]
   String gotoName;
 
   /// The label of the current error handler.
   String handlerName;
 
   /// Current caught error.
   String errorName;
 
   /// A stack of labels of finally blocks to visit, and the label to go to after
   /// the last.
   String nextName;
 
+  /// The stack of labels of finally blocks to assign to [nextName] if the
+  /// async* [StreamSubscription] was canceled during a yield.
+  String nextWhenCanceledName;
+
   /// The current returned value (a finally block may overwrite it).
   String returnValueName;
 
+  /// If we are in the process of handling an error, stores the current error.
+  String currentErrorName;
+
   /// The label of the outer loop.
   ///
   /// Used if there are untransformed loops containing break or continues to
   /// targets outside the loop.
   String outerLabelName;
 
   /// If javascript `this` is used, it is accessed via this variable, in the
-  /// [helperName] function.
+  /// [bodyName] function.
   String selfName;
 
   // These expressions are hooks for communicating with the runtime.
 
   /// The function called by an async function to simulate an await or return.
   ///
   /// For an await it is called with:
   ///
   /// - The value to await
-  /// - The [helperName]
-  /// - The [completerName]
-  /// - A JavaScript function that is executed if the future completed with
-  ///   an error. That function is responsible for executing the right error
-  ///   handler and/or finally blocks).
+  /// - The body function [bodyName]
+  /// - The completer object [completerName]
   ///
   /// For a return it is called with:
   ///
   /// - The value to complete the completer with.
-  /// - null
+  /// - 0
   /// - The [completerName]

[floitsch, 2015/02/16 18:26:36]

nit: the completer object [completerName]

[sigurdm, 2015/02/17 10:29:05]

Done.

-  /// - null.
-  final js.Expression thenHelper;
+  ///
+  /// For a throw it is called with:
+  ///
+  /// - The value to complete the completer with.
+  /// - 1
+  /// - The [completerName]

[floitsch, 2015/02/16 18:26:36]

ditto.

[sigurdm, 2015/02/17 10:29:06]

Done.

+  final js.Expression asyncHelper;
 
   /// The function called by an async* function to simulate an await, yield or
   /// yield*.
   ///
   /// For an await/yield/yield* it is called with:
   ///
   /// - The value to await/yieldExpression(value to yield)/
   /// yieldStarExpression(stream to yield)
-  /// - The [helperName]
-  /// - The [controllerName]
-  /// - A JavaScript function that is executed if the future completed with
-  ///   an error. That function is responsible for executing the right error
-  ///   handler and/or finally blocks).
+  /// - The body function [bodyName]
+  /// - The controller object [controllerName]
   ///
   /// For a return it is called with:
   ///
   /// - null
   /// - null
   /// - The [controllerName]
   /// - null.
   final js.Expression streamHelper;
 
   /// Contructor used to initialize the [completerName] variable.
   ///
   /// Specific to async methods.
   final js.Expression newCompleter;
 
   /// Contructor used to initialize the [controllerName] variable.
   ///
   /// Specific to async* methods.
   final js.Expression newController;
 
   /// Used to get the `Stream` out of the [controllerName] variable.
   ///
   /// Specific to async* methods.
   final js.Expression streamOfController;
 
   /// Contructor creating the Iterable for a sync* method. Called with
-  /// [helperName].
+  /// [bodyName].
   final js.Expression newIterable;
 
   /// A JS Expression that creates a marker showing that iteration is over.
   ///
   /// Called without arguments.
   final js.Expression endOfIteration;
 
   /// A JS Expression that creates a marker indicating a 'yield' statement.
   ///
   /// Called with the value to yield.
@@ skipping 22 matching lines @@
 
   js.AsyncModifier async;
 
   bool get isSync => async == const js.AsyncModifier.sync();
   bool get isAsync => async == const js.AsyncModifier.async();
   bool get isSyncStar => async == const js.AsyncModifier.syncStar();
   bool get isAsyncStar => async == const js.AsyncModifier.asyncStar();
 
   AsyncRewriter(this.diagnosticListener,
                 spannable,
-                {this.thenHelper,
+                {this.asyncHelper,
                  this.streamHelper,
                  this.streamOfController,
                  this.newCompleter,
                  this.newController,
                  this.endOfIteration,
                  this.newIterable,
                  this.yieldExpression,
                  this.yieldStarExpression,
                  this.safeVariableName})
       : _spannable = spannable;
 
   /// Main entry point.
   /// Rewrites a sync*/async/async* function to an equivalent normal function.
   ///
   /// [spannable] can be passed to have a location for error messages.
   js.Fun rewrite(js.Fun node, [Spannable spannable]) {
     _spannable = spannable;
 
     async = node.asyncModifier;
     assert(!isSync);
 
     analysis = new PreTranslationAnalysis(unsupported);
     analysis.analyze(node);
 
     // To avoid name collisions with existing names, the fresh names are
     // generated after the analysis.
     resultName = freshName("result");
+    errorCodeName = freshName("errorCode");
     completerName = freshName("completer");
     controllerName = freshName("controller");
-    helperName = freshName("helper");
+    bodyName = freshName("body");
     gotoName = freshName("goto");
     handlerName = freshName("handler");
     errorName = freshName("error");
     nextName = freshName("next");
+    nextWhenCanceledName = freshName("nextWhenCanceled");
     returnValueName = freshName("returnValue");
+    currentErrorName = freshName("storedError");

[floitsch, 2015/02/16 18:26:36]

freshName("currentError")

[sigurdm, 2015/02/17 10:29:06]

Done.

     outerLabelName = freshName("outer");
     selfName = freshName("self");
 
     return node.accept(this);
   }
 
   js.Expression get currentErrorHandler {
-    return errorHandlerLabels.isEmpty
-        ? new js.LiteralNull()
-        : js.number(errorHandlerLabels.last);
+    return js.number(handlerLabels[targetsCatchesAndFinallies.lastWhere(
+        (node) => handlerLabels[node] != null)]);
   }
 
   int allocateTempVar() {
     assert(tempVarHighWaterMark >= currentTempVarIndex);
     currentTempVarIndex++;
     tempVarHighWaterMark = max(currentTempVarIndex, tempVarHighWaterMark);
     return currentTempVarIndex;
   }
 
   js.VariableUse useTempVar(int i) {
@@ skipping 241 matching lines @@
     }));
     var result = fn(visited);
     currentTempVarIndex = oldTempVarIndex;
     return result;
   }
 
   /// Emits the return block that all returns should jump to (after going
   /// through all the enclosing finally blocks). The jump to here is made in
   /// [visitReturn].
   ///
-  /// Returning from an async method calls the [thenHelper] with the result.
+  /// Returning from an async method calls the [asyncHelper] with the result.
   /// (the result might have been stored in [returnValueName] by some finally
   /// block).
   ///
   /// Returning from a sync* function returns an [endOfIteration] marker.
   ///
   /// Returning from an async* function calls the [streamHelper] with an
   /// [endOfIteration] marker.
   void addExit() {
     if (analysis.hasExplicitReturns || isAsyncStar) {
       beginLabel(exitLabel);
     } else {
       addStatement(new js.Comment("implicit return"));
     }
     switch (async) {
       case const js.AsyncModifier.async():
         String returnValue =
             analysis.hasExplicitReturns ? returnValueName : "null";
         addStatement(js.js.statement(
-            "return #thenHelper($returnValue, null, $completerName, null)", {
-          "thenHelper": thenHelper
-        }));
+            "return #thenHelper($returnValue, #successCode, "
+                "$completerName, null)", {
+             "thenHelper": asyncHelper,
+             "successCode": js.number(error_codes.SUCCESS)}));
         break;
       case const js.AsyncModifier.syncStar():
         addStatement(new js.Return(new js.Call(endOfIteration, [])));
         break;
       case const js.AsyncModifier.asyncStar():
         addStatement(js.js.statement(
-            "return #streamHelper(null, null, $controllerName, null)", {
-          "streamHelper": streamHelper
-        }));
+            "return #streamHelper(null, #successCode, $controllerName)", {
+          "streamHelper": streamHelper,
+          "successCode": js.number(error_codes.SUCCESS)}));
         break;
       default:
         diagnosticListener.internalError(
             spannable, "Internal error, unexpected asyncmodifier $async");
     }
+    if (isAsync || isAsyncStar) {
+      beginLabel(rethrowLabel);
+      addStatement(js.js.statement(
+          "return #thenHelper($currentErrorName, #errorCode, "
+          "${isAsync ? completerName : controllerName})", {
+            "thenHelper": isAsync ? asyncHelper : streamHelper,
+            "errorCode": js.number(error_codes.ERROR)}));
+    }
   }
 
-  /// The initial call to [thenHelper]/[streamHelper].
+  /// The initial call to [asyncHelper]/[streamHelper].
   ///
   /// There is no value to await/yield, so the first argument is `null` and
   /// also the errorCallback is `null`.
   ///
   /// Returns the [Future]/[Stream] coming from [completerName]/
   /// [controllerName].
   js.Statement generateInitializer() {
     if (isAsync) {
       return js.js.statement(
-          "return #thenHelper(null, $helperName, $completerName, null);", {
-        "thenHelper": thenHelper
+          "return #asyncHelper(null, $bodyName, $completerName, null);", {
+        "asyncHelper": asyncHelper
       });
     } else if (isAsyncStar) {
       return js.js.statement(
           "return #streamOfController($controllerName);", {
         "streamOfController": streamOfController
       });
     } else {
       throw diagnosticListener.internalError(
           spannable, "Unexpected asyncModifier: $async");
     }
   }
 
   /// Rewrites an async/sync*/async* function to a normal Javascript function.
   ///
   /// The control flow is flattened by simulating 'goto' using a switch in a
-  /// loop and a state variable [gotoName] inside a helper-function that can be
-  /// called back by [thenHelper]/[streamHelper]/the [Iterator].
+  /// loop and a state variable [gotoName] inside a nested function [bodyName]
+  /// that can be called back by [asyncHelper]/[asyncStarHelper]/the [Iterator].
   ///
   /// Local variables are hoisted outside the helper.
   ///
   /// Awaits in async/async* are translated to code that remembers the current
   /// location (so the function can resume from where it was) followed by a call
-  /// to the [thenHelper]. The helper sets up the waiting for the awaited value
+  /// to the [asyncHelper]. The helper sets up the waiting for the awaited value
   /// and returns a future which is immediately returned by the translated
   /// await.
-  /// Yields in async* are translated to a call to the [streamHelper]. They,
+  /// Yields in async* are translated to a call to the [asyncStarHelper]. They,
   /// too, need to be prepared to be interrupted in case the stream is paused or
   /// canceled. (Currently we always suspend - this is different from the spec,
   /// see `streamHelper` in `js_helper.dart`).
   ///
   /// Yield/yield* in a sync* function is translated to a return of the value,
   /// wrapped into a "IterationMarker" that signals the type (yield or yield*).
   /// Sync* functions are executed on demand (when the user requests a value) by
   /// the Iterable that knows how to handle these values.
   ///
   /// Simplified examples (not the exact translation, but intended to show the
   /// ideas):
   ///
   /// function (x, y, z) async {
   ///   var p = await foo();
   ///   return bar(p);
   /// }
   ///
-  /// Becomes:
+  /// Becomes (without error handling):
   ///
   /// function(x, y, z) {
   ///   var goto = 0, returnValue, completer = new Completer(), p;
   ///   function helper(result) {
   ///     while (true) {
   ///       switch (goto) {
   ///         case 0:
   ///           goto = 1 // Remember where to continue when the future succeeds.
-  ///           return thenHelper(foo(), helper, completer, null);
+  ///           return thenHelper(foo(), helper, completer);
   ///         case 1:
   ///           p = result;
   ///           returnValue = bar(p);
   ///           goto = 2;
   ///           break;
   ///         case 2:
-  ///           return thenHelper(returnValue, null, completer, null)
+  ///           return thenHelper(returnValue, null, completer)
   ///       }
   ///     }
-  ///     return thenHelper(null, helper, completer, null);
+  ///     return thenHelper(null, helper, completer);
   ///   }
   /// }
   ///
   /// Try/catch is implemented by maintaining [handlerName] to contain the label
   /// of the current handler. The switch is nested inside a try/catch that will
   /// redirect the flow to the current handler.
   ///
   /// A `finally` clause is compiled similar to normal code, with the additional
   /// complexity that `finally` clauses need to know where to jump to after the
   /// clause is done. In the translation, each flow-path that enters a `finally`
@@ skipping 14 matching lines @@
   ///   }
   /// }
   ///
   /// Translates into (besides the fact that structures not containing
   /// await/yield/yield* are left intact):
   ///
   /// function(x, y, z) {
   ///   var goto = 0;
   ///   var returnValue;
   ///   var completer = new Completer();
-  ///   var handler = null;
+  ///   var handler = 8; // Outside try-blocks go to the rethrow label.
   ///   var p;
+  ///   var storedError;
   ///   // The result can be either the result of an awaited future, or an
   ///   // error if the future completed with an error.
-  ///   function helper(result) {
+  ///   function helper(errorCode, result) {
+  ///     if (errorCode == 1) {
+  ///       storedError = result;
+  ///       goto = handler;
+  ///     }
   ///     while (true) {
   ///       try {
   ///         switch (goto) {
   ///           case 0:
   ///             handler = 4; // The outer catch-handler
   ///             handler = 1; // The inner (implicit) catch-handler
   ///             throw "error";
   ///             next = [3];
   ///             // After the finally (2) continue normally after the try.
   ///             goto = 2;
   ///             break;
   ///           case 1: // (implicit) catch handler for inner try.
   ///             next = [3]; // destination after the finally.
   ///             // fall-though to the finally handler.
   ///           case 2: // finally for inner try
   ///             handler = 4; // catch-handler for outer try.
   ///             finalize1();
   ///             goto = next.pop();
   ///             break;
   ///           case 3: // exiting inner try.
   ///             next = [6];
   ///             goto = 5; // finally handler for outer try.
   ///             break;
   ///           case 4: // catch handler for outer try.
-  ///             e = result;
+  ///             handler = 5; // If the handler throws, do the finally ..
+  ///             next = [8] // ... and rethrow.
+  ///             e = storedError;
   ///             handle(e);
   ///             // Fall through to finally.
   ///           case 5: // finally handler for outer try.
   ///             handler = null;
   ///             finalize2();
   ///             goto = next.pop();
   ///             break;
   ///           case 6: // Exiting outer try.
   ///           case 7: // return
-  ///             return thenHelper(returnValue, null, completer, null);
+  ///             return thenHelper(returnValue, 0, completer);
+  ///           case 8: // Rethrow
+  ///             return thenHelper(storedError, 1, completer);
   ///         }
   ///       } catch (error) {
-  ///         result = error;
+  ///         storedError = error;
   ///         goto = handler;
   ///       }
   ///     }
-  ///     return thenHelper(null, helper, completer, null);
+  ///     return thenHelper(null, helper, completer);
   ///   }
   /// }
   ///
   @override
   js.Expression visitFun(js.Fun node) {
     if (isSync) return node;
 
     beginLabel(newLabel("Function start"));
     // AsyncStar needs a returnlabel for its handling of cancelation. See
     // [visitDartYield].
     exitLabel =
         analysis.hasExplicitReturns || isAsyncStar ? newLabel("return") : null;
+    rethrowLabel = newLabel("rethrow");
+    handlerLabels[node] = rethrowLabel;
     js.Statement body = node.body;
-    targetsAndTries.add(node);
+    targetsCatchesAndFinallies.add(node);
     visitStatement(body);
-    targetsAndTries.removeLast();
+    targetsCatchesAndFinallies.removeLast();
     addExit();
 
     List<js.SwitchClause> clauses = labelledParts.keys.map((label) {
       return new js.Case(js.number(label), new js.Block(labelledParts[label]));
     }).toList();
     js.Statement helperBody =
         new js.Switch(new js.VariableUse(gotoName), clauses);
     if (hasJumpThoughOuterLabel) {
       helperBody = js.js.statement("$outerLabelName: #", [helperBody]);
     }
-    if (hasTryBlocks) {
-      helperBody = js.js.statement("""
-          try {
-            #body
-          } catch ($errorName){
-            if ($handlerName === null)
-              throw $errorName;
-            $resultName = $errorName;
-            $gotoName = $handlerName;
-          }""", {"body": helperBody});
-    }
+    helperBody = js.js.statement("""
+        try {
+          #body
+        } catch ($errorName){
+          $currentErrorName = $errorName;
+          $gotoName = $handlerName;
+        }""", {"body": helperBody});
     List<js.VariableInitialization> inits = <js.VariableInitialization>[];
 
     js.VariableInitialization makeInit(String name, js.Expression initValue) {
       return new js.VariableInitialization(
           new js.VariableDeclaration(name), initValue);
     }
 
     inits.add(makeInit(gotoName, js.number(0)));
     if (isAsync) {
       inits.add(makeInit(completerName, new js.New(newCompleter, [])));
     } else if (isAsyncStar) {
       inits.add(makeInit(controllerName,
-          new js.Call(newController, [new js.VariableUse(helperName)])));
+          new js.Call(newController, [new js.VariableUse(bodyName)])));
     }
-    if (hasTryBlocks) {
-      inits.add(makeInit(handlerName, new js.LiteralNull()));
-    }
+    inits.add(makeInit(handlerName, js.number(rethrowLabel)));
+    inits.add(makeInit(currentErrorName, null));
     if (hasJumpThroughFinally || analysis.hasYield) {
       inits.add(makeInit(nextName, null));
     }
     if (analysis.hasExplicitReturns && isAsync) {
       inits.add(makeInit(returnValueName, null));
     }
+    if (isSyncStar) {
+      inits.add(makeInit(resultName, null));
+    }
     if (analysis.hasThis && !isSyncStar) {
       // Sync* functions must remember `this` on the level of the outer
       // function.
       inits.add(makeInit(selfName, new js.This()));
     }
     inits.addAll(localVariables.map((js.VariableDeclaration decl) {
       return new js.VariableInitialization(decl, null);
     }));
     inits.addAll(new Iterable.generate(tempVarHighWaterMark,
         (int i) => makeInit(useTempVar(i + 1).name, null)));
     js.VariableDeclarationList varDecl = new js.VariableDeclarationList(inits);
+    // TODO(sigurdm): Explain the difference between these cases.
     if (isSyncStar) {
       return js.js("""
           function (#params) {
             if (#needsThis)
               var $selfName = this;
             return new #newIterable(function () {
               #varDecl;
-              return function $helperName($resultName) {
+              return function $bodyName() {
                 while (true)
                   #helperBody;
               };
             });
           }
           """, {
         "params": node.params,
         "needsThis": analysis.hasThis,
         "helperBody": helperBody,
         "varDecl": varDecl,
         "newIterable": newIterable
       });
     }
     return js.js("""
         function (#params) {
           #varDecl;
-          function $helperName($resultName) {
+          function $bodyName($errorCodeName, $resultName) {
+            if (#hasYield)
+              switch ($errorCodeName) {
+                case #streamWasCanceled:
+                  $nextName = $nextWhenCanceledName;
+                  $gotoName = $nextName.pop();
+                  break;
+                case #errorCode:
+                  $currentErrorName = $resultName;
+                  $gotoName = $handlerName;
+              }
+            else
+              if ($errorCodeName == #errorCode) {
+                  $currentErrorName = $resultName;
+                  $gotoName = $handlerName;
+              }
             while (true)
               #helperBody;
           }
           #init;
         }""", {
       "params": node.params,
+      "varDecl": varDecl,
+      "streamWasCanceled": js.number(error_codes.STREAM_WAS_CANCELED),
+      "errorCode": js.number(error_codes.ERROR),
+      "hasYield": analysis.hasYield,
       "helperBody": helperBody,
-      "varDecl": varDecl,
       "init": generateInitializer()
     });
   }
 
   @override
   js.Expression visitAccess(js.PropertyAccess node) {
     return withExpression2(node.receiver, node.selector,
         (receiver, selector) => new js.PropertyAccess(receiver, selector));
   }
 
@@ skipping 28 matching lines @@
       ], (evaluated) {
         return new js.Assignment.compound(
             new js.PropertyAccess(evaluated[0], evaluated[1]), node.op,
             evaluated[2]);
       });
     } else {
       throw "Unexpected assignment left hand side $leftHandSide";
     }
   }
 
-  /// An await is translated to a call to [thenHelper]/[streamHelper].
+  /// An await is translated to a call to [asyncHelper]/[streamHelper].
   ///
   /// See the comments of [visitFun] for an example.
   @override
   js.Expression visitAwait(js.Await node) {
     assert(isAsync || isAsyncStar);
     int afterAwait = newLabel("returning from await.");
     withExpression(node.expression, (js.Expression value) {
       addStatement(setGotoVariable(afterAwait));
-      js.Expression errorCallback = errorHandlerLabels.isEmpty
-          ? new js.LiteralNull()
-          : js.js("""
-            function($errorName) {
-              $gotoName = #currentHandler;
-              $helperName($errorName);
-            }""", {"currentHandler": currentErrorHandler});
-
       addStatement(js.js.statement("""
-          return #thenHelper(#value,
-                             $helperName,
-                             ${isAsync ? completerName : controllerName},
-                             #errorCallback);
+          return #asyncHelper(#value,
+                             $bodyName,

[floitsch, 2015/02/16 18:26:36]

nit: indentation.

[sigurdm, 2015/02/17 10:29:06]

Done.

+                             ${isAsync ? completerName : controllerName});
           """, {
-        "thenHelper": isAsync ? thenHelper : streamHelper,
+        "asyncHelper": isAsync ? asyncHelper : streamHelper,
         "value": value,
-        "errorCallback": errorCallback
       }));
     }, store: false);
     beginLabel(afterAwait);
     return new js.VariableUse(resultName);
   }
 
   /// Checks if [node] is the variable named [resultName].
   ///
   /// [resultName] is used to hold the result of a transformed computation
   /// for example the result of awaiting, or the result of a conditional or
@@ skipping 139 matching lines @@
   ///
   /// It is necessary to run all nesting finally-handlers between the jump and
   /// the target. For that [nextName] is used as a stack of places to go.
   ///
   /// See also [visitFun].
   void translateJump(js.Node target, int targetLabel) {
     // Compute a stack of all the 'finally' nodes that must be visited before
     // the jump.
     // The bottom of the stack is the label where the jump goes to.
     List<int> jumpStack = new List<int>();
-    for (js.Node node in targetsAndTries.reversed) {
-      if (node is js.Try) {
-        assert(node.finallyPart != null);
+    for (js.Node node in targetsCatchesAndFinallies.reversed) {
+      if (finallyLabels[node] != null) {
         jumpStack.add(finallyLabels[node]);
       } else if (node == target) {
         jumpStack.add(targetLabel);
         break;
       }
       // Ignore other nodes.
     }
     jumpStack = jumpStack.reversed.toList();
     // As the program jumps directly to the top of the stack, it is taken off
     // now.
@@ skipping 24 matching lines @@
     int startLabel = newLabel("do body");
 
     int continueLabel = newLabel("do condition");
     continueLabels[node] = continueLabel;
 
     int afterLabel = newLabel("after do");
     breakLabels[node] = afterLabel;
 
     beginLabel(startLabel);
 
-    targetsAndTries.add(node);
+    targetsCatchesAndFinallies.add(node);
     visitStatement(node.body);
-    targetsAndTries.removeLast();
+    targetsCatchesAndFinallies.removeLast();
 
     beginLabel(continueLabel);
     withExpression(node.condition, (js.Expression condition) {
       addStatement(new js.If.noElse(condition, gotoAndBreak(startLabel)));
     }, store: false);
     beginLabel(afterLabel);
   }
 
   @override
   void visitEmptyStatement(js.EmptyStatement node) {
@@ skipping 48 matching lines @@
     js.Expression condition = node.condition;
     if (condition == null ||
         (condition is js.LiteralBool && condition.value == true)) {
       addStatement(new js.Comment("trivial condition"));
     } else {
       withExpression(condition, (js.Expression condition) {
         addStatement(new js.If.noElse(
             new js.Prefix("!", condition), gotoAndBreak(afterLabel)));
       }, store: false);
     }
-    targetsAndTries.add(node);
+    targetsCatchesAndFinallies.add(node);
     visitStatement(node.body);
-    targetsAndTries.removeLast();
+    targetsCatchesAndFinallies.removeLast();
     if (node.update != null) {
       beginLabel(continueLabel);
       visitExpressionIgnoreResult(node.update);
     }
     addGoto(startLabel);
     beginLabel(afterLabel);
   }
 
   @override
   void visitForIn(js.ForIn node) {
@@ skipping 80 matching lines @@
       addStatement(
           new js.LabeledStatement(node.label, translateInBlock(node.body)));
       return;
     }
     int breakLabel = newLabel("break ${node.label}");
     int continueLabel = newLabel("continue ${node.label}");
     breakLabels[node] = breakLabel;
     continueLabels[node] = continueLabel;
 
     beginLabel(continueLabel);
-    targetsAndTries.add(node);
+    targetsCatchesAndFinallies.add(node);
     visitStatement(node.body);
-    targetsAndTries.removeLast();
+    targetsCatchesAndFinallies.removeLast();
     beginLabel(breakLabel);
   }
 
   @override
   js.Expression visitLiteralBool(js.LiteralBool node) => node;
 
   @override
   visitLiteralExpression(js.LiteralExpression node) => unsupported(node);
 
   @override
@@ skipping 185 matching lines @@
         i++;
       }
       withExpression(node.key, (js.Expression key) {
         addStatement(new js.Switch(key, clauses));
       }, store: false);
       if (!hasDefault) {
         addGoto(after);
       }
     }
 
-    targetsAndTries.add(node);
+    targetsCatchesAndFinallies.add(node);
     for (int i = 0; i < labels.length; i++) {
       beginLabel(labels[i]);
       visitStatement(node.cases[i].body);
     }
     beginLabel(after);
-    targetsAndTries.removeLast();
+    targetsCatchesAndFinallies.removeLast();
   }
 
   @override
   js.Expression visitThis(js.This node) {
     return new js.VariableUse(selfName);
   }
 
   @override
   void visitThrow(js.Throw node) {
     withExpression(node.expression, (js.Expression expression) {
       addStatement(new js.Throw(expression));
     }, store: false);
   }
 
-  setErrorHandler() {
+  setErrorHandler([int errorHandler]) {
     addExpressionStatement(new js.Assignment(
-        new js.VariableUse(handlerName), currentErrorHandler));
+        new js.VariableUse(handlerName),
+        errorHandler == null ? currentErrorHandler : js.number(errorHandler)));
   }
 
-  @override
+  List<int> _finalliesUpToAndEnclosingHandler() {
+    List<int> result = new List<int>();
+    for (int i = targetsCatchesAndFinallies.length - 1; i >= 0; i--) {
+      js.Node node = targetsCatchesAndFinallies[i];
+      int handlerLabel = handlerLabels[node];
+      if (handlerLabel != null) {
+        result.add(handlerLabel);
+        break;
+      }
+      int finallyLabel = finallyLabels[node];
+      if (finallyLabel != null) {
+        result.add(finallyLabel);
+      }
+    }
+    return result.reversed.toList();
+  }
+
+  /// See the comments of [visitFun] for more explanation.
   void visitTry(js.Try node) {
     if (!shouldTransform(node)) {
       js.Block body = translateInBlock(node.body);
       js.Catch catchPart = (node.catchPart == null)
           ? null
           : new js.Catch(node.catchPart.declaration,
               translateInBlock(node.catchPart.body));
       js.Block finallyPart = (node.finallyPart == null)
           ? null
           : translateInBlock(node.finallyPart);
       addStatement(new js.Try(body, catchPart, finallyPart));
       return;
     }
+
     hasTryBlocks = true;
-    int handlerLabel = newLabel("catch");
+    int uncaughtLabel = newLabel("uncaught");
+    int handlerLabel = (node.catchPart == null)
+        ? uncaughtLabel
+        : newLabel("catch");
+
     int finallyLabel = newLabel("finally");
     int afterFinallyLabel = newLabel("after finally");
-    errorHandlerLabels.add(handlerLabel);
+    if (node.finallyPart != null) {
+      finallyLabels[node.finallyPart] = finallyLabel;
+      targetsCatchesAndFinallies.add(node.finallyPart);
+    }
+    if (node.catchPart != null) {
+      handlerLabels[node.catchPart] = uncaughtLabel;

[floitsch, 2015/02/16 18:26:36]

This needs a comment.
Could you delay this until line 1658?

[sigurdm, 2015/02/17 10:29:05]

Comment added
Yes, it belongs better there.

+    }
+
+    handlerLabels[node] = handlerLabel;
+    targetsCatchesAndFinallies.add(node);
+
     // Set the error handler here. It must be cleared on every path out;
     // normal and error exit.
     setErrorHandler();
-    if (node.finallyPart != null) {
-      finallyLabels[node] = finallyLabel;
-      targetsAndTries.add(node);
-    }
+
     visitStatement(node.body);
-    errorHandlerLabels.removeLast();
-    addStatement(
-        js.js.statement("$nextName = [#];", [js.number(afterFinallyLabel)]));
+
+    js.Node last = targetsCatchesAndFinallies.removeLast();
+    assert(last == node);
+
     if (node.finallyPart == null) {
       setErrorHandler();
       addGoto(afterFinallyLabel);
     } else {
-      // The handler is set as the first thing in the finally block.
+      // The handler is reset as the first thing in the finally block.
+      addStatement(
+          js.js.statement("$nextName = [#];", [js.number(afterFinallyLabel)]));
       addGoto(finallyLabel);
     }
-    beginLabel(handlerLabel);
+
     if (node.catchPart != null) {
+      beginLabel(handlerLabel);
+      targetsCatchesAndFinallies.add(node.catchPart);
       setErrorHandler();
       // The catch declaration name can shadow outer variables, so a fresh name
       // is needed to avoid collisions.  See Ecma 262, 3rd edition,
       // section 12.14.
       String errorRename = freshName(node.catchPart.declaration.name);
       localVariables.add(new js.VariableDeclaration(errorRename));
       variableRenamings
           .add(new Pair(node.catchPart.declaration.name, errorRename));
       addExpressionStatement(new js.Assignment(
-          new js.VariableUse(errorRename), new js.VariableUse(resultName)));
+          new js.VariableUse(errorRename),
+          new js.VariableUse(currentErrorName)));
       visitStatement(node.catchPart.body);
       variableRenamings.removeLast();
+      if (node.finallyPart != null) {
+        // The error has been caught, so after the finally, continue after the
+        // try.
+        addStatement(js.js.statement("$nextName = [#];",
+                                     [js.number(afterFinallyLabel)]));
+        addGoto(finallyLabel);
+      } else {
+        addGoto(afterFinallyLabel);
+      }
+      js.Node last = targetsCatchesAndFinallies.removeLast();
+      assert(last == node.catchPart);
+    }
+
+    // The "uncaught"-handler tells the finally-block to continue with
+    // the enclosing finally-blocks until the current catch-handler.
+    beginLabel(uncaughtLabel);
+
+    List<int> enclosingFinallies = _finalliesUpToAndEnclosingHandler();
+
+    int nextLabel = enclosingFinallies.removeLast();
+    if (enclosingFinallies.isNotEmpty) {
+      // enclosingFinallies can be empty if

[floitsch, 2015/02/16 18:26:36]

Unfinished comment.

[sigurdm, 2015/02/17 10:29:06]

Done.

[sigurdm, 2015/02/17 10:29:06]

Done.

+      addStatement(
+          js.js.statement("$nextName = #;", new js.ArrayInitializer(
+              enclosingFinallies.map(js.number).toList())));
+    }
+    if (node.finallyPart == null) {
+      // The finally-block belonging to [node] will be visited because of
+      // fallthrough. If it does not exist, add an explicit goto.
+      addGoto(nextLabel);
     }
     if (node.finallyPart != null) {
-      targetsAndTries.removeLast();
-      setErrorHandler();
-      // This belongs to the catch-part, but is only needed if there is a
-      // `finally`. Therefore it is in this branch.
-      // This is needed even if there is no explicit catch-branch, because
-      // if an exception is raised the finally part has to be run.
-      addStatement(
-          js.js.statement("$nextName = [#];", [js.number(afterFinallyLabel)]));
+      js.Node last = targetsCatchesAndFinallies.removeLast();
+      assert(last == node.finallyPart);
+
       beginLabel(finallyLabel);
       setErrorHandler();
       visitStatement(node.finallyPart);
       addStatement(new js.Comment("// goto the next finally handler"));
       addStatement(js.js.statement("$gotoName = $nextName.pop();"));
       addBreak();
     }
     beginLabel(afterFinallyLabel);
   }
 
@@ skipping 49 matching lines @@
     int afterLabel = newLabel("after while");
     breakLabels[node] = afterLabel;
     js.Expression condition = node.condition;
     // If the condition is `true`, a test is not needed.
     if (!(condition is js.LiteralBool && condition.value == true)) {
       withExpression(node.condition, (js.Expression condition) {
         addStatement(new js.If.noElse(
             new js.Prefix("!", condition), gotoAndBreak(afterLabel)));
       }, store: false);
     }
-    targetsAndTries.add(node);
+    targetsCatchesAndFinallies.add(node);
     visitStatement(node.body);
-    targetsAndTries.removeLast();
+    targetsCatchesAndFinallies.removeLast();
     addGoto(continueLabel);
     beginLabel(afterLabel);
   }
 
   /// Translates a yield/yield* in an sync*.
   ///
   /// `yield` in a sync* function just returns [value].
   /// `yield*` wraps [value] in a [yieldStarExpression] and returns it.
   void addSyncYield(js.DartYield node, js.Expression expression) {
     assert(isSyncStar);
     if (node.hasStar) {
       addStatement(
           new js.Return(new js.Call(yieldStarExpression, [expression])));
     } else {
       addStatement(new js.Return(expression));
     }
   }
 
   /// Translates a yield/yield* in an async* function.
   ///
   /// yield/yield* in an async* function is translated much like the `await` is
   /// translated in [visitAwait], only the object is wrapped in a
-  /// [yieldExpression]/[yieldStarExpression] to let [streamHelper] distinguish.
-  ///
-  /// Because there is no Future that can fail (as there is in await) null is
-  /// passed as the errorCallback.
+  /// [yieldExpression]/[yieldStarExpression] to let [asyncStarHelper]
+  /// distinguish them.
+  /// Also a [nextWhenCanceledName] is set up to

[floitsch, 2015/02/16 18:26:36]

Unfinished sentence.

[sigurdm, 2015/02/17 10:29:05]

Done.

   void addAsyncYield(js.DartYield node, js.Expression expression) {
     assert(isAsyncStar);
     // Find all the finally blocks that should be performed if the stream is
     // canceled during the yield.
     // At the bottom of the stack is the return label.
     List<int> enclosingFinallyLabels = <int>[exitLabel];
-    enclosingFinallyLabels.addAll(targetsAndTries
-        .where((js.Node node) => node is js.Try)
-        .map((js.Try node) => finallyLabels[node]));
-    int destinationOnCancel = enclosingFinallyLabels.removeLast();
-    js.ArrayInitializer finallyListInitializer = new js.ArrayInitializer(
-        enclosingFinallyLabels.map(js.number).toList());
+    enclosingFinallyLabels.addAll(targetsCatchesAndFinallies
+        .where((js.Node node) => finallyLabels[node] != null)
+        .map((js.Block node) => finallyLabels[node]));
+    addStatement(js.js.statement("$nextWhenCanceledName = #",
+        [new js.ArrayInitializer(enclosingFinallyLabels.map(js.number)
+            .toList())]));
     addStatement(js.js.statement("""
         return #streamHelper(#yieldExpression(#expression),
-            $helperName, $controllerName, function () {
-              if (#notEmptyFinallyList)
-                $nextName = #finallyList;
-              $gotoName = #destinationOnCancel;
-              $helperName();
-            });""", {
+            $bodyName, $controllerName);""", {
       "streamHelper": streamHelper,
       "yieldExpression": node.hasStar ? yieldStarExpression : yieldExpression,
       "expression": expression,
-      "notEmptyFinallyList": enclosingFinallyLabels.isNotEmpty,
-      "finallyList": finallyListInitializer,
-      "destinationOnCancel": js.number(destinationOnCancel)
     }));
   }
 
   @override
   void visitDartYield(js.DartYield node) {
     assert(isSyncStar || isAsyncStar);
     int label = newLabel("after yield");
     // Don't do a break here for the goto, but instead a return in either
     // addSynYield or addAsyncYield.
     withExpression(node.expression, (js.Expression expression) {
@@ skipping 417 matching lines @@
     return condition || body;
   }
 
   @override
   bool visitDartYield(js.DartYield node) {
     hasYield = true;
     visit(node.expression);
     return true;
   }
 }