[jdk17] RFR: 8269146: Missing unreported constraints on pattern and other case label combination [v2]

[Maurizio Cimadamore]

On Tue, 6 Jul 2021 14:59:39 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

>> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java line 1684:
>> 
>>> 1682:             boolean hasTotalPattern = false;      // Is there a total pattern?
>>> 1683:             boolean hasNullPattern = false;       // Is there a null pattern?
>>> 1684:             boolean wasConstant = false;          // Seen a constant in the same case label
>> 
>> I'd suggest to move the `wasXYZ` variables inside the case label loop - since they are not meant to be variables that are global to the entire switch construct?
>
> I agree it would be much better to have the flags closer, but the flags sadly generally apply across case clauses. The conditions don't allow `case <type pattern>, default:` as well as `case <type pattern>: /*no statements*/ case default:`, because both are `SwitchLabel`s. So we need to keep information across `case` clauses if the case has an empty list of statements, and in some cases even if the list of statements is not empty, but there is a fall-through to the following case (which is to prevent cases like `case null: System.err.println(); /*no break*/ case Integer i:`).

OK, thanks for the clarification, I think I now get what the code is trying to do and the fact that multiple `cases` (even if separated by commas) are really dealt with separately by the logic, so you need to keep these flags in scope for longer.

I still think that some of these checks could be perhaps moved closer to parser - and then in Attr we could check stuff that depends on types (e.g. dominance), or stuff that depends on control flow (e.g. cannot fall through to a pattern).

I understand that the code was like this (e.g. duplicate labels, or duplicate defaults has always been caught in Attr) - but as the number of checks grows larger, it feels like keep adding checks in Attr doesn't scale very well.

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

PR: https://git.openjdk.java.net/jdk17/pull/194