RFR (XS): 8019184: MethodHandles.catchException() fails when methods have 8 args + varargs

[Christian Thalinger]

Thank you, John.  -- Chris

On Jul 2, 2013, at 3:00 PM, John Rose <john.r.rose at oracle.com> wrote:

> Good; ship it!  — John
> 
> On Jul 2, 2013, at 2:50 PM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
> 
>> 
>> On Jul 2, 2013, at 2:04 PM, John Rose <john.r.rose at oracle.com> wrote:
>> 
>>> Good fix, good test.
>>> 
>>> Suggestions for the test:
>>> 
>>> Change e.printStackTrace() to throw new AssertionError(e), to get earlier detection of config error.
>> 
>> Done.
>> 
>>> 
>>> Suggest using a more specific type than Exception, maybe even a specific instance (like firstArg), to rule out interference from accidental errors.  Then detect the caught exception in the handler, before returning the desired value.
>> 
>> It seems to be part of the magic that the exception doesn't end up in the handler so I can't check for the exception instance.
>> 
>>> 
>>> Suggest defining MAX_ARITY (MAX_MH_ARITY?) more categorically as 254, with a comment.  Then have the loop go up to MAX_ARITY-1 with a comment, or put a break at the bottom of the loop body:
>>> if (handlerWithArgs.type().parameterCount() == MAX_MH_ARITY)  break;  // no more cases possible
>> 
>> Done.
>> 
>>> 
>>> As we have seen, arity limits are hard to reason about!
>>> 
>>> (Instead of dropArgs you could use something like MethodType.methodType(Object.class, Exception.class, ptypes.toArray()).)
>> 
>> This doesn't work because ptypes.toArray() returns an Object[] and I think that the drop is also part of the magic.  (I have engineered this test after the test case of the bug report.)
>> 
>> Anyway, here is an updated webrev:
>> 
>> http://cr.openjdk.java.net/~twisti/8019184/webrev/
>> 
>> -- Chris
>> 
>>> 
>>> — John
>>> 
>>> On Jul 2, 2013, at 1:33 PM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
>>> 
>>>> http://cr.openjdk.java.net/~twisti/8019184/webrev/
>>>> 
>>>> 8019184: MethodHandles.catchException() fails when methods have 8 args + varargs
>>>> Reviewed-by:
>>>> 
>>>> The bug seems to be in MethodHandlesImpl.makeGuardWithCatch using ValueConversions.varargsArray:
>>>> 
>>>> return makeCollectArguments(ginvoker, ValueConversions.varargsArray(nargs), 0, false);
>>>> 
>>>> ValueConversions.varargsArray returns a MethodHandle which's type is:
>>>> 
>>>> MethodHandle(Object,Object,Object,Object,Object,Object,Object,Object,Object)Object[]
>>>> 
>>>> It doesn't preserve the trailing Object[].
>>>> 
>>>> The fix is to call makePairwiseConvert on the result of makeCollectArguments.
>>>> 
>>>> src/share/classes/java/lang/invoke/MethodHandleImpl.java
>>>> test/java/lang/invoke/TestCatchExceptionWithVarargs.java
>>>> 
>>> 
>> 
>