diff options
author | Paul Phillips <paulp@improving.org> | 2010-06-04 05:03:51 +0000 |
---|---|---|
committer | Paul Phillips <paulp@improving.org> | 2010-06-04 05:03:51 +0000 |
commit | ebb6c4a2d9d882ea258ac64afb5235041f4f40ab (patch) | |
tree | e4f1b43bbe6b833412318d6be38d94cc9d53222c /src/library | |
parent | 09f490bd56a44dd9084f2ea843ac954a586e6d32 (diff) | |
download | scala-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.
Diffstat (limited to 'src/library')
-rw-r--r-- | src/library/scala/collection/immutable/Range.scala | 16 |
1 files changed, 10 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. * |