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

[Paul Sandoz]

On Jul 16, 2014, at 12:53 PM, Vladimir Ivanov <vladimir.x.ivanov at oracle.com> wrote:

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

Doh! right it's "||" not "&&".


> And if exception is thrown outside permuteArgumentChecks, we lose detailed error message.
> 

> So, I'd leave this code as is.
> 

Fair enough, what i proposed only solves one possible source of IllegalArgumentException,i dunno how fastidious one should be about ensuring asserts throw AssertionError rather than another runtime exception or error that could potentially be handled differently, or be misconstrued as a bug in the assertion code itself.

Paul.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 841 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://mail.openjdk.java.net/pipermail/mlvm-dev/attachments/20140716/8674899c/signature.asc>