These changes look good to me. Is there a new CR for the javadoc changes? Mike
On Feb 28 2012, at 09:03 , Joe Darcy wrote: > Hi David, > > Belatedly catching up on email, please review the patch below to address the > issues you've raised. I searched for "method" and replaced it with > "executable" as appropriate throughout the javadoc of the class. > > Thanks, > > -Joe > > --- a/src/share/classes/java/lang/reflect/Executable.java Tue Feb 28 > 17:00:28 2012 +0400 > +++ b/src/share/classes/java/lang/reflect/Executable.java Tue Feb 28 > 09:01:27 2012 -0800 > @@ -180,7 +180,7 @@ > > /** > * Returns the {@code Class} object representing the class or interface > - * that declares the method represented by this executable object. > + * that declares the executable represented by this object. > */ > public abstract Class<?> getDeclaringClass(); > > @@ -215,18 +215,18 @@ > * Returns an array of {@code Class} objects that represent the formal > * parameter types, in declaration order, of the executable > * represented by this object. Returns an array of length > - * 0 if the underlying method takes no parameters. > + * 0 if the underlying executable takes no parameters. > * > - * @return the parameter types for the method this object > + * @return the parameter types for the executable this object > * represents > */ > public abstract Class<?>[] getParameterTypes(); > > /** > * Returns an array of {@code Type} objects that represent the formal > - * parameter types, in declaration order, of the method represented by > - * this executable object. Returns an array of length 0 if the > - * underlying method takes no parameters. > + * parameter types, in declaration order, of the executable represented > by > + * this object. Returns an array of length 0 if the > + * underlying executable takes no parameters. > * > * <p>If a formal parameter type is a parameterized type, > * the {@code Type} object returned for it must accurately reflect > @@ -236,16 +236,16 @@ > * type, it is created. Otherwise, it is resolved. > * > * @return an array of {@code Type}s that represent the formal > - * parameter types of the underlying method, in declaration order > + * parameter types of the underlying executable, in declaration order > * @throws GenericSignatureFormatError > * if the generic method signature does not conform to the format > * specified in > * <cite>The Java™ Virtual Machine Specification</cite> > * @throws TypeNotPresentException if any of the parameter > - * types of the underlying method refers to a non-existent type > + * types of the underlying executable refers to a non-existent type > * declaration > * @throws MalformedParameterizedTypeException if any of > - * the underlying method's parameter types refer to a parameterized > + * the underlying executable's parameter types refer to a > parameterized > * type that cannot be instantiated for any reason > */ > public Type[] getGenericParameterTypes() { > @@ -277,15 +277,15 @@ > * type, it is created. Otherwise, it is resolved. > * > * @return an array of Types that represent the exception types > - * thrown by the underlying method > + * thrown by the underlying executable > * @throws GenericSignatureFormatError > * if the generic method signature does not conform to the format > * specified in > * <cite>The Java™ Virtual Machine Specification</cite> > - * @throws TypeNotPresentException if the underlying method's > + * @throws TypeNotPresentException if the underlying executable's > * {@code throws} clause refers to a non-existent type declaration > * @throws MalformedParameterizedTypeException if > - * the underlying method's {@code throws} clause refers to a > + * the underlying executable's {@code throws} clause refers to a > * parameterized type that cannot be instantiated for any reason > */ > public Type[] getGenericExceptionTypes() { > @@ -330,7 +330,7 @@ > * Returns an array of arrays that represent the annotations on > * the formal parameters, in declaration order, of the executable > * represented by this object. (Returns an array of length zero if > - * the underlying method is parameterless. If the executable has > + * the underlying executable is parameterless. If the executable has > * one or more parameters, a nested array of length zero is > * returned for each parameter with no annotations.) The > * annotation objects contained in the returned arrays are > > > On 07/20/2011 01:03 AM, David Holmes wrote: >> Just realized this has come in too late ... >> >> Joe Darcy said the following on 07/20/11 05:49: >>> Agreed; I've posted a BlenderRev corresponding to the current patch at: >>> >>> http://cr.openjdk.java.net/~darcy/7007535.4/BR-7007535.html >> >> Thanks. So now I can more readily see that the doc for Executable isn't >> quite suitable for Constructor in a few places: >> >> getDeclaringClass: >> >> 183 /** >> 184 * Returns the {@code Class} object representing the class or >> interface >> 185 * that declares the method represented by this executable object. >> 186 */ >> >> For Constructor "method" should be "constructor". But I think, looking at >> the terminology elsewhere that the above could be rewritten as: >> >> "Returns the Class object representing the class or interface that declares >> the executable represented by this object." >> >> getParameterTypes: >> >> 219 * represented by this object. Returns an array of length >> 220 * 0 if the underlying method takes no parameters. >> >> method -> executable >> >> getGenericParameterTypes: >> >> 229 * parameter types, in declaration order, of the method represented >> by >> 230 * this executable object. Returns an array of length 0 if the >> 231 * underlying method takes no parameters. >> >> Again change to " the executable represented by this object". >> And then: underlying method -> underlying executable >> >> 241 * parameter types of the underlying method, in declaration order >> 243 * if the generic method signature does not conform to the format >> 247 * types of the underlying method refers to a non-existent type >> 250 * the underlying method's parameter types refer to a >> parameterized >> >> method -> executable >> >> In fact do a search for "method" in Executable and most occurrences should >> be changed to "executable". >> >> >> Finally, in getModifiers, why delete the "as an integer. The Modifier class >> should be used to decode the modifiers. " ? >> >> >> BTW I also find the multiple "Specified by:" references to be quite strange. >> There can only be one specification for a method. >> >> David >> ----- >> >>> 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.da...@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. >>>> >>> >