RFR: 8326404: Assertion error when trying to compile switch with fallthrough with pattern

[Vicente Romero]

On Fri, 23 Feb 2024 12:31:01 GMT, Aggelos Biboudis <abimpoudis at openjdk.org> wrote:

> The assertion error that was raised in the bug report is a result of local variables not being defined in the case of switches that mix type patterns and record patterns with fall-through. e.g., a print after the `TransPatterns` phase:
> 
> 
> private static int run(Object o) {
>   int i = 0;
>   /*synthetic*/ Object selector2$temp = <*nullchk*>(o);
>   /*synthetic*/ int index$3 = 0;
>   switch (java.lang.runtime.SwitchBootstraps.typeSwitch(selector2$temp, index$3)) {
>     case 0:
>       String s;
>       if (!(let s = (String)selector2$temp; in true)) {
>         index$3 = 1;
>         continue;
>       }
>       i++;
>     case 1:
>       {
>       /*synthetic*/ Object selector4$temp = $b$O.a(); // $b$O appears undefined here 
>       ...
>       }
>     ...
>   }
> }
> 
> 
> The code for unrolling a case label into local variable declaration statements appears in `handleSwitch` here: https://github.com/openjdk/jdk/pull/17981/files#diff-e50bbfa8783f3bc8f5542452740b78f3167bee19be7365a87da2298c6333cca6L593-L606
> 
> `patchCompletingNormallyCases` takes care of statement block propagation for code that completes normally. Unfortunately this code is called late (after the optimization that merges common type tests together e.g., `case R(...): .... case R(...): ....`). 
> 
> The fix bails out of this optimization in the presence of fall-through. For example in the `run3_break1` included, the optimization is triggered. In the rest, not.
> 
> Additionally, this patch elides the unnecessary binding generation of unnamed pattern variables from the binding context.
> 
> Thanks @lahodaj for contributing this solution 🥇

looks sensible

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

Marked as reviewed by vromero (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17981#pullrequestreview-2053553368