RFR: 8177276: MethodHandles.insertArguments doesn't specify IllegalArgumentException on index mismatch

[Vivek Theeyarath]

Hi All,
	Thanks for the comments. I have incorporated the changes as per Nadeesh's and Paul's comments. The test runs fine with jtreg post the changes. Also, the typo errors Mandy pointed out has also been fixed. Please find the updated webrev.

http://cr.openjdk.java.net/~vtheeyarath/8177276/webrev.03/ 

Bug : https://bugs.openjdk.java.net/browse/JDK-8177276 
CSR : https://bugs.openjdk.java.net/browse/JDK-8202991 

Regards
Vivek
-----Original Message-----
From: Paul Sandoz 
Sent: Monday, May 21, 2018 10:19 PM
To: Vivek Theeyarath <vivek.theeyarath at oracle.com>
Cc: core-libs-dev <core-libs-dev at openjdk.java.net>; Nadeesh TV <nadeesh.thatathil.valappu at mdcpartners.be>
Subject: Re: RFR: 8177276: MethodHandles.insertArguments doesn't specify IllegalArgumentException on index mismatch



> On May 19, 2018, at 7:14 AM, Nadeesh TV <nadeesh.thatathil.valappu at mdcpartners.be> wrote:
> 
> Hi Vivek,
> 
> IMHO, assigning back to methodHandle on  line 109, 115, 122,123 is confusing
> 

MethodHandlesInsertArgumentsTest.java
—

Yes, not just confusing but incorrect, the updated static field will affect what is tested so you are dependent on the order of test method execution. Make the field final and use the convention for static field names, and drop the assignment for the insertArgument calls.


 118     @Test
 119     public void testInsertArgumentsPosZero() {
 120         countTest();
 121         try {
 122             methodHandle = MethodHandles.insertArguments(methodHandle, 0, "First");
 123             methodHandle = MethodHandles.insertArguments(methodHandle, 1, "First", new Object());
 124             Assert.fail("ClassCastException not thrown");
 125         }
 126         catch(ClassCastException cce) {
 127         }
 128     }

You can split this out into multiple try/catch, one for each MethodHandles.insertArguments call so the assertion directly captures the failure point, otherwise declare the exception in the @Test.


MethodHandles.java
—

3489      * @throws ClassCastException If an argument does not mach the corresponding bound parameter

Lower case the “If”

Paul.