ResolveSuper: add missing methods overrides for disambiguation

In mixin-overrides.scala, when we compute the overrides for QInt in
ResolveSuper, we need to add an override for Baz#op because it matches
the concrete method Bar#op (otherwise we'll get a "Conflicting default
methods" error at runtime). Before this commit, this did not happen
because Baz#op was not considered to match Bar#op, because one refers to
Baz.this.A and the other to Bar.this.A.

We fix this by calling `matchingDenotation` with a `targetType`
corresponding to the current class (`QInt` here) instead of the
trait where the denotation is defined.

This also allows us to remove the PrimitiveForwarders mini-phase which was
apparently doing a subset of this work and is now redundant.

Author: smarter

compiler/src/dotty/tools/dotc/Compiler.scala

@@ -83,7 +83,6 @@ class Compiler {
          new ElimOuterSelect,        // Expand outer selections
          new AugmentScala2Traits,    // Expand traits defined in Scala 2.x to simulate old-style rewritings
          new ResolveSuper,           // Implement super accessors and add forwarders to trait methods
-         new PrimitiveForwarders,    // Add forwarders to trait methods that have a mismatch between generic and primitives
          new FunctionXXLForwarders,  // Add forwarders for FunctionXXL apply method
          new ArrayConstructors) ::   // Intercept creation of (non-generic) arrays and intrinsify.
     List(new Erasure) ::             // Rewrite types to JVM model, erasing all type parameters, abstract types and refinements.

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

@@ -1125,10 +1125,10 @@ object SymDenotations {
     final def memberCanMatchInheritedSymbols(implicit ctx: Context): Boolean =
       !isConstructor && !is(Private)
 
-    /** The symbol, in class `inClass`, that is overridden by this denotation. */
-    final def overriddenSymbol(inClass: ClassSymbol)(implicit ctx: Context): Symbol =
+    /** The symbol, in class `inClass`, that is overridden by this denotation in class `siteClass`.*/
+    final def overriddenSymbol(inClass: ClassSymbol, siteClass: ClassSymbol = owner.asClass)(implicit ctx: Context): Symbol =
       if (!canMatchInheritedSymbols && (owner ne inClass)) NoSymbol
-      else matchingDecl(inClass, owner.thisType)
+      else matchingDecl(inClass, siteClass.thisType)
 
     /** All symbols overridden by this denotation. */
     final def allOverriddenSymbols(implicit ctx: Context): Iterator[Symbol] =
@@ -1142,7 +1142,7 @@ object SymDenotations {
 
     private def overriddenFromType(tp: Type)(implicit ctx: Context): Iterator[Symbol] =
       tp.baseClasses match {
-        case _ :: inherited => inherited.iterator map overriddenSymbol filter (_.exists)
+        case _ :: inherited => inherited.iterator.map(overriddenSymbol(_)).filter(_.exists)
         case Nil => Iterator.empty
       }
 

compiler/src/dotty/tools/dotc/transform/MixinOps.scala

@@ -72,28 +72,6 @@ class MixinOps(cls: ClassSymbol, thisPhase: DenotTransformer)(implicit ctx: Cont
     meth.annotations.nonEmpty && JUnit4Annotations.exists(annot => meth.hasAnnotation(annot))
   }
 
-  /** Get `sym` of the method that needs a forwarder
-    *  Method needs a forwarder in those cases:
-    *   - there is a trait that defines a primitive version of implemented polymorphic method.
-    *   - there is a trait that defines a polymorphic version of implemented primitive method.
-    */
-  def needsPrimitiveForwarderTo(meth: Symbol): Option[Symbol] = {
-    def hasPrimitiveMissMatch(tp1: Type, tp2: Type): Boolean = (tp1, tp2) match {
-      case (tp1: MethodicType, tp2: MethodicType) =>
-        hasPrimitiveMissMatch(tp1.resultType, tp2.resultType) ||
-        tp1.paramInfoss.flatten.zip(tp1.paramInfoss.flatten).exists(args => hasPrimitiveMissMatch(args._1, args._2))
-      case _ =>
-        def isPrimitiveOrValueClass(sym: Symbol): Boolean = sym.isPrimitiveValueClass || sym.isValueClass
-        isPrimitiveOrValueClass(tp1.typeSymbol) ^ isPrimitiveOrValueClass(tp2.typeSymbol)
-    }
-
-    def needsPrimitiveForwarder(m: Symbol): Boolean =
-      m.owner != cls && !m.is(Deferred) && hasPrimitiveMissMatch(meth.info, m.info)
-
-    if (!meth.is(Method | Deferred, butNot = PrivateOrAccessor) || meth.overriddenSymbol(cls).exists || needsForwarder(meth)) None
-    else competingMethodsIterator(meth).find(needsPrimitiveForwarder)
-  }
-
   final val PrivateOrAccessor: FlagSet = Private | Accessor
   final val PrivateOrAccessorOrDeferred: FlagSet = Private | Accessor | Deferred
 
@@ -104,7 +82,7 @@ class MixinOps(cls: ClassSymbol, thisPhase: DenotTransformer)(implicit ctx: Cont
   private def competingMethodsIterator(meth: Symbol): Iterator[Symbol] = {
     cls.baseClasses.iterator
       .filter(_ ne meth.owner)
-      .map(meth.overriddenSymbol)
+      .map(base => meth.overriddenSymbol(base, cls))
       .filter(_.exists)
   }
 }

compiler/src/dotty/tools/dotc/transform/PrimitiveForwarders.scala

@@ -142,7 +142,6 @@ phases. The current list of phases is specified in class [Compiler] as follows:
          new ElimOuterSelect,        // Expand outer selections
          new AugmentScala2Traits,    // Expand traits defined in Scala 2.x to simulate old-style rewritings
          new ResolveSuper,           // Implement super accessors and add forwarders to trait methods
-         new PrimitiveForwarders,    // Add forwarders to trait methods that have a mismatch between generic and primitives
          new FunctionXXLForwarders,  // Add forwarders for FunctionXXL apply method
          new ArrayConstructors) ::   // Intercept creation of (non-generic) arrays and intrinsify.
     List(new Erasure) ::             // Rewrite types to JVM model, erasing all type parameters, abstract types and refinements.

docs/docs/internals/overall-structure.md

@@ -0,0 +1,19 @@
+trait Foo[A] {
+  def op(x: A): Int
+}
+trait Bar[A] extends Foo[A] {
+  override def op(x: A): Int = 1
+}
+trait Baz[A] extends Foo[A] {
+  override def op(x: A): Int = 2
+}
+trait Qux[A] extends Bar[A] with Baz[A]
+
+class QInt extends Qux[Int]
+
+object Test {
+  def main(args: Array[String]): Unit = {
+    val qint = new QInt
+    assert(qint.op(1) == 2) // Used to fail with java.lang.IncompatibleClassChangeError: Conflicting default methods: Bar.op Baz.op
+  }
+}