RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-13 Thread Ivan Gerasimov
Hello! Would you please review a simple fix of the javadoc for ArrayList#removeRange() method? The doc says that IndexOutOfBoundsException is thrown if fromIndex or toIndex is out of range (fromIndex < 0 || fromIndex >= size() || toIndex > size() || toIndex < fromIndex). The condition 'fro

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-13 Thread Chris Hegarty
Ivan, This does look a little odd, but since fromIndex is inclusive I would think that it should throw if passed a value of size() ?? -Chris. On 13 Mar 2014, at 15:29, Ivan Gerasimov wrote: > Hello! > > Would you please review a simple fix of the javadoc for > ArrayList#removeRange() method

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-13 Thread Ivan Gerasimov
Thank you Chris! It is System.arraycopy() method, where checking is performed and the exception is thrown. Here's this code: if ( (((unsigned int) length + (unsigned int) src_pos) > (unsigned int) s->length()) || (((unsigned int) length + (unsigned int) dst_pos) > (unsigned int) d->len

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-13 Thread Ivan Gerasimov
Thank you Martin for your feedback! I'll add the test with the next webrev. Here's the test I used to check that the exception isn't thrown: class Test extends java.util.ArrayList { public static void main(String[] args) throws Throwable { Test list = new Test(); list.add(1);

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-13 Thread Ivan Gerasimov
On 13.03.2014 20:37, Martin Buchholz wrote: The corner case for removeRange(SIZE, SIZE) does seem rather tricky, and it's easy for an implementation to get it wrong. All the more reason to add tests! It was a good idea to add a test for removeRange(). I just discovered that IOOB isn't throw

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-13 Thread Ivan Gerasimov
On 13.03.2014 22:16, Ivan Gerasimov wrote: On 13.03.2014 20:37, Martin Buchholz wrote: The corner case for removeRange(SIZE, SIZE) does seem rather tricky, and it's easy for an implementation to get it wrong. All the more reason to add tests! It was a good idea to add a test for removeRan

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-13 Thread Ivan Gerasimov
Would you please take a look at the updated webrev: http://cr.openjdk.java.net/~igerasim/8014066/1/webrev/ In addition to the javadoc correction, it includes a regression test and a check for (fromIndex > toIndex). The last check turns out to be sufficient for removeRange() method to agree with

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-13 Thread Ivan Gerasimov
Sorry, I forgot to incorporate changes to AbstractList.removeRange() documentation, which you Martin had suggested. Here's the webrev with those included: http://cr.openjdk.java.net/~igerasim/8014066/2/webrev/ Sincerely yours, Ivan On 13.03.2014 23:03, Ivan Gerasimov wrote: Would you please

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-13 Thread Doug Lea
On 03/13/2014 12:34 PM, Martin Buchholz wrote: I notice there are zero jtreg tests for removeRange. That should be fixed. I notice there is a removeRange in CopyOnWriteArrayList, but it is package-private instead of "protected", which seems like an oversight. Can Doug remember any history on t

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-13 Thread David Holmes
Hi Ivan, I think we need to apply the brakes here and back up a bit :) First of all these aren't typo fixes they are spec changes and they will need to go through CCC for approval. Second point is that the condition "fromIndex >= size()" is already captured by the condition "if {@code fromIn

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-14 Thread Ivan Gerasimov
Thank you David for your reply! On 14.03.2014 8:56, David Holmes wrote: Hi Ivan, I think we need to apply the brakes here and back up a bit :) First of all these aren't typo fixes they are spec changes and they will need to go through CCC for approval. Yes, sure. I haven't done that before,

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-14 Thread David Holmes
Hi Ivan, On 14/03/2014 5:05 PM, Ivan Gerasimov wrote: Thank you David for your reply! On 14.03.2014 8:56, David Holmes wrote: Hi Ivan, I think we need to apply the brakes here and back up a bit :) First of all these aren't typo fixes they are spec changes and they will need to go through CCC

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-14 Thread Ivan Gerasimov
Thanks David! On 14.03.2014 12:31, David Holmes wrote: Second point is that the condition "fromIndex >= size()" is already captured by the condition "if {@code fromIndex} or {@code toIndex} is out of range". By definition fromIndex is out-of-range if it is < 0 or >= size(). So I don't see an

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-14 Thread Peter Levart
On 03/14/2014 08:05 AM, Ivan Gerasimov wrote: One thing I noticed is that some methods I mentioned above (List.subList(), Arrays.sort(), etc) throw IllegalArgumentException when fromIndex > toIndex, not IndexOutOfBoundException. Wouldn't it be more correct to adopt this into removeRange() too?

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-14 Thread Roger Riggs
Hi, Without getting into the details, make sure that the behavior seen by applications is not changed without careful and exhaustive review. Usually, the specification is updated to match the existing long standing behavior including exceptions and edge cases. Roger On 3/14/14 6:50 AM, Ivan

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-14 Thread Ivan Gerasimov
Thanks Peter for the comments. On 14.03.2014 14:53, Peter Levart wrote: On 03/14/2014 08:05 AM, Ivan Gerasimov wrote: One thing I noticed is that some methods I mentioned above (List.subList(), Arrays.sort(), etc) throw IllegalArgumentException when fromIndex > toIndex, not IndexOutOfBoundExce

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-14 Thread Doug Lea
On 03/14/2014 02:38 AM, Martin Buchholz wrote: On Thu, Mar 13, 2014 at 3:59 PM, Doug Lea mailto:d...@cs.oswego.edu>> wrote: On 03/13/2014 12:34 PM, Martin Buchholz wrote: I notice there are zero jtreg tests for removeRange. That should be fixed. I notice there is a removeRa

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-14 Thread David Holmes
Ivan, On 14/03/2014 9:19 PM, Ivan Gerasimov wrote: Thanks Peter for the comments. On 14.03.2014 14:53, Peter Levart wrote: On 03/14/2014 08:05 AM, Ivan Gerasimov wrote: One thing I noticed is that some methods I mentioned above (List.subList(), Arrays.sort(), etc) throw IllegalArgumentExcepti

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-14 Thread David Holmes
Hi Doug, On 14/03/2014 9:42 PM, Doug Lea wrote: On 03/14/2014 02:38 AM, Martin Buchholz wrote: On Thu, Mar 13, 2014 at 3:59 PM, Doug Lea mailto:d...@cs.oswego.edu>> wrote: On 03/13/2014 12:34 PM, Martin Buchholz wrote: I notice there are zero jtreg tests for removeRange. That shoul

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-14 Thread Ivan Gerasimov
On 14.03.2014 16:02, David Holmes wrote: Ivan, On 14/03/2014 9:19 PM, Ivan Gerasimov wrote: Thanks Peter for the comments. On 14.03.2014 14:53, Peter Levart wrote: On 03/14/2014 08:05 AM, Ivan Gerasimov wrote: One thing I noticed is that some methods I mentioned above (List.subList(), Array

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-14 Thread David Holmes
On 14/03/2014 10:14 PM, Ivan Gerasimov wrote: On 14.03.2014 16:02, David Holmes wrote: Ivan, On 14/03/2014 9:19 PM, Ivan Gerasimov wrote: Thanks Peter for the comments. On 14.03.2014 14:53, Peter Levart wrote: On 03/14/2014 08:05 AM, Ivan Gerasimov wrote: One thing I noticed is that some m

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-14 Thread Doug Lea
On 03/14/2014 08:07 AM, David Holmes wrote: So what you are saying is that protected overrides of protected methods are not required to honor the specification of the super method? Not always, but ... That certainly gives some implementation flexibility, but I don't think I've ever seen it

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-14 Thread Peter Levart
On 03/14/2014 01:14 PM, Ivan Gerasimov wrote: On 14.03.2014 16:02, David Holmes wrote: Ivan, On 14/03/2014 9:19 PM, Ivan Gerasimov wrote: Thanks Peter for the comments. On 14.03.2014 14:53, Peter Levart wrote: On 03/14/2014 08:05 AM, Ivan Gerasimov wrote: One thing I noticed is that some m

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-14 Thread Ivan Gerasimov
On 14.03.2014 18:18, Peter Levart wrote: On 03/14/2014 01:14 PM, Ivan Gerasimov wrote: On 14.03.2014 16:02, David Holmes wrote: Ivan, On 14/03/2014 9:19 PM, Ivan Gerasimov wrote: Thanks Peter for the comments. On 14.03.2014 14:53, Peter Levart wrote: On 03/14/2014 08:05 AM, Ivan Gerasimov

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-14 Thread Mike Duigou
On Mar 14 2014, at 05:14 , Ivan Gerasimov wrote: > > On 14.03.2014 16:02, David Holmes wrote: >> Ivan, >> >> On 14/03/2014 9:19 PM, Ivan Gerasimov wrote: >>> Thanks Peter for the comments. >>> >>> On 14.03.2014 14:53, Peter Levart wrote: On 03/14/2014 08:05 AM, Ivan Gerasimov wrote:

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-15 Thread Ivan Gerasimov
Thank you Martin! On 15.03.2014 17:24, Martin Buchholz wrote: We understand the intent of removeRange better now. I agree that making further improvements is tricky. I think we can all agree that the initial patch http://cr.openjdk.java.net/~igerasim/8014066/0/webrev/jdk.patch is a clear improv

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-15 Thread David Holmes
On 14/03/2014 10:22 PM, Doug Lea wrote: On 03/14/2014 08:07 AM, David Holmes wrote: So what you are saying is that protected overrides of protected methods are not required to honor the specification of the super method? Not always, but ... That certainly gives some implementation flexibil

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-15 Thread David Holmes
Hi Martin, On 15/03/2014 11:24 PM, Martin Buchholz wrote: We understand the intent of removeRange better now. I agree that making further improvements is tricky. Almost impossible I'd say :) I think we can all agree that the initial patch http://cr.openjdk.java.net/~igerasim/8014066/0/webrev

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-16 Thread Ivan Gerasimov
Here is yet another iteration of the fix: http://cr.openjdk.java.net/~igerasim/8014066/3/webrev/ 1) The condition 'fromIndex >= size()' is removed from the spec. I prefer removing it rather than replacing it with 'fromIndex > size()' for two reasons: - 'fromIndex > size()' already follows on fro

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-16 Thread Ulf Zibis
Am 16.03.2014 23:37, schrieb Ivan Gerasimov: Here is yet another iteration of the fix: http://cr.openjdk.java.net/~igerasim/8014066/3/webrev/ 2) Kept the check for 'fromIndex > toIndex' in removeRange(). While I understand that this should not add anything significant to the current code, as cur

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-16 Thread Ivan Gerasimov
On 17.03.2014 2:52, Ulf Zibis wrote: Am 16.03.2014 23:37, schrieb Ivan Gerasimov: Here is yet another iteration of the fix: http://cr.openjdk.java.net/~igerasim/8014066/3/webrev/ 2) Kept the check for 'fromIndex > toIndex' in removeRange(). While I understand that this should not add anything

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-17 Thread roger riggs
Hi Ivan, Just to see the effect of the change, I ran the test without the code change and it reported 331 failures and only 1 IndexOutOfBoundsException not thrown and that was in java.util.Collections$SingletonMap. (I can't appreciate the error reporting style with just a stack trace; especia

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-17 Thread David Holmes
Hi Ivan, On 17/03/2014 8:37 AM, Ivan Gerasimov wrote: Here is yet another iteration of the fix: http://cr.openjdk.java.net/~igerasim/8014066/3/webrev/ 1) The condition 'fromIndex >= size()' is removed from the spec. I prefer removing it rather than replacing it with 'fromIndex > size()' for two

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-18 Thread Ivan Gerasimov
Hello! Would you please take a look at the next iteration of webrev? I incorporated the last suggestions in it. When I first proposed a simple typo fix, I didn't think it's going to cause such a big discussion :) Assuming this last iteration is OK, should the next step be a CCC request? Sinc

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-18 Thread Ivan Gerasimov
Sorry, here's the link: http://cr.openjdk.java.net/~igerasim/8014066/4/webrev/ On 18.03.2014 22:28, Ivan Gerasimov wrote: Hello! Would you please take a look at the next iteration of webrev? I incorporated the last suggestions in it. When I first proposed a simple typo fix, I didn't think it's

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-18 Thread Ulf Zibis
Am 18.03.2014 19:28, schrieb Ivan Gerasimov: Assuming this last iteration is OK, should the next step be a CCC request? Do you mean? : /* * ... + * It is assumed that fromIndex <= toIndex, otherwise the behaviour of this method is undefined. * ... - * toInde

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-18 Thread Mike Duigou
Looks good. On Mar 18 2014, at 11:37 , Ivan Gerasimov wrote: > Sorry, here's the link: > http://cr.openjdk.java.net/~igerasim/8014066/4/webrev/ > > On 18.03.2014 22:28, Ivan Gerasimov wrote: >> Hello! >> >> Would you please take a look at the next iteration of webrev? >> I incorporated the las

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-18 Thread Ulf Zibis
Am 18.03.2014 23:17, schrieb Martin Buchholz: modCount is an imprecise concurrent modification mechanism. It doesn't have to be kept transactionally correct. Thanks, yes I know. But does it hurt to make it more precise? See this as a concept for a RFE. -Ulf

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-19 Thread Ivan Gerasimov
Thank you Ulf! On 19.03.2014 2:12, Ulf Zibis wrote: Am 18.03.2014 19:28, schrieb Ivan Gerasimov: Assuming this last iteration is OK, should the next step be a CCC request? Do you mean? : /* * ... + * It is assumed that fromIndex <= toIndex, otherwise the behaviour of this me

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-19 Thread David Holmes
Fine by me. David On 19/03/2014 4:37 AM, Ivan Gerasimov wrote: Sorry, here's the link: http://cr.openjdk.java.net/~igerasim/8014066/4/webrev/ On 18.03.2014 22:28, Ivan Gerasimov wrote: Hello! Would you please take a look at the next iteration of webrev? I incorporated the last suggestions in

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-19 Thread Chris Hegarty
On 19/03/14 08:29, David Holmes wrote: Fine by me. David On 19/03/2014 4:37 AM, Ivan Gerasimov wrote: Sorry, here's the link: http://cr.openjdk.java.net/~igerasim/8014066/4/webrev/ Looks fine to me too. -Chris. On 18.03.2014 22:28, Ivan Gerasimov wrote: Hello! Would you please take a l

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-19 Thread Ivan Gerasimov
On 19.03.2014 20:11, Martin Buchholz wrote: On Wed, Mar 19, 2014 at 1:19 AM, Ivan Gerasimov mailto:ivan.gerasi...@oracle.com>> wrote: Improving modCound handling can be done with a different RFE I guess, as it doesn't connected to the rest of the fix. I don't think modCount hand

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-19 Thread Ulf Zibis
Am 19.03.2014 09:19, schrieb Ivan Gerasimov: Thank you Ulf! On 19.03.2014 2:12, Ulf Zibis wrote: Am 18.03.2014 19:28, schrieb Ivan Gerasimov: Assuming this last iteration is OK, should the next step be a CCC request? Do you mean? : /* * ... + * It is assumed that fromIndex <=

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-19 Thread Ulf Zibis
Am 19.03.2014 23:32, schrieb Martin Buchholz: No one is as performance obsessed as Ulf. :-D :-D :-D

Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange

2014-03-20 Thread Ulf Zibis
Am 19.03.2014 23:37, schrieb Ulf Zibis: Am 19.03.2014 23:32, schrieb Martin Buchholz: No one is as performance obsessed as Ulf. :-D :-D :-D And additionally footprint obsessed -Ulf