SI-9388 Fix Range behavior around Int.MaxValue

terminalElement (the element _after_ the last one!) was used to terminate foreach loops and sums of non-standard instances of Numeric.  Unfortunately, this could result in the end wrapping around and hitting the beginning again, making the first element bad.

This patch fixes the behavior by altering the loop to end after the last element is encountered.  The particular flavor was chosen out of a few possibilities because it gave the best microbenchmarks on both large and small ranges.

Test written.  While testing, a bug was also uncovered in NumericRange, and was also fixed.  In brief, the logic around sum is rather complex since division is not unique when you have overflow.  Floating point has its own complexities, too.

Also updated incorrect test t4658 that insisted on incorrect answers (?!) and added logic to make sure it at least stays self-consistent, and fixed the range.scala test which used the same wrong (overflow-prone) formula that the Range collection did.

1 files changed, 13 insertions, 16 deletions

diff --git a/src/library/scala/collection/immutable/Range.scala b/src/library/scala/collection/immutable/Range.scala
index 79cd673932..ca6720da19 100644
--- a/src/library/scala/collection/immutable/Range.scala
+++ b/src/library/scala/collection/immutable/Range.scala
@@ -153,19 +153,15 @@ extends scala.collection.AbstractSeq[Int]
   }
 
   @inline final override def foreach[@specialized(Unit) U](f: Int => U) {
-    validateMaxLength()
-    val isCommonCase = (start != Int.MinValue || end != Int.MinValue)
-    var i = start
-    var count = 0
-    val terminal = terminalElement
-    val step = this.step
-    while(
-      if(isCommonCase) { i != terminal }
-      else             { count < numRangeElements }
-    ) {
-      f(i)
-      count += 1
-      i += step
+    // Implementation chosen on the basis of favorable microbenchmarks
+    // Note--initialization catches step == 0 so we don't need to here
+    if (!isEmpty) {
+      var i = start
+      while (true) {
+        f(i)
+        if (i == lastElement) return
+        i += step
+      }
     }
   }
 
@@ -347,18 +343,19 @@ extends scala.collection.AbstractSeq[Int]
       // this is normal integer range with usual addition. arithmetic series formula can be used
       if (isEmpty) 0
       else if (numRangeElements == 1) head
-      else (numRangeElements.toLong * (head + last) / 2).toInt
+      else ((numRangeElements * (head.toLong + last)) / 2).toInt
     } else {
       // user provided custom Numeric, we cannot rely on arithmetic series formula
       if (isEmpty) num.toInt(num.zero)
       else {
         var acc = num.zero
         var i = head
-        while(i != terminalElement) {
+        while (true) {
           acc = num.plus(acc, i)
+          if (i == lastElement) return num.toInt(acc)
           i = i + step
         }
-        num.toInt(acc)
+        0 // Never hit this--just to satisfy compiler since it doesn't know while(true) has type Nothing
       }
     }
   }