Do package-infos need to be reset between annotation processing rounds?

[Liam Miller-Cushon]

The TODO in checkNameAndExistence seems relevant here. Implementing
proc.type.already.exists sounds good, but I assume it would be a warning
rather than a mandatory error? If so I think we should still improve the
handling of overwriting symbols during annotation processing with something
like the proposed fix for JDK-8193037.

I filed a bug about implementing proc.type.already.exists:
https://bugs.openjdk.java.net/browse/JDK-8193216

On Wed, Dec 6, 2017 at 1:11 AM, Liam Miller-Cushon <cushon at google.com>
wrote:

> I think the current implementation of Filer doesn't check if generated
> classes override arbitrary classes on the classpath, it only checks if they
> clash with previously generated classes, or the sources and classes that
> were explicit inputs to the compilation: http://hg.openjdk.java.net/
> jdk/jdk/file/a9405d9ca8a8/src/jdk.compiler/share/classes/
> com/sun/tools/javac/processing/JavacFiler.java#l719
>
> On Tue, Dec 5, 2017 at 11:48 PM, Jonathan Gibbons <
> jonathan.gibbons at oracle.com> wrote:
>
>> We'll need Joe "Mr Annotation Processing" Darcy to chime in here, but the
>> Filer is supposed to detect clashes, and prevent overwriting/overriding
>> symbols. That's definitely supposed to happen for normal
>> classes/interfaces; I could believe that package-info has been overlooked,
>> but I would expect it to follow the same guidelines.
>>
>> -- Jon
>>
>> On 12/5/17 6:24 PM, Liam Miller-Cushon wrote:
>>
>> Is overriding package-infos different or worse than overriding other
>> symbols? I've seen a number of examples where the same source file was
>> compiled multiple times and the earlier results ended up on the class path
>> of the later compilations, so a processor-generated class was both on the
>> class path and generated during the compilation. Making that an error would
>> be a somewhat invasive breaking change. I agree that it should be
>> discouraged, but I'm not sure it's worth making an error? (Unless there's
>> something special about package-infos that I'm missing.)
>>
>> On Tue, Dec 5, 2017 at 6:09 PM, Jonathan Gibbons <
>> jonathan.gibbons at oracle.com> wrote:
>>
>>> Generally, annotation processing is supposed to only create information
>>> where there was no information previously,  so if a package had a
>>> package-info with annotations, it seems like it should be an error to
>>> override it with another package-info, with or without annotations.
>>>
>>> -- Jon
>>>
>>>
>>> On 12/05/2017 05:39 PM, Liam Miller-Cushon wrote:
>>>
>>> Thanks! If an annotated package-info is loaded from the class path, and
>>> then a processor generates a package-info that contains no annotations, the
>>> reset is necessary. (The reset isn't necessary if the new package-info is
>>> annotated, since the old annotations get overwritten even if they weren't
>>> reset between rounds.)
>>>
>>> On Tue, Dec 5, 2017 at 4:46 PM, Jonathan Gibbons <
>>> jonathan.gibbons at oracle.com> wrote:
>>>
>>>> Liam,
>>>>
>>>> What about the case where an annotation processor generates the
>>>> package-info.java file? Is that a case where it is important to reinit the
>>>> packge symbol correctly, so that the newly generated code is read?
>>>>
>>>> -- Jon
>>>>
>>>>
>>>> On 12/05/2017 03:39 PM, Liam Miller-Cushon wrote:
>>>>
>>>>> I have a question about the logic in JavacProcessingEnvironment's
>>>>> treeCleaner that resets package-info state between annotation processing
>>>>> rounds [1].
>>>>>
>>>>> JDK-8193037 describes an issue where package-infos loaded from the
>>>>> classpath are reset by treeCleaner. Those symbols doesn't get reinitialized
>>>>> correctly, and package annotations are not visible during subsequent
>>>>> annotation processing rounds.
>>>>>
>>>>> I wondered if the logic was only meant to apply to package-infos being
>>>>> compiled from source in the current compilation (similar to how
>>>>> module-infos are handle by treeCleaner), but I'm having trouble
>>>>> understanding when that logic is necessary. Commenting out
>>>>> `node.packge.package_info.reset();` and `node.packge.reset();` in
>>>>> treeCleaner doesn't break any jtreg tests. Does anyone have examples where
>>>>> that code is needed? I'd like to add a regression test to ensure the fix
>>>>> for JDK-8193037 doesn't interfere with the original purpose of that code.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> [1] http://hg.openjdk.java.net/jdk/jdk/file/a358ebcfacfb/src/jdk
>>>>> .compiler/share/classes/com/sun/tools/javac/processing/Javac
>>>>> ProcessingEnvironment.java#l1518
>>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20171207/f944f323/attachment-0001.html>