RFR: JDK-8267936: PreserveAllAnnotations option isn't tested

[Peter Levart]

On Tue, 1 Jun 2021 09:30:40 GMT, Jaroslav Tulach <github.com+26887752+JaroslavTulach at openjdk.org> wrote:

> There doesn't seem to be much support for the complete changes in #4245. To get at least something useful from that endeavor I have extracted the test for existing behavior of `-XX:+PreserveAllAnnotations` and I am offering it in this pull request without any changes to the JVM behavior.

To share the information... I contacted one of the original authors of Java annotations, but he doesn't remember anything about the option. Here's the conversation...

On Thu, Jun 3, 2021 at 11:53 PM Peter Levart <peter.levart at gmail.com> wrote:
>
>             There is an undocumented JVM flag called "-XX+PreserveAllAnnotations"
>             which according to JVM code affects the way
>             "RuntimeInvisibleAnnotations" kind of class attributes are treated at
>             runtime. Normally they are ignored (since they normally contain only
>             metadata for annotation types with CLASS retention), but when this flag
>             is passed to JVM, such attributes are also passed to AnnotationParser
>             together with "RuntimeVisibleAnnotations" kind of class attributes.
>
>             So the question is: "What was the original intent of this JVM flag". A
>             debate sprung in a review thread on JDK GitHub and some say that this is
>             probably just a "leftover from an early implementation" and are
>             proposing to deprecate and remove it. Others (me mostly) see some sense
>             in the feature as it allows migrating an annotation type from CLASS
>             retention to RUNTIME without recompiling all the classes that use the
>             annotation. Do you happen to know the original intent of this JVM flag?

On Fri, Jun 4, 2021 at 6:54 AM Neal Gafter <neal at gafter.com> wrote:
>
>         I don’t recall that flag, but my suspicion is that it was used to assist testing.
>         If you ask me, I think the ecosystem would be better without it.

On 04/06/2021 15:56, Neal Gafter wrote:
>
>       One more thing.  You don’t have to recompile uses of an annotation to migrate 
>       from class to runtime retention.  You only have to recompile the attribute declaration.

On Fri, Jun 4, 2021 at 7:15 AM Peter Levart <peter.levart at gmail.com> wrote:
>
>     Not really. The AnnotationParser does filter out annotations based on current "retention" 
>     of annotation type which is just a meta-annotation on the annotation type, so it would appear 
>     that only the annotation declaration has to be recompiled, but as I described, the annotation 
>     uses in classes that use the annotation get compiled into two kinds of class attributes: 
>     RuntimeVisibleAnnotations (those with RUNTIME retention) and RuntimeInvisibleAnnotations 
>     (those with CLASS retention). Only those from RuntimeVisibleAnnotations are passed to 
>     AnnotationParser at runtime unless this VM flag is given which makes the VM pass a concatenation 
>     of RuntimeVisibleAnnotations and RuntimeInvisibleAnnotations to the AnnotationParser.
>     So merely changing the annotation retention and recompiling the annotation declaration is not enough. 
>     You have to also either recompile annotation uses or pass this VM flag. 
>     So I was wondering if the flag was actually meant for this purpose.

On 04/06/2021 17:30, Neal Gafter wrote:
>
>     Got it. I don't remember anything about the VM flag, but I doubt it was designed to be used for this purpose.

-------------

PR: https://git.openjdk.java.net/jdk/pull/4280