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

Joe Darcy joe.darcy at oracle.com
Tue Jul 19 21:07:31 UTC 2011


Hi Mike.

On 7/19/2011 1:54 PM, Mike Duigou wrote:
> I reviewed the BlenderRev fairly closely and did not find any errors. The only weirdness I saw was several cases where multiple "Specified by:" declarations were present for a method and one of the instances referenced a class to which it didn't appear to be able to link to.
>
> Example: Method.getTypeParameters():
>
> Specified by:
> getTypeParameters in interface java.lang.reflect.GenericDeclaration
>
> It wasn't clear to me why it needed two "Specified by:" entries and only one of them was hot linked to the specifying class.

I did a one-off javadoc run just of the classes in question to get the 
BlenderRev; I didn't include GenericDeclaration in the set of types for 
which javadoc was generated, which is probably why the link was missing 
for that type.

I don't know all the criteria for the generation of the "Specified by:" 
references; however, there are other cases in the platform where more 
than one appears, such as ArrayList.size:

http://download.java.net/jdk7/docs/api/java/util/ArrayList.html#size()

Thanks,

-Joe


> Just javadoc weirdness?
>
> Mike
>
> On Jul 19 2011, at 12:49 , Joe Darcy wrote:
>
>> Agreed; I've posted a BlenderRev corresponding to the current patch at:
>>
>>    http://cr.openjdk.java.net/~darcy/7007535.4/BR-7007535.html
>>
>> Thanks,
>>
>> -Joe
>>
>> Mike Duigou wrote:
>>> This looks good to me but I agree with David that it's probably important to look at the generated javadoc and specdiff output before finalizing.
>>>
>>> Mike
>>>
>>> On Jul 18 2011, at 00:51 , Joe Darcy wrote:
>>>
>>>
>>>> Peter and David.
>>>>
>>>> Thanks for the careful review; the @throws information still needs its own {@inheritDoc}; I've uploaded a webrev with this and other corrections:
>>>>
>>>> http://cr.openjdk.java.net/~darcy/7007535.4
>>>>
>>>> More comments interspersed below.
>>>>
>>>> Thanks,
>>>>
>>>> -Joe
>>>>
>>>> Peter Jones wrote:
>>>>
>>>>> Hi Joe,
>>>>>
>>>>> On Jul 15, 2011, at 1:49 AM, David Holmes wrote:
>>>>>
>>>>>> On 14/07/2011 12:21 PM, joe.darcy at oracle.com wrote:
>>>>>>
>>>>>>> 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
>>>>>>> */
>>>>>>>
>>>>>> Unless they have fixed/changed javadoc (entirely possible) it used to be that the above would not cause @throws declarations for unchecked exceptions to be inherited - you have/had to explicitly repeat them as:
>>>>>>
>>>>>> @throws<exception-type>  {@inheritDoc}
>>>>>>
>>>>> Yes, that would seem to be needed for some of the inherited getters of generics info, which specify unchecked exception types.
>>>>>
>>>>>
>>>>>>> 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.
>>>>>>>
>>>>> In Executable.java, getAnnotation and getDeclaredAnnotations do have "@since 1.5"-- oversight?
>>>>>
>>>> Yes; that was incorrect.
>>>>
>>>>
>>>>> In Constructor.java and Method.java, getExceptionTypes has "@since 1.5", but that method has existed in those classes since 1.1.
>>>>>
>>>>>
>>>> Fixed.
>>>>
>>>>
>>>>> In Executable.java:
>>>>>
>>>>> 216     /**
>>>>> 217      * Returns an array of {@code Class} objects that represent the formal
>>>>> 218      * parameter types, in declaration order, of the method
>>>>> 219      * represented by this {@code Method} object.  Returns an array of length
>>>>> 220      * 0 if the underlying method takes no parameters.
>>>>> 221      *
>>>>> 222      * @return the parameter types for the method this object
>>>>> 223      * represents
>>>>>
>>>>> At least "{@code Method}" needs to be generalized, and perhaps all occurrences of "method"?
>>>>>
>>>> Corrected.
>>>>
>>>




More information about the core-libs-dev mailing list