New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Range bug: Wrong result for Long.MinValue to Long.MaxValue by Int.MaxValue #4370
Comments
Imported From: https://issues.scala-lang.org/browse/SI-4370?orig=1 |
@soc said: Is there some information on how to write tests? I would at least sit down and try to catch all the tricky stuff on the boundaries. |
@paulp said:
It's not difficult. Writing tests is by far the most useful thing you could do. Just look at the existing tests. Here are the ones which mention Range somehow. % ./partest --grep Range
Testing individual files
testing: [...]/files/pos/spec-cyclic.scala [ OK ]
Testing individual files
testing: [...]/files/run/colltest.scala [ OK ]
testing: [...]/files/run/range.scala [ OK ]
testing: [...]/files/run/concurrent-stream.scala [ OK ]
testing: [...]/files/run/pc-conversions.scala [ OK ]
testing: [...]/files/run/colltest1.scala [ OK ]
Testing individual files
testing: [...]/files/jvm/serialization.scala [ OK ]
Testing individual files
testing: [...]/files/scalacheck/range.scala [ OK ] |
@soc said: Some smaller additions to files/run/range.scala, new file is: import scala.collection.immutable.{ Range, NumericRange }
object Test {
def rangeForeach(range : Range) = {
val buffer = new scala.collection.mutable.ListBuffer[Int];
range.foreach(buffer += _);
assert(buffer.toList == range.iterator.toList, buffer.toList+"/"+range.iterator.toList)
}
def boundaryTests() = {
// SI-4321
assert((Int.MinValue to Int.MaxValue by Int.MaxValue).size == 3)
//Some "boundary" tests for BigInt.
assert((BigInt(Long.MinValue) to BigInt(Long.MaxValue) by BigInt(Long.MaxValue)).size == 3)
assert((BigInt(Long.MinValue) to BigInt(Long.MaxValue-1) by BigInt(Long.MaxValue)).size == 3)
assert((BigInt(Long.MinValue) to BigInt(Long.MaxValue-2) by BigInt(Long.MaxValue)).size == 2)
val biIntMinUpRanges = for(i <- -3 to 3) yield (BigInt(Int.MinValue)-i to BigInt(Int.MinValue))
assert(biIntMinUpRanges.map(_.size).sum == 10)
val biIntMaxUpRanges = for(i <- -3 to 3) yield (BigInt(Int.MaxValue)-i to BigInt(Int.MaxValue))
assert(biIntMaxUpRanges.map(_.size).sum == 10)
val biIntMinDownRanges = for(i <- -3 to 3) yield (BigInt(Int.MinValue) to BigInt(Int.MinValue)-i by -1)
assert(biIntMinUpRanges.map(_.size).sum == 10)
val biIntMaxDownRanges = for(i <- -3 to 3) yield (BigInt(Int.MaxValue) to BigInt(Int.MaxValue)-i by -1)
assert(biIntMaxDownRanges.map(_.size).sum == 10)
val biLongMinUpRanges = for(i <- -3 to 3) yield (BigInt(Long.MinValue)-i to BigInt(Long.MinValue))
assert(biLongMinUpRanges.map(_.size).sum == 10)
val biLongMaxUpRanges = for(i <- -3 to 3) yield (BigInt(Long.MaxValue)-i to BigInt(Long.MaxValue))
assert(biLongMaxUpRanges.map(_.size).sum == 10)
val biLongMinDownRanges = for(i <- -3 to 3) yield (BigInt(Long.MinValue) to BigInt(Long.MinValue)-i by -1)
assert(biLongMinUpRanges.map(_.size).sum == 10)
val biLongMaxDownRanges = for(i <- -3 to 3) yield (BigInt(Long.MaxValue) to BigInt(Long.MaxValue)-i by -1)
assert(biLongMaxDownRanges.map(_.size).sum == 10)
//Overflows. Source of errors.
val intOverflowRange = (Int.MaxValue+1 to Int.MinValue)
assert(intOverflowRange.size == 1)
assert(intOverflowRange.head == Int.MinValue)
val longOverflowRange = (Long.MaxValue+1 to Long.MinValue)
assert(longOverflowRange.size == 1)
assert(longOverflowRange.head == Long.MinValue)
assert(((-7:Byte) to (7:Byte) by 2).size == 8)
//The following ranges should fail.
// SI-4308
val range0 = Long.MinValue to Long.MaxValue
// SI-4370
val range1 = Long.MinValue to Long.MaxValue by Int.MaxValue
val range2 = Long.MaxValue to Long.MinValue by -Int.MaxValue
val range3 = BigInt(Int.MinValue) to BigInt(Int.MaxValue)
val range4 = BigInt(Long.MinValue) to BigInt(Long.MaxValue)
val failRanges = List(range0, range1, range2, range3, range4)
failRanges.foreach(range => assert(exceptionThrown(range)))
def exceptionThrown(range: IndexedSeq[_]) =
try { range.length; false }
catch { case _: IllegalArgumentException => true }
}
case class GR[T](val x: T)(implicit val num: Integral[T]) {
import num._
def negated = GR[T](-x)
def gr1 = NumericRange(x, x, x)
def gr2 = NumericRange.inclusive(x, x, x)
def gr3 = NumericRange(x, x * fromInt(10), x)
def gr4 = NumericRange.inclusive(x, x * fromInt(10), x)
def gr5 = gr3.toList ::: negated.gr3.toList
def check = {
assert(gr1.isEmpty && !gr2.isEmpty)
assert(gr3.size == 9 && gr4.size == 10)
assert(gr5.sum == num.zero, gr5.toString)
assert(!(gr3 contains (x * fromInt(10))))
assert((gr4 contains (x * fromInt(10))))
}
}
def main(args: Array[String]): Unit = {
implicit val imp1 = Numeric.BigDecimalAsIfIntegral
implicit val imp2 = Numeric.DoubleAsIfIntegral
val _grs = List[GR[_]](
GR(BigDecimal(5.0)),
GR(BigInt(5)),
GR(5L),
GR(5.0d),
GR(2.toByte)
)
val grs = _grs ::: (_grs map (_.negated))
grs foreach (_.check)
assert(NumericRange(1, 10, 1) sameElements (1 until 10))
assert(NumericRange.inclusive(1, 10, 1) sameElements (1 to 10))
assert(NumericRange.inclusive(1, 100, 3) sameElements (1 to 100 by 3))
// SI-2518
assert((3L to 7 by 2) sameElements List(3L, 5L, 7L))
rangeForeach(1 to 10);
rangeForeach(1 until 10);
rangeForeach(10 to 1 by -1);
rangeForeach(10 until 1 by -1);
rangeForeach(10 to 1 by -3);
rangeForeach(10 until 1 by -3);
// living on the edges
boundaryTests()
}
} (Two of them (range1, range2) will fail currently.) |
@adriaanm said: |
@adriaanm said: |
Daniel Darabos (darabos) said: scala> 0.0 until 5.0 by 10.0 In Scala 2.11.0: scala> 0.0 until 5.0 by 10.0 Is this change in behavior intended? It is due to the above pull request, and looks like a bug to me. |
@Ichoran said: Another one: 0.0 until 0.3 by 0.1 does not give the expected result. |
Daniel Darabos (darabos) said: I'm trying to fix #8518. I've added a test for it, and have a fix. It also fixes "0.0 until 0.3 by 0.1". Perhaps someone can help me sort out the binary incompatibility warnings? The changes are in: Should I send this as a pull request even if it is unfinished? |
@Ichoran said: |
Daniel Darabos (darabos) said: I appreciate you jumping on the problem right away! It's just that #8518 was broken in 2.10 too, so even if you fix this tomorrow, I still have to figure it out for my change. Thanks! |
@Ichoran said: |
Daniel Darabos (darabos) said: After struggling with binary compatibility a bit longer, I'm of the opinion as well to submit it against 2.12. But how do I do that? Is it on a separate branch, or do I tell test.bc somehow which version to test against? Sorry for polluting your ticket. Let me know if I should take my questions elsewhere. Thanks! |
@Ichoran said: I don't think the 2.12 branch has been created yet. Just hang on until it is, I guess, and then rebase and submit against that branch. But I think I'm going to have to fix the inaccuracy problem anyway. I haven't quite figured out yet what is possible without breaking compatibility. |
Daniel Darabos (darabos) said: |
@Ichoran said: |
Daniel Darabos (darabos) said: |
@paulp said: scala> 0.0 until 0.3 by 0.1
res5: scala.collection.immutable.NumericRange[Double] = NumericRange(0.0, 0.1, 0.2, 0.30000000000000004)
scala> Range.Double(0.0, 0.3, 0.1)
res6: scala.collection.immutable.NumericRange[Double] = NumericRange(0.0, 0.1, 0.2) |
@Ichoran said: |
@Ichoran said: |
=== What steps will reproduce the problem? ===
=== What is the expected behavior? ===
(2*9223372036854775807)/2147483647 = 8589934596
and 8589934596 > Int.MaxValue.
It should be rejected, because it has more than Int.MaxValue elements.
=== What do you see instead? ===
=== Additional information ===
I think Paul is the expert here, see #4308 and #4321.
=== What versions of the following are you using? ===
The text was updated successfully, but these errors were encountered: