On Wed, 23 Jun 2021 11:58:06 GMT, Jan Lahoda <jlah...@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
