On 31/05/2021 4:11 pm, Jaroslav Tulach wrote:
On Sun, 30 May 2021 23:03:55 GMT, David Holmes <david.hol...@oracle.com> wrote:
But we should add in some tests.
Right, I was surprised by missing tests as well. I've just created a
_"standalone"_ commit 0f689ea that adds such test and also shows the way I am
currently using the `-XX:+PreserveAllAnnotations` flag. It [requires some
tricks](https://github.com/openjdk/jdk/pull/4245/files#r642231785) with `ClassLoader` and
bytecode manipulation to change the retention to `RUNTIME` and then (in combination with
`-XX:+PreserveAllAnnotations`) the annotation is accessible.
I was thinking (not for this PR of course) of something a bit more
comprehensive that simply defined a class with methods/fields with both
class-retention and runtime-retention attributes (there are 8 cases of
"invisible" annotations IIRC) that verified their presence/absence both
with and without PreserveAllAnnotation.
The test in 0f689ea is _"standalone"_ - e.g. it can be merged without my other
changes. However I continue to hope the change in `Class.getAnnotation` could get in: the
`-XX:+PreserveAllAnnotation` option hasn't caused any issues since Java 5 - this is the first
improvement ever requested. Possibly also the last one - tests are written & the
functionality works - what else could one want?
I'll have to defer to core-libs folk here. As I said I would have
expected the Java code to just ask the VM for the annotations applied to
a given element, and that the VM would return the CLASS retention ones
if PreserveAllAnnotation was set. I don't understand how the
AnnotationParser is used in general and as it is not part of the VM the
part of the JVMS that talks about exposing invisible annotations is
not really applicable. Perhaps an oversight when this API was created,
but I don't know.
Cheers,
David
-------------
PR: https://git.openjdk.java.net/jdk/pull/4245
