repeating annotations comments

[Remi Forax]

On 12/16/2012 05:07 AM, Paul Benedict wrote:
> I think there is an argument to say that since container annotations
> are mandatory for repeating annotations, there shouldn't be any
> looking through. The container annotation should be the thing always
> returned. I think repeating annotations should be looked at merely as
> syntactic compiler sugar. Then all these complex rules can go away.

yes, no magic is a good argument.

>
> Paul

Rémi

>
> On Fri, Dec 14, 2012 at 7:41 PM, Alex Buckley <alex.buckley at oracle.com> wrote:
>> Hi Mike,
>>
>> Thanks for the quick feedback. (I know you've been playing with the JDK8 b68
>> binary.)
>>
>>
>> On 12/14/2012 1:59 PM, michael keith wrote:
>>> 1. The getAnnotation(Class<T>) method is being changed to look through
>>> container annotations. This method returns an Annotation, i.e. a single
>>> instance, so applying it to repeated ones is necessarily going to have
>>> to return only one. But having it look through container annotation ends
>>> up not being very productive, and may be causing it to be
>>> counter-productive.
>>>
>>> Look-through for this method is not productive because the call is being
>>> made precisely in the legacy cases when only one annotation was
>>> expected, but a container annotation will only be synthesized when
>>> multiples are present. So look-through does not really help get "the"
>>> expected annotation. The only case when it would be useful is in the
>>> unlikely case when the container annotation is used explicitly and
>>> contains a single contained annotation.
>>
>> I feel the author of the repeatable annotation type (who "opted in") and the
>> author of the multiple annotations (@Foo @Foo class C ...) bear some
>> responsibility when a legacy caller of getAnnotation(Class<T>) sees
>> one-out-of-many annotations. Someone decided there is no more of "'the'
>> expected annotation".
>>
>>
>>> It may be counter-productive for three reasons.
>>> The first is because getting a random single annotation in the presence
>>> of multiples is potentially dangerous since the calling code will
>>> continue without knowing that multiples are present, so legacy code is
>>> essentially broken, but will not actually fail. It will just be
>>> semantically incorrect.
>>
>> Acknowledged.
>>
>>
>>> The second is because I didn't see (and I may just have missed it) where
>>> it was defined that if both T and TC were present that T would be
>>> guaranteed to be returned and not some instance within TC. Example 1.2-3
>>> shows this to be the case but I didn't see that it was specified
>>> behavior rather than just because the T occurred before the TC. If the
>>> example had been TC and then T would the instance of T be returned?
>>
>> If both @T and @TC are present, we have not yet specified [1] whether
>> get[Declared]Annotation(Class<T>) returns always the lone @T v. always a
>> containerized @T v. something different every time. That's what your
>> feedback is for :-)
>>
>> [1]
>> http://download.java.net/jdk8/docs/api/java/lang/reflect/AnnotatedElement.html#getAnnotation%28java.lang.Class%29
>>
>>
>>> This could be important because existing code to handle this case
>>> checks for the container annotation TC and then checks for the single
>>> annotation T. However, if checking for T happens to look through TC
>>> and return one of the contained annotations in TC instead of the lone
>>> T annotation then the lone annotation would not get found.
>>
>> The implementation currently "prefers" a lone @T over a @T inside @TC. If
>> you have a declaration  @TC(...) @T class C {}  then
>> C.class.getAnnotation(T.class) will return the standalone @T. So existing
>> code to handle this case will still work.
>>
>>
>>> The third is because all of the code that is currently out there that
>>> needs to check for the previous case, i.e. both T and TC occurring, will
>>> get a false positive in the simpler case of multiple instances of T
>>> annotations (as per the second part of example 1.2-1 in the doc). The
>>> getAnnotation(TC) will return the synthesized TC containing all of the
>>> instances of T, and then getAnnotation(T) will return one of those same
>>> instances again. Legacy code will assume that the result of
>>> getAnnotation(T) is disjoint from the contained annotations of the TC
>>> returned by getAnnotation(TC).
>>
>> Acknowledged.
>>
>>
>>> 2. Given that get[Declared]Annotations() is going to change and return
>>> multiple instances of the same annotation (rather than a single instance
>>> of a container annotation) existing implementations are going to have to
>>> be fixed since there is no backwards compatibility there at all.
>>
>> Acknowledged. The category of existing implementations which assume there
>> will never be more than one annotation of a given type in the returned array
>> is in for a surprise.
>>
>> But there is a category of existing implementations which will continue to
>> work fine. Suppose code calls get[Declared]Annotations() and looks first for
>> @T in the array; only if @T is not found does the code look for @TC in the
>> array. This seems like rational behavior for a non-repeating world, as it
>> means one call to get annotations rather than getAnnotation(T.class) plus
>> getAnnotation(TC.class). Now, run the code on SE 8 to get the new behavior
>> of get[Declared]Annotations(). The code will find an @T in the array (the
>> first of many, perhaps) and there will be no search for @TC and everything
>> will be fine. (Assume a top-level @T is returned before containerized @T's.)
>>
>>
>>> 3. In general, it appears that once an annotation is made repeatable the
>>> annotation processing code is inevitably going to have to change in
>>> order to handle the repeatability. We were willing to take that hit,
>>> particularly since the new behavior requires opting in, and that means
>>> that specs could do so at their own discretion. However, the code to
>>> handle both the new and the old (i.e. be able to process both new
>>> repeatable annotations as well as the older idiomatic container
>>> annotations) is a little less intuitive to write than I had hoped it
>>> would be. The easiest way might just be to create a Set of instances to
>>> avoid duplicates. Not rocket science, just not what I would have expected.
>>
>> I'm not sure if you are proposing that AnnotatedElement should return Set
>> instances or your framework code should create Set instances. By policy,
>> core reflection does not depend on the rest of the Java SE API, so uses
>> array types as aggregates.
>>
>> So, where are we? Joe and I have always recognized that changing
>> getAnnotation(Class<T>) and get[Declared]Annotations() to look through
>> official containers would affect legacy callers who assume there is at most
>> one @T. A legacy caller which allows for both @T and @TC is perhaps hit
>> harder than the rest - we weren't sure how many legacy callers were this
>> careful, but you imply it's non-trivial.
>>
>> Here's an idea. We pondered having the "existing" methods -
>> get[Declared]Annotation(Class<T>) and get[Declared]Annotations() - be
>> unaware of official containers, so they would return exactly what's in the
>> class file. Essentially this is perfect behavioral compatibility. Meanwhile,
>> the new get[Declared]Annotations(Class<T>) methods would look through
>> official containers if present. How does that grab you?
>>
>> Alex