RFR: 8262889: Compiler implementation for Record Patterns [v2]

[Maurizio Cimadamore]

On Fri, 6 May 2022 14:09:24 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

>> 8262889: Compiler implementation for Record Patterns
>> 
>> A first version of a patch that introduces record patterns into javac as a preview feature. For the specification, please see:
>> http://cr.openjdk.java.net/~gbierman/jep427+405/jep427+405-20220426/specs/patterns-switch-record-patterns-jls.html
>> 
>> There are two notable tricky parts:
>> -in the parser, it was necessary to improve the `analyzePattern` method to handle nested/record patterns, while still keeping error recovery reasonable
>> -in the `TransPatterns`, the desugaring of the record patterns is very straightforward - effectivelly the record patterns are desugared into guards/conditions. This will likely be improved in some future version/preview
>> 
>> `MatchException` has been extended to cover additional cases related to record patterns.
>
> Jan Lahoda has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Reflecting review feedback.

Looks good

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java line 752:

> 750:                                            Iterable<? extends JCCaseLabel> labels) {
> 751:             Set<Symbol> coveredSymbols = new HashSet<>();
> 752:             Map<Symbol, List<JCRecordPattern>> deconstructionPatternsBySymbol = new HashMap<>();

since you seem to have settled on "recordPattern" for implementation names - you can probably revisit some of these names to say "record" instead of "deconstruction".

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java line 801:

> 799:             //i.e. represent all possible combinations.
> 800:             //This is done by categorizing the patterns based on the type covered by the given
> 801:             //starting component.

Example needed here. For instance (I discussed this with @biboudis):


record Outer(R r) { };
sealed interface I { };
class A implements I { };
class B implements I { };
sealed interface R { };
record Foo(I i) implements R { }
record Bar(I i) implements R { }

switch (o) {
   case Outer(Foo(A), Foo(A)):
   case Outer(Foo(B), Foo(B)):
   case Outer(Foo(A), Foo(B)):
   case Outer(Foo(B), Foo(A)):
   case Outer(Bar(A), Bar(A)):
   case Outer(Bar(B), Bar(B)):
   case Outer(Bar(A), Bar(B)):
   case Outer(Bar(B), Bar(A)):
}


Which generates two sets:


   case Outer(Foo(A), Foo(A)):
   case Outer(Foo(B), Foo(B)):
   case Outer(Foo(A), Foo(B)):
   case Outer(Foo(B), Foo(A)):


And


   case Outer(Bar(A), Bar(A)):
   case Outer(Bar(B), Bar(B)):
   case Outer(Bar(A), Bar(B)):
   case Outer(Bar(B), Bar(A)):


Sorry for being pedantic - this code is tricky and I'm worried we'll all forget exactly how it works in 2 months :-)

src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java line 1014:

> 1012:                         pattern = parsePattern(patternPos, mods, type, false);
> 1013:                     } else if (token.kind == LPAREN) {
> 1014:                         pattern = parsePattern(patternPos, mods, type, false);

Nice!

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

Marked as reviewed by mcimadamore (Reviewer).

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