On Thu, 17 Jun 2021 18:33:56 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 
>> `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

Reply via email to