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.ha...@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@openjdk.java.net
> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
signature.asc
Description: Message signed with OpenPGP using GPGMail
_______________________________________________ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
