RFR 8135248: Add utility methods to check indexes and ranges

[Remi Forax]

Hi Paul,



----- Mail original -----
> De: "Paul Sandoz" <paul.sandoz at oracle.com>
> À: "core-libs-dev" <core-libs-dev at openjdk.java.net>
> 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:
> 
>   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 that do not accept an exception mapping function and
> throw IndexOutOfBoundsException.

I still think that you should not use null as Stephen noticed,
what's wrong with writing checkIndex like this ?

  int checkIndex(int index, int length) throws IndexOutOfBoundsException {
    return checkIndex(index, length, IndexOutOfBoundsException::new);
  }

> 
> - Add 2 argument constructors to Array/String/IndexOutOfBoundsException, thus
> allowing support for method refs.
> 
> Remi, i left the generic signatures as is, i am presuming there is an
> advantage to naming T, rather than using a wildcard e.g. IDEs could use that
> information (although IntelliJ does not appear to do so at the moment).

I don't get it. Why it's important for an IDE to know that a code throws a precise *runtime* exception. Any codes can throw any runtime exceptions.

> 
> Paul.


Rémi

> 
> On 21 Sep 2015, at 15:42, Paul Sandoz <Paul.Sandoz at oracle.com> wrote:
> 
> > 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, to
> > check if an index is within bounds. Such an intrinsic guides HotSpot
> > towards better optimisations for bounds checks using one unsigned
> > comparison instead of two signed comparisons, and better eliding of
> > integer to long conversions when an index is used to create an offset for
> > Unsafe access. The end result is more efficient array access especially so
> > from within unrolled loops. The VarHandles work will use Arrays.checkIndex
> > for array access.
> > 
> > A follow up issue [1] will track the intrinsification of Arrays.checkIndex.
> > 
> > We thought it would be opportunistic to support two further common
> > use-cases for sub-range checks, Arrays.checkFromToIndex and Arrays.
> > checkFromIndexSize. There is no current plan to intrinsify these methods.
> > 
> > Bounds checking is not difficult but it can be easy to make trivial
> > mistakes. Thus it is advantageous to consolidate such checks not just from
> > an optimization perspective but from a correctness and security/integrity
> > perspective.
> > 
> > There are many areas in the JDK where such checks are performed. A follow
> > up issue [2] will track updates to use the new methods.
> > 
> > The main challenge for these new methods is to design in such a way that
> > 
> > 1) existing use-cases can still report the same set of exceptions with the
> > same messages;
> > 2) method byte code size is not unduly increased, thus perturbing inlining;
> > and
> > 3) there is a reasonable path for any future support of long indexes.
> > 
> > Paul.
> > 
> > [1] https://bugs.openjdk.java.net/browse/JDK-8042997
> > [2] https://bugs.openjdk.java.net/browse/JDK-8135250
> 
>