From 1ef2574fa7c61ac6bd2f1c38c663a4e2dec5d887 Mon Sep 17 00:00:00 2001 From: odersky Date: Fri, 1 Mar 2024 20:15:23 +0100 Subject: [PATCH 1/2] Drop FreeSeqFactory from stdlib-cc FreeSeqFactory was a construction to demonstrate type safety for certain iterableFactory.from calls where we rely in the fact that for all Seqs iterableFactory has an eager implementation of from. While that shows that we _can_ make it typesafe, it does not work at runtime as a drop-in replacement for stdlib since of course stdlib does not have a FreeSeqFactory. This commit drop FreeSeqFactory and adds three unsafeAssumePure calls instead, with explanations. Fixes #19745 --- scala2-library-cc/src/scala/collection/Factory.scala | 12 ++++++++---- scala2-library-cc/src/scala/collection/Seq.scala | 8 ++++---- .../src/scala/collection/generic/IsSeq.scala | 4 ++-- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/scala2-library-cc/src/scala/collection/Factory.scala b/scala2-library-cc/src/scala/collection/Factory.scala index 5392d7e2c642..d66ab08e4abb 100644 --- a/scala2-library-cc/src/scala/collection/Factory.scala +++ b/scala2-library-cc/src/scala/collection/Factory.scala @@ -297,16 +297,20 @@ object IterableFactory { } // !!! Needed to add this separate trait -trait FreeSeqFactory[+CC[A]] extends IterableFactory[CC]: - def from[A](source: IterableOnce[A]^): CC[A] - override def apply[A](elems: A*): CC[A] = from(elems) +//trait FreeSeqFactory[+CC[A]] extends IterableFactory[CC]: +// def from[A](source: IterableOnce[A]^): CC[A] +// override def apply[A](elems: A*): CC[A] + +// type FreeSeqFactory[+CC[A] <: SeqOps[A, Seq, Seq[A]]] = SeqFactory[CC] /** * @tparam CC Collection type constructor (e.g. `List`) */ -trait SeqFactory[+CC[A] <: SeqOps[A, Seq, Seq[A]]] extends FreeSeqFactory[CC] { +trait SeqFactory[+CC[A] <: SeqOps[A, Seq, Seq[A]]] extends IterableFactory[CC] { import SeqFactory.UnapplySeqWrapper final def unapplySeq[A](x: CC[A] @uncheckedVariance): UnapplySeqWrapper[A] = new UnapplySeqWrapper(x) // TODO is uncheckedVariance sound here? + def from[A](source: IterableOnce[A]^): CC[A] + override def apply[A](elems: A*): CC[A] = from(elems) } object SeqFactory { diff --git a/scala2-library-cc/src/scala/collection/Seq.scala b/scala2-library-cc/src/scala/collection/Seq.scala index 334546d67dad..65927154c4b6 100644 --- a/scala2-library-cc/src/scala/collection/Seq.scala +++ b/scala2-library-cc/src/scala/collection/Seq.scala @@ -81,8 +81,6 @@ trait SeqOps[+A, +CC[_], +C] extends Any with SeqViewOps[A, CC, C] { self => override def view: SeqView[A] = new SeqView.Id[A](this) - def iterableFactory: FreeSeqFactory[CC] - /** Get the element at the specified index. This operation is provided for convenience in `Seq`. It should * not be assumed to be efficient unless you have an `IndexedSeq`. */ @throws[IndexOutOfBoundsException] @@ -167,7 +165,7 @@ trait SeqOps[+A, +CC[_], +C] extends Any with SeqViewOps[A, CC, C] { self => def prependedAll[B >: A](prefix: IterableOnce[B]^): CC[B] = iterableFactory.from(prefix match { case prefix: Iterable[B] => new View.Concat(prefix, this) case _ => prefix.iterator ++ iterator - }) + }).unsafeAssumePure // assume pure OK since iterableFactory.from is eager for Seq /** Alias for `prependedAll` */ @`inline` override final def ++: [B >: A](prefix: IterableOnce[B]^): CC[B] = prependedAll(prefix) @@ -532,6 +530,7 @@ trait SeqOps[+A, +CC[_], +C] extends Any with SeqViewOps[A, CC, C] { self => @deprecated("Use .reverseIterator.map(f).to(...) instead of .reverseMap(f)", "2.13.0") def reverseMap[B](f: A => B): CC[B] = iterableFactory.from(new View.Map(View.fromIteratorProvider(() => reverseIterator), f)) + .unsafeAssumePure // assume pure OK since iterableFactory.from is eager for Seq /** Iterates over distinct permutations of elements. * @@ -947,7 +946,8 @@ trait SeqOps[+A, +CC[_], +C] extends Any with SeqViewOps[A, CC, C] { self => */ def patch[B >: A](from: Int, other: IterableOnce[B]^, replaced: Int): CC[B] = iterableFactory.from(new View.Patched(this, from, other, replaced)) - + .unsafeAssumePure // assume pure OK since iterableFactory.from is eager for Seq + /** A copy of this $coll with one single replaced element. * @param index the position of the replacement * @param elem the replacing element diff --git a/scala2-library-cc/src/scala/collection/generic/IsSeq.scala b/scala2-library-cc/src/scala/collection/generic/IsSeq.scala index 041d74f84d9c..7b5e50499c0c 100644 --- a/scala2-library-cc/src/scala/collection/generic/IsSeq.scala +++ b/scala2-library-cc/src/scala/collection/generic/IsSeq.scala @@ -84,7 +84,7 @@ object IsSeq { def toIterable: Iterable[Char] = new immutable.WrappedString(s) protected[this] def coll: String = s protected[this] def fromSpecific(coll: IterableOnce[Char]^): String = coll.iterator.mkString - def iterableFactory: FreeSeqFactory[immutable.ArraySeq] = immutable.ArraySeq.untagged + def iterableFactory: IterableFactory[immutable.ArraySeq] = immutable.ArraySeq.untagged override def empty: String = "" protected[this] def newSpecificBuilder: mutable.Builder[Char, String] = new StringBuilder def iterator: Iterator[Char] = s.iterator @@ -102,7 +102,7 @@ object IsSeq { def toIterable: Iterable[A] = mutable.ArraySeq.make[A](a) protected def coll: Array[A] = a protected def fromSpecific(coll: IterableOnce[A]^): Array[A] = Array.from(coll) - def iterableFactory: FreeSeqFactory[mutable.ArraySeq] = mutable.ArraySeq.untagged + def iterableFactory: IterableFactory[mutable.ArraySeq] = mutable.ArraySeq.untagged override def empty: Array[A] = Array.empty[A] protected def newSpecificBuilder: mutable.Builder[A, Array[A]] = Array.newBuilder def iterator: Iterator[A] = a.iterator From 4464a73f7b016366be5ac06283223a1fddeb043f Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 2 Mar 2024 11:12:07 +0100 Subject: [PATCH 2/2] Drop override in SeqFactory Another method that was added for the typesafety demonstration, which should be removed now. --- scala2-library-cc/src/scala/collection/Factory.scala | 9 --------- 1 file changed, 9 deletions(-) diff --git a/scala2-library-cc/src/scala/collection/Factory.scala b/scala2-library-cc/src/scala/collection/Factory.scala index d66ab08e4abb..99f584b972fc 100644 --- a/scala2-library-cc/src/scala/collection/Factory.scala +++ b/scala2-library-cc/src/scala/collection/Factory.scala @@ -296,21 +296,12 @@ object IterableFactory { } } -// !!! Needed to add this separate trait -//trait FreeSeqFactory[+CC[A]] extends IterableFactory[CC]: -// def from[A](source: IterableOnce[A]^): CC[A] -// override def apply[A](elems: A*): CC[A] - -// type FreeSeqFactory[+CC[A] <: SeqOps[A, Seq, Seq[A]]] = SeqFactory[CC] - /** * @tparam CC Collection type constructor (e.g. `List`) */ trait SeqFactory[+CC[A] <: SeqOps[A, Seq, Seq[A]]] extends IterableFactory[CC] { import SeqFactory.UnapplySeqWrapper final def unapplySeq[A](x: CC[A] @uncheckedVariance): UnapplySeqWrapper[A] = new UnapplySeqWrapper(x) // TODO is uncheckedVariance sound here? - def from[A](source: IterableOnce[A]^): CC[A] - override def apply[A](elems: A*): CC[A] = from(elems) } object SeqFactory {