diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 51182f52a382..c4fa382fa94c 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -993,6 +993,7 @@ class Definitions { // Annotation classes @tu lazy val AllowConversionsAnnot: ClassSymbol = requiredClass("scala.annotation.allowConversions") @tu lazy val AnnotationDefaultAnnot: ClassSymbol = requiredClass("scala.annotation.internal.AnnotationDefault") + @tu lazy val AssignedNonLocallyAnnot: ClassSymbol = requiredClass("scala.annotation.internal.AssignedNonLocally") @tu lazy val BeanPropertyAnnot: ClassSymbol = requiredClass("scala.beans.BeanProperty") @tu lazy val BooleanBeanPropertyAnnot: ClassSymbol = requiredClass("scala.beans.BooleanBeanProperty") @tu lazy val BodyAnnot: ClassSymbol = requiredClass("scala.annotation.internal.Body") diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 3dfa5225df5b..99ae0dd60905 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/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 isAccessPrivilegedThisType(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 = @@ -933,7 +932,7 @@ object SymDenotations { || boundary.isRoot || (accessWithin(boundary) || accessWithinLinked(boundary)) && ( !this.is(Local) - || isCorrectThisType(pre) + || isAccessPrivilegedThisType(pre) || canBeLocal(name, flags) && { resetFlag(Local) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 431e863f85d2..a5225ad46b73 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1103,6 +1103,18 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer // allow assignments from the primary constructor to class fields ctx.owner.name.is(TraitSetterName) || ctx.owner.isStaticConstructor + /** Mark private variables that are assigned with a prefix other than + * the `this` type of their owner with a `annotation.internal.AssignedNonLocally` + * annotation. The annotation influences the variance check for these + * variables, which is done at PostTyper. It will be removed after the + * variance check. + */ + def rememberNonLocalAssignToPrivate(sym: Symbol) = lhs1 match + case Select(qual, _) + if sym.is(Private, butNot = Local) && !sym.isAccessPrivilegedThisType(qual.tpe) => + sym.addAnnotation(Annotation(defn.AssignedNonLocallyAnnot, lhs1.span)) + case _ => + lhsCore match case Apply(fn, _) if fn.symbol.is(ExtensionMethod) => def toSetter(fn: Tree): untpd.Tree = fn match @@ -1136,15 +1148,16 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer case _ => lhsCore.tpe match { case ref: TermRef => val lhsVal = lhsCore.denot.suchThat(!_.is(Method)) - if (canAssign(lhsVal.symbol)) { - // lhsBounds: (T .. Any) as seen from lhs prefix, where T is the type of lhsVal.symbol + val lhsSym = lhsVal.symbol + if canAssign(lhsSym) then + rememberNonLocalAssignToPrivate(lhsSym) + // lhsBounds: (T .. Any) as seen from lhs prefix, where T is the type of lhsSym // This ensures we do the as-seen-from on T with variance -1. Test case neg/i2928.scala val lhsBounds = - TypeBounds.lower(lhsVal.symbol.info).asSeenFrom(ref.prefix, lhsVal.symbol.owner) + TypeBounds.lower(lhsSym.info).asSeenFrom(ref.prefix, lhsSym.owner) assignType(cpy.Assign(tree)(lhs1, typed(tree.rhs, lhsBounds.loBound))) .computeAssignNullable() - } - else { + else val pre = ref.prefix val setterName = ref.name.setterName val setter = pre.member(setterName) @@ -1157,7 +1170,6 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer case _ => reassignmentToVal } - } case TryDynamicCallType => typedDynamicAssign(tree, pt) case tpe => diff --git a/compiler/src/dotty/tools/dotc/typer/VarianceChecker.scala b/compiler/src/dotty/tools/dotc/typer/VarianceChecker.scala index a5e0b62094ef..21fa9eed0df4 100644 --- a/compiler/src/dotty/tools/dotc/typer/VarianceChecker.scala +++ b/compiler/src/dotty/tools/dotc/typer/VarianceChecker.scala @@ -148,12 +148,20 @@ 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) && !base.hasAnnotation(defn.AssignedNonLocallyAnnot) + if base.is(Mutable, butNot = Method) && !isLocal then + base.removeAnnotation(defn.AssignedNonLocallyAnnot) + variance = 0 try checkInfo(base.info) - finally this.base = saved - } + finally + this.base = savedBase + this.variance = savedVariance } private object Traverser extends TreeTraverser { diff --git a/library/src/scala/annotation/internal/AssignedNonLocally.scala b/library/src/scala/annotation/internal/AssignedNonLocally.scala new file mode 100644 index 000000000000..8095ba91cfee --- /dev/null +++ b/library/src/scala/annotation/internal/AssignedNonLocally.scala @@ -0,0 +1,7 @@ +package scala.annotation +package internal + +/** An annotation to indicate that a private `var` was assigned with a prefix + * other than the `this` type of its owner. + */ +class AssignedNonLocally() extends Annotation diff --git a/tests/neg/i18588.check b/tests/neg/i18588.check new file mode 100644 index 000000000000..5f7d6181a93c --- /dev/null +++ b/tests/neg/i18588.check @@ -0,0 +1,8 @@ +-- 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 +-- Error: tests/neg/i18588.scala:17:14 --------------------------------------------------------------------------------- +17 | private var cached: A = value // error + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | covariant type A occurs in invariant position in type A of variable cached diff --git a/tests/neg/i18588.scala b/tests/neg/i18588.scala new file mode 100644 index 000000000000..e8c89c1b06f1 --- /dev/null +++ b/tests/neg/i18588.scala @@ -0,0 +1,42 @@ +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 + } +} + +class BoxWithCompanion[+A](value: A) { + private var cached: A = value // error + def get: A = cached +} + +class BoxValid[+A](value: A, orig: A) { + private var cached: A = value // ok + def get: A = cached + + def reset(): Unit = + cached = orig // ok: mutated through this prefix +} + +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 + + +object BoxWithCompanion { + def put[A](box: BoxWithCompanion[A], value: A): Unit = { + box.cached = value + } +} \ No newline at end of file