RFR: 8302344: Compiler Implementation for Unnamed patterns and variables (Preview) [v13]

[Jan Lahoda]

On Thu, 4 May 2023 17:56:37 GMT, Vicente Romero <vromero at openjdk.org> wrote:

>> Aggelos Biboudis has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add test
>
> src/jdk.compiler/share/classes/com/sun/source/tree/VariableTree.java line 54:
> 
>> 52: 
>> 53:     /**
>> 54:      * Returns the name of the variable being declared or `_` if the variable
> 
> I guess if we go full for the empty name then this will need to change too

Maybe also add a mention that unnamed is a preview feature?

> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransPatterns.java line 535:
> 
>> 533:                 if (clearedPatterns.size() > 1) {
>> 534:                     validCaseLabelList = clearedPatterns.stream().allMatch(cP -> cP.hasTag(Tag.PATTERNCASELABEL));
>> 535:                 } else validCaseLabelList = clearedPatterns.size() == 1 && clearedPatterns.head.hasTag(Tag.PATTERNCASELABEL);
> 
> style: I would use `{}` for the code in the `else` branch here, same way as it has been done for the `then` branch. Also in the `else` branch isn't the `clearedPatterns.size() == 1` condition always true?

True - if I read the code correctly, the list should never be empty at this place.

> src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeCopier.java line 496:
> 
>> 494:     @DefinedBy(Api.COMPILER_TREE)
>> 495:     public JCTree visitAnyPattern(AnyPatternTree node, P p) {
>> 496:         JCBindingPattern t = (JCBindingPattern) node;
> 
> not sure this is correct, are we returning a JCBindingPattern?

+1

> test/langtools/tools/javac/patterns/Unnamed.java line 4:
> 
>> 2: 
>> 3: /**
>> 4:  * @test /nodynamiccopyright/
> 
> there is no golden file for this test so full copyright will be needed

Also, it is a custom that the comment with `@test` is before any imports, AFAIK.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13528#discussion_r1185807820
PR Review Comment: https://git.openjdk.org/jdk/pull/13528#discussion_r1185823113
PR Review Comment: https://git.openjdk.org/jdk/pull/13528#discussion_r1185870099
PR Review Comment: https://git.openjdk.org/jdk/pull/13528#discussion_r1185871058