Fix check for unimplemented members in a concrete class

Before this commit, this check was done using Type#matchesLoosely, which
behaves differently from Denotation#matchesLoosely, in particular the
latter requires matching signatures and the former does not, and because
bridge generation at Erasure relies on denotation matching, we could end
up with unimplemented members at runtime. Fixed by switching to
denotation matching which should now be used uniformly everywhere we
check for overrides.

This commit also improves the error message of this check to list where
the abstract member and non-matching concrete members come from, since
this can be difficult to figure out in a complex hierarchy.

Fixes #10666.

Author: smarter

community-build/community-projects/scala-stm

@@ -541,11 +541,11 @@ object RefChecks {
         || mbr.is(JavaDefined) && hasJavaErasedOverriding(mbr)
 
       def isImplemented(mbr: Symbol) =
-        val mbrType = clazz.thisType.memberInfo(mbr)
+        val mbrDenot = mbr.asSeenFrom(clazz.thisType)
         def isConcrete(sym: Symbol) = sym.exists && !sym.isOneOf(NotConcrete)
         clazz.nonPrivateMembersNamed(mbr.name)
           .filterWithPredicate(
-            impl => isConcrete(impl.symbol) && mbrType.matchesLoosely(impl.info))
+            impl => isConcrete(impl.symbol) && mbrDenot.matchesLoosely(impl))
           .exists
 
       /** The term symbols in this class and its baseclasses that are
@@ -602,8 +602,10 @@ object RefChecks {
         }
 
         for (member <- missing) {
+          def showDclAndLocation(sym: Symbol) =
+            s"${sym.showDcl} in ${sym.owner.showLocated}"
           def undefined(msg: String) =
-            abstractClassError(false, s"${member.showDcl} is not defined $msg")
+            abstractClassError(false, s"${showDclAndLocation(member)} is not defined $msg")
           val underlying = member.underlyingSymbol
 
           // Give a specific error message for abstract vars based on why it fails:
@@ -641,13 +643,13 @@ object RefChecks {
                     val abstractSym = pa.typeSymbol
                     val concreteSym = pc.typeSymbol
                     def subclassMsg(c1: Symbol, c2: Symbol) =
-                      s": ${c1.showLocated} is a subclass of ${c2.showLocated}, but method parameter types must match exactly."
+                      s"${c1.showLocated} is a subclass of ${c2.showLocated}, but method parameter types must match exactly."
                     val addendum =
                       if (abstractSym == concreteSym)
                         (pa.typeConstructor, pc.typeConstructor) match {
                           case (TypeRef(pre1, _), TypeRef(pre2, _)) =>
-                            if (pre1 =:= pre2) ": their type parameters differ"
-                            else ": their prefixes (i.e. enclosing instances) differ"
+                            if (pre1 =:= pre2) "their type parameters differ"
+                            else "their prefixes (i.e. enclosing instances) differ"
                           case _ =>
                             ""
                         }
@@ -657,18 +659,22 @@ object RefChecks {
                         subclassMsg(concreteSym, abstractSym)
                       else ""
 
-                    undefined(s"\n(Note that ${pa.show} does not match ${pc.show}$addendum)")
+                    undefined(s"""
+                                 |(Note that
+                                 | parameter ${pa.show} in ${showDclAndLocation(underlying)} does not match
+                                 | parameter ${pc.show} in ${showDclAndLocation(concrete.symbol)}
+                                 | $addendum)""".stripMargin)
                   case xs =>
                     undefined(
                       if concrete.symbol.is(AbsOverride) then
-                        s"\n(The class implements ${concrete.showDcl} but that definition still needs an implementation)"
+                        s"\n(The class implements ${showDclAndLocation(concrete.symbol)} but that definition still needs an implementation)"
                       else
-                        s"\n(The class implements a member with a different type: ${concrete.showDcl})")
+                        s"\n(The class implements a member with a different type: ${showDclAndLocation(concrete.symbol)})")
                 }
               case Nil =>
                 undefined("")
               case concretes =>
-                undefined(s"\n(The class implements members with different types: ${concretes.map(_.showDcl)}%\n  %)")
+                undefined(s"\n(The class implements members with different types: ${concretes.map(c => showDclAndLocation(c.symbol))}%\n  %)")
             }
           }
           else undefined("")

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

@@ -7,7 +7,7 @@ import javax.management.{Notification, NotificationEmitter, NotificationListener
 class Foo {
 
   def bar(): Unit = {
-    val listener = new NotificationListener() { // error: object creation impossible
+    val listener = new NotificationListener() {
       override def handleNotification(n: Notification|Null, emitter: Object): Unit = { // error: method handleNotification overrides nothing
       }
     }
@@ -17,7 +17,7 @@ class Foo {
       }
     }
 
-    val listener3 = new NotificationListener() { // error: object creation impossible
+    val listener3 = new NotificationListener() {
       override def handleNotification(n: Notification, emitter: Object|Null): Unit = { // error: method handleNotification overrides nothing
       }
     }

tests/explicit-nulls/neg/override-java-object-arg.scala

@@ -4,7 +4,7 @@ import javax.management.{Notification, NotificationEmitter, NotificationListener
 class Foo {
 
   def bar(): Unit = {
-    val listener4 = new NotificationListener() { // error: duplicate symbol error
+    val listener4 = new NotificationListener() {
       def handleNotification(n: Notification|Null, emitter: Object): Unit = { // error
       }
     }

tests/explicit-nulls/neg/override-java-object-arg2.scala

@@ -1,7 +1,7 @@
 -- Error: tests/neg/abstract-givens.scala:11:8 -------------------------------------------------------------------------
 11 |  given s[T](using T): Seq[T] with // error
    |        ^
-   |        instance cannot be created, since def iterator: => Iterator[A] is not defined 
+   |instance cannot be created, since def iterator: => Iterator[A] in trait IterableOnce in package scala.collection is not defined 
 -- Error: tests/neg/abstract-givens.scala:8:8 --------------------------------------------------------------------------
 8 |  given y(using Int): String = summon[Int].toString * 22 // error
   |        ^

tests/neg/abstract-givens.check

@@ -0,0 +1,8 @@
+-- Error: tests/neg/i10666.scala:8:6 -----------------------------------------------------------------------------------
+8 |class Bar extends Foo { // error
+  |      ^
+  |      class Bar needs to be abstract, since def foo: [T <: B](tx: T): Unit in trait Foo is not defined 
+  |      (Note that
+  |       parameter T in def foo: [T <: B](tx: T): Unit in trait Foo does not match
+  |       parameter T in def foo: [T <: A](tx: T): Unit in class Bar
+  |       class B is a subclass of class A, but method parameter types must match exactly.)

tests/neg/i10666.check

@@ -0,0 +1,17 @@
+class A
+class B extends A
+
+trait Foo {
+  def foo[T <: B](tx: T): Unit
+}
+
+class Bar extends Foo { // error
+  def foo[T <: A](tx: T): Unit = {}
+}
+
+object Test {
+  def main(args: Array[String]): Unit = {
+    val f: Foo = new Bar
+    f.foo(new B)
+  }
+}

tests/neg/i10666.scala

@@ -1,12 +1,12 @@
 object Test extends App {
   def foo[S <: String]: String => Int =
-    new (String => Int) { def apply(s: S): Int = 0 } // error // error
+    new (String => Int) { def apply(s: S): Int = 0 } // error
 
   trait Fn[A, B] {
     def apply(x: A): B
   }
 
-  class C[S <: String] extends Fn[String, Int] {  // error
+  class C[S <: String] extends Fn[String, Int] {
     def apply(s: S): Int = 0 // error
   }
 

tests/neg/i7597.scala

@@ -1,5 +1,5 @@
 -- Error: tests/neg/i9329.scala:8:6 ------------------------------------------------------------------------------------
 8 |class GrandSon extends Son // error
   |      ^
-  |    class GrandSon needs to be abstract, since def name: => String is not defined 
-  |    (The class implements abstract override def name: => String but that definition still needs an implementation)
+  |class GrandSon needs to be abstract, since def name: => String in trait Parent is not defined 
+  |(The class implements abstract override def name: => String in trait Son but that definition still needs an implementation)