From 3f4f2858e15f41ef031fdbbbb1282a603224bafb Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Mon, 18 Nov 2019 14:16:13 +0100
Subject: [PATCH] Fix #7567: Variance checking fixes

Do check private[this] members for variances but cut relative variance
computations at private[this] members.
---
 .../tools/dotc/typer/VarianceChecker.scala    | 30 ++++++++-----------
 tests/neg/i7567.scala                         | 13 ++++++++
 2 files changed, 26 insertions(+), 17 deletions(-)
 create mode 100644 tests/neg/i7567.scala

diff --git a/compiler/src/dotty/tools/dotc/typer/VarianceChecker.scala b/compiler/src/dotty/tools/dotc/typer/VarianceChecker.scala
index 96cef1948994..9258db7839b3 100644
--- a/compiler/src/dotty/tools/dotc/typer/VarianceChecker.scala
+++ b/compiler/src/dotty/tools/dotc/typer/VarianceChecker.scala
@@ -83,24 +83,21 @@ class VarianceChecker()(implicit ctx: Context) {
   private object Validator extends TypeAccumulator[Option[VarianceError]] {
     private var base: Symbol = _
 
-    /** Is no variance checking needed within definition of `base`? */
-    def ignoreVarianceIn(base: Symbol): Boolean = (
-         base.isTerm
-      || base.is(Package)
-      || base.isAllOf(PrivateLocal)
-    )
-
     /** The variance of a symbol occurrence of `tvar` seen at the level of the definition of `base`.
      *  The search proceeds from `base` to the owner of `tvar`.
      *  Initially the state is covariant, but it might change along the search.
      */
     def relativeVariance(tvar: Symbol, base: Symbol, v: Variance = Covariant): Variance = /*trace(i"relative variance of $tvar wrt $base, so far: $v")*/
-      if (base == tvar.owner) v
-      else if (base.is(Param) && base.owner.isTerm)
+      if base == tvar.owner then
+        v
+      else if base.is(Param) && base.owner.isTerm && !base.owner.isAllOf(PrivateLocal) then
         relativeVariance(tvar, paramOuter(base.owner), flip(v))
-      else if (ignoreVarianceIn(base.owner)) Bivariant
-      else if (base.isAliasType) relativeVariance(tvar, base.owner, Invariant)
-      else relativeVariance(tvar, base.owner, v)
+      else if base.owner.isTerm || base.owner.is(Package) || base.isAllOf(PrivateLocal) then
+        Bivariant
+      else if base.isAliasType then
+        relativeVariance(tvar, base.owner, Invariant)
+      else
+        relativeVariance(tvar, base.owner, v)
 
     /** The next level to take into account when determining the
      *  relative variance with a method parameter as base. The method
@@ -189,11 +186,10 @@ class VarianceChecker()(implicit ctx: Context) {
     override def traverse(tree: Tree)(implicit ctx: Context) = {
       def sym = tree.symbol
       // No variance check for private/protected[this] methods/values.
-      def skip =
-        !sym.exists ||
-        sym.isAllOf(PrivateLocal) ||
-        sym.name.is(InlineAccessorName) || // TODO: should we exclude all synthetic members?
-        sym.is(TypeParam) && sym.owner.isClass // already taken care of in primary constructor of class
+      def skip = !sym.exists
+        || sym.name.is(InlineAccessorName) // TODO: should we exclude all synthetic members?
+        || sym.isAllOf(LocalParamAccessor) // local class parameters are construction only
+        || sym.is(TypeParam) && sym.owner.isClass // already taken care of in primary constructor of class
       try tree match {
         case defn: MemberDef if skip =>
           ctx.debuglog(s"Skipping variance check of ${sym.showDcl}")
diff --git a/tests/neg/i7567.scala b/tests/neg/i7567.scala
new file mode 100644
index 000000000000..c902f80037a3
--- /dev/null
+++ b/tests/neg/i7567.scala
@@ -0,0 +1,13 @@
+class A
+class B extends A
+class C extends A
+
+object Foo {
+  private[this] class Bar[+T](var x: T) // error: covariant type T occurs in contravariant position in type T of value x_=
+  def foo: B = {
+    val barB: Bar[B] = new Bar(new B)
+    val barA: Bar[A] = barB
+    barA.x = new C
+    barB.x
+  }
+}
\ No newline at end of file