dart2js: CPS translation of switches with continue to their labels.

dart2js: CPS translation of switches with continue to their labels.

Implement switches with continue to their labels by a first 'non-recursive'
switch followed by a second (only) 'recursive' switch inside a loop.
Continue is implemented by assignment to a state variable that the second
switch switches over.

R=asgerf@google.com

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

[Kevin Millikin (Google), 2016-01-19 10:41:45]

Description was changed from

==========
dart2js: A more useful implementation of translation of switch statements.

The separation of AST and IR is clumsy for a reusable translation of switch
statements.  Instead, use a helper that builds the translation of the entire
switch given functions performing translations of the conditions and bodies.

BUG=
==========

to

==========
dart2js: CPS translation of switches with continue to their labels.

Implement switches with continue to their labels by a first 'non-recursive'
switch followed by a second (only) 'recursive' switch inside a loop.
Continue is implemented by assignment to a state variable that the second
switch switches over.
==========

[Kevin Millikin (Google), 2016-01-19 10:41:45]

kmillikin@google.com changed reviewers:
+ asgerf@google.com, sra@google.com

[Kevin Millikin (Google), 2016-01-19 10:44:37]

https://codereview.chromium.org/1585503002/diff/80001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart (right):

https://codereview.chromium.org/1585503002/diff/80001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart:1629: void
buildSimpleSwitch(JumpCollector join,
This is a more useful way to structure this helper: as one that takes a list of
pairs of functions to translate case conditions and bodies, and a function that
translates the default body.

[asgerf, 2016-01-19 22:45:25]

Please add clarifying comments to the code with examples.

This type of code is inevitably going to be complex and at the high level I
think the complexity has been contained quite nicely.  But from a more low-level
view I just can't follow the logic at every step. The branch conditions I
pointed out are especially unintuitive.

https://codereview.chromium.org/1585503002/diff/80001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart (right):

https://codereview.chromium.org/1585503002/diff/80001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart:1640: // A LetCont.tw0 term has
a hole as the body of the first listed
tw0 -> two

https://codereview.chromium.org/1585503002/diff/80001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart (right):

https://codereview.chromium.org/1585503002/diff/80001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:1229: // continued to
themselves.
I don't understand what's going one here, and the comment is not helping.

It seems to me that code:

case 1: continue LABEL;
LABEL:
default: BODY

gets translated to:

case 1: continue LABEL;
LABEL:
default: continue LABEL; // discard BODY

It would help a lot with some pseudo-code as an example of what case we are
looking at.

https://codereview.chromium.org/1585503002/diff/80001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:1275: // Recursive cases
are as if they immediately continued to themselves.
Same as above, I just can't tell what the branch condition means.

https://codereview.chromium.org/1585503002/diff/80001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/cps_ir_nodes_sexpr.dart (right):

https://codereview.chromium.org/1585503002/diff/80001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_ir_nodes_sexpr.dart:53: return node is
Expression
Dead code, null is not an expression.

[Kevin Millikin (Google), 2016-01-25 12:46:37]

https://codereview.chromium.org/1585503002/diff/80001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart (right):

https://codereview.chromium.org/1585503002/diff/80001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_ir_builder.dart:1640: // A LetCont.tw0 term has
a hole as the body of the first listed
On 2016/01/19 22:45:24, asgerf wrote:
> tw0 -> two

Done.

https://codereview.chromium.org/1585503002/diff/80001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart (right):

https://codereview.chromium.org/1585503002/diff/80001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_ir_builder_task.dart:1229: // continued to
themselves.
Sorry.  I originally had such an example but I didn't think it helped much. 
There are too many different possibilities with continue in branches, loops,
etc. that it seemed better just to describe the translation compositionally.

I've tried to add an example.

You're basically right, except that there is no LABEL in the translation:

var state = -1;
switch (...) {
  ...
  case 1: state = N; break;
  default: state = N; break;  // discard (?) BODY
}
while (state != -1) {
  switch (state) {
    ...
    case N: BODY
  }
}

Maybe "continue to itself" is too cute, but that's how I think of it.

https://codereview.chromium.org/1585503002/diff/80001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/cps_ir_nodes_sexpr.dart (right):

https://codereview.chromium.org/1585503002/diff/80001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_ir_nodes_sexpr.dart:53: return node is
Expression
On 2016/01/19 22:45:24, asgerf wrote:
> Dead code, null is not an expression.

Done.

[asgerf, 2016-01-26 08:05:09]

Thanks, that helped. LGTM.

[Kevin Millikin (Google), 2016-01-26 08:43:23]

Description was changed from

==========
dart2js: CPS translation of switches with continue to their labels.

Implement switches with continue to their labels by a first 'non-recursive'
switch followed by a second (only) 'recursive' switch inside a loop.
Continue is implemented by assignment to a state variable that the second
switch switches over.
==========

to

==========
dart2js: CPS translation of switches with continue to their labels.

Implement switches with continue to their labels by a first 'non-recursive'
switch followed by a second (only) 'recursive' switch inside a loop.
Continue is implemented by assignment to a state variable that the second
switch switches over.

R=asgerf@google.com

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

[Kevin Millikin (Google), 2016-01-26 08:43:24]

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