Refactor reifier to make it less dart_codegen specific.

Refactor reifier to make it less dart_codegen specific.

This is some first steps towards hooking the reifier into the js backend.  I've made the reifier operate on whole libraries instead of per compilation unit so that it can be hoisted out as a separate pass.  I changed the API of the reifier to return a map from new type identifiers to information about their library of origin, so that the code generators can deal with these appropriately.  Take a look, see what you think - suggestions as to how to structure this to interact well with the JS backend welcome.

BUG=
R=jmesserly@google.com, vsm@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/2a9727503c762a148e7ff0017175a50f29e76c28

[Leaf, 2015-04-07 22:29:41]

leafp@google.com changed reviewers:
+ jmesserly@google.com, vsm@google.com

[Leaf, 2015-04-07 22:29:59]

https://codereview.chromium.org/1070453002/diff/1/lib/src/codegen/dart_codege...
File lib/src/codegen/dart_codegen.dart (left):

https://codereview.chromium.org/1070453002/diff/1/lib/src/codegen/dart_codege...
lib/src/codegen/dart_codegen.dart:162: void visitIdentifier(Identifier id) {
All of this logic is now computed directly from the map returned by the coercion
reifier in computeExtraImports below.

https://codereview.chromium.org/1070453002/diff/1/lib/src/codegen/dart_codege...
lib/src/codegen/dart_codegen.dart:423: void generateUnit(CompilationUnit unit,
LibraryInfo info, String libraryDir) {
This seemed kind of pointless as a separate function so I just folded it into
the body of the for loop in generateLibrary below.

https://codereview.chromium.org/1070453002/diff/1/lib/src/codegen/reify_coerc...
File lib/src/codegen/reify_coercions.dart (right):

https://codereview.chromium.org/1070453002/diff/1/lib/src/codegen/reify_coerc...
lib/src/codegen/reify_coercions.dart:742: if (dType is FunctionType) return
_typeNameFromFunctionType(dType);
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.

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

lgtm

https://codereview.chromium.org/1070453002/diff/1/lib/src/codegen/reify_coerc...
File lib/src/codegen/reify_coercions.dart (right):

https://codereview.chromium.org/1070453002/diff/1/lib/src/codegen/reify_coerc...
lib/src/codegen/reify_coercions.dart:34: // True => use/def in same library
Does class this describe one specific use?  If not, all uses in the same library
or one?

https://codereview.chromium.org/1070453002/diff/1/lib/src/codegen/reify_coerc...
lib/src/codegen/reify_coercions.dart:554: final Map<Identifier, NewTypeIdDesc>
addedTypes =
Omit the type on the left?

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

https://codereview.chromium.org/1070453002/diff/1/lib/src/codegen/reify_coerc...
File lib/src/codegen/reify_coercions.dart (right):

https://codereview.chromium.org/1070453002/diff/1/lib/src/codegen/reify_coerc...
lib/src/codegen/reify_coercions.dart:611: (type.element != null && type.element
is FunctionTypeAlias)) {
BTW, shouldn't this be:

type is FunctionTypeAlias

or

type.element is FunctionTypeAliasElement

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

lgtm

https://codereview.chromium.org/1070453002/diff/1/lib/src/codegen/dart_codege...
File lib/src/codegen/dart_codegen.dart (left):

https://codereview.chromium.org/1070453002/diff/1/lib/src/codegen/dart_codege...
lib/src/codegen/dart_codegen.dart:423: void generateUnit(CompilationUnit unit,
LibraryInfo info, String libraryDir) {
On 2015/04/07 22:29:58, leafp wrote:
> This seemed kind of pointless as a separate function so I just folded it into
> the body of the for loop in generateLibrary below.

+1, looks nice

https://codereview.chromium.org/1070453002/diff/1/lib/src/codegen/dart_codege...
File lib/src/codegen/dart_codegen.dart (right):

https://codereview.chromium.org/1070453002/diff/1/lib/src/codegen/dart_codege...
lib/src/codegen/dart_codegen.dart:397: _log.fine("Generating unit " +
uri.toString());
"Generating unit $uri" ?

https://codereview.chromium.org/1070453002/diff/1/lib/src/codegen/reify_coerc...
File lib/src/codegen/reify_coercions.dart (right):

https://codereview.chromium.org/1070453002/diff/1/lib/src/codegen/reify_coerc...
lib/src/codegen/reify_coercions.dart:31: // If null, then this is not a library
level identifier (i.e. it's
nit: triple slash for doc comment style:

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

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

https://codereview.chromium.org/1070453002/diff/1/lib/src/codegen/reify_coerc...
lib/src/codegen/reify_coercions.dart:110: _vm = new VariableManager();
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 :)

https://codereview.chromium.org/1070453002/diff/1/lib/src/codegen/reify_coerc...
lib/src/codegen/reify_coercions.dart:121: for (var unit in
_library.partsThenLibrary) {
_library.partsThenLibrary.forEach(generateUnit); ?

[Leaf, 2015-04-13 23:14:36]

Committed patchset #2 (id:20001) manually as
2a9727503c762a148e7ff0017175a50f29e76c28 (presubmit successful).