Implementation of async-await transformation on js ast.

pkg/compiler/lib/src/ssa/builder.dart

Index: pkg/compiler/lib/src/ssa/builder.dart
diff --git a/pkg/compiler/lib/src/ssa/builder.dart b/pkg/compiler/lib/src/ssa/builder.dart
index 43798cd7243067a82a320d3321048a8e36a8730b..ad78b8f377a06333b536845845790ff9ea47c289 100644
--- a/pkg/compiler/lib/src/ssa/builder.dart
+++ b/pkg/compiler/lib/src/ssa/builder.dart
@@ -19,7 +19,42 @@ class SsaFunctionCompiler implements FunctionCompiler {
   js.Fun compile(CodegenWorkItem work) {
     HGraph graph = builder.build(work);
     optimizer.optimize(work, graph);
-    return generator.generateCode(work, graph);
+    Element element = work.element;
+    js.Expression result = generator.generateCode(work, graph);
+    if (element is FunctionElement) {

[floitsch, 2015/02/02 22:00:10]

For another CL: the FunctionCompiler should *not* be in the ssa-builder file.

[sigurdm, 2015/02/03 16:59:31]

Acknowledged.

+      JavaScriptBackend backend = builder.backend;
+      if (element.asyncMarker == AsyncMarker.ASYNC) {
+        result = new AsyncRewriter(
+            thenHelper:
+                backend.emitter.staticFunctionAccess(backend.getThenHelper()),
+            newController: new js.New(backend.emitter.staticFunctionAccess(
+                backend.getCompleterConstructor()), []),
+            safeVariableName: backend.namer.safeVariableName)
+            .rewrite(result);

[floitsch, 2015/02/02 22:00:11]

The "rewrite" drowns in the whole expression.
Create a rewriter first, and then call 'rewrite' on it.

[sigurdm, 2015/02/03 16:59:31]

Done.

+      } else if (element.asyncMarker == AsyncMarker.SYNC_STAR) {
+        result = new AsyncRewriter(
+            endOfIteration: backend.emitter.staticFunctionAccess(
+                backend.getEndOfIteration()),
+            newIterable: backend.emitter.staticFunctionAccess(
+                backend.getSyncStarIterableConstructor()),
+            yieldStarExpression: backend.emitter.staticFunctionAccess(
+                backend.getSyncStarYieldStar()),
+            safeVariableName: backend.namer.safeVariableName)
+            .rewrite(result);
+      }
+      else if (element.asyncMarker == AsyncMarker.ASYNC_STAR) {
+        result = new AsyncRewriter(
+            thenHelper: backend.emitter.staticFunctionAccess(
+                backend.getStreamHelper()),
+            newController: new js.Call(backend.emitter.staticFunctionAccess(
+                backend.getMakeController()), []),
+            safeVariableName: backend.namer.safeVariableName,
+            yieldExpression: backend.emitter.staticFunctionAccess(
+                backend.getYieldSingle()))
+            .rewrite(result);
+      }
+    }
+    return result;
   }
 
   Iterable<CompilerTask> get tasks {
@@ -362,7 +397,7 @@ class LocalsHandler {
   bool isAccessedDirectly(Local local) {
     assert(local != null);
     return !redirectionMapping.containsKey(local)
-        && !closureData.usedVariablesInTry.contains(local);
+        && !closureData.variablesUsedInTryOrGenerator.contains(local);
   }
 
   bool isStoredInClosureField(Local local) {
@@ -379,8 +414,8 @@ class LocalsHandler {
     return redirectionMapping.containsKey(local);
   }
 
-  bool isUsedInTry(Local local) {
-    return closureData.usedVariablesInTry.contains(local);
+  bool isUsedInTryOrGenerator(Local local) {
+    return closureData.variablesUsedInTryOrGenerator.contains(local);
   }
 
   /**
@@ -422,7 +457,7 @@ class LocalsHandler {
       builder.add(lookup);
       return lookup;
     } else {
-      assert(isUsedInTry(local));
+      assert(isUsedInTryOrGenerator(local));
       HLocalValue localValue = getLocal(local);
       HInstruction instruction = new HLocalGet(
           local, localValue, builder.backend.dynamicType);
@@ -478,7 +513,7 @@ class LocalsHandler {
       HInstruction box = readLocal(redirect.box);
       builder.add(new HFieldSet(redirect, box, value));
     } else {
-      assert(isUsedInTry(local));
+      assert(isUsedInTryOrGenerator(local));
       HLocalValue localValue = getLocal(local);
       builder.add(new HLocalSet(local, localValue, value));
     }
@@ -5164,14 +5199,18 @@ class SsaBuilder extends ResolvedVisitor {
   }
 
   visitYield(ast.Yield node) {
-    // Dummy implementation.
     visit(node.expression);
+    HInstruction yielded = pop();
+    push(new HYield(yielded, node.hasStar));

[floitsch, 2015/02/02 22:00:10]

I think there is a function that does this. Maybe 'add' or something.
(in any case: push followed by pop should not be necessary).

[sigurdm, 2015/02/03 16:59:31]

Right, add does it.

     pop();
   }
 
   visitAwait(ast.Await node) {
-    // Dummy implementation.
     visit(node.expression);
+    HInstruction awaited = pop();
+    // TODO(herhut): Improve this type.
+    push(new HAwait(awaited, new TypeMask.subclass(compiler.objectClass,
+                                                   compiler.world)));
   }
 
   visitTypeAnnotation(ast.TypeAnnotation node) {
@@ -5315,7 +5354,67 @@ class SsaBuilder extends ResolvedVisitor {
     return new JumpHandler(this, element);
   }
 
+

[floitsch, 2015/02/02 22:00:10]

no need for the new line.

[sigurdm, 2015/02/03 16:59:31]

Done.

   visitForIn(ast.ForIn node) {
+    if (node.isAsync) {

[floitsch, 2015/02/02 22:00:10]

Move the code into a separate function.

[sigurdm, 2015/02/03 16:59:32]

Done.

+      // The async for is implemented with a StreamIterator.

[floitsch, 2015/02/02 22:00:10]

async-for

[sigurdm, 2015/02/03 16:59:31]

Done.

+      HInstruction streamIterator;
+
+      visit(node.expression);
+      HInstruction expression = pop();
+      pushInvokeStatic(node,
+                       backend.getStreamIteratorConstructor(),
+                       [expression, graph.addConstantNull(compiler)]);
+      streamIterator = pop();
+      void buildInitializer() {

[floitsch, 2015/02/02 22:00:11]

nested functions should have a new line before and after.

[sigurdm, 2015/02/03 16:59:31]

Done.

+      }
+      HInstruction buildCondition() {
+        Selector selector = elements.getMoveNextSelector(node);
+        pushInvokeDynamic(node, selector, [streamIterator]);
+        HInstruction future = pop();
+        push(new HAwait(future, new TypeMask.subclass(compiler.objectClass,
+                                                      compiler.world)));
+        return popBoolified();
+      }
+      void buildBody() {
+        Selector call = elements.getCurrentSelector(node);
+        pushInvokeDynamic(node, call, [streamIterator]);
+
+        ast.Node identifier = node.declaredIdentifier;
+        Element variable = elements.getForInVariable(node);
+        Selector selector = elements.getSelector(identifier);
+
+        HInstruction value = pop();
+        if (identifier.asSend() != null
+            && Elements.isInstanceSend(identifier, elements)) {
+          HInstruction receiver = generateInstanceSendReceiver(identifier);
+          assert(receiver != null);
+          generateInstanceSetterWithCompiledReceiver(
+              null,
+              receiver,
+              value,
+              selector: selector,
+              location: identifier);
+        } else {
+          generateNonInstanceSetter(
+              null, variable, value, location: identifier);
+        }
+        pop(); // Pop the value pushed by the setter call.
+
+        visit(node.body);
+      }
+      buildProtected(() {
+        handleLoop(node, buildInitializer, buildCondition, () {}, buildBody);

[floitsch, 2015/02/02 22:00:10]

Since you already give a name to the buildInitializer, might as well do the same
for the updater.
Alternatively assign them to local variables, so that they have a name:

Function buildInitializer = () {};
Function buildUpdate = () {};
handleLoop(...);

[sigurdm, 2015/02/03 16:59:31]

Done.

+      }, () {
+        pushInvokeDynamic(node, new Selector.call("cancel", null, 0),
+            [streamIterator]);
+        push(new HAwait(pop(), new TypeMask.subclass(compiler.objectClass,
+            compiler.world)));
+        pop();
+      });
+      return;
+    }
+
     // Generate a structure equivalent to:
     //   Iterator<E> $iter = <iterable>.iterator;
     //   while ($iter.moveNext()) {
@@ -5851,6 +5950,97 @@ class SsaBuilder extends ResolvedVisitor {
     compiler.internalError(node, 'SsaFromAstMixin.visitCaseMatch.');
   }
 
+  buildProtected(void buildTry(), void buildFinally()) {

[floitsch, 2015/02/02 22:00:11]

return type.

Change name, since it's not a general "buildProtected".

[sigurdm, 2015/02/03 16:59:32]

Done.

[floitsch, 2015/02/04 12:31:27]

That's not what I meant.
If the buildProtected works in the general case, then keep the name. I was under
the impression that it is specialized for the buildAsyncForIn. (It definitely
would be if you don't clone the locals).

+    // Save the current locals. The catch block and the finally block
+    // must not reuse the existing locals handler. None of the variables
+    // that have been defined in the body-block will be used, but for
+    // loops we will add (unnecessary) phis that will reference the body
+    // variables. This makes it look as if the variables were used
+    // in a non-dominated block.

[floitsch, 2015/02/02 22:00:11]

Does this comment apply for us?

Do we really need to clone the localshandler?

[sigurdm, 2015/02/03 16:59:32]

Not sure - I will ask

[sigurdm, 2015/02/04 11:43:39]

I discussed with Karl, and you are right - it is only needed for the
catch-block. I will remove it.

[floitsch, 2015/02/04 12:31:27]

I would say no, since the only variable that is used in the finally is the
iterator, and that one is finally. It might be safer to do a clone, though.

+    LocalsHandler savedLocals = new LocalsHandler.from(localsHandler);
+    HBasicBlock enterBlock = openNewBlock();
+    HTry tryInstruction = new HTry();
+    close(tryInstruction);
+    bool oldInTryStatement = inTryStatement;
+    inTryStatement = true;
+
+    HBasicBlock startTryBlock;
+    HBasicBlock endTryBlock;
+    HBasicBlock startFinallyBlock;
+    HBasicBlock endFinallyBlock;
+
+    startTryBlock = graph.addNewBlock();
+    open(startTryBlock);
+    buildTry();
+    // We use a [HExitTry] instead of a [HGoto] for the try block
+    // because it will have multiple successors: the join block, and
+    // the catch or finally block.

[floitsch, 2015/02/02 22:00:10]

It won't have any catch block.

[sigurdm, 2015/02/03 16:59:31]

Done.

+    if (!isAborted()) endTryBlock = close(new HExitTry());
+    SubGraph bodyGraph = new SubGraph(startTryBlock, lastOpenedBlock);
+    SubGraph catchGraph = null;

[floitsch, 2015/02/02 22:00:11]

Remove variable.

[sigurdm, 2015/02/03 16:59:31]

Done.

+    HLocalValue exception = null;

[floitsch, 2015/02/02 22:00:11]

Remove (and inline) ?

[sigurdm, 2015/02/03 16:59:32]

Done.

+
+    SubGraph finallyGraph = null;
+
+    localsHandler = new LocalsHandler.from(savedLocals);
+    startFinallyBlock = graph.addNewBlock();
+    open(startFinallyBlock);
+    buildFinally();
+    if (!isAborted()) endFinallyBlock = close(new HGoto());
+    tryInstruction.finallyBlock = startFinallyBlock;
+    finallyGraph = new SubGraph(startFinallyBlock, lastOpenedBlock);
+
+    HBasicBlock exitBlock = graph.addNewBlock();
+
+    addExitTrySuccessor(successor) {

[floitsch, 2015/02/02 22:00:10]

add return type. (especially for nested functions).
(also missing type for argument).

[sigurdm, 2015/02/03 16:59:31]

Done.

+      if (successor == null) return;

[floitsch, 2015/02/02 22:00:10]

Check should be unnecessary.

[sigurdm, 2015/02/03 16:59:31]

Done.

+      // Iterate over all blocks created inside this try/catch, and
+      // attach successor information to blocks that end with
+      // [HExitTry].
+      for (int i = startTryBlock.id; i < successor.id; i++) {
+        HBasicBlock block = graph.blocks[i];
+        var last = block.last;
+        if (last is HExitTry) {
+          block.addSuccessor(successor);
+        }
+      }
+    }
+
+    // Setup all successors. The entry block that contains the [HTry]
+    // has 1) the body, 2) the catch, 3) the finally, and 4) the exit

[floitsch, 2015/02/02 22:00:11]

no catch.

[sigurdm, 2015/02/03 16:59:31]

Done.

+    // blocks as successors.
+    enterBlock.addSuccessor(startTryBlock);
+    enterBlock.addSuccessor(startFinallyBlock);
+    enterBlock.addSuccessor(exitBlock);
+
+    // The body has either the catch or the finally block as successor.

[floitsch, 2015/02/02 22:00:10]

no catch.

[sigurdm, 2015/02/03 16:59:31]

Done.

+    if (endTryBlock != null) {
+      endTryBlock.addSuccessor(startFinallyBlock);
+      endTryBlock.addSuccessor(exitBlock);
+    }
+
+    // The finally block has the exit block as successor.
+    endFinallyBlock.addSuccessor(exitBlock);
+
+    // If a block inside try/catch aborts (eg with a return statement),
+    // we explicitely mark this block a predecessor of the catch
+    // block and the finally block.
+    addExitTrySuccessor(startFinallyBlock);
+
+    // Use the locals handler not altered by the catch and finally
+    // blocks.
+    localsHandler = savedLocals;
+    open(exitBlock);
+    enterBlock.setBlockFlow(
+        new HTryBlockInformation(
+          wrapStatementGraph(bodyGraph),
+          exception,
+          null, // No catchGraph
+          wrapStatementGraph(finallyGraph)),
+        exitBlock);
+    inTryStatement = oldInTryStatement;
+  }
+
   visitTryStatement(ast.TryStatement node) {
     // Save the current locals. The catch block and the finally block
     // must not reuse the existing locals handler. None of the variables
@@ -6045,6 +6235,7 @@ class SsaBuilder extends ResolvedVisitor {
     inTryStatement = oldInTryStatement;
   }
 
+
   visitCatchBlock(ast.CatchBlock node) {
     visit(node.block);
   }
@@ -6220,6 +6411,7 @@ class InlineWeeder extends ast.Visitor {
                            {bool allowLoops: false}) {
     InlineWeeder weeder =
         new InlineWeeder(maxInliningNodes, useMaxInliningNodes, allowLoops);
+    if (functionExpression.asyncModifier != null) return false;
     weeder.visit(functionExpression.initializers);
     weeder.visit(functionExpression.body);
     return !weeder.tooDifficult;
@@ -6306,6 +6498,11 @@ class InlineWeeder extends ast.Visitor {
       node.visitChildren(this);
     }
   }
+
+  void visitAwait(ast.Await node) {
+    if (!registerNode()) return;
+    tooDifficult = true;

[floitsch, 2015/02/02 22:00:10]

How can this happen, when we already return if there is an async-modifier?

[sigurdm, 2015/02/03 16:59:32]

Right - I added this one first, then realized that it was not enough. But now it
is redundant.

+  }
 }
 
 abstract class InliningState {