[jdk17] RFR: 8268766: Desugaring of pattern matching enum switch should be improved [v5]

[Paul Sandoz]

On Wed, 23 Jun 2021 11:58:06 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

>> Currently, an enum switch with patterns is desugared in a very non-standard, and potentially slow, way. It would be better to use the standard `typeSwitch` bootstrap to classify the enum constants. The bootstrap needs to accept enum constants as labels in order to allow this. A complication is that if an enum constant is missing, that is not an incompatible change for the switch, and the switch should simply work as if the case for the missing constant didn't exist. So, the proposed solution is to have a new bootstrap `enumSwitch` that accepts `String`s in place of the enum constants, and will internally convert them to the appropriate enum constants, and then it will find the proper case similarly to `typeSwitch`.
>> 
>> How does this look?
>
> Jan Lahoda has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Improving javadoc.

Bootstrap method looks. At first i thought why not let the class label just be an actual `Class.class` instance, signaling that target enum class should be used, but then thought perhaps it's better to be more literal: clarity of inputs, unlikely label lists will be shared, and further it might be easier to adapt them.

src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 227:

> 225:                                       String invocationName,
> 226:                                       MethodType invocationType,
> 227:                                       Object... labels) throws NullPointerException,

I don't think there are any benefits to declaring the runtime exceptions in the method declaration.

Other bootstrap methods declare a checked exception when certain linkage constraints are violated (and the line between whats an `IllegalArgumentException` or an explicit linkage checked exception can be blurry). In your case, given the simplicity i think what you have is ok. We could refine later with a specific checked exception for switch linkage violations.

src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 256:

> 254:         if (labelClass == Class.class) {
> 255:             if (label != enumClassTemplate) {
> 256:                 throw new IllegalArgumentException("illegal Class label: " + label);

Can we refine the message to state the class label is not equal to the enum class that is the target of the switch? LIkewise in the `else` block to state the label is not of class String or the target enum class.

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

Marked as reviewed by psandoz (Reviewer).

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