Hello Eric,
A few initial comments.
I'm not convinced a getNumParameters method pulls its weight. At the
very least, the specification should be updated to say whether or not it
includes synthetic + "natural" parameters or just natural ones.
(Different methods in core reflection indirectly return a synthetic +
"natural" count vs a natural count today.)
I recommend referencing syntheticness in the way core reflection was
recently update to do:
8005097: Tie isSynthetic javadoc to the JLS
http://hg.openjdk.java.net/jdk8/tl/jdk/rev/1f9c19741285
I think the notion of parameter equality should be expanded to include
the declaring executable.
In the fullness of time, the getAnnotations methods will need to account
for synthetic parameters and skip over them. One example of this
wrinkle today is if the parameters of a constructor for an inner class
are annotated.
For the tests, I recommend a few changes. First, code like this
for(int i = 0; i < parameters.length; i++) {
Parameter p = parameters[i];
....
}
is more natural as a for-each loop:
for(Paramter p : m.getParameters()) {
....
}
The per method or per parameter expected results can be encoded using
annotations. One recent example of this technique can be found in the
fix for:
8005042: Add Method.isDefault to core reflection
http://hg.openjdk.java.net/jdk8/tl/jdk/rev/0a1398021c7c
HTH,
-Joe
On 12/19/2012 2:43 PM, Eric McCorkle wrote:
Hello,
Please review the implementation of the core reflection API for method
parameter reflection. This changeset introduces a new class, Parameter,
which represents information about method parameters. It introduces a
new method into Executable which returns an array of the parameters for
the method or constructor represented by the Executable.
This is part of a larger set of changes which implement portions of the
method parameter reflection functionality in hotspot and javac.
The open webrev is here:
http://cr.openjdk.java.net/~jgish/~emccorkl/webrev/JDK-8004729/
The feature request is here:
http://bugs.sun.com/view_bug.do?bug_id=8004729
The latest version of the spec can be found here:
http://cr.openjdk.java.net/~abuckley/8misc.pdf
Thanks,
Eric
