allow JS builtin to be typed as needed

allow JS builtin to be typed as needed

this removes a lot of bogus casts. It also revealed one test that appeared to be passing, but was not actually passing.

R=vsm@google.com

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

[Jennifer Messerly, 2016-01-28 00:43:17]

Description was changed from

==========
allow JS builtin to be typed as needed
==========

to

==========
allow JS builtin to be typed as needed

this removes a lot of bogus casts. It also revealed one test that appeared to be
passing, but was not actually passing.
==========

[Jennifer Messerly, 2016-01-28 00:43:17]

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

[Jennifer Messerly, 2016-01-28 00:46:18]

https://codereview.chromium.org/1649583002/diff/1/test/browser/language_tests.js
File test/browser/language_tests.js (right):

https://codereview.chromium.org/1649583002/diff/1/test/browser/language_tests...
test/browser/language_tests.js:197: 'interceptor9_test',
The interceptor9_test expects to create a typed array, checks that an "is" check
works, then triggers a no such method on it.

However the catch is not precise enough--it would allow any error other than
ExpectException(!), and made no attempt to verify where the error was thrown.
Previously we were failing on a DDC cast inside typed_data constructor, so we
never even reached the "is" check. Now we make it there, but we fail the `is`
check, causing the test to fail.

It is good the test fails, because typed_data is not implemented correctly yet.
:)

Aside: the test could've been more straightforwardly written, but it goes to
great pains to confuse dart2js, confusing itself in the process. ;)

[Jennifer Messerly, 2016-03-08 19:00:42]

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

[Jennifer Messerly, 2016-03-08 19:00:42]

Hey Vijay, did you ever get to take a look at this?

I vaguely recall that we wanted some additional changes and/or felt this wasn't
quite right, but I don't recall the details. Did we want to instead implement
this on the Analyzer side?

(What Analyzer is currently doing for "JS" builtin is very wrong. I'd like to
fix it somehow, and layering it in DDC feels better than hacking something into
Analyzer. But maybe Analyzer is the only place to fix it.)

[vsm, 2016-03-08 23:19:40]

On 2016/03/08 19:00:42, John Messerly wrote:
> Hey Vijay, did you ever get to take a look at this?
> 
> I vaguely recall that we wanted some additional changes and/or felt this
wasn't
> quite right, but I don't recall the details. Did we want to instead implement
> this on the Analyzer side?
> 
> (What Analyzer is currently doing for "JS" builtin is very wrong. I'd like to
> fix it somehow, and layering it in DDC feels better than hacking something
into
> Analyzer. But maybe Analyzer is the only place to fix it.)

Do we still need this?  It came up because of a regression in how the Analyzer
was computing the static type of JS expressions - always dynamic for a short
period - but that was fixed upstream.

[vsm, 2016-03-08 23:51:30]

lgtm after chatting with john

parsing the types is rather brittle right.  john will file a bug suggesting a
better approach.

[Jennifer Messerly, 2016-04-27 19:16:55]

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

[Jennifer Messerly, 2016-04-27 19:18:47]

Hey all, I've updated this CL if you want to take another look.

Not sure why I didn't land it previously after the LGTM :) ... think I just
forgot about it. But this was affecting perf of a lot of things

https://codereview.chromium.org/1649583002/diff/20001/lib/src/closure/closure...
File lib/src/closure/closure_annotator.dart (left):

https://codereview.chromium.org/1649583002/diff/20001/lib/src/closure/closure...
lib/src/closure/closure_annotator.dart:5: import
'package:analyzer/dart/ast/ast.dart' hide ConstantEvaluator;
this was triggering a hint in "master" analyzer, causing ./tool/analyzer.sh to
fail. It was not needed so removing.

[vsm, 2016-04-27 19:55:40]

lgtm

[Jennifer Messerly, 2016-04-27 20:04:30]

Description was changed from

==========
allow JS builtin to be typed as needed

this removes a lot of bogus casts. It also revealed one test that appeared to be
passing, but was not actually passing.
==========

to

==========
allow JS builtin to be typed as needed

this removes a lot of bogus casts. It also revealed one test that appeared to be
passing, but was not actually passing.

R=vsm@google.com

Committed:
https://github.com/dart-lang/dev_compiler/commit/ec5180eb173c09b62109195025bc...
==========

[Jennifer Messerly, 2016-04-27 20:04:31]

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