[9] RFR (S) 8057654: Extract checks performed during MethodHandle construction into separate methods
http://cr.openjdk.java.net/~vlivanov/8057654/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8057654 The idea is to separate construction logic and different checks performed before/during method handle construction. For example: move checks from MHs.foldArguments into MHs.foldArgumentChecks. Testing: jdk/java/lang/invoke, jdk/java/util/streams, nashorn, octane w/ "-ea -esa" and COMPILE_THRESHOLD={0,30}. Reviewed-by: vlivanov, ? Contributed-by: john.r.rose@oracle.com Thanks! Best regards, Vladimir Ivanov
On Sep 5, 2014, at 10:09 AM, Vladimir Ivanov <vladimir.x.ivanov@oracle.com> wrote:
http://cr.openjdk.java.net/~vlivanov/8057654/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8057654
+1 Paul.
On 09/05/2014 12:31 PM, Paul Sandoz wrote:
On Sep 5, 2014, at 10:09 AM, Vladimir Ivanov <vladimir.x.ivanov@oracle.com> wrote:
http://cr.openjdk.java.net/~vlivanov/8057654/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8057654
+1
Paul.
*@SuppressWarnings("LocalVariableHidesMemberVariable")* AFAIK, this is not a standard suppress warnings name, i would prefer not having this kind of annotation in the code because IMO, it doesn't pull it's own weight. cheers; Rémi
On Sep 5, 2014, at 2:30 PM, Remi Forax <forax@univ-mlv.fr> wrote:
On 09/05/2014 12:31 PM, Paul Sandoz wrote:
On Sep 5, 2014, at 10:09 AM, Vladimir Ivanov <vladimir.x.ivanov@oracle.com> wrote:
http://cr.openjdk.java.net/~vlivanov/8057654/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8057654
+1
Paul.
*@SuppressWarnings("LocalVariableHidesMemberVariable")*
AFAIK, this is not a standard suppress warnings name, i would prefer not having this kind of annotation in the code because IMO, it doesn't pull it's own weight.
I nearly called that out too (sitting on the fence a little in this respect). Might be worth doing as a separate pass. Paul.
Paul, Remi, thanks for review. Renamed type -> mtype & removed @SuppressWarnings. Updated webrev in place. Best regards, Vladimir Ivanov On 9/5/14, 5:13 PM, Paul Sandoz wrote:
On Sep 5, 2014, at 2:30 PM, Remi Forax <forax@univ-mlv.fr> wrote:
On 09/05/2014 12:31 PM, Paul Sandoz wrote:
On Sep 5, 2014, at 10:09 AM, Vladimir Ivanov <vladimir.x.ivanov@oracle.com> wrote:
http://cr.openjdk.java.net/~vlivanov/8057654/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8057654
+1
Paul.
*@SuppressWarnings("LocalVariableHidesMemberVariable")*
AFAIK, this is not a standard suppress warnings name, i would prefer not having this kind of annotation in the code because IMO, it doesn't pull it's own weight.
I nearly called that out too (sitting on the fence a little in this respect). Might be worth doing as a separate pass.
Paul.
On 09/05/2014 12:09 PM, Vladimir Ivanov wrote:
http://cr.openjdk.java.net/~vlivanov/8057654/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8057654
Random style rant of the week, not particularly about this concrete patch. Can we please try to systematically use more readable/robust/secure idioms? E.g.: a) Always have curly braces around the blocks? if (ok && ...) { ok = false; } if (!ok) { throw misMatchedTypes(...); } return rtype; vs. if (ok && ...) ok = false; if (!ok) throw misMatchedTypes(...); return rtype; Apple's "goto fail;" bug, anyone? b) Have only a single initialization per line? boolean match = true; boolean fail = false; vs. boolean match = true, fail = false; c) Always have parentheses in ternary operators predicates? int foldVals = (rtype == void.class) ? 0 : 1; vs. int foldVals = rtype == void.class ? 0 : 1; Thanks, -Aleksey.
+1 On 05 Sep 2014, at 12:46, Aleksey Shipilev <aleksey.shipilev@oracle.com> wrote:
On 09/05/2014 12:09 PM, Vladimir Ivanov wrote:
http://cr.openjdk.java.net/~vlivanov/8057654/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8057654
Random style rant of the week, not particularly about this concrete patch. Can we please try to systematically use more readable/robust/secure idioms? E.g.:
a) Always have curly braces around the blocks?
if (ok && ...) { ok = false; } if (!ok) { throw misMatchedTypes(...); } return rtype;
vs.
if (ok && ...) ok = false; if (!ok) throw misMatchedTypes(...); return rtype;
Apple's "goto fail;" bug, anyone?
b) Have only a single initialization per line?
boolean match = true; boolean fail = false; vs. boolean match = true, fail = false;
c) Always have parentheses in ternary operators predicates?
int foldVals = (rtype == void.class) ? 0 : 1; vs. int foldVals = rtype == void.class ? 0 : 1;
Thanks, -Aleksey.
To the style rant, I mean. On 05 Sep 2014, at 13:40, Marcus Lagergren <marcus.lagergren@oracle.com> wrote:
+1
On 05 Sep 2014, at 12:46, Aleksey Shipilev <aleksey.shipilev@oracle.com> wrote:
On 09/05/2014 12:09 PM, Vladimir Ivanov wrote:
http://cr.openjdk.java.net/~vlivanov/8057654/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8057654
Random style rant of the week, not particularly about this concrete patch. Can we please try to systematically use more readable/robust/secure idioms? E.g.:
a) Always have curly braces around the blocks?
if (ok && ...) { ok = false; } if (!ok) { throw misMatchedTypes(...); } return rtype;
vs.
if (ok && ...) ok = false; if (!ok) throw misMatchedTypes(...); return rtype;
Apple's "goto fail;" bug, anyone?
b) Have only a single initialization per line?
boolean match = true; boolean fail = false; vs. boolean match = true, fail = false;
c) Always have parentheses in ternary operators predicates?
int foldVals = (rtype == void.class) ? 0 : 1; vs. int foldVals = rtype == void.class ? 0 : 1;
Thanks, -Aleksey.
participants (5)
-
Aleksey Shipilev
-
Marcus Lagergren
-
Paul Sandoz
-
Remi Forax
-
Vladimir Ivanov