RFR: JDK-8222169: java.lang.AssertionError switch expression in ternary operator - ?

[Vicente Romero]

looks good,

Thanks,
Vicente

On 5/15/19 4:05 AM, Jan Lahoda wrote:
> Hi,
>
> There is a regression in how let expressions are handled in Gen, 
> related to recent changes for switch expressions. Consider code like:
>
> int t(boolean b, Integer i) {
>     return b ? 0 : i++;
> }
>
> This gets desugared into:
> int t(boolean b, Integer i) {
>         return b ? 0 : (let /*synthetic*/ final Integer $1357563986 = 
> i in (let i = Integer.valueOf((int)(i.intValue() + 1)); in 
> $1357563986)).intValue();
>     }
>
> When Gen.visitLetExpr is generating the let expression, it records the 
> stack size, so that for further statements, it can assert the 
> statements start at an appropriate stack depth (at "statement start"). 
> But, the issue here is that when the stack size is recorded, the state 
> of the bytecode is before the target of the jump for "b == false", 
> which jumps to the "else" section. I.e. the state of the bytecode is 
> before the ':', not after. And so '0' is on the stack. But, the code 
> for the let expression is after the target of the jump, and so the 
> real stack at the beginning of the code for the let expression is empty.
>
> The proposed solution is to simply resolve the jumps before recording 
> the size of the stack - that should ensure the correct stack size is 
> recorded. We already do that for Gen.visitSwitchExpression, but missed 
> it for visitLetExpr.
>
> Webrev: http://cr.openjdk.java.net/~jlahoda/8222169/webrev.00/
> JBS: https://bugs.openjdk.java.net/browse/JDK-8222169
>
> The patch also includes test based on Vicente's:
> http://cr.openjdk.java.net/~vromero/8222795/webrev.00/
>
> How does this look?
>
> Thanks,
>     Jan