Bug in DDC switch+loop+continue

[lrhn]

There are new test failures on Remove unnecessary leftover @dart=2.19 from tests..

The test

language/switch/case_fallthrough__test

is failing on configuration

ddc-linux-chrome-try

See: https://ci.chromium.org/ui/p/dart/builders/try/ddc-linux-chrome-try/11688/overview

The test contains a do { switch (x) { ... case 3: ... continue; } } while (false); where appears the continue makes DDC continue the do loop from the start, not from the condition. That makes it go into an infinite loop when x == 3.
A continue inside a loop, targeting the loop, should be equivalent to breaking the body of the loop, going to the test (or increment+test for for loops). Since the test here is false, it shouldn't loop.

[lrhn]

I instrumented the test to show when it goes into an invalid loop. Inlined below (run in DartPad).
It prints that it loops in the following cases:

3: 2
4: 2
5: 2
13: 2
14: 2
15: 2
22: 2
23: 2
24: 2

This is every occurrence of continue, the first two in each triple is continuing the outer loop (which should end, because its loop condition is false) and the third one is continuing to another case of the same switch, which should exit. Those are probably spurious, the check in that case triggers the loop detection, I should try again with a > 2 check instead.

Instrumented code:

// Copyright (c) 2024, the Dart project authors.  Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

// Test switch statement "fallthrough" behavior.
//
// Tests behavior for Dart from version 3.0, where there is no error or
// warning related to control reaching the end of switch cases.
// See `case_fallthrough_legacy_error_test.dart` for previous behavior.

// Return type `dynamic` to avoid a warning for either of
// `return;` or `return x;`.
dynamic testSwitch(int x) {
  var rep = 0;
  chk() {
    if (++rep > 1) {
      print("$x: $rep");
      throw "OUT";
    }
  }

  // Catch all control flow leaving the switch.
  // Run switch in catch clause to check that `rethrow` is control flow.
  TRY:
  try {
    throw x;
  } catch (_) {
    // Add loop as break/continue target.
    LOOP:
    do {
      switch (x) {
        case 0:
        case 1:
          chk();
          nop(x);
          break; // Break switch.
        case 2:
          chk();
          nop(x);
          break LOOP;
        case 3:
          chk();
          nop(x);
          continue; // Continue loop.
        case 4:
          chk();
          nop(x);
          continue LOOP;
        case 5:
          chk();
          nop(x);
          continue LAST;
        case 6:
          chk();
          nop(x);
          return;
        case 7:
          chk();
          nop(x);
          return x;
        case 8:
          chk();
          nop(x);
          throw x;
        case 9:
          chk();
          nop(x);
          rethrow;
        case 10:
        case 11:
          {
            chk();
            nop(x);
            break;
          }
        case 12:
          {
            chk();
            nop(x);
            break LOOP;
          }
        case 13:
          {
            chk();
            nop(x);
            continue; // Continue loop.
          }
        case 14:
          {
            chk();
            nop(x);
            continue LOOP;
          }
        case 15:
          {
            chk();
            nop(x);
            continue LAST;
          }
        case 16:
          {
            chk();
            nop(x);
            return;
          }
        case 17:
          {
            chk();
            nop(x);
            return x;
          }
        case 18:
          {
            chk();
            nop(x);
            throw x;
          }
        case 19:
          {
            chk();
            nop(x);
            rethrow;
          }
        case 20:
          {
            {
              chk();
              nop(x);
              break; // Break and exit switch.
            }
          }
        case 21:
          {
            {
              chk();
              nop(x);
              break LOOP; // Break loop, exits switch (and loop).
            }
          }
        case 22:
          {
            {
              chk();
              nop(x);
              continue; // Continue loop, exits switch.
            }
          }
        case 23:
          {
            {
              chk();
              nop(x);
              continue LOOP; // Continue loop, exits switch.
            }
          }
        case 24:
          {
            {
              chk();
              nop(x);
              continue LAST; // Exits case, goto other case.
            }
          }
        case 25:
          {
            {
              chk();
              nop(x);
              return; // Exits function, loop and switch.
            }
          }
        case 26:
          {
            {
              chk();
              nop(x);
              return x; // Exits function, loop and switch.
            }
          }
        case 27:
          {
            {
              chk();
              nop(x);
              throw x; // Exits switch.
            }
          }
        case 28:
          {
            {
              chk();
              nop(x);
              rethrow; // Exits switch.
            }
          }
        case 29:
          chk();
          while (true) break; // Breaks loop, reaches end.
        case 30:
          chk();
          do break; while (true); // Breaks loop, reaches end.
        case 31:
          chk();
          for (;;) break; // Breaks loop, reaches end.
        case 32:
          chk();
          for (var _ in []) break LOOP; // Can be empty, reaches end.
        case 33:
          chk();
          if (x.isEven) {
            break; // Breaks and exits switch.
          } else {
            break LOOP; // Breaks loop and exits switch.
          }
        case 34:
          chk();
          (throw 0); // Exits switch.
        case 35:
          chk();
          nop(x); // Reaches end.
        case 36:
          chk();
          L:
          break; // Breaks and exits switch.
        case 37:
          chk();
          L:
          break L; // Breaks break statement only, reaches end.
        case 38:
          chk();
          L:
          if (x.isOdd) {
            break L; // Breaks if, reaches end.
          } else {
            break LOOP;
          }
        case 39:
          chk();
          notEver(x); // Type Never implies not completing.
        case 40:
          {
            chk();
            notEver(x);
          }
        case 41:
          {
            chk();
            {
              notEver(x);
            }
          }
        case 42:
          {
            chk();
            do {
              notEver(x);
            } while (true);
          }
        case 43:
          {
            chk();
            if (x.isEven) {
              notEver(x);
            } else {
              notEver(-x);
            }
          }
        case 44:
          chk();
          nop(x.isEven ? notEver(x) : notEver(-x));
        LAST:
        case 99:
          chk();
      }
    } while (false);
  } finally {
    // Catch all control flow leaving the switch and ignore it.
    break TRY;
  }
}

Never notEver(int x) {
  if (x < 0) {
    notEver(x + 1);
  } else {
    throw AssertionError("Never");
  }
}

main() {
  print("start");
  // Don't let compiler optimize for a specific value.
  for (int i = 0; i <= 100; i++) {
    testSwitch(i);
  }
  print("done");
}

/// Don't make it obvious that a switch case isn't doing anything.
void nop(Object? x) {}

[biggs0125]

DDC lowers switches with labeled continues to a JS while (true) loop and synthesizes label strings for any cases targeted by continues.

There's a bug in this lowering where it's treating the unlabeled Dart continue as safe to translate to a JS continue. But the while (true) loop that was introduced ends up being the target of the continue rather than the intended outer loop.

I have a fix in progress!

[biggs0125]

Fix landed here: https://dart-review.googlesource.com/c/sdk/+/397105

@lrhn If you rebase your change that test should start to pass!