RFR: 8004912: Repeating annotations - getAnnotationsByType is not working as expected

Tue Oct 22 13:20:11 UTC 2013

I think the problem could be solved in two ways:

- by explicitly scanning the inheritance chain for the 1st class that 
has the annotations of type T present directly or indirectly (by 
containment).

- by "canonicalizing" the representation of the repeating annotations in 
the class file attributes. By this I mean that each repeating annotation 
is placed inside it's container at compile-time even if it is a single 
annotation of a particular type. This would mean that some features like 
specifying different @Targets or @Retentions for repeating annotation 
types and their containers would be prohibited and the specification 
would have to be changed. But I think this way the "inheritance" aspect 
of repeating annotations would be consistent even if not "looking 
through" containers (by using the old JDK7 API)...

The "canonicalization" could be performed at runtime though, and I think 
there were such attempts already in the past. What happened to them?

Regards, Peter

On 10/22/2013 02:57 PM, Peter Levart wrote:
> Hi,
>
> The spec says:
>
> When the new get[Declared]AnnotationsByType(Class<T>) methods are
> called for a repeatable annotation type T, the question is how to 
> extend the policy
> to handle multiple annotations of type T on the superclass and/or 
> subclass. Oracle
> proposes the following policy for Java SE 8:
>
> . If a class declaration does not have either a "directly present" 
> annotation of
> type T or an "indirectly present by containment" annotation of type T, 
> the class
> declaration may be "associated" with an annotation of type T due to 
> inheritance.
>
> . If a class declaration does have either a "directly present" 
> annotation of type T
> or an "indirectly present by containment" annotation of type T, the 
> annotation
> is deemed to "override" every annotation of type T which is 
> "associated" with
> the superclass.
>
> This policy for Java SE 8 is reified in the following definitions:
>
> . An annotation A is directly present on an element E if E has a
> RuntimeVisibleAnnotations or RuntimeVisibleParameterAnnotations or
> RuntimeVisibleTypeAnnotations attribute, and the attribute contains 
> A.1.2 Core Reflection API REPEATING ANNOTATIONS
> 12
>
> . An annotation A is indirectly present on an element E if E has a
> RuntimeVisibleAnnotations or RuntimeVisibleParameterAnnotations or
> RuntimeVisibleTypeAnnotations attribute, and A's type is repeatable, 
> and the
> attribute contains exactly one annotation whose value element contains 
> A and
> whose type is the containing annotation type of A's type (§9.6).
>
> . An annotation A is present on an element E if either:
>  - A is directly present on E; or
>  - No annotation of A's type is directly present on E, and E is a 
> class, and A's type
> is inheritable (§9.6.3.3), and A is present on the superclass of E.
>
> . An annotation A is associated with an element E if either:
>  - A is directly or indirectly present on E; or
>  - No annotation of A's type is directly or indirectly present on E, 
> and E is a class,
> and A's type is inheritable (§9.6.3.3), and A is associated with the 
> superclass
> of E.
>
>
> So I guess E.class.getAnnotations() returns the annotations "present" 
> on the E
> and E.class.getAnnotationsByType(A.class) returns the annotations of 
> type A "associated" with E
>
> Am I right?
>
> Let's just focus on "associated" part now - the getAnnotationsByType 
> method. The following test:
>
> http://cr.openjdk.java.net/~plevart/jdk8-tl/RepearingAnnotations/InheritedRepeatingAnnotationsTest.java 
> <http://cr.openjdk.java.net/%7Eplevart/jdk8-tl/RepearingAnnotations/InheritedRepeatingAnnotationsTest.java>
>
> dumps @Repeatable @Inherited annotations "associated" with the 
> following classes:
>
> @Ann(10)
> class A1 {}
>
> @Ann(20)
> class A2 extends A1 {}
>
> class A3 extends A2 {}
>
>
> @Ann(10) @Ann(11)
> class B1 {}
>
> @Ann(20)
> class B2 extends B1 {}
>
> class B3 extends B2 {}
>
>
> @Ann(10)
> class C1 {}
>
> @Ann(20) @Ann(21)
> class C2 extends C1 {}
>
> class C3 extends C2 {}
>
>
> @Ann(10) @Ann(11)
> class D1 {}
>
> @Ann(20) @Ann(21)
> class D2 extends D1 {}
>
> class D3 extends D2 {}
>
>
> Here's the output:
>
>
> class A2: [@Ann(value=20)]
> class B2: [@Ann(value=20)]
> class C2: [@Ann(value=20), @Ann(value=21)]
> class D2: [@Ann(value=20), @Ann(value=21)]
>
> class A3: [@Ann(value=20)]
> class B3: [@Ann(value=10), @Ann(value=11), @Ann(value=20)]
> class C3: [@Ann(value=10), @Ann(value=20), @Ann(value=21)]
> class D3: [@Ann(value=20), @Ann(value=21)]
>
>
> Class B3 is showing @Ann(10) and @Ann(11), while class C3 is showing 
> @Ann(10) as being "associated" with them, but they should not, right?
>
> Class B3 should show the same annotations as B2 and class C3 should 
> show the same annotations as class C2, I think.
>
>
> Regards, Peter
>
>
>  On 10/22/2013 01:03 PM, Joel Borggrén-Franck wrote:
>> Hi Peter,
>>
>> Spec is here:http://cr.openjdk.java.net/~abuckley/8misc.pdf
>>
>> FYI I pushed this before I saw your mail but I do think the code is correct. An extra pair of eyes would be great though!
>>
>> FWIW I suspect we can be better with regards to allocation especially if we optimize for the common case where there is either a single annotation or a single container, but that will have to be a follow up.
>>
>> Btw there are a lot of extra test cases in the langtools repository (!?!) look in langtools/test/tools/javac/annotations/repeatingAnnotations/combo/ReflectionTest.java
>>
>> cheers
>> /Joel
>>
>> On 22 okt 2013, at 18:50, Peter Levart<peter.levart at gmail.com>  wrote:
>>
>>> Hi, I would just like to ask for a pointer to some specification document that describes inheritance of repeating annotations. I couldn't find one on the net.
>>>
>>> I have a feeling there's something wrong with the logic of AnnotationsSuport.getAssociatedAnnotations. Why? Because it is based on two maps: declaredAnnotations map and allAnnotations map. The later is a map of inherited and/or declared annotations which is aggregated without the knowledge of repeating annotations (the containers). I doubt this map keeps enough information to reconstruct a sound set of inherited and/or declared repeating annotations in all situations.
>>>
>>> But I'd like to 1st see the specification before showing you some examples where problems arise.
>>>
>>> Regards, Peter
>>>
>>> On 10/22/2013 12:21 PM, Joel Borggrén-Franck wrote:
>>>> Hi Andreas,
>>>>
>>>> A few nits:
>>>>
>>>> Class.java:
>>>>
>>>>   import java.util.Collection;
>>>> +import java.util.Collections;
>>>>   import java.util.HashSet;
>>>>
>>>> unused import.
>>>>
>>>> AnnotationSupport.java:
>>>>
>>>> +    /**
>>>> +     * Equivalent to calling {@code getDirectlyAndIndirectlyPresentAnnotations(
>>>> +     * annotations, annoClass, false)}.
>>>> +     */
>>>>
>>>> I think it is equivalent to annotations, annoClass, true
>>>>
>>>> Otherwise looks good. I can sponsor this fix.
>>>>
>>>> cheers
>>>> /Joel
>>>>
>>>> On 21 okt 2013, at 21:01, Andreas Lundblad<andreas.lundblad at oracle.com>  wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> New revision up for review:
>>>>>
>>>>> http://aoeu.se/webrevs/8019420-and-8004912/webrev.01
>>>>>
>>>>> The following has been addressed since webrev.00:
>>>>>
>>>>> - Order of directly / indirectly present annotations now respects the order of the keys in the given map of annotations.
>>>>>
>>>>> - A new test has been added to test the above behavior.
>>>>>
>>>>> best regards,
>>>>> Andreas
>>>>>
>>>>>
>>>>> ----- Original Message -----
>>>>> From:andreas.lundblad at oracle.com
>>>>> To:core-libs-dev at openjdk.java.net
>>>>> Sent: Wednesday, October 16, 2013 4:00:08 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna
>>>>> Subject: RFR: 8004912: Repeating annotations - getAnnotationsByType is not working as expected
>>>>>
>>>>> Hi,
>>>>>
>>>>> Please review the fix for JDK-8004912 and JDK-8019420 below.
>>>>>
>>>>> Description:
>>>>>
>>>>> The behavior of Class.get[Declared]AnnotationsByType was wrong. These methods delegate to sun.reflect.annotation.AnnotationSupport which has been rewritten.
>>>>>
>>>>> NonInheritableContainee.java is added and contains the test referred to in JDK-8019420.
>>>>>
>>>>> RepeatedUnitTest.java have been updated to include the test cases in JDK-8004912.
>>>>>
>>>>> There are more tests available in tl/langtools/test/tools/javac/annotations/repeatingAnnotations/combo/ReflectionTest.java (NB. this file is in the langtools repo)
>>>>>
>>>>>
>>>>> Link to web review:
>>>>> http://cr.openjdk.java.net/~alundblad/8019420-and-8004912/
>>>>>
>>>>> Link to bug reports:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8004912
>>>>> https://bugs.openjdk.java.net/browse/JDK-8019420
>>>>>
>>>>>
>>>>> -- Andreas Lundblad
>