On Wed, 30 Jun 2021 10:23:32 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 with a new target base due to a merge 
> or a rebase. The pull request now contains ten commits:
> 
>  - Reflecting review comments.
>  - Merge branch 'master' into JDK-8268766
>  - Improving javadoc.
>  - Updating javadoc, as suggested.
>  - Updating javadoc, code and tests as suggested.
>  - Creating a new bootstrap method for (pattern matching) enum switches, as 
> suggested.
>  - Adding and fixing test.
>  - Merging master.
>  - 8268766: Desugaring of pattern matching enum switch should be improved

Looks good - added minor cosmetic comments

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

> 203:      *
> 204:      * @param lookup Represents a lookup context with the accessibility
> 205:      *               privileges of the caller.  When used with {@code 
> invokedynamic},

Suggestion:

     *               privileges of the caller. When used with {@code 
invokedynamic},

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

> 214:      * @throws NullPointerException if any argument is {@code null}
> 215:      * @throws IllegalArgumentException if any element in the labels 
> array is null, if the
> 216:      * invocation type is not a method type of first parameter of an 
> enum type,

Suggestion:

     * invocation type is not a method type whose first parameter type is an 
enum type,

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

> 215:      * @throws IllegalArgumentException if any element in the labels 
> array is null, if the
> 216:      * invocation type is not a method type of first parameter of an 
> enum type,
> 217:      * second parameter of type {@code int} and with {@code int} as its 
> return type,

Suggestion:

     * second parameter of type {@code int} and whose return type is {@code 
int},

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

Marked as reviewed by mcimadamore (Reviewer).

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

Reply via email to