Implement continue for switch.

Implement continue for switch.

The overall strategy is to convert the following:

switch (e) {
  l_0: case e0: s_0; break;
  l_1: case e1: s_1; continue l_i;
  ...
  l_n: default: s_n; continue l_j;
}

into this:

var label;
switch (e) {
  case e0: label = 0; break;
  case e1: label = 1; break;
  ...
  default: label = n; break;
}
l: while (true) {
  switch (label) {
  case 0: s_0; break l;
  case 1: s_1; label = i; continue l;
  ...
  case n: s_n; label = j; continue l;
  }
}


BUG=http://dartbug.com/8270
R=ngeoffray@google.com

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

[ngeoffray, 2013-05-14 07:08:50]

LGTM

https://codereview.chromium.org/14969004/diff/2001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/elements/modelx.dart (right):

https://codereview.chromium.org/14969004/diff/2001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/elements/modelx.dart:2005:
ElementKind.STATEMENT, enclosingElement);
Odd indentation.

https://codereview.chromium.org/14969004/diff/2001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/ssa/builder.dart (right):

https://codereview.chromium.org/14969004/diff/2001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:802: int switchIndex
= 1;
Explain why you start with 1.

https://codereview.chromium.org/14969004/diff/2001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:809: if
(labelElement.isContinueTarget) {
I prever avoiding nested if and do if (labelElement != null &&
labelElement.isContinueTarget).

https://codereview.chromium.org/14969004/diff/2001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:839: if (label !=
null && targetIndexMap.containsKey(label.target)) {
It would read nicer with a helper method whose name explains what it means: if
(isContinueToSwithCase(label)) {...

https://codereview.chromium.org/14969004/diff/2001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:4609: * Creates a
switch statement [node].
Do you mean a [HSwitch] ?

https://codereview.chromium.org/14969004/diff/2001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:4688:
jumpHandler.forEachContinue((HContinue instruction, LocalsHandler locals) {
If that's a simple switch, those continues should be to something out of the
switch right?

https://codereview.chromium.org/14969004/diff/2001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/ssa/nodes.dart (right):

https://codereview.chromium.org/14969004/diff/2001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/nodes.dart:1700: final bool
breakSwitchContinueLoop;
Please add a comment on this field.

https://codereview.chromium.org/14969004/diff/2001/tests/language/infinite_sw...
File tests/language/infinite_switch_label_test.dart (right):

https://codereview.chromium.org/14969004/diff/2001/tests/language/infinite_sw...
tests/language/infinite_switch_label_test.dart:20: }
New line.

https://codereview.chromium.org/14969004/diff/2001/tests/language/nested_swit...
File tests/language/nested_switch_label_test.dart (right):

https://codereview.chromium.org/14969004/diff/2001/tests/language/nested_swit...
tests/language/nested_switch_label_test.dart:14: }
New line.

[Johnni Winther, 2013-05-16 14:17:59]

PTAL

[Johnni Winther, 2013-05-17 07:03:04]

https://codereview.chromium.org/14969004/diff/2001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/elements/modelx.dart (right):

https://codereview.chromium.org/14969004/diff/2001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/elements/modelx.dart:2005:
ElementKind.STATEMENT, enclosingElement);
On 2013/05/14 07:08:51, ngeoffray wrote:
> Odd indentation.

Done.

https://codereview.chromium.org/14969004/diff/2001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/ssa/builder.dart (right):

https://codereview.chromium.org/14969004/diff/2001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:802: int switchIndex
= 1;
On 2013/05/14 07:08:51, ngeoffray wrote:
> Explain why you start with 1.

Done.

https://codereview.chromium.org/14969004/diff/2001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:809: if
(labelElement.isContinueTarget) {
On 2013/05/14 07:08:51, ngeoffray wrote:
> I prever avoiding nested if and do if (labelElement != null &&
> labelElement.isContinueTarget).

Done.

https://codereview.chromium.org/14969004/diff/2001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:839: if (label !=
null && targetIndexMap.containsKey(label.target)) {
On 2013/05/14 07:08:51, ngeoffray wrote:
> It would read nicer with a helper method whose name explains what it means: if
> (isContinueToSwithCase(label)) {...

Done.

https://codereview.chromium.org/14969004/diff/2001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:4609: * Creates a
switch statement [node].
On 2013/05/14 07:08:51, ngeoffray wrote:
> Do you mean a [HSwitch] ?

A 'for' was missing. Now it doesn't apply.

https://codereview.chromium.org/14969004/diff/2001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:4688:
jumpHandler.forEachContinue((HContinue instruction, LocalsHandler locals) {
On 2013/05/14 07:08:51, ngeoffray wrote:
> If that's a simple switch, those continues should be to something out of the
> switch right?

It should not happen: If simple, no continue should target [jumpHandler]. If
complex, the jump handler should have no targets.

Inserted an assertion instead.

https://codereview.chromium.org/14969004/diff/2001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/ssa/nodes.dart (right):

https://codereview.chromium.org/14969004/diff/2001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/nodes.dart:1700: final bool
breakSwitchContinueLoop;
On 2013/05/14 07:08:51, ngeoffray wrote:
> Please add a comment on this field.

Done.

https://codereview.chromium.org/14969004/diff/2001/tests/language/infinite_sw...
File tests/language/infinite_switch_label_test.dart (right):

https://codereview.chromium.org/14969004/diff/2001/tests/language/infinite_sw...
tests/language/infinite_switch_label_test.dart:20: }
On 2013/05/14 07:08:51, ngeoffray wrote:
> New line.

Done.

https://codereview.chromium.org/14969004/diff/2001/tests/language/nested_swit...
File tests/language/nested_switch_label_test.dart (right):

https://codereview.chromium.org/14969004/diff/2001/tests/language/nested_swit...
tests/language/nested_switch_label_test.dart:14: }
On 2013/05/14 07:08:51, ngeoffray wrote:
> New line.

Done.

[ngeoffray, 2013-05-17 09:00:10]

LGTM

https://codereview.chromium.org/14969004/diff/15001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/ssa/builder.dart (right):

https://codereview.chromium.org/14969004/diff/15001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:4522: // in the
switch expression.
This sentence is not fully correct. The reason we're adding a default is because
otherwise, there would be no good place to give a default value to the local.

https://codereview.chromium.org/14969004/diff/15001/tests/language/language.s...
File tests/language/language.status (left):

https://codereview.chromium.org/14969004/diff/15001/tests/language/language.s...
tests/language/language.status:473: # Bug in dart2js frontent: does not support
this feature.
frontent -> fronted

https://codereview.chromium.org/14969004/diff/15001/tests/language/language_d...
File tests/language/language_dart2js.status (left):

https://codereview.chromium.org/14969004/diff/15001/tests/language/language_d...
tests/language/language_dart2js.status:199: switch_label_test: Fail # error:
target of continue is not a loop or switch case
Why is this one failing now? Is it a bad test?

[ngeoffray, 2013-05-17 09:12:34]

https://codereview.chromium.org/14969004/diff/15001/tests/language/language_d...
File tests/language/language_dart2js.status (left):

https://codereview.chromium.org/14969004/diff/15001/tests/language/language_d...
tests/language/language_dart2js.status:199: switch_label_test: Fail # error:
target of continue is not a loop or switch case
On 2013/05/17 09:00:10, ngeoffray wrote:
> Why is this one failing now? Is it a bad test?

Nevermind. Brain not functioning.

[Johnni Winther, 2013-05-17 09:13:34]

https://codereview.chromium.org/14969004/diff/15001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/ssa/builder.dart (right):

https://codereview.chromium.org/14969004/diff/15001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:4522: // in the
switch expression.
On 2013/05/17 09:00:10, ngeoffray wrote:
> This sentence is not fully correct. The reason we're adding a default is
because
> otherwise, there would be no good place to give a default value to the local.

Done.

[Johnni Winther, 2013-05-17 09:35:40]

Committed patchset #4 manually as r22838 (presubmit successful).