Hi Simon, I chose hyphens because they are used in other AIOOB messages, too. But others (including me) seem not to like them either, so I'll leave them out.
Best regards, Goetz. > -----Original Message----- > From: Simon Nash [mailto:si...@cjnash.com] > Sent: Freitag, 20. April 2018 17:17 > To: Lindenmaier, Goetz <goetz.lindenma...@sap.com> > Cc: David Holmes <david.hol...@oracle.com>; hotspot-runtime- > d...@openjdk.java.net; hotspot-compiler-...@openjdk.java.net; aarch64- > port-...@openjdk.java.net; aarch32-port-...@openjdk.java.net; core-libs- > dev Libs <core-libs-dev@openjdk.java.net> > Subject: Re: RFR(M): 8201593: Print array length in > ArrayIndexOutOfBoundsException. > > Hi Goetz, > It should be "out of bounds" without hyphens. > > Simon > > On 20/04/2018 11:03, Lindenmaier, Goetz wrote: > > Hi David, > > > > What about this: > > java.lang.ArrayIndexOutOfBoundsException: Arraycopy source index -1 > out-of-bounds for double[10]. > > java.lang.ArrayIndexOutOfBoundsException: Arraycopy target index 10 > out-of-bounds for object array[10]. > > java.lang.ArrayIndexOutOfBoundsException: Negative length -19 in > arraycopy from int[3] to int[9]. > > > > I'll reply to the other points in a comprehensive mail when I know > > how to put the message. > > > > Thanks, > > Goetz. > > > > > >> -----Original Message----- > >> From: David Holmes [mailto:david.hol...@oracle.com] > >> Sent: Freitag, 20. April 2018 09:25 > >> To: Lindenmaier, Goetz <goetz.lindenma...@sap.com>; hotspot-runtime- > >> d...@openjdk.java.net; hotspot-compiler-...@openjdk.java.net; > aarch64- > >> port-...@openjdk.java.net; aarch32-port-...@openjdk.java.net; core- > libs- > >> dev Libs <core-libs-dev@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. > >> > >> 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! > >> > >>> It's because I extracted this change from a bigger one. Our message > reads > >> like this: > >>> Object [] oa1 = new Object[10] > >>> oa1[12] > >>> "ArrayIndexOutOfBoundsException while trying to load from index 12 of > an > >> object array with length 10, loaded from local variable 'oa1'" > >>> ... which seems not optimal, either. But it mentions the type (object), > the > >> operation (load, store etc ...) and the variable name. > >>> Naming the array is quite detailed if it is in a complex expression (like > >> returned from a call). > >>> I'll contribute more of this if appreciated, this is a first step. > >> I've never thought this complexity was warranted. We've had a few RFE's > >> of this nature over the years and they have been closed (not necessarily > >> by me I should point out!). > >> > >>> Printing "index N is outside range [0, length-1]" is problematic > >>> for length '0'. I followed the proposal by Roger: > >>> "Index -1 out-of-bounds for length 10." > >> You can easily special-case length 0. But Roger's proposal for > >> consistency with existing messages make sense - not withstanding > >> Andrew's dislike for the hyphens. > >> > >>> I removed the change to ArrayIndexOutOfBoundsException.java. > >> That's good. > >> > >>> 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. > >> > >> 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.hol...@oracle.com] > >>>> Sent: Mittwoch, 18. April 2018 10:55 > >>>> To: Lindenmaier, Goetz <goetz.lindenma...@sap.com>; hotspot- > runtime- > >>>> d...@openjdk.java.net; hotspot-compiler-...@openjdk.java.net; > >> aarch64- > >>>> port-...@openjdk.java.net; aarch32-port-...@openjdk.java.net; > core- > >> libs- > >>>> dev Libs <core-libs-dev@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. > >>>>> > >>>>>
