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