Reifier hookup

Reifier hookup

Hook the coercion reifier into the JS backend.

The principle observed changes in the output are from the downward type inference eliminating casts.

The coercion reifier changes in this CL in two ways.

The first is a relatively minor change to factor out the instrumented runtime calls.  The reifier is now parameterized by a runtime object.  If present, casts, wraps, and type object creation are delegated to the runtime.  Otherwise the reifier just produces cast objects directly.  The JS codegen always does the latter.

The larger change is an attempt to make the new AST nodes be resolved.  This is principally done for the typedefs produced for casts - we're not using wrapping right now so I've left that for future if we want it.  This is a bit painful - if we continue down this path I think I'll want to factor this out into a ResolvedAstBuilder or somesuch, but for now this seems to be working.

The rest of the changes are just plumbing in the coercion reifier and its dependencies into the js backend.

BUG=
R=jmesserly@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/91564690b106f330fda826535f55f685705662b5

[Leaf, 2015-04-16 20:31:39]

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

[Jennifer Messerly, 2015-04-16 23:31:38]

LGTM. Any idea if this affects compilation time? (my guess is it wouldn't be
noticeable). I added a few comments but they could easily be follow ups.

https://codereview.chromium.org/1096583002/diff/20001/lib/src/codegen/js_code...
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/1096583002/diff/20001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:210: assert(false);
IMO would be fine to throw if we shouldn't get these

https://codereview.chromium.org/1096583002/diff/20001/lib/src/codegen/reify_c...
File lib/src/codegen/reify_coercions.dart (right):

https://codereview.chromium.org/1096583002/diff/20001/lib/src/codegen/reify_c...
lib/src/codegen/reify_coercions.dart:289: return
AstBuilder.identifierFromString(s);
it'd be worth double checking how this identifier comes through to JS codegen.
Ideally _isTemporary in JSCodegenVisitor returns `true` ... I think it will be
since it doesn't have a source span. That's awesome because it means the js
renamer will kick in and ensure these are distinct names. The catch is we need
to use the same Element everywhere, but it looks to be doing that.

https://codereview.chromium.org/1096583002/diff/20001/lib/src/codegen/reify_c...
lib/src/codegen/reify_coercions.dart:675: ParameterElementImpl fe = new
ParameterElementImpl.forNode(fp.identifier);
maybe try to encapsulate this? it seems like we're duplicating similar logic

https://codereview.chromium.org/1096583002/diff/20001/lib/src/codegen/reify_c...
lib/src/codegen/reify_coercions.dart:745: // The name of the typedef (unresolved
at this point)
maybe a TODO for now and we can fix later, but: it'd be nice if the typedef had
a more meaningful name.
In theory you can use any name you want (i.e. make it maximally friendly), and
if it collides it will get renamed.

[Leaf, 2015-04-17 20:49:36]

https://codereview.chromium.org/1096583002/diff/20001/lib/src/codegen/js_code...
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/1096583002/diff/20001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:210: assert(false);
On 2015/04/16 23:31:38, John Messerly wrote:
> IMO would be fine to throw if we shouldn't get these

Done.

https://codereview.chromium.org/1096583002/diff/20001/lib/src/codegen/reify_c...
File lib/src/codegen/reify_coercions.dart (right):

https://codereview.chromium.org/1096583002/diff/20001/lib/src/codegen/reify_c...
lib/src/codegen/reify_coercions.dart:289: return
AstBuilder.identifierFromString(s);
On 2015/04/16 23:31:38, John Messerly wrote:
> it'd be worth double checking how this identifier comes through to JS codegen.
> Ideally _isTemporary in JSCodegenVisitor returns `true` ... I think it will be
> since it doesn't have a source span. That's awesome because it means the js
> renamer will kick in and ensure these are distinct names. The catch is we need
> to use the same Element everywhere, but it looks to be doing that.

The way JS code gen currently works, I don't think these ever get looked at (at
least unless we turn on closure wrapping).  Without closure wrapping (and
without runtime instrumentation) the only temporaries I need to introduce are
for 1) typedef parameters, which get ignored by the js code generator, and 2)
typedef names, which don't go though the 'visitSimpleIdentifier' path in the js
code generator.

At some point here we should definitely rationalize and unify the treatment of
temporaries though.

https://codereview.chromium.org/1096583002/diff/20001/lib/src/codegen/reify_c...
lib/src/codegen/reify_coercions.dart:675: ParameterElementImpl fe = new
ParameterElementImpl.forNode(fp.identifier);
On 2015/04/16 23:31:38, John Messerly wrote:
> maybe try to encapsulate this? it seems like we're duplicating similar logic

Done.

https://codereview.chromium.org/1096583002/diff/20001/lib/src/codegen/reify_c...
lib/src/codegen/reify_coercions.dart:745: // The name of the typedef (unresolved
at this point)
On 2015/04/16 23:31:38, John Messerly wrote:
> maybe a TODO for now and we can fix later, but: it'd be nice if the typedef
had
> a more meaningful name.
> In theory you can use any name you want (i.e. make it maximally friendly), and
> if it collides it will get renamed.

Suggestions as to meaningful names?  I made the hint into "CastType", but not
sure that it's much of an improvement.

[Leaf, 2015-04-17 20:56:08]

Committed patchset #3 (id:40001) manually as
91564690b106f330fda826535f55f685705662b5 (presubmit successful).