RefChecks cleanuo

Remove illogical conditions that suppressed certain checks. Replace them
by a separate condition that certain override checks are suppressed if the
types of the members do not match (i.e. it's just the signatures that match
but not the types). In that case, we have arguably overshot with declaring
an override. Bridge generation is also suppressed in this case. Scala 2 behaves
effectively the same way, by never taking signatures into account.

Author: odersky

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

@@ -36,7 +36,7 @@ class Bridges(root: ClassSymbol, thisPhase: DenotTransformer)(using Context) {
       !sym.isOneOf(MethodOrModule) || super.exclude(sym)
   }
 
-  //val site = root.thisType
+  val site = root.thisType
 
   private var toBeRemoved = immutable.Set[Symbol]()
   private val bridges = mutable.ListBuffer[Tree]()
@@ -77,7 +77,13 @@ class Bridges(root: ClassSymbol, thisPhase: DenotTransformer)(using Context) {
                       |clashes with definition of the member itself; both have erased type ${info(member)(using elimErasedCtx)}."""",
                   bridgePosFor(member))
     }
-    else if (!bridgeExists)
+    else if !inContext(preErasureCtx)(site.memberInfo(member).matches(site.memberInfo(other))) then
+      // Neither symbol signatures nor pre-erasure types seen from root match; this means
+      // according to Scala 2 semantics there is no override.
+      // A bridge might introduce a classcast exception.
+      // Example where this was observed: run/i12828a.scala and MapView in stdlib213
+      report.log(i"suppress bridge in $root for ${member} in ${member.owner} and ${other.showLocated} since member infos ${site.memberInfo(member)} and ${site.memberInfo(other)} do not match")
+    else if !bridgeExists then
       addBridge(member, other)
   }
 

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

@@ -323,6 +323,13 @@ object RefChecks {
             overrideErrorMsg("no longer has compatible type"),
             (if (member.owner == clazz) member else clazz).srcPos))
 
+      /** Do types of `member` and `other` as seen from `self` match?
+       *  If not we treat them as not a real override and don't issue certain
+       *  error messages. Also, bridges are not generated in this case.
+       */
+      def trueMatch: Boolean =
+        memberTp(self).matches(otherTp(self))
+
       def emitOverrideError(fullmsg: Message) =
         if (!(hasErrors && member.is(Synthetic) && member.is(Module))) {
           // suppress errors relating toi synthetic companion objects if other override
@@ -360,31 +367,6 @@ object RefChecks {
 
       //Console.println(infoString(member) + " overrides " + infoString(other) + " in " + clazz);//DEBUG
 
-      // return if we already checked this combination elsewhere
-      if (member.owner != clazz) {
-        def deferredCheck = member.is(Deferred) || !other.is(Deferred)
-        def subOther(s: Symbol) = s derivesFrom other.owner
-        def subMember(s: Symbol) = s derivesFrom member.owner
-
-        if (subOther(member.owner) && deferredCheck)
-          //println(i"skip 1 ${member.showLocated}, ${other.showLocated}")
-          //Console.println(infoString(member) + " shadows1 " + infoString(other) " in " + clazz);//DEBUG
-          return
-        /*
-        val parentSymbols = clazz.info.parents.map(_.typeSymbol)
-        def matchIn(parent: Symbol): Boolean = considerMatching(member, other, parent.thisType)
-        if parentSymbols.exists(p =>
-          subOther(p) && subMember(p) && deferredCheck && matchIn(p))
-        then
-          println(i"skip 2 ${member.showLocated}, ${other.showLocated}")
-          //Console.println(infoString(member) + " shadows2 " + infoString(other) + " in " + clazz);//DEBUG
-          return
-        if parentSymbols.forall(p => subOther(p) == subMember(p) && matchIn(p)) then
-          println(i"skip 3 ${member.showLocated}, ${other.showLocated}")
-          //Console.println(infoString(member) + " shadows " + infoString(other) + " in " + clazz);//DEBUG
-          return*/
-      }
-
       /* Is the intersection between given two lists of overridden symbols empty? */
       def intersectionIsEmpty(syms1: Iterator[Symbol], syms2: Iterator[Symbol]) = {
         val set2 = syms2.toSet
@@ -419,7 +401,7 @@ object RefChecks {
         overrideError("cannot be used here - class definitions cannot be overridden")
       else if (!other.is(Deferred) && member.isClass)
         overrideError("cannot be used here - classes can only override abstract types")
-      else if (other.isEffectivelyFinal) // (1.2)
+      else if other.isEffectivelyFinal && trueMatch then // (1.2)
         overrideError(i"cannot override final member ${other.showLocated}")
       else if (member.is(ExtensionMethod) && !other.is(ExtensionMethod)) // (1.3)
         overrideError("is an extension method, cannot override a normal method")
@@ -440,15 +422,18 @@ object RefChecks {
           member.setFlag(Override)
         else if (member.isType && self.memberInfo(member) =:= self.memberInfo(other))
           () // OK, don't complain about type aliases which are equal
-        else if (member.owner != clazz && other.owner != clazz &&
-                 !(other.owner derivesFrom member.owner))
+        else if member.owner != clazz
+             && other.owner != clazz
+             && !other.owner.derivesFrom(member.owner)
+             && trueMatch
+        then
           emitOverrideError(
             s"$clazz inherits conflicting members:\n  "
               + infoStringWithLocation(other) + "  and\n  " + infoStringWithLocation(member)
               + "\n(Note: this can be resolved by declaring an override in " + clazz + ".)")
         else if member.is(Exported) then
           overrideError("cannot override since it comes from an export")
-        else
+        else if trueMatch then
           overrideError("needs `override` modifier")
       else if (other.is(AbsOverride) && other.isIncompleteIn(clazz) && !member.is(AbsOverride))
         overrideError("needs `abstract override` modifiers")

tests/neg/i12828c.scala

@@ -0,0 +1,12 @@
+abstract class Foo[A] {
+  def foo(x: A): Unit
+}
+abstract class Bar[A] extends Foo[A] {
+  def foo(x: A with String): Unit = println(x.toUpperCase)
+}
+object Baz extends Bar[Int] // error overriding foo: incompatible type
+                            // Scala 2 gives: object creation impossible. Missing implementation for `foo`
+
+object Test {
+  def main(args: Array[String]) = Baz.foo(42)
+}

tests/neg/i12828d.scala

@@ -0,0 +1,18 @@
+trait A[X] {
+  def foo(x: X): Unit =
+    println("A.foo")
+}
+trait B[X] extends A[X] {
+  def foo(x: Int): Unit =
+    println("B.foo")
+}
+object C extends B[Int] // error: conflicting members
+                        // Scala 2: same
+
+object Test {
+  def main(args: Array[String]) = {
+    C.foo(1)
+    val a: A[Int] = C
+    a.foo(1)
+  }
+}

tests/neg/varargs-annot-2.scala

@@ -0,0 +1,9 @@
+import annotation.varargs
+
+trait C {
+  @varargs def v(i: Int*) = ()
+}
+
+class D extends C { // error: name clash between defined and inherited member
+  def v(i: Array[Int]) = ()
+}

tests/neg/varargs-annot.scala

@@ -17,7 +17,7 @@ object Test {
 
   class D extends C {
     override def v(i: Int*) = () // error
-    def v(i: Array[Int]) = () // error
+    def v(i: Array[Int]) = () // ok, reported when used alone (see varargs-annot-2.scala)
   }
 
   @varargs def nov(a: Int) = 0 // error: A method without repeated parameters cannot be annotated with @varargs

tests/run/i12828a.scala

@@ -0,0 +1,12 @@
+trait Foo[A] {
+  def foo(x: A): Unit = ()
+}
+trait Bar[A] extends Foo[A] {
+  def foo(x: A with String): Unit = println(x.toUpperCase)
+}
+object Baz extends Bar[Int] // was: error: Baz inherits conflicting members, now like Scala 2
+                            // Scala 2 compiles and runs
+
+object Test {
+  def main(args: Array[String]) = Baz.foo(42)
+}

tests/run/i12828b.scala

@@ -0,0 +1,12 @@
+class Foo[A] {
+  def foo(x: A): Unit = ()
+}
+class Bar[A] extends Foo[A] {
+  def foo(x: A with String): Unit = println(x.toUpperCase)
+}
+object Baz extends Bar[Int] // was error: Baz inherits conflicting members, now like Scalac
+                            // Scala 2 compiles and runs
+
+object Test {
+  def main(args: Array[String]) = Baz.foo(42)
+}