RFR(L): 8151179: address issues raised by JCK team on JEP 274 API

John Rose john.r.rose at oracle.com
Wed Sep 28 06:26:16 UTC 2016


On Sep 26, 2016, at 12:01 PM, Michael Haupt <michael.haupt at oracle.com> wrote:
> 
> thank you very much for your reviews - may I ask for a second round?
> 
> The updated webrev is at http://cr.openjdk.java.net/~mhaupt/8151179/webrev.01/ <http://cr.openjdk.java.net/~mhaupt/8151179/webrev.01/><http://cr.openjdk.java.net/~mhaupt/8151179/webrev.01/ <http://cr.openjdk.java.net/~mhaupt/8151179/webrev.01/>>;
> an accompanying specdiff, at http://cr.openjdk.java.net/~mhaupt/8151179/specdiff.01/ <http://cr.openjdk.java.net/~mhaupt/8151179/specdiff.01/><http://cr.openjdk.java.net/~mhaupt/8151179/specdiff.01/ <http://cr.openjdk.java.net/~mhaupt/8151179/specdiff.01/>>.

Reviewed.  This is a huge leap forward.

A few small comments to consider:

In FacLoop, the argument k should be named acc:
+-  int fin(int i, int k) { return k; }
++  int fin(int i, int acc) { return acc; }

This affects *two* files, a JDE.java and MHs.java.

Fix awkwardness:  s/A similar same example/A similar example

I'm really glad to see the FacLoop example, BTW; I think it is often the right pattern to use.

There is a redundant phrase in the javadoc which reads oddly:

+ A non-void value returned from the body (which must also be of type V) 

But the previous line just defined V as the return value of the body:
+ If the body handle returns a non-void type V, a leading loop iteration variable of that type is also present. 

Seems like you can just omit "(which must also be of type V)" or shorten it to "(of type V)" or perhaps "(which is the leading iteration variable)".  In any case, the role of "V" is fully described in the bullet item list that follows.

The whole section beginning "Example. As a consequence of step 1A above" is kind of fluffy.  It doesn't add much, although it is technically correct.  You could take it out if you feel the same.  (The fact that the "pred" guys are assumed to take no arguments means that it's a pretty useless loop.)  I don't mind leaving it in, though, in the interests of converging this review process!

Thank you for putting in the extra working examples.  That will really help users pick the right patterns.

— John


More information about the core-libs-dev mailing list