Handle closures, aka FunctionExpression in Kernel!

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

Index: pkg/compiler/lib/src/ssa/builder_kernel.dart
diff --git a/pkg/compiler/lib/src/ssa/builder_kernel.dart b/pkg/compiler/lib/src/ssa/builder_kernel.dart
index 6acd15dc3215aeb04e5482829cab9546b950ff30..f5f48d70dcbdab1d190f9ce7c14ad6cd75b37c98 100644
--- a/pkg/compiler/lib/src/ssa/builder_kernel.dart
+++ b/pkg/compiler/lib/src/ssa/builder_kernel.dart
@@ -4,6 +4,7 @@
 
 import 'package:kernel/ast.dart' as ir;
 
+import '../closure.dart';
 import '../common.dart';
 import '../common/codegen.dart' show CodegenRegistry, CodegenWorkItem;
 import '../common/names.dart';
@@ -114,6 +115,14 @@ class KernelSsaBuilder extends ir.Visitor with GraphBuilder {
     }
     if (originTarget is FunctionElement) {
       target = kernel.functions[originTarget];
+      // Closures require a lookup one level deeper in the closure class mapper.
+      if (target == null) {
+        ClosureClassMap classMap = compiler.closureToClassMapper
+            .getClosureToClassMapping(originTarget.resolvedAst);
+        if (classMap.closureElement != null) {
+          target = kernel.localFunctions[classMap.closureElement];
+        }
+      }
     } else if (originTarget is FieldElement) {
       target = kernel.fields[originTarget];
     }
@@ -128,6 +137,10 @@ class KernelSsaBuilder extends ir.Visitor with GraphBuilder {
       buildField(target);
     } else if (target is ir.Constructor) {
       buildConstructor(target);
+    } else if (target is ir.FunctionExpression) {
+      buildFuncExpr(target);
+    } else {
+      throw 'No case implemented to handle $target';
     }
     assert(graph.isValid());
     return graph;
@@ -341,6 +354,16 @@ class KernelSsaBuilder extends ir.Visitor with GraphBuilder {
     closeFunction();
   }
 
+  // TODO(efortuna): This function is identical to the one above. Waiting for
+  // response from Asger and Kevin on if we should implement a common interface
+  // between Procedure and FunctionExpression. Regardless, this function is
+  // definitely not permanent.
+  void buildFuncExpr(ir.FunctionExpression procedure) {
+    openFunction();
+    procedure.function.body.accept(this);
+    closeFunction();
+  }
+
   void addImplicitInstantiation(DartType type) {
     if (type != null) {
       currentImplicitInstantiations.add(type);
@@ -433,10 +456,19 @@ class KernelSsaBuilder extends ir.Visitor with GraphBuilder {
     if (returnStatement.expression == null) {
       value = graph.addConstantNull(compiler);
     } else {
-      assert(target is ir.Procedure);
+      assert(target is ir.Procedure || target is ir.FunctionExpression);
       returnStatement.expression.accept(this);
+      // More stupidity until we resolve the interface issue with
+      // FunctionExpressions and Procedures.

[sra1, 2016/12/06 02:35:41]

It might be possible to simply use ir.FunctionNode as the type of [target]. That
might have been the intention.

[Emily Fortuna, 2016/12/06 18:49:22]

Done.

+      // TODO(efortuna): Remove!
+      ir.FunctionNode functionNode;
+      if (target is ir.Procedure) {
+        functionNode = (target as ir.Procedure).function;
+      } else if (target is ir.FunctionExpression) {
+        functionNode = (target as ir.FunctionExpression).function;
+      }
       value = typeBuilder.potentiallyCheckOrTrustType(pop(),
-          astAdapter.getFunctionReturnType((target as ir.Procedure).function));
+          astAdapter.getFunctionReturnType(functionNode));
     }
     // TODO(het): Add source information
     // TODO(het): Set a return value instead of closing the function when we
@@ -1319,8 +1351,8 @@ class KernelSsaBuilder extends ir.Visitor with GraphBuilder {
     String isolateName = backend.namer.staticStateHolder;
     SideEffects sideEffects = new SideEffects.empty();
     sideEffects.setAllSideEffects();
-    push(new HForeignCode(js.js.parseForeignJS("$isolateName = #"),
-        backend.dynamicType, inputs,
+    push(new HForeignCode(
+        js.js.parseForeignJS("$isolateName = #"), backend.dynamicType, inputs,
         nativeBehavior: native.NativeBehavior.CHANGES_OTHER,
         effects: sideEffects));
   }
@@ -1562,6 +1594,37 @@ class KernelSsaBuilder extends ir.Visitor with GraphBuilder {
     }
   }
 
+  @override
+  void visitFunctionExpression(ir.FunctionExpression funcExpression) {
+    LocalFunctionElement methodElement = astAdapter.getElement(funcExpression);
+    ClosureClassMap nestedClosureData = compiler.closureToClassMapper
+        .getClosureToClassMapping(methodElement.resolvedAst);
+    assert(nestedClosureData != null);
+    assert(nestedClosureData.closureClassElement != null);
+    ClosureClassElement closureClassElement =
+        nestedClosureData.closureClassElement;
+    FunctionElement callElement = nestedClosureData.callElement;
+    // TODO(ahe): This should be registered in codegen, not here.
+    // TODO(johnniwinther): Is [registerStaticUse] equivalent to
+    // [addToWorkList]?
+    registry?.registerStaticUse(new StaticUse.foreignUse(callElement));

[Harry Terkelsen, 2016/12/06 18:46:36]

johnni is making some changes to the registry. I would add him to this CL to see
if this is still correct to do

[Emily Fortuna, 2016/12/06 18:49:22]

Acknowledged.

+
+    List<HInstruction> capturedVariables = <HInstruction>[];
+    closureClassElement.closureFields.forEach((ClosureFieldElement field) {
+      Local capturedLocal =
+          nestedClosureData.getLocalVariableForClosureField(field);
+      assert(capturedLocal != null);
+      capturedVariables.add(localsHandler.readLocal(capturedLocal));
+    });
+
+    TypeMask type =
+        new TypeMask.nonNullExact(closureClassElement, compiler.closedWorld);
+    // TODO(efortuna): Add source information here.
+    push(new HCreate(closureClassElement, capturedVariables, type));
+
+    registry?.registerInstantiatedClosure(methodElement);
+  }
+
   // TODO(het): Decide when to inline
   @override
   void visitMethodInvocation(ir.MethodInvocation invocation) {