Fix variance loophole for private vars

compiler/src/dotty/tools/dotc/core/SymDenotations.scala

@@ -864,6 +864,17 @@ object SymDenotations {
     final def isNullableClassAfterErasure(using Context): Boolean =
       isClass && !isValueClass && !is(ModuleClass) && symbol != defn.NothingClass
 
+    /** Is `pre` the same as C.this, where C is exactly the owner of this symbol,
+     *  or, if this symbol is protected, a subclass of the owner?
+     */
+    def isCorrectThisType(pre: Type)(using Context): Boolean = pre match
+      case pre: ThisType =>
+        (pre.cls eq owner) || this.is(Protected) && pre.cls.derivesFrom(owner)
+      case pre: TermRef =>
+        pre.symbol.moduleClass == owner
+      case _ =>
+        false
+
     /** Is this definition accessible as a member of tree with type `pre`?
      *  @param pre          The type of the tree from which the selection is made
      *  @param superAccess  Access is via super
@@ -888,18 +899,6 @@ object SymDenotations {
         (linked ne NoSymbol) && accessWithin(linked)
       }
 
-      /** Is `pre` the same as C.thisThis, where C is exactly the owner of this symbol,
-       *  or, if this symbol is protected, a subclass of the owner?
-       */
-      def isCorrectThisType(pre: Type): Boolean = pre match {
-        case pre: ThisType =>
-          (pre.cls eq owner) || this.is(Protected) && pre.cls.derivesFrom(owner)
-        case pre: TermRef =>
-          pre.symbol.moduleClass == owner
-        case _ =>
-          false
-      }
-
       /** Is protected access to target symbol permitted? */
       def isProtectedAccessOK: Boolean =
         inline def fail(str: String): false =

compiler/src/dotty/tools/dotc/transform/PostTyper.scala

@@ -109,6 +109,13 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
       try op finally noCheckNews = saved
     }
 
+    /** The set of all private class variables that are assigned
+     *  when selected with a qualifier other than the `this` of the owning class.
+     *  Such variables can contain only invariant type parameters in
+     *  their types.
+     */
+    private var privateVarsSetNonLocally: Set[Symbol] = Set()
+
     def isCheckable(t: New): Boolean = !inJavaAnnot && !noCheckNews.contains(t)
 
     /** Mark parameter accessors that are aliases of like-named parameters
@@ -149,6 +156,8 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
     private def processMemberDef(tree: Tree)(using Context): tree.type = {
       val sym = tree.symbol
       Checking.checkValidOperator(sym)
+      if sym.isClass then
+        VarianceChecker.check(tree, privateVarsSetNonLocally)
       sym.transformAnnotations(transformAnnot)
       sym.defTree = tree
       tree
@@ -257,6 +266,14 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
       }
     }
 
+    /** Update privateVarsSetNonLocally is symbol is a private variable
+     *  that is selected from something other than `this` when assigned
+     */
+    private def markVarAccess(tree: Tree, qual: Tree)(using Context): Unit =
+      val sym = tree.symbol
+      if sym.is(Private, butNot = Local) && !sym.isCorrectThisType(qual.tpe) then
+        privateVarsSetNonLocally += sym
+
     def checkNoConstructorProxy(tree: Tree)(using Context): Unit =
       if tree.symbol.is(ConstructorProxy) then
         report.error(em"constructor proxy ${tree.symbol} cannot be used as a value", tree.srcPos)
@@ -395,7 +412,6 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
           registerIfHasMacroAnnotations(tree)
           val sym = tree.symbol
           if (sym.isClass)
-            VarianceChecker.check(tree)
             annotateExperimental(sym)
             checkMacroAnnotation(sym)
             if sym.isOneOf(GivenOrImplicit) then
@@ -472,6 +488,9 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
               case tpe => tpe
             }
           )
+        case Assign(lhs @ Select(qual, _), _) =>
+          markVarAccess(lhs, qual)
+          super.transform(tree)
         case Typed(Ident(nme.WILDCARD), _) =>
           withMode(Mode.Pattern)(super.transform(tree))
             // The added mode signals that bounds in a pattern need not

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

@@ -19,8 +19,8 @@ import printing.Formatting.hl
  */
 object VarianceChecker {
   case class VarianceError(tvar: Symbol, required: Variance)
-  def check(tree: tpd.Tree)(using Context): Unit =
-    VarianceChecker().Traverser.traverse(tree)
+  def check(tree: tpd.Tree, privateVarsSetNonLocally: collection.Set[Symbol])(using Context): Unit =
+    VarianceChecker(privateVarsSetNonLocally).Traverser.traverse(tree)
 
   /** Check that variances of type lambda correspond to their occurrences in its body.
    *  Note: this is achieved by a mechanism separate from checking class type parameters.
@@ -62,7 +62,7 @@ object VarianceChecker {
   end checkLambda
 }
 
-class VarianceChecker(using Context) {
+class VarianceChecker(privateVarsSetNonLocally: collection.Set[Symbol])(using Context) {
   import VarianceChecker._
   import tpd._
 
@@ -148,12 +148,19 @@ class VarianceChecker(using Context) {
       case _ =>
         apply(None, info)
 
-    def validateDefinition(base: Symbol): Option[VarianceError] = {
-      val saved = this.base
+    def validateDefinition(base: Symbol): Option[VarianceError] =
+      val savedBase = this.base
       this.base = base
+      val savedVariance = variance
+      def isLocal =
+        base.isAllOf(PrivateLocal)
+        || base.is(Private) && !privateVarsSetNonLocally.contains(base)
+      if base.is(Mutable, butNot = Method) && !isLocal then
+        variance = 0
       try checkInfo(base.info)
-      finally this.base = saved
-    }
+      finally
+        this.base = savedBase
+        this.variance = savedVariance
   }
 
   private object Traverser extends TreeTraverser {

tests/neg/i18588.check

@@ -0,0 +1,4 @@
+-- Error: tests/neg/i18588.scala:7:14 ----------------------------------------------------------------------------------
+7 |  private var cached: A = value // error
+  |  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  |  covariant type A occurs in invariant position in type A of variable cached

tests/neg/i18588.scala

@@ -0,0 +1,22 @@
+class ROBox[+A](value: A) {
+  private var cached: A = value
+  def get: A = ROBox[A](value).cached
+}
+
+class Box[+A](value: A) {
+  private var cached: A = value // error
+  def get: A = cached
+
+  def put[AA >: A](value: AA): Unit = {
+    val box: Box[AA] = this
+    box.cached = value
+  }
+}
+
+trait Animal
+object Dog extends Animal
+object Cat extends Animal
+
+val dogBox: Box[Dog.type] = new Box(Dog)
+val _ = dogBox.put(Cat)
+val dog: Dog.type = dogBox.get