RFR: 8304883: Record Deconstruction causes bytecode error

[Vicente Romero]

On Mon, 27 Mar 2023 13:46:16 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

> Originally, we used to support cases like:
> `case null, String s when s.isEmpty() -> <expression>`
> where `s` could be `null` inside the expression, but the guard was only evaluated when `s != null`. This was implemented by augmenting the guard with `s == null || <original-guard>`.
> 
> This feature has been dropped since then.
> 
> Also note that in other cases, like:
> 
> record R(Object o) {}
> Object o = new R(null);
> switch (o) {
>     case R(var v) when v.toString().isEmpty() -> {}
> }
> 
> 
> We always expected a NPE on dereferencing the binding with value `null` (`v` in this case) in guard.
> 
> But, the code to implement the `case null, <binding>` remained in javac, and is causing problems, because, while the user cannot write `case null, <binding>` anymore, internally the desugaring sometimes generates that (when factoring out common prefixes). And the original code will still augment the guards to `<binding> == null || <guard>`, which causes the variables that are assigned in the guard to not being definitely assigned after the augmented guard.
> 
> The proposal is to simply remove the code that adds `<binding> == null || <guard>`, as there's no case where we would skip guards for `null` bindings anymore.

lgtm

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransPatterns.java line 539:

> 537:                             JCExpression guard = translate(label.guard);
> 538:                             if (hasJoinedNull) {
> 539:                                 JCPattern pattern = label.pat;

side: this idiom seems to be useful, currently used in a couple of places in Check, could it make sense to extract it into a method?

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

Marked as reviewed by vromero (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/13192#pullrequestreview-1359366690
PR Review Comment: https://git.openjdk.org/jdk/pull/13192#discussion_r1149453512