JDK 8 code review request for 8014836: Have GenericDeclaration extend AnnotatedElement

[Joseph Darcy]

Hi Remi,

On 5/22/2013 8:36 AM, Remi Forax wrote:
> On 05/21/2013 03:16 AM, Joseph Darcy wrote:
>> Hi Remi,
>>
>> On 5/20/2013 2:28 PM, Remi Forax wrote:
>>> On 05/20/2013 11:10 PM, Joe Darcy wrote:
>>>> Hello,
>>>>
>>>> Please review the patch below which implements
>>>>
>>>>     8014836: Have GenericDeclaration extend AnnotatedElement
>>>>
>>>> All the existing implementations of GenericDeclaration in the JDK 
>>>> already implement AnnotatedElement. Some code in java.lang.Class 
>>>> needed to be adjusted slightly since AnnotatedElement declares a 
>>>> default method and calling an interface's default method in an 
>>>> implementing class has to go through the direct interface type.
>>>
>>> GenericDeclaration is a public interface, so you will break the code 
>>> of everyone that implements it.
>>> By example, Guava's Invokable also implements GenericDeclaration
>>> http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/reflect/Invokable.html 
>>>
>>
>> That is technically true; in effect adding several methods to 
>> GenericDeclaration would break implementations outside of the JDK 
>> that do not already implement annotated element.
>>
>> However, this is (only) a source incompatible change and not a binary 
>> incompatible change so is possible in-bounds for a platform release 
>> like JDK 8:
>>
>> http://cr.openjdk.java.net/~darcy/OpenJdkDevGuide/OpenJdkDevelopersGuide.v0.777.html#general_evolution_policy 
>>
>>
>> The Invokable type you reference is the moral equivalent of the 
>> "Executable" superclass of Constructor and Method added earlier in 
>> JDK 8. The Invokable type itself is marked as "@Beta" to warn users 
>> that it could change or even to away. Therefore, I'm not very 
>> concerned about "breaking" a type like this, which could easily be 
>> retrofitted to also implement AnnotatedElement as it already a 
>> wrapper around a Method or a Constructor.
>
> For me, it's not just a source incompatible change,
> let suppose that I use Guava reflection API in my program and the jdk8,
> I will be able to get a GenericDeclaration  that will fail at runtime 
> if I try to call a method of AnnotatedElement on it.
> So this change make the existing Guava library not forward compatible.

Again, that part of Guava is marked as "@Beta" to warn against relying 
on it too much. The JDK generally makes weaker promises about runtime 
reflection modeling as compared to compile-time or non-reflective 
runtime behavior.

>
> With the introduction of default methods, we now have a way to make 
> change in interfaces backward compatible,
> I don't say that we should use default methods here, I don't know.
> But I think it worth to think about that.
>
> I've just seen that AnnotatedElement also includes new new abstract 
> methods.

For now, I'm going to push the changes to GenericDeclaration as-is. If 
there are undo compatibility issues, default methods for the 
AnnotatedElement methods could be added later in 8 (but they would be a 
bit disappointing since they could have to either return nothing or 
throw Unsupporte-Operation-Exception.)

Thanks,

-Joe

>
>>
>> (Also interesting to see a "Parameter" class in Guava given that we 
>> have added a Parameter class to java.lang.reflect.)
>
> yes, but there is no getName() :)

More reason to upgrade to JDK 8 :-)

>
>>
>> Thanks,
>>
>> -Joe
>
> cheers,
> Rémi
>
>>
>>>
>>>>
>>>> Thanks,
>>>>
>>>> -Joe
>>>
>>> Rémi
>>>
>>>>
>>>> --- a/src/share/classes/java/lang/Class.java    Mon May 20 11:56:46 
>>>> 2013 -0700
>>>> +++ b/src/share/classes/java/lang/Class.java    Mon May 20 14:07:15 
>>>> 2013 -0700
>>>> @@ -28,6 +28,7 @@
>>>>  import java.lang.reflect.AnnotatedElement;
>>>>  import java.lang.reflect.Array;
>>>>  import java.lang.reflect.GenericArrayType;
>>>> +import java.lang.reflect.GenericDeclaration;
>>>>  import java.lang.reflect.Member;
>>>>  import java.lang.reflect.Field;
>>>>  import java.lang.reflect.Executable;
>>>> @@ -115,9 +116,9 @@
>>>>   * @since   JDK1.0
>>>>   */
>>>>  public final class Class<T> implements java.io.Serializable,
>>>> - java.lang.reflect.GenericDeclaration,
>>>> -                              java.lang.reflect.Type,
>>>> - java.lang.reflect.AnnotatedElement {
>>>> +                              GenericDeclaration,
>>>> +                              Type,
>>>> +                              AnnotatedElement {
>>>>      private static final int ANNOTATION= 0x00002000;
>>>>      private static final int ENUM      = 0x00004000;
>>>>      private static final int SYNTHETIC = 0x00001000;
>>>> @@ -3182,7 +3183,7 @@
>>>>       */
>>>>      @Override
>>>>      public boolean isAnnotationPresent(Class<? extends Annotation> 
>>>> annotationClass) {
>>>> -        return 
>>>> AnnotatedElement.super.isAnnotationPresent(annotationClass);
>>>> +        return 
>>>> GenericDeclaration.super.isAnnotationPresent(annotationClass);
>>>>      }
>>>>
>>>>      /**
>>>> diff -r 6a9148865139 
>>>> src/share/classes/java/lang/reflect/GenericDeclaration.java
>>>> --- a/src/share/classes/java/lang/reflect/GenericDeclaration.java 
>>>> Mon May 20 11:56:46 2013 -0700
>>>> +++ b/src/share/classes/java/lang/reflect/GenericDeclaration.java 
>>>> Mon May 20 14:07:15 2013 -0700
>>>> @@ -1,5 +1,5 @@
>>>>  /*
>>>> - * Copyright (c) 2003, 2011, Oracle and/or its affiliates. All 
>>>> rights reserved.
>>>> + * Copyright (c) 2003, 2013, Oracle and/or its affiliates. All 
>>>> rights reserved.
>>>>   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>>>   *
>>>>   * This code is free software; you can redistribute it and/or 
>>>> modify it
>>>> @@ -30,7 +30,7 @@
>>>>   *
>>>>   * @since 1.5
>>>>   */
>>>> -public interface GenericDeclaration {
>>>> +public interface GenericDeclaration extends AnnotatedElement {
>>>>      /**
>>>>       * Returns an array of {@code TypeVariable} objects that
>>>>       * represent the type variables declared by the generic
>>>>
>>>
>>
>