On Nov 13, 2015, at 8:39 AM, 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/ 
> <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

Reply via email to