Skip to content

Commit 0729b29

Browse files
committed
Merge pull request scala#3801 from Ichoran/issue/8474
SI-8474 Inconsistent behavior of patch method
2 parents 068237a + 3d64dee commit 0729b29

File tree

2 files changed

+47
-16
lines changed

2 files changed

+47
-16
lines changed

src/library/scala/collection/Iterator.scala

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1088,6 +1088,9 @@ trait Iterator[+A] extends TraversableOnce[A] {
10881088
}
10891089

10901090
/** Returns this iterator with patched values.
1091+
* Patching at negative indices is the same as patching starting at 0.
1092+
* Patching at indices at or larger than the length of the original iterator appends the patch to the end.
1093+
* If more values are replaced than actually exist, the excess is ignored.
10911094
*
10921095
* @param from The start index from which to patch
10931096
* @param patchElems The iterator of patch values
@@ -1096,18 +1099,33 @@ trait Iterator[+A] extends TraversableOnce[A] {
10961099
*/
10971100
def patch[B >: A](from: Int, patchElems: Iterator[B], replaced: Int): Iterator[B] = new AbstractIterator[B] {
10981101
private var origElems = self
1099-
private var i = 0
1100-
def hasNext: Boolean =
1101-
if (i < from) origElems.hasNext
1102-
else patchElems.hasNext || origElems.hasNext
1102+
private var i = (if (from > 0) from else 0) // Counts down, switch to patch on 0, -1 means use patch first
1103+
def hasNext: Boolean = {
1104+
if (i == 0) {
1105+
origElems = origElems drop replaced
1106+
i = -1
1107+
}
1108+
origElems.hasNext || patchElems.hasNext
1109+
}
11031110
def next(): B = {
1104-
// We have to do this *first* just in case from = 0.
1105-
if (i == from) origElems = origElems drop replaced
1106-
val result: B =
1107-
if (i < from || !patchElems.hasNext) origElems.next()
1108-
else patchElems.next()
1109-
i += 1
1110-
result
1111+
if (i == 0) {
1112+
origElems = origElems drop replaced
1113+
i = -1
1114+
}
1115+
if (i < 0) {
1116+
if (patchElems.hasNext) patchElems.next()
1117+
else origElems.next()
1118+
}
1119+
else {
1120+
if (origElems.hasNext) {
1121+
i -= 1
1122+
origElems.next()
1123+
}
1124+
else {
1125+
i = -1
1126+
patchElems.next()
1127+
}
1128+
}
11111129
}
11121130
}
11131131

src/library/scala/collection/SeqViewLike.scala

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -154,17 +154,27 @@ trait SeqViewLike[+A,
154154
}
155155
}
156156

157+
// Note--for this to work, must ensure 0 <= from and 0 <= replaced
158+
// Must also take care to allow patching inside an infinite stream
159+
// (patching in an infinite stream is not okay)
157160
trait Patched[B >: A] extends Transformed[B] {
158161
protected[this] val from: Int
159162
protected[this] val patch: GenSeq[B]
160163
protected[this] val replaced: Int
161164
private lazy val plen = patch.length
162165
override def iterator: Iterator[B] = self.iterator patch (from, patch.iterator, replaced)
163-
def length: Int = self.length + plen - replaced
164-
def apply(idx: Int): B =
165-
if (idx < from) self.apply(idx)
166-
else if (idx < from + plen) patch.apply(idx - from)
166+
def length: Int = {
167+
val len = self.length
168+
val pre = math.min(from, len)
169+
val post = math.max(0, len - pre - replaced)
170+
pre + plen + post
171+
}
172+
def apply(idx: Int): B = {
173+
val actualFrom = if (self.lengthCompare(from) < 0) self.length else from
174+
if (idx < actualFrom) self.apply(idx)
175+
else if (idx < actualFrom + plen) patch.apply(idx - actualFrom)
167176
else self.apply(idx - plen + replaced)
177+
}
168178
final override protected[this] def viewIdentifier = "P"
169179
}
170180

@@ -210,7 +220,10 @@ trait SeqViewLike[+A,
210220
override def reverse: This = newReversed.asInstanceOf[This]
211221

212222
override def patch[B >: A, That](from: Int, patch: GenSeq[B], replaced: Int)(implicit bf: CanBuildFrom[This, B, That]): That = {
213-
newPatched(from, patch, replaced).asInstanceOf[That]
223+
// Be careful to not evaluate the entire sequence! Patch should work (slowly, perhaps) on infinite streams.
224+
val nonNegFrom = math.max(0,from)
225+
val nonNegRep = math.max(0,replaced)
226+
newPatched(nonNegFrom, patch, nonNegRep).asInstanceOf[That]
214227
// was: val b = bf(repr)
215228
// if (b.isInstanceOf[NoBuilder[_]]) newPatched(from, patch, replaced).asInstanceOf[That]
216229
// else super.patch[B, That](from, patch, replaced)(bf)

0 commit comments

Comments
 (0)