From f1b8a1d8e3831f5713b82b71636bd9d2a553a3a9 Mon Sep 17 00:00:00 2001 From: Georgi Krastev Date: Sun, 17 Nov 2019 21:34:38 +0100 Subject: [PATCH] Plug many variance holes (pos and neg) --- .../tools/nsc/typechecker/RefChecks.scala | 10 +- .../scala/reflect/internal/Variances.scala | 109 +++++++++++++----- test/files/neg/t7872.check | 2 +- test/files/neg/t7872b.check | 4 +- test/files/neg/variance-holes.check | 22 ++++ test/files/neg/variance-holes.scala | 22 ++++ test/files/neg/variances.check | 5 +- test/files/pos/variance-holes.scala | 37 ++++++ 8 files changed, 166 insertions(+), 45 deletions(-) create mode 100644 test/files/neg/variance-holes.check create mode 100644 test/files/neg/variance-holes.scala create mode 100644 test/files/pos/variance-holes.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index 2cabea26d226..10252b633b34 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -892,9 +892,9 @@ abstract class RefChecks extends Transform { case ClassInfoType(parents, _, clazz) => "supertype "+intersectionType(parents, clazz.owner) case _ => "type "+tp } - override def issueVarianceError(base: Symbol, sym: Symbol, required: Variance): Unit = { + override def issueVarianceError(base: Symbol, sym: Symbol, required: Variance, tpe: Type): Unit = { reporter.error(base.pos, - s"${sym.variance} $sym occurs in $required position in ${tpString(base.info)} of $base") + s"${sym.variance} $sym occurs in $required position in ${tpString(tpe)} of $base") } } @@ -1774,13 +1774,11 @@ abstract class RefChecks extends Transform { result.transform(this) } result1 match { - case ClassDef(_, _, _, _) - | TypeDef(_, _, _, _) - | ModuleDef(_, _, _) => + case ClassDef(_, _, _, _) | TypeDef(_, _, _, _) | ModuleDef(_, _, _) => if (result1.symbol.isLocalToBlock || result1.symbol.isTopLevel) varianceValidator.traverse(result1) case tt @ TypeTree() if tt.original != null => - varianceValidator.traverse(tt.original) // See scala/bug#7872 + varianceValidator.validateVarianceOfPolyTypesIn(tt.tpe) case _ => } diff --git a/src/reflect/scala/reflect/internal/Variances.scala b/src/reflect/scala/reflect/internal/Variances.scala index f2aee0a9315f..7e0705d34254 100644 --- a/src/reflect/scala/reflect/internal/Variances.scala +++ b/src/reflect/scala/reflect/internal/Variances.scala @@ -49,7 +49,7 @@ trait Variances { else escapedLocals += sym } - protected def issueVarianceError(base: Symbol, sym: Symbol, required: Variance): Unit = () + protected def issueVarianceError(base: Symbol, sym: Symbol, required: Variance, tpe: Type): Unit = () // Flip occurrences of type parameters and parameters, unless // - it's a constructor, or case class factory or extractor @@ -66,6 +66,7 @@ trait Variances { private object ValidateVarianceMap extends VariancedTypeMap { private[this] var base: Symbol = _ + private[this] var lowerBoundStack: List[Symbol] = Nil /** 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`. @@ -85,10 +86,11 @@ trait Variances { else v @tailrec - def loop(sym: Symbol, v: Variance): Variance = ( - if (sym == tvar.owner || v.isBivariant) v + def loop(sym: Symbol, v: Variance): Variance = + if (v.isBivariant) v + else if (sym == tvar.owner) if (lowerBoundStack.contains(sym)) v.flip else v else loop(sym.owner, nextVariance(sym, v)) - ) + loop(base, Covariant) } def isUncheckedVariance(tp: Type) = tp match { @@ -104,17 +106,18 @@ trait Variances { def base_s = s"$base in ${base.owner}" + (if (base.owner.isClass) "" else " in " + base.owner.enclClass) log(s"verifying $sym_s is $required at $base_s") if (sym.variance != required) - issueVarianceError(base, sym, required) + issueVarianceError(base, sym, required, base.info) } } override def mapOver(decls: Scope): Scope = { decls foreach (sym => withVariance(if (sym.isAliasType) Invariant else variance)(this(sym.info))) decls } + private def resultTypeOnly(tp: Type) = tp match { - case mt: MethodType => !inRefinement - case pt: PolyType => true - case _ => false + case _: MethodType => !inRefinement + case _: PolyType => true + case _ => false } /** For PolyTypes, type parameters are skipped because they are defined @@ -125,12 +128,12 @@ trait Variances { def apply(tp: Type): Type = { tp match { case _ if isUncheckedVariance(tp) => - case _ if resultTypeOnly(tp) => this(tp.resultType) - case TypeRef(_, sym, _) if shouldDealias(sym) => this(tp.normalize) - case TypeRef(_, sym, _) if !sym.variance.isInvariant => checkVarianceOfSymbol(sym) ; tp.mapOver(this) + case _ if resultTypeOnly(tp) => apply(tp.resultType) + case TypeRef(_, sym, _) if shouldDealias(sym) => apply(tp.normalize) + case TypeRef(_, sym, _) if !sym.variance.isInvariant => checkVarianceOfSymbol(sym); tp.mapOver(this) case RefinedType(_, _) => withinRefinement(tp.mapOver(this)) - case ClassInfoType(parents, _, _) => parents foreach this - case mt @ MethodType(_, result) => flipped(mt.paramTypes foreach this) ; this(result) + case ClassInfoType(parents, _, _) => parents.foreach(apply) + case mt @ MethodType(_, result) => flipped(mt.paramTypes.foreach(apply)); apply(result) case _ => tp.mapOver(this) } // We're using TypeMap here for type traversal only. To avoid wasteful symbol @@ -144,19 +147,63 @@ trait Variances { // As such, we need to expand references to them to retain soundness. Example: neg/t8079a.scala sym.isAliasType && isExemptFromVariance(sym) } - def validateDefinition(base: Symbol): Unit = { + + def validateDefinition(base: Symbol)(continue: => Unit): Unit = { val saved = this.base this.base = base - try apply(base.info) - finally this.base = saved + try { + base.info match { + case PolyType(_, TypeBounds(lo, hi)) => + lowerBoundStack ::= base + try flipped(apply(lo)) + finally lowerBoundStack = lowerBoundStack.tail + apply(hi) + case other => + apply(other) + } + + continue + } finally this.base = saved } } - /** Validate variance of info of symbol `base` */ - private def validateVariance(base: Symbol): Unit = { - ValidateVarianceMap validateDefinition base + private object PolyTypeVarianceMap extends TypeMap { + + private def ownerOf(pt: PolyType): Symbol = + pt.typeParams.head.owner + + private def checkPolyTypeParam(pt: PolyType, tparam: Symbol, tpe: Type): Unit = + if (!tparam.isInvariant) { + val required = varianceInType(tpe)(tparam) + if (!required.isBivariant && tparam.variance != required) + issueVarianceError(ownerOf(pt), tparam, required, pt) + } + + def apply(tp: Type): Type = { + tp match { + case pt @ PolyType(typeParams, TypeBounds(lo, hi)) => + typeParams.foreach { tparam => + checkPolyTypeParam(pt, tparam, lo) + checkPolyTypeParam(pt, tparam, hi) + } + + pt.mapOver(this) + + case pt @ PolyType(typeParams, resultType) => + typeParams.foreach(checkPolyTypeParam(pt, _, resultType)) + pt.mapOver(this) + + case _ => + tp.mapOver(this) + } + + tp + } } + def validateVarianceOfPolyTypesIn(tpe: Type): Unit = + PolyTypeVarianceMap(tpe) + override def traverse(tree: Tree): Unit = { def sym = tree.symbol // No variance check for object-private/protected methods/values. @@ -167,33 +214,31 @@ trait Variances { || sym.owner.isConstructor || sym.owner.isCaseApplyOrUnapply ) + tree match { - case defn: MemberDef if skip => + case ModuleDef(_, _, _) | ValDef(_, _, _, _) | DefDef(_, _, _, _, _, _) if skip => debuglog(s"Skipping variance check of ${sym.defString}") case ClassDef(_, _, _, _) | TypeDef(_, _, _, _) => - validateVariance(sym) - tree.traverse(this) + ValidateVarianceMap.validateDefinition(sym)(tree.traverse(this)) case ModuleDef(_, _, _) => - validateVariance(sym.moduleClass) - tree.traverse(this) + ValidateVarianceMap.validateDefinition(sym.moduleClass)(tree.traverse(this)) case ValDef(_, _, _, _) => - validateVariance(sym) + ValidateVarianceMap.validateDefinition(sym)(()) case DefDef(_, _, tparams, vparamss, _, _) => - validateVariance(sym) - traverseTrees(tparams) - traverseTreess(vparamss) + ValidateVarianceMap.validateDefinition(sym) { + traverseTrees(tparams) + traverseTreess(vparamss) + } case Template(_, _, _) => tree.traverse(this) - case CompoundTypeTree(templ) => + case CompoundTypeTree(_) => tree.traverse(this) - // scala/bug#7872 These two cases make sure we don't miss variance exploits // in originals, e.g. in `foo[({type l[+a] = List[a]})#l]` case tt @ TypeTree() if tt.original != null => tt.original.traverse(this) case tt : TypTree => tt.traverse(this) - case _ => } } @@ -230,7 +275,7 @@ trait Variances { case ThisType(_) | ConstantType(_) => Bivariant case TypeRef(_, tparam, _) if tparam eq this.tparam => Covariant case NullaryMethodType(restpe) => inType(restpe) - case SingleType(pre, sym) => inType(pre) + case SingleType(pre, _) => inType(pre) case TypeRef(pre, _, _) if tp.isHigherKinded => inType(pre) // a type constructor cannot occur in tp's args case TypeRef(pre, sym, args) => inType(pre) & inArgs(sym, args) case TypeBounds(lo, hi) => inType(lo).flip & inType(hi) diff --git a/test/files/neg/t7872.check b/test/files/neg/t7872.check index 57d9772abc5d..a5d052b31c96 100644 --- a/test/files/neg/t7872.check +++ b/test/files/neg/t7872.check @@ -1,7 +1,7 @@ t7872.scala:6: error: contravariant type a occurs in covariant position in type [-a]Cov[a] of type l type x = {type l[-a] = Cov[a]} ^ -t7872.scala:8: error: covariant type a occurs in contravariant position in type [+a]Inv[a] of type l +t7872.scala:8: error: covariant type a occurs in contravariant position in type [+a]Inv[a] of value foo[({type l[+a] = Inv[a]})#l] ^ t7872.scala:5: error: contravariant type a occurs in covariant position in type [-a]Cov[a] of type l diff --git a/test/files/neg/t7872b.check b/test/files/neg/t7872b.check index 0dc4e76301a4..c399d6a1ebe0 100644 --- a/test/files/neg/t7872b.check +++ b/test/files/neg/t7872b.check @@ -1,7 +1,7 @@ -t7872b.scala:8: error: contravariant type a occurs in covariant position in type [-a]List[a] of type l +t7872b.scala:8: error: contravariant type a occurs in covariant position in type [-a]List[a] of value def oops1 = down[({type l[-a] = List[a]})#l](List('whatever: Object)).head + "oops" ^ -t7872b.scala:19: error: covariant type a occurs in contravariant position in type [+a]coinv.Stringer[a] of type l +t7872b.scala:19: error: covariant type a occurs in contravariant position in type [+a]a => String of value def oops2 = up[({type l[+a] = Stringer[a]})#l]("printed: " + _) ^ two errors found diff --git a/test/files/neg/variance-holes.check b/test/files/neg/variance-holes.check new file mode 100644 index 000000000000..10f6fe5dbada --- /dev/null +++ b/test/files/neg/variance-holes.check @@ -0,0 +1,22 @@ +variance-holes.scala:9: error: covariant type x occurs in contravariant position in type [+x, +y] >: F[x,y] of type F2 + def asWiden[F2[+x, +y] >: F[x, y]]: F2[Int, Int] = v + ^ +variance-holes.scala:2: error: contravariant type A occurs in covariant position in type [-A] >: List[A] of type Lower1 + type Lower1[-A] >: List[A] + ^ +variance-holes.scala:5: error: covariant type x occurs in contravariant position in type [+x] >: F[x] of type G + type G[+x] >: F[x] + ^ +variance-holes.scala:13: error: covariant type A occurs in contravariant position in type => AnyRef{type T >: A} of method foo + def foo: { type T >: A } + ^ +variance-holes.scala:17: error: covariant type A occurs in contravariant position in type AnyRef{type T <: A} of value x + def foo(x: { type T <: A }): Unit + ^ +variance-holes.scala:20: error: covariant type A occurs in contravariant position in type <: AnyRef{type T >: A} of type x + class RefinedLower[+A, x <: { type T >: A }] + ^ +variance-holes.scala:21: error: covariant type A occurs in contravariant position in type A of value x_= + private[this] class PrivateThis[+A](var x: A) + ^ +7 errors found diff --git a/test/files/neg/variance-holes.scala b/test/files/neg/variance-holes.scala new file mode 100644 index 000000000000..439a9449bde1 --- /dev/null +++ b/test/files/neg/variance-holes.scala @@ -0,0 +1,22 @@ +object Test { + type Lower1[-A] >: List[A] + + class Lower2[F[-_]] { + type G[+x] >: F[x] + } + + class Lower3[F[-_, -_]](v: F[Int, Int]) { + def asWiden[F2[+x, +y] >: F[x, y]]: F2[Int, Int] = v + } + + trait Refined1[+A] { + def foo: { type T >: A } + } + + trait Refined2[+A] { + def foo(x: { type T <: A }): Unit + } + + class RefinedLower[+A, x <: { type T >: A }] + private[this] class PrivateThis[+A](var x: A) +} \ No newline at end of file diff --git a/test/files/neg/variances.check b/test/files/neg/variances.check index 3c1545a375d7..0d4c0a4781dd 100644 --- a/test/files/neg/variances.check +++ b/test/files/neg/variances.check @@ -1,9 +1,6 @@ variances.scala:4: error: covariant type A occurs in contravariant position in type test.Vector[A] of value x def append(x: Vector[A]): Vector[A] ^ -variances.scala:75: error: covariant type A occurs in contravariant position in type => A => A of value m - val m: A => A - ^ variances.scala:18: error: covariant type A occurs in contravariant position in type A of value a private def setA3(a : A) = this.a = a ^ @@ -22,4 +19,4 @@ variances.scala:89: error: covariant type T occurs in invariant position in type 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 +7 errors found diff --git a/test/files/pos/variance-holes.scala b/test/files/pos/variance-holes.scala new file mode 100644 index 000000000000..2b15c518f11d --- /dev/null +++ b/test/files/pos/variance-holes.scala @@ -0,0 +1,37 @@ +object Test { + type Lower1[+A] >: List[A] + + class Lower2[F[+_]] { + type G[+x] >: F[x] + } + + class Lower3[F[+_, +_]](v: F[Int, Int]) { + def asWiden[F2[+x, +y] >: F[x, y]]: F2[Int, Int] = v + } + + trait Refined1[+A] { + def foo: { type T <: A } + } + + trait Refined2[+A] { + def foo(x: { type T >: A }): Unit + } + + class Refined3[+A] { + generic[{ type T >: A } => Int] + } + + class Refined4[+A] { + generic[{ type T <: A } => Int] + } + + class RefinedUpper1[+A, x <: { type T <: A }] + class RefinedUpper2[+A, x <: { type T[_ <: A] }] + trait RefinedLower[+A, x <: { type T[_ >: A] }] + + class PrivateThis[+A] { + private[this] object Foo { var x: A = _ } + } + + def generic[A]: Unit = () +} \ No newline at end of file