On Thu, 12 May 2022 10:54:16 GMT, Jan Lahoda <jlah...@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 two additional > commits since the last revision: > > - Updating naming to more closely follow the spec: total patterns are > renamed to unconditional patterns, unrefined is now unguarded. > - Case label elements cannot be unguarded if they have a guard of a type > different than boolean. Looks good - left some minor comments and questions. src/java.base/share/classes/java/lang/MatchException.java line 58: > 56: * @param message the detail message (which is saved for later > retrieval > 57: * by the {@link #getMessage()} method). > 58: * @param cause the cause (which is saved for later retrieval by the This looks odd - it seems like the sentence is like this: `the cause ( foo ). (bar)`. E.g. the test in parenthesis exceeds the test outside parenthesis by a wide margin. I suggest both here and in the "message" @param to avoid the parenthesis and split the sentence instead. Examples: * @param message the detail message. The message is saved for later retrieval * by the {@link #getMessage()} method). and * @param cause the cause. The cause is saved for later retrieval by the * {@link #getCause()} method). A {@code null} value is * permitted, and indicates that the cause is nonexistent or * unknown. Of course this is just an idea. src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java line 177: > 175: allowPatternSwitch = (preview.isEnabled() || > !preview.isPreview(Feature.PATTERN_SWITCH)) && > 176: > Feature.PATTERN_SWITCH.allowedInSource(source); > 177: allowUnconditionalPatternsInstance = (preview.isEnabled() || > !preview.isPreview(Feature.UNCONDITIONAL_PATTERN_IN_INSTANCEOF)) && Nit: perhaps adding "Of" to this already long variable name doesn't add much in terms of chars, but makes the meaning of the variable name clearer? src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java line 1802: > 1800: unguarded && > 1801: !patternType.isErroneous() && > 1802: > types.isSubtype(types.boxedTypeOrType(types.erasure(seltype)), This seems to be a change compared to the previous code - e.g. handling of boxing in the switch target type. Is this code even exercised? The test "NotApplicableTypes" seems to rule the combination `switch (int) ... case Integer` out. src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java line 1325: > 1323: } > 1324: > 1325: public record PatternPrimaryType(Type type) {} This is no longer needed - seems just a wrapper around a type. ------------- PR: https://git.openjdk.java.net/jdk/pull/8182
