RFR: 8323657: Compilation of snippet results in VerifyError at runtime with --release 9 (and above)

[Vicente Romero]

On Thu, 18 Jan 2024 10:57:24 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

> Consider code like:
> 
> public class Test {
>     public static void main(String... args) {
>         System.err.println((test() ? null : null) + "");
>     }
>     private static boolean test() {
>         System.err.println("test called!");
>         return true;
>     }
> }
> 
> 
> This should print:
> 
> test called!
> null
> 
> 
> but it prints:
> 
> $ java /tmp/Test.java
> null
> 
> 
> The correct semantics can be achieved by using `--source 8`:
> 
> $ java --source 8 /tmp/Test.java
> test called!
> null
> 
> 
> Or `-XDstringConcat=inline` when compiling.
> 
> The reason is that when `StringConcat.IndyConstants` converts String concatenation parameters, it will short-circuit any expressions whose type is BOT (i.e. basically the `null` type). But, in this case, the expression that has the BOT type is a complex expression with a side-effect, so short-circuiting it leads to a broken semantics.
> 
> My proposal is to "short-circuit" only `null` literals, and generate other expressions normally. There may be some corner cases where we'd generate the expression unnecessarily (if it does not have any side-effect), but looking for side-effects seems unnecessarily complex and fragile given I'd expect non-trivial expressions with the BOT type are very rare. (The only case which comes to mind is exactly the above one - a conditional expressions, whose both result values are `null`.)
> 
> Please note that when the expression is incorrectly short-circuited, it may lead to various other effects - the original bug report shows a case where the resulting code does not pass the verification.

lgtm

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

Marked as reviewed by vromero (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17483#pullrequestreview-1834333082