RFR JDK14-8236597: issues inferring type annotations on records

[Maurizio Cimadamore]

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. But I 
take that this code has nothing to do with the patch under review.

Maurizio

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/20200111/02502a02/attachment-0001.htm>