summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPaul Phillips <paulp@improving.org>2010-06-04 05:03:51 +0000
committerPaul Phillips <paulp@improving.org>2010-06-04 05:03:51 +0000
commitebb6c4a2d9d882ea258ac64afb5235041f4f40ab (patch)
treee4f1b43bbe6b833412318d6be38d94cc9d53222c
parent09f490bd56a44dd9084f2ea843ac954a586e6d32 (diff)
downloadscala-ebb6c4a2d9d882ea258ac64afb5235041f4f40ab.tar.gz
scala-ebb6c4a2d9d882ea258ac64afb5235041f4f40ab.tar.bz2
scala-ebb6c4a2d9d882ea258ac64afb5235041f4f40ab.zip
Discovered and disproved this unlikely truth:
scala> (1 to 1 drop 1) == (1 to 1) res0: Boolean = true It was introduced in r21349 which was to fix #2535, but led to #3529. I humbly suggest we can't afford to introduce bugs of this severity in the pursuit of corner cases such as Ranges which use Int.MaxValue as a boundary. And looking at the patch I find the "after" code a lot harder to follow. Closes #3529. Review by prokopec.
-rw-r--r--src/library/scala/collection/immutable/Range.scala16
-rw-r--r--test/files/run/bug3529.scala12
2 files changed, 22 insertions, 6 deletions
diff --git a/src/library/scala/collection/immutable/Range.scala b/src/library/scala/collection/immutable/Range.scala
index cef043555f..930b007389 100644
--- a/src/library/scala/collection/immutable/Range.scala
+++ b/src/library/scala/collection/immutable/Range.scala
@@ -87,6 +87,9 @@ class Range(val start: Int, val end: Int, val step: Int) extends IndexedSeq[Int]
}
// take and drop have to be tolerant of large values without overflowing
+ // warning! this is buggy, and gives wrong answers on boundary cases.
+ // The known bugs are avoided by drop not calling it in those cases.
+ // See ticket #3529. It should be revised.
private def locationAfterN(n: Int) = if (n > 0) {
if (step > 0)
((start.toLong + step.toLong * n.toLong) min last.toLong).toInt
@@ -103,11 +106,11 @@ class Range(val start: Int, val end: Int, val step: Int) extends IndexedSeq[Int]
* @param n the number of elements to take.
* @return a new range consisting of `n` first elements.
*/
- final override def take(n: Int): Range = if (n > 0 && length > 0) {
- Range(start, locationAfterN(n - 1), step).inclusive
- } else {
- Range(start, start, step)
- }
+ final override def take(n: Int): Range =
+ if (n > 0 && length > 0)
+ Range(start, locationAfterN(n - 1), step).inclusive
+ else
+ Range(start, start, step)
/** Creates a new range containing all the elements of this range except the first `n` elements.
*
@@ -117,7 +120,8 @@ class Range(val start: Int, val end: Int, val step: Int) extends IndexedSeq[Int]
* @return a new range consisting of all the elements of this range except `n` first elements.
*/
final override def drop(n: Int): Range =
- copy(locationAfterN(n), end, step)
+ if (n >= length) copy(end + 1, end, step)
+ else copy(locationAfterN(n), end, step)
/** Creates a new range containing all the elements of this range except the last one.
*
diff --git a/test/files/run/bug3529.scala b/test/files/run/bug3529.scala
new file mode 100644
index 0000000000..4d83bcfa22
--- /dev/null
+++ b/test/files/run/bug3529.scala
@@ -0,0 +1,12 @@
+object Test {
+ def main(args: Array[String]): Unit = {
+ assert(1 to 10 drop 10 isEmpty)
+ assert(1 until 10 drop 9 isEmpty)
+ assert(1 to 10 by 2 drop 5 isEmpty)
+
+ assert((1 to 10 drop 9) == Seq(10))
+ assert((1 until 10 drop 9) == Nil)
+
+ assert(Stream(1 to 10).flatten.toList == Stream(1 until 11).flatten.toList)
+ }
+}