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

[Maurizio Cimadamore]

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