Skip to content

Fix #2770: Tighten type compatibility checks for overrides #2886

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 3 commits into from
Jul 19, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions compiler/src/dotty/tools/dotc/core/TypeApplications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,19 @@ class TypeApplications(val self: Type) extends AnyVal {
case _ => NoType
}

/** Do self and other have the same kinds (not counting bounds and variances) */
def hasSameKindAs(other: Type)(implicit ctx: Context): Boolean = {
// println(i"check kind $self $other") // DEBUG
val selfResult = self.hkResult
val otherResult = other.hkResult
if (selfResult.exists)
otherResult.exists &&
selfResult.hasSameKindAs(otherResult) &&
self.typeParams.corresponds(other.typeParams)((sparam, oparam) =>
sparam.paramInfo.hasSameKindAs(oparam.paramInfo))
else !otherResult.exists
}

/** Dealias type if it can be done without forcing the TypeRef's info */
def safeDealias(implicit ctx: Context): Type = self match {
case self: TypeRef if self.denot.exists && self.symbol.isAliasType =>
Expand Down
23 changes: 3 additions & 20 deletions compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -109,26 +109,9 @@ object Checking {
* types. Self application needs to be avoided since it can lead to stack overflows.
* Test cases are neg/i2771.scala and neg/i2771b.scala.
*/
def preCheckKind(arg: Tree, paramBounds: TypeBounds)(implicit ctx: Context): Tree = {
def result(tp: Type): Type = tp match {
case tp: HKTypeLambda => tp.resultType
case tp: TypeProxy => result(tp.superType)
case _ => defn.AnyType
}
def kindOK(argType: Type, boundType: Type): Boolean = {
// println(i"check kind rank2$arg $argType $boundType") // DEBUG
val argResult = argType.hkResult
val boundResult = argType.hkResult
if (argResult.exists)
boundResult.exists &&
kindOK(boundResult, argResult) &&
argType.typeParams.corresponds(boundType.typeParams)((ap, bp) =>
kindOK(ap.paramInfo, bp.paramInfo))
else !boundResult.exists
}
if (kindOK(arg.tpe, paramBounds.hi)) arg
else errorTree(arg, em"${arg.tpe} has wrong kind")
}
def preCheckKind(arg: Tree, paramBounds: TypeBounds)(implicit ctx: Context): Tree =
if (arg.tpe.widen.isRef(defn.NothingClass) || arg.tpe.hasSameKindAs(paramBounds.hi)) arg
else errorTree(arg, em"Type argument ${arg.tpe} has not the same kind as its bound $paramBounds")

def preCheckKinds(args: List[Tree], paramBoundss: List[TypeBounds])(implicit ctx: Context): List[Tree] = {
val args1 = args.zipWithConserve(paramBoundss)(preCheckKind)
Expand Down
37 changes: 21 additions & 16 deletions compiler/src/dotty/tools/dotc/typer/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -183,25 +183,27 @@ object RefChecks {
def infoString0(sym: Symbol, showLocation: Boolean) = {
val sym1 = sym.underlyingSymbol
def info = self.memberInfo(sym1)
i"${if (showLocation) sym1.showLocated else sym1}${
val infoStr =
if (sym1.isAliasType) i", which equals ${info.bounds.hi}"
else if (sym1.isAbstractType) i" with bounds$info"
else if (sym1.is(Module)) ""
else if (sym1.isTerm) i" of type $info"
else ""
}"
i"${if (showLocation) sym1.showLocated else sym1}$infoStr"
}

/* Check that all conditions for overriding `other` by `member`
* of class `clazz` are met.
*/
def checkOverride(member: Symbol, other: Symbol): Unit = {
def memberTp = self.memberInfo(member)
def otherTp = self.memberInfo(other)
def memberTp(self: Type) =
if (member.isClass) TypeAlias(member.typeRef.EtaExpand(member.typeParams))
else self.memberInfo(member)
def otherTp(self: Type) = self.memberInfo(other)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why isn't other eta-expanded like member?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other cannot be a class anyway. Classes cannot be overridden.


ctx.debuglog("Checking validity of %s overriding %s".format(member.showLocated, other.showLocated))

def noErrorType = !memberTp.isErroneous && !otherTp.isErroneous
def noErrorType = !memberTp(self).isErroneous && !otherTp(self).isErroneous

def overrideErrorMsg(msg: String): String = {
val isConcreteOverAbstract =
Expand All @@ -212,10 +214,10 @@ object RefChecks {
infoStringWithLocation(other),
infoStringWithLocation(member))
else if (ctx.settings.debug.value)
err.typeMismatchMsg(memberTp, otherTp)
err.typeMismatchMsg(memberTp(self), otherTp(self))
else ""

"overriding %s;\n %s %s%s".format(
"error overriding %s;\n %s %s%s".format(
infoStringWithLocation(other), infoString(member), msg, addendum)
}

Expand Down Expand Up @@ -248,13 +250,16 @@ object RefChecks {

def compatibleTypes(memberTp: Type, otherTp: Type): Boolean =
try
if (member.isType) { // intersection of bounds to refined types must be nonempty
if (member.isType) // intersection of bounds to refined types must be nonempty
member.is(BaseTypeArg) ||
(memberTp frozen_<:< otherTp) || {
val jointBounds = (memberTp.bounds & otherTp.bounds).bounds
jointBounds.lo frozen_<:< jointBounds.hi
}
}
memberTp.bounds.hi.hasSameKindAs(otherTp.bounds.hi) &&
((memberTp frozen_<:< otherTp) ||
!member.owner.derivesFrom(other.owner) && {
// if member and other come from independent classes or traits, their
// bounds must have non-empty-intersection
val jointBounds = (memberTp.bounds & otherTp.bounds).bounds
jointBounds.lo frozen_<:< jointBounds.hi
})
else
member.name.is(DefaultGetterName) || // default getters are not checked for compatibility
memberTp.overrides(otherTp,
Expand Down Expand Up @@ -367,9 +372,9 @@ object RefChecks {
overrideError("cannot be used here - term macros cannot override abstract methods")
} else if (other.is(Macro) && !member.is(Macro)) { // (1.10)
overrideError("cannot be used here - only term macros can override term macros")
} else if (!compatibleTypes(memberTp, otherTp) &&
!compatibleTypes(upwardsSelf.memberInfo(member), upwardsSelf.memberInfo(other))) {
overrideError("has incompatible type" + err.whyNoMatchStr(memberTp, otherTp))
} else if (!compatibleTypes(memberTp(self), otherTp(self)) &&
!compatibleTypes(memberTp(upwardsSelf), otherTp(upwardsSelf))) {
overrideError("has incompatible type" + err.whyNoMatchStr(memberTp(self), otherTp(self)))
} else {
checkOverrideDeprecated()
}
Expand Down
13 changes: 10 additions & 3 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1131,16 +1131,23 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
if (tpt1.symbol.isClass)
tparam match {
case tparam: Symbol =>
// This is needed to get the test `compileParSetSubset` to work
tparam.ensureCompleted()
tparam.ensureCompleted() // This is needed to get the test `compileParSetSubset` to work
case _ =>
}
if (desugaredArg.isType) typed(desugaredArg, argPt)
else desugaredArg.withType(UnspecifiedErrorType)
}
args.zipWithConserve(tparams)(typedArg(_, _)).asInstanceOf[List[Tree]]
}
val args2 = preCheckKinds(args1, tparams.map(_.paramInfo.bounds))
val paramBounds = (tparams, args).zipped.map {
case (tparam, TypeBoundsTree(EmptyTree, EmptyTree)) =>
// if type argument is a wildcard, suppress kind checking since
// there is no real argument.
TypeBounds.empty
case (tparam, _) =>
tparam.paramInfo.bounds
}
val args2 = preCheckKinds(args1, paramBounds)
// check that arguments conform to bounds is done in phase PostTyper
assignType(cpy.AppliedTypeTree(tree)(tpt1, args2), tpt1, args2)
}
Expand Down
14 changes: 14 additions & 0 deletions tests/neg/i2770.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
trait A { type L }
trait B extends A { type L[X] } // error: illegal override

trait A2 { type L <: String }
trait B2 extends A2 { type L[X] <: String } // error: illegal override
trait C2 extends B2 { type L[X, Y] <: String } // error: illegal override

trait D { type I }
trait E extends D { type I <: String }
trait F extends D { type I >: String }
trait G extends E with F // ok

trait H extends D { type I >: Int }
trait H2 extends E with H // error: illegal override
2 changes: 1 addition & 1 deletion tests/neg/i2771.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ object Test {
type LL[F[_]] <: LB[F] // ok

def foo[X[_] <: Any]() = ()
foo[Int]() // an error would be raised later, during PostTyper.
foo[Int]() // error: Int has wrong kind

def bar[X, Y]() = ()
bar[List, Int]() // error: List has wrong kind
Expand Down
4 changes: 2 additions & 2 deletions tests/neg/tcpoly_overloaded.scala
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ trait Monad[T <: Bound[T], MyType[x <: Bound[x]], Bound[_]] {

trait Test {
def moo: MList[Int]
class MList[T](el: T) extends Monad[T, List, Any] { // error: does not conform to upper bound // error
class MList[T](el: T) extends Monad[T, List, Any] { // error: wrong kind
def flatMap[S <: RBound[S], RContainer[x <: RBound[x]], RBound[_],
Result[x <: RBound[x]] <: Monad[x, RContainer, RBound]]
(f: T => Result[S]): Result[S] = sys.error("foo")
Expand All @@ -21,5 +21,5 @@ trait Test {
def flatMap[S]
(f: T => List[S], foo: Int): List[S] = sys.error("foo")
}
val l: MList[String] = moo.flatMap[String, List, Any, MList]((x: Int) => new MList("String")) // error: does not conform to upper bound // error
val l: MList[String] = moo.flatMap[String, List, Any, MList]((x: Int) => new MList("String")) // error: wrong kind
}