[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-compiler-dev
Subject:    Re: RFR: 8326404: Assertion error when trying to compile switch with fallthrough with pattern
From:       Vicente Romero <vromero () openjdk ! org>
Date:       2024-05-13 19:13:04
Message-ID: 42GIJ9mfZiVDW9AnQ8e0g6TkdBHNCVNWCJv9DXvHZoI=.d1f18a2e-1f59-4064-bc20-71c3cf7c4656 () github ! com
[Download RAW message or body]

On Fri, 23 Feb 2024 12:31:01 GMT, Aggelos Biboudis <abimpoudis@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


[prev in list] [next in list] [prev in thread] [next in thread]