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

[Maurizio Cimadamore]

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