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

Joel Borggrén-Franck joel.franck at oracle.com
Tue Oct 22 15:26:08 UTC 2013 

Hi Peter,

On my way to dinner I realized we have a problem for inherited annotations there are inherited over two (or more) declarations. I believe this is essentially the issue you highlighted though I will look into this tomorrow.

On 22 okt 2013, at 21:20, Peter Levart <peter.levart at gmail.com> wrote:

> 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).
> 

Yes, this is how I solved it for javax.lang.model and this is how I think we should fix this.

> - 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?
> 

There hasn't been any attempt to do this at runtime. Fwiw I don't think this is worth the complexity unless someone proves the iterative lookup on super types is a performance issue.

cheers
/Joel

> 
> 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
>> 
>> 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
>>>>>> 
>> 
>

More information about the core-libs-dev mailing list