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

[Vicente Romero]

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
>>>>
>>