8150956: j.l.i.MethodHandles.whileLoop(...) and .iteratedLoop(...) throw unexpected exceptions in the case of 'init' return type is void
Reviewed-by: psandoz
--- 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);
}