On Thu, 17 Jun 2021 18:33:56 GMT, Jan Lahoda <[email protected]> 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
>> `enumConstant` that converts the enum constant name to the enum constant,
>> returning `null`, if the constant does not exist. It delegates to
>> `ConstantBootstraps.enumConstant` to do the actual conversion. And
>> `typeSwitch` accepts `null`s as padding.
>>
>> How does this look?
>
> Jan Lahoda has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Creating a new bootstrap method for (pattern matching) enum switches, as
> suggested.
Very good piece of work. I like all the code that can be removed because of
this.
I assume that the new code only kicks in if there's at least a pattern in the
switch, otherwise we fallback to legacy translation (meaning that compiling
with source < 17 is still ok), right?
I left some comments to help and clarify the javadoc text of the enum BSM.
src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 173:
> 171:
> 172: /**
> 173: * Bootstrap method for linking an {@code invokedynamic} call site
> that
This should be made clearer - e.g. the first argument is of type `Class` and
represents the enum we want to switch on. The remaining constants should be of
type `String`, the names of the various constants.
src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 178:
> 176: * {@code String} (representing enum constants) or {@code Class}.
> 177: * <p>
> 178: * The type of the returned {@code CallSite}'s method handle will
> have
Suggestion:
* The returned {@code CallSite}'s method handle will have
src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 179:
> 177: * <p>
> 178: * The type of the returned {@code CallSite}'s method handle will
> have
> 179: * a return type of {@code int}. It has two parameters: the first
> argument
Suggestion:
* a return type of {@code int}. It has two parameters: the first argument
Suggestion:
* a return type of {@code int} and accepts two parameters: the first
argument
test/langtools/tools/javac/patterns/EnumTypeChanges.java line 80:
> 78: A,
> 79: B;
> 80: }
missing newline here
test/langtools/tools/javac/patterns/EnumTypeChanges2.java line 27:
> 25: A,
> 26: C;
> 27: }
missing newline here
-------------
PR: https://git.openjdk.java.net/jdk17/pull/81
