Method Parameter Reflection spec

[Eric McCorkle]

I will give Joe a day to add his comments.

Here's a list of all the ways MethodParameters attributes could go wrong:

(i) Actual length does not match attribute_length
(ii) The number of parameters (parameter_count) is wrong for the method
(iii) A constant pool index is out of bounds.
(iv) A constant pool index does not refer to a UTF-8 entry
(v) A parameter's name is "", or contains an illegal character [0]
(vi) The flags field contains an illegal flag (something other than
FINAL, SYNTHETIC, or MANDATED)

(i) causes a ClassFileFormatError when parsing the class file (as it
should).  Right now, (iii) and (iv) cause an IllegalArgumentException to
be thrown.

I am working on a patch to correct the behavior for (ii)-(vi), so I can
have Executable.getParameters() throw whatever exception you choose (ie.
this is one of those rare moments of freedom).


[0]: I seem to recall there being some discussion about backspaces, but
the current version doesn't seem to rule them out.  Has this been changed?

On 08/20/13 18:10, Alex Buckley wrote:
> Sounds good. I'd wait a day in case Joe has a comment, then do it.
> Please send a summary to this list of which exceptions are thrown by
> Executable#getParameters and for what reasons. This will let me record
> some more color in section 2.3 of 8misc.pdf, above and beyond the change
> from:
> 
> Methods of class java.lang.reflect.Parameter must throw
> ReflectiveOperationException if ...
> 
> to:
> 
> java.lang.reflect.Executable#getParameters must throw
> ReflectiveOperationException if ...
> 
> Alex
> 
> On 8/20/2013 1:41 PM, Eric McCorkle wrote:
>> 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.
>>>>>>