Re: RFR [10] 8194554: filterArguments runs multiple filters in the wrong order

[Aleksey Shipilev]

On 04/12/2018 08:23 PM, joe darcy wrote:
> I've manually created a CSR for this issue and linked it in to the backport; 
> HTH,

Thank you Joe!

-Aleksey

Re: RFR [10] 8194554: filterArguments runs multiple filters in the wrong order

[joe darcy]

I've manually created a CSR for this issue and linked it in to the 
backport; HTH,


-Joe


On 4/12/2018 11:15 AM, joe darcy wrote:

On 4/12/2018 11:11 AM, Aleksey Shipilev wrote:

On 04/12/2018 07:43 PM, Paul Sandoz wrote:
Sorry to be the bearer of more bureaucratic news i just found out we 
require a CSR for 10 as
well… which means you need to explicitly create a back port issue 
for 10 and hook the CSR for 10
to that (there may be some small consolation that the CSR for 11 can 
be mostly copied verbatim).

So I created the backport:
   https://bugs.openjdk.java.net/browse/JDK-8201502

...but I cannot create the CSR. "New Issue" -> "CSR" says "CSRs 
cannot be created manually, instead
use the Create CSR operation on the Issue View page". But there is no 
"Create CSR" to be found anywhere.


Hello,

CSR hat on, that is the proper procedure, but a bug fix to allow the 
creation of CSRs for backport isn't in production yet. I'm checking 
with the JBS team.


Sorry for the delay,

-Joe

Re: RFR [10] 8194554: filterArguments runs multiple filters in the wrong order

[joe darcy]

On 4/12/2018 11:11 AM, Aleksey Shipilev wrote:

On 04/12/2018 07:43 PM, Paul Sandoz wrote:

Sorry to be the bearer of more bureaucratic news i just found out we require a 
CSR for 10 as
well… which means you need to explicitly create a back port issue for 10 and 
hook the CSR for 10
to that (there may be some small consolation that the CSR for 11 can be mostly 
copied verbatim).

So I created the backport:
   https://bugs.openjdk.java.net/browse/JDK-8201502

...but I cannot create the CSR. "New Issue" -> "CSR" says "CSRs cannot be 
created manually, instead
use the Create CSR operation on the Issue View page". But there is no "Create 
CSR" to be found anywhere.


Hello,

CSR hat on, that is the proper procedure, but a bug fix to allow the 
creation of CSRs for backport isn't in production yet. I'm checking with 
the JBS team.


Sorry for the delay,

-Joe

Re: RFR [10] 8194554: filterArguments runs multiple filters in the wrong order

[Aleksey Shipilev]

On 04/12/2018 07:43 PM, Paul Sandoz wrote:
> Sorry to be the bearer of more bureaucratic news i just found out we require 
> a CSR for 10 as
> well… which means you need to explicitly create a back port issue for 10 and 
> hook the CSR for 10
> to that (there may be some small consolation that the CSR for 11 can be 
> mostly copied verbatim).
So I created the backport:
  https://bugs.openjdk.java.net/browse/JDK-8201502

...but I cannot create the CSR. "New Issue" -> "CSR" says "CSRs cannot be 
created manually, instead
use the Create CSR operation on the Issue View page". But there is no "Create 
CSR" to be found anywhere.

-Aleksey

Re: RFR [10] 8194554: filterArguments runs multiple filters in the wrong order

[Paul Sandoz]

Hi Aleksey,

Sorry to be the bearer of more bureaucratic news i just found out we require a 
CSR for 10 as well… which means you need to explicitly create a back port issue 
for 10 and hook the CSR for 10 to that (there may be some small consolation 
that the CSR for 11 can be mostly copied verbatim). 

Paul.

> On Apr 12, 2018, at 10:06 AM, Aleksey Shipilev  wrote:
> 
> Thanks Mandy!
> 
> I added the comments to the JBS issue, hoping that would be enough to get the 
> backport moving.
> 
> -Aleksey
> 
> On 04/12/2018 12:24 PM, mandy chung wrote:
>> I was on vacation last week.  Paul - thanks for submitting CSR for 
>> JDK-8194554 for the resulting
>> behavioral change.
>> 
>> This backport looks good to me.
>> 
>> Thanks
>> Mandy
>> 
>> On 4/5/18 11:33 PM, Aleksey Shipilev wrote:
>>> Hi,
>>> 
>>> Please review this jdk10 backport.
>>> 
>>> Original JDK 11 bug:
>>>  https://bugs.openjdk.java.net/browse/JDK-8194554
>>> 
>>> Original JDK 11 fix:
>>>  http://hg.openjdk.java.net/jdk/jdk/rev/050352ed64d5
>>> 
>>> Please note the discussion in JBS comments about the issue. It seems to 
>>> include the more verbose
>>> specification text, and Rob says it makes the patch not directly 
>>> backportable. Therefore, requesting
>>> to backport without the Javadoc change. I just dropped that single line 
>>> from the original changeset:
>>> 
>>> 
>>> 8194554: filterArguments runs multiple filters in the wrong order
>>> Reviewed-by: psandoz, jrose
>>> 
>>> diff -r b09e56145e11 -r ab2dc3096cdb 
>>> src/java.base/share/classes/java/lang/invoke/MethodHandles.java
>>> --- a/src/java.base/share/classes/java/lang/invoke/MethodHandles.java   
>>> Thu Mar 08 04:23:31 2018 +
>>> +++ b/src/java.base/share/classes/java/lang/invoke/MethodHandles.java   
>>> Wed Jan 17 15:17:50 2018 -0800
>>> @@ -3836,11 +3836,12 @@
>>> MethodHandle filterArguments(MethodHandle target, int pos, 
>>> MethodHandle... filters) {
>>> filterArgumentsCheckArity(target, pos, filters);
>>> MethodHandle adapter = target;
>>> -int curPos = pos-1;  // pre-incremented
>>> -for (MethodHandle filter : filters) {
>>> -curPos += 1;
>>> +// process filters in reverse order so that the invocation of
>>> +// the resulting adapter will invoke the filters in left-to-right 
>>> order
>>> +for (int i = filters.length - 1; i >= 0; --i) {
>>> +MethodHandle filter = filters[i];
>>> if (filter == null)  continue;  // ignore null elements of 
>>> filters
>>> -adapter = filterArgument(adapter, curPos, filter);
>>> +adapter = filterArgument(adapter, pos + i, filter);
>>> }
>>> return adapter;
>>> }
>>> diff -r b09e56145e11 -r ab2dc3096cdb 
>>> test/jdk/java/lang/invoke/FilterArgumentsTest.java
>>> --- /dev/null   Thu Jan 01 00:00:00 1970 +
>>> +++ b/test/jdk/java/lang/invoke/FilterArgumentsTest.javaWed Jan 17 
>>> 15:17:50 2018 -0800
>>> @@ -0,0 +1,132 @@
>>> +/*
>>> + * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
>>> + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>> + *
>>> + * This code is free software; you can redistribute it and/or modify it
>>> + * under the terms of the GNU General Public License version 2 only, as
>>> + * published by the Free Software Foundation.
>>> + *
>>> + * This code is distributed in the hope that it will be useful, but WITHOUT
>>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
>>> + * version 2 for more details (a copy is included in the LICENSE file that
>>> + * accompanied this code).
>>> + *
>>> + * You should have received a copy of the GNU General Public License 
>>> version
>>> + * 2 along with this work; if not, write to the Free Software Foundation,
>>> + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
>>> + *
>>> + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
>>> + * or visit www.oracle.com if you need additional information or have any
>>> + * questions.
>>> + */
>>> +
>>> +/*
>>> + * @test
>>> + * @bug 8194554
>>> + * @run testng/othervm test.java.lang.invoke.FilterArgumentsTest
>>> + */
>>> +
>>> +package test.java.lang.invoke;
>>> +
>>> +import java.lang.invoke.MethodHandle;
>>> +import java.lang.invoke.MethodHandles;
>>> +import java.util.ArrayList;
>>> +import java.util.List;
>>> +
>>> +import static java.lang.invoke.MethodHandles.*;
>>> +import static java.lang.invoke.MethodType.methodType;
>>> +
>>> +import org.testng.annotations.*;
>>> +import static org.testng.Assert.*;
>>> +
>>> +public class FilterArgumentsTest {
>>> +
>>> +@Test
>>> +public static void testFilterA_B_C() throws Throwable {
>>> +FilterTest test = new FilterTest(
>>> +filterArguments(MH_TEST, 0, MH_FILTER_A, MH_FILTER_B, 
>>> MH_FILTER_C));
>>> +test.run(List.of("A", "B",

Re: RFR [10] 8194554: filterArguments runs multiple filters in the wrong order

[Aleksey Shipilev]

Thanks Mandy!

I added the comments to the JBS issue, hoping that would be enough to get the 
backport moving.

-Aleksey

On 04/12/2018 12:24 PM, mandy chung wrote:
> I was on vacation last week.  Paul - thanks for submitting CSR for 
> JDK-8194554 for the resulting
> behavioral change.
> 
> This backport looks good to me.
> 
> Thanks
> Mandy
> 
> On 4/5/18 11:33 PM, Aleksey Shipilev wrote:
>> Hi,
>>
>> Please review this jdk10 backport.
>>
>> Original JDK 11 bug:
>>   https://bugs.openjdk.java.net/browse/JDK-8194554
>>
>> Original JDK 11 fix:
>>   http://hg.openjdk.java.net/jdk/jdk/rev/050352ed64d5
>>
>> Please note the discussion in JBS comments about the issue. It seems to 
>> include the more verbose
>> specification text, and Rob says it makes the patch not directly 
>> backportable. Therefore, requesting
>> to backport without the Javadoc change. I just dropped that single line from 
>> the original changeset:
>>
>>
>> 8194554: filterArguments runs multiple filters in the wrong order
>> Reviewed-by: psandoz, jrose
>>
>> diff -r b09e56145e11 -r ab2dc3096cdb 
>> src/java.base/share/classes/java/lang/invoke/MethodHandles.java
>> --- a/src/java.base/share/classes/java/lang/invoke/MethodHandles.java
>> Thu Mar 08 04:23:31 2018 +
>> +++ b/src/java.base/share/classes/java/lang/invoke/MethodHandles.java
>> Wed Jan 17 15:17:50 2018 -0800
>> @@ -3836,11 +3836,12 @@
>>  MethodHandle filterArguments(MethodHandle target, int pos, 
>> MethodHandle... filters) {
>>  filterArgumentsCheckArity(target, pos, filters);
>>  MethodHandle adapter = target;
>> -int curPos = pos-1;  // pre-incremented
>> -for (MethodHandle filter : filters) {
>> -curPos += 1;
>> +// process filters in reverse order so that the invocation of
>> +// the resulting adapter will invoke the filters in left-to-right 
>> order
>> +for (int i = filters.length - 1; i >= 0; --i) {
>> +MethodHandle filter = filters[i];
>>  if (filter == null)  continue;  // ignore null elements of 
>> filters
>> -adapter = filterArgument(adapter, curPos, filter);
>> +adapter = filterArgument(adapter, pos + i, filter);
>>  }
>>  return adapter;
>>  }
>> diff -r b09e56145e11 -r ab2dc3096cdb 
>> test/jdk/java/lang/invoke/FilterArgumentsTest.java
>> --- /dev/nullThu Jan 01 00:00:00 1970 +
>> +++ b/test/jdk/java/lang/invoke/FilterArgumentsTest.java Wed Jan 17 
>> 15:17:50 2018 -0800
>> @@ -0,0 +1,132 @@
>> +/*
>> + * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
>> + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>> + *
>> + * This code is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License version 2 only, as
>> + * published by the Free Software Foundation.
>> + *
>> + * This code is distributed in the hope that it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
>> + * version 2 for more details (a copy is included in the LICENSE file that
>> + * accompanied this code).
>> + *
>> + * You should have received a copy of the GNU General Public License version
>> + * 2 along with this work; if not, write to the Free Software Foundation,
>> + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
>> + *
>> + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
>> + * or visit www.oracle.com if you need additional information or have any
>> + * questions.
>> + */
>> +
>> +/*
>> + * @test
>> + * @bug 8194554
>> + * @run testng/othervm test.java.lang.invoke.FilterArgumentsTest
>> + */
>> +
>> +package test.java.lang.invoke;
>> +
>> +import java.lang.invoke.MethodHandle;
>> +import java.lang.invoke.MethodHandles;
>> +import java.util.ArrayList;
>> +import java.util.List;
>> +
>> +import static java.lang.invoke.MethodHandles.*;
>> +import static java.lang.invoke.MethodType.methodType;
>> +
>> +import org.testng.annotations.*;
>> +import static org.testng.Assert.*;
>> +
>> +public class FilterArgumentsTest {
>> +
>> +@Test
>> +public static void testFilterA_B_C() throws Throwable {
>> +FilterTest test = new FilterTest(
>> +filterArguments(MH_TEST, 0, MH_FILTER_A, MH_FILTER_B, 
>> MH_FILTER_C));
>> +test.run(List.of("A", "B", "C"));
>> +}
>> +
>> +@Test
>> +public static void testFilterA_B() throws Throwable {
>> +FilterTest test = new FilterTest(
>> +filterArguments(MH_TEST, 0, MH_FILTER_A, MH_FILTER_B));
>> +test.run(List.of("A", "B"));
>> +}
>> +
>> +@Test
>> +public static void testFilterB_C() throws Throwable {
>> +FilterTest test = new FilterTest(
>> +filterArguments(MH_TEST, 1, MH_FILTER_B, MH_FILTER_C));
>> +test.run(List.of("B",

Re: RFR [10] 8194554: filterArguments runs multiple filters in the wrong order

[mandy chung]

I was on vacation last week.  Paul - thanks for submitting CSR for 
JDK-8194554 for the resulting behavioral change.


This backport looks good to me.

Thanks
Mandy

On 4/5/18 11:33 PM, Aleksey Shipilev wrote:

Hi,

Please review this jdk10 backport.

Original JDK 11 bug:
   https://bugs.openjdk.java.net/browse/JDK-8194554

Original JDK 11 fix:
   http://hg.openjdk.java.net/jdk/jdk/rev/050352ed64d5

Please note the discussion in JBS comments about the issue. It seems to include 
the more verbose
specification text, and Rob says it makes the patch not directly backportable. 
Therefore, requesting
to backport without the Javadoc change. I just dropped that single line from 
the original changeset:


8194554: filterArguments runs multiple filters in the wrong order
Reviewed-by: psandoz, jrose

diff -r b09e56145e11 -r ab2dc3096cdb 
src/java.base/share/classes/java/lang/invoke/MethodHandles.java
--- a/src/java.base/share/classes/java/lang/invoke/MethodHandles.java   Thu Mar 
08 04:23:31 2018 +
+++ b/src/java.base/share/classes/java/lang/invoke/MethodHandles.java   Wed Jan 
17 15:17:50 2018 -0800
@@ -3836,11 +3836,12 @@
  MethodHandle filterArguments(MethodHandle target, int pos, 
MethodHandle... filters) {
  filterArgumentsCheckArity(target, pos, filters);
  MethodHandle adapter = target;
-int curPos = pos-1;  // pre-incremented
-for (MethodHandle filter : filters) {
-curPos += 1;
+// process filters in reverse order so that the invocation of
+// the resulting adapter will invoke the filters in left-to-right order
+for (int i = filters.length - 1; i >= 0; --i) {
+MethodHandle filter = filters[i];
  if (filter == null)  continue;  // ignore null elements of filters
-adapter = filterArgument(adapter, curPos, filter);
+adapter = filterArgument(adapter, pos + i, filter);
  }
  return adapter;
  }
diff -r b09e56145e11 -r ab2dc3096cdb 
test/jdk/java/lang/invoke/FilterArgumentsTest.java
--- /dev/null   Thu Jan 01 00:00:00 1970 +
+++ b/test/jdk/java/lang/invoke/FilterArgumentsTest.javaWed Jan 17 
15:17:50 2018 -0800
@@ -0,0 +1,132 @@
+/*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8194554
+ * @run testng/othervm test.java.lang.invoke.FilterArgumentsTest
+ */
+
+package test.java.lang.invoke;
+
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+
+import static java.lang.invoke.MethodHandles.*;
+import static java.lang.invoke.MethodType.methodType;
+
+import org.testng.annotations.*;
+import static org.testng.Assert.*;
+
+public class FilterArgumentsTest {
+
+@Test
+public static void testFilterA_B_C() throws Throwable {
+FilterTest test = new FilterTest(
+filterArguments(MH_TEST, 0, MH_FILTER_A, MH_FILTER_B, 
MH_FILTER_C));
+test.run(List.of("A", "B", "C"));
+}
+
+@Test
+public static void testFilterA_B() throws Throwable {
+FilterTest test = new FilterTest(
+filterArguments(MH_TEST, 0, MH_FILTER_A, MH_FILTER_B));
+test.run(List.of("A", "B"));
+}
+
+@Test
+public static void testFilterB_C() throws Throwable {
+FilterTest test = new FilterTest(
+filterArguments(MH_TEST, 1, MH_FILTER_B, MH_FILTER_C));
+test.run(List.of("B", "C"));
+}
+
+@Test
+public static void testFilterB() throws Throwable {
+FilterTest test = new FilterTest(filterArguments(MH_TEST, 1, 
MH_FILTER_B));
+test.run(List.of("B"));
+}
+
+@Test
+public static void testFilterC() throws Throwable {
+FilterTest test = new FilterTest(filterArguments(MH_TEST, 2, 
MH_FILTER_C));
+test.run(List.of("C"));
+}
+
+static class FilterTest {
+static List filters = new ArrayList<>();
+
+final MethodHandle mh;
+   

Re: RFR [10] 8194554: filterArguments runs multiple filters in the wrong order

[Aleksey Shipilev]

On 04/10/2018 06:40 PM, Paul Sandoz wrote:
> Here we go:
> 
>   https://bugs.openjdk.java.net/browse/JDK-8201371
> 
> Kindly requesting a reviewer.

Not sure if that counts for CSR, but added myself as Reviewer there.

-Aleksey

Re: RFR [10] 8194554: filterArguments runs multiple filters in the wrong order

[Paul Sandoz]

> On Apr 10, 2018, at 3:21 AM, Aleksey Shipilev  wrote:
> 
> On 04/10/2018 08:42 AM, Alan Bateman wrote:
>> On 09/04/2018 18:58, Paul Sandoz wrote:
>>> I am supportive of this change (the risk to impacting order-dependent 
>>> stateful MH filter code is
>>> smaller than the risk of hitting a string concatenation bug). (We erred on 
>>> the side of this being
>>> a bug and not being a spec change given the pseudo-code in the Java doc.)
>>> 
>>> IIRC this will require a nod of approval from the project lead.
>>> 
>>> Paul.
>> I think this is a CSR for Java SE 11 first as it adds a testable assertion 
>> to the spec and also
>> changes existing behavior.
> 

Note that the testable assertion was already in the spec, just buried in the 
normative pseudocode. So this is really about clarification from the spec 
perspective. And of course any bug fix can change behavior. In hindsight i 
would be more inclined to do a CSR if a bug fix is required to conform to 
specification.


> Ok, fine! Paul, can you submit the CSR for this?
> 

Here we go:

  https://bugs.openjdk.java.net/browse/JDK-8201371 


Kindly requesting a reviewer.

Thanks,
Paul.

Re: RFR [10] 8194554: filterArguments runs multiple filters in the wrong order

[Aleksey Shipilev]

On 04/10/2018 08:42 AM, Alan Bateman wrote:
> On 09/04/2018 18:58, Paul Sandoz wrote:
>> I am supportive of this change (the risk to impacting order-dependent 
>> stateful MH filter code is
>> smaller than the risk of hitting a string concatenation bug). (We erred on 
>> the side of this being
>> a bug and not being a spec change given the pseudo-code in the Java doc.)
>>
>> IIRC this will require a nod of approval from the project lead.
>>
>> Paul.
> I think this is a CSR for Java SE 11 first as it adds a testable assertion to 
> the spec and also
> changes existing behavior.

Ok, fine! Paul, can you submit the CSR for this?

-Aleksey

Re: RFR [10] 8194554: filterArguments runs multiple filters in the wrong order

[Alan Bateman]

On 09/04/2018 18:58, Paul Sandoz wrote:

I am supportive of this change (the risk to impacting order-dependent stateful 
MH filter code is smaller than the risk of hitting a string concatenation bug). 
(We erred on the side of this being a bug and not being a spec change given the 
pseudo-code in the Java doc.)

IIRC this will require a nod of approval from the project lead.

Paul.
I think this is a CSR for Java SE 11 first as it adds a testable 
assertion to the spec and also changes existing behavior.


-Alan

Re: RFR [10] 8194554: filterArguments runs multiple filters in the wrong order

[Paul Sandoz]

I am supportive of this change (the risk to impacting order-dependent stateful 
MH filter code is smaller than the risk of hitting a string concatenation bug). 
(We erred on the side of this being a bug and not being a spec change given the 
pseudo-code in the Java doc.)

IIRC this will require a nod of approval from the project lead.

Paul.

> On Apr 9, 2018, at 2:52 AM, Aleksey Shipilev  wrote:
> 
> Ping.
> 
> -Aleksey
> 
> On 04/05/2018 05:33 PM, Aleksey Shipilev wrote:
>> Hi,
>> 
>> Please review this jdk10 backport.
>> 
>> Original JDK 11 bug:
>>  https://bugs.openjdk.java.net/browse/JDK-8194554
>> 
>> Original JDK 11 fix:
>>  http://hg.openjdk.java.net/jdk/jdk/rev/050352ed64d5
>> 
>> Please note the discussion in JBS comments about the issue. It seems to 
>> include the more verbose
>> specification text, and Rob says it makes the patch not directly 
>> backportable. Therefore, requesting
>> to backport without the Javadoc change. I just dropped that single line from 
>> the original changeset:
>> 
>> 
>> 8194554: filterArguments runs multiple filters in the wrong order
>> Reviewed-by: psandoz, jrose
>> 
>> diff -r b09e56145e11 -r ab2dc3096cdb 
>> src/java.base/share/classes/java/lang/invoke/MethodHandles.java
>> --- a/src/java.base/share/classes/java/lang/invoke/MethodHandles.java
>> Thu Mar 08 04:23:31 2018 +
>> +++ b/src/java.base/share/classes/java/lang/invoke/MethodHandles.java
>> Wed Jan 17 15:17:50 2018 -0800
>> @@ -3836,11 +3836,12 @@
>> MethodHandle filterArguments(MethodHandle target, int pos, 
>> MethodHandle... filters) {
>> filterArgumentsCheckArity(target, pos, filters);
>> MethodHandle adapter = target;
>> -int curPos = pos-1;  // pre-incremented
>> -for (MethodHandle filter : filters) {
>> -curPos += 1;
>> +// process filters in reverse order so that the invocation of
>> +// the resulting adapter will invoke the filters in left-to-right 
>> order
>> +for (int i = filters.length - 1; i >= 0; --i) {
>> +MethodHandle filter = filters[i];
>> if (filter == null)  continue;  // ignore null elements of 
>> filters
>> -adapter = filterArgument(adapter, curPos, filter);
>> +adapter = filterArgument(adapter, pos + i, filter);
>> }
>> return adapter;
>> }
>> diff -r b09e56145e11 -r ab2dc3096cdb 
>> test/jdk/java/lang/invoke/FilterArgumentsTest.java
>> --- /dev/nullThu Jan 01 00:00:00 1970 +
>> +++ b/test/jdk/java/lang/invoke/FilterArgumentsTest.java Wed Jan 17 
>> 15:17:50 2018 -0800
>> @@ -0,0 +1,132 @@
>> +/*
>> + * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
>> + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>> + *
>> + * This code is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License version 2 only, as
>> + * published by the Free Software Foundation.
>> + *
>> + * This code is distributed in the hope that it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
>> + * version 2 for more details (a copy is included in the LICENSE file that
>> + * accompanied this code).
>> + *
>> + * You should have received a copy of the GNU General Public License version
>> + * 2 along with this work; if not, write to the Free Software Foundation,
>> + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
>> + *
>> + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
>> + * or visit www.oracle.com if you need additional information or have any
>> + * questions.
>> + */
>> +
>> +/*
>> + * @test
>> + * @bug 8194554
>> + * @run testng/othervm test.java.lang.invoke.FilterArgumentsTest
>> + */
>> +
>> +package test.java.lang.invoke;
>> +
>> +import java.lang.invoke.MethodHandle;
>> +import java.lang.invoke.MethodHandles;
>> +import java.util.ArrayList;
>> +import java.util.List;
>> +
>> +import static java.lang.invoke.MethodHandles.*;
>> +import static java.lang.invoke.MethodType.methodType;
>> +
>> +import org.testng.annotations.*;
>> +import static org.testng.Assert.*;
>> +
>> +public class FilterArgumentsTest {
>> +
>> +@Test
>> +public static void testFilterA_B_C() throws Throwable {
>> +FilterTest test = new FilterTest(
>> +filterArguments(MH_TEST, 0, MH_FILTER_A, MH_FILTER_B, 
>> MH_FILTER_C));
>> +test.run(List.of("A", "B", "C"));
>> +}
>> +
>> +@Test
>> +public static void testFilterA_B() throws Throwable {
>> +FilterTest test = new FilterTest(
>> +filterArguments(MH_TEST, 0, MH_FILTER_A, MH_FILTER_B));
>> +test.run(List.of("A", "B"));
>> +}
>> +
>> +@Test
>> +public static void testFilterB_C() throws Throwable {
>> +FilterTest test = new FilterTest(
>> +filterA

Re: RFR [10] 8194554: filterArguments runs multiple filters in the wrong order

[Aleksey Shipilev]

Ping.

-Aleksey

On 04/05/2018 05:33 PM, Aleksey Shipilev wrote:
> Hi,
> 
> Please review this jdk10 backport.
> 
> Original JDK 11 bug:
>   https://bugs.openjdk.java.net/browse/JDK-8194554
> 
> Original JDK 11 fix:
>   http://hg.openjdk.java.net/jdk/jdk/rev/050352ed64d5
> 
> Please note the discussion in JBS comments about the issue. It seems to 
> include the more verbose
> specification text, and Rob says it makes the patch not directly 
> backportable. Therefore, requesting
> to backport without the Javadoc change. I just dropped that single line from 
> the original changeset:
> 
> 
> 8194554: filterArguments runs multiple filters in the wrong order
> Reviewed-by: psandoz, jrose
> 
> diff -r b09e56145e11 -r ab2dc3096cdb 
> src/java.base/share/classes/java/lang/invoke/MethodHandles.java
> --- a/src/java.base/share/classes/java/lang/invoke/MethodHandles.java Thu Mar 
> 08 04:23:31 2018 +
> +++ b/src/java.base/share/classes/java/lang/invoke/MethodHandles.java Wed Jan 
> 17 15:17:50 2018 -0800
> @@ -3836,11 +3836,12 @@
>  MethodHandle filterArguments(MethodHandle target, int pos, 
> MethodHandle... filters) {
>  filterArgumentsCheckArity(target, pos, filters);
>  MethodHandle adapter = target;
> -int curPos = pos-1;  // pre-incremented
> -for (MethodHandle filter : filters) {
> -curPos += 1;
> +// process filters in reverse order so that the invocation of
> +// the resulting adapter will invoke the filters in left-to-right 
> order
> +for (int i = filters.length - 1; i >= 0; --i) {
> +MethodHandle filter = filters[i];
>  if (filter == null)  continue;  // ignore null elements of 
> filters
> -adapter = filterArgument(adapter, curPos, filter);
> +adapter = filterArgument(adapter, pos + i, filter);
>  }
>  return adapter;
>  }
> diff -r b09e56145e11 -r ab2dc3096cdb 
> test/jdk/java/lang/invoke/FilterArgumentsTest.java
> --- /dev/null Thu Jan 01 00:00:00 1970 +
> +++ b/test/jdk/java/lang/invoke/FilterArgumentsTest.java  Wed Jan 17 
> 15:17:50 2018 -0800
> @@ -0,0 +1,132 @@
> +/*
> + * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
> + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> + *
> + * This code is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 only, as
> + * published by the Free Software Foundation.
> + *
> + * This code is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
> + * version 2 for more details (a copy is included in the LICENSE file that
> + * accompanied this code).
> + *
> + * You should have received a copy of the GNU General Public License version
> + * 2 along with this work; if not, write to the Free Software Foundation,
> + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
> + *
> + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
> + * or visit www.oracle.com if you need additional information or have any
> + * questions.
> + */
> +
> +/*
> + * @test
> + * @bug 8194554
> + * @run testng/othervm test.java.lang.invoke.FilterArgumentsTest
> + */
> +
> +package test.java.lang.invoke;
> +
> +import java.lang.invoke.MethodHandle;
> +import java.lang.invoke.MethodHandles;
> +import java.util.ArrayList;
> +import java.util.List;
> +
> +import static java.lang.invoke.MethodHandles.*;
> +import static java.lang.invoke.MethodType.methodType;
> +
> +import org.testng.annotations.*;
> +import static org.testng.Assert.*;
> +
> +public class FilterArgumentsTest {
> +
> +@Test
> +public static void testFilterA_B_C() throws Throwable {
> +FilterTest test = new FilterTest(
> +filterArguments(MH_TEST, 0, MH_FILTER_A, MH_FILTER_B, 
> MH_FILTER_C));
> +test.run(List.of("A", "B", "C"));
> +}
> +
> +@Test
> +public static void testFilterA_B() throws Throwable {
> +FilterTest test = new FilterTest(
> +filterArguments(MH_TEST, 0, MH_FILTER_A, MH_FILTER_B));
> +test.run(List.of("A", "B"));
> +}
> +
> +@Test
> +public static void testFilterB_C() throws Throwable {
> +FilterTest test = new FilterTest(
> +filterArguments(MH_TEST, 1, MH_FILTER_B, MH_FILTER_C));
> +test.run(List.of("B", "C"));
> +}
> +
> +@Test
> +public static void testFilterB() throws Throwable {
> +FilterTest test = new FilterTest(filterArguments(MH_TEST, 1, 
> MH_FILTER_B));
> +test.run(List.of("B"));
> +}
> +
> +@Test
> +public static void testFilterC() throws Throwable {
> +FilterTest test = new FilterTest(filterArguments(MH_TEST, 2, 
> MH_FILTER_C));
> +test.run(List.of("C"));
> +}
> +
> +static clas