Dr Andrew John Hughes wrote:
On 19:21 Wed 13 Jul , joe.da...@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
