implement generic method runtime behavior, fixes #301

implement generic method runtime behavior, fixes #301

R=leafp@google.com, vsm@google.com

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

[Jennifer Messerly, 2016-04-28 22:58:27]

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

[Jennifer Messerly, 2016-04-28 22:58:28]

This fixes https://github.com/dart-lang/dev_compiler/issues/301.

See for issues that are not implemented yet:
https://github.com/dart-lang/dev_compiler/issues/435

[Jennifer Messerly, 2016-04-28 23:00:09]

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

[Jennifer Messerly, 2016-04-28 23:00:10]

FYI to Leaf as well, in case interested to see it

[Jennifer Messerly, 2016-04-28 23:28:54]

https://codereview.chromium.org/1926283002/diff/40001/tool/input_sdk/private/...
File tool/input_sdk/private/ddc_runtime/operations.dart (right):

https://codereview.chromium.org/1926283002/diff/40001/tool/input_sdk/private/...
tool/input_sdk/private/ddc_runtime/operations.dart:124: $typeArgs =
Array(formalCount).fill($dynamicR);
This should be instantiating to bounds.
Related to: https://github.com/dart-lang/dev_compiler/issues/532

Let me know, I try and fix it in this version or split off into its own bug.

Perhaps something like:

    dart.genericBounds((T) => [T, [T, T]], [core.num])

(e.g. for `min` in dart:math)

[vsm, 2016-04-29 14:48:53]

LGTM

General thought: the dollar naming convention for classes might be nice here as
a visual hint to the user.

Also (not this CL) might be good to pull in erik's new generic method lang tests
(https://github.com/dart-lang/sdk/commit/4dd22a87bb43f2ef2d0ce755d6528cc6a7afc0a0).

https://codereview.chromium.org/1926283002/diff/40001/lib/src/compiler/code_g...
File lib/src/compiler/code_generator.dart (right):

https://codereview.chromium.org/1926283002/diff/40001/lib/src/compiler/code_g...
lib/src/compiler/code_generator.dart:1841: JS.FunctionExpression/*=T*/
_makeGenericFunction/*<T extends JS.FunctionExpression>*/(
nit len

https://codereview.chromium.org/1926283002/diff/40001/lib/src/compiler/code_g...
lib/src/compiler/code_generator.dart:1851: return new JS.Fun(
Could this be an ArrayFun?

https://codereview.chromium.org/1926283002/diff/40001/lib/src/compiler/code_g...
lib/src/compiler/code_generator.dart:2154: // different from universal function
types.
do you want to file a bug?  is there a language open here?

[vsm, 2016-04-29 14:51:20]

jakemac will test ng2 when this lands.

[Leaf, 2016-04-29 15:55:03]

lgtm

https://codereview.chromium.org/1926283002/diff/40001/lib/src/compiler/code_g...
File lib/src/compiler/code_generator.dart (right):

https://codereview.chromium.org/1926283002/diff/40001/lib/src/compiler/code_g...
lib/src/compiler/code_generator.dart:2154: // different from universal function
types.
On 2016/04/29 14:48:53, vsm wrote:
> do you want to file a bug?  is there a language open here?

There's active language discussion about how to sort out the universal function
type syntax.

https://codereview.chromium.org/1926283002/diff/40001/tool/input_sdk/private/...
File tool/input_sdk/private/ddc_runtime/types.dart (right):

https://codereview.chromium.org/1926283002/diff/40001/tool/input_sdk/private/...
tool/input_sdk/private/ddc_runtime/types.dart:251: // compiler instead, at a
slight cost to code size.
If we did this in the compiler, could we generate this as a function taking an
exact number of parameters, and then use reflection to get the type formal
count?  Not sure if it matter, just trying to understand the options.

[Jennifer Messerly, 2016-04-29 17:18:44]

Thanks, didn't know about those tests, I will add a note to our tracking bug

https://codereview.chromium.org/1926283002/diff/40001/lib/src/compiler/code_g...
File lib/src/compiler/code_generator.dart (right):

https://codereview.chromium.org/1926283002/diff/40001/lib/src/compiler/code_g...
lib/src/compiler/code_generator.dart:1841: JS.FunctionExpression/*=T*/
_makeGenericFunction/*<T extends JS.FunctionExpression>*/(
On 2016/04/29 14:48:53, vsm wrote:
> nit len

fixed. I guess dartfmt can't help until we have generic methods

https://codereview.chromium.org/1926283002/diff/40001/lib/src/compiler/code_g...
lib/src/compiler/code_generator.dart:1851: return new JS.Fun(
On 2016/04/29 14:48:53, vsm wrote:
> Could this be an ArrayFun?

it preserves the kind of the incoming function:
* if it was an =>, it stays => (see 4 lines above)
* otherwise it stays a Fun.

When building a JS AST we need raw JS.Funs sometimes, such as to fit in a
JS.Method. So changing the type wouldn't be valid. If an => could have been
used, the outside code should've already done that.

https://codereview.chromium.org/1926283002/diff/40001/lib/src/compiler/code_g...
lib/src/compiler/code_generator.dart:2154: // different from universal function
types.
On 2016/04/29 15:55:03, Leaf wrote:
> On 2016/04/29 14:48:53, vsm wrote:
> > do you want to file a bug?  is there a language open here?
> 
> There's active language discussion about how to sort out the universal
function
> type syntax.

Haha, yeah. Funny when theory and implementation collide like that. :)

We could implement these more like the universal types, then just treat the use
of the typedef with no type args as instantiated w/ dynamic.

Not sure it's worth changing though.

https://codereview.chromium.org/1926283002/diff/40001/tool/input_sdk/private/...
File tool/input_sdk/private/ddc_runtime/types.dart (right):

https://codereview.chromium.org/1926283002/diff/40001/tool/input_sdk/private/...
tool/input_sdk/private/ddc_runtime/types.dart:251: // compiler instead, at a
slight cost to code size.
On 2016/04/29 15:55:03, Leaf wrote:
> If we did this in the compiler, could we generate this as a function taking an
> exact number of parameters, and then use reflection to get the type formal
> count?  Not sure if it matter, just trying to understand the options.

yup, exactly! They would just be functions:

(T) => dart.definiteFunctionType(T, [T])

    instead of:

dart.definiteFunctionType((T) => [T, [T]])


I started that way, but it involved a lot of changes to the method signatures:

    'map': [T => [core.Iterable$(T), [dart.functionType(T, [E])]]

(now that I look at it, I'm not sure what that extra [] is for on the outside
...)

... vs ...

    'map': dart.definiteFunctionType(T => [core.Iterable$(T),
[dart.functionType(T, [E])])

The second one is better IMO, but it involved a bit more runtime helper logic
refactoring, which I found hard to get right (they're all untyped :) ... dynamic
language+slow iteration cycle makes for tough refactoring).

I may take a look after though, if it sounds good to you? Now I know that tests
are passing it'll be easier to do that kind of change I think.

[Jennifer Messerly, 2016-04-29 17:41:52]

Description was changed from

==========
implement generic method runtime behavior, fixes #301
==========

to

==========
implement generic method runtime behavior, fixes #301

R=leafp@google.com, vsm@google.com

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

[Jennifer Messerly, 2016-04-29 17:41:53]

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