Pattern matching for switch: Spec and Javac inconsistency

[Gavin Bierman]

Hi Ilyas

On 22 Jun 2021, at 12:38, Ilyas Selimov <ilyas.selimov at jetbrains.com<mailto:ilyas.selimov at jetbrains.com>> wrote:

Hello!

I found several differences between some rules in the spec draft (http://cr.openjdk.java.net/~gbierman/jep406/jep406-20210527/specs/patterns-switch-jls.html#jls-14.11.1)
and its javac implementation (OpenJDK 17 build 17-ea+27-2476).
Maybe ones are intentional or I missed something during the past discussions.

1)
Spec:
If a switch label has a constant case label element then if the switch label also has other case element labels they must be either a constant case label element, the default case label element, or the null case label element.

The next code is compiled fine despite the existence of both constant and pattern case label elements:
void testInt(Integer i) {
  switch (i) {
    case Integer o && o != null, 1:
      System.out.println("s1");
    default:
      System.out.println("def");
  }
}

while the similar code isn't compiled:

enum Day { MONDAY, TUESDAY, WEDNESDAY }
void testEnum(Day d) {
  switch (d) {
    case Day day && day != null, TUESDAY: // illegal fall-through to a pattern
      System.out.println("day");
    default:
      System.out.println("def");
  }
}

Hmm… that looks like a bug.


2)
Spec:
If a switch label has a null case label element then if the switch label also has any pattern case element labels, they must be type patterns (14.30.1).

Does "must be be type patterns" mean here that I can't declare guarded patterns together with
'null' label element like in the code below:
void testInt(Integer i) {
  switch (i) {
    case null, Integer o && o != null:
      System.out.println("s1");
    default:
      System.out.println("def");
  }
}


This is the feature that we discussed. The idea is that you can append a null case to a type pattern. So if you see a label such as `case null, T t` then if the label matches it is the case that `t` is either null or is an object whose type is convertible to T. We could have instead added some sort of T? t pattern, but this felt more readable.

The problem with what you suggest is, I think, readability. The guarded pattern suggests to the casual reader that `o` is never null. Except that by prepending with null, it could be.

We are being conservative in this first preview. The idea is that if restrictions like this actually bite, then we can look at loosening it up. Let us know how it goes.


3)
Spec:
A switch label that has a pattern case label element p that is total for the type of the selector expression of the enclosing switch statement or switch expression dominates a switch label that has a 'null' label element.

Here are two similar code samples:
void testInt1(Integer i) {
  switch (i) {
    case Object o, null:
      System.out.println("s1");
  }
}

void testInt2(Integer i) {
  switch (i) {
    case Object o:
      System.out.println("s1");
    case null: // this case label is dominated by a preceding case label
      System.out.println("null");
  }
}

The first one compiles fine, while the second one is not.
The spec says only about dominance between label elements within separate switch labels. Anyway, what is the main reason that allows the first sample to be compiled?

Yes, I considered this. But it felt like it would be too tough on the user, and it seemed okay to allow them to add a redundant null label element. (Of course, compilers and IDEs could add warnings here to suggest to the programmer that they can remove the null.) We can look again at this for the second preview version of course.

Thanks,
Gavin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20210622/7ec0058f/attachment-0001.htm>