Fixes from scala/scala repository #493
Conversation
|
I’m wondering what happened to scala/scala#5664, it seems to have been re 8000 verted (scala/scala@a11918d, scala/scala@76babbb and scala/scala@0365d58). Also, it seems that we have a similar PR (#354). What should we do about this stuff? |
|
scala/scala#5664 was partially reverted because it was binary incompatible. We don't need to worry about that here. |
|
#354 is the intended replacement for the parts in scala/scala#5664 that had to be reverted due to binary compatibility constraints. |
| * @return a $coll consisting only of the first `n` elements of this $coll, | ||
| * or else the whole $coll, if it has less than `n` elements. | ||
| * If `n` is negative, returns an empty $coll. | ||
| */ |
There was a problem hiding this comment.
The doc comments for take and drop should be in IterableOnceOps
There was a problem hiding this comment.
I’ve put them here to override IterableOnce’s comments which give information about the fact that these operation consume the Iterator. This information is not relevant in Iterable.
But maybe we could organize the scaladoc differently…
There was a problem hiding this comment.
The iterator comsumption stuff is contained in templates (or whatever you call those scaladoc variables). They should be defined as empty in Iterable and with the relevant text in Iterator.
| @@ -171,4 +171,8 @@ trait LinearSeqOps[+A, +CC[X] <: LinearSeq[X], +C <: LinearSeq[A]] extends Any w | |||
| } | |||
| last | |||
| } | |||
|
|
|||
| override def tails: Iterator[C] = | |||
| Iterator.iterate(coll)(_.tail.asInstanceOf[C]).takeWhile(_.nonEmpty) ++ Iterator(newSpecificBuilder().result()) | |||
There was a problem hiding this comment.
Yes, the static return type of tail is LinearSeq[A], whereas we want a C.
There was a problem hiding this comment.
Hm, that's not nice. We lose the precise type on repeated applications of tail. this.type.tail results in C but C.tail is only LinearSeq[A].
There was a problem hiding this comment.
Having a more precise type would require a more complex upper bound on C.
There was a problem hiding this comment.
The problem lies in the definition of LinearSeqOps. We need to make C recursive:
trait LinearSeqOps[+A, +CC[X] <: LinearSeq[X], +C <: LinearSeq[A] with LinearSeqOps[A, CC, C]] extends Any with SeqOps[A, CC, C]And the same for immutable.LinearSeqOps and LazyListOps. With these changes the cast is no longer needed.
There was a problem hiding this comment.
Yes, that’s what I was trying to say in my previous comment. The drawback is that it becomes harder to abstract over some LinearSeqOps. See how it is done for immutable.MapOps, which has similar bounds: https://github.com/scala/collection-strawman/blob/master/collections-contrib/src/main/scala/strawman/collection/decorators/HasImmutableMapOps.scala#L12-L17
There was a problem hiding this comment.
I don't think this is a problem for LinearSeqOps. The only changes I had to do downstream was to repeat the bounds in immutable.LinearSeqOps and LazyListOps. LinearSeqOps is just an implementation detail. You'd abstract over SeqOps, possibly restricted to subtypes of LinearSeq if you care about performance characteristics.
There was a problem hiding this comment.
Allright, I’ve fixed that in a304aaf.
| /** Splits a sequence into init :+ last. | ||
| * @return Some((init, last)) if sequence is non-empty. None otherwise. | ||
| /** Splits a sequence into init :+ tail. | ||
| * @return Some((init, tail)) if sequence is non-empty. None otherwise. |
There was a problem hiding this comment.
This is wrong. :+ does split the sequence into (init, last).
d049876 to
7a57af9
Compare
OK, so that’s fine to not have it in this PR. |
…y instance. Fixes scala/bug#10399 Backport of 34d2f332d48adfd77c3060f948197326528b40d9 (by @joymufeng)
Backport of 58be110785aaef46333d59856c5f12cb575f4159 (by @Philippus)
Backport of 41133d93f78911bc60ce5e7ce7c9a242ce1b23a8 (by @id-ilych)
Backport of e29bff586f4c819e5b1f03ed8a497ac4a245f5fa (by @markus1189)
Backport of 55541745edcbb7b1d5a9830bad880cf5f543c0fe (by @markus1189)
Backport of 836297f305bd6901b60c7410bce1be25725b6ce3 (by Steve Robinson)
Backport of 7de41a40ecb0c5fbb8450823dc08218d93f4dc81 (by Håkon Hjelde Wold)
Backport of ea65e04beef05708f0d11377338605377c86d3b6 (by @ashawley)
Backport of 37dd50c0f1204354ec147b445652cd725126e063 (by @DanielMoss)
Backport of 5c7b14219b746812089c39876edaba9ee226a4bb (by @som-snytt). Note that our implementation was already lazy enough, so I didn’t copy the implementation of the referenced commit.
Backport of 0ee887573512ecd6411ac656ca03fc43696fa710 (by @martijnhoekstra)
7a57af9 to
a304aaf
Compare
I went through the history of the collections on the scala/scala repo, for the last year, and backported all the things that were missing in the strawman.
Fixes #214.