RFR: 8282274: Compiler implementation for Pattern Matching for switch (Third Preview) [v3]

[Maurizio Cimadamore]

On Tue, 12 Apr 2022 13:18:14 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

>> This is a (preliminary) patch for javac implementation for the third preview of pattern matching for switch (type patterns in switches).
>> 
>> Draft JLS:
>> http://cr.openjdk.java.net/~gbierman/PatternSwitchPlusRecordPatterns/PatternSwitchPlusRecordPatterns-20220407/specs/patterns-switch-jls.html
>> 
>> The changes are:
>> -there are no guarded patterns anymore, guards are not bound to the CaseElement (JLS 15.28)
>> -a new contextual keyword `when` is used to add a guard, instead of `&&`
>> -`null` selector value is handled on switch level (if a switch has `case null`, it is used, otherwise a NPE is thrown), rather than on pattern matching level.
>> -total patterns are allowed in `instanceof`
>> -`java.lang.MatchException` is added for the case where a switch is exhaustive (due to sealed types) at compile-time, but not at runtime.
>> 
>> Feedback is welcome!
>> 
>> Thanks!
>
> Jan Lahoda has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Cleanup.

Looks good - I like how moving the guard from pattern to an expression inside a switch made the code simpler to reason about, and also simplified the compiler API. I left some comments in the javadoc of MatchException.

src/java.base/share/classes/java/lang/MatchException.java line 33:

> 31:  * Thrown to indicate an unexpected failure in pattern matching.
> 32:  *
> 33:  * MatchException may be thrown when an exhaustive pattern matching language construct

Maybe wrap with `{@code}`

src/java.base/share/classes/java/lang/MatchException.java line 34:

> 32:  *
> 33:  * MatchException may be thrown when an exhaustive pattern matching language construct
> 34:  * (such as a switch expression) encounters a match target that does not match any of the provided

Is the term "match target" defined somewhere? In general, it would be good quoting relevant sections of the JLS, and using consistent terminology

src/java.base/share/classes/java/lang/MatchException.java line 37:

> 35:  * patterns at runtime.  This can currently arise from the following case:
> 36:  * <ul>
> 37:  *     <li>Separate compilation anomalies, where a sealed interface has a different

Why a list if there's only one case? I know we have plans to expand this to cover nulls in records patterns, but right now the javadoc should stand on its own

src/java.base/share/classes/java/lang/MatchException.java line 52:

> 50: 
> 51:     /**
> 52:      * Constructs an {@code MatchException} with no detail  message.

double space before "message" ?

src/java.base/share/classes/java/lang/MatchException.java line 73:

> 71:      *
> 72:      * @param  cause the cause (which is saved for later retrieval by the
> 73:      *         {@link #getCause()} method).  (A {@code null} value is

double space before "(A" ?

src/java.base/share/classes/java/lang/MatchException.java line 88:

> 86:      *         by the {@link #getMessage()} method).
> 87:      * @param  cause the cause (which is saved for later retrieval by the
> 88:      *         {@link #getCause()} method).  (A {@code null} value is

Same here, double space before "(A"

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

PR: https://git.openjdk.java.net/jdk/pull/8182