JDK-8151903 : MethodHandles.iteratedLoop(MH, MH, MH) doc issues
  • Type: Sub-task
  • Component: core-libs
  • Sub-Component: java.lang.invoke
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2016-03-15
  • Updated: 2016-06-20
  • Resolved: 2016-06-20
Related Reports
Blocks :  
Relates :  
Description
1. There is no information about different argument constratins (iteratior, init, body). Actually, some information could be deduced using pseudocode, but it looks like it would be better to describe it in words. That's because 'init' and 'iterator' accepts 'A' params (by pseudocode), but I assume they could have different, but 'effectively equivalent' params (removing parameter from example's newArrayList changes nothing). Anyway, list of questions not covered in 'words' could be found below: 

Constraints for 'init' argument list: 
-- Should argument list be the same for 'init' and 'iterator' (or be 'effectively equals')?
-- Which parameter list we should expect as a result of loop creation? Should it be the  same as 'init' parameters list or not? What kind of parameter list should we expect in cases:
---- 'init'=null
---- 'iterator'=null
---- 'init'=null && 'iterator'=null?  
It looks like JDK-8150964. 
-- 'body' return type could be deduced using pseudocode, but it would be good to specify it in doc as long as other functions types.

2. 'Implementation requirements' code will fail with NPE for iterator=null and body=null. This is not equivalent to MethodHandles.iteratedLoop(...) call.

3. body - the body of the loop, which must not be null. It must accept an initial T parameter (for the iterated values), and then any additional loop-local variable plus loop parameters.

There is no 'definition' of 'local loop variables', although it could be deduced from 
>>  The result of the loop execution is the final value of the additional local state obtained by running init.


I suppose it would be good to mention what is 'loop-local variables' in clearer way.
Comments
With the API redesign done for the resolution of JDK-8151179, this issue is fixed internally, and the push for JDK-8151179 will contain the pertaining changes.
20-06-2016

Thanks, [~asolodkaya]! Please comment on the updated documentation below. /** * 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. If non-{@code null}, this handle must have {@link Iterator} or * a subclass of {@link Iterator} as its return type. If this handle is {@code null}, the resulting loop will obtain * its iterator by applying {@link Iterable#iterator} to the leading argument of the loop handle invocation. * <p> * The loop can have additional local state, which is initialized by evaluating the {@code init} handle. In case * {@code init} is {@code null}, the local state can be inferred from the {@code body} signature: if its second * parameter type matches its return type, that will be used as the loop's local state and initialized to the * appropriate "zero" value. The local state's type determines the resulting loop handle's result type. * <p> * From the point of view of the {@linkplain #loop(MethodHandle[]...) generic loop combinator}, both {@code init} * and {@code iterator} are treated as clause initializers. Consequently, they determine the resulting loop handle's * parameters. If only {@code init} is present, it determines the parameters. If both are present, their parameter * type lists must be effectively identical, and the longer parameter list will be the resulting handle's parameter * list. If {@code iterator} is absent and {@code init} has no parameters, or both are absent, the resulting loop * handle's parameter types will be the {@code body} handle's <em>second (if the loop does not have additional local * state) or third (otherwise) up to and including its last</em> parameter types (see below for details on the * {@code body} handle's signature). * <p> * Each value produced by the iterator is passed to the {@code body}, which must accept an initial {@code T} * parameter. If the loop has additional local state ({@code init} is not {@code null}, or returns {@code void}), * that state is passed as the second argument to each invocation of the {@code body} handle (which must then have * an according parameter of the same type as {@code init}'s result type), and the {@code body} handle execution's * result is used to update the local state. The remaining parameters of the {@code body} handle must match those of * the {@code init} handle, if that is present. If {@code init} is given and the {@code body} signature has the * leading {@code T} parameter, the remaining parameters (loop-local state and loop argument) can be omitted. * <p> * The result of the loop execution is the final value of the additional local state. * <p> * This is a convenience wrapper for the {@linkplain MethodHandles#loop(MethodHandle[][]) generic loop combinator}, * and the constraints imposed on the {@code body} handle follow directly from those described for the latter. * <p> * Here is pseudocode for the resulting loop handle. The code covers the essentials and does not deal with corner * cases. In the code, {@code V}/{@code v} represent the type / value of the loop variable as well as the result * type of the loop; {@code T}/{@code t}, that of the elements of the structure the loop iterates over, and * {@code A}/{@code a}, that of the argument passed to the loop. * <blockquote><pre>{@code * Iterator<T> iterator(A); // defaults to Iterable::iterator * V init(A); * V body(T,V,A); * V iteratedLoop(A a) { * Iterator<T> it = iterator(a); * V v = init(a); * for (T t : it) { * v = body(t, v, a); * } * return v; * } * }</pre></blockquote> * <p> * The type {@code T} may be either a primitive or reference. Since type {@code Iterator<T>} is erased in the method * handle representation to the raw type {@code Iterator}, the {@code iteratedLoop} combinator adjusts the leading * argument type for {@code body} to {@code Object} as if by the {@link MethodHandle#asType asType} conversion * method. Therefore, if an iterator of the wrong type appears as the loop is executed, runtime exceptions may occur * as the result of dynamic conversions performed by {@code asType}. * <p> * @apiNote Example: * <blockquote><pre>{@code * // reverse a list * static List<String> reverseStep(String e, List<String> r, List<String> l) { * r.add(0, e); * return r; * } * static List<String> newArrayList(List<String> l) { return new ArrayList<>(); } * // assume MH_reverseStep, MH_newArrayList are handles to the above methods * MethodHandle loop = MethodHandles.iteratedLoop(null, MH_newArrayList, MH_reverseStep); * List<String> list = Arrays.asList("a", "b", "c", "d", "e"); * List<String> reversedList = Arrays.asList("e", "d", "c", "b", "a"); * assertEquals(reversedList, (List<String>) loop.invoke(list)); * }</pre></blockquote> * <p> * @implSpec The implementation of this method is equivalent to (excluding error handling): * <blockquote><pre>{@code * MethodHandle iteratedLoop(MethodHandle iterator, MethodHandle init, MethodHandle body) { * boolean voidResult = ...; // true iff the loop result type will be void * // assume MH_next and MH_hasNext are handles to the corresponding methods of Iterator * // assume MH_initit is a handle to Iterable.iterator() * MethodHandle initIterator = iterator == null ? * MH_initit.asType(MH_initit.type().changeParameterType(0, body.type().parameterType(voidResult ? 1 : 2))) : * iterator.asType(iterator.type().changeReturnType(Iterator.class)); * Class<?> ttype = body.type().parameterType(0); * MethodHandle returnVar = * dropArguments(voidResult ? zero(void.class) : identity(init.type().returnType()), 0, Iterator.class); * MethodHandle nextVal = MH_next.asType(MH_next.type().changeReturnType(ttype)); * MethodHandle[] iterVar = {initIterator, null, MH_hasNext, returnVar}; // it = iterator; while (it.hasNext()) * MethodHandle[] bodyClause = {init, filterArgument(body, 0, nextVal)}; // v = body(t, v, a) * return loop(iterVar, bodyClause); * } * }</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. * Passing {@code null} or a {@code void} init function will make the loop's result type * {@code void}. * @param body the body of the loop, which must not be {@code null}. * It must accept an initial {@code T} parameter (for the iterated values), and then any * additional loop-local variable plus loop parameters. * * @return a method handle embodying the iteration loop functionality. * @throws IllegalArgumentException if any argument violates the above requirements. * * @since 9 */
19-04-2016

It seems it would be good to mention the fact that given pseudocode is 'essence' to prevent misunderstanding. I mean that there is no init=null case, etc. It's impossible to include all cases in the single listing, so it could be useful to mention this fact. What do you think about it?
25-03-2016

Please, specify restrictions on 'iterator' types too. There is no restrictions, so it is expected that, e.g. iterator=(I)Iterator init=(List)I body=(String, I, List) is permitted. However, it violates MethodHandles.loop(...) restrictions and leads to IAE. Please, observe IteratedLoop8.java for details of this case. Some additional cases not described in spec: 1. could iterator be void? Now it leads to IAE (see JDK-8150956) - see IteratedLoop12 2. is there any restrictions on iterator return type? It looks like it should be Iterator (not subclass). Please, see IteratedLoop13 - iterator returns int IteratedLoop15, IteratedLoop16 - iterator returns subclass of Iterator
24-03-2016

>> If the loop has additional local state (init is not null), that state is passed as the second argument to each invocation of the body handle (which must then have an according parameter of the same type as init's result type), and the body handle execution's result is used to update the local state. Implementation permits body without loop-arguments and without local-loop arguments (no exception). Please, observe examples: IteratedLoop9.java iterator=()Iterator init=(List)I body=(String, I)I <- no loop argument (List) IteratedLoop10.java iterator=()Iterator init=(List)I body=(String)I <- no loop variable (I), no loop argument (List) >> If the loop has additional local state (init is not null) It looks like void init produces no local state to. Am I correct?
24-03-2016

Thanks, Michael! Now it's really clear. Following comments (in this issue) describe 'current' implementation state. I suppose, some of them could be potential doc enhancement and some of them could be filed as bugs on implementation. >> The loop can have additional local state, which is initialized by evaluating the init handle. This handle's result type determines the resulting loop handle's result type; and its parameter types, the resulting loop handle's parameter types. It seems that, according to current implementation, 'init' presence doesn't guarantee that resulting MethodHandle will have parameter list determined by init. Please, observe example IteratedLoop1.java. Following cases produce method handle having type (List)I instead of ()I (actual type was determined from body) : 1. iterator=null; init=()I; body=(String, I, List)I 2. iterator=(List)Iterator; init=()I; body=(String, I, List)I Please, observe example IteratedLoop7.java. Following cases produce method handle having type (List,I)I instead of (List)I (actual type was determined from iterator): iterator=(List, I)Iterator; init=(List)I; body=(String, I, List)I
24-03-2016

>> If the init handle is null, the resulting loop handle's result type will be void. In that case, the resulting loop handle's parameter types will be the body handle's second to last parameter types Please, observe IteratedLoop6.java It seems that in the case of ommited init 'iterator' parameters list of resulting handle.could be inherited from 'iterator' instead of 'body. E.g. init=null iterator=(List, I) body=(String, List) Expected result - (List)V, since there is no restrictions on 'iterator' arguments. Actual result (List, I)V - parameter list the same as 'iterator' parameter list.
24-03-2016

It would be good to clarify 'additional loop state' in the case of init=null. It seems, it wouldn't be additional loop state in this case. However, there is no restrictions on 'body' return type in this case. Please, observe IteratedLoop2.java: 1. In the first case I use body=(String, I, List)I. It is not prohibited to use I return type to body. I expect to get resulting handle (I, List)V. That's because init=null => parameters 2 and 3 should be resulting handle parameters. ACTUAL result is (List)V. It looks like implementation treats second parameter as if it is 'local state parameter' 2. In the second case I use body=(String, List)I. It is not prohibited too, because, according to spec, there is no 'local state' values (init=null). However it leads to IAE: Exception in thread "main" java.lang.IllegalArgumentException: found non-effectively identical parameter type lists: step: [null, MethodHandle(Iterator,List)int] pred: [MethodHandle(Iterator)boolean, null] fini: [MethodHandle(Iterator)void, null] (common parameter sequence: [interface java.util.Iterator, int, interface java.util.List]) at java.lang.invoke.MethodHandleStatics.newIllegalArgumentException(MethodHandleStatics.java:112) at java.lang.invoke.MethodHandles.checkLoop2(MethodHandles.java:4131) at java.lang.invoke.MethodHandles.loop(MethodHandles.java:3453) at java.lang.invoke.MethodHandles.iteratedLoop(MethodHandles.java:3871) at IteratedLoop2.main(IteratedLoop2.java:14) What do you think about it?
24-03-2016

[~asolodkaya], please comment on this proposal for addressing the documentation issues: /** * Constructs a loop that ranges over the elements produced by an {@code Iterator<T>}. * <p> * The iterator will be produced by the evaluation of the {@code iterator} handle. If this handle is {@code null}, * the resulting loop will obtain its iterator by applying {@link Iterable#iterator} to the leading argument of the * loop handle invocation. * <p> * The loop can have additional local state, which is initialized by evaluating the {@code init} handle. This * handle's result type determines the resulting loop handle's result type; and its parameter types, the resulting * loop handle's parameter types. If the {@code init} handle is {@code null}, the resulting loop handle's result * type will be {@code void}. In that case, the resulting loop handle's parameter types will be the {@code body} * handle's <em>second to last</em> parameter types (see below for details on the {@code body} handle's signature). * <p> * Each value produced by the iterator is passed to the {@code body}, which must accept an initial {@code T} * parameter. If the loop has additional local state ({@code init} is not {@code null}), that state is passed as the * second argument to each invocation of the {@code body} handle (which must then have an according parameter of the * same type as {@code init}'s result type), and the {@code body} handle execution's result is used to update the * local state. The remaining parameters of the {@code body} handle must match those of the {@code init} handle, if * that is present. * <p> * The result of the loop execution is the final value of the additional local state. * <p> * This is a convenience wrapper for the {@linkplain MethodHandles#loop(MethodHandle[][]) generic loop combinator}, * and the constraints imposed on the {@code body} handle follow directly from those described for the latter. * <p> * Here is pseudocode for the resulting loop handle. In the code, {@code V}/{@code v} represent the type / value of * the loop variable as well as the result type of the loop; {@code T}/{@code t}, that of the elements of the * structure the loop iterates over, and {@code A}/{@code a}, that of the argument passed to the loop. * <blockquote><pre>{@code * Iterator<T> iterator(A); // defaults to Iterable::iterator * V init(A); * V body(T,V,A); * V iteratedLoop(A a) { * Iterator<T> it = iterator(a); * V v = init(a); * for (T t : it) { * v = body(t, v, a); * } * return v; * } * }</pre></blockquote> * <p> * The type {@code T} may be either a primitive or reference. * Since type {@code Iterator<T>} is erased in the method handle representation to the raw type * {@code Iterator}, the {@code iteratedLoop} combinator adjusts the leading argument type for {@code body} * to {@code Object} as if by the {@link MethodHandle#asType asType} conversion method. * Therefore, if an iterator of the wrong type appears as the loop is executed, * runtime exceptions may occur as the result of dynamic conversions performed by {@code asType}. * <p> * @apiNote Example: * <blockquote><pre>{@code * // reverse a list * static List<String> reverseStep(String e, List<String> r, List<String> l) { * r.add(0, e); * return r; * } * static List<String> newArrayList(List<String> l) { return new ArrayList<>(); } * // assume MH_reverseStep, MH_newArrayList are handles to the above methods * MethodHandle loop = MethodHandles.iteratedLoop(null, MH_newArrayList, MH_reverseStep); * List<String> list = Arrays.asList("a", "b", "c", "d", "e"); * List<String> reversedList = Arrays.asList("e", "d", "c", "b", "a"); * assertEquals(reversedList, (List<String>) loop.invoke(list)); * }</pre></blockquote> * <p> * @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 the corresponding methods of Iterator * // assume MH_initit is a handle to Iterable.iterator() * MethodHandle initIterator = iterator == null ? * MH_initit.asType(MH_initit.type().changeParameterType(0, body.type().parameterType(init == null ? 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); * MethodHandle nextVal = MH_next.asType(MH_next.type().changeReturnType(ttype)); * MethodHandle[] iterVar = {initIterator, null, MH_hasNext, returnVar}; * MethodHandle[] bodyClause = {init, filterArgument(body, 0, nextVal)}; * return loop(iterVar, bodyClause); * }</pre></blockquote> * * @param iterator a handle to return the iterator to start the loop. * 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. * Passing {@code null} or a {@code void} init function will make the loop's result type * {@code void}. * @param body the body of the loop, which must not be {@code null}. * It must accept an initial {@code T} parameter (for the iterated values), and then any * additional loop-local variable plus loop parameters. * * @return a method handle embodying the iteration loop functionality. * @throws IllegalArgumentException if any argument violates the above requirements. * * @since 9 */ Note that a small fix in the implementation is required when the zero method is introduced with JDK-8150829; the documentation change for the @implSpec tag already reflects this. I'm holding off on marking this as internally resolved until the patch can be applied against that.
22-03-2016