Fix overriding problems

src/dotty/annotation/internal/Child.scala

@@ -2,5 +2,14 @@ package dotty.annotation.internal
 
 import scala.annotation.Annotation
 
-/** An annotation to indicate a child class or object of the annotated class. */
+/** An annotation to indicate a child class or object of the annotated class.
+ *  E.g. if we have
+ *
+ *    sealed class A
+ *    case class B() extends A
+ *    case class C() extends A
+ *
+ *  Then `A` would carry the annotations `@Child[B] @Child[C]` where
+ *  `B`, `C` are TypeRefs.
+ */
 class Child[T] extends Annotation

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

@@ -122,11 +122,11 @@ object Denotations {
     /** The signature of the denotation. */
     def signature(implicit ctx: Context): Signature
 
-    /** Resolve overloaded denotation to pick the one with the given signature
+    /** Resolve overloaded denotation to pick the ones with the given signature
      *  when seen from prefix `site`.
      *  @param relaxed  When true, consider only parameter signatures for a match.
      */
-    def atSignature(sig: Signature, site: Type = NoPrefix, relaxed: Boolean = false)(implicit ctx: Context): SingleDenotation
+    def atSignature(sig: Signature, site: Type = NoPrefix, relaxed: Boolean = false)(implicit ctx: Context): Denotation
 
     /** The variant of this denotation that's current in the given context, or
      *  `NotDefinedHereDenotation` if this denotation does not exist at current phase, but
@@ -157,7 +157,10 @@ object Denotations {
      *  or NoDenotation if no satisfying alternative exists.
      *  @throws TypeError if there is at more than one alternative that satisfies `p`.
      */
-    def suchThat(p: Symbol => Boolean): SingleDenotation
+    def suchThat(p: Symbol => Boolean)(implicit ctx: Context): SingleDenotation
+
+    /** If this is a SingleDenotation, return it, otherwise throw a TypeError */
+    def checkUnique(implicit ctx: Context): SingleDenotation = suchThat(alwaysTrue)
 
     /** Does this denotation have an alternative that satisfies the predicate `p`? */
     def hasAltWith(p: SingleDenotation => Boolean): Boolean
@@ -227,13 +230,17 @@ object Denotations {
     /** The alternative of this denotation that has a type matching `targetType` when seen
      *  as a member of type `site`, `NoDenotation` if none exists.
      */
-    def matchingDenotation(site: Type, targetType: Type)(implicit ctx: Context): SingleDenotation =
-      if (isOverloaded)
-        atSignature(targetType.signature, site, relaxed = true).matchingDenotation(site, targetType)
-      else if (exists && !site.memberInfo(symbol).matchesLoosely(targetType))
-        NoDenotation
-      else
-        asSingleDenotation
+    def matchingDenotation(site: Type, targetType: Type)(implicit ctx: Context): SingleDenotation = {
+      def qualifies(sym: Symbol) = site.memberInfo(sym).matchesLoosely(targetType)
+      if (isOverloaded) {
+        atSignature(targetType.signature, site, relaxed = true) match {
+          case sd: SingleDenotation => sd.matchingDenotation(site, targetType)
+          case md => md.suchThat(qualifies(_))
+        }
+      }
+      else if (exists && !qualifies(symbol)) NoDenotation
+      else asSingleDenotation
+     }
 
     /** Form a denotation by conjoining with denotation `that`.
      *
@@ -282,8 +289,10 @@ object Denotations {
             val info2 = denot2.info
             val sym1 = denot1.symbol
             val sym2 = denot2.symbol
-            val sym2Accessible = sym2.isAccessibleFrom(pre)
 
+            if (isDoubleDef(sym1, sym2)) doubleDefError(denot1, denot2, pre)
+
+            val sym2Accessible = sym2.isAccessibleFrom(pre)
             /** Does `sym1` come before `sym2` in the linearization of `pre`? */
             def precedes(sym1: Symbol, sym2: Symbol) = {
               def precedesIn(bcs: List[ClassSymbol]): Boolean = bcs match {
@@ -418,19 +427,21 @@ object Denotations {
     final def validFor = denot1.validFor & denot2.validFor
     final def isType = false
     final def signature(implicit ctx: Context) = Signature.OverloadedSignature
-    def atSignature(sig: Signature, site: Type, relaxed: Boolean)(implicit ctx: Context): SingleDenotation =
-      denot1.atSignature(sig, site, relaxed) orElse denot2.atSignature(sig, site, relaxed)
+    def atSignature(sig: Signature, site: Type, relaxed: Boolean)(implicit ctx: Context): Denotation =
+      derivedMultiDenotation(denot1.atSignature(sig, site, relaxed), denot2.atSignature(sig, site, relaxed))
     def currentIfExists(implicit ctx: Context): Denotation =
       derivedMultiDenotation(denot1.currentIfExists, denot2.currentIfExists)
     def current(implicit ctx: Context): Denotation =
       derivedMultiDenotation(denot1.current, denot2.current)
     def altsWith(p: Symbol => Boolean): List[SingleDenotation] =
       denot1.altsWith(p) ++ denot2.altsWith(p)
-    def suchThat(p: Symbol => Boolean): SingleDenotation = {
+    def suchThat(p: Symbol => Boolean)(implicit ctx: Context): SingleDenotation = {
       val sd1 = denot1.suchThat(p)
       val sd2 = denot2.suchThat(p)
       if (sd1.exists)
-        if (sd2.exists) throw new TypeError(s"failure to disambiguate overloaded reference $this")
+        if (sd2.exists)
+          if (isDoubleDef(denot1.symbol, denot2.symbol)) doubleDefError(denot1, denot2)
+          else throw new TypeError(s"failure to disambiguate overloaded reference $this")
         else sd1
       else sd2
     }
@@ -480,7 +491,7 @@ object Denotations {
     def altsWith(p: Symbol => Boolean): List[SingleDenotation] =
       if (exists && p(symbol)) this :: Nil else Nil
 
-    def suchThat(p: Symbol => Boolean): SingleDenotation =
+    def suchThat(p: Symbol => Boolean)(implicit ctx: Context): SingleDenotation =
       if (exists && p(symbol)) this else NoDenotation
 
     def hasAltWith(p: SingleDenotation => Boolean): Boolean =
@@ -900,6 +911,27 @@ object Denotations {
    */
   case class NoQualifyingRef(alts: List[SingleDenotation])(implicit ctx: Context) extends ErrorDenotation
 
+  /** A double definition
+   */
+  def isDoubleDef(sym1: Symbol, sym2: Symbol)(implicit ctx: Context): Boolean =
+    (sym1.exists && sym2.exists &&
+    (sym1 ne sym2) && (sym1.owner eq sym2.owner) &&
+    !sym1.is(Bridge) && !sym2.is(Bridge))
+
+  def doubleDefError(denot1: Denotation, denot2: Denotation, pre: Type = NoPrefix)(implicit ctx: Context): Nothing = {
+    val sym1 = denot1.symbol
+    val sym2 = denot2.symbol
+    def fromWhere = if (pre == NoPrefix) "" else i"\nwhen seen as members of $pre"
+    throw new MergeError(
+      i"""cannot merge
+         |  $sym1: ${sym1.info}  and
+         |  $sym2: ${sym2.info};
+         |they are both defined in ${sym1.owner} but have matching signatures
+         |  ${denot1.info} and
+         |  ${denot2.info}$fromWhere""".stripMargin,
+      denot2.info, denot2.info)
+  }
+
   // --------------- PreDenotations -------------------------------------------------
 
   /** A PreDenotation represents a group of single denotations

src/dotty/tools/dotc/core/Types.scala

@@ -1437,7 +1437,11 @@ object Types {
       asMemberOf(prefix) match {
         case NoDenotation => d.current
         case newd: SingleDenotation => newd
-        case newd => newd.atSignature(d.signature).orElse(d.current)
+        case newd =>
+          newd.atSignature(d.signature) match {
+            case newd1: SingleDenotation if newd1.exists => newd1
+            case _ => d.current
+          }
       }
 
     private def denotOfSym(sym: Symbol)(implicit ctx: Context): Denotation = {
@@ -1729,7 +1733,7 @@ object Types {
     override def loadDenot(implicit ctx: Context): Denotation = {
       val d = super.loadDenot
       if (sig eq Signature.OverloadedSignature) d
-      else d.atSignature(sig)
+      else d.atSignature(sig).checkUnique
     }
 
     override def newLikeThis(prefix: Type)(implicit ctx: Context): TermRef = {

src/dotty/tools/dotc/transform/Erasure.scala

@@ -41,24 +41,30 @@ class Erasure extends Phase with DenotTransformer { thisTransformer =>
         // Aftre erasure, all former Any members are now Object members
         val ClassInfo(pre, _, ps, decls, selfInfo) = ref.info
         val extendedScope = decls.cloneScope
-        defn.AnyClass.classInfo.decls.foreach(extendedScope.enter)
+        for (decl <- defn.AnyClass.classInfo.decls)
+          if (!decl.isConstructor) extendedScope.enter(decl)
         ref.copySymDenotation(
           info = transformInfo(ref.symbol,
               ClassInfo(pre, defn.ObjectClass, ps, extendedScope, selfInfo))
         )
       }
       else {
+        val oldSymbol = ref.symbol
+        val newSymbol =
+          if ((oldSymbol.owner eq defn.AnyClass) && oldSymbol.isConstructor)
+            defn.ObjectClass.primaryConstructor
+        else oldSymbol
         val oldOwner = ref.owner
         val newOwner = if (oldOwner eq defn.AnyClass) defn.ObjectClass else oldOwner
         val oldInfo = ref.info
         val newInfo = transformInfo(ref.symbol, oldInfo)
         val oldFlags = ref.flags
         val newFlags = ref.flags &~ Flags.HasDefaultParams // HasDefaultParams needs to be dropped because overriding might become overloading
         // TODO: define derivedSymDenotation?
-        if ((oldOwner eq newOwner) && (oldInfo eq newInfo) && (oldFlags == newFlags)) ref
+        if ((oldSymbol eq newSymbol) && (oldOwner eq newOwner) && (oldInfo eq newInfo) && (oldFlags == newFlags)) ref
         else {
           assert(!ref.is(Flags.PackageClass), s"trans $ref @ ${ctx.phase} oldOwner = $oldOwner, newOwner = $newOwner, oldInfo = $oldInfo, newInfo = $newInfo ${oldOwner eq newOwner} ${oldInfo eq newInfo}")
-          ref.copySymDenotation(owner = newOwner, initFlags = newFlags, info = newInfo)
+          ref.copySymDenotation(symbol = newSymbol, owner = newOwner, initFlags = newFlags, info = newInfo)
         }
       }
     case ref =>
@@ -553,43 +559,47 @@ object Erasure extends TypeTestsCasts{
             before match {
               case Nil => emittedBridges.toList
               case (oldMember: untpd.DefDef) :: oldTail =>
-                val oldSymbol = oldMember.symbol(beforeCtx)
-                val newSymbol = member.symbol(ctx)
-                assert(oldSymbol.name(beforeCtx) == newSymbol.name,
-                  s"${oldSymbol.name(beforeCtx)} bridging with ${newSymbol.name}")
-                val newOverridden = oldSymbol.denot.allOverriddenSymbols.toSet // TODO: clarify new <-> old in a comment; symbols are swapped here
-                val oldOverridden = newSymbol.allOverriddenSymbols(beforeCtx).toSet // TODO: can we find a more efficient impl? newOverridden does not have to be a set!
-                def stillInBaseClass(sym: Symbol) = ctx.owner derivesFrom sym.owner
-                val neededBridges = (oldOverridden -- newOverridden).filter(stillInBaseClass)
-
-                var minimalSet = Set[Symbol]()
-                // compute minimal set of bridges that are needed:
-                for (bridge <- neededBridges) {
-                  val isRequired = minimalSet.forall(nxtBridge => !(bridge.info =:= nxtBridge.info))
-
-                  if (isRequired) {
-                    // check for clashes
-                    val clash: Option[Symbol] = oldSymbol.owner.info.decls.lookupAll(bridge.name).find {
-                      sym =>
-                        (sym.name eq bridge.name) && sym.info.widen =:= bridge.info.widen
-                    }.orElse(
+                try {
+                  val oldSymbol = oldMember.symbol(beforeCtx)
+                  val newSymbol = member.symbol(ctx)
+                  assert(oldSymbol.name(beforeCtx) == newSymbol.name,
+                    s"${oldSymbol.name(beforeCtx)} bridging with ${newSymbol.name}")
+                  val newOverridden = oldSymbol.denot.allOverriddenSymbols.toSet // TODO: clarify new <-> old in a comment; symbols are swapped here
+                  val oldOverridden = newSymbol.allOverriddenSymbols(beforeCtx).toSet // TODO: can we find a more efficient impl? newOverridden does not have to be a set!
+                  def stillInBaseClass(sym: Symbol) = ctx.owner derivesFrom sym.owner
+                  val neededBridges = (oldOverridden -- newOverridden).filter(stillInBaseClass)
+
+                  var minimalSet = Set[Symbol]()
+                  // compute minimal set of bridges that are needed:
+                  for (bridge <- neededBridges) {
+                    val isRequired = minimalSet.forall(nxtBridge => !(bridge.info =:= nxtBridge.info))
+
+                    if (isRequired) {
+                      // check for clashes
+                      val clash: Option[Symbol] = oldSymbol.owner.info.decls.lookupAll(bridge.name).find {
+                        sym =>
+                          (sym.name eq bridge.name) && sym.info.widen =:= bridge.info.widen
+                      }.orElse(
                         emittedBridges.find(stat => (stat.name == bridge.name) && stat.tpe.widen =:= bridge.info.widen)
-                          .map(_.symbol)
-                      )
-                    clash match {
-                      case Some(cl) =>
-                        ctx.error(i"bridge for method ${newSymbol.showLocated(beforeCtx)} of type ${newSymbol.info(beforeCtx)}\n" +
-                          i"clashes with ${cl.symbol.showLocated(beforeCtx)} of type ${cl.symbol.info(beforeCtx)}\n" +
-                          i"both have same type after erasure: ${bridge.symbol.info}")
-                      case None => minimalSet += bridge
+                          .map(_.symbol))
+                      clash match {
+                        case Some(cl) =>
+                          ctx.error(i"bridge for method ${newSymbol.showLocated(beforeCtx)} of type ${newSymbol.info(beforeCtx)}\n" +
+                            i"clashes with ${cl.symbol.showLocated(beforeCtx)} of type ${cl.symbol.info(beforeCtx)}\n" +
+                            i"both have same type after erasure: ${bridge.symbol.info}")
+                        case None => minimalSet += bridge
+                      }
                     }
                   }
-                }
 
-                val bridgeImplementations = minimalSet.map {
-                  sym => makeBridgeDef(member, sym)(ctx)
+                  val bridgeImplementations = minimalSet.map {
+                    sym => makeBridgeDef(member, sym)(ctx)
+                  }
+                  emittedBridges ++= bridgeImplementations
+                } catch {
+                  case ex: MergeError => ctx.error(ex.getMessage, member.pos)
                 }
-                emittedBridges ++= bridgeImplementations
+
                 traverse(newTail, oldTail, emittedBridges)
               case notADefDef :: oldTail =>
                 traverse(after, oldTail, emittedBridges)

src/dotty/tools/dotc/transform/Splitter.scala

@@ -46,7 +46,7 @@ class Splitter extends MiniPhaseTransform { thisTransform =>
       val mbr = tp.member(name)
       if (!mbr.isOverloaded) mbr.asSingleDenotation
       else tree.tpe match {
-        case tref: TermRefWithSignature => mbr.atSignature(tref.sig)
+        case tref: TermRefWithSignature => mbr.atSignature(tref.sig).checkUnique
         case _ =>
           def alts = mbr.alternatives.map(alt => i"$alt: ${alt.info}").mkString(", ")
           ctx.error(s"cannot disambiguate overloaded members $alts", tree.pos)

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

@@ -837,14 +837,18 @@ class RefChecks extends MiniPhase { thisTransformer =>
       if (tree.symbol is Macro) EmptyTree else tree
     }
 
-    override def transformTemplate(tree: Template)(implicit ctx: Context, info: TransformerInfo) = {
+    override def transformTemplate(tree: Template)(implicit ctx: Context, info: TransformerInfo) = try {
       val cls = ctx.owner
       checkOverloadedRestrictions(cls)
       checkParents(cls)
       checkCompanionNameClashes(cls)
       checkAllOverrides(cls)
       checkDerivedValueClass(cls, tree.body)
       tree
+    } catch {
+      case ex: MergeError =>
+        ctx.error(ex.getMessage, tree.pos)
+        tree
     }
 
     override def transformTypeTree(tree: TypeTree)(implicit ctx: Context, info: TransformerInfo) = {

test/dotc/tests.scala

@@ -119,10 +119,11 @@ class tests extends CompilerTest {
   @Test def neg_typedIdents() = compileDir(negDir, "typedIdents")
 
   val negCustomArgs = negDir + "customArgs/"
-  @Test def neg_typers() = compileFile(negCustomArgs, "typers")(allowDoubleBindings)
+  @Test def neg_typers = compileFile(negCustomArgs, "typers")(allowDoubleBindings)
   @Test def neg_overrideClass = compileFile(negCustomArgs, "overrideClass", List("-language:Scala2"))
   @Test def neg_autoTupling = compileFile(negCustomArgs, "autoTuplingTest", args = "-language:noAutoTupling" :: Nil)
   @Test def neg_i1050 = compileFile(negCustomArgs, "i1050", List("-strict"))
+  @Test def neg_i1240 = compileFile(negCustomArgs, "i1240")(allowDoubleBindings)
 
   val negTailcallDir = negDir + "tailcall/"
   @Test def neg_tailcall_t1672b = compileFile(negTailcallDir, "t1672b")
@@ -155,7 +156,8 @@ class tests extends CompilerTest {
 
   @Test def dotc_ast = compileDir(dotcDir, "ast")
   @Test def dotc_config = compileDir(dotcDir, "config")
-  @Test def dotc_core = compileDir(dotcDir, "core")("-Yno-double-bindings" :: allowDeepSubtypes)// twice omitted to make tests run faster
+  @Test def dotc_core = compileDir(dotcDir, "core")(allowDeepSubtypes)// twice omitted to make tests run faster
+  @Test def dotc_core_nocheck = compileDir(dotcDir, "core")(noCheckOptions)
 
 // This directory doesn't exist anymore
 //  @Test def dotc_core_pickling = compileDir(coreDir, "pickling")(allowDeepSubtypes)// twice omitted to make tests run faster

tests/neg/customArgs/i1240.scala

@@ -0,0 +1,26 @@
+package test
+
+class C[T] {
+
+    def foo(x: D) = { System.out.println("D foo"); }
+    def foo(x: T) = { System.out.println("T foo"); }
+}
+
+object C {
+  def main(args: Array[String]) =
+        new C[D]().foo(new D()) // error: ambiguous
+}
+
+class C1[T] {
+    def foo(x: D) = { System.out.println("D foo"); }
+}
+class C2[T] {
+    def foo(x: D) = { System.out.println("D foo"); }
+}
+
+class D {}
+
+class X {
+  def foo(x: D): D
+  def foo(x: D): D // error: already defined
+}

tests/neg/i1240a.scala

@@ -0,0 +1,17 @@
+
+// more complicated example
+abstract class A {
+  type C[X]
+  def foo[B](x: C[B]): C[B]       = {println("A.C"); x}
+  def foo[B](x: List[B]): List[B] = {println("A.List"); x}
+  def give[X]: C[X]
+}
+
+class B extends A {
+  type C[X] = List[X]
+  override def give[X] = Nil
+  override def foo[B](x: C[B]): C[B] =  {println("B.C"); x} // error: merge error during erasure
+  val a: A = this
+  a.foo(a.give[Int]) // what method should be called here in runtime?
+}
+

tests/neg/i1240b.scala

@@ -0,0 +1,12 @@
+// yet another variant, testing type parameters
+trait T[X] {
+  def foo(x: X): X
+}
+abstract class A[X] extends T[X] {
+  def foo(x: X): X           = {println("A.X"); x}
+  def foo(x: String): String = {println("A.String"); x}
+}
+trait U[X] extends T[X] {
+  abstract override def foo(x: X): X = super.foo(x)
+}
+object Test extends A[String] with U[String] // error: accidental override