RFR(L): 8139885: implement JEP 274: enhanced method handles

Paul Sandoz paul.sandoz at oracle.com
Thu Nov 19 10:45:20 UTC 2015

HI Michael,

Nice work, not easy this area!

It may be worth considering, as a separate issue, updating the example section in JavaDoc to be under @apiNote.

I have a mild preference to maintain the 80 char limit for JavaDoc, to me it’s easier to read. For code i don’t mind a 100 or more limit.

Add @since 9 to new public methods.


 470     /** Replace the last arrayLength parameter types with the component type of arrayType.
 471      * @param arrayType any array type
 472      * @param arrayLength the number of parameter types to change
 473      * @return the resulting type
 474      */

 475     /*non-public*/ MethodType asSpreaderType(Class<?> arrayType, int pos, int arrayLength) {

 476         assert(parameterCount() >= arrayLength);

 477         int spreadPos = pos;

Might as well adjust the JavaDoc for the new pos parameter (as you have done for asCollectorType).


 898      * @param spreadArgPos the position (zero-based index) in the argument list at which spreading should start.
 899      * @param arrayType usually {@code Object[]}, the type of the array argument from which to extract the spread arguments
 900      * @param arrayLength the number of arguments to spread from an incoming array argument
 901      * @return a new method handle which spreads an array argument at a given position,
 902      *         before calling the original method handle
 903      * @throws NullPointerException if {@code arrayType} is a null reference
 904      * @throws IllegalArgumentException if {@code arrayType} is not an array type,
 905      *         or if target does not have at least
 906      *         {@code arrayLength} parameter types,
 907      *         or if {@code arrayLength} is negative,
 908      *         or if the resulting method handle's type would have
 909      *         <a href="MethodHandle.html#maxarity">too many parameters</a>

For the new asSpreader method, what are the constraints for spreadArgPos? What happens if < 0 or >= arrayLength for example?

Same applies to asCollector.

This is not in your patch but the assert in the following method is redundant (as reported by the IDE, it’s really handy sometimes apply the patch and view via diffs in the IDE), as DirectMethodHandle is not a subtype of BoundMethodHandle:

MethodHandle viewAsType(MethodType newType, boolean strict) {
    // No actual conversions, just a new view of the same method.
    // Note that this operation must not produce a DirectMethodHandle,
    // because retyped DMHs, like any transformed MHs,
    // cannot be cracked into MethodHandleInfo.
    assert viewAsTypeChecks(newType, strict);
    BoundMethodHandle mh = rebind();
    assert(!((MethodHandle)mh instanceof DirectMethodHandle));
    return mh.copyWith(newType, mh.form);

 958         /**
 959          * Determines if a class can be accessed from the lookup context defined by this {@code Lookup} object. The
 960          * static initializer of the class is not run.
 961          *
 962          * @param targetClass the class to be accessed
 963          *
 964          * @return the class that has been accessed

Has it really been accessed by “accessClass” or just checked that it can be accessed?

 965          *
 966          * @throws IllegalAccessException if the class is not accessible from the lookup class, using the allowed access
 967          * modes.
 968          * @exception SecurityException if a security manager is present and it
 969          *                              <a href="MethodHandles.Lookup.html#secmgr">refuses access</a>
 970          */
 971         public Class<?> accessClass(Class<?> targetClass) throws IllegalAccessException {

Great use of streams in MethodHandles.loop and good correlation with the specified steps in the JavaDoc to that in the code. Really clear and well specified.

3458      * The loop handle's result type is the same as the sole loop variable's, i.e., the result type of {@code init}.

s/variable’s/variable ?

3488      * // assume MH_initZip, MH_zipPred, and MH_zipStep are handles to the above methods
3489      * MethodHandle loop = MethodHandles.doWhileLoop(MH_initZip, MH_zipPred, MH_zipStep);
3490      * List<String> a = Arrays.asList(new String[]{"a", "b", "c", "d"});
3491      * List<String> b = Arrays.asList(new String[]{"e", "f", "g", "h"});
3492      * List<String> zipped = Arrays.asList(new String[]{"a", "e", "b", "f", "c", "g", "d", "h"});
3493      * assertEquals(zipped, (List<String>) loop.invoke(a.iterator(), b.iterator()));
3494      * }</pre></blockquote>

You don’t need to create an explicit array and pass that into Arrays.asList. Same for other related methods.


1720     static MethodHandle makeLoop(Class<?> tloop, List<Class<?>> targs, List<Class<?>> tvars, List<MethodHandle> init,
1721                                  List<MethodHandle> step, List<MethodHandle> pred, List<MethodHandle> fini) {
1722         MethodHandle[] ainit = toArrayArgs(init).toArray(MH_ARRAY_SENTINEL);
1723         MethodHandle[] astep = toArrayArgs(step).toArray(MH_ARRAY_SENTINEL);
1724         MethodHandle[] apred = toArrayArgs(pred).toArray(MH_ARRAY_SENTINEL);
1725         MethodHandle[] afini = toArrayArgs(fini).toArray(MH_ARRAY_SENTINEL);
1751     static List<MethodHandle> toArrayArgs(List<MethodHandle> hs) {
1752         return hs.stream().map(h -> h.asSpreader(Object[].class, h.type().parameterCount())).collect(Collectors.toList());
1753     }

Can you use Stream.toArray(MethodHandle[]::new) rather than creating a List and then an array from that? (or expand to a lambda expression if you want to reuse MH_ARRAY_SENTINEL for an empty array).

I guess in the future there may be ample opporunity to specialize the generic “loop” mechanism with LambdaForms?


> On 13 Nov 2015, at 17:39, Michael Haupt <michael.haupt at oracle.com> wrote:
> Dear all,
> please review this change.
> RFE: https://bugs.openjdk.java.net/browse/JDK-8139885
> Corresponding JEP: https://bugs.openjdk.java.net/browse/JDK-8130227
> Webrev: http://cr.openjdk.java.net/~mhaupt/8139885/webrev.00/
> Thanks,
> Michael
> --
> 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 is committed to developing practices and products that help protect the environment
> _______________________________________________
> mlvm-dev mailing list
> mlvm-dev at openjdk.java.net
> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

More information about the core-libs-dev mailing list