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

[John Rose]

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
>>> 
>> 
>