Name and hoist types

lib/src/compiler/code_generator.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.
 
 import 'dart:collection' show HashMap, HashSet;
 import 'dart:math' show min, max;
 
 import 'package:analyzer/analyzer.dart' hide ConstantEvaluator;
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/token.dart' show Token, TokenType;
@@ skipping 48 matching lines @@
   /// We sometimes special case codegen for a single library, as it simplifies
   /// name scoping requirements.
   final _libraries = new Map<LibraryElement, JS.Identifier>();
 
   /// Imported libraries, and the temporaries used to refer to them.
   final _imports = new Map<LibraryElement, JS.TemporaryId>();
 
   /// The list of output module items, in the order they need to be emitted in.
   final _moduleItems = <JS.ModuleItem>[];
 
+  /// Table of named and possibly hoisted types.
+  final _typeTable = new _TypeTable();
+
   /// The global extension type table.
   final ExtensionTypeSet _extensionTypes;
 
   /// The variable for the target of the current `..` cascade expression.
   ///
   /// Usually a [SimpleIdentifier], but it can also be other expressions
   /// that are safe to evaluate multiple times, such as `this`.
   Expression _cascadeTarget;
 
   /// The variable for the current catch clause
@@ skipping 166 matching lines @@
     // Visit each compilation unit and emit its code.
     //
     // NOTE: declarations are not necessarily emitted in this order.
     // Order will be changed as needed so the resulting code can execute.
     // This is done by forward declaring items.
     compilationUnits.forEach(visitCompilationUnit);
 
     // Declare imports
     _finishImports(items);
 
+    // Discharge the type table cache variables and
+    // hoisted definitions.
+    items.addAll(_typeTable.discharge());
+
     // Add the module's code (produced by visiting compilation units, above)
     _copyAndFlattenBlocks(items, _moduleItems);
 
     // Build the module.
     var module = new JS.Program(items, name: _buildUnit.name);
 
     // Optional: lower module format. Otherwise just return it.
     switch (options.moduleFormat) {
       case ModuleFormat.legacy:
         return new LegacyModuleBuilder().build(module);
@@ skipping 296 matching lines @@
     return null;
   }
 
   @override
   visitFunctionTypeAlias(FunctionTypeAlias node) {
     FunctionTypeAliasElement element = node.element;
 
     JS.Expression body = annotate(
         js.call('dart.typedef(#, () => #)', [
           js.string(element.name, "'"),
-          _emitType(element.type, lowerTypedef: true)
+          _emitType(element.type, nameType: false, lowerTypedef: true)
         ]),
         node,
         element);
 
     var typeFormals = element.typeParameters;
     if (typeFormals.isNotEmpty) {
       return _defineClassTypeArguments(element, typeFormals,
           js.statement('const # = #;', [element.name, body]));
     } else {
       return js.statement('# = #;', [_emitTopLevelName(element), body]);
@@ skipping 251 matching lines @@
     result.add(js.statement(
         '#.values = dart.constList(#, #);', [id, values, _emitType(type)]));
 
     return _statement(result);
   }
 
   /// Wraps a possibly generic class in its type arguments.
   JS.Statement _defineClassTypeArguments(TypeDefiningElement element,
       List<TypeParameterElement> formals, JS.Statement body) {
     assert(formals.isNotEmpty);
-    var genericCall = js.call('dart.generic((#) => { #; return #; })',
-        [_emitTypeFormals(formals), body, element.name]);
+    var genericCall = js.call('dart.generic((#) => { #; #; return #; })', [
+      _emitTypeFormals(formals),
+      _typeTable.discharge(formals),
+      body,
+      element.name
+    ]);
     if (element.library.isDartAsync &&
         (element.name == "Future" || element.name == "_Future")) {
       genericCall = js.call('dart.flattenFutures(#)', [genericCall]);
     }
     var genericDef = js.statement(
         '# = #;', [_emitTopLevelName(element, suffix: r'$'), genericCall]);
     var dynType = fillDynamicTypeArgs(element.type);
     var genericInst = _emitType(dynType, lowerGeneric: true);
     return js.statement(
         '{ #; # = #; }', [genericDef, _emitTopLevelName(element), genericInst]);
@@ skipping 32 matching lines @@
     _loader.startTopLevel(element);
 
     // Find the super type
     JS.Expression heritage;
     var supertype = type.superclass;
     if (_deferIfNeeded(supertype, element)) {
       // Fall back to raw type.
       supertype = fillDynamicTypeArgs(supertype.element.type);
       _hasDeferredSupertype.add(element);
     }
-    heritage = _emitType(supertype);
+    // We could choose to name the superclasses, but it's
+    // not clear that there's much benefit
+    heritage = _emitType(supertype, nameType: false);
 
     if (type.mixins.isNotEmpty) {
-      var mixins = type.mixins.map(_emitType).toList();
+      var mixins =
+          type.mixins.map((t) => _emitType(t, nameType: false)).toList();
       mixins.insert(0, heritage);
       heritage = js.call('dart.mixin(#)', [mixins]);
     }
 
     _loader.finishTopLevel(element);
 
     return heritage;
   }
 
   /// Provide Dart getters and setters that forward to the underlying native
@@ skipping 241 matching lines @@
 
   void _setBaseClass(ClassElement classElem, JS.Expression className,
       String jsPeerName, List<JS.Statement> body) {
     if (jsPeerName != null && classElem.typeParameters.isNotEmpty) {
       // TODO(jmesserly): we should really just extend Array in the first place.
       var newBaseClass = js.call('dart.global.#', [jsPeerName]);
       body.add(js.statement(
           'dart.setExtensionBaseClass(#, #);', [className, newBaseClass]));
     } else if (_hasDeferredSupertype.contains(classElem)) {
       var newBaseClass = _emitType(classElem.type.superclass,
-          subClass: classElem, className: className);
+          nameType: false, subClass: classElem, className: className);
       body.add(
           js.statement('dart.setBaseClass(#, #);', [className, newBaseClass]));
     }
   }
 
   void _defineNamedConstructors(List<ConstructorDeclaration> ctors,
       List<JS.Statement> body, JS.Expression className) {
     for (ConstructorDeclaration member in ctors) {
       if (member.name != null && member.factoryKeyword == null) {
         body.add(js.statement('dart.defineNamedConstructor(#, #);',
@@ skipping 79 matching lines @@
       if (!(node.isSetter || node.isGetter || node.isAbstract)) {
         var name = node.name.name;
         var element = node.element;
         var inheritedElement =
             classElem.lookUpInheritedConcreteMethod(name, currentLibrary);
         if (inheritedElement != null && inheritedElement.type == element.type) {
           continue;
         }
         var memberName = _elementMemberName(element,
             useExtension: _extensionTypes.isNativeClass(classElem));
-        var parts = _emitFunctionTypeParts(element.type);
-        var property =
-            new JS.Property(memberName, new JS.ArrayInitializer(parts));
+        // TODO(leafp):  Settle on a policy for this.  Naming these makes the
+        // signature more compact, but it generates a lot of noise at the
+        // top level, and it's not clear that there's much benefit.
+        var type =
+            _emitFunctionType(element.type, nameType: false, definite: true);
+        var property = new JS.Property(memberName, type);
         if (node.isStatic) {
           tStatics.add(property);
           sNames.add(memberName);
         } else {
           tMethods.add(property);
         }
       }
     }
 
     var tCtors = <JS.Property>[];
     for (ConstructorDeclaration node in ctors) {
       var memberName = _constructorName(node.element);
       var element = node.element;
-      var parts = _emitFunctionTypeParts(element.type,
-          parameters: node.parameters.parameters);
-      var property =
-          new JS.Property(memberName, new JS.ArrayInitializer(parts));
+      var type = _emitFunctionType(element.type,
+          parameters: node.parameters.parameters,
+          nameType: false,
+          definite: true);
+      var property = new JS.Property(memberName, type);
       tCtors.add(property);
     }
 
     JS.Property build(String name, List<JS.Property> elements) {
       var o =
           new JS.ObjectInitializer(elements, multiline: elements.length > 1);
       var e = js.call('() => #', o);
       return new JS.Property(_propertyName(name), e);
     }
     var sigFields = <JS.Property>[];
@@ skipping 619 matching lines @@
     }
     if (type.isDynamic || type.isVoid || type.isBottom) return true;
     if (type is ParameterizedType && !type.typeArguments.every(_typeIsLoaded)) {
       return false;
     }
     return _loader.isLoaded(type.element);
   }
 
   JS.Expression _emitFunctionTagged(JS.Expression fn, DartType type,
       {topLevel: false}) {
-    var name = type.name;
     var lazy = topLevel && !_typeIsLoaded(type);
-
-    if (type is FunctionType && (name == '' || name == null)) {
-      if (type.typeFormals.isEmpty &&
-          type.returnType.isDynamic &&
-          type.optionalParameterTypes.isEmpty &&
-          type.namedParameterTypes.isEmpty &&
-          type.normalParameterTypes.every((t) => t.isDynamic)) {
-        return js.call('dart.fn(#)', [fn]);
-      }
-
-      var parts = _emitFunctionTypeParts(type);
-      if (lazy) {
-        return js.call(
-            'dart.lazyFn(#, () => #)', [fn, new JS.ArrayInitializer(parts)]);
-      } else {
-        return js.call('dart.fn(#, #)', [fn, parts]);
-      }
+    var typeRep = _emitFunctionType(type, definite: true);
+    if (lazy) {
+      return js.call('dart.lazyFn(#, () => #)', [fn, typeRep]);
+    } else {
+      return js.call('dart.fn(#, #)', [fn, typeRep]);
     }
     throw 'Function has non function type: $type';
   }
 
   /// Emits an arrow FunctionExpression node.
   ///
   /// This should be used for all places in Dart's AST where FunctionExpression
   /// appears and the function is actually in an Expression context. These
   /// correspond to arrow functions in Dart.
   ///
@@ skipping 55 matching lines @@
   /// Contrast with [visitFunctionExpression].
   JS.Fun _emitFunction(FunctionExpression node) {
     var fn = _emitFunctionBody(node.element, node.parameters, node.body);
     return annotate(_makeGenericFunction(fn), node);
   }
 
   JS.Fun _emitFunctionBody(ExecutableElement element,
       FormalParameterList parameters, FunctionBody body) {
     FunctionType type = element.type;
 
-    // sync*, async, async*
-    if (element.isAsynchronous || element.isGenerator) {
-      return new JS.Fun(
-          visitFormalParameterList(parameters, destructure: false),
-          new JS.Block([
-            _emitGeneratorFunctionBody(element, parameters, body).toReturn()
-          ]),
-          typeParams: _emitTypeFormals(type.typeFormals),
-          returnType: emitTypeRef(type.returnType));
+    // normal function (sync), vs (sync*, async, async*)
+    var stdFn = !(element.isAsynchronous || element.isGenerator);
+    var formals = visitFormalParameterList(parameters, destructure: stdFn);
+    var code = (stdFn)
+        ? _visit(body)
+        : new JS.Block(
+            [_emitGeneratorFunctionBody(element, parameters, body).toReturn()]);
+    var typeFormals = _emitTypeFormals(type.typeFormals);
+    var returnType = emitTypeRef(type.returnType);
+    if (type.typeFormals.isNotEmpty) {
+      code = new JS.Block(
+          [new JS.Block(_typeTable.discharge(type.typeFormals)), code]);
     }
-
-    // normal function (sync)
-    return new JS.Fun(visitFormalParameterList(parameters), _visit(body),
-        typeParams: _emitTypeFormals(type.typeFormals),
-        returnType: emitTypeRef(type.returnType));
+    return new JS.Fun(formals, code,
+        typeParams: typeFormals, returnType: returnType);
   }
 
   JS.Expression _emitGeneratorFunctionBody(ExecutableElement element,
       FormalParameterList parameters, FunctionBody body) {
     var kind = element.isSynchronous ? 'sync' : 'async';
     if (element.isGenerator) kind += 'Star';
 
     // Transforms `sync*` `async` and `async*` function bodies
     // using ES6 generators.
     //
@@ skipping 202 matching lines @@
 
     var type = declaration ? emitTypeRef(element.type) : null;
     return new JS.Identifier(element.name, type: type);
   }
 
   List<Annotation> _parameterMetadata(FormalParameter p) =>
       (p is NormalFormalParameter)
           ? p.metadata
           : (p as DefaultFormalParameter).parameter.metadata;
 
-  JS.ArrayInitializer _emitTypeNames(List<DartType> types,
-      [List<FormalParameter> parameters]) {
+  JS.ArrayInitializer _emitTypeNames(
+      List<DartType> types, List<FormalParameter> parameters,
+      {bool nameType: true, bool hoistType: true}) {
     var result = <JS.Expression>[];
     for (int i = 0; i < types.length; ++i) {
       var metadata =
           parameters != null ? _parameterMetadata(parameters[i]) : [];
-      var typeName = _emitType(types[i]);
+      var typeName =
+          _emitType(types[i], nameType: nameType, hoistType: hoistType);
       var value = typeName;
       if (options.emitMetadata && metadata.isNotEmpty) {
         metadata = metadata.map(_instantiateAnnotation).toList();
         value = new JS.ArrayInitializer([typeName]..addAll(metadata));
       }
       result.add(value);
     }
     return new JS.ArrayInitializer(result);
   }
 
   JS.ObjectInitializer _emitTypeProperties(Map<String, DartType> types) {
     var properties = <JS.Property>[];
     types.forEach((name, type) {
       var key = _propertyName(name);
       var value = _emitType(type);
       properties.add(new JS.Property(key, value));
     });
     return new JS.ObjectInitializer(properties);
   }
 
   /// Emit the pieces of a function type, as an array of return type,
   /// regular args, and optional/named args.
+  JS.Expression _emitFunctionType(FunctionType type,
+      {List<FormalParameter> parameters,
+      bool lowerTypedef: false,
+      bool nameType: true,
+      bool hoistType: true,

[Jennifer Messerly, 2016/05/24 21:56:39]

do we ever set this to false? I wasn't able to find it, seems like we always let
the default value kick in or just pass it along.

[Leaf, 2016/05/25 00:56:13]

It was here so that we can play with different strategies.  Added flags so its
controllable from the command line, plus test.

+      definite: false}) {
+    var parts = _emitFunctionTypeParts(type,
+        parameters: parameters,
+        lowerTypedef: lowerTypedef,
+        nameType: nameType,
+        hoistType: hoistType);
+    var helper = (definite) ? 'definiteFunctionType' : 'functionType';
+    var fullType = js.call('dart.${helper}(#)', [parts]);
+    if (!nameType) return fullType;
+    return _typeTable.nameType(type, fullType, hoistType, definite: definite);
+  }
+
+  /// Emit the pieces of a function type, as an array of return type,
+  /// regular args, and optional/named args.
   List<JS.Expression> _emitFunctionTypeParts(FunctionType type,
-      {List<FormalParameter> parameters, bool lowerTypedef: false}) {
+      {List<FormalParameter> parameters,
+      bool lowerTypedef: false,
+      bool nameType: true,
+      bool hoistType: true}) {
     var parameterTypes = type.normalParameterTypes;
     var optionalTypes = type.optionalParameterTypes;
     var namedTypes = type.namedParameterTypes;
-    var rt = _emitType(type.returnType);
-    var ra = _emitTypeNames(parameterTypes, parameters);
+    var rt =
+        _emitType(type.returnType, nameType: nameType, hoistType: hoistType);
+    var ra = _emitTypeNames(parameterTypes, parameters,
+        nameType: nameType, hoistType: hoistType);
 
     List<JS.Expression> typeParts;
     if (namedTypes.isNotEmpty) {
       assert(optionalTypes.isEmpty);
       // TODO(vsm): Pass in annotations here as well.
       var na = _emitTypeProperties(namedTypes);
       typeParts = [rt, ra, na];
     } else if (optionalTypes.isNotEmpty) {
       assert(namedTypes.isEmpty);
       var oa = _emitTypeNames(
-          optionalTypes, parameters?.sublist(parameterTypes.length));
+          optionalTypes, parameters?.sublist(parameterTypes.length),
+          nameType: nameType, hoistType: hoistType);
       typeParts = [rt, ra, oa];
     } else {
       typeParts = [rt, ra];
     }
 
     var typeFormals = type.typeFormals;
     if (typeFormals.isNotEmpty && !lowerTypedef) {
       // TODO(jmesserly): this is a suboptimal representation for universal
       // function types (as callable functions). See discussion at:
       // https://github.com/dart-lang/dev_compiler/issues/526
       var tf = _emitTypeFormals(typeFormals);
-      typeParts = [new JS.ArrowFun(tf, new JS.ArrayInitializer(typeParts))];
+      var names = _typeTable.discharge(typeFormals);
+      var parts = new JS.ArrayInitializer(typeParts);
+      if (names.isEmpty) {
+        typeParts = [
+          js.call('(#) => #', [tf, parts])
+        ];
+      } else {
+        typeParts = [
+          js.call('(#) => {#; return #;}', [tf, names, parts])
+        ];
+      }
     }
     return typeParts;
   }
 
   /// Emits a Dart [type] into code.
   ///
   /// If [lowerTypedef] is set, a typedef will be expanded as if it were a
   /// function type. Similarly if [lowerGeneric] is set, the `List$()` form
   /// will be used instead of `List`. These flags are used when generating
   /// the definitions for typedefs and generic types, respectively.
   ///
   /// If [subClass] is set, then we are setting the base class for the given
   /// class and should emit the given [className], which will already be
   /// defined.
   JS.Expression _emitType(DartType type,
       {bool lowerTypedef: false,
       bool lowerGeneric: false,
+      bool nameType: true,
+      bool hoistType: true,
       ClassElement subClass,
       JS.Expression className}) {
     // The void and dynamic types are not defined in core.
     if (type.isVoid) {
       return js.call('dart.void');
     } else if (type.isDynamic) {
       return js.call('dart.dynamic');
     } else if (type.isBottom) {
       return js.call('dart.bottom');
     }
 
     _declareBeforeUse(type.element);
 
     // TODO(jmesserly): like constants, should we hoist function types out of
     // methods? Similar issue with generic types. For all of these, we may want
     // to canonicalize them too, at least when inside the same library.
     var name = type.name;
     var element = type.element;
     if (name == '' || name == null || lowerTypedef) {
       // TODO(jmesserly): should we change how typedefs work? They currently
       // go through use similar logic as generic classes. This makes them
       // different from universal function types.
-      var ft = type as FunctionType;
-      var parts = _emitFunctionTypeParts(ft, lowerTypedef: lowerTypedef);
-      return js.call('dart.functionType(#)', [parts]);
+      return _emitFunctionType(type as FunctionType,
+          lowerTypedef: lowerTypedef, nameType: nameType, hoistType: hoistType);
     }
 
     if (type is TypeParameterType) {
       _typeParamInConst?.add(type);
       return new JS.Identifier(name);
     }
 
     if (type == subClass?.type) {
       return className;
     }
 
     if (type is ParameterizedType) {
       var args = type.typeArguments;
       Iterable jsArgs = null;
       if (args.any((a) => !a.isDynamic)) {
-        jsArgs = args
-            .map((x) => _emitType(x, subClass: subClass, className: className));
+        jsArgs = args.map((x) => _emitType(x,
+            nameType: nameType,
+            hoistType: hoistType,
+            subClass: subClass,
+            className: className));
       } else if (lowerGeneric) {
         jsArgs = [];
       }
       if (jsArgs != null) {
         var genericName = _emitTopLevelName(element, suffix: '\$');
-        return js.call('#(#)', [genericName, jsArgs]);
+        var typeRep = js.call('#(#)', [genericName, jsArgs]);
+        return nameType
+            ? _typeTable.nameType(type, typeRep, hoistType)
+            : typeRep;
       }
     }
 
     return _emitTopLevelName(element);
   }
 
   JS.PropertyAccess _emitTopLevelName(Element e, {String suffix: ''}) {
     var interop = _emitJSInterop(e);
     if (interop != null) return interop;
     String name = getJSExportName(e) + suffix;
@@ skipping 2172 matching lines @@
 }
 
 bool isLibraryPrefix(Expression node) =>
     node is SimpleIdentifier && node.staticElement is PrefixElement;
 
 LibraryElement _getLibrary(AnalysisContext c, String uri) =>
     c.computeLibraryElement(c.sourceFactory.forUri(uri));
 
 bool _isDartRuntime(LibraryElement l) =>
     l.isInSdk && l.source.uri.toString() == 'dart:_runtime';
+
+Set<TypeParameterElement> _freeTypeParameters(DartType t) {
+  var result = new HashSet<TypeParameterElement>();
+  void find(DartType t) {
+    if (t is TypeParameterType) {
+      result.add(t.element);
+    } else if (t is FunctionType) {
+      find(t.returnType);
+      t.parameters.forEach((p) => find(p.type));
+      t.typeFormals.forEach((p) => find(p.bound));
+      t.typeFormals.forEach(result.remove);
+    } else if (t is InterfaceType) {
+      t.typeArguments.forEach(find);
+    }
+  }
+  find(t);
+  return result;
+}
+
+/// _CacheTable tracks cache variables for variables that
+/// are emitted in place with a hoisted variable for a cache.

[Jennifer Messerly, 2016/05/24 21:56:39]

Consider: including an example here of what it looks like in the code. Might
make it easier to visualize.

[Leaf, 2016/05/25 00:56:13]

Done below.

+class _CacheTable {

[Jennifer Messerly, 2016/05/24 21:56:39]

consider: putting these new classes into their own file(s)? They look pretty
self contained (which is good!)

[Leaf, 2016/05/25 00:56:13]

Done.

+  /// Mapping from types to their canonical names.
+  final _names = new Map<DartType, JS.TemporaryId>();
+  Iterable<DartType> get keys => _names.keys.toList();
+
+  JS.Statement _dischargeType(DartType type) {
+    var name = _names.remove(type);
+    if (name != null) {
+      return (js.statement('let #;', [name]));

[Jennifer Messerly, 2016/05/24 21:56:39]

nit: parens not needed around return value

[Leaf, 2016/05/25 00:56:13]

Done.

[Leaf, 2016/05/25 00:56:13]

Done.

+    }
+    return null;
+  }
+
+  /// Emit a list of statements declaring the cache variables for

[Jennifer Messerly, 2016/05/24 21:56:39]

doc comment style nit -- first sentence should be its own paragraph:
https://www.dartlang.org/effective-dart/documentation/#do-make-the-first-sent...
(applies to all of them)

That said content is more important and these are private APIs so I wouldn't
worry about it, but as a general FYI.

[Leaf, 2016/05/25 00:56:13]

Acknowledged.

+  /// types tracked by this table.  If [typeFilter] is given,
+  /// only emit the types listed in the filter.
+  List<JS.Statement> discharge([Iterable<DartType> typeFilter]) {
+    var decls = <JS.Statement>[];
+    var types = typeFilter ?? keys;
+    for (var t in types) {
+      var stmt = _dischargeType(t);
+      if (stmt != null) decls.add(stmt);
+    }
+    return decls;
+  }
+
+  bool isNamed(DartType type) => _names.containsKey(type);
+
+  /// If [type] is not already in the table, choose a new canonical
+  /// variable to contain it. Emit an expression which uses [typeRep] to
+  /// lazily initialize the cache in place.
+  JS.Expression nameType(DartType type, JS.Expression typeRep) {
+    var temp = _names[type];

[Jennifer Messerly, 2016/05/24 21:56:39]

you could also do

    var temp = _names.putIfAbsent(type, () => chooseTypeName(type));

[Leaf, 2016/05/25 00:56:13]

Acknowledged.

+    if (temp == null) {
+      ;

[Jennifer Messerly, 2016/05/24 21:56:39]

unintended empty statement?

[Leaf, 2016/05/25 00:56:13]

Done.

+      _names[type] = temp = chooseTypeName(type);
+    }
+    return js.call('# || (# = #)', [temp, temp, typeRep]);
+  }
+
+  String _typeString(DartType type, {bool flat: false}) {
+    if (type is ParameterizedType && type.name != null) {
+      var clazz = type.name;
+      var params = type.typeArguments;
+      if (params == null) return clazz;
+      if (params.every((p) => p.isDynamic)) return clazz;
+      var paramStrings = params.map(_typeString);
+      var paramString = paramStrings.join("\$");
+      return "${clazz}Of${paramString}";
+    }
+    if (type is FunctionType) {
+      if (flat) return "Fn";
+      var rType = _typeString(type.returnType, flat: true);
+      var paramStrings = type.normalParameterTypes
+          .take(3)
+          .map((p) => _typeString(p, flat: true));
+      var paramString = paramStrings.join("And");
+      var count = type.normalParameterTypes.length;
+      if (count > 3 ||
+          type.namedParameterTypes.isNotEmpty ||
+          type.optionalParameterTypes.isNotEmpty) {
+        paramString = "${paramString}__";
+      } else if (count == 0) {
+        paramString = "Void";
+      }
+      return "${paramString}To${rType}";
+    }
+    if (type is TypeParameterType) return type.name;
+    if (type.name != null) return type.name;
+    return "type";
+  }
+
+  /// Heuristically choose a good name for the cache and generator
+  /// variables.
+  JS.Identifier chooseTypeName(DartType type) {
+    return new JS.TemporaryId(_typeString(type));
+  }
+}
+
+/// _GeneratorTable tracks types which have been
+/// named and hoisted.
+class _GeneratorTable extends _CacheTable {
+  final _defs = new Map<DartType, JS.Expression>();
+
+  _GeneratorTable();

[Jennifer Messerly, 2016/05/24 21:56:39]

note: this is not needed

[Leaf, 2016/05/25 00:56:13]

Done.

+
+  JS.Statement _dischargeType(DartType t) {
+    var name = _names.remove(t);
+    if (name != null) {
+      JS.Expression init = _defs.remove(t);
+      assert(init != null);
+      return js.statement(
+          'let # = () => ((# = dart.constFn(#))());', [name, name, init]);
+    }
+    return null;
+  }
+
+  /// If [type] does not already have a generator name chosen for it,
+  /// assign it one, using [typeRep] as the initializer for it.
+  /// Emit an expression which calls the generator name.
+  JS.Expression nameType(DartType type, JS.Expression typeRep) {

[Jennifer Messerly, 2016/05/24 21:56:39]

this looks mostly identical to the super method, might be worth sharing

[Leaf, 2016/05/25 00:56:13]

It's 3 of 5 lines that are duplicated.  I don't see an easy way to share that
doesn't just add ceremony.

+    var temp = _names[type];
+    if (temp == null) {
+      _names[type] = temp = chooseTypeName(type);
+      _defs[type] = typeRep;
+    }
+    return js.call('#()', [temp]);
+  }
+}
+
+class _TypeTable {
+  /// Cache variable names for types emitted in place.
+  final _cacheNames = new _CacheTable();
+
+  /// Cache variable names for definite function types emitted in place.
+  final _definiteCacheNames = new _CacheTable();
+
+  /// Generator variable names for hoisted types.
+  final _generators = new _GeneratorTable();
+
+  /// Generator variable names for hoisted definite function types.
+  final _definiteGenerators = new _GeneratorTable();
+
+  /// Mapping from type parameters to the types which must have their
+  /// cache/generator variables discharged at the binding site for the
+  /// type variable since the type definition depends on the type
+  /// parameter.
+  final _scopeDependencies = new Map<TypeParameterElement, List<DartType>>();
+
+  /// Emit a list of statements declaring the cache variables and generator
+  /// definitions tracked by the table.  If [formals] is present, only
+  /// emit the definitions which depend on the formals.
+  List<JS.Statement> discharge([List<TypeParameterElement> formals]) {
+    var filter = formals?.expand((p) => _scopeDependencies[p] ?? []);
+    var stmts = [
+      _cacheNames,
+      _definiteCacheNames,
+      _generators,
+      _definiteGenerators
+    ].expand((c) => c.discharge(filter)).toList();
+    formals?.forEach(_scopeDependencies.remove);
+    return stmts;
+  }
+
+  /// Record the dependencies of the type on its free variables
+  bool recordScopeDependencies(DartType type) {
+    var fvs = _freeTypeParameters(type);
+    // TODO(leafp): This is a hack to avoid trying to hoist out of
+    // generic functions and generic function types.  This often degrades
+    // readability to little or no benefit.  It would be good to do this
+    // when we know that we can hoist it to an outer scope, but for
+    // now we just disable it.
+    if (fvs.any((i) => i.enclosingElement is FunctionTypedElement)) {
+      return true;
+    }
+    void addScope(TypeParameterElement i) {
+      List<DartType> types = _scopeDependencies[i];
+      if (types == null) _scopeDependencies[i] = types = [];
+      types.add(type);
+    }
+    fvs.forEach(addScope);
+    return false;
+  }
+
+  /// Given a type [type], and a JS expression [typeRep] which implements it,
+  /// Add the type and its representation to the table, returning an

[Jennifer Messerly, 2016/05/24 21:56:39]

nit: lowercase "add"

[Leaf, 2016/05/25 00:56:13]

Done.

+  /// expression which implements the type (but which caches the value).
+  /// If [hoist] is true, then the JS representation will be hoisted up

[Jennifer Messerly, 2016/05/24 21:56:39]

consider: splitting this into a few paragraphs?

[Leaf, 2016/05/25 00:56:13]

Done.

+  /// as far as possible and shared between instances of the type.
+  /// If [hoist] is false, the cache variable will be hoisted up as
+  /// far as possible and shared between instances of the type, but the
+  /// initializer expression will be emitted in place.
+  /// The boolean parameter [definite] distinguishes between definite function
+  /// types and other types (since the same DartType may have different
+  /// representations as definite and indefinite function types).

[Jennifer Messerly, 2016/05/24 21:56:39]

I seem to recall at one point we had considered putting the arrow type (fuzzy or
definite) on FunctionTypeImpl in Analyzer. Makes me wonder about that again.
Maybe that's something we should look into at some point?

[Leaf, 2016/05/25 00:56:13]

Acknowledged.

+  JS.Expression nameType(DartType type, JS.Expression typeRep, bool hoist,

[Jennifer Messerly, 2016/05/24 21:56:39]

any reason to make "hoist" required and "definite" a named arg?

usual rule is to avoid required booleans:
https://www.dartlang.org/effective-dart/design/#avoid-positional-boolean-para...

[Leaf, 2016/05/25 00:56:13]

Made them both named, asserted hoist is set.  Definite has an obvious default
(almost no types are definite), but hoist is something that is specific to every
call site and shouldn't be omitted.

+      {bool definite: false}) {
+    var table = hoist
+        ? (definite ? _definiteGenerators : _generators)
+        : (definite ? _definiteCacheNames : _cacheNames);
+    if (!table.isNamed(type)) {
+      if (recordScopeDependencies(type)) return typeRep;
+    }
+    return table.nameType(type, typeRep);
+  }
+}