JDK 8 RFR for JDK-8023471,,Add compatibility note to AnnotatedElement

[Joel Borggren-Franck]

Hi Joe, Stuart,

I think this looks good enough but Stuart has a point.

Perhaps add that it is a binary compatible change to make an annotation
type repeatable to begin with.

cheers
/Joel

On 2013-12-04, Stuart Marks wrote:
> Hi Joe,
> 
> The changes look sensible to me. Funnily enough, when I read the
> next, the voice echoing in my head sounds like Alex's....
> 
> A small clarification:
> 
> >+ * <li> The addition of the additional annotations is both source
> >+ * compatible and binary compatible. That is, the source containing
> >+ * the element and its annotations will still compile and the class
> >+ * file resulting from the compilation will continue to link.
> 
> This seems to imply that binary compatibility is with respect to the
> binary resulting from having recompiled the source. I have no doubt
> that such a binary will link successfully, but that's not quite what
> I had expected when I read "binary compatibility".
> 
> I would think that binary compatibility in this context would refer
> to clients that have *not* been recompiled, but which are linked
> with a newer version of the library that has changed to allow
> repeating annotations. Or perhaps it would refer to an old class
> binary that *uses an annotation, being linked with a library that
> *defines* that annotation, whose definition has been changed to
> allow that annotation to repeat. I don't know if these are real
> issues, but they're what came to mind when "binary compatibility"
> was mentioned.
> 
> The statement in the paragraph I quoted above would seem more
> sensible to me if the mention of "binary compatible" were struck.
> 

On 2013-12-03, Joe Darcy wrote:
> Hello,
> 
> Please review the patch below to address
> 
>     JDK-8023471 Add compatibility note to AnnotatedElement
> 
> Thanks,
> 
> -Joe
> 
> diff -r cd4aabc40f72
> src/share/classes/java/lang/reflect/AnnotatedElement.java
> --- a/src/share/classes/java/lang/reflect/AnnotatedElement.java Tue
> Dec 03 11:52:18 2013 -0800
> +++ b/src/share/classes/java/lang/reflect/AnnotatedElement.java Tue
> Dec 03 21:23:16 2013 -0800
> @@ -135,7 +135,63 @@
>   * annotations on <i>E</i> are directly present on <i>E</i> in place
>   * of their container annotation, in the order in which they appear in
>   * the value element of the container annotation.
> -
> + *
> + * <p>There are several compatibility concerns to keep in mind if an
> + * annotation type <i>T</i> is <em>not</em> repeatable in one release
> + * of a library and retrofitted to be repeatable in a subsequent
> + * release.
> + *
> + * <ul>
> + *
> + * <li>If an annotation of type <i>T</i> is present on an
> + * element and <i>T</i> is made repeatable and more annotations of
> + * type <i>T</i> are added to the element:
> + *
> + *
> + * <ul>
> + *
> + * <li> The addition of the additional annotations is both source
> + * compatible and binary compatible. That is, the source containing
> + * the element and its annotations will still compile and the class
> + * file resulting from the compilation will continue to link.
> + *
> + * <li>The addition of the additional annotations is <em>not</em>
> + * behaviorally compatible with respect to the {@code
> + * get[Declared]Annotation(Class<T>)} methods and {@code
> + * get[Declared]Annotations()} methods, because those methods will now
> + * only see a container annotations on the element and not see an
> + * annotation of type <i>T</i>.
> + *
> + * </ul>
> + *
> + * <li>If an annotation type <i>TC</i> is present
> + * on an element, then making some other annotation type <i>T</i>
> + * repeatable with <i>TC</i> as its containing annotation type then:
> + *
> + * <ul>
> + *
> + * <li>The change to <i>T</i>is source and binary compatible with
> + * existing uses of annotations of type <i>T</i> as well as
> + * annotations of type <i>TC</i>.
> + *
> + * <li>The change to <i>T</i> is behaviorally compatible with respect
> + * to the {@code get[Declared]Annotation(Class<T>)} (called with an
> + * argument of <i>T</i> or <i>TC</i>) and {@code
> + * get[Declared]Annotations()} methods because the results of the
> + * methods will not change due to <i>TC</i> becoming the containing
> + * annotation type for <i>T</i>.
> + *
> + * <li>The change to <i>T</i> is <em>not</em> behaviorally compatible
> + * with respect to the {@code
> + * get[Declared]AnnotationsByType(Class<T>)} methods, because those
> + * methods will now recognize an annotation of type <i>TC</i> as a
> + * container annotation and "look through" it to expose annotations of
> + * type <i>T</i>.
> + *
> + * </ul>
> + *
> + * </ul>
> + *
>   * <p>If an annotation returned by a method in this interface contains
>   * (directly or indirectly) a {@link Class}-valued member referring to
>   * a class that is not accessible in this VM, attempting to read the class
>