Merge pull request #926 from dotty-staging/stdlib-variance

Handle Scalac variance unsoundness with regards to constructors.

Author: odersky

src/dotty/tools/dotc/config/ScalaSettings.scala

@@ -18,6 +18,7 @@ class ScalaSettings extends Settings.SettingGroup {
    */
   val dependencyfile = StringSetting("-dependencyfile", "file", "Set dependency tracking file.", ".scala_dependencies")
   val deprecation = BooleanSetting("-deprecation", "Emit warning and location for usages of deprecated APIs.")
+  val migration = BooleanSetting("-migration", "Emit warning and location for migration issues from Scala 2.")
   val encoding = StringSetting("-encoding", "encoding", "Specify character encoding used by source files.", Properties.sourceEncoding)
   val explaintypes = BooleanSetting("-explaintypes", "Explain type errors in more detail.")
   val feature = BooleanSetting("-feature", "Emit warning and location for usages of features that should be imported explicitly.")

src/dotty/tools/dotc/core/TypeOps.scala

@@ -5,6 +5,7 @@ package core
 import Contexts._, Types._, Symbols._, Names._, Flags._, Scopes._
 import SymDenotations._, Denotations.Denotation
 import config.Printers._
+import util.Positions._
 import Decorators._
 import StdNames._
 import util.SimpleMap
@@ -572,6 +573,15 @@ trait TypeOps { this: Context => // TODO: Make standalone object.
   /** Is auto-tupling enabled? */
   def canAutoTuple =
     !featureEnabled(defn.LanguageModuleClass, nme.noAutoTupling)
+
+  def scala2Mode =
+    featureEnabled(defn.LanguageModuleClass, nme.Scala2)
+
+  def testScala2Mode(msg: String, pos: Position) = {
+    if (scala2Mode) migrationWarning(msg, pos)
+    scala2Mode
+  }
+
 }
 
 object TypeOps {

src/dotty/tools/dotc/parsing/Parsers.scala

@@ -259,7 +259,11 @@ object Parsers {
     }
 
     /** Cannot use ctx.featureEnabled because accessing the context would force too much */
-    private def scala2mode = ctx.settings.language.value.contains(nme.Scala2.toString)
+    private def testScala2Mode(msg: String, pos: Position = Position(in.offset)) = {
+      val s2 = ctx.settings.language.value.contains(nme.Scala2.toString)
+      if (s2) ctx.migrationWarning(msg, source atPos pos)
+      s2
+    }
 
 /* ---------- TREE CONSTRUCTION ------------------------------------------- */
 
@@ -1726,12 +1730,13 @@ object Parsers {
      *  DefSig ::= id [DefTypeParamClause] ParamClauses
      */
     def defDefOrDcl(mods: Modifiers): Tree = atPos(tokenRange) {
-      def atScala2Brace = scala2mode && in.token == LBRACE
+      def scala2ProcedureSyntax =
+        testScala2Mode("Procedure syntax no longer supported; `=' should be inserted here")
       if (in.token == THIS) {
         in.nextToken()
         val vparamss = paramClauses(nme.CONSTRUCTOR)
         val rhs = {
-          if (!atScala2Brace) accept(EQUALS)
+          if (!(in.token == LBRACE && scala2ProcedureSyntax)) accept(EQUALS)
           atPos(in.offset) { constrExpr() }
         }
         makeConstructor(Nil, vparamss, rhs).withMods(mods)
@@ -1748,7 +1753,7 @@ object Parsers {
           }
           else if (!tpt.isEmpty)
             EmptyTree
-          else if (scala2mode) {
+          else if (scala2ProcedureSyntax) {
             tpt = scalaUnit
             if (in.token == LBRACE) expr()
             else EmptyTree

src/dotty/tools/dotc/reporting/Reporter.scala

@@ -66,6 +66,9 @@ object Reporter {
   class DeprecationWarning(msgFn: => String, pos: SourcePosition) extends ConditionalWarning(msgFn, pos) {
     def enablingOption(implicit ctx: Context) = ctx.settings.deprecation
   }
+  class MigrationWarning(msgFn: => String, pos: SourcePosition) extends ConditionalWarning(msgFn, pos) {
+    def enablingOption(implicit ctx: Context) = ctx.settings.migration
+  }
 }
 
 import Reporter._
@@ -82,6 +85,9 @@ trait Reporting { this: Context =>
   def deprecationWarning(msg: => String, pos: SourcePosition = NoSourcePosition): Unit =
     reporter.report(new DeprecationWarning(msg, pos))
 
+  def migrationWarning(msg: => String, pos: SourcePosition = NoSourcePosition): Unit =
+    reporter.report(new MigrationWarning(msg, pos))
+
   def uncheckedWarning(msg: => String, pos: SourcePosition = NoSourcePosition): Unit =
     reporter.report(new UncheckedWarning(msg, pos))
 

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

@@ -110,16 +110,20 @@ class VarianceChecker()(implicit ctx: Context) {
   private object Traverser extends TreeTraverser {
     def checkVariance(sym: Symbol) = Validator.validateDefinition(sym) match {
       case Some(VarianceError(tvar, required)) =>
-        ctx.error(
-          i"${varianceString(tvar.flags)} $tvar occurs in ${varianceString(required)} position in type ${sym.info} of $sym",
-          sym.pos)
+        def msg = i"${varianceString(tvar.flags)} $tvar occurs in ${varianceString(required)} position in type ${sym.info} of $sym"
+        if (ctx.scala2Mode && sym.owner.isConstructor)
+          ctx.migrationWarning(s"According to new variance rules, this is no longer accepted; need to annotate with @uncheckedVariance:\n$msg", sym.pos)
+        else ctx.error(msg, sym.pos)
       case None =>
     }
 
     override def traverse(tree: Tree)(implicit ctx: Context) = {
       def sym = tree.symbol
       // No variance check for private/protected[this] methods/values.
-      def skip = !sym.exists || sym.is(Local)
+      def skip =
+        !sym.exists ||
+        sym.is(Local) || // !!! watch out for protected local!
+        sym.is(TypeParam) && sym.owner.isClass // already taken care of in primary constructor of class
       tree match {
         case defn: MemberDef if skip =>
           ctx.debuglog(s"Skipping variance check of ${sym.showDcl}")

test/dotc/scala-collections.whitelist

@@ -97,9 +97,7 @@
 # ./scala-scala/src/library/scala/collection/IndexedSeqOptimized.scala
 
 ./scala-scala/src/library/scala/collection/IterableLike.scala
-
-# https://github.com/lampepfl/dotty/issues/913
-#./scala-scala/src/library/scala/collection/Iterator.scala
+./scala-scala/src/library/scala/collection/Iterator.scala
 ./scala-scala/src/library/scala/collection/LinearSeqOptimized.scala
 
 # https://github.com/lampepfl/dotty/issues/914

test/dotc/tests.scala

@@ -102,6 +102,7 @@ class tests extends CompilerTest {
 
   @Test def pos_t2613 = compileFile(posSpecialDir, "t2613")(allowDeepSubtypes)
   @Test def pos_i871 = compileFile(posSpecialDir, "i871", scala2mode)
+  @Test def pos_variancesConstr = compileFile(posSpecialDir, "variances-constr", scala2mode)
 
   @Test def new_all = compileFiles(newDir, twice)
 
@@ -131,12 +132,12 @@ class tests extends CompilerTest {
   @Test def neg_tailcall2 = compileFile(negTailcallDir, "tailrec-2", xerrors = 2)
   @Test def neg_tailcall3 = compileFile(negTailcallDir, "tailrec-3", xerrors = 2)
 
-  @Test def neg_t1279a = compileFile(negDir, "t1279a", xerrors = 1)
   @Test def neg_t1843_variances = compileFile(negDir, "t1843-variances", xerrors = 1)
   @Test def neg_t2660_ambi = compileFile(negDir, "t2660", xerrors = 2)
   @Test def neg_t2994 = compileFile(negDir, "t2994", xerrors = 2)
   @Test def neg_subtyping = compileFile(negDir, "subtyping", xerrors = 5)
   @Test def neg_variances = compileFile(negDir, "variances", xerrors = 2)
+  @Test def neg_variancesConstr = compileFile(negDir, "variances-constr", xerrors = 2)
   @Test def neg_i871_missingReturnType = compileFile(negDir, "i871", xerrors = 2)
   @Test def neg_badAuxConstr = compileFile(negDir, "badAuxConstr", xerrors = 2)
   @Test def neg_typetest = compileFile(negDir, "typetest", xerrors = 1)
@@ -176,7 +177,7 @@ class tests extends CompilerTest {
    .filter(_.nonEmpty)
    .toList
 
-  @Test def compileStdLib = compileList("compileStdLib", stdlibFiles)
+  @Test def compileStdLib = compileList("compileStdLib", stdlibFiles, "-migration" :: scala2mode)
   @Test def dotty = compileDir(dottyDir, ".", "-deep" :: "-Ycheck-reentrant" :: allowDeepSubtypes) // note the -deep argument
 
   @Test def dotc_ast = compileDir(dotcDir, "ast")

tests/neg/variances-constr.scala

@@ -0,0 +1,26 @@
+class C[+A] {
+
+  private[this] var y: A = _
+  def getY: A = y
+
+  class Inner(x: A) { // error A appears contravariantly
+    y = x
+  }
+  class Inner2[B <: A](x: B) { // error A appears contravariantly
+    y = x
+  }
+}
+
+object Test {
+
+  def main(args: Array[String]) = {
+    val x = new C[String]
+    val y: C[Any] = x
+    val i = new y.Inner(1)
+    val s: String = x.getY
+    val i2 = new y.Inner2(1)
+    val s2: String = x.getY
+    println(s)
+  }
+}
+

tests/pos-special/variances-constr.scala

@@ -0,0 +1,27 @@
+class C[+A] {
+
+  private[this] var y: A = _
+  def getY: A = y
+
+  class Inner(x: A) {
+    y = x
+  }
+}
+
+object Test {
+
+  def main(args: Array[String]) = {
+    val x = new C[String]
+    val y: C[Any] = x
+    val i = new y.Inner(1)
+    val s: String = x.getY
+    println(s)
+  }
+}
+
+class CC[+A] {
+  class Inner {
+    def this(a: A) = this()
+  }
+}
+