diff options
-rw-r--r-- | src/dotty/tools/dotc/core/Types.scala | 1 | ||||
-rw-r--r-- | src/dotty/tools/dotc/typer/Typer.scala | 1 | ||||
-rw-r--r-- | src/dotty/tools/dotc/typer/VarianceChecker.scala (renamed from src/dotty/tools/dotc/typer/CheckVariances.scala) | 82 | ||||
-rw-r--r-- | src/dotty/tools/dotc/typer/Variances.scala | 12 | ||||
-rw-r--r-- | test/dotc/tests.scala | 2 | ||||
-rw-r--r-- | tests/neg/t1843-variances.scala | 25 | ||||
-rw-r--r-- | tests/neg/variances.scala | 44 | ||||
-rw-r--r-- | tests/pos/t1843.scala | 2 | ||||
-rw-r--r-- | tests/pos/typers.scala | 2 | ||||
-rw-r--r-- | tests/pos/variances.scala | 3 |
10 files changed, 134 insertions, 40 deletions
diff --git a/src/dotty/tools/dotc/core/Types.scala b/src/dotty/tools/dotc/core/Types.scala index 19141a873..474958b86 100644 --- a/src/dotty/tools/dotc/core/Types.scala +++ b/src/dotty/tools/dotc/core/Types.scala @@ -2203,6 +2203,7 @@ object Types { override def variance = 1 override def toString = "Co" + super.toString } + final class ContraTypeBounds(lo: Type, hi: Type, hc: Int) extends CachedTypeBounds(lo, hi, hc) { override def variance = -1 override def toString = "Contra" + super.toString diff --git a/src/dotty/tools/dotc/typer/Typer.scala b/src/dotty/tools/dotc/typer/Typer.scala index 729536308..0ce02d198 100644 --- a/src/dotty/tools/dotc/typer/Typer.scala +++ b/src/dotty/tools/dotc/typer/Typer.scala @@ -844,6 +844,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit checkNoDoubleDefs(cls) val impl1 = cpy.Template(impl, constr1, parents1, self1, body1) .withType(dummy.termRef) + VarianceChecker.check(impl1) assignType(cpy.TypeDef(cdef, mods1, name, impl1), cls) // todo later: check that diff --git a/src/dotty/tools/dotc/typer/CheckVariances.scala b/src/dotty/tools/dotc/typer/VarianceChecker.scala index f21a99566..36310767a 100644 --- a/src/dotty/tools/dotc/typer/CheckVariances.scala +++ b/src/dotty/tools/dotc/typer/VarianceChecker.scala @@ -1,63 +1,81 @@ package dotty.tools.dotc -package transform +package typer import dotty.tools.dotc.ast.{ Trees, tpd } import core._ -import Types._, Contexts._, Flags._, Symbols._, Annotations._, Trees._ +import Types._, Contexts._, Flags._, Symbols._, Annotations._, Trees._, NameOps._ import Decorators._ import Variances._ object VarianceChecker { - - case class VarianceError(tvar: Symbol, required: Variance) + private case class VarianceError(tvar: Symbol, required: Variance) + def check(tree: tpd.Tree)(implicit ctx: Context) = + new VarianceChecker()(ctx).Traverser.traverse(tree) } /** See comments at scala.reflect.internal.Variance. */ -class VarianceChecker(implicit ctx: Context) { +class VarianceChecker()(implicit ctx: Context) { import VarianceChecker._ import tpd._ 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.is(Local) + ) + /** 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 = { - if (base.owner == tvar.owner) v - else if ((base is Param) && base.owner.isTerm) relativeVariance(tvar, base.owner.owner, flip(v)) - else if (base.isTerm) Bivariant + def relativeVariance(tvar: Symbol, base: Symbol, v: Variance = Covariant): Variance = /*ctx.traceIndented(i"relative variance of $tvar wrt $base, so far: $v")*/ { + if (base == tvar.owner) v + else if ((base is Param) && base.owner.isTerm) + 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) } - def isUncheckedVariance(tp: Type): Boolean = tp match { - case AnnotatedType(annot, tp1) => - annot.symbol == defn.UncheckedVarianceAnnot || isUncheckedVariance(tp1) - case _ => false - } + /** The next level to take into account when determining the + * relative variance with a method parameter as base. The method + * is always skipped. If the method is a constructor, we also skip + * its class owner, because constructors are not checked for variance + * relative to the type parameters of their own class. On the other + * hand constructors do count for checking the variance of type parameters + * of enclosing classes. I believe the Scala 2 rules are too lenient in + * that respect. + */ + private def paramOuter(meth: Symbol) = + if (meth.isConstructor) meth.owner.owner else meth.owner + /** Check variance of abstract type `tvar` when referred from `base`. */ private def checkVarianceOfSymbol(tvar: Symbol): Option[VarianceError] = { val relative = relativeVariance(tvar, base) - val required = Variances.compose(relative, this.variance) if (relative == Bivariant) None else { - def tvar_s = s"$tvar (${tvar.variance}${tvar.showLocated})" + val required = compose(relative, this.variance) + def tvar_s = s"$tvar (${varianceString(tvar.flags)} ${tvar.showLocated})" def base_s = s"$base in ${base.owner}" + (if (base.owner.isClass) "" else " in " + base.owner.enclosingClass) - ctx.log(s"verifying $tvar_s is $required at $base_s") - if (tvar.variance == required) None + ctx.log(s"verifying $tvar_s is ${varianceString(required)} at $base_s") + ctx.log(s"relative variance: ${varianceString(relative)}") + ctx.log(s"current variance: ${this.variance}") + ctx.log(s"owner chain: ${base.ownersIterator.toList}") + if (tvar is required) None else Some(VarianceError(tvar, required)) } } /** For PolyTypes, type parameters are skipped because they are defined * explicitly (their TypeDefs will be passed here.) For MethodTypes, the - * same is true of the parameters (ValDefs) unless we are inside a - * refinement, in which case they are checked from here. + * same is true of the parameters (ValDefs). */ - def apply(status: Option[VarianceError], tp: Type): Option[VarianceError] = + def apply(status: Option[VarianceError], tp: Type): Option[VarianceError] = ctx.traceIndented(s"variance checking $tp of $base at $variance") { if (status.isDefined) status else tp match { case tp: TypeRef => @@ -71,11 +89,12 @@ class VarianceChecker(implicit ctx: Context) { this(status, tp.resultType) // params will be checked in their ValDef nodes. case AnnotatedType(annot, _) if annot.symbol == defn.UncheckedVarianceAnnot => status - case tp: ClassInfo => - ??? + //case tp: ClassInfo => + // ??? not clear what to do here yet. presumably, it's all checked at local typedefs case _ => foldOver(status, tp) } + } def validateDefinition(base: Symbol): Option[VarianceError] = { val saved = this.base @@ -85,12 +104,7 @@ class VarianceChecker(implicit ctx: Context) { } } - def varianceString(v: Variance) = - if (v is Covariant) "covariant" - else if (v is Contravariant) "contravariant" - else "invariant" - - object Traverser extends TreeTraverser { + private object Traverser extends TreeTraverser { def checkVariance(sym: Symbol) = Validator.validateDefinition(sym) match { case Some(VarianceError(tvar, required)) => ctx.error( @@ -101,14 +115,8 @@ class VarianceChecker(implicit ctx: Context) { override def traverse(tree: Tree) = { def sym = tree.symbol - // No variance check for object-private/protected methods/values. - // Or constructors, or case class factory or extractor. - def skip = ( - sym == NoSymbol - || sym.is(Local) - || sym.owner.isConstructor - //|| sym.owner.isCaseApplyOrUnapply // not clear why needed - ) + // No variance check for private/protected[this] methods/values. + def skip = !sym.exists || sym.is(Local) tree match { case defn: MemberDef if skip => ctx.debuglog(s"Skipping variance check of ${sym.showDcl}") diff --git a/src/dotty/tools/dotc/typer/Variances.scala b/src/dotty/tools/dotc/typer/Variances.scala index d793f7942..fadf9f952 100644 --- a/src/dotty/tools/dotc/typer/Variances.scala +++ b/src/dotty/tools/dotc/typer/Variances.scala @@ -1,5 +1,5 @@ package dotty.tools.dotc -package transform +package typer import dotty.tools.dotc.ast.{Trees, tpd} import core._ @@ -90,4 +90,14 @@ object Variances { case _ => Bivariant } + + def varianceString(v: Variance) = + if (v is Covariant) "covariant" + else if (v is Contravariant) "contravariant" + else "invariant" + + def varianceString(v: Int) = + if (v > 0) "+" + else if (v < 0) "-" + else "" } diff --git a/test/dotc/tests.scala b/test/dotc/tests.scala index 2d06e73a8..fa577573a 100644 --- a/test/dotc/tests.scala +++ b/test/dotc/tests.scala @@ -85,7 +85,9 @@ class tests extends CompilerTest { @Test def neg_tailcall2 = compileFile(negDir, "tailcall/tailrec-2", xerrors = 2) @Test def neg_tailcall3 = compileFile(negDir, "tailcall/tailrec-3", xerrors = 2) @Test def neg_t1843 = compileFile(negDir, "t1843", xerrors = 1) + @Test def neg_t1843_variances = compileFile(negDir, "t1843-variances", xerrors = 1) @Test def neg_t2994 = compileFile(negDir, "t2994", xerrors = 2) + @Test def neg_variances = compileFile(negDir, "variances", xerrors = 2) @Test def dotc = compileDir(dotcDir + "tools/dotc", twice) @Test def dotc_ast = compileDir(dotcDir + "tools/dotc/ast", twice) diff --git a/tests/neg/t1843-variances.scala b/tests/neg/t1843-variances.scala new file mode 100644 index 000000000..e9b5c5d2d --- /dev/null +++ b/tests/neg/t1843-variances.scala @@ -0,0 +1,25 @@ +/** +* Scala Compiler Will Crash On this File +* ... Or Will It? +* +*/ + +object Crash { + trait UpdateType[A] + case class StateUpdate[+A](updateType : UpdateType[A], value : A) + case object IntegerUpdateType extends UpdateType[Integer] + + //However this method will cause a crash + def crash(updates: List[StateUpdate[_]]): Unit = { + updates match { + case Nil => + case u::us => + u match { + //Line below seems to be the crashing line + case StateUpdate(key, newValue) if (key == IntegerUpdateType) => + println("Requires a statement to induce the crash") + case _ => + } + } + } +} diff --git a/tests/neg/variances.scala b/tests/neg/variances.scala new file mode 100644 index 000000000..30390ad22 --- /dev/null +++ b/tests/neg/variances.scala @@ -0,0 +1,44 @@ +// Tests variance checking on default methods +import reflect.ClassTag + +class Foo[+A: ClassTag](x: A) { + + private[this] val elems: Array[A] = Array(x) + + def f[B](x: Array[B] = elems): Array[B] = x // (1) should give a variance error here or ... + +} + +object Test extends App { + + val foo: Foo[Object] = new Foo[String]("A") + + val arr = foo.f[Object]() + + arr(0) = new Integer(1) // (1) ... will give an ArrayStoreException here + +} + +class Outer[+A](x: A) { + + private[this] var elem: A = x + + def getElem: A = elem + + class Inner(constrParam: A) { // (2) should give a variance error here or ... + elem = constrParam + } + +} + +object Test2 extends App { + val o1: Outer[String] = new Outer[String]("A") + val o2: Outer[Object] = o1 + new o2.Inner(new Integer(1)) + + val x: String = o1.getElem // (2) ... will give a classcast exeption here + +} + + + diff --git a/tests/pos/t1843.scala b/tests/pos/t1843.scala index e9b5c5d2d..5e8554a93 100644 --- a/tests/pos/t1843.scala +++ b/tests/pos/t1843.scala @@ -5,7 +5,7 @@ */ object Crash { - trait UpdateType[A] + trait UpdateType[+A] case class StateUpdate[+A](updateType : UpdateType[A], value : A) case object IntegerUpdateType extends UpdateType[Integer] diff --git a/tests/pos/typers.scala b/tests/pos/typers.scala index a95af558e..b2d786c1c 100644 --- a/tests/pos/typers.scala +++ b/tests/pos/typers.scala @@ -32,7 +32,7 @@ object typers { } class List[+T] { - def :: (x: T) = new :: (x, this) + def :: [U >: T](x: U): List[U] = new :: (x, this) def len: Int = this match { case x :: xs1 => 1 + xs1.len diff --git a/tests/pos/variances.scala b/tests/pos/variances.scala new file mode 100644 index 000000000..db858fd5d --- /dev/null +++ b/tests/pos/variances.scala @@ -0,0 +1,3 @@ +trait C[+T <: C[T, U], -U <: C[T, U]] { + +} |