RFR: 8326129: Java Record Pattern Match leads to infinite loop

[Vicente Romero]

On Wed, 21 Feb 2024 15:05:28 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

> Consider code like:
> 
> private int test(Box b) {
>     return switch (b) {
>         case Box(Integer i) -> 0;
>         case Box(Object o) when check(o) -> 1;
>         case Box(Object o) -> 2;
>     };
> }
> 
> public record Box(Object o) {}
> 
> 
> Note the nested `Object o` both accept `null`.
> 
> javac will compile this using two nested switches, like:
> 
> 
> int $restartIndex = 0;
> return switch (b, $restartIndex) { //will start matching as $restartIndex
>      case Box $b -> {
>          int $nestedRestartIndex = 0;
>          yield switch ($b.o(), $nestedRestartIndex) {  //will start matching as $nestedRestartIndex
>              case Integer i -> 0;
>              case null, Object o -> {
>                  if (!check(o)) {
>                      $nestedRestartIndex = 2;
>                      continue-nested-switch; //goto to the start of the nested switch again, with the updated $nestedRestartIndex
>                  }
>                  yield 1;
>              }
>              default -> {
>                   $restartIndex = 1;
>                   continue-main-switch; //goto to begining of the main switch again, with the updated $restartIndex
>              }
>         };
>     case Box(Object o) -> 2;
> }
> 
> 
> javac uses the restart index to implement (especially) guards - if the guard returns true, the matching (the whole switch) is restarted on the next case. javac tries to merge multiple cases with patterns with the same prefix into the same case, to reduce unnecessary matching.
> 
> There's a problem with the restarts, and that is that for `case null`, the restart index is ignored. I.e. even though the nested switch in `case null, Object o` sets the `$nestedRestartIndex` to 2, the switch will jump to `null` again after being restarted. This is because for the `null` selector, `SwitchBootstraps.typeSwitch` always returns `-1`.
> 
> The solution proposed here is to avoid factoring the nullable patterns into nested switches with any other patterns, as we cannot keep the semantics for the nested switch in such a case.

looks sensible

-------------

Marked as reviewed by vromero (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17951#pullrequestreview-1896612411