Skip to content

Commit f551646

Browse files
oderskyodersky
committed
Close the protected[local] escape hatch
1 parent 9b326e5 commit f551646

13 files changed

+64
-58
lines changed

‎compiler/src/dotty/tools/dotc/typer/VarianceChecker.scala

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ class VarianceChecker()(implicit ctx: Context) {
3232
def ignoreVarianceIn(base: Symbol): Boolean = (
3333
base.isTerm
3434
|| base.is(Package)
35-
|| base.is(Local)
35+
|| base.is(PrivateLocal)
3636
)
3737

3838
/** The variance of a symbol occurrence of `tvar` seen at the level of the definition of `base`.
@@ -112,9 +112,13 @@ class VarianceChecker()(implicit ctx: Context) {
112112
def checkVariance(sym: Symbol, pos: Position) = Validator.validateDefinition(sym) match {
113113
case Some(VarianceError(tvar, required)) =>
114114
def msg = i"${varianceString(tvar.flags)} $tvar occurs in ${varianceString(required)} position in type ${sym.info} of $sym"
115-
if (ctx.scala2Mode && sym.owner.isConstructor) {
115+
if (ctx.scala2Mode &&
116+
(sym.owner.isConstructor || sym.ownersIterator.exists(_.is(ProtectedLocal)))) {
116117
ctx.migrationWarning(s"According to new variance rules, this is no longer accepted; need to annotate with @uncheckedVariance:\n$msg", pos)
117-
patch(Position(pos.end), " @scala.annotation.unchecked.uncheckedVariance") // TODO use an import or shorten if possible
118+
// patch(Position(pos.end), " @scala.annotation.unchecked.uncheckedVariance")
119+
// Patch is disabled until two TODOs are solved:
120+
// TODO use an import or shorten if possible
121+
// TODO need to use a `:' if annotation is on term
118122
}
119123
else ctx.error(msg, pos)
120124
case None =>
@@ -125,7 +129,7 @@ class VarianceChecker()(implicit ctx: Context) {
125129
// No variance check for private/protected[this] methods/values.
126130
def skip =
127131
!sym.exists ||
128-
sym.is(Local) || // !!! watch out for protected local!
132+
sym.is(PrivateLocal) ||
129133
sym.is(TypeParam) && sym.owner.isClass // already taken care of in primary constructor of class
130134
tree match {
131135
case defn: MemberDef if skip =>

‎tests/neg/i3989d.scala

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
trait A[+_X] {
2+
protected[this] type X = _X // error: variance
3+
def f: X
4+
}
5+
6+
trait B extends A[B] {
7+
def f: X = new B {}
8+
}
9+
10+
class C extends B with A[C] {
11+
// should be required because the inherited f is of type B, not C
12+
// override def f: X = new C
13+
}
14+
15+
object Test extends App {
16+
val c1 = new C
17+
val c2: C = c1.f
18+
}

‎tests/neg/t3272.scala

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
trait A {
2+
trait C[+T] {
3+
protected[this] def f(t: T): Unit = {} // error: covariant type T occurs in contravariant position in type T of value t
4+
}
5+
trait D[T] extends C[T] {
6+
def g(t: T): Unit = { f(t) }
7+
}
8+
}

‎tests/pos/t7093.scala renamed to ‎tests/neg/t7093.scala

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
1-
object Test {
2-
3-
trait A[+X] {
4-
protected[this] def f(x: X): X = x
5-
}
1+
trait A[+X] {
2+
protected[this] def f(x: X): X = x // error: variance
3+
}
64

7-
trait B extends A[B] {
8-
def kaboom = f(new B {})
9-
}
5+
trait B extends A[B] {
6+
def kaboom = f(new B {})
7+
}
108

119
// protected[this] disables variance checking
1210
// of the signature of `f`.
@@ -16,12 +14,12 @@ object Test {
1614
// The protected[this] loophole is widely used
1715
// in the collections, every newBuilder method
1816
// would fail variance checking otherwise.
19-
class C extends B with A[C] {
20-
override protected[this] def f(c: C) = c
21-
}
17+
class C extends B with A[C] {
18+
override protected[this] def f(c: C) = c
19+
}
2220

2321
// java.lang.ClassCastException: B$$anon$1 cannot be cast to C
2422
// at C.f(<console>:15)
23+
object Test extends App {
2524
new C().kaboom
2625
}
27-

‎tests/pos-special/strawman-collections/CollectionStrawMan5.scala

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ object CollectionStrawMan5 {
5656

5757
/** Base trait for strict collections */
5858
trait Buildable[+A, +To <: Iterable[A]] extends Iterable[A] {
59-
protected[this] def newBuilder: Builder[A, To]
59+
protected[this] def newBuilder: Builder[A @uncheckedVariance, To]
6060
override def partition(p: A => Boolean): (To, To) = {
6161
val l, r = newBuilder
6262
iterator.foreach(x => (if (p(x)) l else r) += x)
@@ -95,7 +95,7 @@ object CollectionStrawMan5 {
9595
with IterableOps[A]
9696
with IterableMonoTransforms[A, C[A @uncheckedVariance]] // sound bcs of VarianceNote
9797
with IterablePolyTransforms[A, C] {
98-
protected[this] def fromLikeIterable(coll: Iterable[A]): C[A] = fromIterable(coll)
98+
protected[this] def fromLikeIterable(coll: Iterable[A @uncheckedVariance]): C[A @uncheckedVariance] = fromIterable(coll)
9999
}
100100

101101
/** Base trait for Seq operations */
@@ -115,7 +115,7 @@ object CollectionStrawMan5 {
115115

116116
trait IterableMonoTransforms[+A, +Repr] extends Any {
117117
protected def coll: Iterable[A]
118-
protected[this] def fromLikeIterable(coll: Iterable[A]): Repr
118+
protected[this] def fromLikeIterable(coll: Iterable[A] @scala.annotation.unchecked.uncheckedVariance): Repr
119119
def filter(p: A => Boolean): Repr = fromLikeIterable(View.Filter(coll, p))
120120
def partition(p: A => Boolean): (Repr, Repr) = {
121121
val pn = View.Partition(coll, p)
@@ -166,7 +166,7 @@ object CollectionStrawMan5 {
166166
}
167167
def length: Int =
168168
if (isEmpty) 0 else 1 + tail.length
169-
protected[this] def newBuilder = new ListBuffer[A]
169+
protected[this] def newBuilder = new ListBuffer[A @uncheckedVariance]
170170
def ++:[B >: A](prefix: List[B]): List[B] =
171171
if (prefix.isEmpty) this
172172
else Cons(prefix.head, prefix.tail ++: this)

‎tests/pos-special/strawman-collections/CollectionStrawMan6.scala

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ object CollectionStrawMan6 extends LowPriority {
168168
trait Buildable[+A, +Repr] extends Any with IterableMonoTransforms[A, Repr] {
169169

170170
/** Creates a new builder. */
171-
protected[this] def newBuilder: Builder[A, Repr]
171+
protected[this] def newBuilder: Builder[A, Repr] @uncheckedVariance
172172

173173
/** Optimized, push-based version of `partition`. */
174174
override def partition(p: A => Boolean): (Repr, Repr) = {
@@ -226,7 +226,7 @@ object CollectionStrawMan6 extends LowPriority {
226226
/** Create a collection of type `C[A]` from the elements of `coll`, which has
227227
* the same element type as this collection. Overridden in StringOps and ArrayOps.
228228
*/
229-
protected[this] def fromIterableWithSameElemType(coll: Iterable[A]): C[A] = fromIterable(coll)
229+
protected[this] def fromIterableWithSameElemType(coll: Iterable[A] @uncheckedVariance): C[A]@uncheckedVariance = fromIterable(coll)
230230
}
231231

232232
/** Base trait for Seq operations */
@@ -349,7 +349,7 @@ object CollectionStrawMan6 extends LowPriority {
349349
*/
350350
trait IterableMonoTransforms[+A, +Repr] extends Any {
351351
protected def coll: Iterable[A]
352-
protected[this] def fromIterableWithSameElemType(coll: Iterable[A]): Repr
352+
protected[this] def fromIterableWithSameElemType(coll: Iterable[A] @uncheckedVariance): Repr
353353

354354
/** All elements satisfying predicate `p` */
355355
def filter(p: A => Boolean): Repr = fromIterableWithSameElemType(View.Filter(coll, p))
@@ -420,7 +420,7 @@ object CollectionStrawMan6 extends LowPriority {
420420

421421
def fromIterable[B](c: Iterable[B]): List[B] = List.fromIterable(c)
422422

423-
protected[this] def newBuilder = new ListBuffer[A].mapResult(_.toList)
423+
protected[this] def newBuilder = new ListBuffer[A].mapResult(_.toList): @uncheckedVariance
424424

425425
/** Prepend element */
426426
def :: [B >: A](elem: B): List[B] = new ::(elem, this)

‎tests/pos/GenericTraversableTemplate.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ trait GenericTraversableTemplate[+A, +CC[X] <: GenTraversable[X]] extends HasNew
6262

6363
/** The builder that builds instances of type $Coll[A]
6464
*/
65-
protected[this] def newBuilder: Builder[A, CC[A]] = companion.newBuilder[A]
65+
protected[this] def newBuilder: Builder[A, CC[A]] @uncheckedVariance = companion.newBuilder[A]
6666

6767
/** The generic builder that builds instances of $Coll
6868
* at arbitrary element types.

‎tests/pos/t3272.scala

Lines changed: 0 additions & 8 deletions
This file was deleted.

‎tests/pos/t4345.scala

Lines changed: 0 additions & 7 deletions
This file was deleted.

‎tests/pos/variances-local.scala

Lines changed: 0 additions & 7 deletions
This file was deleted.

‎tests/run/CollectionTests.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ object CollectionStrawMan5 {
5555

5656
/** Base trait for strict collections */
5757
trait Buildable[+A, +To <: Iterable[A]] extends Iterable[A] {
58-
protected[this] def newBuilder: Builder[A, To]
58+
protected[this] def newBuilder: Builder[A @uncheckedVariance, To]
5959
override def partition(p: A => Boolean): (To, To) = {
6060
val l, r = newBuilder
6161
iterator.foreach(x => (if (p(x)) l else r) += x)
@@ -94,7 +94,7 @@ object CollectionStrawMan5 {
9494
with IterableOps[A]
9595
with IterableMonoTransforms[A, C[A @uncheckedVariance]] // sound bcs of VarianceNote
9696
with IterablePolyTransforms[A, C] {
97-
protected[this] def fromLikeIterable(coll: Iterable[A]): C[A] = fromIterable(coll)
97+
protected[this] def fromLikeIterable(coll: Iterable[A @uncheckedVariance]): C[A @uncheckedVariance] = fromIterable(coll)
9898
}
9999

100100
/** Base trait for Seq operations */
@@ -114,7 +114,7 @@ object CollectionStrawMan5 {
114114

115115
trait IterableMonoTransforms[+A, +Repr] extends Any {
116116
protected def coll: Iterable[A]
117-
protected[this] def fromLikeIterable(coll: Iterable[A]): Repr
117+
protected[this] def fromLikeIterable(coll: Iterable[A @uncheckedVariance ]): Repr
118118
def filter(p: A => Boolean): Repr = fromLikeIterable(View.Filter(coll, p))
119119
def partition(p: A => Boolean): (Repr, Repr) = {
120120
val pn = View.Partition(coll, p)

‎tests/run/colltest5/CollectionStrawMan5_1.scala

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ object CollectionStrawMan5 {
4848

4949
/** Base trait for strict collections */
5050
trait Buildable[+A, +To <: Iterable[A]] extends Iterable[A] {
51-
protected[this] def newBuilder: Builder[A, To]
51+
protected[this] def newBuilder: Builder[A, To] @uncheckedVariance
5252
override def partition(p: A => Boolean): (To, To) = {
5353
val l, r = newBuilder
5454
iterator.foreach(x => (if (p(x)) l else r) += x)
@@ -87,7 +87,7 @@ object CollectionStrawMan5 {
8787
with IterableOps[A]
8888
with IterableMonoTransforms[A, C[A @uncheckedVariance]] // sound bcs of VarianceNote
8989
with IterablePolyTransforms[A, C] {
90-
protected[this] def fromLikeIterable(coll: Iterable[A]): C[A] = fromIterable(coll)
90+
protected[this] def fromLikeIterable(coll: Iterable[A] @uncheckedVariance): C[A] @uncheckedVariance = fromIterable(coll)
9191
}
9292

9393
/** Base trait for Seq operations */
@@ -107,7 +107,7 @@ object CollectionStrawMan5 {
107107

108108
trait IterableMonoTransforms[+A, +Repr] extends Any {
109109
protected def coll: Iterable[A]
110-
protected[this] def fromLikeIterable(coll: Iterable[A]): Repr
110+
protected[this] def fromLikeIterable(coll: Iterable[A] @uncheckedVariance): Repr
111111
def filter(p: A => Boolean): Repr = fromLikeIterable(View.Filter(coll, p))
112112
def partition(p: A => Boolean): (Repr, Repr) = {
113113
val pn = View.Partition(coll, p)
@@ -158,7 +158,7 @@ object CollectionStrawMan5 {
158158
}
159159
def length: Int =
160160
if (isEmpty) 0 else 1 + tail.length
161-
protected[this] def newBuilder = new ListBuffer[A]
161+
protected[this] def newBuilder = new ListBuffer[A] @uncheckedVariance
162162
def ++:[B >: A](prefix: List[B]): List[B] =
163163
if (prefix.isEmpty) this
164164
else Cons(prefix.head, prefix.tail ++: this)

‎tests/run/colltest6/CollectionStrawMan6_1.scala

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ object CollectionStrawMan6 extends LowPriority {
169169
trait Buildable[+A, +Repr] extends Any with IterableMonoTransforms[A, Repr] {
170170

171171
/** Creates a new builder. */
172-
protected[this] def newBuilder: Builder[A, Repr]
172+
protected[this] def newBuilder: Builder[A @uncheckedVariance, Repr]
173173

174174
/** Optimized, push-based version of `partition`. */
175175
override def partition(p: A => Boolean): (Repr, Repr) = {
@@ -227,7 +227,7 @@ object CollectionStrawMan6 extends LowPriority {
227227
/** Create a collection of type `C[A]` from the elements of `coll`, which has
228228
* the same element type as this collection. Overridden in StringOps and ArrayOps.
229229
*/
230-
protected[this] def fromIterableWithSameElemType(coll: Iterable[A]): C[A] = fromIterable(coll)
230+
protected[this] def fromIterableWithSameElemType(coll: Iterable[A @uncheckedVariance]): C[A @uncheckedVariance] = fromIterable(coll)
231231
}
232232

233233
/** Base trait for Seq operations */
@@ -350,7 +350,7 @@ object CollectionStrawMan6 extends LowPriority {
350350
*/
351351
trait IterableMonoTransforms[+A, +Repr] extends Any {
352352
protected def coll: Iterable[A]
353-
protected[this] def fromIterableWithSameElemType(coll: Iterable[A]): Repr
353+
protected[this] def fromIterableWithSameElemType(coll: Iterable[A @uncheckedVariance]): Repr
354354

355355
/** All elements satisfying predicate `p` */
356356
def filter(p: A => Boolean): Repr = fromIterableWithSameElemType(View.Filter(coll, p))
@@ -421,7 +421,7 @@ object CollectionStrawMan6 extends LowPriority {
421421

422422
def fromIterable[B](c: Iterable[B]): List[B] = List.fromIterable(c)
423423

424-
protected[this] def newBuilder = new ListBuffer[A].mapResult(_.toList)
424+
protected[this] def newBuilder = new ListBuffer[A @uncheckedVariance].mapResult(_.toList)
425425

426426
/** Prepend element */
427427
def :: [B >: A](elem: B): List[B] = new ::(elem, this)

0 commit comments

Comments
 (0)