RFR [XXS] 8130646: JCK test api/java_io/StringWriter/index_SWWrite5Test start failing after JDK-8077242
Hi! With the fix for JDK-8077242 ((str) Optimize AbstractStringBuilder.append(CharSequence, int, int) for String argument) a change in behavior was introduced. In the places, where sb.append(str.substring(from, to)) used to be called, in the case of wrong indices StringIndexOutOfBoundsException was thrown. Now, after I replaced the code to sb.append(str, from, to), in such cases IndexOutOfBoundsException is thrown. The proposal is to make AbstractStringBuilder.append(CharSequence, int, int) throw more specific StringIndexOutOfBoundsException, which 1) will eliminate the regression, 2) will be more consistent with most other methods of AbstractStringBuilder, which throw StringIndexOutOfBoundsException. BUGURL: https://bugs.openjdk.java.net/browse/JDK-8130646 WEBREV: http://cr.openjdk.java.net/~igerasim/8130646/00/webrev/ Sincerely yours, Ivan
Resending the request with a new bug id. On 07.07.2015 15:55, Ivan Gerasimov wrote:
Hi!
With the fix for JDK-8077242 ((str) Optimize AbstractStringBuilder.append(CharSequence, int, int) for String argument) a change in behavior was introduced.
In the places, where sb.append(str.substring(from, to)) used to be called, in the case of wrong indices StringIndexOutOfBoundsException was thrown. Now, after I replaced the code to sb.append(str, from, to), in such cases IndexOutOfBoundsException is thrown.
The proposal is to make AbstractStringBuilder.append(CharSequence, int, int) throw more specific StringIndexOutOfBoundsException, which 1) will eliminate the regression, 2) will be more consistent with most other methods of AbstractStringBuilder, which throw StringIndexOutOfBoundsException.
BUGURL: https://bugs.openjdk.java.net/browse/JDK-8130646 WEBREV: http://cr.openjdk.java.net/~igerasim/8130646/00/webrev/
Sincerely yours, Ivan
Hi, Arguably the "StringIndexOutOfBoundsException" should only be used when the index is out of a "String" (the builder included) boundary, but the offending object and the index here is CharSequence/index. The places that throw StringIndexoutOfBoundsException are all for the index/offset/length of the ASB. -sherman On 07/08/2015 10:02 AM, Ivan Gerasimov wrote:
Resending the request with a new bug id.
On 07.07.2015 15:55, Ivan Gerasimov wrote:
Hi!
With the fix for JDK-8077242 ((str) Optimize AbstractStringBuilder.append(CharSequence, int, int) for String argument) a change in behavior was introduced.
In the places, where sb.append(str.substring(from, to)) used to be called, in the case of wrong indices StringIndexOutOfBoundsException was thrown. Now, after I replaced the code to sb.append(str, from, to), in such cases IndexOutOfBoundsException is thrown.
The proposal is to make AbstractStringBuilder.append(CharSequence, int, int) throw more specific StringIndexOutOfBoundsException, which 1) will eliminate the regression, 2) will be more consistent with most other methods of AbstractStringBuilder, which throw StringIndexOutOfBoundsException.
BUGURL: https://bugs.openjdk.java.net/browse/JDK-8130646 WEBREV: http://cr.openjdk.java.net/~igerasim/8130646/00/webrev/
Sincerely yours, Ivan
Thanks Sherman for looking to that! On 11.07.2015 20:22, Xueming Shen wrote:
Hi,
Arguably the "StringIndexOutOfBoundsException" should only be used when the index is out of a "String" (the builder included) boundary, but the offending object and the index here is CharSequence/index. The places that throw StringIndexoutOfBoundsException are all for the index/offset/length of the ASB.
That was my first thought too, but I found places where StringIndexoutOfBoundsException is thrown when violating array char[] boundaries. So, I thought we can use it with CharSequence too. Here's an example: public AbstractStringBuilder insert(int index, char[] str, int offset, int len) { if ((index < 0) || (index > length())) throw new StringIndexOutOfBoundsException(index); if ((offset < 0) || (len < 0) || (offset > str.length - len)) throw new StringIndexOutOfBoundsException( "offset " + offset + ", len " + len + ", str.length " + str.length); The second check is about str array, though the exception is StringIndexOutOfBoundsException. Sincerely yours, Ivan
-sherman
On 07/08/2015 10:02 AM, Ivan Gerasimov wrote:
Resending the request with a new bug id.
On 07.07.2015 15:55, Ivan Gerasimov wrote:
Hi!
With the fix for JDK-8077242 ((str) Optimize AbstractStringBuilder.append(CharSequence, int, int) for String argument) a change in behavior was introduced.
In the places, where sb.append(str.substring(from, to)) used to be called, in the case of wrong indices StringIndexOutOfBoundsException was thrown. Now, after I replaced the code to sb.append(str, from, to), in such cases IndexOutOfBoundsException is thrown.
The proposal is to make AbstractStringBuilder.append(CharSequence, int, int) throw more specific StringIndexOutOfBoundsException, which 1) will eliminate the regression, 2) will be more consistent with most other methods of AbstractStringBuilder, which throw StringIndexOutOfBoundsException.
BUGURL: https://bugs.openjdk.java.net/browse/JDK-8130646 WEBREV: http://cr.openjdk.java.net/~igerasim/8130646/00/webrev/
Sincerely yours, Ivan
What about this variant? http://cr.openjdk.java.net/~igerasim/8130778/00/webrev/ So, we'll throw SOOB only if the CharSequence is actually a String. I've also added the same code to insert(int,CharSequence,int,int) for the sake of symmetry. Sincerely yours, Ivan On 11.07.2015 20:22, Xueming Shen wrote:
Hi,
Arguably the "StringIndexOutOfBoundsException" should only be used when the index is out of a "String" (the builder included) boundary, but the offending object and the index here is CharSequence/index. The places that throw StringIndexoutOfBoundsException are all for the index/offset/length of the ASB.
-sherman
On 07/08/2015 10:02 AM, Ivan Gerasimov wrote:
Resending the request with a new bug id.
On 07.07.2015 15:55, Ivan Gerasimov wrote:
Hi!
With the fix for JDK-8077242 ((str) Optimize AbstractStringBuilder.append(CharSequence, int, int) for String argument) a change in behavior was introduced.
In the places, where sb.append(str.substring(from, to)) used to be called, in the case of wrong indices StringIndexOutOfBoundsException was thrown. Now, after I replaced the code to sb.append(str, from, to), in such cases IndexOutOfBoundsException is thrown.
The proposal is to make AbstractStringBuilder.append(CharSequence, int, int) throw more specific StringIndexOutOfBoundsException, which 1) will eliminate the regression, 2) will be more consistent with most other methods of AbstractStringBuilder, which throw StringIndexOutOfBoundsException.
BUGURL: https://bugs.openjdk.java.net/browse/JDK-8130646 WEBREV: http://cr.openjdk.java.net/~igerasim/8130646/00/webrev/
Sincerely yours, Ivan
participants (2)
-
Ivan Gerasimov
-
Xueming Shen