RFR JDK-8220018: javac crash when compiling try-catch-finally inside switch expression

[Maurizio Cimadamore]

Modulo 'yield' vs 'break', this loos good.

Maurizio

On 11/06/2019 16:07, Jan Lahoda wrote:
> Hi Bernard,
>
> Looks good to me. Any comments, Maurizio?
>
> Thanks,
>     Jan
>
> On 19. 05. 19 16:46, B. Blaser wrote:
>> Hi Jan and Maurizio,
>>
>> I've updated the webrev [1] as you both suggested:
>> * 'unwindBreak()' commented to warn for a possible dead code marking.
>> * 'reloadStackBeforeSwitchExpr()' added to factorize the stack 
>> reloading.
>> * 'test4()' below added to Jan's test cases.
>>
>> Does this look good to both of you (tier1 is OK)?
>>
>> Thanks,
>> Bernard
>>
>> [1] http://cr.openjdk.java.net/~bsrbnd/jdk8220018/webrev.01/
>>
>>
>> On Sat, 11 May 2019 at 13:29, B. Blaser <bsrbnd at gmail.com> wrote:
>>>
>>> Hi Jan and Maurizio,
>>>
>>> Thanks for both feedback.
>>>
>>> On Thu, 9 May 2019 at 14:35, Maurizio Cimadamore
>>> <maurizio.cimadamore at oracle.com> wrote:
>>>>
>>>> Hi,
>>>> I look at the code and discussed it offline with Jan extensively.
>>>>
>>>> I think the approach looks good, and it is inspired by what we already
>>>> do for return expressions (where we use intermediate locals to store
>>>> return expressions in case there's a finalizer).
>>>>
>>>> The only problem I have with this code is that it implicitly 
>>>> relies, in
>>>> the conditional context case, on the fact that, should the finalizer
>>>> complete abnormally (isAlive = false), then Code will refuse to 
>>>> emit any
>>>> additional statements (such as the loads required to restore the stack
>>>> state).
>>>>
>>>> I think the is brittle, and might bite us back in case we revisit this
>>>> code and add another statement whose behavior is NOT guarded by 
>>>> isAlive.
>>>>
>>>> So, I suggest either adding explicit comments, saying that unwindBreak
>>>> might affect isAlive and, as a result, following statements might 
>>>> not be
>>>> generated - or, wrap the code in an explicit `if (isAlive)` guard, as
>>>> done in the other branch of the visitBreak code.
>>>
>>> As first step, I guess I'd be more for simply adding a comment but we
>>> can still rework and consolidate this part later on if necessary.
>>>
>>>> As bonus point, since
>>>> we're restoring the stack in three different places, perhaps we can
>>>> factor the restoring code out to some common function.
>>>
>>> Of course ;-)
>>>
>>>> Cheers
>>>> Maurizio
>>>>
>>>>
>>>> On 09/05/2019 09:34, Jan Lahoda wrote:
>>>>> Hi Bernard,
>>>>>
>>>>> I apologize for late reply.
>>>
>>> No problem, I'm rather busy too...
>>>
>>>>> Thanks for the patch, looks OK to me, but
>>>>> I guess it would be good to have another review on this.
>>>>>
>>>>> Would it be possible to enhance the tests with the testcase below 
>>>>> (and
>>>>> any other testcases needed)?
>>>
>>> Yes, I'll also include this one.
>>>
>>> Cheers,
>>> Bernard
>>>
>>>
>>>>> Thanks,
>>>>>      Jan
>>>>>
>>>>> On 27. 03. 19 19:20, B. Blaser wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Please review the following fix for [1] which has already been
>>>>>> discussed in the corresponding thread [2]:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~bsrbnd/jdk8220018/webrev.00/
>>>>>>
>>>>>> It includes the variant I suggested to store the switch expression
>>>>>> result in a single temporary variable along with Jan's solution for
>>>>>> switch expressions used as conditions. Note that I had to do a tiny
>>>>>> correction to the latter (see 'code.resolve(value.trueJumps)') to 
>>>>>> make
>>>>>> the following example succeed:
>>>>>>
>>>>>>       public static void test4() {
>>>>>>           if (!switch (0) {
>>>>>>               default -> {
>>>>>>                   try {
>>>>>>                       break switch(0) { default -> true; };
>>>>>>                   }
>>>>>>                   finally {
>>>>>>                       break false;
>>>>>>                   }
>>>>>>               }
>>>>>>           }) {
>>>>>>               System.out.println("OK!");
>>>>>>           }
>>>>>>       }
>>>>>>
>>>>>> It also includes Jan's test case.
>>>>>>
>>>>>> Any feedback is welcome (tier1 is OK).
>>>>>>
>>>>>> Thanks,
>>>>>> Bernard
>>>>>>
>>>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8220018
>>>>>> [2]
>>>>>> http://mail.openjdk.java.net/pipermail/compiler-dev/2019-March/013073.html 
>>>>>>
>>>>>>