Skip to content

Handle Scalac variance unsoundness with regards to constructors. #926

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Nov 9, 2015
1 change: 1 addition & 0 deletions src/dotty/tools/dotc/config/ScalaSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
Expand Down
10 changes: 10 additions & 0 deletions src/dotty/tools/dotc/core/TypeOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
13 changes: 9 additions & 4 deletions src/dotty/tools/dotc/parsing/Parsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 ------------------------------------------- */

Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
6 changes: 6 additions & 0 deletions src/dotty/tools/dotc/reporting/Reporter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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._
Expand All @@ -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))

Expand Down
12 changes: 8 additions & 4 deletions src/dotty/tools/dotc/typer/VarianceChecker.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
Expand Down
4 changes: 1 addition & 3 deletions test/dotc/scala-collections.whitelist
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions test/dotc/tests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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")
Expand Down
26 changes: 26 additions & 0 deletions tests/neg/variances-constr.scala
Original file line number Diff line number Diff line change
@@ -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)
}
}

27 changes: 27 additions & 0 deletions tests/pos-special/variances-constr.scala
Original file line number Diff line number Diff line change
@@ -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()
}
}