Issue MergeError exception for double def situations

When finding two symbols in the same class that have the same signature
as seen from some prefix, issue a merge error.

This is simpler and more robust than the alternative of producing an overloaded
denotation and dealing with it afterwards.

4 files changed, 85 insertions, 57 deletions

diff --git a/src/dotty/tools/dotc/core/Denotations.scala b/src/dotty/tools/dotc/core/Denotations.scala
index 8003e68ac..0bcb3198b 100644
--- a/src/dotty/tools/dotc/core/Denotations.scala
+++ b/src/dotty/tools/dotc/core/Denotations.scala
@@ -283,12 +283,7 @@ object Denotations {
             val sym1 = denot1.symbol
             val sym2 = denot2.symbol
 
-            if (sym1.exists && sym2.exists &&
-                (sym1 ne sym2) && (sym1.owner eq sym2.owner) &&
-                !sym1.is(Bridge) && !sym2.is(Bridge))
-              // double definition of two methods with same signature in one class;
-              // don't merge them.
-              return NoDenotation
+            if (isDoubleDef(sym1, sym2)) doubleDefError(denot1, denot2, pre)
 
             val sym2Accessible = sym2.isAccessibleFrom(pre)
             /** Does `sym1` come before `sym2` in the linearization of `pre`? */
@@ -425,8 +420,12 @@ 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): SingleDenotation = {
+      val atSig1 = denot1.atSignature(sig, site, relaxed)
+      val atSig2 = denot2.atSignature(sig, site, relaxed)
+      if (isDoubleDef(atSig1.symbol, atSig2.symbol)) doubleDefError(atSig1, atSig2, site)
+      atSig1.orElse(atSig2)
+    }
     def currentIfExists(implicit ctx: Context): Denotation =
       derivedMultiDenotation(denot1.currentIfExists, denot2.currentIfExists)
     def current(implicit ctx: Context): Denotation =
@@ -907,6 +906,28 @@ object Denotations {
    */
   case class NoQualifyingRef(alts: List[SingleDenotation])(implicit ctx: Context) extends ErrorDenotation
 
+  /** A double defifinition
+   */
+  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: SingleDenotation, denot2: SingleDenotation, pre: Type)(implicit ctx: Context): Unit = {
+    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
diff --git a/src/dotty/tools/dotc/transform/Erasure.scala b/src/dotty/tools/dotc/transform/Erasure.scala
index ee8040168..24dea5118 100644
--- a/src/dotty/tools/dotc/transform/Erasure.scala
+++ b/src/dotty/tools/dotc/transform/Erasure.scala
@@ -559,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)
diff --git a/tests/neg/customArgs/i1240.scala b/tests/neg/customArgs/i1240.scala
index 438199b4a..3f4d1e210 100644
--- a/tests/neg/customArgs/i1240.scala
+++ b/tests/neg/customArgs/i1240.scala
@@ -20,21 +20,7 @@ class C2[T] {
 
 class D {}
 
-// 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 X {
+  def foo(x: D): D
+  def foo(x: D): D // error: already defined
 }
-
-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}
-                                           // which method is overriden?
-                                           // should any bridges be generated?
-  val a: A = this
-  a.foo(a.give[Int]) // what method should be called here in runtime?
-}
-
diff --git a/tests/neg/i1240a.scala b/tests/neg/i1240a.scala
new file mode 100644
index 000000000..dc711454e
--- /dev/null
+++ b/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?
+}
+