Add primitive to create closures and use it for closure conversion

Add primitive to create closures and use it for closure conversion

A new AST node 'ClosureCreation' is added. It takes a name of a
top-level function, a context, and a closure function type and creates a
closure of the given type. The effect of this closure invocation is the
same as of the invocation of the given top-level function with the
contexts as the first argument.

In order to use 'ClosureCreation', the closure conversion pass now
transforms closures into top-level functions, not closure classes. These
functions receive the context as the first argument.

The type of the expression represented by 'ClosureCreation' is its third
parameter. It its the responsibility of closure conversion pass to
create the correct types for 'ClosureCreation' nodes based on types of
closures transformed into top-level functions.

R=asgerf@google.com

Committed: https://github.com/dart-lang/sdk/commit/42f82d1d2128161f402d5a69b20f82e7ee23f210

[Dmitry Stefantsov, 2017-03-31 10:31:22]

dmitryas@google.com changed reviewers:
+ asgerf@google.com, karlklose@google.com

[asgerf, 2017-03-31 11:03:52]

https://codereview.chromium.org/2778223002/diff/100001/pkg/kernel/binary.md
File pkg/kernel/binary.md (right):

https://codereview.chromium.org/2778223002/diff/100001/pkg/kernel/binary.md#n...
pkg/kernel/binary.md:686: String topLevelFunctionName;
This should be a MemberReference

https://codereview.chromium.org/2778223002/diff/100001/pkg/kernel/lib/ast.dart
File pkg/kernel/lib/ast.dart (right):

https://codereview.chromium.org/2778223002/diff/100001/pkg/kernel/lib/ast.dar...
pkg/kernel/lib/ast.dart:2862: String topLevelFunctionName;
Again, please use a reference to the member

https://codereview.chromium.org/2778223002/diff/100001/pkg/kernel/lib/transfo...
File pkg/kernel/lib/transformations/closure/converter.dart (right):

https://codereview.chromium.org/2778223002/diff/100001/pkg/kernel/lib/transfo...
pkg/kernel/lib/transformations/closure/converter.dart:379:
.toList()..removeAt(0),
How about positionalParameters.skip(1).map(...)?  Or just build the type before
mutating the function parameter list.

[Dmitry Stefantsov, 2017-03-31 11:54:13]

PTAL

https://codereview.chromium.org/2778223002/diff/100001/pkg/kernel/binary.md
File pkg/kernel/binary.md (right):

https://codereview.chromium.org/2778223002/diff/100001/pkg/kernel/binary.md#n...
pkg/kernel/binary.md:686: String topLevelFunctionName;
On 2017/03/31 11:03:41, asgerf wrote:
> This should be a MemberReference

Thanks! Done.

https://codereview.chromium.org/2778223002/diff/100001/pkg/kernel/lib/ast.dart
File pkg/kernel/lib/ast.dart (right):

https://codereview.chromium.org/2778223002/diff/100001/pkg/kernel/lib/ast.dar...
pkg/kernel/lib/ast.dart:2862: String topLevelFunctionName;
On 2017/03/31 11:03:52, asgerf wrote:
> Again, please use a reference to the member

Done.

https://codereview.chromium.org/2778223002/diff/100001/pkg/kernel/lib/transfo...
File pkg/kernel/lib/transformations/closure/converter.dart (right):

https://codereview.chromium.org/2778223002/diff/100001/pkg/kernel/lib/transfo...
pkg/kernel/lib/transformations/closure/converter.dart:379:
.toList()..removeAt(0),
On 2017/03/31 11:03:52, asgerf wrote:
> How about positionalParameters.skip(1).map(...)?  Or just build the type
before
> mutating the function parameter list.

I like the 'skip' approach better. Done. Thanks!

[asgerf, 2017-03-31 12:08:43]

Just a few more things :)

https://codereview.chromium.org/2778223002/diff/140001/pkg/kernel/lib/ast.dart
File pkg/kernel/lib/ast.dart (right):

https://codereview.chromium.org/2778223002/diff/140001/pkg/kernel/lib/ast.dar...
pkg/kernel/lib/ast.dart:2862: Reference topLevelFunctionReference;
Great!

To be consistent with the other AST nodes:
- Add a getter/setter pair for direct access to the Member
- Let the constructor take the Member as argument
- Add a .byReference constructor.

See the StaticGet class for an example.

https://codereview.chromium.org/2778223002/diff/140001/pkg/kernel/lib/binary/...
File pkg/kernel/lib/binary/ast_to_binary.dart (right):

https://codereview.chromium.org/2778223002/diff/140001/pkg/kernel/lib/binary/...
pkg/kernel/lib/binary/ast_to_binary.dart:756:
writeMemberReference(node.topLevelFunctionReference.asMember);
Please use:
writeReference(node.topLevelFunctionReference)

In fact, writeMemberReference ought to be removed.

https://codereview.chromium.org/2778223002/diff/140001/pkg/kernel/lib/text/as...
File pkg/kernel/lib/text/ast_to_text.dart (right):

https://codereview.chromium.org/2778223002/diff/140001/pkg/kernel/lib/text/as...
pkg/kernel/lib/text/ast_to_text.dart:1030:
writeMemberReference(node.topLevelFunctionReference.asMember);
When adding the convenience getter, please use that here.

[Dmitry Stefantsov, 2017-03-31 12:40:18]

Thanks for the new comments! Hope, this time I got it right.

PTAL.

https://codereview.chromium.org/2778223002/diff/140001/pkg/kernel/lib/ast.dart
File pkg/kernel/lib/ast.dart (right):

https://codereview.chromium.org/2778223002/diff/140001/pkg/kernel/lib/ast.dar...
pkg/kernel/lib/ast.dart:2862: Reference topLevelFunctionReference;
On 2017/03/31 12:08:43, asgerf wrote:
> Great!
> 
> To be consistent with the other AST nodes:
> - Add a getter/setter pair for direct access to the Member
> - Let the constructor take the Member as argument
> - Add a .byReference constructor.
> 
> See the StaticGet class for an example.

Good idea! Obviously, I didn't payed attention to this pattern. Fixed. Thanks!

https://codereview.chromium.org/2778223002/diff/140001/pkg/kernel/lib/binary/...
File pkg/kernel/lib/binary/ast_to_binary.dart (right):

https://codereview.chromium.org/2778223002/diff/140001/pkg/kernel/lib/binary/...
pkg/kernel/lib/binary/ast_to_binary.dart:756:
writeMemberReference(node.topLevelFunctionReference.asMember);
On 2017/03/31 12:08:43, asgerf wrote:
> Please use:
> writeReference(node.topLevelFunctionReference)
> 
> In fact, writeMemberReference ought to be removed.

Thanks! Done.

https://codereview.chromium.org/2778223002/diff/140001/pkg/kernel/lib/text/as...
File pkg/kernel/lib/text/ast_to_text.dart (right):

https://codereview.chromium.org/2778223002/diff/140001/pkg/kernel/lib/text/as...
pkg/kernel/lib/text/ast_to_text.dart:1030:
writeMemberReference(node.topLevelFunctionReference.asMember);
On 2017/03/31 12:08:43, asgerf wrote:
> When adding the convenience getter, please use that here.

Thanks! Done.

[asgerf, 2017-03-31 12:42:12]

LGTM

[Dmitry Stefantsov, 2017-03-31 12:44:11]

Description was changed from

==========
Add primitive to create closures and use it for closure conversion

A new AST node 'ClosureCreation' is added. It takes a name of a
top-level function, a context, and a closure function type and creates a
closure of the given type. The effect of this closure invocation is the
same as of the invocation of the given top-level function with the
contexts as the first argument.

In order to use 'ClosureCreation', the closure conversion pass now
transforms closures into top-level functions, not closure classes. These
functions receive the context as the first argument.

The type of the expression represented by 'ClosureCreation' is its third
parameter. It its the responsibility of closure conversion pass to
create the correct types for 'ClosureCreation' nodes based on types of
closures transformed into top-level functions.
==========

to

==========
Add primitive to create closures and use it for closure conversion

A new AST node 'ClosureCreation' is added. It takes a name of a
top-level function, a context, and a closure function type and creates a
closure of the given type. The effect of this closure invocation is the
same as of the invocation of the given top-level function with the
contexts as the first argument.

In order to use 'ClosureCreation', the closure conversion pass now
transforms closures into top-level functions, not closure classes. These
functions receive the context as the first argument.

The type of the expression represented by 'ClosureCreation' is its third
parameter. It its the responsibility of closure conversion pass to
create the correct types for 'ClosureCreation' nodes based on types of
closures transformed into top-level functions.

R=asgerf@google.com

Committed:
https://github.com/dart-lang/sdk/commit/42f82d1d2128161f402d5a69b20f82e7ee23f210
==========

[Dmitry Stefantsov, 2017-03-31 12:44:13]

Committed patchset #9 (id:160001) manually as
42f82d1d2128161f402d5a69b20f82e7ee23f210 (presubmit successful).