Skip to content

Commit 51ec62a

Browse files
committed
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 scala#30 // Method g:()Lp/ImplementationOnly; // 4: invokeinterface scala#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.
1 parent f787086 commit 51ec62a

File tree

15 files changed

+205
-14
lines changed

15 files changed

+205
-14
lines changed

src/library/scala/collection/Iterable.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,4 +51,4 @@ object Iterable extends TraversableFactory[Iterable] {
5151
}
5252

5353
/** Explicit instantiation of the `Iterable` trait to reduce class file size in subclasses. */
54-
private[scala] abstract class AbstractIterable[+A] extends AbstractTraversable[A] with Iterable[A]
54+
abstract class AbstractIterable[+A] extends AbstractTraversable[A] with Iterable[A]

src/library/scala/collection/Iterator.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1171,4 +1171,4 @@ trait Iterator[+A] extends TraversableOnce[A] {
11711171
}
11721172

11731173
/** Explicit instantiation of the `Iterator` trait to reduce class file size in subclasses. */
1174-
private[scala] abstract class AbstractIterator[+A] extends Iterator[A]
1174+
abstract class AbstractIterator[+A] extends Iterator[A]

src/library/scala/collection/Map.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,4 +56,4 @@ object Map extends MapFactory[Map] {
5656
}
5757

5858
/** Explicit instantiation of the `Map` trait to reduce class file size in subclasses. */
59-
private[scala] abstract class AbstractMap[A, +B] extends AbstractIterable[(A, B)] with Map[A, B]
59+
abstract class AbstractMap[A, +B] extends AbstractIterable[(A, B)] with Map[A, B]

src/library/scala/collection/Seq.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,4 @@ object Seq extends SeqFactory[Seq] {
3838
}
3939

4040
/** Explicit instantiation of the `Seq` trait to reduce class file size in subclasses. */
41-
private[scala] abstract class AbstractSeq[+A] extends AbstractIterable[A] with Seq[A]
41+
abstract class AbstractSeq[+A] extends AbstractIterable[A] with Seq[A]

src/library/scala/collection/Set.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,4 @@ object Set extends SetFactory[Set] {
4444
}
4545

4646
/** Explicit instantiation of the `Set` trait to reduce class file size in subclasses. */
47-
private[scala] abstract class AbstractSet[A] extends AbstractIterable[A] with Set[A]
47+
abstract class AbstractSet[A] extends AbstractIterable[A] with Set[A]

src/library/scala/collection/Traversable.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,4 +101,4 @@ object Traversable extends TraversableFactory[Traversable] { self =>
101101
}
102102

103103
/** Explicit instantiation of the `Traversable` trait to reduce class file size in subclasses. */
104-
private[scala] abstract class AbstractTraversable[+A] extends Traversable[A]
104+
abstract class AbstractTraversable[+A] extends Traversable[A]

src/library/scala/collection/immutable/Map.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,9 @@ trait Map[A, +B] extends Iterable[(A, B)]
3232
with MapLike[A, B, Map[A, B]] { self =>
3333

3434
override def empty: Map[A, B] = Map.empty
35-
35+
3636
/** Returns this $coll as an immutable map.
37-
*
37+
*
3838
* A new map will not be built; lazy collections will stay lazy.
3939
*/
4040
@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] {
191191
}
192192

193193
/** Explicit instantiation of the `Map` trait to reduce class file size in subclasses. */
194-
private[scala] abstract class AbstractMap[A, +B] extends scala.collection.AbstractMap[A, B] with Map[A, B]
194+
abstract class AbstractMap[A, +B] extends scala.collection.AbstractMap[A, B] with Map[A, B]

src/library/scala/collection/mutable/Buffer.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,4 @@ object Buffer extends SeqFactory[Buffer] {
4646
}
4747

4848
/** Explicit instantiation of the `Buffer` trait to reduce class file size in subclasses. */
49-
private[scala] abstract class AbstractBuffer[A] extends AbstractSeq[A] with Buffer[A]
49+
abstract class AbstractBuffer[A] extends AbstractSeq[A] with Buffer[A]

src/library/scala/collection/mutable/Iterable.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,4 @@ object Iterable extends TraversableFactory[Iterable] {
3838
}
3939

4040
/** Explicit instantiation of the `Iterable` trait to reduce class file size in subclasses. */
41-
private[scala] abstract class AbstractIterable[A] extends scala.collection.AbstractIterable[A] with Iterable[A]
41+
abstract class AbstractIterable[A] extends scala.collection.AbstractIterable[A] with Iterable[A]

src/library/scala/collection/mutable/Map.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,4 +89,4 @@ object Map extends MutableMapFactory[Map] {
8989
}
9090

9191
/** Explicit instantiation of the `Map` trait to reduce class file size in subclasses. */
92-
private[scala] abstract class AbstractMap[A, B] extends scala.collection.AbstractMap[A, B] with Map[A, B]
92+
abstract class AbstractMap[A, B] extends scala.collection.AbstractMap[A, B] with Map[A, B]

src/library/scala/collection/mutable/Seq.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,4 +45,4 @@ object Seq extends SeqFactory[Seq] {
4545
}
4646

4747
/** Explicit instantiation of the `Seq` trait to reduce class file size in subclasses. */
48-
private[scala] abstract class AbstractSeq[A] extends scala.collection.AbstractSeq[A] with Seq[A]
48+
abstract class AbstractSeq[A] extends scala.collection.AbstractSeq[A] with Seq[A]

src/library/scala/collection/mutable/Set.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,4 @@ object Set extends MutableSetFactory[Set] {
4343
}
4444

4545
/** Explicit instantiation of the `Set` trait to reduce class file size in subclasses. */
46-
private[scala] abstract class AbstractSet[A] extends AbstractIterable[A] with Set[A]
46+
abstract class AbstractSet[A] extends AbstractIterable[A] with Set[A]

test/files/pos/t6948.scala

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
object t6948 {
2+
val rand = new scala.util.Random()
3+
def a1 = rand.shuffle(0 to 5)
4+
// Tis not to be
5+
// def a2 = rand.shuffle(0 until 5)
6+
def a3 = rand.shuffle(Vector(1, 2, 3))
7+
def a4 = rand.shuffle(scala.collection.Seq(1, 2, 3))
8+
def a5 = rand.shuffle(scala.collection.immutable.Seq(1, 2, 3))
9+
def a6 = rand.shuffle(scala.collection.mutable.Seq(1, 2, 3))
10+
}
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
warning: there were 2 feature warning(s); re-run with -feature for details
2+
p.Iterable[Int]
3+
p.Set[Int]
4+
p.Seq[Int]
5+
p.m.Set[Int]
6+
p.m.Seq[Int]
7+
private[m] p.m.ASet[Int]
8+
p.i.Seq[Int]
9+
private[i] p.i.ASet[Int]
10+
private[i] p.i.ASeq[Int]
11+
p.Iterable[Int]
12+
p.Iterable[Int]
13+
p.Iterable[Int]
14+
p.Iterable[Int]
15+
p.Iterable[Int]
16+
p.Iterable[Int]
17+
p.Iterable[Int]
18+
p.Iterable[Int]
19+
p.Iterable[Int]
20+
p.Set[Int]
21+
p.Iterable[Int]
22+
p.Set[Int]
23+
p.Iterable[Int]
24+
p.Set[Int]
25+
p.Iterable[Int]
26+
p.Iterable[Int]
27+
p.Seq[Int]
28+
p.Iterable[Int]
29+
p.Seq[Int]
30+
p.Iterable[Int]
31+
p.Seq[Int]
32+
p.Iterable[Int]
33+
p.m.Set[Int]
34+
p.Iterable[Int]
35+
p.Set[Int]
36+
p.Iterable[Int]
37+
p.Iterable[Int]
38+
p.Seq[Int]
39+
p.Iterable[Int]
40+
p.Seq[Int]
41+
p.Iterable[Int]
42+
private[p] p.ASet[Int]
43+
private[p] p.AIterable[Int]
44+
p.Iterable[Int]
45+
p.i.Seq[Int]
46+
private[p] p.AIterable[Int]
47+
List[Nothing]
48+
scala.collection.immutable.Vector[Nothing]
49+
scala.collection.immutable.Iterable[(Int, Int)]
50+
scala.collection.immutable.Set[Int]
51+
Seq[Int]
52+
Array[Int]
53+
scala.collection.AbstractSet[Int]
54+
Comparable[java.lang.String]
55+
scala.collection.immutable.LinearSeq[Int]
56+
Iterable[Int]
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
package p {
2+
trait TCon[+CC[X]] {
3+
def fPublic: CC[Int] = ???
4+
private[p] def fPackagePrivate: CC[Int] = ???
5+
protected[p] def fPackageProtected: CC[Int] = ???
6+
}
7+
trait Iterable[+A] extends TCon[Iterable]
8+
trait Set[A] extends Iterable[A] with TCon[Set]
9+
trait Seq[+A] extends Iterable[A] with TCon[Seq]
10+
11+
private[p] abstract class AIterable[+A] extends Iterable[A]
12+
private[p] abstract class ASeq[+A] extends AIterable[A] with Seq[A]
13+
private[p] abstract class ASet[A] extends AIterable[A] with Set[A]
14+
15+
package m {
16+
private[m] abstract class ASeq[A] extends p.ASeq[A] with Seq[A]
17+
private[m] abstract class ASet[A] extends p.ASet[A] with Set[A]
18+
trait Set[A] extends p.Set[A] with TCon[Set]
19+
trait Seq[A] extends p.Seq[A] with TCon[Seq]
20+
trait BitSet extends ASet[Int]
21+
trait IntSeq extends ASeq[Int]
22+
}
23+
24+
package i {
25+
private[i] abstract class ASeq[+A] extends p.ASeq[A] with Seq[A]
26+
private[i] abstract class ASet[A] extends p.ASet[A] with Set[A]
27+
trait Set[A] extends p.Set[A] with TCon[Set]
28+
trait Seq[+A] extends p.Seq[A] with TCon[Seq]
29+
trait BitSet extends ASet[Int]
30+
trait IntSeq extends ASeq[Int]
31+
}
32+
}
33+
34+
object Test {
35+
import scala.reflect.runtime.universe._
36+
// Complicated by the absence of usable type constructor type tags.
37+
def extract[A, CC[X]](xs: CC[A]): CC[A] = xs
38+
def whatis[T: TypeTag](x: T): Unit = {
39+
val tpe = typeOf[T]
40+
val access = tpe.typeSymbol.asInstanceOf[scala.reflect.internal.HasFlags].accessString.replaceAllLiterally("package ", "")
41+
println(f"$access%15s $tpe")
42+
}
43+
44+
trait IntIterable extends p.Iterable[Int]
45+
trait IntSet extends p.Set[Int]
46+
trait IntSeq extends p.Seq[Int]
47+
48+
trait MutableIntSet extends p.m.Set[Int]
49+
trait MutableIntSeq extends p.m.Seq[Int]
50+
51+
trait ImmutableIntSet extends p.i.Set[Int]
52+
trait ImmutableIntSeq extends p.i.Seq[Int]
53+
54+
def f1: IntIterable = null
55+
def f2: IntSet = null
56+
def f3: IntSeq = null
57+
58+
def g1: MutableIntSet = null
59+
def g2: MutableIntSeq = null
60+
def g3: p.m.BitSet = null
61+
62+
def h1: ImmutableIntSeq = null
63+
def h2: p.i.BitSet = null
64+
def h3: p.i.IntSeq = null
65+
66+
def main(args: Array[String]): Unit = {
67+
whatis(extract(f1))
68+
whatis(extract(f2))
69+
whatis(extract(f3))
70+
whatis(extract(g1))
71+
whatis(extract(g2))
72+
whatis(extract(g3))
73+
whatis(extract(h1))
74+
whatis(extract(h2))
75+
whatis(extract(h3))
76+
77+
whatis(extract(if (true) f1 else f2))
78+
whatis(extract(if (true) f1 else f3))
79+
whatis(extract(if (true) f1 else g1))
80+
whatis(extract(if (true) f1 else g2))
81+
whatis(extract(if (true) f1 else g3))
82+
whatis(extract(if (true) f1 else h1))
83+
whatis(extract(if (true) f1 else h2))
84+
whatis(extract(if (true) f1 else h3))
85+
whatis(extract(if (true) f2 else f3))
86+
whatis(extract(if (true) f2 else g1))
87+
whatis(extract(if (true) f2 else g2))
88+
whatis(extract(if (true) f2 else g3))
89+
whatis(extract(if (true) f2 else h1))
90+
whatis(extract(if (true) f2 else h2))
91+
whatis(extract(if (true) f2 else h3))
92+
whatis(extract(if (true) f3 else g1))
93+
whatis(extract(if (true) f3 else g2))
94+
whatis(extract(if (true) f3 else g3))
95+
whatis(extract(if (true) f3 else h1))
96+
whatis(extract(if (true) f3 else h2))
97+
whatis(extract(if (true) f3 else h3))
98+
whatis(extract(if (true) g1 else g2))
99+
whatis(extract(if (true) g1 else g3))
100+
whatis(extract(if (true) g1 else h1))
101+
whatis(extract(if (true) g1 else h2))
102+
whatis(extract(if (true) g1 else h3))
103+
whatis(extract(if (true) g2 else g3))
104+
whatis(extract(if (true) g2 else h1))
105+
whatis(extract(if (true) g2 else h2))
106+
whatis(extract(if (true) g2 else h3))
107+
whatis(extract(if (true) g3 else h1))
108+
whatis(extract(if (true) g3 else h2))
109+
whatis(extract(if (true) g3 else h3))
110+
whatis(extract(if (true) h1 else h2))
111+
whatis(extract(if (true) h1 else h3))
112+
whatis(extract(if (true) h2 else h3))
113+
114+
whatis(extract(Nil))
115+
whatis(extract(Vector()))
116+
whatis(extract(Map[Int,Int]()))
117+
whatis(extract(Set[Int]()))
118+
whatis(extract(Seq[Int]()))
119+
whatis(extract(Array[Int]()))
120+
whatis(extract(scala.collection.immutable.BitSet(1)))
121+
whatis(extract("abc"))
122+
whatis(extract(if (true) Stream(1) else List(1)))
123+
whatis(extract(if (true) Seq(1) else Set(1)))
124+
}
125+
}

0 commit comments

Comments
 (0)