Refactor rewrite_async

Refactor rewrite_async

Split AsyncRewriter into a base class and threee subclasses.

Avoid using -name variables everywhere.
BUG=
R=floitsch@google.com

Committed: https://code.google.com/p/dart/source/detail?r=44085

[sigurdm, 2015-02-25 11:43:18]

sigurdm@google.com changed reviewers:
+ floitsch@google.com

[sigurdm, 2015-02-25 11:43:18]

https://codereview.chromium.org/953283003/diff/60001/pkg/compiler/lib/src/js/...
File pkg/compiler/lib/src/js/rewrite_async.dart (right):

https://codereview.chromium.org/953283003/diff/60001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/rewrite_async.dart:487: js.Fun
generateFunction(List<js.Parameter> parameters,
Needs a better name...

[floitsch, 2015-02-25 16:28:00]

LGTM.

https://codereview.chromium.org/953283003/diff/60001/pkg/compiler/lib/src/js/...
File pkg/compiler/lib/src/js/rewrite_async.dart (right):

https://codereview.chromium.org/953283003/diff/60001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/rewrite_async.dart:472: /// Emits the return block that
all returns jumps to (after going
jump

https://codereview.chromium.org/953283003/diff/60001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/rewrite_async.dart:487: js.Fun
generateFunction(List<js.Parameter> parameters,
On 2015/02/25 11:43:18, sigurdm wrote:
> Needs a better name...

finishFunction ?

https://codereview.chromium.org/953283003/diff/60001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/rewrite_async.dart:678: // TODO(sigurdm): Explain the
difference between syncStar and the other
I think you can remove that todo now.

https://codereview.chromium.org/953283003/diff/60001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/rewrite_async.dart:733: js.Expression
visitAwait(js.Await node) {
This should probably go into an AsyncRewriterBase.
The sync* shouldn't need to deal with it.
But not sure if it's worth the effort...

https://codereview.chromium.org/953283003/diff/60001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/rewrite_async.dart:1555: void
visitDartYield(js.DartYield node) {
Ideally this one, too, would only be shared by the two subclasses that need it.
Since there is already a distinction with "await" you would need a mixin.
Not really sure it's worth the effort.

https://codereview.chromium.org/953283003/diff/60001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/rewrite_async.dart:1556: print(this.runtimeType);
debug print.

https://codereview.chromium.org/953283003/diff/60001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/rewrite_async.dart:1645: /// Returning from an async
method calls the [asyncStarHelper] with the result.
long line.

https://codereview.chromium.org/953283003/diff/60001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/rewrite_async.dart:1697: js.Statement rewrittenBody,
indentation.

https://codereview.chromium.org/953283003/diff/60001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/rewrite_async.dart:1707: while (true)
In the next? CL please move the "while" to the rewrittenBody. (unless it's
surprisingly hard).

[sigurdm, 2015-02-27 09:46:33]

https://codereview.chromium.org/953283003/diff/60001/pkg/compiler/lib/src/js/...
File pkg/compiler/lib/src/js/rewrite_async.dart (right):

https://codereview.chromium.org/953283003/diff/60001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/rewrite_async.dart:472: /// Emits the return block that
all returns jumps to (after going
On 2015/02/25 16:27:59, floitsch wrote:
> jump

Done.

https://codereview.chromium.org/953283003/diff/60001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/rewrite_async.dart:472: /// Emits the return block that
all returns jumps to (after going
On 2015/02/25 16:27:59, floitsch wrote:
> jump

Done.

https://codereview.chromium.org/953283003/diff/60001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/rewrite_async.dart:487: js.Fun
generateFunction(List<js.Parameter> parameters,
On 2015/02/25 16:27:59, floitsch wrote:
> On 2015/02/25 11:43:18, sigurdm wrote:
> > Needs a better name...
> 
> finishFunction ?

Done.

https://codereview.chromium.org/953283003/diff/60001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/rewrite_async.dart:678: // TODO(sigurdm): Explain the
difference between syncStar and the other
On 2015/02/25 16:27:59, floitsch wrote:
> I think you can remove that todo now.

Done.

https://codereview.chromium.org/953283003/diff/60001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/rewrite_async.dart:733: js.Expression
visitAwait(js.Await node) {
On 2015/02/25 16:27:59, floitsch wrote:
> This should probably go into an AsyncRewriterBase.
> The sync* shouldn't need to deal with it.
> But not sure if it's worth the effort...

Left it for now

https://codereview.chromium.org/953283003/diff/60001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/rewrite_async.dart:1555: void
visitDartYield(js.DartYield node) {
On 2015/02/25 16:27:59, floitsch wrote:
> Ideally this one, too, would only be shared by the two subclasses that need
it.
> Since there is already a distinction with "await" you would need a mixin.
> Not really sure it's worth the effort.

Left it for now

https://codereview.chromium.org/953283003/diff/60001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/rewrite_async.dart:1556: print(this.runtimeType);
On 2015/02/25 16:27:59, floitsch wrote:
> debug print.

Done.

https://codereview.chromium.org/953283003/diff/60001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/rewrite_async.dart:1645: /// Returning from an async
method calls the [asyncStarHelper] with the result.
On 2015/02/25 16:27:59, floitsch wrote:
> long line.

Done.

https://codereview.chromium.org/953283003/diff/60001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/rewrite_async.dart:1697: js.Statement rewrittenBody,
On 2015/02/25 16:27:59, floitsch wrote:
> indentation.

Done.

https://codereview.chromium.org/953283003/diff/60001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/rewrite_async.dart:1707: while (true)
On 2015/02/25 16:27:59, floitsch wrote:
> In the next? CL please move the "while" to the rewrittenBody. (unless it's
> surprisingly hard).

Did it here.

[sigurdm, 2015-02-27 09:55:23]

Committed patchset #7 (id:120001) manually as 44085 (presubmit successful).