Review request:Updated JDK-8004728 Implement core reflection API for parameter reflection

[Joe Darcy]

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