Merge pull request #4782 from dotty-staging/fix/poly-override

Fix #4771: Relax overriding check for polymorphic methods

Author: smarter

compiler/src/dotty/tools/dotc/core/Denotations.scala

@@ -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
         }
@@ -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)

compiler/src/dotty/tools/dotc/core/TypeComparer.scala

@@ -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
@@ -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 {
@@ -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 &&
@@ -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 =:=

tests/neg/poly-override.scala

@@ -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
+}

tests/pos/poly-override.scala

@@ -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]] = {}
+}