On 2/28/2012 10:52 AM, Mike Duigou wrote:
These changes look good to me. Is there a new CR for the javadoc changes?

Thanks Mike; I was planning to file the bug after the reviews came in.

-Joe

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&trade; 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&trade; 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.

Reply via email to