On 31/05/2021 4:44 am, Alan Bateman wrote:
On Fri, 28 May 2021 12:56:39 GMT, Jaroslav Tulach
<github.com+26887752+jaroslavtul...@openjdk.org> wrote:
This PR exposes runtime invisible annotations via `Class.getAnnotation` when
`-XX:+PreserveAllAnnotations` option is passed to the JVM.
Existing `-XX:+PreserveAllAnnotations` option can be very useful for code that
needs to access annotations with `RetentionPolicy.CLASS` without the need to
parse the .class files manually. While the RuntimeInvisibleAnnotations are kept
in the runtime, they are not visible via java.lang.reflect API. I assume that's
just an omission.
This PR provides a new test and a fix to make `Class.getAnnotation(...)` useful
when `-XX:+PreserveAllAnnotations` option is on.
I checked the pre-OpenJDK history and `-XX:+PreserveAllAnnotations` dates from
when JSR-175 support was added in JDK 5. It may have been useful during the
development but I don't see any tests using it now. I don't think we want to
create an attractive nuisance here and maybe this XX option should be
deprecated so that it can be obsoleted and eventually removed.
I was a bit confused by this PR until I realized that annotations with
CLASS retention policy must be represented in the classfile as
RuntimeInvisible annotations - as the only affect of
PreserveAllAnnotations is to make invisible annotations remain present
in the runtime representation of the classfile, as allowed for by JVMS
4.7.17 (and related).
As Alan notes we don't seem to have any tests (current or historical)
for the operation of this flag (which is quite surprising) but its
affect in the VM code is quite simple and obvious - and was recently
extended to handle record attributes.
I don't know what the design intent on the core reflection side was in
relation to this. I could imagine a design that always asks the VM for
any CLASS or RUNTIME retention annotation, knowing that the VM may have
been instructed to preserve CLASS annotations. In that way the core
reflection code would be oblivious to the means by which the VM was
instructed to preserve said annotations.
If core reflection has never exposed CLASS retention annotations even
when PreserveAllAnnotations is set, then I would not support starting
now. But if it is the case that only some recently added annotations are
not conforming to pre-existing behaviour then I would support fixing
those cases. However, given we have not previously needed to tell core
reflection about the PreserveAllAnnotations flag, I would not want to
start doing so now, as it should not be necessary.
If PreserveAllAnnotations has hitherto been working exactly as one would
expect given JVMS 4.7.17 etc, then I would certainly not support
deprecating and removing it. But we should add in some tests.
Cheers,
David
-----
-------------
PR: https://git.openjdk.java.net/jdk/pull/4245
