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

[David Holmes]

Hi Peter,

On 3/06/2021 6:52 pm, Peter Levart wrote:
> On Thu, 3 Jun 2021 07:31:14 GMT, David Holmes <david.holmes at oracle.com> wrote:
> 
>>>> 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.
>>>
>>>
>>> Yes, there could be a CLASS retention annotation there (even though `-XX+PreserveAllAnnotations` was not used at runtime, so rawAnnotations contains the content of `RuntimeVisibleAnnotations` only). Again, such annotation was RUNTIME retention when its use was compiled into a class, but at runtime such annotation may be updated to have CLASS or even SOURCE retention. Such annotation use is filtered out.
>>
>> Sorry Peter I'm not following you. I am only talking about runtime here.
>> The VM loads a class at runtime and examines the annotation attributes
>> that exist in that classfile.
> 
> Right, but into which annotation attribute (`RuntimeVisibleAnnotations` vs. `RuntimeInvisibleAnnotations`) of that class the annotation use was encoded depends on what retention the annotation had at compile time, because those attributes are created by `javac` compiler.

Okay

>> Some will be visible annotations (which
>> implies RUNTIME retention), others may be invisible (which implies
>> !RUNTIME which I assume means CLASS). It provides access to these via
>> the "raw annotations" and the reflection code then processes that
>> through the AnnotationProcessor.
> 
> Right, so reflection code at runtime accesses the annotations that were compiled into the class annotation attributes by parsing them and filtering out all but RUNTIME annotations. But this filtering is done at runtime and uses annotation's current set of meta-annotations values (i.e. `@Retention`) which can differ from what this same annotation had at class compile time. So this is how current RUNTIME annotations can end up in the `RuntimeInvisibleAnnotations` class attribute and how current CLASS annotations can end up in `RuntimeVisibleAnnotations` class attribute. It's the consequence of separate compilation.

Okay I see what you are talking about in regards to separate compilation.

>>
>> I don't know exactly what you mean by an annotation use being compiled
>> into a class, but I assume it is something like the way compile-time
>> constants are compiled in. But I don't see how that relates to the
>> current discussion.
> 
> An annotation use is the use of annotation in the source code to annotate something (Class, Method, Field, ...). For example:
> 
> 
> @AnnA
> public class Use { ... }
> 
> 
> `javac` decides into which class attribute (`RuntimeVisibleAnnotations` vs. `RuntimeInvisibleAnnotations`) of Use.class file it will encode the `@AnnA` use by examining `AnnA`'s `@Retention` meta-annotation at that time. Say that at that time the annotation was this:
> 
> 
> @Retention(CLASS)
> public @interface AnnA {}
> 
> 
> `javac` would encode the `Use`'s use of that annotation into the `RuntimeInvisibleAnnotations` attribute of `Use.class`.
> 
> 
> Now comes runtime. Annotation maintainer decides to lift the retention of `AnnA` annotation to RUNTIME and now it looks like this:
> 
> 
> @Retention(RUNTIME)
> public @interface AnnA {}
> 
> 
> He tries to run some new code to extract the annotation value from `Use` class at runtime via reflection, because now at runtime the annotation is updated to have RUNTIME retention. But he also doesn't have access to `Use.java` to recompile it with updated annotation. He just has access to `Use.class` that was compiled with an old version of the same annotation. As we said, `Use.class` has encoded the annotation use in its `RuntimeInvisibleAnnotations` attribute. Voila, here comes `-XX+PreserveAllAnnotations` option to enable passing `RuntimeInvisibleAnnotations` attribute with encoded annotations to the reflection runtime and annotation parser which would return such annotation use.

I see the picture you are painting and that things could work that way, 
but I don't know if I agree that this was an intention or that they 
should work that way.

The separate compilation story with annotations is somewhat different to 
other separate compilation issues as the JVMS does not really provide 
any guidance here. The RuntimeInvisibleAnnotations are not even parsed 
by the VM so it know nothing about which annotation types are referred 
to, never loads those types or does any checking to see if the 
properties of that type (like Retention) remain the same. Even 
RuntimeVisibleAnnotations are only structurally parsed to see of there 
are annotations the VM has to be aware of (ie @Contended) but those can 
never be out-of-sync through separate compilation.

So in essence these annotations are transparent to the VM and it simply 
hands them up to the JDK for interpretation. The code in 
AnnotationParser uses the current definition of the annotation type to 
determine things like Retention policy and uses that when checking what 
the VM has returned.

> Now imagine whole libraries with classes that use an annotation which began its life as CLASS annotation because it was paired with AnnotationProcessor(s) that processed those annotation uses at compile time, but now some runtime tool emerges that would like to see those annotations too. The maintainer accepts the proposal to promote annotation's retention from CLASS to RUNTIME in new version of annotation. But who is going to re-compile all those libraries?

Yes good point. I don't have an answer. I don't know what the evolution 
philosophy is/was with annotations.

Cheers,
David
-----

>>
>> David
> 
> Regards, Peter
> 
> -------------
> 
> PR: https://git.openjdk.java.net/jdk/pull/4280
>