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

[Joel Borggrén-Franck]

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
>