fixes browser/runtime_test, and a few extension member fixes

fixes browser/runtime_test, and a few of extension member fixes

* fixes generic List. Because generic arrays have the proto of JSArray, which itself extends Array, we need to make sure we don't override members from Array.

* fixes tear offs, they were not preserving type signatures in some cases. This doesn't change how we store signatures, but perhaps something we could consider, to make this bug less likely.

* the defineExtensionMembers call was not getting emitted unless the type implemented at least one interface.

R=leafp@google.com

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

[Jennifer Messerly, 2015-06-05 19:06:09]

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

[Jennifer Messerly, 2015-06-05 19:06:17]

https://codereview.chromium.org/1156273010/diff/1/lib/runtime/dart/_intercept...
File lib/runtime/dart/_interceptors.js (right):

https://codereview.chromium.org/1156273010/diff/1/lib/runtime/dart/_intercept...
lib/runtime/dart/_interceptors.js:26:
[dart.extensionMember('checkGrowable')](reason) {
this is kind of ugly/confusing. Alternatively, we could leave the readable
names, and metaprogram after the fact, and rename the original names to
extension names. Thoughts?

[Leaf, 2015-06-05 20:42:24]

lgtm

https://codereview.chromium.org/1156273010/diff/1/lib/runtime/dart/_intercept...
File lib/runtime/dart/_interceptors.js (right):

https://codereview.chromium.org/1156273010/diff/1/lib/runtime/dart/_intercept...
lib/runtime/dart/_interceptors.js:26:
[dart.extensionMember('checkGrowable')](reason) {
On 2015/06/05 19:06:17, John Messerly wrote:
> this is kind of ugly/confusing. Alternatively, we could leave the readable
> names, and metaprogram after the fact, and rename the original names to
> extension names. Thoughts?

It seems like the meta-programming could make debugging a little weird, since
what you see in the source and what you see in the debugger wouldn't match up. 
But we're already a bit down that path - not sure which is more confusing.  I
don't think this is horrible. 

I wonder if pre-generating the extension member names so that the callee/caller
match up more closely would be better?  That is, pre-generate the extension
members, so that you could write this signature as:

[dartx.checkGrowable](reason) ...

which I think is how the call site will look?

https://codereview.chromium.org/1156273010/diff/1/lib/runtime/dart_runtime.js
File lib/runtime/dart_runtime.js (right):

https://codereview.chromium.org/1156273010/diff/1/lib/runtime/dart_runtime.js...
lib/runtime/dart_runtime.js:828: // Ensure the signature is available too.
We could attach something to each method, but it seems a bit wasteful to have
one field for each method instead of one per class, given that tear-offs are
relatively uncommon.  An alternative would be to go ahead and eagerly put the
extension names in the original signature, so you don't have to add them in
here.

[Jennifer Messerly, 2015-06-05 21:08:57]

https://codereview.chromium.org/1156273010/diff/1/lib/runtime/dart/_intercept...
File lib/runtime/dart/_interceptors.js (right):

https://codereview.chromium.org/1156273010/diff/1/lib/runtime/dart/_intercept...
lib/runtime/dart/_interceptors.js:26:
[dart.extensionMember('checkGrowable')](reason) {
On 2015/06/05 20:42:23, Leaf wrote:
> On 2015/06/05 19:06:17, John Messerly wrote:
> > this is kind of ugly/confusing. Alternatively, we could leave the readable
> > names, and metaprogram after the fact, and rename the original names to
> > extension names. Thoughts?
> 
> It seems like the meta-programming could make debugging a little weird, since
> what you see in the source and what you see in the debugger wouldn't match up.

> But we're already a bit down that path - not sure which is more confusing.  I
> don't think this is horrible. 
> 
> I wonder if pre-generating the extension member names so that the
callee/caller
> match up more closely would be better?  That is, pre-generate the extension
> members, so that you could write this signature as:
> 
> [dartx.checkGrowable](reason) ...
> 
> which I think is how the call site will look?

yup, that's another option. would work for me.

https://codereview.chromium.org/1156273010/diff/1/lib/runtime/dart_runtime.js
File lib/runtime/dart_runtime.js (right):

https://codereview.chromium.org/1156273010/diff/1/lib/runtime/dart_runtime.js...
lib/runtime/dart_runtime.js:828: // Ensure the signature is available too.
On 2015/06/05 20:42:23, Leaf wrote:
> We could attach something to each method, but it seems a bit wasteful to have
> one field for each method instead of one per class, given that tear-offs are
> relatively uncommon.  An alternative would be to go ahead and eagerly put the
> extension names in the original signature, so you don't have to add them in
> here.

yeah, not sure in what sense it's wasteful, though? it's (conceptually) the same
amount of data stored either way right?

    { add: fn0, remove: fn1, ... } + { add: sig0, remove: sig1 }
    vs { add: fn0_w_sig, remove: fn1_w_sig, ... }

instead of having two maps, one for the methods and one for the signatures,
we're just folding them together

[Jennifer Messerly, 2015-06-05 21:10:34]

https://codereview.chromium.org/1156273010/diff/1/lib/runtime/dart_runtime.js
File lib/runtime/dart_runtime.js (right):

https://codereview.chromium.org/1156273010/diff/1/lib/runtime/dart_runtime.js...
lib/runtime/dart_runtime.js:828: // Ensure the signature is available too.
On 2015/06/05 21:08:57, John Messerly wrote:
> On 2015/06/05 20:42:23, Leaf wrote:
> > We could attach something to each method, but it seems a bit wasteful to
have
> > one field for each method instead of one per class, given that tear-offs are
> > relatively uncommon.  An alternative would be to go ahead and eagerly put
the
> > extension names in the original signature, so you don't have to add them in
> > here.
> 
> yeah, not sure in what sense it's wasteful, though? it's (conceptually) the
same
> amount of data stored either way right?
> 
>     { add: fn0, remove: fn1, ... } + { add: sig0, remove: sig1 }
>     vs { add: fn0_w_sig, remove: fn1_w_sig, ... }
> 
> instead of having two maps, one for the methods and one for the signatures,
> we're just folding them together

also, wouldn't hypothetical Typed-ES likely work this way, by storing the type
signature and annotations on the method object?

[Leaf, 2015-06-05 21:34:44]

https://codereview.chromium.org/1156273010/diff/1/lib/runtime/dart_runtime.js
File lib/runtime/dart_runtime.js (right):

https://codereview.chromium.org/1156273010/diff/1/lib/runtime/dart_runtime.js...
lib/runtime/dart_runtime.js:828: // Ensure the signature is available too.
On 2015/06/05 21:10:34, John Messerly wrote:
> On 2015/06/05 21:08:57, John Messerly wrote:
> > On 2015/06/05 20:42:23, Leaf wrote:
> > > We could attach something to each method, but it seems a bit wasteful to
> have
> > > one field for each method instead of one per class, given that tear-offs
are
> > > relatively uncommon.  An alternative would be to go ahead and eagerly put
> the
> > > extension names in the original signature, so you don't have to add them
in
> > > here.
> > 
> > yeah, not sure in what sense it's wasteful, though? it's (conceptually) the
> same
> > amount of data stored either way right?
> > 
> >     { add: fn0, remove: fn1, ... } + { add: sig0, remove: sig1 }
> >     vs { add: fn0_w_sig, remove: fn1_w_sig, ... }
> > 
> > instead of having two maps, one for the methods and one for the signatures,
> > we're just folding them together
> 
> also, wouldn't hypothetical Typed-ES likely work this way, by storing the type
> signature and annotations on the method object?

It's no big deal really.  As it stands, we have a single field on the class,
which points to a closure, which if it ever gets called will produce a map with
N entries (N is number of methods).  If we attach it to methods, then each of
the N methods has a field which points to a closure, which if it ever gets
called will produce the type, which can then get cached, and will be copied onto
the torn off/bound function.  So in the assumed common case that nothing gets
torn off (or dynamically called), it's one field vs N, but in the tear off case
you're right, it's the same.  And since we need to look at the signature for
dynamic calls/puts/reflection and other checks, it may not be as uncommon as I'm
thinking? So yeah, not worth worrying about.

I don't know how ES6 will implement it - if you implement it natively you have a
lot more freedom.  My 30 second stab at a design would be to put it on the class
and have a pointer back to the class from the vtable/gc header, but there are
lots of other options, and probably lots of constraints I don't know about... :)

[Jennifer Messerly, 2015-06-05 23:20:41]

thanks! updated to define the symbols first and tried to improve wording on that
comment ... going to land this to unbreak things, but we can keep iterating.

[Jennifer Messerly, 2015-06-05 23:22:43]

Committed patchset #4 (id:60001) manually as
b8f6240286e2a81bf7d97e8933cebdb01eb06d7d (presubmit successful).