Handle break as continue

Handle break as continue

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

[Johnni Winther, 2017-06-23 14:43:32]

johnniwinther@google.com changed reviewers:
+ sigmund@google.com

[Siggi Cherem (dart-lang), 2017-06-23 21:04:00]

this might be nice to test end-to-end, like we do with hello world in d8.

https://codereview.chromium.org/2954663002/diff/20001/pkg/compiler/lib/src/ke...
File pkg/compiler/lib/src/kernel/kernel_backend_strategy.dart (right):

https://codereview.chromium.org/2954663002/diff/20001/pkg/compiler/lib/src/ke...
pkg/compiler/lib/src/kernel/kernel_backend_strategy.dart:356: //assert(target !=
null, failedAt(currentMember, 'No target for $node.'));
delete altogether then?

https://codereview.chromium.org/2954663002/diff/20001/pkg/compiler/lib/src/ke...
pkg/compiler/lib/src/kernel/kernel_backend_strategy.dart:370: return
_jumpTargetMap[node];
remove .parent also in Do, ForIn, While?

https://codereview.chromium.org/2954663002/diff/20001/pkg/compiler/lib/src/ss...
File pkg/compiler/lib/src/ssa/builder_kernel.dart (right):

https://codereview.chromium.org/2954663002/diff/20001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/builder_kernel.dart:1472:
handler.generateContinue(handler.labels.first);
is this label in the right place?

or do we need to move it? ( `for (;;) l1: { ... }`  => `l1: for(;;) { ... } `)

https://codereview.chromium.org/2954663002/diff/20001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/builder_kernel.dart:1497: JumpTarget jumpTarget =
localsMap.getJumpTargetForLabel(labeledStatement);
I thought eventually the visitor was going to generate jump targets eagerly for
all labeled statements, is that not the case?

[Johnni Winther, 2017-06-26 11:27:56]

compile_from_dill_test is more thorough than run_from_dill_test (the former
requires identical output as well as an equivalent model)

Committed assuming LGTM after the added test.

https://codereview.chromium.org/2954663002/diff/20001/pkg/compiler/lib/src/ke...
File pkg/compiler/lib/src/kernel/kernel_backend_strategy.dart (right):

https://codereview.chromium.org/2954663002/diff/20001/pkg/compiler/lib/src/ke...
pkg/compiler/lib/src/kernel/kernel_backend_strategy.dart:356: //assert(target !=
null, failedAt(currentMember, 'No target for $node.'));
On 2017/06/23 21:03:59, Siggi Cherem (dart-lang) wrote:
> delete altogether then?

Done.

https://codereview.chromium.org/2954663002/diff/20001/pkg/compiler/lib/src/ke...
pkg/compiler/lib/src/kernel/kernel_backend_strategy.dart:370: return
_jumpTargetMap[node];
On 2017/06/23 21:03:59, Siggi Cherem (dart-lang) wrote:
> remove .parent also in Do, ForIn, While?

Will do when JumpVisitor recognizes these as break/continue targets.

https://codereview.chromium.org/2954663002/diff/20001/pkg/compiler/lib/src/ss...
File pkg/compiler/lib/src/ssa/builder_kernel.dart (right):

https://codereview.chromium.org/2954663002/diff/20001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/builder_kernel.dart:1472:
handler.generateContinue(handler.labels.first);
On 2017/06/23 21:03:59, Siggi Cherem (dart-lang) wrote:
> is this label in the right place?
> 
> or do we need to move it? ( `for (;;) l1: { ... }`  => `l1: for(;;) { ... } `)

It's the responsibility of the jump target (here the for-statement). Currently
labels are not supported.

https://codereview.chromium.org/2954663002/diff/20001/pkg/compiler/lib/src/ss...
pkg/compiler/lib/src/ssa/builder_kernel.dart:1497: JumpTarget jumpTarget =
localsMap.getJumpTargetForLabel(labeledStatement);
On 2017/06/23 21:03:59, Siggi Cherem (dart-lang) wrote:
> I thought eventually the visitor was going to generate jump targets eagerly
for
> all labeled statements, is that not the case?

No, only for those that _are_ targets. An untargetted label statement is
replaced by its body (as they are in ssa/builder)

[Johnni Winther, 2017-06-26 11:28:24]

Description was changed from

==========
Handle break as continue
==========

to

==========
Handle break as continue

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

[Johnni Winther, 2017-06-26 11:28:25]

Committed patchset #5 (id:80001) manually as
aa345515fef6c090dcf670c719c6aebf55a8be9a (presubmit successful).

[Siggi Cherem (dart-lang), 2017-06-26 16:42:39]

lgtm