Method Parameter Reflection spec

[Eric McCorkle]

getAnnotations for each of these calls out to AnnotationParser, which
does indeed throw an AnnotationFormatError in a variety of cases
(duplicated annotations, malformed or incomplete annotations, etc).  In
other cases (ie type mismatches), it delays throwing of exceptions.
Exceptions coming from the VM (illegal access, bad constant pool
indexes, parameter mismatches, class casts, and the like) are caught and
rethrown as AnnotationFormatErrors.

In light of this, I'm going to argue the precedent's there.  General
formatting issues (which I'll argue covers bad names and wrong numbers
of parameters) cause errors to be thrown immediately, and the completely
analogous case of a bad constant pool index does cause an error to be
thrown immediately.

I'll also put forward that parameters are a bit different, and favor the
approach of having Executable.getParameters() be able to throw
exceptions for bad formatting.  Annotations are generally complex, with
far more complex correctness conditions, and far higher cost to
transform them from their storage format to reflective objects.  It
makes sense to defer some of this processing.  MethodParameters, on the
other hand, are very simple, and the main use case is to get the names
of parameters.  Therefore, the spec ought to imply, or at least allow
for an implementation that makes a single trip to the VM per method,
after which the names are immediately available.  (Also, there are
several practical advantages to allowing Parameter to be an immutable
object that cannot be subclassed...)

On 08/20/13 15:38, Alex Buckley wrote:
> The precedent is AnnotatedElement#getAnnotations as found in Class,
> Field, Executable, etc. Do those methods validate the
> RuntimeVisibleAnnotations attribute before returning an array of
> Annotation objects? I think they do. Can you describe the validation
> they do which may cause an AnnotationTypeMismatchException or
> IncompleteAnnotationException?
> 
> If their validation is "equivalent" to what Executable#getParameters
> would do, then it would be acceptable to change the spec and
> implementation of Executable#getParameters and
> Parameter#getName/Type/etc at this late stage. Please comment ASAP.
> 
> Alex
> 
> On 8/20/2013 12:10 PM, Eric McCorkle wrote:
>> Okay, strike #2 from my requests.  What about having
>> Executable.getParameters() throw ReflectiveOperationException instead of
>> methods in Parameter, for the reasons I discuss?
>>
>> On 08/19/13 18:12, Alex Buckley wrote:
>>> The 8misc.pdf file doesn't say anything about bad constant pool indexes.
>>>
>>> If MethodParameters was an attribute that the JVM is required to
>>> recognize and correctly read, then the JVM would be required to throw
>>> ClassFormatError for bad constant pool indexes. But since
>>> MethodParameters is an attribute that the class libraries of the Java SE
>>> platform - as opposed to the JVM - are required to recognize and
>>> correctly read, it's the responsibility of the class libraries to throw
>>> something which is a) useful and b) consistent with bad constant pool
>>> indexes for similar situations such as AnnotatedElement#getAnnotations
>>> on a RuntimeVisibleAnnotations attribute.
>>>
>>> How the class libraries interact with a JVM implementation to determine
>>> good or bad constant pool entries (e.g. private functions of the JVM
>>> implementation throw IllegalArgumentException, which
>>> AnnotatedElement#getAnnotations then wraps as a FooBarException) is not
>>> a matter for the SE API specification. The only thing that matters for
>>> the SE API specification is what something like
>>> AnnotatedElement#getAnnotations is specified to throw today.
>>>
>>> Alex
>>>
>>> On 8/19/2013 2:57 PM, Eric McCorkle wrote:
>>>> The current version of the spec indicates that methods in
>>>> java.lang.reflect.Parameter should throw ReflectiveOperationException
>>>> for several cases (wrong number of parameters, bad parameter name, bad
>>>> constant pool index).
>>>>
>>>> I'd like to suggest two (separate) minor revisions:
>>>>
>>>> 1) Executable.getParameters() should throw the
>>>> ReflectiveOperationException in the case of an invalid name or the
>>>> wrong
>>>> number of parameters.  The rationale is as follows:
>>>>
>>>>    * Doing it this way makes it straightforward to implement
>>>> j.l.r.Parameter as an immutable object.  Having methods in Parameter
>>>> throw the exception means Parameter objects will have to remember
>>>> whether or not they have verified their name (or else verify the name
>>>> every time they are called).
>>>>
>>>>    * Having every single method in Parameter possibly check whether the
>>>> number of parameters that were returned from the VM is correct seems
>>>> rather awkward, as opposed to having a single check when Executable
>>>> obtains the Parameter array back from the VM.
>>>>
>>>> 2) In the event of a bad constant pool index, an
>>>> IllegalArgumentException.
>>>>
>>>>    * The current behavior of hotspot whenever it expects a UTF-8
>>>> constant
>>>> pool entry, but sees either an out-of-bounds index or something other
>>>> than a UTF-8 is to throw an IllegalArgumentException.
>>>>