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

[Remi Forax]

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.

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.

>
> (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() :)

>
> 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
>>>
>>
>