RFR: JDK-8240998: Implement javac changes for deconstruction patterns.

[Maurizio Cimadamore]

Hi Jan,
the code looks generally good. I have some high-level questions:

* it would be useful to see what kind of diagnostics are generated - I 
see some weird ones in the golden files (e.g. 
"DeconstructionPatternErrors.out" refer to "compiler.misc.type.none")

* this might be a spec issue - there seems to be no support for varargs 
extraction - e.g. if a record is varargs:

record Foo(int... is) { }

shouldn't I be able to do this:

if (o instanceof Foo(int i1, int i2, int i3)) { ... }


Some more detailed comments below:

* Attr.java - verifyCastable seems very similar to Check.checkCastable, 
except for non-reifiable types - should we try to unify a bit?

* Attr.java - not sure I get the logic by which target type is 
propagated or not in visitDeconstructionPattern. Seems to me that the 
only case where you need propagation is when you have a nested component 
of the kind "var x" - in which case the type is determined by the 
expected record component type. But this:

+ boolean nestedIsValidPattern = 
!nestedPatterns.head.hasTag(BINDINGPATTERN) ||
+ ((JCBindingPattern) nestedPatterns.head).vartype == null;

Seems to point to a slightly different direction  - e.g. target type is 
propagated even if the nested is a deconstruction pattern, but then I 
don't see any use of resultInfo inside this visitor?

* parser changes - looks good :-)

* TransPattern - as usual, some comments illustrating basic desugared 
shapes would be nice

Maurizio

On 19/03/2020 17:25, Jan Lahoda wrote:
> Hi,
>
> Turned out the patch had two bugs - one related to the owners of the 
> temporary variables created, which then broke type annotations; and 
> another that broken de-duplication. I am deeply sorry for that. An 
> updated webrev is here:
> http://cr.openjdk.java.net/~jlahoda/8240998/webrev.01/
>
> A delta from previous round:
> http://cr.openjdk.java.net/~jlahoda/8240998/webrev.delta.00.01/
>
> I am sorry for any inconvenience.
>
> Jan
>
> On 18. 03. 20 9:55, Jan Lahoda wrote:
>> Hi,
>>
>> I would like to ask for a review for a patch that implements the 
>> deconstruction patterns, as described in JEP 375:
>> https://bugs.openjdk.java.net/browse/JDK-8235186
>>
>> The current specification draft is here:
>> http://cr.openjdk.java.net/~gbierman/jep375/jep375-20200316/specs/patterns-instanceof-jls.html 
>>
>>
>> For this phase, the proposal is for javac to desugar the 
>> deconstruction patterns using record accessors.
>>
>> The CSR for this change is being written here:
>> https://bugs.openjdk.java.net/browse/JDK-8240999
>>
>> The proposed patch:
>> http://cr.openjdk.java.net/~jlahoda/8240998/webrev.00
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8240998
>>
>> Any feedback is welcome!
>>
>> Thanks,
>>      Jan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20200323/fa52cfe9/attachment-0001.htm>