Avoid inheriting suboptimal concat implementation for immutable collections

[odd]

Fixes scala/bug#11128 by separating the StrictOptimizedOps-family of traits into one set for mutable collections (which continue to use the builder based concat/appended/appendedAll/prepended/prependedAll/padTo implementations) and another set for the immutable collections (which does not have the builder based implementations since they have implementations inherited from e.g. immutable.SetOps that are more optimal). The root StrictOptimizedIterableOps-trait is still left in the collection package and is inherited by the specific implementations in the mutable/immutable subsets.

[lrytz]

Needs a rebase, and updates to some tests.

Looks good to me, but probably @szeiger should review this too.

[odd]

/rebuild

[szeiger]

Pinging @odd. This still has some test failures and also needs to be rebased.

[odd]

@szeiger The purpose of this PR was to avoid the performance degradation introduced by the StrictOptimizedOps-family of traits for persistent immutable collections that could offer better implementations by exploiting the persistent nature of their inner structure (i.e. avoiding the need to always build a new instance from scratch). To get to that goal I tried to separate thecollection.StrictOptimizedOps-family into mutable and immutable sub-families which were only connected at the base in collection.StrictOptimizedIterableOps, but this lead to strange lubs with structural types in some cases (e.g. the lub of List(List[Int], ListBuffer[Int])).

Instead I then started looking at the actual cases where a sub-optimal implementation from a StrictOptimized-trait was overriding a more appropriate implementation, which has so far resulted in PR #7454 and PR #7455. There might be more such cases however.

So, in conclusion, this PR might be a bit too far reaching (if the lubs containing structural types are seen as something to avoid), or perhaps it is not going far enough (by removing collection.StrictOptimizedIterableOps and instead adding a mutable and an immutable version, the StrictOptimizedOps-families become unrelated; this will lead to some code duplication but the lubs would probably be simpler). Any ideas for the best way of going forward are most welcome.

[szeiger]

Right, the lubs are ugly:

scala> def lub[A, B >: A](a: A, b: B): B = null.asInstanceOf[B]
lub: [A, B >: A](a: A, b: B)B

scala> :type lub(List(42), collection.mutable.ListBuffer(42))
scala.collection.AbstractSeq[Int] with scala.collection.StrictOptimizedIterableOps[Int,[_]scala.collection.AbstractSeq[_] with scala.collection.StrictOptimizedIterableOps[_,[_]scala.collection.AbstractSeq[_] with scala.collection.StrictOptimizedIterableOps[_,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[_]]{def appendedAll[B >: _](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prependedAll[B >: _](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prepended[B >: _](elem: B): scala.collection.AbstractSeq[B]},scala.collection.AbstractSeq[_] with scala.collection.StrictOptimizedIterableOps[_,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[_]]{def appendedAll[B >: _](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prependedAll[B >: _](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prepended[B >: _](elem: B): scala.collection.AbstractSeq[B]}]{def appendedAll[B >: _](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B] with scala.collection.StrictOptimizedIterableOps[B,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[B]]{def appendedAll[B >: B](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prependedAll[B >: B](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prepended[B >: B](elem: B): scala.collection.AbstractSeq[B]}; def prependedAll[B >: _](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B] with scala.collection.StrictOptimizedIterableOps[B,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[B]]{def appendedAll[B >: B](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prependedAll[B >: B](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prepended[B >: B](elem: B): scala.collection.AbstractSeq[B]}; def prepended[B >: _](elem: B): scala.collection.AbstractSeq[B] with scala.collection.StrictOptimizedIterableOps[B,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[B]]{def appendedAll[B >: B](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prependedAll[B >: B](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prepended[B >: B](elem: B): scala.collection.AbstractSeq[B]}},scala.collection.AbstractSeq[Int] with scala.collection.StrictOptimizedIterableOps[Int,[_]scala.collection.AbstractSeq[_] with scala.collection.StrictOptimizedIterableOps[_,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[_]]{def appendedAll[B >: _](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prependedAll[B >: _](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prepended[B >: _](elem: B): scala.collection.AbstractSeq[B]},scala.collection.AbstractSeq[Int] with scala.collection.StrictOptimizedIterableOps[Int,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[Int]]{def appendedAll[B >: Int](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prependedAll[B >: Int](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prepended[B >: Int](elem: B): scala.collection.AbstractSeq[B]}]{def appendedAll[B >: Int](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B] with scala.collection.StrictOptimizedIterableOps[B,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[B]]{def appendedAll[B >: B](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prependedAll[B >: B](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prepended[B >: B](elem: B): scala.collection.AbstractSeq[B]}; def prependedAll[B >: Int](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B] with scala.collection.StrictOptimizedIterableOps[B,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[B]]{def appendedAll[B >: B](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prependedAll[B >: B](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prepended[B >: B](elem: B): scala.collection.AbstractSeq[B]}; def prepended[B >: Int](elem: B): scala.collection.AbstractSeq[B] with scala.collection.StrictOptimizedIterableOps[B,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[B]]{def appendedAll[B >: B](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prependedAll[B >: B](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prepended[B >: B](elem: B): scala.collection.AbstractSeq[B]}}]{def appendedAll[B >: Int](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B] with scala.collection.StrictOptimizedIterableOps[B,[_]scala.collection.AbstractSeq[_] with scala.collection.StrictOptimizedIterableOps[_,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[_]]{def appendedAll[B >: _](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prependedAll[B >: _](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prepended[B >: _](elem: B): scala.collection.AbstractSeq[B]},scala.collection.AbstractSeq[B] with scala.collection.StrictOptimizedIterableOps[B,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[B]]{def appendedAll[B >: B](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prependedAll[B >: B](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prepended[B >: B](elem: B): scala.collection.AbstractSeq[B]}]{def appendedAll[B >: B](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B] with scala.collection.StrictOptimizedIterableOps[B,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[B]]{def appendedAll[B >: B](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prependedAll[B >: B](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prepended[B >: B](elem: B): scala.collection.AbstractSeq[B]}; def prependedAll[B >: B](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B] with scala.collection.StrictOptimizedIterableOps[B,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[B]]{def appendedAll[B >: B](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prependedAll[B >: B](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prepended[B >: B](elem: B): scala.collection.AbstractSeq[B]}; def prepended[B >: B](elem: B): scala.collection.AbstractSeq[B] with scala.collection.StrictOptimizedIterableOps[B,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[B]]{def appendedAll[B >: B](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prependedAll[B >: B](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prepended[B >: B](elem: B): scala.collection.AbstractSeq[B]}}; def prependedAll[B >: Int](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B] with scala.collection.StrictOptimizedIterableOps[B,[_]scala.collection.AbstractSeq[_] with scala.collection.StrictOptimizedIterableOps[_,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[_]]{def appendedAll[B >: _](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prependedAll[B >: _](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prepended[B >: _](elem: B): scala.collection.AbstractSeq[B]},scala.collection.AbstractSeq[B] with scala.collection.StrictOptimizedIterableOps[B,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[B]]{def appendedAll[B >: B](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prependedAll[B >: B](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prepended[B >: B](elem: B): scala.collection.AbstractSeq[B]}]{def appendedAll[B >: B](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B] with scala.collection.StrictOptimizedIterableOps[B,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[B]]{def appendedAll[B >: B](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prependedAll[B >: B](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prepended[B >: B](elem: B): scala.collection.AbstractSeq[B]}; def prependedAll[B >: B](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B] with scala.collection.StrictOptimizedIterableOps[B,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[B]]{def appendedAll[B >: B](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prependedAll[B >: B](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prepended[B >: B](elem: B): scala.collection.AbstractSeq[B]}; def prepended[B >: B](elem: B): scala.collection.AbstractSeq[B] with scala.collection.StrictOptimizedIterableOps[B,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[B]]{def appendedAll[B >: B](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prependedAll[B >: B](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prepended[B >: B](elem: B): scala.collection.AbstractSeq[B]}}; def prepended[B >: Int](elem: B): scala.collection.AbstractSeq[B] with scala.collection.StrictOptimizedIterableOps[B,[_]scala.collection.AbstractSeq[_] with scala.collection.StrictOptimizedIterableOps[_,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[_]]{def appendedAll[B >: _](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prependedAll[B >: _](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prepended[B >: _](elem: B): scala.collection.AbstractSeq[B]},scala.collection.AbstractSeq[B] with scala.collection.StrictOptimizedIterableOps[B,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[B]]{def appendedAll[B >: B](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prependedAll[B >: B](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prepended[B >: B](elem: B): scala.collection.AbstractSeq[B]}]{def appendedAll[B >: B](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B] with scala.collection.StrictOptimizedIterableOps[B,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[B]]{def appendedAll[B >: B](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prependedAll[B >: B](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prepended[B >: B](elem: B): scala.collection.AbstractSeq[B]}; def prependedAll[B >: B](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B] with scala.collection.StrictOptimizedIterableOps[B,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[B]]{def appendedAll[B >: B](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prependedAll[B >: B](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prepended[B >: B](elem: B): scala.collection.AbstractSeq[B]}; def prepended[B >: B](elem: B): scala.collection.AbstractSeq[B] with scala.collection.StrictOptimizedIterableOps[B,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[B]]{def appendedAll[B >: B](suffix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prependedAll[B >: B](prefix: scala.collection.IterableOnce[B]): scala.collection.AbstractSeq[B]; def prepended[B >: B](elem: B): scala.collection.AbstractSeq[B]}}}

This is what you get without these changes:

scala> :type lub(List(42), collection.mutable.ListBuffer(42))
scala.collection.AbstractSeq[Int] with scala.collection.StrictOptimizedSeqOps[Int,[_]scala.collection.AbstractSeq[_] with scala.collection.StrictOptimizedSeqOps[_,[_]scala.collection.AbstractSeq[_] with scala.collection.StrictOptimizedSeqOps[_,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[_]],scala.collection.AbstractSeq[_] with scala.collection.StrictOptimizedSeqOps[_,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[_]]],scala.collection.AbstractSeq[Int] with scala.collection.StrictOptimizedSeqOps[Int,[_]scala.collection.AbstractSeq[_] with scala.collection.StrictOptimizedSeqOps[_,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[_]],scala.collection.AbstractSeq[Int] with scala.collection.StrictOptimizedSeqOps[Int,[_]scala.collection.AbstractSeq[_],scala.collection.AbstractSeq[Int]]]]

Is there a ticket for this or some discussion why they occur? The traits contain only overrides and List and ListBuffer already contain several shared and several non-shared traits. It's not clear to me why this change would lead to structural types in lubs.

[lrytz]

There's scala/bug#10908

[szeiger]

I'm closing this because we don't want to introduce more ugly lubs and there's already a ticket for that (and alternative fixes for the issue at hand).