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

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Liam Miller-Cushon
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/JavacProcessingEnvironment.java#l1518
Reply | Threaded
Open this post in threaded view
|

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

Jonathan Gibbons
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/JavacProcessingEnvironment.java#l1518

Reply | Threaded
Open this post in threaded view
|

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

Liam Miller-Cushon
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 <[hidden email]> 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/JavacProcessingEnvironment.java#l1518


Reply | Threaded
Open this post in threaded view
|

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

Jonathan Gibbons
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 <[hidden email]> 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/JavacProcessingEnvironment.java#l1518



Reply | Threaded
Open this post in threaded view
|

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

Liam Miller-Cushon
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 <[hidden email]> 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 <[hidden email]> 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/JavacProcessingEnvironment.java#l1518




Reply | Threaded
Open this post in threaded view
|

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

Jonathan Gibbons

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 <[hidden email]> 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 <[hidden email]> 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/JavacProcessingEnvironment.java#l1518





Reply | Threaded
Open this post in threaded view
|

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

Liam Miller-Cushon
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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/JavacProcessingEnvironment.java#l1518






Reply | Threaded
Open this post in threaded view
|

Re: 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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/JavacProcessingEnvironment.java#l1518







Reply | Threaded
Open this post in threaded view
|

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

joe darcy
In reply to this post by Jonathan Gibbons

Hello,

Catching up on email amid JDK 10 RDP1 activities, the Filer specification states:

During each run of an annotation processing tool, a file with a given pathname may be created only once. If that file already exists before the first attempt to create it, the old contents will be deleted. Any subsequent attempt to create the same file during a run will throw a FilerException, as will attempting to create both a class file and source file for the same type name or same package name. The initial inputs to the tool are considered to be created by the zeroth round; therefore, attempting to create a source or class file corresponding to one of those inputs will result in a FilerException.

In general, processors must not knowingly attempt to overwrite existing files that were not generated by some processor. A Filer may reject attempts to open a file corresponding to an existing type, like java.lang.Object. Likewise, the invoker of the annotation processing tool must not knowingly configure the tool such that the discovered processors will attempt to overwrite existing files that were not generated.

http://download.java.net/java/jdk10/docs/api/javax/annotation/processing/Filer.html

For handling source files in particular:

A source file can also be created to hold information about a package, including package annotations. To create a source file for a named package, have the name argument be the package's name followed by ".package-info"; to create a source file for an unnamed package, use "package-info".

To separate a few cases, the javac Filer implementation tracks state about which files have been opened through the Filer on previous rounds of processing and throws exceptions if an attempt is made to re-open a file. (The check is a bit more involved; the Filer also checks the you don't open a class file for a type you've already opened a source file for, etc.) Package info files are handled the same way too. There is a short regression test on that functionality

    test/langtools/tools/javac/processing/filer/TestPackageInfo.java

but the exception on re-opening isn't explicitly tested there. Let me do a quick check... the implementation has the expected behavior for package-info files explicitly opened by annotation processors.. I'll file a RFE to enhance the test to explicitly cover this case.

However, contrary to the documentation, the implementation does not handle the case of a package-info file given on the initial inputs, although types on the initial inputs are checked for properly. I believe something like the patch below to JavacFiler *should* resolve that discrepancy:

-        ClassSymbol existing;
         boolean alreadySeen = aggregateGeneratedSourceNames.contains(Pair.of(mod, typename)) ||
                               aggregateGeneratedClassNames.contains(Pair.of(mod, typename)) ||
                               initialClassNames.contains(typename) ||
-                              ((existing = elementUtils.getTypeElement(typename)) != null &&
-                               initialInputs.contains(existing.sourcefile));
+                              containedInInitialInputs(typename);
         if (alreadySeen) {
             if (lint)
                 log.warning(Warnings.ProcTypeRecreate(typename));
@@ -731,6 +736,27 @@
         }
     }
 
+    private boolean containedInInitialInputs(String typename) {
+        // Name could be a type name or the name of a package-info file
+        JavaFileObject sourceFile = null;
+
+        ClassSymbol existingClass = elementUtils.getTypeElement(typename);
+        if (existingClass != null) {
+            sourceFile = existingClass.sourcefile;
+        } else if (typename.endsWith(".package-info")) {
+            String targetName = typename.substring(0, typename.length() - ".package-info".length());
+            PackageSymbol existingPackage = elementUtils.getPackageElement(targetName);
+            if (existingPackage != null)
+                sourceFile = existingPackage.sourcefile;
+        }
+        return (sourceFile == null) ? false : initialInputs.contains(sourceFile);
+    }
+

*However,* the sourcefile field of the PackageSymbol doesn't seem to be getting set to a non-null value via the lookup process. I didn't fully run down which part of JavacElements or Symbol takes care of that.

In the broader sense of the intention of the Filer, in the simplest case of input locations and output locations being disjoint, than the rules quoted above should be enforced. However, people can and do run configurations where the input and output locations overlap, complicating implementing the proper intentions.

In general, package-info files should be handled analogously to source files for types.

HTH,

-Joe


On 12/5/2017 11:48 PM, Jonathan Gibbons 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 <[hidden email]> 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 <[hidden email]> 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/JavacProcessingEnvironment.java#l1518






Reply | Threaded
Open this post in threaded view
|

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

joe darcy
PS Filed

     JDK-8193462: Fix Filer handling of package-info initial elements

The not-quite-working version of the fix is at:

     http://cr.openjdk.java.net/~darcy/8193462.0/

-Joe


Reply | Threaded
Open this post in threaded view
|

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

Liam Miller-Cushon
Hello,

the sourcefile field of the PackageSymbol doesn't seem to be getting set to a non-null value via the lookup process. I didn't fully run down which part of JavacElements or Symbol takes care of that.

It looks like PackageSymbol.sourcefile is only initialized by JavadocClassFinderPackageSymbol.package_info.sourcefile does get set up correctly in Enter, so using `existingPackage.package_info.sourcefile` (or setting the PackageSymbol's sourcefile in Enter at the same time as package_info) allows the test to pass, i.e.:

    private boolean containedInInitialInputs(String typename) {
...
            if (existingPackage != null && existingPackage.package_info != null)
                sourceFile = existingPackage.package_info.sourcefile;
or:

--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Enter.java
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Enter.java
@@ -379,6 +379,7 @@
             c.completer = Completer.NULL_COMPLETER;
                 c.members_field = WriteableScope.create(c);
                 tree.packge.package_info = c;
+                tree.packge.sourcefile = tree.sourcefile;
             }

And to check my understanding: the issue you discovered is separate from both JDK-8193037 and JDK-8193216, right? Does my diagnosis of JDK-8193037 sound reasonable? And for JDK-8193216, do you think it makes sense to consider warning in that case, even if it isn't required by the spec?

Thanks,
Liam

On Wed, Dec 13, 2017 at 1:51 PM, joe darcy <[hidden email]> wrote:
PS Filed

    JDK-8193462: Fix Filer handling of package-info initial elements

The not-quite-working version of the fix is at:

    http://cr.openjdk.java.net/~darcy/8193462.0/

-Joe