From a0fbe09d945b6b6ef1279e7d6f828bdd3b93cce2 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 2 Nov 2015 18:30:39 +0100 Subject: [PATCH 1/9] Add migration warnings The idea is that whenever Dotty detects a migration problem under -language:Scala2, it should issue a migration warning, so we know what needs to be rewritten. --- src/dotty/tools/dotc/config/ScalaSettings.scala | 1 + src/dotty/tools/dotc/core/TypeComparer.scala | 4 +++- src/dotty/tools/dotc/reporting/Reporter.scala | 6 ++++++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/config/ScalaSettings.scala b/src/dotty/tools/dotc/config/ScalaSettings.scala index 05fefc8b41f4..62b0713724ab 100644 --- a/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/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.") diff --git a/src/dotty/tools/dotc/core/TypeComparer.scala b/src/dotty/tools/dotc/core/TypeComparer.scala index 684e9cbfd8a7..d60828a210ab 100644 --- a/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/src/dotty/tools/dotc/core/TypeComparer.scala @@ -500,7 +500,9 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling { projection.name == tpnme.hkApply && !other.isHKApply && other.testLifted(projection.prefix.LambdaClass(forcing = true).typeParams, - if (inOrder) isSubType(projection.prefix, _) else isSubType(_, projection.prefix), + { xx => println(i"test lifted with $xx") + if (inOrder) isSubType(projection.prefix, xx) else isSubType(xx, projection.prefix) + }, if (inOrder) Nil else classBounds(projection.prefix)) /** The class symbols bounding the type of the `Apply` member of `tp` */ diff --git a/src/dotty/tools/dotc/reporting/Reporter.scala b/src/dotty/tools/dotc/reporting/Reporter.scala index 0358f71f63a1..4a72c2066656 100644 --- a/src/dotty/tools/dotc/reporting/Reporter.scala +++ b/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)) From 4850629d244156d96a2a1bad231cddc0084d0d31 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 2 Nov 2015 18:55:02 +0100 Subject: [PATCH 2/9] Emit a migration warning in Parser when hitting a Scala2 feature. --- src/dotty/tools/dotc/parsing/Parsers.scala | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/dotty/tools/dotc/parsing/Parsers.scala b/src/dotty/tools/dotc/parsing/Parsers.scala index 63d33e4e1d69..8d230d321569 100644 --- a/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/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 scala2mode(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 = + scala2mode("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 (!scala2ProcedureSyntax || in.token != LBRACE) 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 From 4b76eeaa13176aede421af3fe86c392c438b0e5d Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 2 Nov 2015 22:24:54 +0100 Subject: [PATCH 3/9] Add scalaMode and testScalaMode to TypeOps --- src/dotty/tools/dotc/core/TypeOps.scala | 10 ++++++++++ src/dotty/tools/dotc/parsing/Parsers.scala | 4 ++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/dotty/tools/dotc/core/TypeOps.scala b/src/dotty/tools/dotc/core/TypeOps.scala index 77c6805f01a9..aa643256c50b 100644 --- a/src/dotty/tools/dotc/core/TypeOps.scala +++ b/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 { diff --git a/src/dotty/tools/dotc/parsing/Parsers.scala b/src/dotty/tools/dotc/parsing/Parsers.scala index 8d230d321569..e5202b515b8e 100644 --- a/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/src/dotty/tools/dotc/parsing/Parsers.scala @@ -259,7 +259,7 @@ object Parsers { } /** Cannot use ctx.featureEnabled because accessing the context would force too much */ - private def scala2mode(msg: => String, pos: Position = Position(in.offset)) = { + 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 @@ -1731,7 +1731,7 @@ object Parsers { */ def defDefOrDcl(mods: Modifiers): Tree = atPos(tokenRange) { def scala2ProcedureSyntax = - scala2mode("Procedure syntax no longer supported; `=' should be inserted here") + testScala2Mode("Procedure syntax no longer supported; `=' should be inserted here") if (in.token == THIS) { in.nextToken() val vparamss = paramClauses(nme.CONSTRUCTOR) From 1e280d03886e3b20fa28661b0d25b6b11dec56f3 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 5 Nov 2015 14:13:14 +0100 Subject: [PATCH 4/9] Handle variance unsoundness in scalac The included test pos-special/variances-constr.scala demonstrates an unsoundness in the variance checking of scalac. Scalac excludes symbols owned by constructors from the checking. This is unsound, as can be demonstrated by compiling the test and observing output of the program run: Exception in thread "main" java.lang.ClassCastException: java.lang.Integer cannot be cast to java.lang.String at Test$.main(variances-constr.scala:17) at Test.main(variances-constr.scala) Dotty allows this code only under -language:Scala2 and issues a migration warning. --- .../tools/dotc/typer/VarianceChecker.scala | 12 ++++++--- test/dotc/tests.scala | 2 ++ tests/neg/variances-constr.scala | 26 +++++++++++++++++++ tests/pos-special/variances-constr.scala | 21 +++++++++++++++ 4 files changed, 57 insertions(+), 4 deletions(-) create mode 100644 tests/neg/variances-constr.scala create mode 100644 tests/pos-special/variances-constr.scala diff --git a/src/dotty/tools/dotc/typer/VarianceChecker.scala b/src/dotty/tools/dotc/typer/VarianceChecker.scala index 1d3ceaa578b6..5d3bd4f20862 100644 --- a/src/dotty/tools/dotc/typer/VarianceChecker.scala +++ b/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}") diff --git a/test/dotc/tests.scala b/test/dotc/tests.scala index 596c4a27e80d..e37a530842b9 100644 --- a/test/dotc/tests.scala +++ b/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) @@ -137,6 +138,7 @@ class tests extends CompilerTest { @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) diff --git a/tests/neg/variances-constr.scala b/tests/neg/variances-constr.scala new file mode 100644 index 000000000000..a5259c4da1f0 --- /dev/null +++ b/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) + } +} + diff --git a/tests/pos-special/variances-constr.scala b/tests/pos-special/variances-constr.scala new file mode 100644 index 000000000000..b210b044058c --- /dev/null +++ b/tests/pos-special/variances-constr.scala @@ -0,0 +1,21 @@ +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) + } +} + From 7c999a3a52b107e6cf2becf5a9b268fb4e11e4bf Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 5 Nov 2015 14:32:59 +0100 Subject: [PATCH 5/9] Allows Iterator.scala to compile by itself. Fixes #913. --- test/dotc/scala-collections.whitelist | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/dotc/scala-collections.whitelist b/test/dotc/scala-collections.whitelist index effefb3cc858..ede1a14dc2d7 100644 --- a/test/dotc/scala-collections.whitelist +++ b/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 From 0e7c15a9d7bcb3015c24cbd2e16d8d143cfd21de Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 5 Nov 2015 14:35:28 +0100 Subject: [PATCH 6/9] Remove println accidentally left in code. --- src/dotty/tools/dotc/core/TypeComparer.scala | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/dotty/tools/dotc/core/TypeComparer.scala b/src/dotty/tools/dotc/core/TypeComparer.scala index d60828a210ab..684e9cbfd8a7 100644 --- a/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/src/dotty/tools/dotc/core/TypeComparer.scala @@ -500,9 +500,7 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling { projection.name == tpnme.hkApply && !other.isHKApply && other.testLifted(projection.prefix.LambdaClass(forcing = true).typeParams, - { xx => println(i"test lifted with $xx") - if (inOrder) isSubType(projection.prefix, xx) else isSubType(xx, projection.prefix) - }, + if (inOrder) isSubType(projection.prefix, _) else isSubType(_, projection.prefix), if (inOrder) Nil else classBounds(projection.prefix)) /** The class symbols bounding the type of the `Apply` member of `tp` */ From 230cdf74e555e1553515dcfd981015689bd4985a Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 5 Nov 2015 15:33:31 +0100 Subject: [PATCH 7/9] Turn on -language:Scala2 -migration when compiling stdlib needed to turn some errors into warnings. --- test/dotc/tests.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/dotc/tests.scala b/test/dotc/tests.scala index e37a530842b9..b22752b64bfb 100644 --- a/test/dotc/tests.scala +++ b/test/dotc/tests.scala @@ -178,7 +178,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") From 0650ec0d64f93136d5aa99c5850eedb07dab77a3 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 5 Nov 2015 15:45:50 +0100 Subject: [PATCH 8/9] Drop neg test --- test/dotc/tests.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/test/dotc/tests.scala b/test/dotc/tests.scala index b22752b64bfb..c8cbeec945da 100644 --- a/test/dotc/tests.scala +++ b/test/dotc/tests.scala @@ -132,7 +132,6 @@ 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) From 93753dc557c0b628391f706406bf1f942ec053bd Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 9 Nov 2015 09:27:56 +0100 Subject: [PATCH 9/9] Avoid spurious procedure syntax migration warning --- src/dotty/tools/dotc/parsing/Parsers.scala | 2 +- tests/pos-special/variances-constr.scala | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/parsing/Parsers.scala b/src/dotty/tools/dotc/parsing/Parsers.scala index e5202b515b8e..71a03b9e2299 100644 --- a/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/src/dotty/tools/dotc/parsing/Parsers.scala @@ -1736,7 +1736,7 @@ object Parsers { in.nextToken() val vparamss = paramClauses(nme.CONSTRUCTOR) val rhs = { - if (!scala2ProcedureSyntax || in.token != LBRACE) accept(EQUALS) + if (!(in.token == LBRACE && scala2ProcedureSyntax)) accept(EQUALS) atPos(in.offset) { constrExpr() } } makeConstructor(Nil, vparamss, rhs).withMods(mods) diff --git a/tests/pos-special/variances-constr.scala b/tests/pos-special/variances-constr.scala index b210b044058c..ee4219b100d9 100644 --- a/tests/pos-special/variances-constr.scala +++ b/tests/pos-special/variances-constr.scala @@ -19,3 +19,9 @@ object Test { } } +class CC[+A] { + class Inner { + def this(a: A) = this() + } +} +