Skip to content

Commit

Permalink
Plug many variance holes (pos and neg)
Browse files Browse the repository at this point in the history
  • Loading branch information
joroKr21 committed Nov 17, 2019
1 parent 41e9827 commit a4b53e2
Show file tree
Hide file tree
Showing 8 changed files with 158 additions and 58 deletions.
10 changes: 3 additions & 7 deletions src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
Expand Up @@ -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")
}
}

Expand Down Expand Up @@ -1774,13 +1774,9 @@ 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
case _ =>
}

Expand Down
101 changes: 60 additions & 41 deletions src/reflect/scala/reflect/internal/Variances.scala
Expand Up @@ -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
Expand All @@ -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`.
Expand All @@ -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 {
Expand All @@ -104,17 +106,29 @@ 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 ownerOf(pt: PolyType): Symbol =
pt.typeParams.head.owner

private def resultTypeOnly(tp: Type) = tp match {
case mt: MethodType => !inRefinement
case pt: PolyType => true
case _ => false
case _: MethodType => !inRefinement
case pt: PolyType => !ownerOf(pt).isLocalDummy
case _ => false
}

private def checkPolyType(pt: PolyType): Unit = pt.typeParams.foreach { tparam =>
validateDefinition(tparam)(())
if (!tparam.isInvariant) {
val required = varianceInType(pt.resultType)(tparam)
if (tparam.variance != required) issueVarianceError(ownerOf(pt), tparam, required, pt)
}
}

/** For PolyTypes, type parameters are skipped because they are defined
Expand All @@ -125,12 +139,13 @@ 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 pt @ PolyType(_, result) => checkPolyType(pt); apply(result)
case _ => tp.mapOver(this)
}
// We're using TypeMap here for type traversal only. To avoid wasteful symbol
Expand All @@ -144,17 +159,24 @@ 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)
}

/** Validate variance of info of symbol `base` */
private def validateVariance(base: Symbol): Unit = {
ValidateVarianceMap validateDefinition base
continue
} finally this.base = saved
}
}

override def traverse(tree: Tree): Unit = {
Expand All @@ -167,34 +189,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)
case ValDef(_, _, _, _) =>
validateVariance(sym)
case DefDef(_, _, tparams, vparamss, _, _) =>
validateVariance(sym)
traverseTrees(tparams)
traverseTreess(vparamss)
case Template(_, _, _) =>
tree.traverse(this)
case CompoundTypeTree(templ) =>
tree.traverse(this)

ValidateVarianceMap.validateDefinition(sym.moduleClass)(tree.traverse(this))
case ValDef(_, _, _, rhs) =>
ValidateVarianceMap.validateDefinition(sym)(traverse(rhs))
case DefDef(_, _, tparams, vparamss, _, rhs) =>
ValidateVarianceMap.validateDefinition(sym) {
traverseTrees(tparams)
traverseTreess(vparamss)
traverse(rhs)
}
case TypeApply(fun, targs) =>
for (targ <- targs) ValidateVarianceMap(targ.tpe)
traverse(fun)
// 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 _ =>
tree.traverse(this)
}
}
}
Expand Down Expand Up @@ -230,7 +249,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)
Expand Down
8 changes: 4 additions & 4 deletions test/files/neg/t7872.check
@@ -1,10 +1,10 @@
t7872.scala:5: error: contravariant type a occurs in covariant position in type [-a]Cov[a] of type l
type l[-a] = Cov[a]
^
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 <local l>
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
type l[-a] = Cov[a]
^
three errors found
4 changes: 2 additions & 2 deletions 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 <local l>
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 <local l>
def oops2 = up[({type l[+a] = Stringer[a]})#l]("printed: " + _)
^
two errors found
27 changes: 27 additions & 0 deletions test/files/neg/variance-holes.check
@@ -0,0 +1,27 @@
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 [+A]AnyRef {
def <init>(): Test.Refined3[A]
} of class Refined3
class Refined3[+A] {
^
variance-holes.scala:24: 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:25: error: covariant type A occurs in contravariant position in type A of value x_=
private[this] class PrivateThis[+A](var x: A)
^
8 errors found
28 changes: 28 additions & 0 deletions test/files/neg/variance-holes.scala
@@ -0,0 +1,28 @@
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 RefinedLower[+A, x <: { type T >: A }]
private[this] class PrivateThis[+A](var x: A)

def generic[A]: Unit = ()
}
5 changes: 1 addition & 4 deletions 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
^
Expand All @@ -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
33 changes: 33 additions & 0 deletions test/files/pos/variance-holes.scala
@@ -0,0 +1,33 @@
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 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 = ()
}

0 comments on commit a4b53e2

Please sign in to comment.