RFR JDK14-8236597: issues inferring type annotations on records

[Vicente Romero]

On 1/10/20 7:58 PM, Maurizio Cimadamore wrote:
>
> Uhm I see - you copy the tree as is from one place to another? This 
> might be a bit suboptimal (e.g. tree position and all that), and can 
> misfire if the two trees are ever added into a map of some kind.
>

true I can fix that,

> But I take that this code has nothing to do with the patch under review.
>
> Maurizio
>

Vicente

> On 10/01/2020 23:55, Vicente Romero wrote:
>> while we wait on the decision regarding the change in the spec I 
>> realized that for the constructor I was already setting the type as 
>> defined in the record component, see method: 
>> RecordConstructorHelper::finalAdjustment,
>>
>> public JCMethodDecl finalAdjustment(JCMethodDecl md) {
>>      List<JCVariableDecl> tmpRecordFieldDecls =recordFieldDecls;
>>      for (JCVariableDecl arg : md.params) {
>>          /* at this point we are passing all the annotations in the field to 
>> the corresponding * parameter in the constructor. */ arg.mods.annotations = tmpRecordFieldDecls.head.mods.annotations;
>>          arg.vartype = tmpRecordFieldDecls.head.vartype;     <------------------------- here
>>          tmpRecordFieldDecls = tmpRecordFieldDecls.tail;
>>      }
>>      return md;
>> }
>>
>> Thanks,
>> Vicente
>>
>> On 1/10/20 2:23 PM, Maurizio Cimadamore wrote:
>>> Looks good - but I have a question - I assume the changes in 
>>> TypeEnter are to 'copy' the tree for the return type 'as is' from 
>>> the record component declaration to the accessor return type. That 
>>> is, is one is a qualified name, the other should be too, and viceversa.
>>>
>>> Ok, my question then is: shouldn't something like this be said 
>>> somewhere in the spec too? After all, since @Nullable foo.bar.Baz is 
>>> invalid but @Nullable Baz is not (or should use generate code for 
>>> foo.bar. at Nullable Baz ?)
>>>
>>> And... what about parameter types? Shouldn't they need same 
>>> treatment too?
>>>
>>> Maurizio
>>>
>>> On 10/01/2020 00:34, Vicente Romero wrote:
>>>> Hi,
>>>>
>>>> Please review this patch [1] to fix a couple of issues regarding 
>>>> inference of type annotations on records [2]. There where two cases 
>>>> where type annotations were reported as missing. For compact 
>>>> records whose arguments are not inheriting the type annotations 
>>>> from the corresponding record component and for accessors for which 
>>>> the annotation was present but just as a declaration annotation 
>>>> applied to the accessor. Not as a type annotation applied to the 
>>>> return type which was the expected outcome.
>>>>
>>>> In the case of the compact constructor, the solution was just to 
>>>> copy the annotations to the parameters which were missing. In the 
>>>> case of the accessor the solution was a bit more complicated. 
>>>> Accessors are created but not added to the list definitions 
>>>> belonging to the record tree. This is done to make them invisible 
>>>> to type attribution as they are not fully fledge methods, but as a 
>>>> side effect they are also invisible to the type annotations 
>>>> machinery. For this reason type annotations were not recognized as 
>>>> such. The solution here has been to make the type annotations 
>>>> machinery to visit accessors for records and set the type 
>>>> annotations correctly. In order to do that the accessor method 
>>>> created at TypeEnter is stored at the record component and visited 
>>>> at the same time type annotations are being classified for the rest 
>>>> of the code.
>>>>
>>>> Thanks,
>>>> Vicente
>>>>
>>>> [1] http://cr.openjdk.java.net/~vromero/8236597/webrev.00/
>>>> [2] https://bugs.openjdk.java.net/browse/JDK-8236597
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20200113/02929c98/attachment.htm>