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.
MethodType
—
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).
MethodHandles
—
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.
MethodHandleImpl
—
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?
Paul.
> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 841 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://mail.openjdk.java.net/pipermail/mlvm-dev/attachments/20151119/7c07f6c7/signature.asc>
More information about the mlvm-dev
mailing list