Support generic method syntax (fixes #25869).

Support generic method syntax (fixes #25869).
Function type parameters are parsed and resolved, but mapped to malformed types
(i.e. dynamic) until full support is implemented.

R=hausner@google.com

Committed: https://github.com/dart-lang/sdk/commit/11562f687433201e25fdb24842864cf0ccf42d98

[regis, 2016-09-16 01:12:50]

regis@google.com changed reviewers:
+ asiva@google.com, hausner@google.com

[regis, 2016-09-16 01:12:51]

Matthias, Siva,

For now, I have added a new field to Function in order to hold type parameters.
We can discuss whether this is acceptable, temporarily acceptable, or whether we
need a better solution before committing.

Thanks,
Regis

[hausner, 2016-09-19 20:28:48]

LGTM. Nicely integrated.

https://codereview.chromium.org/2349593003/diff/1/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/2349593003/diff/1/runtime/vm/parser.cc#newcod...
runtime/vm/parser.cc:5447: void Parser::ParseTypeParameters(bool
parameterizing_class) {
Would it make sense to pass the class or function instead of a bool value here?
The call sites would be more readable, and the implicit assumption that
current_class() and innermost_function() contain the object to be parametrized
could be dropped.

https://codereview.chromium.org/2349593003/diff/1/runtime/vm/parser.cc#newcod...
runtime/vm/parser.cc:5750: result_type =
ParseType(ClassFinalizer::kDoNotResolve);
In other places you added a comment that explains why we can't resolve the type
yet. Maybe here too?

[regis, 2016-09-19 22:02:30]

Thanks!

I also resolved the issue of a mixin application defined across libraries.

https://codereview.chromium.org/2349593003/diff/1/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/2349593003/diff/1/runtime/vm/parser.cc#newcod...
runtime/vm/parser.cc:5447: void Parser::ParseTypeParameters(bool
parameterizing_class) {
On 2016/09/19 20:28:48, hausner wrote:
> Would it make sense to pass the class or function instead of a bool value
here?
> The call sites would be more readable, and the implicit assumption that
> current_class() and innermost_function() contain the object to be parametrized
> could be dropped.

Actually, I tried, but realized that other calls in here like ParseType() and
ResolveType() also rely on current_class() and innermost_function() to be
correctly set. Let me add a comment at each call site instead.

https://codereview.chromium.org/2349593003/diff/1/runtime/vm/parser.cc#newcod...
runtime/vm/parser.cc:5750: result_type =
ParseType(ClassFinalizer::kDoNotResolve);
On 2016/09/19 20:28:48, hausner wrote:
> In other places you added a comment that explains why we can't resolve the
type
> yet. Maybe here too?

Done.

[regis, 2016-09-19 22:17:36]

Description was changed from

==========
Support generic method syntax (fixes #25869).
Function type parameters are parsed and resolved, but mapped to malformed types
(i.e. dynamic) until full support is implemented.
==========

to

==========
Support generic method syntax (fixes #25869).
Function type parameters are parsed and resolved, but mapped to malformed types
(i.e. dynamic) until full support is implemented.

R=hausner@google.com

Committed:
https://github.com/dart-lang/sdk/commit/11562f687433201e25fdb24842864cf0ccf42d98
==========

[regis, 2016-09-19 22:17:37]

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

[siva, 2016-09-19 23:23:46]

DBC.

https://codereview.chromium.org/2349593003/diff/20001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/2349593003/diff/20001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:5897: Function& function = thread->FunctionHandle();
REUSABLE_FUNCTION_HANDLESCOPE(thread);

needed here as thread->FunctionHandle() is being used.

https://codereview.chromium.org/2349593003/diff/20001/runtime/vm/object.h
File runtime/vm/object.h (right):

https://codereview.chromium.org/2349593003/diff/20001/runtime/vm/object.h#new...
runtime/vm/object.h:2301: }
Do we really want this version of NumTypeParmeters? it seems to be used in only
one place IsGeneric, why not pass Thread::Current() there. That way it would
avoid future use without the thread argument.

We are planning on similarly getting rid of the Handle() without the zone
parameter but it is taking forever as more new code keeps getting added.

https://codereview.chromium.org/2349593003/diff/20001/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/2349593003/diff/20001/runtime/vm/parser.cc#ne...
runtime/vm/parser.cc:2147: signature_function.set_data(Object::Handle());
Handle(Z) possible here?

[regis, 2016-09-23 00:35:24]

I forgot to send these replies to Siva's comments.
They have been addressed in this cl committed 2 days ago:
https://codereview.chromium.org/2359543003/

https://codereview.chromium.org/2349593003/diff/20001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/2349593003/diff/20001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:5897: Function& function = thread->FunctionHandle();
On 2016/09/19 23:23:46, siva wrote:
> REUSABLE_FUNCTION_HANDLESCOPE(thread);
> 
> needed here as thread->FunctionHandle() is being used.

Done.

https://codereview.chromium.org/2349593003/diff/20001/runtime/vm/object.h
File runtime/vm/object.h (right):

https://codereview.chromium.org/2349593003/diff/20001/runtime/vm/object.h#new...
runtime/vm/object.h:2301: }
On 2016/09/19 23:23:46, siva wrote:
> Do we really want this version of NumTypeParmeters? it seems to be used in
only
> one place IsGeneric, why not pass Thread::Current() there. That way it would
> avoid future use without the thread argument.
> 
> We are planning on similarly getting rid of the Handle() without the zone
> parameter but it is taking forever as more new code keeps getting added.

Done.

https://codereview.chromium.org/2349593003/diff/20001/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/2349593003/diff/20001/runtime/vm/parser.cc#ne...
runtime/vm/parser.cc:2147: signature_function.set_data(Object::Handle());
On 2016/09/19 23:23:46, siva wrote:
> Handle(Z) possible here?

Done.