RFR: JDK-8239447: compiler error for annotations applied to record components with target METHOD

[Vicente Romero]

thanks,
Vicente

On 3/13/20 9:07 AM, Maurizio Cimadamore wrote:
> Looks good
>
> Maurizio
>
> On 13/03/2020 12:45, Vicente Romero wrote:
>> I have modified the patch to include the previous code too, please 
>> check [1]
>>
>> Thanks,
>> Vicente
>>
>> [1] http://cr.openjdk.java.net/~vromero/8239447/webrev.02/
>>
>> On 3/13/20 6:39 AM, Maurizio Cimadamore wrote:
>>> I agree that this is a better solution, thanks for investigating.
>>>
>>> In general sharing trees is best avoided as it can lead (as in this 
>>> case) to very odd, and hard to diagnose failures.
>>>
>>> It is possible that the patch you had yesterday is also needed - you 
>>> might want to double check with Jan about that; perhaps a different 
>>> test case might reveal some issues with the lack of cleanup in the 
>>> annotation processing round.
>>>
>>> Maurizio
>>>
>>> On 13/03/2020 06:02, Vicente Romero wrote:
>>>> Hi
>>>>
>>>> On 3/12/20 8:13 PM, Maurizio Cimadamore wrote:
>>>>> Looks good - I wonder: why wasn't this failing if TYPE_USE was 
>>>>> used as a target? I know that processing for type annotations is 
>>>>> quite different, but I'm still a bit surprised to see that the 
>>>>> data from previous rounds doesn't cause issues in that case.
>>>>
>>>> I also thought that the difference was due to the different nature 
>>>> of type annotations. But while double checking to answer you I 
>>>> discovered that the cause is a bug in the records implementation, 
>>>> let's assume we have:
>>>>
>>>> record R(@MyAnno int i) {}
>>>>
>>>> currently what is happening is that the same AST created by the 
>>>> parser for the annotation in the record header is shared by the 
>>>> generated field, the record component, the accessor and the 
>>>> parameter in the generated constructor. Exactly the same AST. This 
>>>> is the root cause behind the reason why in some cases the 
>>>> information from a previous round was being cleared and sometimes 
>>>> not. I think that a cleaner solution should be to create different 
>>>> ASTs per each record member. So this way each member including 
>>>> record components will have their own AST. This fixes this bug and 
>>>> probably others that were potentially lurking in this area. I have 
>>>> uploaded a new iteration to [1].
>>>>
>>>>>
>>>>> Maurizio
>>>>
>>>> Thanks,
>>>> Vicente
>>>>
>>>> [1] http://cr.openjdk.java.net/~vromero/8239447/webrev.01/
>>>>>
>>>>> On 12/03/2020 17:57, Vicente Romero wrote:
>>>>>> ping
>>>>>>
>>>>>> On 3/7/20 12:34 AM, Vicente Romero wrote:
>>>>>>> Please review fix for [1] at [2]. When there are APs involved, 
>>>>>>> every time a new round of annotation processing will start, a 
>>>>>>> visitor, see [3], sets the attribute field of annotation ASTs to 
>>>>>>> null. This visitor was not visiting some annotations stored at 
>>>>>>> the record component. This provoked that data from previous 
>>>>>>> rounds could be accessible to later rounds provoking later 
>>>>>>> annotation validation code to fail. Just visiting the 
>>>>>>> annotations at the record component as it is done for the rest 
>>>>>>> of the annotations has fixed the issue.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Vicente
>>>>>>>
>>>>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8239447
>>>>>>> [2] http://cr.openjdk.java.net/~vromero/8239447/webrev.00/
>>>>>>> [3] 
>>>>>>> http://hg.openjdk.java.net/jdk/jdk/file/7af6364e1792/src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java#l1591
>>>>>>
>>>>
>>