Transform async code only for source libraries.

Transform async code only for source libraries.

R=ahe@google.com, kmillikin@google.com, paulberry@google.com, sigmund@google.com
BUG=

Committed: https://github.com/dart-lang/sdk/commit/2b8fd9dee81a4004cfe3eff33898c00787253893

[Paul Berry, 2017-05-23 21:41:44]

lgtm assuming Siggi or Peter agrees

[Siggi Cherem (dart-lang), 2017-05-23 22:06:18]

Overall looks good with comment below. Please wait to hear from Peter/Kevin on
the small questions I added below, just to be safe.


This CL actually hits on a topic Peter and I were discussing yesterday.

We want to change how to invoke transformations to prevent us from duplicating
how kernel internally bundles transformations for each target (we ran into an
issue because of this earlier this week).

The idea we discussed was that instead of calling individual transformers,
delegate to the [backendTarget] (which is a Target from package:kernel) by
calling:

  backendTarget.performModularTransformations();
  backendTarget.performGlobalTransformations(); // **

From looking at how performModularTransformations and
performGlobalTransformations are implemented, it seems like the changes you did
to transformAsync will be needed anyways, so I'm OK landing what you have here
(after addressing the comments below) as a first step and then doing that API
switch as a second step.

The second step then would require that we:
 (a) extend [Target.perform*Transformations] in package:kernel to customize
which libraries to apply the transformation to. This could be by adding a filter
like you did, or by having 2 APIs (performModularTransformationsOnProgram and
performModularTransformationsOnLibraries).

 (b) change fasta to call transformers using those APIs in backendTarget.


** I'm guessing with time we should have no global transformations - right now
some transformations are run in the global step, even though I would expect them
to be in the modular step (like async and erasure).

https://codereview.chromium.org/2905463002/diff/1/pkg/front_end/lib/src/incre...
File pkg/front_end/lib/src/incremental_kernel_generator_impl.dart (right):

https://codereview.chromium.org/2905463002/diff/1/pkg/front_end/lib/src/incre...
pkg/front_end/lib/src/incremental_kernel_generator_impl.dart:304: void
runBuildTransformations() {
Not sure if we need this subclass: even outside the incremental generator, I
believe we should only apply transformations to sourcelibraries and not to code
loaded from the dillTarget (since we overall expect the dillTarget to only have
outlines anyways)

I'd be inclined to simply modify the call to transformProgram directly in
KernelTarget. Peter - do you agree?

https://codereview.chromium.org/2905463002/diff/1/pkg/kernel/lib/transformati...
File pkg/kernel/lib/transformations/continuation.dart (right):

https://codereview.chromium.org/2905463002/diff/1/pkg/kernel/lib/transformati...
pkg/kernel/lib/transformations/continuation.dart:15: Program
transformProgram(Program program,
API design question for Kevin/Peter:

Would you rather have:
  Program transformProgram(Program program, filter);

Or

  void transformLibraries(List<Library> libraries);


I'm fine with either at this time, but have a slight preference for the latter
when it comes to modular transformations: you don't need a program if you are
applying a modular transformation.

(this relates to my general comment on the CL, we'll have similar API choice
when we switch to use Target.performModularTransformations.)

[Siggi Cherem (dart-lang), 2017-05-23 22:34:18]

+kevin for the API question

[scheglov, 2017-05-24 00:07:54]

PTAL

Thank you, Siggi.
I made moved the changes into KernelTarget and change to use
transformLibraries() instead.

[Kevin Millikin (Google), 2017-05-24 10:31:06]

kmillikin@google.com changed reviewers:
+ kmillikin@google.com

[Kevin Millikin (Google), 2017-05-24 10:31:06]

Kernel changes LGTM.

[ahe, 2017-05-24 12:15:55]

lgtm

I think this makes sense as a step in the right direction, but that we need more
in this area.

Ideally, we would want to avoid transformations, but if that's not feasible,
have them applied immediately after BodyBuilder. This should work for at least
async, closure conversion, and erasure.

https://codereview.chromium.org/2905463002/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/kernel/kernel_target.dart (left):

https://codereview.chromium.org/2905463002/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/kernel/kernel_target.dart:673: // TODO(kmillikin):
Make this run on a per-method basis.
I think this TODO still applies.

https://codereview.chromium.org/2905463002/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/kernel/kernel_target.dart (right):

https://codereview.chromium.org/2905463002/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/kernel/kernel_target.dart:43: import
'package:kernel/core_types.dart';
Please use show.

[scheglov, 2017-05-24 22:45:51]

https://codereview.chromium.org/2905463002/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/kernel/kernel_target.dart (left):

https://codereview.chromium.org/2905463002/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/kernel/kernel_target.dart:673: // TODO(kmillikin):
Make this run on a per-method basis.
On 2017/05/24 12:15:55, ahe wrote:
> I think this TODO still applies.

Done.

https://codereview.chromium.org/2905463002/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/kernel/kernel_target.dart (right):

https://codereview.chromium.org/2905463002/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/kernel/kernel_target.dart:43: import
'package:kernel/core_types.dart';
On 2017/05/24 12:15:55, ahe wrote:
> Please use show.

Done.

[scheglov, 2017-05-24 22:51:25]

Description was changed from

==========
Transform async code only for source libraries.

R=ahe@google.com, paulberry@google.com, sigmund@google.com
BUG=
==========

to

==========
Transform async code only for source libraries.

R=ahe@google.com, kmillikin@google.com, paulberry@google.com, sigmund@google.com
BUG=

Committed:
https://github.com/dart-lang/sdk/commit/2b8fd9dee81a4004cfe3eff33898c00787253893
==========

[scheglov, 2017-05-24 22:51:27]

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