Re: RFR 8135248: Add utility methods to check indexes and ranges

> On 30 Sep 2015, at 16:13, Remi Forax wrote: > > > > - Mail original - >> De: "Paul Benedict" >> À: "Peter Levart" >> Cc: "core-libs-dev" >> Envoyé: Mercredi 30 Septembre 2015 16:06:18 >> Objet: Re: RF

Re: RFR 8135248: Add utility methods to check indexes and ranges

"core-libs-dev" > > Envoyé: Mercredi 30 Septembre 2015 16:06:18 > > Objet: Re: RFR 8135248: Add utility methods to check indexes and ranges > > > > Ah, I was going to write about "values" ... glad this was mentioned. With > > Valhalla working on va

Re: RFR 8135248: Add utility methods to check indexes and ranges

- Mail original - > De: "Paul Benedict" > À: "Peter Levart" > Cc: "core-libs-dev" > Envoyé: Mercredi 30 Septembre 2015 16:06:18 > Objet: Re: RFR 8135248: Add utility methods to check indexes and ranges > > Ah, I was going to wri

Re: RFR 8135248: Add utility methods to check indexes and ranges

Ah, I was going to write about "values" ... glad this was mentioned. With Valhalla working on value classes, it does raise the question if range-checking is particular to Objects. Clearly it won't be once values are introduced. PS: I am still in favor of using Objects at the time being though just

Re: RFR 8135248: Add utility methods to check indexes and ranges

On 09/29/2015 10:01 PM, Paul Sandoz wrote: >I think it's worth introducing Preconditions class. checkNotNull overloads are equally well suited for Objects as they are for Preconditions, so it's not wrong to have them in both, while checkIndex and friends don't really suit any of the existing

Re: RFR 8135248: Add utility methods to check indexes and ranges

On 9/29/2015 2:28 PM, Stephen Colebourne wrote: On 29 September 2015 at 21:01, Paul Sandoz wrote: The concern i have is once Preconditions is let loose the scope expands with proposals for “just one more method” (there is even the opportunity to bike shed over the names checkNotNull or requir

Re: RFR 8135248: Add utility methods to check indexes and ranges

On 29 September 2015 at 21:01, Paul Sandoz wrote: > The concern i have is once Preconditions is let loose the scope expands with > proposals for “just one more method” (there is even the opportunity to bike > shed over the names checkNotNull or requiresNotNull etc. etc.) I don’t want > to disc

Re: RFR 8135248: Add utility methods to check indexes and ranges

> On 29 Sep 2015, at 21:24, Peter Levart wrote: >> Just a few *more* bike sheds to paint :-) I am concerned i will never finish >> this nor other un/related tasks. >> >> For now I am ok with Objects being that "argument checker class” simply >> because it already has a gravitational pull due t

Re: RFR 8135248: Add utility methods to check indexes and ranges

It would be nice to introduce a Preconditions class (although I am not opposed to continue maturing Objects). I was waiting for the right time for this to be mentioned again (as it was mentioned in the past). Checking indices aren't the only thing we could add; another thing would be a method that

Re: RFR 8135248: Add utility methods to check indexes and ranges

On 09/29/2015 06:05 PM, Paul Sandoz wrote: On 29 Sep 2015, at 12:57, Stephen Colebourne wrote: Just to note that an ideal location for this would be on a new class, one that has been discussed before, an "argument checker class". See Guava Preconditions: https://github.com/google/guava/blob/

Re: RFR 8135248: Add utility methods to check indexes and ranges

> On 29 Sep 2015, at 12:57, Stephen Colebourne wrote: > > Just to note that an ideal location for this would be on a new class, > one that has been discussed before, an "argument checker class". > > See Guava Preconditions: > https://github.com/google/guava/blob/master/guava/src/com/google/comm

Re: RFR 8135248: Add utility methods to check indexes and ranges

Just to note that an ideal location for this would be on a new class, one that has been discussed before, an "argument checker class". See Guava Preconditions: https://github.com/google/guava/blob/master/guava/src/com/google/common/base/Preconditions.java See Commons Lang Validate: https://common

Re: RFR 8135248: Add utility methods to check indexes and ranges

> On 29 Sep 2015, at 06:48, John Rose wrote: > > On Sep 28, 2015, at 5:10 PM, Joseph D. Darcy > wrote: >> >> Joining this thread late, I think the range checking methods would have a >> happier life where they are more often found and used if the they live >> som

Re: RFR 8135248: Add utility methods to check indexes and ranges

On Sep 28, 2015, at 5:10 PM, Joseph D. Darcy wrote: > > Joining this thread late, I think the range checking methods would have a > happier life where they are more often found and used if the they live > somewhere other than the exception classes. > > The class java.util.Objects is not an ide

Re: RFR 8135248: Add utility methods to check indexes and ranges

e the LinkedList would depend on the Arrays class to check bounds. Jason From: core-libs-dev on behalf of Paul Sandoz Sent: Monday, September 21, 2015 8:42 AM To: core-libs-dev Subject: RFR 8135248: Add utility methods to check indexes and ranges Hi, P

Re: RFR 8135248: Add utility methods to check indexes and ranges

On 25 Sep 2015, at 09:39, Remi Forax wrote: > Hi Paul, hi chris, > > - Mail original - >> De: "Chris Hegarty" >> À: "Paul Sandoz" >> Cc: "core-libs-dev" >> Envoyé: Jeudi 24 Septembre 2015 11:06:36 >> Objet: Re: RFR

Re: RFR 8135248: Add utility methods to check indexes and ranges

Hi Paul, hi chris, - Mail original - > De: "Chris Hegarty" > À: "Paul Sandoz" > Cc: "core-libs-dev" > Envoyé: Jeudi 24 Septembre 2015 11:06:36 > Objet: Re: RFR 8135248: Add utility methods to check indexes and ranges > > On 24 Sep 2

Re: RFR 8135248: Add utility methods to check indexes and ranges

On 24 Sep 2015, at 11:06, Chris Hegarty wrote: > On 24 Sep 2015, at 08:40, Paul Sandoz wrote: > >> On 23 Sep 2015, at 18:11, Rémi Forax wrote: Because i want to support the simple cases without pulling in indy. >>> >>> and what about using an anonymous class instead ? >>> >> >>

Re: RFR 8135248: Add utility methods to check indexes and ranges

On 24 Sep 2015, at 08:40, Paul Sandoz wrote: > On 23 Sep 2015, at 18:11, Rémi Forax wrote: >>> >>> Because i want to support the simple cases without pulling in indy. >> >> and what about using an anonymous class instead ? >> > > Not an anonymous one, but i think i know what you mean :-) >

Re: RFR 8135248: Add utility methods to check indexes and ranges

On 23 Sep 2015, at 18:11, Rémi Forax wrote: >> >> Because i want to support the simple cases without pulling in indy. > > and what about using an anonymous class instead ? > Not an anonymous one, but i think i know what you mean :-) Also, i don’t want to impose a null check of the exception m

Re: RFR 8135248: Add utility methods to check indexes and ranges

;> Envoyé: Mardi 22 Septembre 2015 12:40:03 >>> Objet: Re: RFR 8135248: Add utility methods to check indexes and >ranges >>> >>> Hi, >>> >>> Thanks for all the feedback. >>> >>> Here is a new webrev: >>> >&g

Re: RFR 8135248: Add utility methods to check indexes and ranges

On 23 Sep 2015, at 01:18, Remi Forax wrote: > Hi Paul, > > > > - Mail original - >> De: "Paul Sandoz" >> À: "core-libs-dev" >> Envoyé: Mardi 22 Septembre 2015 12:40:03 >> Objet: Re: RFR 8135248: Add utility methods to ch

Re: RFR 8135248: Add utility methods to check indexes and ranges

Hi Paul, - Mail original - > De: "Paul Sandoz" > À: "core-libs-dev" > Envoyé: Mardi 22 Septembre 2015 12:40:03 > Objet: Re: RFR 8135248: Add utility methods to check indexes and ranges > > Hi, > > Thanks for all the feedbac

Re: RFR 8135248: Add utility methods to check indexes and ranges

Hi, Thanks for all the feedback. Here is a new webrev: http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8135248-ioobe-check-index-range/webrev/ Changes: - Move check methods to IndexOutOfBoundsException - Use BiFunction, rather than a specific exception mapping function. - Add check methods t

Re: RFR 8135248: Add utility methods to check indexes and ranges

___ > From: core-libs-dev on behalf of > Paul Sandoz > Sent: Monday, September 21, 2015 8:42 AM > To: core-libs-dev > Subject: RFR 8135248: Add utility methods to check indexes and ranges > > Hi, > > Please review the following which adds methods to Arrays to chec

Re: RFR 8135248: Add utility methods to check indexes and ranges

Hi Remi, On 22 Sep 2015, at 08:08, Remi Forax wrote: > Hi Paul, > to summarize, there are a lot of codes that do bound checking in the JDK that > report different kind of exceptions. > Yes, and I suspect it is a similar state of affairs for code outside of the JDK too. > A way to retrofit

Re: RFR 8135248: Add utility methods to check indexes and ranges

: "Paul Sandoz" > Cc: "core-libs-dev" > Envoyé: Lundi 21 Septembre 2015 17:46:07 > Objet: Re: RFR 8135248: Add utility methods to check indexes and ranges > > > On 21 Sep 2015, at 16:45, Remi Forax wrote: > > > I agree with Stephen. > >

Re: RFR 8135248: Add utility methods to check indexes and ranges

Hi Jeff, I did wonder about that when i saw that code too. Without looking at generated machine code i do not know. Paul. On 21 Sep 2015, at 20:12, Jeff Hain wrote: > > Hi. > > > > In your webrev: > > + if ((length | fromIndex | size) < 0 || size > length - fromIndex) > + throw outOfBo

Re: RFR 8135248: Add utility methods to check indexes and ranges

LinkedList would depend on the Arrays class to check bounds. Jason From: core-libs-dev on behalf of Paul Sandoz Sent: Monday, September 21, 2015 8:42 AM To: core-libs-dev Subject: RFR 8135248: Add utility methods to check indexes and ranges Hi, Please

Re: RFR 8135248: Add utility methods to check indexes and ranges

Hi. In your webrev: + if ((length | fromIndex | size) < 0 || size > length - fromIndex) +   throw outOfBounds(fromIndex, size, length, oobe); In Buffer.java (equivalent if adding "| size"):     static void checkBounds(int off, int len, int size) { // package-private     if ((off | len |

Re: RFR 8135248: Add utility methods to check indexes and ranges

Hi Peter, You can find some discussion in the comments of https://bugs.openjdk.java.net/browse/JDK-8042997 which may help. I originally thought we could intrinsify unsigned comparison or have a boolean returning method. Over a series of discussions i was convinced by the current shape that mak

Re: RFR 8135248: Add utility methods to check indexes and ranges

On 21 Sep 2015, at 18:11, Peter Levart wrote: >> I would be happy to sacrifice the use-case in AbstractStringBuilder for >> using BiFunction, and add appropriate >> constructors or factory methods on the common exception types for method ref >> usage. >> >> Paul. > > Don't forget that Abstrac

Re: RFR 8135248: Add utility methods to check indexes and ranges

On 09/21/2015 05:46 PM, Paul Sandoz wrote: On 21 Sep 2015, at 16:45, Remi Forax wrote: I agree with Stephen. Calling the function interface with the name ...Exception seems very wrong to me. Agreed, need to think of a better name. One solution is to remove it all together :-) see below.

Re: RFR 8135248: Add utility methods to check indexes and ranges

On 09/21/2015 05:22 PM, Peter Levart wrote: Hi Paul, It seems that all that is needed for performance is to intrinsify static methods Integer.compareUnsigned(int, int) and Long.compareUnsigned(long, long). Or would that not be enough? Then perhaps explicit compare operations would suffice:

Re: RFR 8135248: Add utility methods to check indexes and ranges

On 21 Sep 2015, at 16:45, Remi Forax wrote: > I agree with Stephen. > > Calling the function interface with the name ...Exception seems very wrong to > me. > Agreed, need to think of a better name. One solution is to remove it all together :-) see below. > The convention of ArrayIOOBE or

Re: RFR 8135248: Add utility methods to check indexes and ranges

Hi Paul, It seems that all that is needed for performance is to intrinsify static methods Integer.compareUnsigned(int, int) and Long.compareUnsigned(long, long). Or would that not be enough? Then perhaps explicit compare operations would suffice: public class Integer { ... public static

Re: RFR 8135248: Add utility methods to check indexes and ranges

On 21 Sep 2015, at 16:17, Roger Riggs wrote: > Hi Paul, > > java.util.Arrays.java: line 5236: new IndexOutOfBoundsException() > - It is always appreciated when debugging to be given a message with the > index that is out of range and the range expected. > Added: ? new IndexOutOfBoundsExcep

Re: RFR 8135248: Add utility methods to check indexes and ranges

On 21 September 2015 at 15:28, Paul Sandoz wrote: >> It would seem very worthwhile to add overloaded versions of each of >> these methods that do not have the OutOfBoundsToException in the >> argument list. Instead, these overloads would throw a "standard" >> exception. > > That seems reasonable e

Re: RFR 8135248: Add utility methods to check indexes and ranges

ot;Remi Forax" > À: "Stephen Colebourne" > Cc: "core-libs-dev" > Envoyé: Lundi 21 Septembre 2015 16:45:32 > Objet: Re: RFR 8135248: Add utility methods to check indexes and ranges > > I agree with Stephen. > > Calling the function interface with the n

Re: RFR 8135248: Add utility methods to check indexes and ranges

nal - > De: "Vitaly Davidovich" > À: "Rémi Forax" > Cc: "core-libs-dev" , "Paul Sandoz" > > Envoyé: Lundi 21 Septembre 2015 16:31:18 > Objet: Re: RFR 8135248: Add utility methods to check indexes and ranges > Interesting - I didn't

Re: RFR 8135248: Add utility methods to check indexes and ranges

t be alble to use those methods inside the JDK without introducing incompatibilities. cheers, Rémi - Mail original - > De: "Stephen Colebourne" > À: "core-libs-dev" > Envoyé: Lundi 21 Septembre 2015 15:52:53 > Objet: Re: RFR 8135248: Add utility methods

Re: RFR 8135248: Add utility methods to check indexes and ranges

y Davidovich" > > À: "Paul Sandoz" > > Cc: "core-libs-dev" > > Envoyé: Lundi 21 Septembre 2015 16:01:16 > > Objet: Re: RFR 8135248: Add utility methods to check indexes and ranges > > > > So is this saying that enhancing Hotspot JIT to detect

Re: RFR 8135248: Add utility methods to check indexes and ranges

- > De: "Vitaly Davidovich" > À: "Paul Sandoz" > Cc: "core-libs-dev" > Envoyé: Lundi 21 Septembre 2015 16:01:16 > Objet: Re: RFR 8135248: Add utility methods to check indexes and ranges > > So is this saying that enhancing Hotspot JIT to detect ran

Re: RFR 8135248: Add utility methods to check indexes and ranges

On 21 Sep 2015, at 15:52, Stephen Colebourne wrote: > While I think I understand the need for the lambda/exception interface > (backwards compatibility) it is definitely weird as a method > signature. Backwards compat has definitely influenced things, but the approach was also considered usefu

Re: RFR 8135248: Add utility methods to check indexes and ranges

Hi Paul, java.util.Arrays.java: line 5236: new IndexOutOfBoundsException() - It is always appreciated when debugging to be given a message with the index that is out of range and the range expected. When converting existing code, is it expected that the exception messages will be unchanged?

Re: RFR 8135248: Add utility methods to check indexes and ranges

So is this saying that enhancing Hotspot JIT to detect range check patterns better is too much work? On the surface, it seems odd to need intrinsics, a functional interface, and fixed template for getting efficient range checks. Also, this may end up with similar profile pollution problems as thin

Re: RFR 8135248: Add utility methods to check indexes and ranges

While I think I understand the need for the lambda/exception interface (backwards compatibility) it is definitely weird as a method signature. It would seem very worthwhile to add overloaded versions of each of these methods that do not have the OutOfBoundsToException in the argument list. Instead

RFR 8135248: Add utility methods to check indexes and ranges

Hi, Please review the following which adds methods to Arrays to check indexes and ranges: https://bugs.openjdk.java.net/browse/JDK-8135248 http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8135248-array-check-index-range/webrev/ The original motivation was an intrinsic method, Arrays.checkIndex