Initial support for runtime function types and type checking

Initial support for runtime function types and type checking

Notes:
- We probably should have a general type object class for all types.  Just handling function types for now.
- Not yet encoding the type on functions during codegen ... falling back on arity effectively.
- typedefs are lazy to avoid ordering issues.  Could perhaps reuse existing logic instead.

R=jmesserly@google.com

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

[vsm, 2015-04-09 16:24:56]

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

[Jennifer Messerly, 2015-04-09 18:44:29]

It would be nice to consolidate function type logic with AtScript somehow if we
can? Since we're basically polyfilling ES7 type annotations, making that
generic-ish would be good

anyway this definitely looks good as a stepping stone (other than the raw
`dynamic` bug) ... just wondering how this will play with future CL that handles
methods, integration with TS/AtScript, etc

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart/_inter...
File lib/runtime/dart/_internal.js (right):

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart/_inter...
lib/runtime/dart/_internal.js:351: let _Transformation =
dart.typedef('_Transformation', () => dart.functionType(T, [S]));
so, the reason I made it a class ... it seemed like function types are basically
a special case:

    typedef R Foo(T t, S s);

is sort-of sugar for:

    class Foo {
      R call(T t, S s);
    }

("sort-of" because nominal vs structural in type checks)

So once we had method type signatures, function types would "fall out" for free,
and we wouldn't need two ways of emitting type signatures: we could just read it
from the "call" method. Granted, we'd still need to mark typedefs, so they are
treated structurally instead of nominally :)

Taking this even further, in hypothetical ES7 we could do signatures like this:

    function printLabel(obj: string) {}

so maybe just polyfill that?

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart/_inter...
lib/runtime/dart/_internal.js:503: return new (ExpandIterator$(S,
T))(this[_iterable][core.$iterator], dart.as(this[_f],
dart.functionType(core.Iterable$(T), [S])));
use functional ordering?

    dart.functionType(S, core.Iterable$(T))

so the type of `TResult foo(TArg0 arg0, TArg1 arg1` would be ]

    dart.functionType(TArg0, TArg1, TResult)

optional and named args are a little wacky, but those could be similar:

    dart.functionType(TArg0, [TArg1, TArg2], TResult)

    dart.functionType(TArg0, {arg1: TArg1, arg2: TArg2}, TResult)

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart/_isola...
File lib/runtime/dart/_isolate_helper.js (right):

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart/_isola...
lib/runtime/dart/_isolate_helper.js:114: return new core.List.from(['map',
x.keys[core.$map](dart.as(serializeTearOff, dart.functionType(dynamic,
[dynamic])))[core.$toList](), x.values[core.$map](dart.as(serializeTearOff,
dart.functionType(dynamic, [dynamic])))[core.$toList]()]);
shouldn't this be `dart.dynamic`?

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart_runtim...
File lib/runtime/dart_runtime.js (right):

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart_runtim...
lib/runtime/dart_runtime.js:129: // TODO(vsm): Should we explicitly check if
this is a core.Object?
I wondered this too, but since we didn't need it yet, left it out. Not sure if
it's worth adding a TODO? Presumably we can add if we need?

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart_runtim...
lib/runtime/dart_runtime.js:130: var result = obj[dart.runtimeType];
what does this do? are you trying to call the Object.runtimeType method or ...?

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart_runtim...
lib/runtime/dart_runtime.js:201: if (_isTop(t2, dynamicIsBottom) ||
_isBottom(t1, dynamicIsBottom)) return true;
some long lines here

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart_runtim...
lib/runtime/dart_runtime.js:212: // TODO(vsm): Handle Objects with call methods.
 Those are functions
does dart_runtime.dart handle this case?

it wouldn't be hard to get the ".call" method here... but I guess the problem is
it won't have any type info on it yet. =)

you could do arity checks though?

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart_runtim...
lib/runtime/dart_runtime.js:237: if (t1 == null) return t2 == core.Object || t2
== dart.dynamic;
Hmmm, canonicalType was supposed to be handling this, so we could avoid worrying
about things like that. But maybe it's broken, I see sunflower doesn't load on
master. Doh. Looking at that.

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart_runtim...
lib/runtime/dart_runtime.js:339: // TODO(vsm): Common type object?
we could, it's kind of nice to reuse the JS type object when possible though.

thinking longer term, but it'd be nice if Dart objects were envisioned as part
of the ES6+ system and the Dart runtime features too. e.g. our generics and
mixins could be used as just a library by an ES6 user. It's pretty neat that for
a Dart Foo class, you can write `new Foo()` in ES and `foo instanceof Foo` and
it all works, and you can also use our subtype library on JS classes. So it'd be
nice if plain `Foo` was the type object. It's also nice in debugging, with
classes you can type Foo at the console and see the class def

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart_runtim...
lib/runtime/dart_runtime.js:340: if (type instanceof AbstractFunctionType)
return type.toString();
could you just set a `name` property on this?

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart_runtim...
lib/runtime/dart_runtime.js:343: return type.toString();
does this happen?

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

https://codereview.chromium.org/1076883003/diff/40001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:309: var result = js.statement('let # =
dart.typedef(#, #);', [
maybe a little cleaner:

'let # = dart.typedef(#, () => #);'

and you can skip the manual `var closure = new JS.ArrowFun([], ...` line

https://codereview.chromium.org/1076883003/diff/40001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:316: // if (isPublic(name)) _addExport(name);
remove?

https://codereview.chromium.org/1076883003/diff/40001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:1116: JS.Expression _emitTypeName(DartType type,
[bool lowerTypedef = false]) {
named arg?

https://codereview.chromium.org/1076883003/diff/40001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:1118: // The void type is not defined in core.
dynamic too?

https://codereview.chromium.org/1076883003/diff/40001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:1126: // TODO(vsm): Support all parameter types.
can you elaborate on this?

[Jennifer Messerly, 2015-04-09 18:47:24]

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart_runtim...
File lib/runtime/dart_runtime.js (right):

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart_runtim...
lib/runtime/dart_runtime.js:339: // TODO(vsm): Common type object?
On 2015/04/09 18:44:28, John Messerly wrote:
> we could, it's kind of nice to reuse the JS type object when possible though.
> 
> thinking longer term, but it'd be nice if Dart objects were envisioned as part
> of the ES6+ system and the Dart runtime features too. e.g. our generics and
> mixins could be used as just a library by an ES6 user. It's pretty neat that
for
> a Dart Foo class, you can write `new Foo()` in ES and `foo instanceof Foo` and
> it all works, and you can also use our subtype library on JS classes. So it'd
be
> nice if plain `Foo` was the type object. It's also nice in debugging, with
> classes you can type Foo at the console and see the class def

having said that, we can certainly have a different internal representation ...
just saying the public APIs should ideally accept JS type objects.

[vsm, 2015-04-09 19:52:25]

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart/_inter...
File lib/runtime/dart/_internal.js (right):

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart/_inter...
lib/runtime/dart/_internal.js:351: let _Transformation =
dart.typedef('_Transformation', () => dart.functionType(T, [S]));
On 2015/04/09 18:44:28, John Messerly wrote:
> so, the reason I made it a class ... it seemed like function types are
basically
> a special case:
> 
>     typedef R Foo(T t, S s);
> 
> is sort-of sugar for:
> 
>     class Foo {
>       R call(T t, S s);
>     }
> 
> ("sort-of" because nominal vs structural in type checks)

I had thought of this, but I found it confusing to conflate the two.  The
semantics are different - not just the nominal vs structural but the difference
in variance.  Type parameters are covariant in generics but not in typedefs.

> 
> So once we had method type signatures, function types would "fall out" for
free,
> and we wouldn't need two ways of emitting type signatures: we could just read
it
> from the "call" method. Granted, we'd still need to mark typedefs, so they are
> treated structurally instead of nominally :)
> 
> Taking this even further, in hypothetical ES7 we could do signatures like
this:
> 
>     function printLabel(obj: string) {}
> 
> so maybe just polyfill that?

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart/_inter...
lib/runtime/dart/_internal.js:503: return new (ExpandIterator$(S,
T))(this[_iterable][core.$iterator], dart.as(this[_f],
dart.functionType(core.Iterable$(T), [S])));
On 2015/04/09 18:44:28, John Messerly wrote:
> use functional ordering?

In Dart source (e.g., typedefs, function params), the return type is first.  :-)

> 
>     dart.functionType(S, core.Iterable$(T))
> 
> so the type of `TResult foo(TArg0 arg0, TArg1 arg1` would be ]
> 
>     dart.functionType(TArg0, TArg1, TResult)
> 
> optional and named args are a little wacky, but those could be similar:
> 
>     dart.functionType(TArg0, [TArg1, TArg2], TResult)
> 
>     dart.functionType(TArg0, {arg1: TArg1, arg2: TArg2}, TResult)

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart/_isola...
File lib/runtime/dart/_isolate_helper.js (right):

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart/_isola...
lib/runtime/dart/_isolate_helper.js:114: return new core.List.from(['map',
x.keys[core.$map](dart.as(serializeTearOff, dart.functionType(dynamic,
[dynamic])))[core.$toList](), x.values[core.$map](dart.as(serializeTearOff,
dart.functionType(dynamic, [dynamic])))[core.$toList]()]);
On 2015/04/09 18:44:28, John Messerly wrote:
> shouldn't this be `dart.dynamic`?

Done.

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart_runtim...
File lib/runtime/dart_runtime.js (right):

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart_runtim...
lib/runtime/dart_runtime.js:129: // TODO(vsm): Should we explicitly check if
this is a core.Object?
On 2015/04/09 18:44:28, John Messerly wrote:
> I wondered this too, but since we didn't need it yet, left it out. Not sure if
> it's worth adding a TODO? Presumably we can add if we need?

Changed the comment a bit.  I think my real question is whether / how we
differentiate between JS objects and Dart ones.

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart_runtim...
lib/runtime/dart_runtime.js:130: var result = obj[dart.runtimeType];
On 2015/04/09 18:44:28, John Messerly wrote:
> what does this do? are you trying to call the Object.runtimeType method or
...?

It's only used by tests are the moment.

I was using Object.runtimeType initially, but that triggers errors on actual
runtimeType getters (in the interceptor code) that don't work yet.

More generally, since we're allowing regular JS objects to get here, perhaps we
don't want runtimeType to be a public field?

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart_runtim...
lib/runtime/dart_runtime.js:201: if (_isTop(t2, dynamicIsBottom) ||
_isBottom(t1, dynamicIsBottom)) return true;
On 2015/04/09 18:44:28, John Messerly wrote:
> some long lines here

Done.  The dart formatter has made me lazy on this front.  :-)

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart_runtim...
lib/runtime/dart_runtime.js:212: // TODO(vsm): Handle Objects with call methods.
 Those are functions
On 2015/04/09 18:44:28, John Messerly wrote:
> does dart_runtime.dart handle this case?

Yes.  

> it wouldn't be hard to get the ".call" method here... but I guess the problem
is
> it won't have any type info on it yet. =)
> 
> you could do arity checks though?

I put this off as I don't have a good mental model of how we'll get this to work
(though the subtyping part isn't hard).  Perhaps we can discuss offline.

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart_runtim...
lib/runtime/dart_runtime.js:237: if (t1 == null) return t2 == core.Object || t2
== dart.dynamic;
On 2015/04/09 18:44:29, John Messerly wrote:
> Hmmm, canonicalType was supposed to be handling this, so we could avoid
worrying
> about things like that. But maybe it's broken, I see sunflower doesn't load on
> master. Doh. Looking at that.

Yeah, I was hitting this at runtime.

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart_runtim...
lib/runtime/dart_runtime.js:339: // TODO(vsm): Common type object?
On 2015/04/09 18:47:24, John Messerly wrote:
> On 2015/04/09 18:44:28, John Messerly wrote:
> > we could, it's kind of nice to reuse the JS type object when possible
though.
> > 
> > thinking longer term, but it'd be nice if Dart objects were envisioned as
part
> > of the ES6+ system and the Dart runtime features too. e.g. our generics and
> > mixins could be used as just a library by an ES6 user. It's pretty neat that
> for
> > a Dart Foo class, you can write `new Foo()` in ES and `foo instanceof Foo`
and
> > it all works, and you can also use our subtype library on JS classes. So
it'd
> be
> > nice if plain `Foo` was the type object. It's also nice in debugging, with
> > classes you can type Foo at the console and see the class def
> 
> having said that, we can certainly have a different internal representation
...
> just saying the public APIs should ideally accept JS type objects.

That's a good point.

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart_runtim...
lib/runtime/dart_runtime.js:340: if (type instanceof AbstractFunctionType)
return type.toString();
On 2015/04/09 18:44:29, John Messerly wrote:
> could you just set a `name` property on this?

Done.

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart_runtim...
lib/runtime/dart_runtime.js:343: return type.toString();
On 2015/04/09 18:44:28, John Messerly wrote:
> does this happen?

Doesn't seem to.  Changed the code to throw.

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

https://codereview.chromium.org/1076883003/diff/40001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:309: var result = js.statement('let # =
dart.typedef(#, #);', [
On 2015/04/09 18:44:29, John Messerly wrote:
> maybe a little cleaner:
> 
> 'let # = dart.typedef(#, () => #);'
> 
> and you can skip the manual `var closure = new JS.ArrowFun([], ...` line

Done.

https://codereview.chromium.org/1076883003/diff/40001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:316: // if (isPublic(name)) _addExport(name);
On 2015/04/09 18:44:29, John Messerly wrote:
> remove?

Done.

https://codereview.chromium.org/1076883003/diff/40001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:1116: JS.Expression _emitTypeName(DartType type,
[bool lowerTypedef = false]) {
On 2015/04/09 18:44:29, John Messerly wrote:
> named arg?

Done.

https://codereview.chromium.org/1076883003/diff/40001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:1118: // The void type is not defined in core.
On 2015/04/09 18:44:29, John Messerly wrote:
> dynamic too?

Done.

https://codereview.chromium.org/1076883003/diff/40001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:1126: // TODO(vsm): Support all parameter types.
On 2015/04/09 18:44:29, John Messerly wrote:
> can you elaborate on this?

Obsolete.  Removed.

[Jennifer Messerly, 2015-04-10 00:57:18]

lgtm

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart/_inter...
File lib/runtime/dart/_internal.js (right):

https://codereview.chromium.org/1076883003/diff/40001/lib/runtime/dart/_inter...
lib/runtime/dart/_internal.js:503: return new (ExpandIterator$(S,
T))(this[_iterable][core.$iterator], dart.as(this[_f],
dart.functionType(core.Iterable$(T), [S])));
On 2015/04/09 19:52:24, vsm wrote:
> On 2015/04/09 18:44:28, John Messerly wrote:
> > use functional ordering?
> 
> In Dart source (e.g., typedefs, function params), the return type is first. 
:-)
> 
> > 
> >     dart.functionType(S, core.Iterable$(T))
> > 
> > so the type of `TResult foo(TArg0 arg0, TArg1 arg1` would be ]
> > 
> >     dart.functionType(TArg0, TArg1, TResult)
> > 
> > optional and named args are a little wacky, but those could be similar:
> > 
> >     dart.functionType(TArg0, [TArg1, TArg2], TResult)
> > 
> >     dart.functionType(TArg0, {arg1: TArg1, arg2: TArg2}, TResult)
> 

i mean, kind of, but not really. In a function declaration, yes. At use sites?
We don't have an expression form for creating function types. If we did, it
might very well follow Arg -> Return type order. I mean consider C#: Func<S, T>
is a function that accepts an S and returns T. Similar is C#'s equivalent of
`map`: list.select<T,R>(arg) // takes a T, returns an R ... ditto Guava for
Java:
http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/base...
... I don't recall that ever coming up as a point of confusion. Especially
nowadays with lambdas: (arg1, arg2) => myResult ... the ordering feels very
natural for programmers.

[vsm, 2015-04-10 01:40:09]

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