RFR JDK14-8236597: issues inferring type annotations on records

[Vicente Romero]

On 1/10/20 3:51 PM, Maurizio Cimadamore wrote:
>
> On 10/01/2020 19:34, Vicente Romero wrote:
>>
>>
>> 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 ?)
>>
>> I'm not sure that we should mention this in the spec as this patch is 
>> just generating verbatim the same type typed by the user and thus it 
>> would be the expected outcome by the user, unless I'm missing something.
> I guess my point here is that it's not sufficient to say that a record 
> class has an accessor with this and that type - for type annotations 
> it is also relevant which syntactic form is used to express the type 
> (ident vs. qualified name). If the syntactic form should be preserved, 
> then I think we should say something, otherwise it's not clear exactly 
> which type annotations will be applicable to e.g. the accessor and 
> which not.

right now if the user writes @TypeAnno java.lang.String, that's a 
compiler error at any location of the type, this is wo records being 
involved. So there is only one representation of the type that can work. 
Also if the user writes:

record R(@TypeAnno java.lang.String s) {}

that's a compiler error too
>>
>>>
>>> And... what about parameter types? Shouldn't they need same 
>>> treatment too?
>>
>> they should but I wasn't sure if I should tackle that in this same 
>> patch, but I can do a: now that I was in the neighborhood...
>>
> Ok
>
> Maurizio
>
>>>
>>> Maurizio
>>
>> Vicente
>>
>>>
>>> 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
>>