RFR(M): 8150635: j.l.i.MethodHandles.loop(...) throws IndexOutOfBoundsException

[Paul Sandoz]

> On 26 Feb 2016, at 14:01, Michael Haupt <michael.haupt at oracle.com> wrote:
> 
> Dear all,
> 
> please review this fix.
> Bug: https://bugs.openjdk.java.net/browse/JDK-8150635
> Webrev: http://cr.openjdk.java.net/~mhaupt/8150635/webrev.00
> 

MethodHandles.java
—

4087     private static List<Class<?>> buildCommonSuffix(List<MethodHandle> init, List<MethodHandle> step, List<MethodHandle> pred, List<MethodHandle> fini, int cpSize) {
4088         final List<Class<?>> empty = new ArrayList<>();
4089         final List<MethodHandle> nonNullInits = init.stream().filter(Objects::nonNull).collect(Collectors.toList());
4090         if (nonNullInits.isEmpty()) {
4091             final List<Class<?>> longest = Stream.of(step, pred, fini).flatMap(List::stream).filter(Objects::nonNull).
4092                     // take only those that can contribute to a common suffix because they are longer than the prefix
4093                     map(MethodHandle::type).filter(t -> t.parameterCount() > cpSize).map(MethodType::parameterList).
4094                     reduce((p, q) -> p.size() >= q.size() ? p : q).orElse(empty);
4095             return longest.size() == 0 ? empty : longest.subList(cpSize, longest.size());
4096         } else {
4097             return nonNullInits.stream().map(MethodHandle::type).map(MethodType::parameterList).
4098                     reduce((p, q) -> p.size() >= q.size() ? p : q).orElse(empty);
4099         }
4100     }

You can use List.of() (or before that method was added Collections.emptyList()) for the empty list as the returned lists will be unmodifiable (that returned from MethodType.parameterList is).

We also know in the second case that the stream can never be empty so arguably a get is sufficient, but that is just a trivial comment (really we want to assert, and the second best thing there would be a orElseThrow, but it looks odd in this context perhaps).


> * @apiNote Example:
> * <blockquote><pre>{@code
> * // iterative implementation of the factorial function as a loop handle
> * static int one(int k) { return 1; }
> * int inc(int i, int acc, int k) { return i + 1; }
> * int mult(int i, int acc, int k) { return i * acc; }
> * boolean pred(int i, int acc, int k) { return i < k; }
> * int fin(int i, int acc, int k) { return acc; }
> * // assume MH_one, MH_inc, MH_mult, MH_pred, and MH_fin are handles to the above methods
> * // null initializer for counter, should initialize to 0
> * MethodHandle[] counterClause = new MethodHandle[]{null, MH_inc};
> * MethodHandle[] accumulatorClause = new MethodHandle[]{MH_one, MH_mult, MH_pred, MH_fin};
> * MethodHandle loop = MethodHandles.loop(counterClause, accumulatorClause);
> * assertEquals(120, loop.invoke(5));
> * }</pre></blockquote>



Do you need to update this example since it is referring to virtual methods? since you need to permute the arguments.


T8139885.java
—

Could we rename that test to say “LoopCombinatorTest” ?

Paul.



> There were two issues to address that led to IOOBE being thrown.
> 
> 1. Lack of support for deriving the loop signature from given parameter type lists in case no init function is given at all.
> 
> 2. The loop combinator reporting a wrong exception in case a parameter type list is longer than the common parameter sequence. In this case, an error about non-effectively identical parameter type lists should be signalled.
> 
> See the JIRA issue for details and examples.
> 
> Thanks,
> 
> Michael
> 
> --
> 
> <http://www.oracle.com/>
> Dr. Michael Haupt | Principal Member of Technical Staff
> Phone: +49 331 200 7277 | Fax: +49 331 200 7561
> Oracle Java Platform Group | LangTools Team | Nashorn
> Oracle Deutschland B.V. & Co. KG | Schiffbauergasse 14 | 14467 Potsdam, Germany
> 
> ORACLE Deutschland B.V. & Co. KG | Hauptverwaltung: Riesstraße 25, D-80992 München
> Registergericht: Amtsgericht München, HRA 95603
> 
> Komplementärin: ORACLE Deutschland Verwaltung B.V. | Hertogswetering 163/167, 3543 AS Utrecht, Niederlande
> Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697
> Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher
> <http://www.oracle.com/commitment>	Oracle is committed to developing practices and products that help protect the environment
>