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 +0000
> +++ 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 +0000
> +++ 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 class FilterTest {
> +        static List<String> filters = new ArrayList<>();
> +
> +        final MethodHandle mh;
> +        FilterTest(MethodHandle mh) {
> +            this.mh = mh;
> +        }
> +
> +        void run(List<String> expected) throws Throwable {
> +            filters.clear();
> +            assertEquals("x-0-z", (String)mh.invokeExact("x", 0, 'z'));
> +            assertEquals(expected, filters);
> +        }
> +
> +        static String filterA(String s) {
> +            filters.add("A");
> +            return s;
> +        }
> +
> +        static int filterB(int value) {
> +            filters.add("B");
> +            return value;
> +        }
> +
> +        static char filterC(char c) {
> +            filters.add("C");
> +            return c;
> +        }
> +
> +        static String test(String s, int i, char c) {
> +            return s + "-" + i + "-" + c;
> +        }
> +    }
> +
> +    static final MethodHandle MH_TEST;
> +    static final MethodHandle MH_FILTER_A;
> +    static final MethodHandle MH_FILTER_B;
> +    static final MethodHandle MH_FILTER_C;
> +    static final Lookup LOOKUP = MethodHandles.lookup();
> +
> +    static {
> +        try {
> +            MH_TEST = LOOKUP.findStatic(FilterTest.class, "test",
> +                methodType(String.class, String.class, int.class, char.class));
> +            MH_FILTER_A = LOOKUP.findStatic(FilterTest.class, "filterA",
> +                methodType(String.class, String.class));
> +            MH_FILTER_B = LOOKUP.findStatic(FilterTest.class, "filterB",
> +                methodType(int.class, int.class));
> +            MH_FILTER_C = LOOKUP.findStatic(FilterTest.class, "filterC",
> +                methodType(char.class, char.class));
> +        } catch (Exception e) {
> +            throw new RuntimeException(e);
> +        }
> +    }
> +}
>
>
> Thanks,
> -Aleksey