diff options
author | Paul Phillips <paulp@improving.org> | 2014-02-06 14:12:18 -0800 |
---|---|---|
committer | Paul Phillips <paulp@improving.org> | 2014-02-06 15:16:47 -0800 |
commit | 51ec62a8c307cd66fe34fe07c85100d48b54aa8f (patch) | |
tree | d39c1fc16b175c1b0d93127a8ae06d8a80bbec7e /src/library | |
parent | f7870863c28d163d1016418783a1169fbd6022d0 (diff) | |
download | scala-51ec62a8c307cd66fe34fe07c85100d48b54aa8f.tar.gz scala-51ec62a8c307cd66fe34fe07c85100d48b54aa8f.tar.bz2 scala-51ec62a8c307cd66fe34fe07c85100d48b54aa8f.zip |
SI-6948 Make the Abstract* classes public.
Several weaknesses in the implementation converge and force multiply.
1) Type constructor inference is not persistent. During implicit search
it will give up after the first seen parent even if some deeper base type
(even another direct parent) would satisfy the search.
2) Type inference is not aware of access restrictions. Inferred types are
calculated with disregard for whether the inferred type is visible at
the point of inference. That means that package-private types - which may be
private for any number of good reasons, such as not wanting them to appear in
bytecode thus creating binary compatibility obligations - are not private.
There is no such thing as a qualified private type.
package p {
trait PublicInterface[T] { def foo(): Int }
private[p] trait ImplementationOnly[T] extends PublicInterface[T] { def foo(): Int = 1 }
class PublicClass extends ImplementationOnly[PublicClass]
}
package q {
object Test {
def f[A, CC[X]](xs: CC[A]): CC[A] = xs
def g = f(new p.PublicClass) // inferred type: p.ImplementationOnly[p.PublicClass]
def h = g.foo()
// Bytecode contains:
// public p.ImplementationOnly<p.PublicClass> g();
// public int h();
// 0: aload_0
// 1: invokevirtual #30 // Method g:()Lp/ImplementationOnly;
// 4: invokeinterface #33, 1 // InterfaceMethod p/ImplementationOnly.foo:()I
// 9: ireturn
}
}
3) The trait encoding leads to a proliferation of forwarder methods, so much so that
1.5 Mb of bytecode was taken off of the standard library size by creating abstract classes
which act as central mixin points so that leaf classes can inherit some methods the
old fashioned way rather than each receiving their own copy of every trait defined method.
This was done for 2.10 through the creation of the Abstract* classes, all of which were
given reduced visibility to keep them out of the API.
private[collection] class AbstractSeq extends ...
This achieved its intended goal very nicely, but also some unintended ones.
In combination with 1) above:
scala> val rand = new scala.util.Random()
rand: scala.util.Random = scala.util.Random@7f85a53b
// this works
scala> rand.shuffle(0 to 5)
res1: scala.collection.immutable.IndexedSeq[Int] = Vector(4, 0, 1, 2, 5, 3)
// and this doesn't! good luck reasoning that one out
scala> rand.shuffle(0 until 5)
<console>:9: error: Cannot construct a collection of type scala.collection.AbstractSeq[Int]
with elements of type Int based on a collection of type scala.collection.AbstractSeq[Int].
rand.shuffle(0 until 5)
^
// Somewhat comically, in scala 2.9 it was flipped: to failed (differently), until worked.
scala> scala.util.Random.shuffle(0 to 5)
<console>:8: error: type mismatch;
found : scala.collection.immutable.Range.Inclusive
required: ?CC[?T]
scala> scala.util.Random.shuffle(0 until 5)
res2: scala.collection.immutable.IndexedSeq[Int] = Vector(4, 3, 1, 2, 0)
In combination with 2) above:
scala> def f[A, CC[X]](xs: CC[A]): CC[A] = xs
f: [A, CC[X]](xs: CC[A])CC[A]
scala> var x = f(1 until 10)
x: scala.collection.AbstractSeq[Int] = Range(1, 2, 3, 4, 5, 6, 7, 8, 9)
// It has inferred a type for our value which it will not allow us to use or even to reference.
scala> var y: scala.collection.AbstractSeq[Int] = x
<console>:10: error: class AbstractSeq in package collection cannot be accessed in package collection
var y: scala.collection.AbstractSeq[Int] = x
^
// This one is a straight regression - in scala 2.9,
scala> var x = f(1 until 10)
x: scala.collection.immutable.IndexedSeq[Int] = Range(1, 2, 3, 4, 5, 6, 7, 8, 9)
Since 1) and 2) are essentially unfixable - at least by me - I propose
to ameliorate these regressions by attacking the symptoms at the leaves.
That means making all the Abstract* classes public - keeping in mind that
they must already be assumed to be in the binary compatibility footprint,
since they have been leaking throughout their existence. This only impacts
the inference of inaccessible collections types - it doesn't help with the
more serious issue with type inference.
Diffstat (limited to 'src/library')
-rw-r--r-- | src/library/scala/collection/Iterable.scala | 2 | ||||
-rw-r--r-- | src/library/scala/collection/Iterator.scala | 2 | ||||
-rw-r--r-- | src/library/scala/collection/Map.scala | 2 | ||||
-rw-r--r-- | src/library/scala/collection/Seq.scala | 2 | ||||
-rw-r--r-- | src/library/scala/collection/Set.scala | 2 | ||||
-rw-r--r-- | src/library/scala/collection/Traversable.scala | 2 | ||||
-rw-r--r-- | src/library/scala/collection/immutable/Map.scala | 6 | ||||
-rw-r--r-- | src/library/scala/collection/mutable/Buffer.scala | 2 | ||||
-rw-r--r-- | src/library/scala/collection/mutable/Iterable.scala | 2 | ||||
-rw-r--r-- | src/library/scala/collection/mutable/Map.scala | 2 | ||||
-rw-r--r-- | src/library/scala/collection/mutable/Seq.scala | 2 | ||||
-rw-r--r-- | src/library/scala/collection/mutable/Set.scala | 2 |
12 files changed, 14 insertions, 14 deletions
diff --git a/src/library/scala/collection/Iterable.scala b/src/library/scala/collection/Iterable.scala index 973efc447e..a5ab8efd5c 100644 --- a/src/library/scala/collection/Iterable.scala +++ b/src/library/scala/collection/Iterable.scala @@ -51,4 +51,4 @@ object Iterable extends TraversableFactory[Iterable] { } /** Explicit instantiation of the `Iterable` trait to reduce class file size in subclasses. */ -private[scala] abstract class AbstractIterable[+A] extends AbstractTraversable[A] with Iterable[A] +abstract class AbstractIterable[+A] extends AbstractTraversable[A] with Iterable[A] diff --git a/src/library/scala/collection/Iterator.scala b/src/library/scala/collection/Iterator.scala index 72a23a0dd0..01a0aa3b51 100644 --- a/src/library/scala/collection/Iterator.scala +++ b/src/library/scala/collection/Iterator.scala @@ -1171,4 +1171,4 @@ trait Iterator[+A] extends TraversableOnce[A] { } /** Explicit instantiation of the `Iterator` trait to reduce class file size in subclasses. */ -private[scala] abstract class AbstractIterator[+A] extends Iterator[A] +abstract class AbstractIterator[+A] extends Iterator[A] diff --git a/src/library/scala/collection/Map.scala b/src/library/scala/collection/Map.scala index 761b65723c..1e40fd8c24 100644 --- a/src/library/scala/collection/Map.scala +++ b/src/library/scala/collection/Map.scala @@ -56,4 +56,4 @@ object Map extends MapFactory[Map] { } /** Explicit instantiation of the `Map` trait to reduce class file size in subclasses. */ -private[scala] abstract class AbstractMap[A, +B] extends AbstractIterable[(A, B)] with Map[A, B] +abstract class AbstractMap[A, +B] extends AbstractIterable[(A, B)] with Map[A, B] diff --git a/src/library/scala/collection/Seq.scala b/src/library/scala/collection/Seq.scala index b21acdd9b7..2f4b3e5f8a 100644 --- a/src/library/scala/collection/Seq.scala +++ b/src/library/scala/collection/Seq.scala @@ -38,4 +38,4 @@ object Seq extends SeqFactory[Seq] { } /** Explicit instantiation of the `Seq` trait to reduce class file size in subclasses. */ -private[scala] abstract class AbstractSeq[+A] extends AbstractIterable[A] with Seq[A] +abstract class AbstractSeq[+A] extends AbstractIterable[A] with Seq[A] diff --git a/src/library/scala/collection/Set.scala b/src/library/scala/collection/Set.scala index 46d5dfa056..f74c26571a 100644 --- a/src/library/scala/collection/Set.scala +++ b/src/library/scala/collection/Set.scala @@ -44,4 +44,4 @@ object Set extends SetFactory[Set] { } /** Explicit instantiation of the `Set` trait to reduce class file size in subclasses. */ -private[scala] abstract class AbstractSet[A] extends AbstractIterable[A] with Set[A] +abstract class AbstractSet[A] extends AbstractIterable[A] with Set[A] diff --git a/src/library/scala/collection/Traversable.scala b/src/library/scala/collection/Traversable.scala index 61d9a42f04..b53724c568 100644 --- a/src/library/scala/collection/Traversable.scala +++ b/src/library/scala/collection/Traversable.scala @@ -101,4 +101,4 @@ object Traversable extends TraversableFactory[Traversable] { self => } /** Explicit instantiation of the `Traversable` trait to reduce class file size in subclasses. */ -private[scala] abstract class AbstractTraversable[+A] extends Traversable[A] +abstract class AbstractTraversable[+A] extends Traversable[A] diff --git a/src/library/scala/collection/immutable/Map.scala b/src/library/scala/collection/immutable/Map.scala index 8933c7cf77..5178d5a862 100644 --- a/src/library/scala/collection/immutable/Map.scala +++ b/src/library/scala/collection/immutable/Map.scala @@ -32,9 +32,9 @@ trait Map[A, +B] extends Iterable[(A, B)] with MapLike[A, B, Map[A, B]] { self => override def empty: Map[A, B] = Map.empty - + /** Returns this $coll as an immutable map. - * + * * A new map will not be built; lazy collections will stay lazy. */ @deprecatedOverriding("Immutable maps should do nothing on toMap except return themselves cast as a map.", "2.11.0") @@ -191,4 +191,4 @@ object Map extends ImmutableMapFactory[Map] { } /** Explicit instantiation of the `Map` trait to reduce class file size in subclasses. */ -private[scala] abstract class AbstractMap[A, +B] extends scala.collection.AbstractMap[A, B] with Map[A, B] +abstract class AbstractMap[A, +B] extends scala.collection.AbstractMap[A, B] with Map[A, B] diff --git a/src/library/scala/collection/mutable/Buffer.scala b/src/library/scala/collection/mutable/Buffer.scala index d2e33badbe..7ec7b06333 100644 --- a/src/library/scala/collection/mutable/Buffer.scala +++ b/src/library/scala/collection/mutable/Buffer.scala @@ -46,4 +46,4 @@ object Buffer extends SeqFactory[Buffer] { } /** Explicit instantiation of the `Buffer` trait to reduce class file size in subclasses. */ -private[scala] abstract class AbstractBuffer[A] extends AbstractSeq[A] with Buffer[A] +abstract class AbstractBuffer[A] extends AbstractSeq[A] with Buffer[A] diff --git a/src/library/scala/collection/mutable/Iterable.scala b/src/library/scala/collection/mutable/Iterable.scala index f7a794e357..92313c9ccd 100644 --- a/src/library/scala/collection/mutable/Iterable.scala +++ b/src/library/scala/collection/mutable/Iterable.scala @@ -38,4 +38,4 @@ object Iterable extends TraversableFactory[Iterable] { } /** Explicit instantiation of the `Iterable` trait to reduce class file size in subclasses. */ -private[scala] abstract class AbstractIterable[A] extends scala.collection.AbstractIterable[A] with Iterable[A] +abstract class AbstractIterable[A] extends scala.collection.AbstractIterable[A] with Iterable[A] diff --git a/src/library/scala/collection/mutable/Map.scala b/src/library/scala/collection/mutable/Map.scala index 01f6f725ab..fe29ce4221 100644 --- a/src/library/scala/collection/mutable/Map.scala +++ b/src/library/scala/collection/mutable/Map.scala @@ -89,4 +89,4 @@ object Map extends MutableMapFactory[Map] { } /** Explicit instantiation of the `Map` trait to reduce class file size in subclasses. */ -private[scala] abstract class AbstractMap[A, B] extends scala.collection.AbstractMap[A, B] with Map[A, B] +abstract class AbstractMap[A, B] extends scala.collection.AbstractMap[A, B] with Map[A, B] diff --git a/src/library/scala/collection/mutable/Seq.scala b/src/library/scala/collection/mutable/Seq.scala index 11fbdd13f3..eafde70a2d 100644 --- a/src/library/scala/collection/mutable/Seq.scala +++ b/src/library/scala/collection/mutable/Seq.scala @@ -45,4 +45,4 @@ object Seq extends SeqFactory[Seq] { } /** Explicit instantiation of the `Seq` trait to reduce class file size in subclasses. */ -private[scala] abstract class AbstractSeq[A] extends scala.collection.AbstractSeq[A] with Seq[A] +abstract class AbstractSeq[A] extends scala.collection.AbstractSeq[A] with Seq[A] diff --git a/src/library/scala/collection/mutable/Set.scala b/src/library/scala/collection/mutable/Set.scala index 4439880976..97574718e8 100644 --- a/src/library/scala/collection/mutable/Set.scala +++ b/src/library/scala/collection/mutable/Set.scala @@ -43,4 +43,4 @@ object Set extends MutableSetFactory[Set] { } /** Explicit instantiation of the `Set` trait to reduce class file size in subclasses. */ -private[scala] abstract class AbstractSet[A] extends AbstractIterable[A] with Set[A] +abstract class AbstractSet[A] extends AbstractIterable[A] with Set[A] |