Merge pull request scala#5280 from retronym/ticket/8079

SI-8079 Only expand local aliases during variance checks

Author: adriaanm

src/reflect/scala/reflect/internal/Variances.scala

@@ -50,11 +50,9 @@ trait Variances {
          sym.isParameter
       && !(tvar.isTypeParameterOrSkolem && sym.isTypeParameterOrSkolem && tvar.owner == sym.owner)
     )
-    // return Bivariant if `sym` is local to a term
-    // or is private[this] or protected[this]
-    def isLocalOnly(sym: Symbol) = !sym.owner.isClass || (
-         sym.isTerm // ?? shouldn't this be sym.owner.isTerm according to the comments above?
-      && (sym.isLocalToThis || sym.isSuperAccessor) // super accessors are implicitly local #4345
+    // Is `sym` is local to a term or is private[this] or protected[this]?
+    def isExemptFromVariance(sym: Symbol): Boolean = !sym.owner.isClass || (
+         (sym.isLocalToThis || sym.isSuperAccessor) // super accessors are implicitly local #4345
       && !escapedLocals(sym)
     )
 
@@ -66,15 +64,15 @@ trait Variances {
        *  Initially the state is covariant, but it might change along the search.
        *
        *  A local alias type is treated as Bivariant;
-       *  this is OK because we always expand aliases for variance checking.
+       *  this is OK because such aliases are expanded for variance checking.
        *  However, for an alias which might be externally visible, we must assume Invariant,
        *  because there may be references to the type parameter that are not checked,
        *  leading to unsoundness (see SI-6566).
        */
       def relativeVariance(tvar: Symbol): Variance = {
         def nextVariance(sym: Symbol, v: Variance): Variance = (
           if (shouldFlip(sym, tvar)) v.flip
-          else if (isLocalOnly(sym)) Bivariant
+          else if (isExemptFromVariance(sym)) Bivariant
           else if (sym.isAliasType) (
             // Unsound pre-2.11 behavior preserved under -Xsource:2.10
             if (settings.isScala211 || sym.isOverridingSymbol) Invariant
@@ -126,7 +124,7 @@ trait Variances {
         tp match {
           case _ if isUncheckedVariance(tp)                    =>
           case _ if resultTypeOnly(tp)                         => this(tp.resultType)
-          case TypeRef(_, sym, _) if sym.isAliasType           => this(tp.normalize)
+          case TypeRef(_, sym, _) if shouldDealias(sym)        => this(tp.normalize)
           case TypeRef(_, sym, _) if !sym.variance.isInvariant => checkVarianceOfSymbol(sym) ; mapOver(tp)
           case RefinedType(_, _)                               => withinRefinement(mapOver(tp))
           case ClassInfoType(parents, _, _)                    => parents foreach this
@@ -138,6 +136,12 @@ trait Variances {
         // than the result of the pattern match above, which normalizes types.
         tp
       }
+      private def shouldDealias(sym: Symbol): Boolean = {
+        // The RHS of (private|protected)[this] type aliases are excluded from variance checks. This is
+        // implemented in relativeVariance.
+        // As such, we need to expand references to them to retain soundness. Example: neg/t8079a.scala
+        sym.isAliasType && isExemptFromVariance(sym)
+      }
       def validateDefinition(base: Symbol) {
         val saved = this.base
         this.base = base

test/files/neg/t8079a.check

@@ -0,0 +1,4 @@
+t8079a.scala:3: error: contravariant type I occurs in covariant position in type C.this.X of value b
+  def f2(b: X): Unit
+         ^
+one error found

test/files/neg/t8079a.scala

@@ -0,0 +1,4 @@
+trait C[-I] {
+  private[this] type X = C[I]
+  def f2(b: X): Unit
+}

test/files/neg/variances.check

@@ -19,7 +19,7 @@ variances.scala:74: error: covariant type A occurs in contravariant position in
 variances.scala:89: error: covariant type T occurs in invariant position in type T of type A
     type A = T
          ^
-variances.scala:90: error: covariant type T occurs in contravariant position in type => test.TestAlias.B[C.this.A] of method foo
+variances.scala:90: error: covariant type A occurs in contravariant position in type => test.TestAlias.B[C.this.A] of method foo
     def foo: B[A]
         ^
 8 errors found

test/files/pos/t8079b.scala

@@ -0,0 +1,7 @@
+trait F1[/* - */T, /* + */ R]
+
+object Test {
+  import scala.annotation.unchecked._
+  type VariantF1[-T, +R] = F1[T @uncheckedVariance, R @uncheckedVariance]
+  trait C[+T] { def foo: VariantF1[Any, T] }
+}

test/pending/neg/t8079d.check

@@ -0,0 +1,4 @@
+t8079d.scala:3: error: contravariant type I occurs in covariant position in type C.this.X of value b
+  def f2(b: X): Unit
+         ^
+one error found

test/pending/neg/t8079d.scala

@@ -0,0 +1,4 @@
+trait C[-I] {
+  protected[this] type X = C[I]
+  def f2(b: X): Unit
+}

test/pending/pos/t8079c.scala

@@ -0,0 +1,7 @@
+trait F1[/* - */T, /* + */ R]
+
+object Test {
+  import scala.annotation.unchecked._
+  private[this] type VariantF1[-T, +R] = F1[T @uncheckedVariance, R @uncheckedVariance]
+  trait C[+T] { def foo: VariantF1[Any, T] }
+}