JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method

[Joe Darcy]

Dr Andrew John Hughes wrote:
> On 19:21 Wed 13 Jul     , joe.darcy at oracle.com wrote:
>   
>> Hello.
>>
>> Please code review my JDK 8 changes for
>>
>>     7007535: (reflect) Please generalize Constructor and Method
>>     http://cr.openjdk.java.net/~darcy/7007535.3
>>
>> To summarize the changes, a new superclass is defined to capture the 
>> common functionality of java.lang.reflect.Method and 
>> java.lang.reflect.Constructor.  That superclass is named "Executable" 
>> along the lines of javax.lang.model.ExecutableElement, which models 
>> constructors and methods in the JSR 269 language model.
>>
>> Both specification and implementation code are shared.  To preserve the 
>> right @since behavior, it is common that in Method/Constructor the 
>> javadoc for a method will now look like:
>>
>> /**
>>  * {@inheritDoc}
>>  * @since 1.5
>>  */
>>
>> Since Executable is being created in JDK 8, it would be incorrect for 
>> methods in that class to have an @since of 1.5; adding the @since in 
>> Method/Constructor preserves the right information.
>>
>>     
>
> I assume this is why we have some methods in the subclass that just
> call the superclass.
>   

Correct.  This would not have been necessary if Executable were added 
back in, say, JDK 5.

>   
>> It would have been natural to also move common fields to Executable; 
>> however, HotSpot treats the Constructor and Method type specially and 
>> relies on the existing field ordering.  Since altering the field layout 
>> would require coordinated HotSpot changes, I'm opting to not perform 
>> such a change right now.  At least one abstract accessor method is 
>> declared in Executable to still allow code sharing even though the 
>> required field is not present.  In other cases, package private instance 
>> methods on Executable are passed the needed state from overridden public 
>> methods in Method/Constructor.
>>
>> All java/lang/reflect regression tests pass on a full build with these 
>> changes.
>>
>>     
>
> The changes look good (though hard to read in places due to additional
> whitespace changes mixed in).  Nice to see these two finally being grouped
> together.
>
>   

Thanks,

-Joe