Skip to content

Commit 5c599a3

Browse files
paulppaulp
committed
Intermediate range commit.
This is not a long-term implementation - I haven't even benchmarked it -- don't worry. I did find a fairly gross bug in the version I checked in a few days ago so I wanted to erase that from the repository memory for the weekend.
1 parent 98108fe commit 5c599a3

File tree

1 file changed

+79
-106
lines changed

1 file changed

+79
-106
lines changed

‎src/library/scala/collection/immutable/Range.scala‎

Lines changed: 79 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -51,15 +51,35 @@ extends collection.AbstractSeq[Int]
5151
{
5252
override def par = new ParRange(this)
5353

54-
// This member is designed to enforce conditions:
55-
// (step != 0) && (length <= Int.MaxValue),
56-
// but cannot be evaluated eagerly because we have a pattern where ranges
57-
// are constructed like: "x to y by z"
58-
// The "x to y" piece should not trigger an exception. So the calculation
59-
// is delayed, which means it will not fail fast for those cases where failing
60-
// was correct.
61-
private lazy val numRangeElements: Int = Range.count(start, end, step, isInclusive)
62-
54+
private def gap = end.toLong - start.toLong
55+
private def isExact = gap % step == 0
56+
private def hasStub = isInclusive || !isExact
57+
private def longLength = gap / step + ( if (hasStub) 1 else 0 )
58+
59+
// Check cannot be evaluated eagerly because we have a pattern where
60+
// ranges are constructed like: "x to y by z" The "x to y" piece
61+
// should not trigger an exception. So the calculation is delayed,
62+
// which means it will not fail fast for those cases where failing was
63+
// correct.
64+
override final val isEmpty = (
65+
(start > end && step > 0)
66+
|| (start < end && step < 0)
67+
|| (start == end && !isInclusive)
68+
)
69+
final val numRangeElements: Int = {
70+
if (step == 0) throw new IllegalArgumentException("step cannot be 0.")
71+
else if (isEmpty) 0
72+
else {
73+
val len = longLength
74+
if (len > scala.Int.MaxValue) -1
75+
else len.toInt
76+
}
77+
}
78+
final val lastElement = start + (numRangeElements - 1) * step
79+
final val terminalElement = start + numRangeElements * step
80+
81+
override def last = if (isEmpty) Nil.last else lastElement
82+
6383
protected def copy(start: Int, end: Int, step: Int): Range = new Range(start, end, step)
6484

6585
/** Create a new range with the `start` and `end` values of this range and
@@ -71,93 +91,46 @@ extends collection.AbstractSeq[Int]
7191

7292
def isInclusive = false
7393

74-
override def length: Int = numRangeElements
75-
override lazy val last: Int =
76-
if (length == 0) Nil.last
77-
else locationAfterN(length - 1)
78-
79-
final override def isEmpty = length == 0
80-
81-
@inline
82-
final def apply(idx: Int): Int = {
83-
if (idx < 0 || idx >= length) throw new IndexOutOfBoundsException(idx.toString)
84-
locationAfterN(idx)
94+
override def size = length
95+
override def length = if (numRangeElements < 0) fail() else numRangeElements
96+
97+
private def description = "%d %s %d by %s".format(start, if (isInclusive) "to" else "until", end, step)
98+
private def fail() = throw new IllegalArgumentException(description + ": seqs cannot contain more than Int.MaxValue elements.")
99+
private def validateMaxLength() {
100+
if (numRangeElements < 0)
101+
fail()
85102
}
86103

87-
/** @note Making foreach run as fast as a while loop is a challenge.
88-
* The key elements which I can observe making a difference are:
89-
*
90-
* - the inner loop should be as small as possible
91-
* - the inner loop should be monomorphic
92-
* - the inner loop should perform no boxing and no avoidable tests
93-
*
94-
* This is achieved by:
95-
*
96-
* - keeping initialization logic out of the inner loop
97-
* - dispatching to custom variations based on initial conditions
98-
* - tricking the compiler into always calling Function1#apply$mcVI$sp
99-
*
100-
* The last one is important and less than obvious. Even when foreach
101-
* was specialized on Unit, only Int => Unit arguments benefited from it.
102-
* Other function types would be accepted, but in the absence of full
103-
* specialization the integer argument was boxed on every call. For example:
104-
*
105-
class A {
106-
final def f(x: Int): Int = x + 1
107-
// Calls Range.foreach, which calls Function1.apply
108-
def g1 = 1 until 100 foreach { x => f(x) }
109-
// Calls Range.foreach$mVc$sp, which calls Function1.apply$mcVI$sp
110-
def g2 = 1 until 100 foreach { x => f(x) ; () }
111-
}
112-
*
113-
* However! Since the result of the closure is always discarded, we
114-
* simply cast it to Int => Unit, thereby executing the fast version.
115-
* The seemingly looming ClassCastException can never arrive.
116-
*/
117-
@inline final override def foreach[U](f: Int => U) {
118-
if (step < 0) {
119-
if (isInclusive) foreachDownIn(f.asInstanceOf[Int => Unit])
120-
else foreachDownEx(f.asInstanceOf[Int => Unit])
121-
}
122-
else {
123-
if (isInclusive) foreachUpIn(f.asInstanceOf[Int => Unit])
124-
else foreachUpEx(f.asInstanceOf[Int => Unit])
104+
def validateRangeBoundaries(f: Int => Any): Boolean = {
105+
validateMaxLength()
106+
107+
start != Int.MinValue || end != Int.MinValue || {
108+
var count = 0
109+
var num = start
110+
while (count < numRangeElements) {
111+
f(num)
112+
count += 1
113+
num += step
114+
}
115+
false
125116
}
126117
}
127118

128-
/** !!! These methods must be public or they will not be inlined.
129-
* But they are certainly not intended to be part of the API.
130-
* This collision between inlining requirements and access semantics
131-
* is highly unfortunate and must be resolved.
132-
*
133-
* Proposed band-aid: an @internal annotation.
134-
*/
135-
@inline final def foreachDownIn(f: Int => Unit) {
136-
var i = start
137-
while (i >= end) {
138-
f(i)
139-
i += step
140-
}
141-
}
142-
@inline final def foreachUpIn(f: Int => Unit) {
143-
var i = start
144-
while (i <= end) {
145-
f(i)
146-
i += step
147-
}
119+
@inline final def apply(idx: Int): Int = {
120+
validateMaxLength()
121+
if (idx < 0 || idx >= numRangeElements) throw new IndexOutOfBoundsException(idx.toString)
122+
else start + (step * idx)
148123
}
149-
@inline final def foreachDownEx(f: Int => Unit) {
150-
var i = start
151-
while (i > end) {
152-
f(i)
153-
i += step
154-
}
155-
}
156-
@inline final def foreachUpEx(f: Int => Unit) {
157-
var i = start
158-
while (i < end) {
159-
f(i)
160-
i += step
124+
125+
@inline final override def foreach[@specialized(Unit) U](f: Int => U) {
126+
if (validateRangeBoundaries(f)) {
127+
var i = start
128+
val terminal = terminalElement
129+
val step = this.step
130+
while (i != terminal) {
131+
f(i)
132+
i += step
133+
}
161134
}
162135
}
163136

@@ -169,8 +142,8 @@ extends collection.AbstractSeq[Int]
169142
* @return a new range consisting of `n` first elements.
170143
*/
171144
final override def take(n: Int): Range = (
172-
if (n <= 0 || length == 0) newEmptyRange(start)
173-
else if (n >= length) this
145+
if (n <= 0 || isEmpty) newEmptyRange(start)
146+
else if (n >= numRangeElements) this
174147
else new Range.Inclusive(start, locationAfterN(n - 1), step)
175148
)
176149

@@ -182,8 +155,8 @@ extends collection.AbstractSeq[Int]
182155
* @return a new range consisting of all the elements of this range except `n` first elements.
183156
*/
184157
final override def drop(n: Int): Range = (
185-
if (n <= 0 || length == 0) this
186-
else if (n >= length) newEmptyRange(end)
158+
if (n <= 0 || isEmpty) this
159+
else if (n >= numRangeElements) newEmptyRange(end)
187160
else copy(locationAfterN(n), end, step)
188161
)
189162

@@ -218,15 +191,15 @@ extends collection.AbstractSeq[Int]
218191
var current = start
219192
var counted = 0
220193

221-
while (counted < length && p(current)) {
194+
while (counted < numRangeElements && p(current)) {
222195
counted += 1
223196
current += step
224197
}
225198
counted
226199
}
227200
// Tests whether a number is within the endpoints, without testing
228201
// whether it is a member of the sequence (i.e. when step > 1.)
229-
private def isWithinBoundaries(elem: Int) = (length > 0) && (
202+
private def isWithinBoundaries(elem: Int) = !isEmpty && (
230203
(step > 0 && start <= elem && elem <= last ) ||
231204
(step < 0 && last <= elem && elem <= start)
232205
)
@@ -255,21 +228,21 @@ extends collection.AbstractSeq[Int]
255228
*
256229
* $doesNotUseBuilders
257230
*/
258-
final override def takeRight(n: Int): Range = drop(length - n)
231+
final override def takeRight(n: Int): Range = drop(numRangeElements - n)
259232

260233
/** Creates a new range consisting of the initial `length - n` elements of the range.
261234
*
262235
* $doesNotUseBuilders
263236
*/
264-
final override def dropRight(n: Int): Range = take(length - n)
237+
final override def dropRight(n: Int): Range = take(numRangeElements - n)
265238

266239
/** Returns the reverse of this range.
267240
*
268241
* $doesNotUseBuilders
269242
*/
270243
final override def reverse: Range =
271-
if (length > 0) new Range.Inclusive(last, start, -step)
272-
else this
244+
if (isEmpty) this
245+
else new Range.Inclusive(last, start, -step)
273246

274247
/** Make range inclusive.
275248
*/
@@ -280,10 +253,9 @@ extends collection.AbstractSeq[Int]
280253
final def contains(x: Int) = isWithinBoundaries(x) && ((x - start) % step == 0)
281254

282255
final override def sum[B >: Int](implicit num: Numeric[B]): Int = {
283-
val len = length
284-
if (len == 0) 0
285-
else if (len == 1) head
286-
else (len.toLong * (head + last) / 2).toInt
256+
if (isEmpty) 0
257+
else if (numRangeElements == 1) head
258+
else (numRangeElements.toLong * (head + last) / 2).toInt
287259
}
288260

289261
override def toIterable = this
@@ -293,7 +265,7 @@ extends collection.AbstractSeq[Int]
293265
override def equals(other: Any) = other match {
294266
case x: Range =>
295267
(x canEqual this) && (length == x.length) && (
296-
(length == 0) || // all empty sequences are equal
268+
isEmpty || // all empty sequences are equal
297269
(start == x.start && last == x.last) // same length and same endpoints implies equality
298270
)
299271
case _ =>
@@ -304,7 +276,7 @@ extends collection.AbstractSeq[Int]
304276
*/
305277

306278
override def toString() = {
307-
val endStr = if (length > Range.MAX_PRINT) ", ... )" else ")"
279+
val endStr = if (numRangeElements > Range.MAX_PRINT) ", ... )" else ")"
308280
take(Range.MAX_PRINT).mkString("Range(", ", ", endStr)
309281
}
310282
}
@@ -415,3 +387,4 @@ object Range {
415387
// super.foreach(f)
416388
}
417389
}
390+

0 commit comments

Comments
 (0)