RFR 8193216: Filer should warn if processors redefine symbols from the classpath or sourcepath

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

RFR 8193216: Filer should warn if processors redefine symbols from the classpath or sourcepath

Liam Miller-Cushon
Hello,

Please review the following fix for JDK-8193216. The patch causes Filer to emit a warning if an annotation processor redefines a symbol on the classpath or sourcepath.
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8193216: Filer should warn if processors redefine symbols from the classpath or sourcepath

Vicente Romero-2
looks good,
Vicente

On 12/08/2017 02:52 PM, Liam Miller-Cushon wrote:
> Hello,
>
> Please review the following fix for JDK-8193216. The patch causes
> Filer to emit a warning if an annotation processor redefines a symbol
> on the classpath or sourcepath.
>
> bug: https://bugs.openjdk.java.net/browse/JDK-8193216
> webrev: http://cr.openjdk.java.net/~cushon/8193216/webrev.00/ 
> <http://cr.openjdk.java.net/%7Ecushon/8193216/webrev.00/>

Reply | Threaded
Open this post in threaded view
|

Re: RFR 8193216: Filer should warn if processors redefine symbols from the classpath or sourcepath

Liam Miller-Cushon
Thanks for the review, the changeset is attached.

On Thu, Dec 14, 2017 at 10:20 AM, Vicente Romero <[hidden email]> wrote:
looks good,
Vicente


On 12/08/2017 02:52 PM, Liam Miller-Cushon wrote:
Hello,

Please review the following fix for JDK-8193216. The patch causes Filer to emit a warning if an annotation processor redefines a symbol on the classpath or sourcepath.

bug: https://bugs.openjdk.java.net/browse/JDK-8193216
webrev: http://cr.openjdk.java.net/~cushon/8193216/webrev.00/ <http://cr.openjdk.java.net/%7Ecushon/8193216/webrev.00/>



8193216.patch (10K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8193216: Filer should warn if processors redefine symbols from the classpath or sourcepath

joe darcy

Hello,

Given the change in long-standing warning behavior, to be thorough I'd like to see a quick CSR created for this issue. (From the issue in JBS, under the "More" box at the top of the page selecte "Create CSR" toward the bottom. Fill out the form presented in the the new issue and when you're ready to have it looked at hit "Finalize.")

For the test, I don't know if there would be additional coverage if a second compile line was added to test a pre-modules world. If Vicente or another javac engineer thinks this would be helpful,

...
* @compile/ref=warn.out -XDrawDiagnostics -Xlint:processing -processor TestProcTypeAlreadyExistsWarning B.java
* @compile/ref=warn.out -XDrawDiagnostics -Xlint:processing --release 8 -processor TestProcTypeAlreadyExistsWarning B.java
...

I'll adjust my eventual fix for

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

to treat package-info files analogously with the changes from this issue after the changes go back.

Perhaps separate from this change, it may be reasonable to check for obviously overlaps in the source locations and target locations.

Thanks,

-Joe


On 12/14/2017 4:17 PM, Liam Miller-Cushon wrote:
Thanks for the review, the changeset is attached.

On Thu, Dec 14, 2017 at 10:20 AM, Vicente Romero <[hidden email]> wrote:
looks good,
Vicente


On 12/08/2017 02:52 PM, Liam Miller-Cushon wrote:
Hello,

Please review the following fix for JDK-8193216. The patch causes Filer to emit a warning if an annotation processor redefines a symbol on the classpath or sourcepath.

bug: https://bugs.openjdk.java.net/browse/JDK-8193216
webrev: http://cr.openjdk.java.net/~cushon/8193216/webrev.00/ <http://cr.openjdk.java.net/%7Ecushon/8193216/webrev.00/>



Reply | Threaded
Open this post in threaded view
|

Re: RFR 8193216: Filer should warn if processors redefine symbols from the classpath or sourcepath

Liam Miller-Cushon
On Thu, Dec 14, 2017 at 8:20 PM, joe darcy <[hidden email]> wrote:

Given the change in long-standing warning behavior, to be thorough I'd like to see a quick CSR created for this issue. (From the issue in JBS, under the "More" box at the top of the page selecte "Create CSR" toward the bottom. Fill out the form presented in the the new issue and when you're ready to have it looked at hit "Finalize.")


Does this require a specification change? The documentation for -Xlint:processing isn't very specific, and the documentation for Filer doesn't discuss warnings.

For the test, I don't know if there would be additional coverage if a second compile line was added to test a pre-modules world...

I double-checked that the test still passes with the additional compile line.

I'll adjust my eventual fix for

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

to treat package-info files analogously with the changes from this issue after the changes go back.

Thanks!

Perhaps separate from this change, it may be reasonable to check for obviously overlaps in the source locations and target locations. 

Do you have an example of that kind of overlap? I'm not certain what source and target mean in this context.

I've seen issues related to overlaps between the sources of a compilation and its classpath, or between jars on the classpath. That kind of overlap is relatively common, but an optional lint warning might be nice to have.