From 23203f72c763ac847de763573731310989e84b2a Mon Sep 17 00:00:00 2001 From: Rex Kerr Date: Fri, 28 Nov 2014 19:24:02 -0800 Subject: SI-8554 Two-arg remove throws exception on large count ListBuffer now throws exceptions like other buffers do when trying to index out of range. Also harmonized the order of testing (`count < 0` tested before `n` in range). Test in scala-collection-laws (gated by SI8554 flag). Also modified test in run/t6634.scala --- .../scala/collection/mutable/ArrayBuffer.scala | 13 ++++++++----- .../scala/collection/mutable/BufferLike.scala | 7 +++++-- .../scala/collection/mutable/ListBuffer.scala | 11 ++++++----- test/files/run/t6634.check | 6 +++++- test/files/run/t6634.scala | 21 +++++++++++---------- 5 files changed, 35 insertions(+), 23 deletions(-) diff --git a/src/library/scala/collection/mutable/ArrayBuffer.scala b/src/library/scala/collection/mutable/ArrayBuffer.scala index 011fd415ee..167e04ccbd 100644 --- a/src/library/scala/collection/mutable/ArrayBuffer.scala +++ b/src/library/scala/collection/mutable/ArrayBuffer.scala @@ -149,13 +149,16 @@ class ArrayBuffer[A](override protected val initialSize: Int) /** Removes the element on a given index position. It takes time linear in * the buffer size. * - * @param n the index which refers to the first element to delete. - * @param count the number of elements to delete - * @throws IndexOutOfBoundsException if `n` is out of bounds. + * @param n the index which refers to the first element to remove. + * @param count the number of elements to remove. + * @throws IndexOutOfBoundsException if the index `n` is not in the valid range + * `0 <= n <= length - count` (with `count > 0`). + * @throws IllegalArgumentException if `count < 0`. */ override def remove(n: Int, count: Int) { - require(count >= 0, "removing negative number of elements") - if (n < 0 || n > size0 - count) throw new IndexOutOfBoundsException(n.toString) + if (count < 0) throw new IllegalArgumentException("removing negative number of elements: " + count.toString) + else if (count == 0) return // Did nothing + if (n < 0 || n > size0 - count) throw new IndexOutOfBoundsException("at " + n.toString + " deleting " + count.toString) copy(n + count, n, size0 - (n + count)) reduceToSize(size0 - count) } diff --git a/src/library/scala/collection/mutable/BufferLike.scala b/src/library/scala/collection/mutable/BufferLike.scala index 8d24538620..bd9c61ae6a 100644 --- a/src/library/scala/collection/mutable/BufferLike.scala +++ b/src/library/scala/collection/mutable/BufferLike.scala @@ -105,15 +105,18 @@ trait BufferLike[A, +This <: BufferLike[A, This] with Buffer[A]] */ def remove(n: Int): A - /** Removes a number of elements from a given index position. + /** Removes a number of elements from a given index position. Subclasses of `BufferLike` + * will typically override this method to provide better performance than `count` + * successive calls to single-element `remove`. * * @param n the index which refers to the first element to remove. * @param count the number of elements to remove. * @throws IndexOutOfBoundsException if the index `n` is not in the valid range - * `0 <= n <= length - count`. + * `0 <= n <= length - count` (with `count > 0`). * @throws IllegalArgumentException if `count < 0`. */ def remove(n: Int, count: Int) { + if (count < 0) throw new IllegalArgumentException("removing negative number of elements: " + count.toString) for (i <- 0 until count) remove(n) } diff --git a/src/library/scala/collection/mutable/ListBuffer.scala b/src/library/scala/collection/mutable/ListBuffer.scala index 2bc6738e53..eece557fe8 100644 --- a/src/library/scala/collection/mutable/ListBuffer.scala +++ b/src/library/scala/collection/mutable/ListBuffer.scala @@ -262,13 +262,14 @@ final class ListBuffer[A] * * @param n the index which refers to the first element to remove. * @param count the number of elements to remove. + * @throws IndexOutOfBoundsException if the index `n` is not in the valid range + * `0 <= n <= length - count` (with `count > 0`). + * @throws IllegalArgumentException if `count < 0`. */ - @migration("Invalid input values will be rejected in future releases.", "2.11") override def remove(n: Int, count: Int) { - if (n >= len) - return - if (count < 0) - throw new IllegalArgumentException(s"removing negative number ($count) of elements") + if (count < 0) throw new IllegalArgumentException("removing negative number of elements: " + count.toString) + else if (count == 0) return // Nothing to do + if (n < 0 || n > len - count) throw new IndexOutOfBoundsException("at " + n.toString + " deleting " + count.toString) if (exported) copy() val n1 = n max 0 val count1 = count min (len - n1) diff --git a/test/files/run/t6634.check b/test/files/run/t6634.check index f6cbb30c67..b085f397e6 100644 --- a/test/files/run/t6634.check +++ b/test/files/run/t6634.check @@ -4,27 +4,31 @@ String OK. Length OK. Trying lb1 ... +java.lang.IndexOutOfBoundsException: at 6 deleting 6 Checking ... String OK. Length OK. Trying lb2 ... +java.lang.IndexOutOfBoundsException: at 99 deleting 6 Checking ... String OK. Length OK. Trying lb3 ... +java.lang.IndexOutOfBoundsException: at 1 deleting 9 Checking ... String OK. Length OK. Trying lb4 ... +java.lang.IndexOutOfBoundsException: at -1 deleting 1 Checking ... String OK. Length OK. Trying lb5 ... -java.lang.IllegalArgumentException: removing negative number (-1) of elements +java.lang.IllegalArgumentException: removing negative number of elements: -1 Checking ... String OK. Length OK. diff --git a/test/files/run/t6634.scala b/test/files/run/t6634.scala index 759e6d519d..081cca7502 100644 --- a/test/files/run/t6634.scala +++ b/test/files/run/t6634.scala @@ -8,7 +8,7 @@ object Test extends App { try { lb0.remove(5, 0) } catch { - // Not thrown in 2.10, will be thrown in 2.11 + // Should not be thrown--nothing is deleted so nothing to do case ex: IndexOutOfBoundsException => println(ex) } checkNotCorrupted(lb0) @@ -17,8 +17,8 @@ object Test extends App { println("Trying lb1 ...") try { lb1.remove(6, 6) - } catch { - // Not thrown in 2.10, will be thrown in 2.11 + } catch { + // Not thrown in 2.11, is thrown in 2.12 case ex: IndexOutOfBoundsException => println(ex) } checkNotCorrupted(lb1) @@ -28,7 +28,7 @@ object Test extends App { try { lb2.remove(99, 6) } catch { - // Not thrown in 2.10, will be thrown in 2.11 + // Not thrown in 2.11, is thrown in 2.12 case ex: IndexOutOfBoundsException => println(ex) } checkNotCorrupted(lb2) @@ -38,26 +38,27 @@ object Test extends App { try { lb3.remove(1, 9) } catch { - // Not thrown in 2.10, will be thrown in 2.11 - case ex: IllegalArgumentException => println(ex) + // Not thrown in 2.11, is thrown in 2.12 + case ex: IndexOutOfBoundsException => println(ex) } - checkNotCorrupted(lb3, "ListBuffer('a)", 1) + checkNotCorrupted(lb3) val lb4 = newLB println("Trying lb4 ...") try { lb4.remove(-1, 1) } catch { - // Not thrown in 2.10, will be thrown in 2.11 + // Not thrown in 2.11, is thrown in 2.12 case ex: IndexOutOfBoundsException => println(ex) } - checkNotCorrupted(lb4, "ListBuffer('b, 'c, 'd, 'e)", 4) + checkNotCorrupted(lb4) val lb5 = newLB println("Trying lb5 ...") try { lb5.remove(1, -1) } catch { + // Was thrown prior to 2.12 also case ex: IllegalArgumentException => println(ex) } checkNotCorrupted(lb5) @@ -77,4 +78,4 @@ object Test extends App { else println("!!! length FAILED: " + len) println() } -} \ No newline at end of file +} -- cgit v1.2.3