JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method
Dr Andrew John Hughes
ahughes at redhat.com
Tue Jul 19 20:08:45 UTC 2011
On 12:49 Tue 19 Jul , 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
>
What is BlenderRev? Google finds others posted by Oracle employees but
not how to generate them.
> 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.
> >>
> >
> >
>
--
Andrew :)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D 0698 0713 C3ED F586 2A37
More information about the core-libs-dev
mailing list