On 3/06/2021 1:38 am, Peter Levart wrote:
On Wed, 2 Jun 2021 13:24:10 GMT, David Holmes <david.hol...@oracle.com> wrote:

Sorry now I see what happens. We aren't combining two arrays of
annotations we're concatenating two streams of byes, each of which
represents a set of annotations, starting with the length.

The code that receives this on the JDK side doesn't seem to understand
that this is a possibility.

Though maybe this isn't a bug, maybe the AnnotationParser is
deliberately ignoring the second byte stream. (Though if it were
deliberate there should be some commentary to that affect!)

David

I think this is not deliberate. Since `rawAnnotations` may end up having any of 
the following:
- `RuntimeVisibleAnnotations` (when there were just `RUNTIME` annotation usages 
compiled into the class or `-XX+PreserveAllAnnotations` was not used at runtime)
- `RuntimeInvisibleAnnotations` (when there were just `CLASS` annotation usages 
compiled into the class and `-XX+PreserveAllAnnotations` was used at runtime)
- `RuntimeVisibleAnnotations + RuntimeInvisibleAnnotations` (when there were 
`RUNTIME` and `CLASS` annotation usages compiled into the class and 
`-XX+PreserveAllAnnotations` was used at runtime)

So why would `RuntimeInvisibleAnnotations` be skipped in 3rd case but not in 
2nd case?

Well in the second case there is no way to know it is looking only at invisible annotations, so it has to read them and then discard them as they were in fact CLASS retention. The skipping could be seen as an optimization.

The code is confusing because it gives no indication that it is aware that runtime invisible annotations could be present:

/**
     * Parses the annotations described by the specified byte array.
     * resolving constant references in the specified constant pool.
     * The array must contain an array of annotations as described
     * in the RuntimeVisibleAnnotations_attribute:

but at the same time it checks for the RUNTIME retention, which means it must have recognised the possibility there could be something else there. That check is day 2 code though, on day 1 there was this comment:

/* XXX Uncomment when this meta-attribute on meta-attributes (Neal's putback)
             if (type.retention() == VisibilityLevel.RUNTIME)
 XXX  */

But the end result is that the code in AnnotationParser correctly filters out all non-RUNTIME annotations, either directly, or by skipping them in the stream in the first place. So in that sense there is no bug, but the code could do with some additional comments.

Cheers,
David
-----


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

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

Reply via email to