Hi Liam,
On 2/24/2018 5:14 PM, Liam Miller-Cushon wrote:
Hi, thanks for the comments.
The updated webrev is at:
http://cr.openjdk.java.net/~cushon/7183985/webrev.02/
<http://cr.openjdk.java.net/%7Ecushon/7183985/webrev.02/>
On Fri, Feb 23, 2018 at 4:29 PM, Joseph D. Darcy <joe.da...@oracle.com
<mailto:joe.da...@oracle.com>> wrote:
Objects implementing the AnnotatedElement interface are also
created in javac during annotation processing. As a secondary
concern, it would be good to be have behavior between both javac
and runtime annotations consistent when possible. (My own to-do
list includes at least once such alignment JDK-8164819: "Make
javac's toString() on annotation objects consistent with core
reflection".)
Do you have a pointer to where that happens? There's
javax.lang.model.AnnotatedConstruct#getAnnotation, which isn't
affected by this issue--it throws MirroredTypesExceptionProxy when
accessing an annotation value that is an array of class literals,
regardless of whether the elements' classes are missing. I'm not
seeing implementations of AnnotatedElement in langtools.
Sorry, I misremembered the situation in javac. For annotation processing
in javac, AnnotatedElement is not a factor, but the class
src/jdk.compiler/share/classes/com/sun/tools/javac/model/AnnotationProxyMaker.java
in javac does create annotation proxies, which is the same general
technique used to create the annotation objects at runtime for core
reflection. These annotation objects in javac for javax.lang.model and
annotation processing, do not follow the full contract of
java.lang.reflect.AnnotatedElement. In particular, the annotations
returned are not necessarily serializable. The intersection of methods
on AnnotatedConstruct and AnnotatedElement is only a proper subset of
the methods on AnnotatedElement; however, getAnnotation​(Class<T>
annotationClass) is in the intersection.
Even in javac we've moved away from test and directory names based
on bugid. I'd recommend incorporating these regression tests into
the existing tests in
test/jdk/java/lang/annotation/Missing
Thanks for the reminder, done.
I believe the new tests could reuse some of the existing types in
test/jdk/java/lang/annotation/Missing. For example, the new
MissingAnnotation.java is an alpha-rename of the existing Missing.java.
If such sharing is not practical, then I'd recommend putting the new
files into a subdirectory under underneath test/.../annotation/Missing
(otherwise it will be confusing to edit these tests in the future since
too many file names will start with "Missing".)
In MissingClassArrayElementTest.java:
71 public static void main(String[] args) throws Exception {
72 ClassArrayAnnotation[] outer =
Test.class.getAnnotation(AnnotationAnnotation.class).value();
73 // The second entry in the values array can be loaded
successfully
74 assertEquals(Arrays.toString(outer[1].value()), "[class
java.lang.String]");
Something like
assertEquals(Arrays.equals(outer[1].value(), {String.class})
would be more robust against any future changes in Class.toString.
Likewise for the analogous comparisons.
It would be worth verifying whether or not this change also covers
java.lang.reflect.Executable.getParameterAnnotations(), more
specifically the implementation of that method in Method and
Constructor. The method getParameterAnnotations() is much less
used than getAnnotations and the other methods on the
AnnotatedElement interface, but still part of the annotations feature.
Done.
Thanks.
Cheers,
-Joe
