Enforce non-variant refinement for parameterized base classes and traits

Author: odersky

compiler/src/dotty/tools/dotc/typer/Inferencing.scala

@@ -190,7 +190,8 @@ object Inferencing {
     def refinementIsInvariant(tp: Type): Boolean = tp match {
       case tp: ClassInfo => tp.cls.is(Final) || tp.cls.is(Case)
       case tp: TypeProxy => refinementIsInvariant(tp.underlying)
-      case tp: AndOrType => refinementIsInvariant(tp.tp1) && refinementIsInvariant(tp.tp2)
+      case tp: AndType => refinementIsInvariant(tp.tp1) && refinementIsInvariant(tp.tp2)
+      case tp: OrType => refinementIsInvariant(tp.tp1) && refinementIsInvariant(tp.tp2)
       case _ => false
     }
 

compiler/src/dotty/tools/dotc/typer/RefChecks.scala

@@ -647,21 +647,11 @@ object RefChecks {
        */
       def checkCaseClassInheritanceInvariant() = {
         for (caseCls <- clazz.info.baseClasses.tail.find(_.is(Case)))
-          for (bc <- caseCls.info.baseClasses.tail)
-            if (bc.typeParams.exists(_.paramVariance != 0)) {
-              val caseBT = self.baseType(caseCls)
-              val thisBT = self.baseType(bc)
-              val combinedBT = caseBT.baseType(bc)
-              if (!(thisBT =:= combinedBT))
-                ctx.errorOrMigrationWarning(
-                  em"""illegal inheritance: $clazz inherits case $caseCls
-                      |but the two have different base type instances for $bc.
-                      |
-                      |  Basetype for $clazz: $thisBT
-                      |  Basetype via $caseCls: $combinedBT""", clazz.pos)
-            }
+          for (baseCls <- caseCls.info.baseClasses.tail)
+            if (baseCls.typeParams.exists(_.paramVariance != 0))
+              for (problem <- variantInheritanceProblems(baseCls, caseCls, "non-variant", "case "))
+                ctx.errorOrMigrationWarning(problem(), clazz.pos)
       }
-
       checkNoAbstractMembers()
       if (abstractErrors.isEmpty)
         checkNoAbstractDecls(clazz)
@@ -684,6 +674,51 @@ object RefChecks {
       }
     }
 
+    if (!clazz.is(Trait)) {
+      // check that parameterized base classes and traits are typed in the same way as from the superclass
+      // I.e. say we have
+      //
+      //    Sub extends Super extends* Base
+      //
+      // where `Base` has value parameters. Enforce that
+      //
+      //    Sub.thisType.baseType(Base)  =:=  Sub.thisType.baseType(Super).baseType(Base)
+      //
+      // This is necessary because parameter values are determined directly or indirectly
+      // by `Super`. So we cannot pretend they have a different type when seen from `Sub`.
+      def checkParameterizedTraitsOK() = {
+        val mixins = clazz.mixins
+        for {
+          cls <- clazz.info.baseClasses.tail
+          if cls.paramAccessors.nonEmpty && !mixins.contains(cls)
+          problem <- variantInheritanceProblems(cls, clazz.asClass.superClass, "parameterized", "super")
+        } ctx.error(problem(), clazz.pos)
+      }
+
+      checkParameterizedTraitsOK()
+    }
+
+    /** Check that `site` does not inherit conflicting generic instances of `baseCls`,
+     *  when doing a direct base type or going via intermediate class `middle`. I.e, we require:
+     *
+     *     site.baseType(baseCls)  =:=  site.baseType(middle).baseType(baseCls)
+     *
+     *  Return an optional by name error message if this test fails.
+     */
+    def variantInheritanceProblems(
+        baseCls: Symbol, middle: Symbol, baseStr: String, middleStr: String): Option[() => String] = {
+      val superBT = self.baseType(middle)
+      val thisBT = self.baseType(baseCls)
+      val combinedBT = superBT.baseType(baseCls)
+      if (combinedBT =:= thisBT) None // ok
+      else
+        Some(() =>
+          em"""illegal inheritance: $clazz inherits conflicting instances of $baseStr base $baseCls.
+              |
+              |  Direct basetype: $thisBT
+              |  Basetype via $middleStr$middle: $combinedBT""")
+    }
+
     /* Returns whether there is a symbol declared in class `inclazz`
        * (which must be different from `clazz`) whose name and type
        * seen as a member of `class.thisType` matches `member`'s.

tests/neg/i3989f.scala

@@ -0,0 +1,13 @@
+object Test extends App {
+  trait A[+X](val x: X)
+  class B[+X](val y: X) extends A[X](y)
+  class C extends B(5) with A[String] // error: illegal inheritance
+
+  class D extends B(5) with A[Any] // ok
+
+  def f(a: A[Int]): String = a match {
+    case c: C => c.x
+    case _ => "hello"
+  }
+  f(new C)
+}

tests/neg/i3989g.scala

@@ -0,0 +1,12 @@
+object Test extends App {
+  class A[+X](val x: X)
+  class B[+X](val y: X) extends A[X](y)
+  trait T[+X] extends A[X]
+  class C extends B(5) with T[String] // error: illegal inheritance
+
+  def f(a: A[Int]): String = a match {
+    case c: C => c.x
+    case _ => "hello"
+  }
+  f(new C)
+}