Tag closures with their types

lib/src/codegen/js_codegen.dart

Index: lib/src/codegen/js_codegen.dart
diff --git a/lib/src/codegen/js_codegen.dart b/lib/src/codegen/js_codegen.dart
index 8af06b9dfb02481a6e62f779bc649adf98c9852c..5b30959dd9b56ba8920733394a18378f6b45e1da 100644
--- a/lib/src/codegen/js_codegen.dart
+++ b/lib/src/codegen/js_codegen.dart
@@ -335,11 +335,14 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
 
     var ctors = <ConstructorDeclaration>[];
     var fields = <FieldDeclaration>[];
+    var methods = <MethodDeclaration>[];
     for (var member in node.members) {
       if (member is ConstructorDeclaration) {
         ctors.add(member);
       } else if (member is FieldDeclaration && !member.isStatic) {
         fields.add(member);
+      } else if (member is MethodDeclaration) {
+        methods.add(member);
       }
     }
 
@@ -352,8 +355,8 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
       jsPeerName = getConstantField(jsPeer, 'name', types.stringType);
     }
 
-    var body =
-        _finishClassMembers(classElem, classExpr, ctors, fields, jsPeerName);
+    var body = _finishClassMembers(
+        classElem, classExpr, ctors, fields, methods, jsPeerName);
 
     var result = _finishClassDef(type, body);
 
@@ -536,7 +539,8 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
   /// inside the ES6 `class { ... }` node.
   JS.Statement _finishClassMembers(ClassElement classElem,
       JS.ClassExpression cls, List<ConstructorDeclaration> ctors,
-      List<FieldDeclaration> fields, String jsPeerName) {
+      List<FieldDeclaration> fields, List<MethodDeclaration> methods,
+      String jsPeerName) {
     var name = classElem.name;
     var body = <JS.Statement>[];
     body.add(new JS.ClassDeclaration(cls));
@@ -578,6 +582,48 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
       }
     }
 
+    {

[vsm, 2015/05/18 17:35:10]

Add comment to this block ... e.g, // Emit signatures

[Leaf, 2015/05/19 00:02:21]

Done.

+      var tStatics = [];
+      var tMethods = [];
+      var sNames = [];
+      var cType = classElem.type;
+      for (MethodDeclaration node in methods) {
+        if (!(node.isSetter || node.isGetter || node.isAbstract)) {
+          var name = node.name.name;
+          var element = node.element;
+          var unary = node.parameters.parameters.isEmpty;
+          var memberName = _emitMemberName(name,
+              type: cType, unary: unary, isStatic: node.isStatic);
+          var property =
+              new JS.Property(memberName, _emitTypeName(element.type));
+          if (node.isStatic) {
+            tStatics.add(property);
+            sNames.add(memberName);
+          } else tMethods.add(property);
+        }
+      }
+      build(name, elements) {
+        var o =
+            new JS.ObjectInitializer(elements, vertical: elements.length > 1);
+        var e = js.call('() => #', o);
+        var p = new JS.Property(_propertyName(name), e);
+        return p;
+      }
+      var sigFields = [];
+      if (!tMethods.isEmpty) sigFields.add(build('methods', tMethods));
+      if (!tStatics.isEmpty) {
+        assert(!sNames.isEmpty);
+        var aNames = new JS.Property(
+            _propertyName('names'), new JS.ArrayInitializer(sNames));
+        sigFields.add(build('statics', tStatics));
+        sigFields.add(aNames);
+      }
+
+      var sig = new JS.ObjectInitializer(sigFields);
+      var classExpr = new JS.Identifier(name);
+      body.add(js.statement('dart.setSignature(#, #);', [classExpr, sig]));
+    }
+
     return _statement(body);
   }
 
@@ -915,10 +961,11 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
     _flushLibraryProperties(body);
 
     var name = node.name.name;
-    body.add(js.comment('Function $name: ${node.element.type}'));
 
-    body.add(new JS.FunctionDeclaration(
-        new JS.Identifier(name), _visit(node.functionExpression)));
+    var id = new JS.Identifier(name);
+    body.add(new JS.FunctionDeclaration(id, _visit(node.functionExpression)));
+    body.add(_emitFunctionTagged(id, node.element.type, topLevel: true)
+        .toStatement());
 
     if (isPublic(name)) _addExport(name);
     return _statement(body);
@@ -930,6 +977,44 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
         isGetter: node.isGetter, isSetter: node.isSetter);
   }
 
+  bool _executesAtTopLevel(AstNode node) {
+    var ancestor = node.getAncestor((n) => n is FunctionBody ||
+        (n is FieldDeclaration && n.staticKeyword == null) ||
+        (n is ConstructorDeclaration && n.constKeyword == null));
+    return ancestor == null;
+  }
+
+  bool _typeIsLoaded(DartType type) {
+    if (type is FunctionType && (type.name == '' || type.name == null)) {
+      return (_typeIsLoaded(type.returnType) &&
+          type.optionalParameterTypes.every(_typeIsLoaded) &&
+          type.namedParameterTypes.values.every(_typeIsLoaded) &&
+          type.normalParameterTypes.every(_typeIsLoaded));
+    }
+    if (type.isDynamic || type.isVoid || type.isBottom) return true;
+    return _loader.isLoaded(type.element);
+  }
+
+  JS.Expression _emitFunctionTagged(JS.Expression clos, DartType type,
+      {topLevel: false}) {
+    var name = type.name;
+    var lazy = topLevel && !_typeIsLoaded(type);

[Jennifer Messerly, 2015/05/19 18:19:01]

Definitely not something that needs to be changed for this CL, but...

The goal of Loader was to avoid duplication like this, where you need to capture
the same dependencies twice, once in a boolean expression (_typeIsLoaded), and
again when emitting (_emitFunctionTypeParts).

An alternative design would remove this function, and have it work more like:

    if (topLevel) _loader.startTopLevel();
    ... emit stuff ...
    bool lazy = topLevel && _loader.finishTopLevel();

That won't work right now, because startTopLevel requires an argument, which is
what Element we're emitting. there's no notion of "anonymous" top level code.

Interestingly, this suggests another design. Right now, if we want to make
something lazy, it's handled here. But instead, we could just say the whole
top-level element needs to be lazy. That way, if the element ends up lazy for
some other reason, we won't add a nested lazy here too. The alternative design
would then be:

    var e = _loader.currentElement;
    if (topLevel) _loader.startTopLevel(e);
    ... emit stuff ...
    if (topLevel) _loader.finishTopLevel(e);

And then the existing laziness logic for the "currentElement" will handle it.
e.g. a field will become a lazy property.
(note: looking at it now, it seems start/finishTopLevel don't really need the
element. They assert that it's the current element anyway.)

[Leaf, 2015/05/19 22:31:12]

Acknowledged.

+
+    if (type is FunctionType && (name == '' || name == null)) {
+      if (type.returnType.isDynamic &&
+          type.optionalParameterTypes.isEmpty &&
+          type.namedParameterTypes.isEmpty &&
+          type.normalParameterTypes.every((t) => t.isDynamic)) {
+        return js.call('dart.fn(#)', [clos]);
+      }
+      if (lazy) {
+        return js.call('dart.fn(#, () => #)', [clos, _emitTypeName(type)]);
+      }
+      return js.call('dart.fn(#, #)', [clos, _emitFunctionTypeParts(type)]);
+    }
+    throw 'Function has non function type: $type';
+  }
+
   @override
   JS.Expression visitFunctionExpression(FunctionExpression node) {
     var params = _visit(node.parameters);
@@ -950,7 +1035,13 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
         code = '(#) => { #; }';
         body = nodeBody;
       }
-      return js.call(code, [params, _visit(body)]);
+      var clos = js.call(code, [params, _visit(body)]);
+      if (parent.parent is! FunctionDeclarationStatement) {

[Jennifer Messerly, 2015/05/19 18:19:01]

maybe pull out `parent.parent is! FunctionDeclarationStatement` into a local
since we use it twice?


var inFunctionStatement = parent.parent is FunctionDeclarationStatement;
...

if (!inFunctionStatement) { ... }

sort of related
https://github.com/dart-lang/dev_compiler/issues/143#issuecomment-103614170

[Leaf, 2015/05/19 22:31:13]

Done.

+        var type = getStaticType(node);
+        return _emitFunctionTagged(clos, type,
+            topLevel: _executesAtTopLevel(node));
+      }
+      return clos;
     }
   }
 
@@ -967,8 +1058,8 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
     // `this`, but it seems harmless enough to just do it always.
     var name = new JS.Identifier(func.name.name);
     return new JS.Block([
-      js.comment("// Function ${func.name.name}: ${func.element.type}\n"),
-      js.statement('let # = #;', [name, _visit(func.functionExpression)])
+      js.statement('let # = #;', [name, _visit(func.functionExpression)]),
+      _emitFunctionTagged(name, func.element.type).toStatement()
     ]);
   }
 
@@ -1059,6 +1150,26 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
     return new JS.ObjectInitializer(properties);
   }
 
+  List<JS.Expression> _emitFunctionTypeParts(FunctionType type) {
+    var returnType = type.returnType;
+    var parameterTypes = type.normalParameterTypes;
+    var optionalTypes = type.optionalParameterTypes;
+    var namedTypes = type.namedParameterTypes;
+    var rt = _emitTypeName(returnType);
+    var ra = _emitTypeNames(parameterTypes);
+    if (!namedTypes.isEmpty) {
+      assert(optionalTypes.isEmpty);
+      var na = _emitTypeProperties(namedTypes);
+      return [rt, ra, na];
+    }
+    if (!optionalTypes.isEmpty) {
+      assert(namedTypes.isEmpty);
+      var oa = _emitTypeNames(optionalTypes);
+      return [rt, ra, oa];
+    }
+    return [rt, ra];
+  }
+
   /// Emits a Dart [type] into code.
   ///
   /// If [lowerTypedef] is set, a typedef will be expanded as if it were a
@@ -1073,6 +1184,8 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
       return js.call('dart.void');
     } else if (type.isDynamic) {
       return js.call('dart.dynamic');
+    } else if (type.isBottom) {
+      return js.call('dart.bottom');
     }
 
     _loader.declareBeforeUse(type.element);
@@ -1082,33 +1195,9 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
     // to canonicalize them too, at least when inside the same library.
     var name = type.name;
     var element = type.element;
-    if (name == '' || lowerTypedef) {
-      var fnType = type as FunctionType;
-      var returnType = fnType.returnType;
-      var parameterTypes = fnType.normalParameterTypes;
-      var optionalTypes = fnType.optionalParameterTypes;
-      var namedTypes = fnType.namedParameterTypes;
-      if (namedTypes.isEmpty) {
-        if (optionalTypes.isEmpty) {
-          return js.call('dart.functionType(#, #)', [
-            _emitTypeName(returnType),
-            _emitTypeNames(parameterTypes)
-          ]);
-        } else {
-          return js.call('dart.functionType(#, #, #)', [
-            _emitTypeName(returnType),
-            _emitTypeNames(parameterTypes),
-            _emitTypeNames(optionalTypes)
-          ]);
-        }
-      } else {
-        assert(optionalTypes.isEmpty);
-        return js.call('dart.functionType(#, #, #)', [
-          _emitTypeName(returnType),
-          _emitTypeNames(parameterTypes),
-          _emitTypeProperties(namedTypes)
-        ]);
-      }
+    if (name == '' || name == null || lowerTypedef) {
+      var parts = _emitFunctionTypeParts(type as FunctionType);
+      return js.call('dart.functionType(#)', [parts]);
     }
 
     if (type is TypeParameterType) {