implement mixins in subtype checks, more codegen fixes

implement mixins in subtype checks, more codegen fixes
or: how creating a SplayTreeSet<String> leads to fixing a bunch of stuff

R=vsm@google.com

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

[Jennifer Messerly, 2015-04-01 22:31:31]

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

[Jennifer Messerly, 2015-04-01 22:31:32]

https://codereview.chromium.org/1058653002/diff/1/lib/src/codegen/js_codegen....
File lib/src/codegen/js_codegen.dart (left):

https://codereview.chromium.org/1058653002/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:263: // TODO(vsm): Do we need to record type
info the generated code for a
all of these changes were because we do a type check against Comparator in
corelib. Hah.

https://codereview.chromium.org/1058653002/diff/1/lib/src/codegen/js_codegen....
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/1058653002/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:269: var classDecl = new JS.ClassDeclaration(new
JS.ClassExpression(
not sure if class is really what we want long term, but it's something.

https://codereview.chromium.org/1058653002/diff/1/lib/src/js/printer.dart
File lib/src/js/printer.dart (right):

https://codereview.chromium.org/1058653002/diff/1/lib/src/js/printer.dart#new...
lib/src/js/printer.dart:978: out('{}');
i've done this a few times but somehow keeps not ending up in my final CLs. So
here it is :)

[vsm, 2015-04-01 23:04:57]

https://codereview.chromium.org/1058653002/diff/1/lib/src/codegen/js_codegen....
File lib/src/codegen/js_codegen.dart (left):

https://codereview.chromium.org/1058653002/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:330: classElem.typeParameters.length,
js.call('dart.dynamic'));
Shouldn't we fill in with core.Object?

https://codereview.chromium.org/1058653002/diff/1/lib/src/codegen/js_codegen....
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/1058653002/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:269: var classDecl = new JS.ClassDeclaration(new
JS.ClassExpression(
On 2015/04/01 22:31:31, John Messerly wrote:
> not sure if class is really what we want long term, but it's something.

We might want this to be some sort of type object.

[Jennifer Messerly, 2015-04-01 23:22:56]

https://codereview.chromium.org/1058653002/diff/1/lib/src/codegen/js_codegen....
File lib/src/codegen/js_codegen.dart (left):

https://codereview.chromium.org/1058653002/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:330: classElem.typeParameters.length,
js.call('dart.dynamic'));
On 2015/04/01 23:04:57, vsm wrote:
> Shouldn't we fill in with core.Object?

It's handled in dart_runtime by the generic function. We can change it to
whatever. But regardless, there's no value in emitting it here.

I'm happy with core.Object, but I thought we left the discussion as: keep
dart.dynamic, but generally treat it the same as Object in type tests, except
for the function parameter case.

Where the user would see this:


typedef void MyFunc<B>(B b);

class Foo<B> {
  void foo(MyFunc<B> f) {  
  }
}

new Foo().foo(...)

Did we substitute dynamic for B in that function type?

https://codereview.chromium.org/1058653002/diff/1/lib/src/codegen/js_codegen....
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/1058653002/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:269: var classDecl = new JS.ClassDeclaration(new
JS.ClassExpression(
On 2015/04/01 23:04:57, vsm wrote:
> On 2015/04/01 22:31:31, John Messerly wrote:
> > not sure if class is really what we want long term, but it's something.
> 
> We might want this to be some sort of type object.

yeah, something that is an instance of Type. Right now classes do that for us,
but there's other ways we could represent it that are probably easier.

The reason I started with classes is they already have generic support.

[vsm, 2015-04-01 23:34:47]

https://codereview.chromium.org/1058653002/diff/1/lib/src/codegen/js_codegen....
File lib/src/codegen/js_codegen.dart (left):

https://codereview.chromium.org/1058653002/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:330: classElem.typeParameters.length,
js.call('dart.dynamic'));
On 2015/04/01 23:22:56, John Messerly wrote:
> On 2015/04/01 23:04:57, vsm wrote:
> > Shouldn't we fill in with core.Object?
> 
> It's handled in dart_runtime by the generic function. We can change it to
> whatever. But regardless, there's no value in emitting it here.

If we have x is T somewhere in the body of the class, we're instantiating T with
undefined if we omit, right?  Are we mapping undefined to Object somewhere?

> 
> I'm happy with core.Object, but I thought we left the discussion as: keep
> dart.dynamic, but generally treat it the same as Object in type tests, except
> for the function parameter case.
> 
> Where the user would see this:
> 
> 
> typedef void MyFunc<B>(B b);
> 
> class Foo<B> {
>   void foo(MyFunc<B> f) {  
>   }
> }
> 
> new Foo().foo(...)
> 
> Did we substitute dynamic for B in that function type?

Semantically, we want Object for B.  I.e.,:

dynamic f = (int x) => print(x+x);
new Foo().foo(f);

should fail as int->void is not a subtype of Object->void (but is of
dynamic->void).

[Jennifer Messerly, 2015-04-02 16:11:08]

https://codereview.chromium.org/1058653002/diff/1/lib/src/codegen/js_codegen....
File lib/src/codegen/js_codegen.dart (left):

https://codereview.chromium.org/1058653002/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:330: classElem.typeParameters.length,
js.call('dart.dynamic'));
On 2015/04/01 23:34:47, vsm wrote:
> On 2015/04/01 23:22:56, John Messerly wrote:
> > On 2015/04/01 23:04:57, vsm wrote:
> > > Shouldn't we fill in with core.Object?
> > 
> > It's handled in dart_runtime by the generic function. We can change it to
> > whatever. But regardless, there's no value in emitting it here.
> 
> If we have x is T somewhere in the body of the class, we're instantiating T
with
> undefined if we omit, right?  Are we mapping undefined to Object somewhere?

yes, like I said it happens in dart_runtime :)
https://github.com/dart-lang/dev_compiler/blob/master/lib/runtime/dart_runtim...
In other words, I haven't changed any behavior, I'm just removing some
unnecessary codegen.

> Semantically, we want Object for B.  I.e.,:
> 
> dynamic f = (int x) => print(x+x);
> new Foo().foo(f);
> 
> should fail as int->void is not a subtype of Object->void (but is of
> dynamic->void).

Yeah, sounds good to me. It's different from this CL but I'm happy to change
that line from dart_runtime.js, whenever is a good time.

[vsm, 2015-04-02 16:30:54]

lgtm

https://codereview.chromium.org/1058653002/diff/1/lib/src/codegen/js_codegen....
File lib/src/codegen/js_codegen.dart (left):

https://codereview.chromium.org/1058653002/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:330: classElem.typeParameters.length,
js.call('dart.dynamic'));
On 2015/04/02 16:11:08, John Messerly wrote:
> On 2015/04/01 23:34:47, vsm wrote:
> > On 2015/04/01 23:22:56, John Messerly wrote:
> > > On 2015/04/01 23:04:57, vsm wrote:
> > > > Shouldn't we fill in with core.Object?
> > > 
> > > It's handled in dart_runtime by the generic function. We can change it to
> > > whatever. But regardless, there's no value in emitting it here.
> > 
> > If we have x is T somewhere in the body of the class, we're instantiating T
> with
> > undefined if we omit, right?  Are we mapping undefined to Object somewhere?
> 
> yes, like I said it happens in dart_runtime :)
>
https://github.com/dart-lang/dev_compiler/blob/master/lib/runtime/dart_runtim...
> In other words, I haven't changed any behavior, I'm just removing some
> unnecessary codegen.

Aha!  :-)

> 
> > Semantically, we want Object for B.  I.e.,:
> > 
> > dynamic f = (int x) => print(x+x);
> > new Foo().foo(f);
> > 
> > should fail as int->void is not a subtype of Object->void (but is of
> > dynamic->void).
> 
> Yeah, sounds good to me. It's different from this CL but I'm happy to change
> that line from dart_runtime.js, whenever is a good time.
>

[Jennifer Messerly, 2015-04-02 19:16:30]

Committed patchset #1 (id:1) manually as
a0b5564ef5e0c248f8d6e6509a6b9278bb3496a0 (presubmit successful).