8150956: j.l.i.MethodHandles.whileLoop(...) and .iteratedLoop(...) throw unexpected exceptions in the case of 'init' return type is void
authormhaupt
Tue, 19 Apr 2016 14:39:35 +0200
changeset 37539 fc220bc54b59
parent 37538 feed20ba7a0c
child 37540 e92d95400f31
8150956: j.l.i.MethodHandles.whileLoop(...) and .iteratedLoop(...) throw unexpected exceptions in the case of 'init' return type is void Reviewed-by: psandoz
jdk/src/java.base/share/classes/java/lang/invoke/MethodHandles.java
jdk/test/java/lang/invoke/LoopCombinatorTest.java
--- a/jdk/src/java.base/share/classes/java/lang/invoke/MethodHandles.java	Tue Apr 19 12:20:26 2016 +0100
+++ b/jdk/src/java.base/share/classes/java/lang/invoke/MethodHandles.java	Tue Apr 19 14:39:35 2016 +0200
@@ -28,6 +28,7 @@
 import java.lang.reflect.*;
 import java.util.ArrayList;
 import java.util.BitSet;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Arrays;
 import java.util.Objects;
@@ -53,6 +54,7 @@
 import jdk.internal.org.objectweb.asm.Opcodes;
 
 import static java.lang.invoke.MethodHandleStatics.newIllegalArgumentException;
+import static java.lang.invoke.MethodType.methodType;
 
 /**
  * This class consists exclusively of static methods that operate on or return
@@ -3000,7 +3002,7 @@
 
     private static final MethodHandle[] IDENTITY_MHS = new MethodHandle[Wrapper.values().length];
     private static MethodHandle makeIdentity(Class<?> ptype) {
-        MethodType mtype = MethodType.methodType(ptype, ptype);
+        MethodType mtype = methodType(ptype, ptype);
         LambdaForm lform = LambdaForm.identityForm(BasicType.basicType(ptype));
         return MethodHandleImpl.makeIntrinsic(mtype, lform, Intrinsic.IDENTITY);
     }
@@ -3018,7 +3020,7 @@
     }
     private static final MethodHandle[] ZERO_MHS = new MethodHandle[Wrapper.values().length];
     private static MethodHandle makeZero(Class<?> rtype) {
-        MethodType mtype = MethodType.methodType(rtype);
+        MethodType mtype = methodType(rtype);
         LambdaForm lform = LambdaForm.zeroForm(BasicType.basicType(rtype));
         return MethodHandleImpl.makeIntrinsic(mtype, lform, Intrinsic.ZERO);
     }
@@ -3929,7 +3931,7 @@
     MethodHandle throwException(Class<?> returnType, Class<? extends Throwable> exType) {
         if (!Throwable.class.isAssignableFrom(exType))
             throw new ClassCastException(exType.getName());
-        return MethodHandleImpl.throwException(MethodType.methodType(returnType, exType));
+        return MethodHandleImpl.throwException(methodType(returnType, exType));
     }
 
     /**
@@ -4166,7 +4168,7 @@
         for (int i = 0; i < nclauses; ++i) {
             Class<?> t = iterationVariableTypes.get(i);
             if (init.get(i) == null) {
-                init.set(i, empty(MethodType.methodType(t, commonSuffix)));
+                init.set(i, empty(methodType(t, commonSuffix)));
             }
             if (step.get(i) == null) {
                 step.set(i, dropArgumentsToMatch(identityOrVoid(t), 0, commonParameterSequence, i));
@@ -4175,7 +4177,7 @@
                 pred.set(i, dropArguments(constant(boolean.class, true), 0, commonParameterSequence));
             }
             if (fini.get(i) == null) {
-                fini.set(i, empty(MethodType.methodType(t, commonParameterSequence)));
+                fini.set(i, empty(methodType(t, commonParameterSequence)));
             }
         }
 
@@ -4269,7 +4271,8 @@
      * @since 9
      */
     public static MethodHandle whileLoop(MethodHandle init, MethodHandle pred, MethodHandle body) {
-        MethodHandle fin = init == null ? zero(void.class) : identity(init.type().returnType());
+        MethodHandle fin = init == null || init.type().returnType() == void.class ? zero(void.class) :
+                identity(init.type().returnType());
         MethodHandle[] checkExit = {null, null, pred, fin};
         MethodHandle[] varBody = {init, body};
         return loop(checkExit, varBody);
@@ -4335,7 +4338,8 @@
      * @since 9
      */
     public static MethodHandle doWhileLoop(MethodHandle init, MethodHandle body, MethodHandle pred) {
-        MethodHandle fin = init == null ? zero(void.class) : identity(init.type().returnType());
+        MethodHandle fin = init == null || init.type().returnType() == void.class ? zero(void.class) :
+                identity(init.type().returnType());
         MethodHandle[] clause = {init, body, pred, fin};
         return loop(clause);
     }
@@ -4472,8 +4476,8 @@
      * @since 9
      */
     public static MethodHandle countedLoop(MethodHandle start, MethodHandle end, MethodHandle init, MethodHandle body) {
-        MethodHandle returnVar = dropArguments(init == null ? zero(void.class) : identity(init.type().returnType()),
-                0, int.class, int.class);
+        MethodHandle returnVar = dropArguments(init == null || init.type().returnType() == void.class ?
+                zero(void.class) : identity(init.type().returnType()), 0, int.class, int.class);
         MethodHandle[] indexVar = {start, MethodHandleImpl.getConstantHandle(MethodHandleImpl.MH_countedLoopStep)};
         MethodHandle[] loopLimit = {end, null, MethodHandleImpl.getConstantHandle(MethodHandleImpl.MH_countedLoopPred), returnVar};
         MethodHandle[] bodyClause = {init,
@@ -4485,6 +4489,7 @@
     /**
      * Constructs a loop that ranges over the elements produced by an {@code Iterator<T>}.
      * The iterator will be produced by the evaluation of the {@code iterator} handle.
+     * This handle must have {@link java.util.Iterator} as its return type.
      * If this handle is passed as {@code null} the method {@link Iterable#iterator} will be used instead,
      * and will be applied to a leading argument of the loop handle.
      * Each value produced by the iterator is passed to the {@code body}, which must accept an initial {@code T} parameter.
@@ -4534,7 +4539,7 @@
      * assertEquals(reversedList, (List<String>) loop.invoke(list));
      * }</pre></blockquote>
      * <p>
-     * @implSpec The implementation of this method is equivalent to:
+     * @implSpec The implementation of this method is equivalent to (excluding error handling):
      * <blockquote><pre>{@code
      * MethodHandle iteratedLoop(MethodHandle iterator, MethodHandle init, MethodHandle body) {
      *     // assume MH_next and MH_hasNext are handles to methods of Iterator
@@ -4550,6 +4555,7 @@
      * }</pre></blockquote>
      *
      * @param iterator a handle to return the iterator to start the loop.
+     *             The handle must have {@link java.util.Iterator} as its return type.
      *             Passing {@code null} will make the loop call {@link Iterable#iterator()} on the first
      *             incoming value.
      * @param init initializer for additional loop state. This determines the loop's result type.
@@ -4565,21 +4571,23 @@
      * @since 9
      */
     public static MethodHandle iteratedLoop(MethodHandle iterator, MethodHandle init, MethodHandle body) {
-        checkIteratedLoop(body);
+        checkIteratedLoop(iterator, body);
+        final boolean voidInit = init == null || init.type().returnType() == void.class;
 
         MethodHandle initit = MethodHandleImpl.getConstantHandle(MethodHandleImpl.MH_initIterator);
         MethodHandle initIterator = iterator == null ?
-                initit.asType(initit.type().changeParameterType(0, body.type().parameterType(init == null ? 1 : 2))) :
+                initit.asType(initit.type().changeParameterType(0, body.type().parameterType(voidInit ? 1 : 2))) :
                 iterator;
         Class<?> itype = initIterator.type().returnType();
         Class<?> ttype = body.type().parameterType(0);
 
         MethodHandle returnVar =
-                dropArguments(init == null ? zero(void.class) : identity(init.type().returnType()), 0, itype);
+                dropArguments(voidInit ? zero(void.class) : identity(init.type().returnType()), 0, itype);
         MethodHandle initnx = MethodHandleImpl.getConstantHandle(MethodHandleImpl.MH_iterateNext);
         MethodHandle nextVal = initnx.asType(initnx.type().changeReturnType(ttype));
 
-        MethodHandle[] iterVar = {initIterator, null, MethodHandleImpl.getConstantHandle(MethodHandleImpl.MH_iteratePred), returnVar};
+        MethodHandle[] iterVar = {initIterator, null, MethodHandleImpl.getConstantHandle(MethodHandleImpl.MH_iteratePred),
+                returnVar};
         MethodHandle[] bodyClause = {init, filterArgument(body, 0, nextVal)};
 
         return loop(iterVar, bodyClause);
@@ -4833,7 +4841,10 @@
         }
     }
 
-    private static void checkIteratedLoop(MethodHandle body) {
+    private static void checkIteratedLoop(MethodHandle iterator, MethodHandle body) {
+        if (null != iterator && !Iterator.class.isAssignableFrom(iterator.type().returnType())) {
+            throw newIllegalArgumentException("iteratedLoop first argument must have Iterator return type");
+        }
         if (null == body) {
             throw newIllegalArgumentException("iterated loop body must not be null");
         }
--- a/jdk/test/java/lang/invoke/LoopCombinatorTest.java	Tue Apr 19 12:20:26 2016 +0100
+++ b/jdk/test/java/lang/invoke/LoopCombinatorTest.java	Tue Apr 19 14:39:35 2016 +0200
@@ -26,6 +26,7 @@
 /* @test
  * @bug 8139885
  * @bug 8150635
+ * @bug 8150956
  * @bug 8150957
  * @bug 8153637
  * @run testng/othervm -ea -esa test.java.lang.invoke.LoopCombinatorTest
@@ -266,6 +267,28 @@
     }
 
     @Test
+    public static void testWhileVoidInit() throws Throwable {
+        While w = new While();
+        int v = 5;
+        MethodHandle loop = MethodHandles.whileLoop(While.MH_voidInit.bindTo(w), While.MH_voidPred.bindTo(w),
+                While.MH_voidBody.bindTo(w));
+        assertEquals(While.MT_void, loop.type());
+        loop.invoke(v);
+        assertEquals(v, w.i);
+    }
+
+    @Test
+    public static void testDoWhileVoidInit() throws Throwable {
+        While w = new While();
+        int v = 5;
+        MethodHandle loop = MethodHandles.doWhileLoop(While.MH_voidInit.bindTo(w), While.MH_voidBody.bindTo(w),
+                While.MH_voidPred.bindTo(w));
+        assertEquals(While.MT_void, loop.type());
+        loop.invoke(v);
+        assertEquals(v, w.i);
+    }
+
+    @Test
     public static void testCountedLoop() throws Throwable {
         // String s = "Lambdaman!"; for (int i = 0; i < 13; ++i) { s = "na " + s; } return s; => a variation on a well known theme
         MethodHandle fit13 = MethodHandles.constant(int.class, 13);
@@ -275,6 +298,14 @@
     }
 
     @Test
+    public static void testCountedLoopVoidInit() throws Throwable {
+        MethodHandle fit5 = MethodHandles.constant(int.class, 5);
+        MethodHandle loop = MethodHandles.countedLoop(fit5, MethodHandles.zero(void.class), Counted.MH_printHello);
+        assertEquals(Counted.MT_countedPrinting, loop.type());
+        loop.invoke();
+    }
+
+    @Test
     public static void testCountedArrayLoop() throws Throwable {
         // int[] a = new int[]{0}; for (int i = 0; i < 13; ++i) { ++a[0]; } => a[0] == 13
         MethodHandle fit13 = MethodHandles.dropArguments(MethodHandles.constant(int.class, 13), 0, int[].class);
@@ -360,7 +391,8 @@
     public static void testIterateNullBody() {
         boolean caught = false;
         try {
-            MethodHandles.iteratedLoop(MethodHandles.identity(int.class), MethodHandles.identity(int.class), null);
+            MethodHandles.iteratedLoop(MethodHandles.empty(methodType(Iterator.class, int.class)),
+                    MethodHandles.identity(int.class), null);
         } catch (IllegalArgumentException iae) {
             assertEquals("iterated loop body must not be null", iae.getMessage());
             caught = true;
@@ -368,6 +400,26 @@
         assertTrue(caught);
     }
 
+    @Test
+    public static void testIterateVoidIterator() {
+        boolean caught = false;
+        MethodType v = methodType(void.class);
+        try {
+            MethodHandles.iteratedLoop(MethodHandles.empty(v), null, MethodHandles.empty(v));
+        } catch(IllegalArgumentException iae) {
+            assertEquals("iteratedLoop first argument must have Iterator return type", iae.getMessage());
+            caught = true;
+        }
+        assertTrue(caught);
+    }
+
+    @Test
+    public static void testIterateVoidInit() throws Throwable {
+        MethodHandle loop = MethodHandles.iteratedLoop(null, Iterate.MH_voidInit, Iterate.MH_printStep);
+        assertEquals(Iterate.MT_print, loop.type());
+        loop.invoke(Arrays.asList("hello", "world"));
+    }
+
     static class Empty {
 
         static void f() { }
@@ -604,6 +656,10 @@
 
         private int i = 0;
 
+        void voidInit(int k) {
+            // empty
+        }
+
         void voidBody(int k) {
             ++i;
         }
@@ -623,6 +679,7 @@
         static final MethodType MT_zipInitZip = methodType(List.class, Iterator.class, Iterator.class);
         static final MethodType MT_zipPred = methodType(boolean.class, List.class, Iterator.class, Iterator.class);
         static final MethodType MT_zipStep = methodType(List.class, List.class, Iterator.class, Iterator.class);
+        static final MethodType MT_voidInit = methodType(void.class, int.class);
         static final MethodType MT_voidBody = methodType(void.class, int.class);
         static final MethodType MT_voidPred = methodType(boolean.class, int.class);
 
@@ -635,6 +692,7 @@
         static final MethodHandle MH_zipInitZip;
         static final MethodHandle MH_zipPred;
         static final MethodHandle MH_zipStep;
+        static final MethodHandle MH_voidInit;
         static final MethodHandle MH_voidBody;
         static final MethodHandle MH_voidPred;
 
@@ -654,6 +712,7 @@
                 MH_zipInitZip = LOOKUP.findStatic(WHILE, "zipInitZip", MT_zipInitZip);
                 MH_zipPred = LOOKUP.findStatic(WHILE, "zipPred", MT_zipPred);
                 MH_zipStep = LOOKUP.findStatic(WHILE, "zipStep", MT_zipStep);
+                MH_voidInit = LOOKUP.findVirtual(WHILE, "voidInit", MT_voidInit);
                 MH_voidBody = LOOKUP.findVirtual(WHILE, "voidBody", MT_voidBody);
                 MH_voidPred = LOOKUP.findVirtual(WHILE, "voidPred", MT_voidPred);
             } catch (Exception e) {
@@ -768,6 +827,10 @@
             System.out.print(s);
         }
 
+        static void voidInit() {
+            // empty
+        }
+
         static final Class<Iterate> ITERATE = Iterate.class;
 
         static final MethodType MT_sumIterator = methodType(Iterator.class, Integer[].class);
@@ -783,6 +846,8 @@
         static final MethodType MT_mapStep = methodType(List.class, String.class, List.class, List.class);
         static final MethodType MT_printStep = methodType(void.class, String.class, List.class);
 
+        static final MethodType MT_voidInit = methodType(void.class);
+
         static final MethodHandle MH_sumIterator;
         static final MethodHandle MH_sumInit;
         static final MethodHandle MH_sumStep;
@@ -797,6 +862,8 @@
         static final MethodHandle MH_mapInit;
         static final MethodHandle MH_mapStep;
 
+        static final MethodHandle MH_voidInit;
+
         static final MethodType MT_sum = methodType(int.class, Integer[].class);
         static final MethodType MT_reverse = methodType(List.class, List.class);
         static final MethodType MT_length = methodType(int.class, List.class);
@@ -815,6 +882,7 @@
                 MH_mapInit = LOOKUP.findStatic(ITERATE, "mapInit", MT_mapInit);
                 MH_mapStep = LOOKUP.findStatic(ITERATE, "mapStep", MT_mapStep);
                 MH_printStep = LOOKUP.findStatic(ITERATE, "printStep", MT_printStep);
+                MH_voidInit = LOOKUP.findStatic(ITERATE, "voidInit", MT_voidInit);
             } catch (Exception e) {
                 throw new ExceptionInInitializerError(e);
             }