Skip to content

Fix #4771: Relax overriding check for polymorphic methods #4782

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 1 commit into from
Jul 10, 2018
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
83 changes: 56 additions & 27 deletions compiler/src/dotty/tools/dotc/core/Denotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -390,33 +390,48 @@ object Denotations {
case tp2: TypeBounds if tp2 contains tp1 => tp1
case _ => mergeConflict(tp1, tp2, that)
}
case tp1: MethodOrPoly =>

// Two remedial strategies:
//
// 1. Prefer method types over poly types. This is necessary to handle
// overloaded definitions like the following
//
// def ++ [B >: A](xs: C[B]): D[B]
// def ++ (xs: C[A]): D[A]
//
// (Code like this is found in the collection strawman)
//
// 2. In the case of two method types or two polytypes with matching
// parameters and implicit status, merge corresponding parameter
// and result types.
case tp1: MethodType =>
tp2 match {
case tp2: PolyType =>
tp1
case tp2: MethodType if ctx.typeComparer.matchingMethodParams(tp1, tp2) &&
tp1.isImplicitMethod == tp2.isImplicitMethod =>
tp1.derivedLambdaType(
mergeParamNames(tp1, tp2),
tp1.paramInfos,
infoMeet(tp1.resultType, tp2.resultType.subst(tp2, tp1)))
case _ =>
mergeConflict(tp1, tp2, that)
}
case tp1: PolyType =>
tp2 match {
case tp2: MethodOrPoly =>
// Two remedial strategies:
//
// 1. Prefer method types over poly types. This is necessary to handle
// overloaded definitions like the following
//
// def ++ [B >: A](xs: C[B]): D[B]
// def ++ (xs: C[A]): D[A]
//
// (Code like this is found in the collection strawman)
//
// 2. In the case of two method types or two polytypes with matching
// parameters and implicit status, merge corresppnding parameter
// and result types.
if (tp1.isInstanceOf[PolyType] && tp2.isInstanceOf[MethodType]) tp2
else if (tp2.isInstanceOf[PolyType] && tp1.isInstanceOf[MethodType]) tp1
else if (ctx.typeComparer.matchingParams(tp1, tp2) &&
tp1.isImplicitMethod == tp2.isImplicitMethod)
tp1.derivedLambdaType(
mergeParamNames(tp1, tp2), tp1.paramInfos,
infoMeet(tp1.resultType, tp2.resultType.subst(tp2, tp1)))
else mergeConflict(tp1, tp2, that)
case tp2: MethodType =>
tp2
case tp2: PolyType if ctx.typeComparer.matchingPolyParams(tp1, tp2) =>
tp1.derivedLambdaType(
mergeParamNames(tp1, tp2),
tp1.paramInfos.zipWithConserve(tp2.paramInfos) { (p1, p2) =>
infoMeet(p1, p2.subst(tp2, tp1)).bounds
},
infoMeet(tp1.resultType, tp2.resultType.subst(tp2, tp1)))
case _ =>
mergeConflict(tp1, tp2, that)
}

case _ =>
tp1 & tp2
}
Expand Down Expand Up @@ -566,13 +581,27 @@ object Denotations {
case tp2: TypeBounds if tp2 contains tp1 => tp2
case _ => mergeConflict(tp1, tp2, that)
}
case tp1: MethodOrPoly =>
case tp1: MethodType =>
tp2 match {
case tp2: MethodOrPoly
if ctx.typeComparer.matchingParams(tp1, tp2) &&
case tp2: MethodType
if ctx.typeComparer.matchingMethodParams(tp1, tp2) &&
tp1.isImplicitMethod == tp2.isImplicitMethod =>
tp1.derivedLambdaType(
mergeParamNames(tp1, tp2), tp1.paramInfos,
mergeParamNames(tp1, tp2),
tp1.paramInfos,
tp1.resultType | tp2.resultType.subst(tp2, tp1))
case _ =>
mergeConflict(tp1, tp2, that)
}
case tp1: PolyType =>
tp2 match {
case tp2: PolyType
if ctx.typeComparer.matchingPolyParams(tp1, tp2) =>
tp1.derivedLambdaType(
mergeParamNames(tp1, tp2),
tp1.paramInfos.zipWithConserve(tp2.paramInfos) { (p1, p2) =>
(p1 | p2.subst(tp2, tp1)).bounds
},
tp1.resultType | tp2.resultType.subst(tp2, tp1))
case _ =>
mergeConflict(tp1, tp2, that)
Expand Down
55 changes: 38 additions & 17 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -534,13 +534,20 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
either(recur(tp1, tp21), recur(tp1, tp22)) || fourthTry
case tp2: MethodType =>
def compareMethod = tp1 match {
case tp1: MethodType => compareMethodOrPoly(tp1, tp2)
case tp1: MethodType =>
(tp1.signature consistentParams tp2.signature) &&
matchingMethodParams(tp1, tp2) &&
(!tp2.isImplicitMethod || tp1.isImplicitMethod) &&
isSubType(tp1.resultType, tp2.resultType.subst(tp2, tp1))
case _ => false
}
compareMethod
case tp2: PolyType =>
def comparePoly = tp1 match {
case tp1: PolyType => compareMethodOrPoly(tp1, tp2)
case tp1: PolyType =>
(tp1.signature consistentParams tp2.signature) &&
matchingPolyParams(tp1, tp2) &&
isSubType(tp1.resultType, tp2.resultType.subst(tp2, tp1))
case _ => false
}
comparePoly
Expand Down Expand Up @@ -582,12 +589,6 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
fourthTry
}

def compareMethodOrPoly(tp1: MethodOrPoly, tp2: MethodOrPoly) =
(tp1.signature consistentParams tp2.signature) &&
matchingParams(tp1, tp2) &&
(!tp2.isImplicitMethod || tp1.isImplicitMethod) &&
isSubType(tp1.resultType, tp2.resultType.subst(tp2, tp1))

def fourthTry: Boolean = tp1 match {
case tp1: TypeRef =>
tp1.info match {
Expand Down Expand Up @@ -1148,7 +1149,7 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
tp2.widen match {
case tp2: MethodType =>
// implicitness is ignored when matching
matchingParams(tp1, tp2) &&
matchingMethodParams(tp1, tp2) &&
matchesType(tp1.resultType, tp2.resultType.subst(tp2, tp1), relaxed)
case tp2 =>
relaxed && tp1.paramNames.isEmpty &&
Expand All @@ -1174,27 +1175,47 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
}
}

/** Do lambda types `lam1` and `lam2` have parameters that have the same types
* and the same implicit status? (after renaming one set to the other)
/** Do the parameter types of `tp1` and `tp2` match in a way that allows `tp1`
* to override `tp2` ? This is the case if they're pairwise =:=, as a special
* case, we allow `Any` in Java methods to match `Object`.
*/
def matchingParams(lam1: MethodOrPoly, lam2: MethodOrPoly): Boolean = {
/** Are `syms1` and `syms2` parameter lists with pairwise equivalent types? */
def matchingMethodParams(tp1: MethodType, tp2: MethodType): Boolean = {
def loop(formals1: List[Type], formals2: List[Type]): Boolean = formals1 match {
case formal1 :: rest1 =>
formals2 match {
case formal2 :: rest2 =>
val formal2a = if (lam2.isParamDependent) formal2.subst(lam2, lam1) else formal2
val formal2a = if (tp2.isParamDependent) formal2.subst(tp2, tp1) else formal2
(isSameTypeWhenFrozen(formal1, formal2a)
|| lam1.isJavaMethod && (formal2 isRef ObjectClass) && (formal1 isRef AnyClass)
|| lam2.isJavaMethod && (formal1 isRef ObjectClass) && (formal2 isRef AnyClass)) &&
|| tp1.isJavaMethod && (formal2 isRef ObjectClass) && (formal1 isRef AnyClass)
|| tp2.isJavaMethod && (formal1 isRef ObjectClass) && (formal2 isRef AnyClass)) &&
loop(rest1, rest2)
case nil =>
false
}
case nil =>
formals2.isEmpty
}
loop(tp1.paramInfos, tp2.paramInfos)
}

/** Do the parameter types of `tp1` and `tp2` match in a way that allows `tp1`
* to override `tp2` ? This is the case if they're pairwise >:>.
*/
def matchingPolyParams(tp1: PolyType, tp2: PolyType): Boolean = {
def loop(formals1: List[Type], formals2: List[Type]): Boolean = formals1 match {
case formal1 :: rest1 =>
formals2 match {
case formal2 :: rest2 =>
val formal2a = formal2.subst(tp2, tp1)
isSubTypeWhenFrozen(formal2a, formal1) &&
loop(rest1, rest2)
case nil =>
false
}
case nil =>
formals2.isEmpty
}
loop(lam1.paramInfos, lam2.paramInfos)
loop(tp1.paramInfos, tp2.paramInfos)
}

// Type equality =:=
Expand Down
25 changes: 25 additions & 0 deletions tests/neg/poly-override.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
class Super {
def a[T] = {}
def b[T](x: String) = {}
def c[F[_ <: String]] = {}
def c1[F[_ >: String]] = {}
def d[F[_]] = {}

def m(x: String) = {}
def n[T](x: String) = {}
}
class Sub1 extends Super {
override def a[T <: String] = {} // error
override def b[T <: String](x: String) = {} // error
override def c[F[_]] = {} // error
override def c1[F[_]] = {} // error
override def d[F[+_]] = {} // error

override def m(x: Any) = {} // error
override def n[T](x: Any) = {} // error
}

class Sub2 extends Super {
override def a[T >: String] = {} // error
override def b[T >: String](x: String) = {} // error
}
15 changes: 15 additions & 0 deletions tests/pos/poly-override.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
class Super {
def a[T <: String] = {}
def b[T <: String](x: String) = {}
def c[F[_]] = {}
def d[F[+_]] = {}
}
class Sub1 extends Super {
override def a[T <: AnyRef] = {}
override def b[T <: AnyRef](x: String) = {}
override def c[F[_ <: String]] = {}
override def d[F[_]] = {}
}
class Sub2 extends Super {
override def c[F[_ >: String]] = {}
}