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.theeyar...@oracle.com>
Cc: core-libs-dev <core-libs-dev@openjdk.java.net>; Nadeesh TV
<nadeesh.thatathil.vala...@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.vala...@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.
