RFR(M): 8201593: Print array length in ArrayIndexOutOfBoundsException.

Mon May 7 08:10:57 UTC 2018

Hi Roger, 

new webrev:
http://cr.openjdk.java.net/~goetz/wr18/8201593-lenInAIOOB/07/

See my comments inline.

> -----Original Message-----
> From: Roger Riggs [mailto:roger.riggs at oracle.com]
> 
> Hi Goetz,
> 
> Just comments on the test/message format.  (found in the 04 version of
> the webrev)
> 
> The "." at the end of the exception messages should be removed.
> The text is not a sentence (its is just a fragment without a verb) and
> it is more flexible to print the exception message in various contexts
> (in which the "." would seem to terminate a sentence).
ok, I removed the "."

> I'm not a big fan of the hyphens in out-of-bounds and it would be more
> consistent across the new messages.
I'll do a follow-up and remove the hyphens everywhere. Also removed here.

> It would be more consistent if the "arraycopy:" prefix always included
> the ":".
Added.

Best regards,
  Goetz.

> 
> Thanks, Roger
> 
> 
> On 4/24/18 12:25 PM, Lindenmaier, Goetz wrote:
> > Hi Simon,
> >
> > Because as stated here,
> > http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-
> April/052665.html
> > it is used in other places like that, too.
> >
> > Later mails agreed on that usage to keep it consistent.
> >
> > Best regards,
> >    Goetz.
> >
> >> -----Original Message-----
> >> From: Simon Nash [mailto:simon at cjnash.com]
> >> Sent: Dienstag, 24. April 2018 18:17
> >> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> >> Cc: David Holmes <david.holmes at oracle.com>; hotspot-runtime-
> >> dev at openjdk.java.net; hotspot-compiler-dev at openjdk.java.net;
> aarch64-
> >> port-dev at openjdk.java.net; aarch32-port-dev at openjdk.java.net; core-
> libs-
> >> dev Libs <core-libs-dev at openjdk.java.net>
> >> Subject: Re: RFR(M): 8201593: Print array length in
> >> ArrayIndexOutOfBoundsException.
> >>
> >> On 24/04/2018 15:08, Lindenmaier, Goetz wrote:
> >>> Hi,
> >>>
> >>> I implemented what we figured out in
> >>> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2018-
> >> April/027555.html
> >>> Some examples:
> >>> "Index 12 out-of-bounds for length 10."
> >>> "arraycopy source index -17 out of bounds for object array[10]."
> >>> "arraycopy destination index -18 out of bounds for double[5]."
> >>> "arraycopy length -19 is negative."
> >>> "arraycopy: last source index 13 out of bounds for double[10]."
> >>> "arraycopy: last destination index 7 out of bounds for long[5]."
> >>>
> >> Is there a reason why the first message says "out-of-bounds" but all
> others
> >> say "out of bounds"?
> >>
> >> Simon
> >>
> >>> Incremental webrev:
> >>> http://cr.openjdk.java.net/~goetz/wr18/8201593-lenInAIOOB/03-
> >> incremental/
> >>> Full webrev:
> >>> http://cr.openjdk.java.net/~goetz/wr18/8201593-lenInAIOOB/03/
> >>>
> >>> I'll push it through our tests tonight.
> >>>
> >>> See further comments in line:
> >>>
> >>>> -----Original Message-----
> >>>> From: David Holmes [mailto:david.holmes at oracle.com]
> >>>> Sent: Freitag, 20. April 2018 09:25
> >>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-
> runtime-
> >>>> dev at openjdk.java.net; hotspot-compiler-dev at openjdk.java.net;
> >> aarch64-
> >>>> port-dev at openjdk.java.net; aarch32-port-dev at openjdk.java.net;
> core-
> >> libs-
> >>>> dev Libs <core-libs-dev at openjdk.java.net>
> >>>> Subject: Re: RFR(M): 8201593: Print array length in
> >>>> ArrayIndexOutOfBoundsException.
> >>>>
> >>>> Hi Goetz,
> >>>>
> >>>> This is not a file by file review ...
> >>>>
> >>>> On 19/04/2018 10:24 PM, Lindenmaier, Goetz wrote:
> >>>>> Hi,
> >>>>>
> >>>>> New webrev:
> >>>>> http://cr.openjdk.java.net/~goetz/wr18/8201593-lenInAIOOB/02/
> >>>>>
> >>>>> I admit my wording is not optimal.
> >>>> src/hotspot/share/oops/typeArrayKlass.cpp
> >>>>
> >>>> Sorry but this is still far too verbose for my tastes. The type of the
> >>>> array is not relevant. For the array copy its okay to indicate src or
> >>>> dst array. But the message should be clear and succinct not prose-like.
> >>>> Plus you have a lot of repetition in the ss.print statements when only
> >>>> one thing is changing.
> >>> We discussed this in some further mails. Implemented as proposed
> there.
> >>>
> >>>> src/hotspot/cpu/x86/c1_CodeStubs_x86.cpp
> >>>>
> >>>> I'm not seeing why the throw_index_out_of_bounds_exception
> should
> >> be
> >>>> tied to whether or not you have an array reference. Certainly I hate
> >>>> seeing the array ref being used as an implicit bool!
> >>> I split the constructor into two, one for ArrayIndexOutOfBounds, the
> other
> >>> for IndexOutOfBounds.
> >>>
> >>> ...
> >>>
> >>>>> I extended the test to cover the exception thrown in arraycopy better
> >>>> The test seems somewhat excessive, and I now see it is because of the
> >>>> more elaborate error messages you have at SAP. But with only the
> index
> >>>> and the array length of interest here the test can be considerably
> smaller.
> >>>>
> >>>> The creation tests for ArrayIndexOutOfBoundsException don't seem
> >>>> relevant in this context either. This looks more TCK like.
> >>> Yes, the constructor tests are for the code not yet contributed.
> >>> I simplified the tests to only check the messages.
> >>>
> >>> Best regards,
> >>>    Goetz.
> >>>
> >>>
> >>>
> >>>
> >>>
> >>>> David
> >>>> -----
> >>>>
> >>>>> and added the elementary type to the message text.  This probably
> >>>>> needs improvement in the text, too.  There are (currently) these
> cases:
> >>>>>
> >>>>> bject[]  oa1 = new Object[10];
> >>>>> Object[]  oa2 = new Object[5];
> >>>>> System.arraycopy(oa1, -17, oa2, 0, 5);
> >>>>> "while trying to copy from index -17 of an object array with length
> 10");
> >>>>> System.arraycopy(oa1, 2, oa2, -18, 5);
> >>>>> "while trying to copy to index -18 of an object array with length 5");
> >>>>> System.arraycopy(oa1, 2, oa2, 0, -19);
> >>>>> "while trying to copy a negative range -19 from an object array with
> >> length
> >>>> 10 to an object array with length 5");
> >>>>> System.arraycopy(oa1, 8, oa2, 0, 5);
> >>>>> "while trying to copy from index 13 of an object array with length 10");
> >>>>> System.arraycopy(oa1, 1, oa2, 0, 7);
> >>>>> "while trying to copy to index 7 of an object array with length 5");
> >>>>> double[]  ta1 = new double[10];
> >>>>> double[]  ta2 = new double[5];
> >>>>> System.arraycopy(ta1, -17, ta2, 0, 5);
> >>>>> "while trying to copy from index -17 of a doubl array with length 10");
> >>>>> System.arraycopy(ta1, 2, ta2, -18, 5);
> >>>>> "while trying to copy to index -18 of a double array with length 5");
> >>>>> System.arraycopy(ta1, 2, ta2, 0, -19);
> >>>>> "while trying to copy a negative range -19 from a double array with
> >> length
> >>>> 10 to a double array with length 5");
> >>>>> System.arraycopy(ta1, 8, ta2, 0, 5);
> >>>>> "while trying to copy from index 13 of a double array with length 10");
> >>>>> System.arraycopy(ta1, 1, ta2, 0, 7);
> >>>>> "while trying to copy to index 7 of a double array with length 5");
> >>>>>
> >>>>> Maybe it should say:
> >>>>> Arraycopy source index -1 out-of-bounds for double array of length
> 10.
> >>>>> Arraycopy target index 10 out-of-bounds for object array of length 10.
> >>>>> Negative range -19 when copying from an object array of length 10 to
> an
> >>>> object array of length 5.
> >>>>> Best regards,
> >>>>>     Goetz.
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: David Holmes [mailto:david.holmes at oracle.com]
> >>>>>> Sent: Mittwoch, 18. April 2018 10:55
> >>>>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-
> >> runtime-
> >>>>>> dev at openjdk.java.net; hotspot-compiler-dev at openjdk.java.net;
> >>>> aarch64-
> >>>>>> port-dev at openjdk.java.net; aarch32-port-dev at openjdk.java.net;
> >> core-
> >>>> libs-
> >>>>>> dev Libs <core-libs-dev at openjdk.java.net>
> >>>>>> Subject: Re: RFR(M): 8201593: Print array length in
> >>>>>> ArrayIndexOutOfBoundsException.
> >>>>>>
> >>>>>> Adding core-libs-dev as you're changing
> >>>>>> java.lang.ArrayIndexOutOfBoundsException.
> >>>>>>
> >>>>>> I appreciate the intent here but I find the messages excessively
> >>>>>> verbose. The basic error is:
> >>>>>>
> >>>>>> index N is outside range [0, length-1]
> >>>>>>
> >>>>>> David
> >>>>>>
> >>>>>> On 18/04/2018 6:09 PM, Lindenmaier, Goetz wrote:
> >>>>>>> Hi,
> >>>>>>>
> >>>>>>> I would like to print a more verbose text on
> ArrayIndexOutOfBounds
> >>>>>> exception
> >>>>>>> that not only mentions the index, but also the length of the array
> >>>> accessed.
> >>>>>>> See the bug for documentation of the change of the message.
> >>>>>>> http://cr.openjdk.java.net/~goetz/wr18/8201593-lenInAIOOB/01/
> >>>>>>>
> >>>>>>> @aarch/arm people:
> >>>>>>> I edited the aarch/arm files. Could you please verify this is correct?
> >>>>>>> I can not build on these platforms.
> >>>>>>>
> >>>>>>> The code on all the other platforms is tested with all the jtreg and
> jck
> >>>> tests
> >>>>>> etc.
> >>>>>>> Best regards,
> >>>>>>>      Goetz.
> >>>>>>>
> >>>>>>>