Refactor reifier to make it less dart_codegen specific.

lib/src/codegen/reify_coercions.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 dev_compiler.src.codegen.reify_coercions;
 
 import 'package:analyzer/analyzer.dart' as analyzer;
 import 'package:analyzer/src/generated/ast.dart';
 import 'package:analyzer/src/generated/element.dart';
 import 'package:logging/logging.dart' as logger;
 import 'package:source_span/source_span.dart' show SourceFile;
 
 import 'package:dev_compiler/src/checker/rules.dart';
 import 'package:dev_compiler/src/info.dart';
 import 'package:dev_compiler/src/utils.dart' as utils;
 
 import 'ast_builder.dart';
 
 final _log = new logger.Logger('dev_compiler.reify_coercions');
 
 // TODO(leafp) Factor this out or use an existing library
 class Tuple2<T0, T1> {
   final T0 e0;
   final T1 e1;
   Tuple2(this.e0, this.e1);
 }
 
 typedef T Function1<S, T>(S _);
 
+class NewTypeIdDesc {
+  // If null, then this is not a library level identifier (i.e. it's

[Jennifer Messerly, 2015/04/08 20:28:16]

nit: triple slash for doc comment style:

/// If null, then [...]

(not that we're currently generating API documentation, mind you, but doesn't
hurt :-) )

+  // a type parameter, or a special type like void, dynamic, etc)
+  LibraryElement importedFrom;
+  // True => use/def in same library

[vsm, 2015/04/08 15:31:02]

Does class this describe one specific use?  If not, all uses in the same library
or one?

+  bool fromCurrent;
+  // True => not a source variable
+  bool synthetic;
+  NewTypeIdDesc({this.fromCurrent, this.importedFrom, this.synthetic});
+}
+
 class _LocatedWrapper {
   final String loc;
   final Wrapper wrapper;
   _LocatedWrapper(this.wrapper, this.loc);
 }
 
 class _Inference extends DownwardsInference {
   TypeManager _tm;
 
   _Inference(TypeRules rules, this._tm) : super(rules);
@@ skipping 37 matching lines @@
 
   @override
   void annotateFunctionExpression(FunctionExpression e, DartType returnType) {
     // Implicitly changes e.staticType
     (e.element as ExecutableElementImpl).returnType = returnType;
   }
 }
 
 // This class implements a pass which modifies (in place) the ast replacing
 // abstract coercion nodes with their dart implementations.
-class UnitCoercionReifier extends analyzer.GeneralizingAstVisitor<Object>
+class CoercionReifier extends analyzer.GeneralizingAstVisitor<Object>
     with ConversionVisitor<Object> {
   CoercionManager _cm;
-  final TypeManager _tm;
-  final VariableManager _vm;
+  TypeManager _tm;
+  VariableManager _vm;
+  LibraryUnit _library;
   SourceFile _file;
   bool _skipCoercions = false;
   final TypeRules _rules;
   _Inference _inferrer;
 
-  UnitCoercionReifier(this._tm, this._vm, this._rules) {
+  CoercionReifier(this._library, this._rules) {
+    _vm = new VariableManager();

[Jennifer Messerly, 2015/04/08 20:28:16]

if you want to keep final fields, it could be done like this:

factory CoercionReifier(LibraryUnit library, TypeRules rules) {
  var vm = ...
  var tm = ...
  var cm = ...
  var inferrer = ...
  return new CoercionReifier._(library, rules, vm, tm, cm, inferrer);
}

CoercionReifier._(this._library, this._rules, this._vm, this._tm, this._cm,
this._inferrer);


Whether that's worth it or not is debatable :)

+    _tm = new TypeManager(_library.library.element.enclosingElement, _vm);
     _cm = new CoercionManager(_vm, _tm, _rules);
     _inferrer = new _Inference(_rules, _tm);
   }
 
   // This should be the entry point for this class.  Entering via the
   // visit functions directly may not do the right thing with respect
   // to discharging the collected definitions.
-  void reify(CompilationUnit unit) {
+  // Returns the set of new type identifiers added by the reifier
+  Map<Identifier, NewTypeIdDesc> reify() {
+    for (var unit in _library.partsThenLibrary) {

[Jennifer Messerly, 2015/04/08 20:28:16]

_library.partsThenLibrary.forEach(generateUnit); ?

+      generateUnit(unit);
+    }
+    return _tm.addedTypes;
+  }
+
+  void generateUnit(CompilationUnit unit) {
     _file = new SourceFile(unit.element.source.contents.data,
         url: unit.element.source.uri);
     visitCompilationUnit(unit);
+    _file = null;
   }
 
   ///////////////// Private //////////////////////////////////
 
   String _locationInfo(Expression e) {
     if (_file != null) {
       final begin = e is AnnotatedNode
           ? (e as AnnotatedNode).firstTokenAfterCommentAndMetadata.offset
           : e.offset;
       if (begin != 0) {
@@ skipping 400 matching lines @@
 }
 
 // A class for managing the interaction between the DartType hierarchy
 // and the AST type representation.  It provides utilities to translate
 // a DartType to AST.  In order to do so, it maintains a map of typedefs
 // naming otherwise un-named types.  These must be discharged at the top
 // level of the compilation unit in order to produce well-formed dart code.
 // Note that in order to hoist the typedefs out of parameterized classes
 // we must close over any type variables.
 class TypeManager {
-  VariableManager _vm;
-  Set<TypeName> _newTypes = new Set<TypeName>();
+  final VariableManager _vm;
+  final LibraryElement _currentLibrary;
+  final Map<Identifier, NewTypeIdDesc> addedTypes =

[vsm, 2015/04/08 15:31:02]

Omit the type on the left?

+      new Map<Identifier, NewTypeIdDesc>();
 
   /// A map containing new function typedefs to be introduced at the top level
   /// This uses LinkedHashMap to emit code in a consistent order.
   final Map<FunctionType, FunctionTypeAlias> _typedefs =
       new Map<FunctionType, FunctionTypeAlias>();
 
-  TypeManager(this._vm);
+  TypeManager(this._currentLibrary, this._vm);
 
   void enterCompilationUnit() {}
   void exitCompilationUnit(CompilationUnit unit) {
     unit.declarations.addAll(_typedefs.values);
     _typedefs.clear();
   }
 
   TypeName typeNameFromDartType(DartType dType) {
     return _typeNameFromDartType(dType);
   }
 
   NormalFormalParameter typedFormal(Identifier v, DartType type) {
     return _typedFormal(v, type);
   }
 
   Expression typeExpressionFromDartType(DartType t) =>
       typeExpression(typeNameFromDartType(t));
 
   Expression typeExpression(TypeName t) => _typeExpression(t);
 
-  Set<TypeName> get addedTypes => _newTypes;
-
   ///////////////// Private //////////////////////////////////
   List<TypeParameterType> _freeTypeVariables(DartType type) {
     var s = new Set<TypeParameterType>();
 
     void _ft(DartType type) {
       void _ftMap(Map<String, DartType> m) {
         if (m == null) return;
         for (var k in m.keys) _ft(m[k]);
       }
       void _ftList(List<DartType> l) {
         if (l == null) return;
         for (int i = 0; i < l.length; i++) _ft(l[i]);
       }
 
       if (type == null) return;
       if (type.isDynamic) return;
       if (type.isBottom) return;
       if (type.isObject) return;
       if (type is TypeParameterType) {
         s.add(type);
         return;
       }
       if (type is ParameterizedType) {
         // TODO(leafp): The inner conditional (testing FunctionType, etc)
         // can be eliminated after the roll to the next analyzer which fixes
         // a bug in how they resolve type names.
         if (type.name != null && type.name != "") {
           if (type is! FunctionType ||
               (type.element != null && type.element is FunctionTypeAlias)) {

[vsm, 2015/04/08 18:27:09]

BTW, shouldn't this be:

type is FunctionTypeAlias

or

type.element is FunctionTypeAliasElement

             _ftList(type.typeArguments);
             return;
           }
         }
         if (type is FunctionType) {
           _ftMap(type.namedParameterTypes);
           _ftList(type.normalParameterTypes);
           _ftList(type.optionalParameterTypes);
           _ft(type.returnType);
           return;
@@ skipping 44 matching lines @@
 
   NormalFormalParameter _typedFormal(Identifier v, DartType type) {
     if (type is FunctionType) {
       return _functionTypedFormal(v, type);
     }
     assert(type.name != null);
     TypeName t = typeNameFromDartType(type);
     return AstBuilder.simpleFormal(v, t);
   }
 
+  Identifier freshTypeDefVariable() {
+    var t = _vm.freshTypeIdentifier("t");
+    var desc = new NewTypeIdDesc(
+        fromCurrent: true, importedFrom: _currentLibrary, synthetic: true);
+    addedTypes[t] = desc;
+    return t;
+  }
+
+  Identifier typeParameterFromString(String name) =>
+      AstBuilder.identifierFromString(name);
+
+  Identifier freshReferenceToNamedType(DartType type) {
+    var name = type.name;
+    assert(name != null);
+    var id = AstBuilder.identifierFromString(name);
+    var element = type.element;
+    id.staticElement = element;
+    var library = null;
+    // This can happen for types like (e.g.) void
+    if (element != null) library = element.library;
+    var desc = new NewTypeIdDesc(
+        fromCurrent: _currentLibrary == library,
+        importedFrom: library,
+        synthetic: false);
+    addedTypes[id] = desc;
+    return id;
+  }
+
   // I think we can avoid alpha-varying type parameters, since
   // the binding forms are so limited, so we just re-use the
   // the original names for the formals and the actuals.
   TypeName _typeNameFromFunctionType(FunctionType type) {
     if (_typedefs.containsKey(type)) {
       var alias = _typedefs[type];
       var ts = null;
       var tpl = alias.typeParameters;
       if (tpl != null) {
         var ltp = tpl.typeParameters;
         ts = new List<TypeName>.from(
             ltp.map((t) => _mkNewTypeName(t.name, null)));
       }
       var name = alias.name;
       return _mkNewTypeName(name, ts);
     }
 
     List<TypeParameterType> ftvs = _freeTypeVariables(type);
-    Identifier t = _vm.freshTypeIdentifier("t");
+    Identifier t = freshTypeDefVariable();
 
     Iterable<Identifier> tNames =
-        ftvs.map((x) => AstBuilder.identifierFromString(x.name));
+        ftvs.map((x) => typeParameterFromString(x.name));
     List<TypeParameter> tps = tNames.map(AstBuilder.typeParameter).toList();
     List<FormalParameter> fps = _formalParameterListForFunctionType(type);
     TypeName ret = _typeNameFromDartType(type.returnType);
     FunctionTypeAlias alias = AstBuilder.functionTypeAlias(ret, t, tps, fps);
 
     _typedefs[type] = alias;
 
     List<TypeName> args = ftvs.map(_typeNameFromDartType).toList();
     TypeName namedType = _mkNewTypeName(t, args);
 
     return namedType;
   }
 
   TypeName _typeNameFromDartType(DartType dType) {
-    // TODO(leafp) This doesn't re-use the name when the function type
-    // is derived from a typedef, since I've moved it above the check
-    // for a name.  I think there's a bug in the resolver here: I
-    // sometimes see function types named not with a typedef name,
-    // but rather with the name of the function that they classify.
-    if (dType is FunctionType) return _typeNameFromFunctionType(dType);
     String name = dType.name;
     if (name == null || name == "" || dType.isBottom) {
+      if (dType is FunctionType) return _typeNameFromFunctionType(dType);

[Leaf, 2015/04/07 22:29:58]

This is the only thing that changes the output code.  The analyzer bug has been
resolved, so we can now re-use typedef names when we have them.

       _log.severe("No name for type, casting through dynamic");
       var d = AstBuilder.identifierFromString("dynamic");
       var t = _mkNewTypeName(d, null);
       t.type = dType;
       return t;
     }
-    SimpleIdentifier id = AstBuilder.identifierFromString(name);
-    id.staticElement = dType.element;
+    SimpleIdentifier id = freshReferenceToNamedType(dType);
     List<TypeName> args = null;
     if (dType is ParameterizedType) {
       List<DartType> targs = dType.typeArguments;
       args = targs.map(_typeNameFromDartType).toList();
     }
     var t = _mkNewTypeName(id, args);
     t.type = dType;
     return t;
   }
 
   TypeName _mkNewTypeName(Identifier id, List<TypeName> args) {
     var t = AstBuilder.typeName(id, args);
-    _newTypes.add(t);
     return t;
   }
 
   Expression _typeExpression(TypeName t) {
     if (t.typeArguments != null && t.typeArguments.length > 0) {
       var w = AstBuilder.identifierFromString("_");
       var fp = AstBuilder.simpleFormal(w, t);
       var f = AstBuilder.blockFunction(<FormalParameter>[fp], <Statement>[]);
       return new RuntimeOperation("type", <Expression>[f]);
     }
     return t.name;
   }
 }