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. > >>> > >>>