[9] RFR (M): 8050166: Get rid of some package-private methods on arguments in j.l.i.MethodHandle

Vladimir Ivanov vladimir.x.ivanov at oracle.com
Wed Jul 16 10:53:36 UTC 2014


Paul, thanks for review.

On 7/16/14 12:34 PM, Paul Sandoz wrote:
>
> On Jul 14, 2014, at 4:04 PM, Vladimir Ivanov <vladimir.x.ivanov at oracle.com> wrote:
>
>> http://cr.openjdk.java.net/~vlivanov/8050166/webrev.00/
>> https://bugs.openjdk.java.net/browse/JDK-8050166
>>
>> Get rid of the following methods in j.l.i.MethodHandle:
>>   * convertArguments(MethodType newType)
>>   * bindArgument(int pos, BasicType basicType, Object value)
>>   * bindReceiver(Object receiver)
>>   * dropArguments(MethodType srcType, int pos, int drops)
>>   * permuteArguments(MethodType newType, int[] reorder)
>>
>> Testing: jdk/java/lang/invoke, jdk/java/util/streams, nashorn, octane w/ "-ea -esa" and COMPILE_THRESHOLD={0,30}.
>>
>
> Looks good. Minor points, take it or leave it:
>
> MethodHandles. permuteArgumentChecks
>
> Might be more preferable for permuteArgumentChecks to return true or false and do the "throw newIllegalArgumentException" on a false return. Then if would play better with it's use in the assert, since it can throw an assertion error.
>
>    assert target == originalTarget;
>    assert permuteArgumentChecks(reorder, newType, target.type()) : "bad reorder array: "+Arrays.toString(reorder)
It's incorrect to split the assert. It's either target & reorder are 
unchanged or new values pass all checks.
And if exception is thrown outside permuteArgumentChecks, we lose 
detailed error message.

So, I'd leave this code as is.

> MethodHandles.insertArgumentPrimitive
>
> Might be more consistent to always use a return value in the switch:
>
> 2258         switch (w) {
> 2259         default:      intValue = ValueConversions.widenSubword(value);    break;
> 2260         case INT:     intValue = (int)value;                              break;
>
> default: return result.bindArgumentI(pos, ValueConversions.widenSubword(value));
> case INT: return result.bindArgumentI(pos, (int)value);
Agree. Fixed.

Best regards,
Vladimir Ivanov


More information about the mlvm-dev mailing list