[deconstruction-patterns] Unconditional patterns and null (again)

[Brian Goetz]

There is a (deliberate) gap between an unconditional pattern and a set 
of patterns that is exhaustive at a type.  `R r` is unconditional on R, 
but `R(var x)` is not, because null -- but the set of patterns { R(var 
x) } is _exhaustive_ at R.  Null is in the _remainder_ of { R(var x) } 
on R.  So the spec says what it means and the compiler is implementing 
that, but ... we are not necessarily doing the user a favor here.

The question is how much analysis the compiler should do to catch errors 
surrounding remainder.  Because a switch without a `case null` already 
rejects null, we could conclude that the null covered by switch and the 
values covered by { R(var x) } together cover everything, and therefore 
the default is dead.  But with the spec we have, since we don't compute 
the remainder explicitly, this would likely have to be a special case 
for "can we prove that null is the only value in the remainder set".

Note that a similar analysis would be needed to make this exhaustive 
when we get to constant patterns:

     Boolean b = ...
     switch (b) {
         case TRUE: ...
         case FALSE: ...
     }

For now, I think it would be prudent for the compiler to issue a warning 
here, and we'll take a note to look into tightening the analysis for 
"null \union everything but null" cases.

On 10/25/2022 5:59 AM, Tagir Valeev wrote:
> Hello!
>
> I've just discovered that recent builds of JDK 19 and JDK 20 accept
> this code, which fails with NPE at runtime:
>
> class Test {
>      record R(int x) {}
>
>      static void test(R r) {
>          switch (r) {
>              case R(int x) -> System.out.println("matched");
>              default -> System.out.println("not matched");
>          }
>      }
>
>      public static void main(String[] args) {
>          test(null);
>      }
> }
>
> If I remember correctly, it was rejected previously with error similar
> to this (you can emulate the error replacing `R(int x)` with `R r1`
>> Test.java:7: error: switch has both an unconditional pattern and a default label
> I see that in a recent spec draft [1] it's said that:
>> Note that record patterns are not unconditional at any type because the null reference does not match any record pattern.
> Which makes sense. And it looks like that compiler follows the spec.
> However, we all know that the default branch is not reachable in this
> switch, so we effectively accept unreachable code. Is this intended or
> should be changed somehow?
>
> With best regards,
> Tagir Valeev
>
> [1] https://docs.oracle.com/javase/specs/jls/se19/preview/specs/patterns-switch-record-patterns-jls.html#jls-14.30.3