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'

[floitsch, 2015/02/13 15:30:06]

I think we import it relative for embedded names. Please verify and do the same.

[sigurdm, 2015/02/17 08:43:10]

At least in js_backend/js_backend.dart it is imported as a package-import.

+    as error_codes;

[floitsch, 2015/02/13 15:30:07]

not 100% about the naming convention of imports. With or without "_".
(just make sure it's the right one)

[sigurdm, 2015/02/17 08:43:10]

The style guide says lower_case_underscores.

+
 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>();
+  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>();
 
   List<int> continueStack = new List<int>();
   List<int> breakStack = new List<int>();
   List<int> returnStack = new List<int>();
@@ skipping 27 matching lines @@
   ///       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 yieldNextName;

[floitsch, 2015/02/13 15:30:06]

nextWhenCanceledName ?

[sigurdm, 2015/02/17 08:43:09]

Better, Donde

+
   /// The current returned value (a finally block may overwrite it).
   String returnValueName;
 
+  /// The thrown error (it might need to be rethrown after a finally block).

[floitsch, 2015/02/13 15:30:06]

currentErrorName ?

/// If we are in the process of handling an error, stores the current error.
String currentErrorName;

[sigurdm, 2015/02/17 08:43:09]

Done.

+  String storedErrorName;
+
   /// 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 [bodyName]
   /// - 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).
   ///
   /// For a return it is called with:
   ///
   /// - The value to complete the completer with.
   /// - null
   /// - The [completerName]
   /// - null.
-  final js.Expression thenHelper;
+  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 [bodyName]

[floitsch, 2015/02/13 15:30:06]

The body function ([bodyName]). ?

Otherwise it sounds too much like we pass in a string.

Same for the others.

[sigurdm, 2015/02/17 08:43:10]

Done.

   /// - 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).
   ///
   /// 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");
+    yieldNextName = freshName("yieldNext");
     returnValueName = freshName("returnValue");
+    storedErrorName = freshName("storedError");
     outerLabelName = freshName("outer");
     selfName = freshName("self");
 
     return node.accept(this);
   }
 
   js.Expression get currentErrorHandler {
     return errorHandlerLabels.isEmpty
-        ? new js.LiteralNull()
+        ? js.number(rethrowLabel)
         : js.number(errorHandlerLabels.last);
   }
 
   int allocateTempVar() {
     assert(tempVarHighWaterMark >= currentTempVarIndex);
     currentTempVarIndex++;
     tempVarHighWaterMark = max(currentTempVarIndex, tempVarHighWaterMark);
     return currentTempVarIndex;
   }
 
@@ skipping 242 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, ${error_codes.SUCCESS}, "

[floitsch, 2015/02/13 15:30:06]

Don't use "$" if you don't need to.

This is a value. Pass it in as expression.

[sigurdm, 2015/02/17 08:43:10]

Done.

+                "$completerName, null)",
+            {"thenHelper": asyncHelper}));
         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)", {
+            "return #streamHelper(null, ${error_codes.SUCCESS}, "

[floitsch, 2015/02/13 15:30:06]

ditto.

[sigurdm, 2015/02/17 08:43:10]

Done.

+                "$controllerName)", {
           "streamHelper": streamHelper
         }));
         break;
       default:
         diagnosticListener.internalError(
             spannable, "Internal error, unexpected asyncmodifier $async");
     }
+    if (isAsync || isAsyncStar) {
+      beginLabel(rethrowLabel);
+      addStatement(js.js.statement(
+          "return #thenHelper($storedErrorName, ${error_codes.ERROR}, "

[floitsch, 2015/02/13 15:30:06]

ditto.

[sigurdm, 2015/02/17 08:43:09]

Done.

+          "${isAsync ? completerName : controllerName})",
+          {"thenHelper": isAsync ? asyncHelper : streamHelper}));
+    }
   }
 
-  /// 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.

[floitsch, 2015/02/13 15:30:06]

Handler is slightly weird, because we also use it for finallies.

For now it's probably ok to mention this at the sites where we assign a finally
as if it was a handler.

[sigurdm, 2015/02/17 08:43:10]

Acknowledged. I mentioned it in visitTry.

   ///   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 we throw in this handler, do the finally

[floitsch, 2015/02/13 15:30:06]

?
// If the handler throws, do the finally ...
// ... and rethrow.

[floitsch, 2015/02/13 15:30:07]

?
// If the handler throws, do the finally ...
// ... and rethrow.

[sigurdm, 2015/02/17 08:43:10]

Acknowledged.

[sigurdm, 2015/02/17 08:43:10]

Done.

+  ///             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);
     visitStatement(body);
     targetsAndTries.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){
+          $storedErrorName = $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(storedErrorName, null));
     if (hasJumpThroughFinally) {
       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);
     if (isSyncStar) {

[floitsch, 2015/02/13 15:30:06]

Not necessarily this CL, but explain why this is different.

[sigurdm, 2015/02/17 08:43:09]

Added todo

       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 ${error_codes.STREAM_WAS_CANCELED}:
+                  $nextName = $yieldNextName;
+                  // fallthrough

[floitsch, 2015/02/13 15:30:06]

Why fall through?
Is it to avoid repeating the $gotoName = $handlerName line? Or do you also need
the storedErrorName ?

[sigurdm, 2015/02/17 08:43:09]

Only for the handler. I changed the code, so it does not fall through anymore.

+                case ${error_codes.ERROR}:
+                  $storedErrorName = $resultName;
+                  $gotoName = $handlerName;
+              }
+            else
+              if ($errorCodeName == ${error_codes.ERROR}) {
+                  $storedErrorName = $resultName;
+                  $gotoName = $handlerName;
+              }
             while (true)
               #helperBody;
           }
           #init;
         }""", {
       "params": node.params,
+      "varDecl": varDecl,
+      "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);
+                             $bodyName,
+                             ${isAsync ? completerName : controllerName});
           """, {
-        "thenHelper": isAsync ? thenHelper : streamHelper,
+        "thenHelper": isAsync ? asyncHelper : streamHelper,

[floitsch, 2015/02/13 15:30:07]

rename "thenHelper" to "asyncHelper" ?

[sigurdm, 2015/02/17 08:43:10]

Done.

         "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 589 matching lines @@
     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> _finalliesAndEnclosingHandler() {
+    List<int> result = new List<int>();
+    for (int i = targetsAndTries.length - 1; i >= 0; i--) {
+      js.Node node = targetsAndTries[i];
+      int handlerLabel = handlerLabels[node];
+      if (handlerLabel != null) {
+        result.add(handlerLabel);
+        break;
+      }
+      if (node is js.Try) {
+        assert(node.finallyPart != null);
+        result.add(finallyLabels[node]);
+      }
+    }
+    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 finallyLabel = newLabel("finally");
     int afterFinallyLabel = newLabel("after finally");
     errorHandlerLabels.add(handlerLabel);
     // 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)]));
     if (node.finallyPart == null) {
       setErrorHandler();
       addGoto(afterFinallyLabel);
     } else {
       // The handler is set as the first thing in the finally block.
       addGoto(finallyLabel);
     }
     beginLabel(handlerLabel);
     if (node.catchPart != null) {
-      setErrorHandler();
+      if (node.finallyPart == null) {
+        setErrorHandler();
+      } else {
+        // If an error is thrown in the catch-part, and there is a finally
+        // block, goto that.
+        // And from that finally block, go to all enclosing finally blocks until

[floitsch, 2015/02/13 15:30:06]

If the catch-part throws visit the finally block first (if there is one).
Then visit all surrounding finallies, until there is a catch-handler or the
'rethrow' label if there is none.

[sigurdm, 2015/02/17 08:43:09]

Done.

+        // the first enclosing catch-handler, and go to those.
+        List<int> finallies = _finalliesAndEnclosingHandler();
+        setErrorHandler(finallies.removeLast());
+        addStatement(js.js.statement("$nextName = #;",
+            [new js.ArrayInitializer(finallies.map(js.number).toList())]));
+      }
       // 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(storedErrorName)));
       visitStatement(node.catchPart.body);
       variableRenamings.removeLast();
     }
+
     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)]));
       beginLabel(finallyLabel);
@@ skipping 83 matching lines @@
           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.

[floitsch, 2015/02/13 15:30:06]

distinguish them.

[sigurdm, 2015/02/17 08:43:09]

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());
+    setErrorHandler(enclosingFinallyLabels.removeLast());

[sigurdm, 2015/02/17 08:43:09]

I realized that I forget to reset the error-handler after the yield.

So now I will instead have all the finally-labels in nextWhenCanceledName and
pop from that before the switch.

+    addStatement(js.js.statement("$yieldNextName = #",
+        [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 20 matching lines @@
   Map<js.Node, js.Node> targets = new Map<js.Node, js.Node>();
   List<js.Node> loopsAndSwitches = new List<js.Node>();
   List<js.LabeledStatement> labelledStatements =
       new List<js.LabeledStatement>();
   Set<String> usedNames = new Set<String>();
 
   bool hasExplicitReturns = false;
 
   bool hasThis = false;
 
+  bool hasYield = false;
+
   // The function currently being analyzed.
   js.Fun currentFunction;
 
   // For error messages.
   final Function unsupported;
 
   PreTranslationAnalysis(void this.unsupported(js.Node node));
 
   bool visit(js.Node node) {
     bool containsAwait = node.accept(this);
@@ skipping 370 matching lines @@
   bool visitWhile(js.While node) {
     loopsAndSwitches.add(node);
     bool condition = visit(node.condition);
     bool body = visit(node.body);
     loopsAndSwitches.removeLast();
     return condition || body;
   }
 
   @override
   bool visitDartYield(js.DartYield node) {
+    hasYield = true;
     visit(node.expression);
     return true;
   }
 }