On Nov 13, 2015, at 8:39 AM, Michael Haupt <[email protected]> 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/
> <http://cr.openjdk.java.net/~mhaupt/8139885/webrev.00/>
Looks great. One bug:
+ static boolean isVoid(Class<?> t) {
+ return t == void.class || t == Void.class;
+ }
I think there's nothing in the spec that calls for java.lang.Void to get
special processing for try/finally.
If I'm right, get rid of isVoid; just check for void.class not Void.class.
I'd prefer to see reuse of misMatchedTypes() or newIllegalArgumentException()
instead of err(). I would prefer to push formatting should into the
error-producing subroutine. This can be cleaned up later. OTOH, the
complexity of string concat instructions will be going down with Aleksey's
work, so maybe I shouldn't be so sensitive to inline concat stuff.
My usual practice, though, is to move argument checking and error reporting
code out of line, away from the main logic. I think this is good style, and it
gives the JIT a little bit (a very little bit) of help.
In that vein, the argument checking function foldArgumentChecks should stay
private, shouldn't it?
I'm impressed to see how useful the Stream API is for doing the loop combinator.
Testing is good. I'm glad to see new BigArity cases.
One wishful item: It would be nice if the javadoc examples could be integrated
into JavaDocExamplesTest.java. I see that is messy since we are now using
TestNG instead of JUnit. The point of JavaDocExamplesTest was to make it easy
to "sync" the javadoc with the examples, by having one place to copy-and-paste
the javadoc.
Anyway, reviewed, conditional on that one bug fix.
— John
_______________________________________________
mlvm-dev mailing list
[email protected]
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev