From 109f3f0068c5c3ca19fa29e55211e501f8bb1620 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 16 Feb 2021 15:25:14 +0100 Subject: [PATCH 1/2] 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. --- community-build/community-projects/scala-stm | 2 +- .../dotty/tools/dotc/typer/RefChecks.scala | 26 ++++++++++++------- .../neg/override-java-object-arg.scala | 4 +-- .../neg/override-java-object-arg2.scala | 2 +- tests/neg/abstract-givens.check | 2 +- tests/neg/i10666.check | 8 ++++++ tests/neg/i10666.scala | 17 ++++++++++++ tests/neg/i7597.scala | 4 +-- tests/neg/i9329.check | 4 +-- 9 files changed, 50 insertions(+), 19 deletions(-) create mode 100644 tests/neg/i10666.check create mode 100644 tests/neg/i10666.scala diff --git a/community-build/community-projects/scala-stm b/community-build/community-projects/scala-stm index 96ef6e56af08..7011331d3684 160000 --- a/community-build/community-projects/scala-stm +++ b/community-build/community-projects/scala-stm @@ -1 +1 @@ -Subproject commit 96ef6e56af08d0ec600ec80ebec68d227a8c048c +Subproject commit 7011331d36848faf9af407e3e85ab75076dfdefa diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index a7e010124f89..566891903344 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -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("") diff --git a/tests/explicit-nulls/neg/override-java-object-arg.scala b/tests/explicit-nulls/neg/override-java-object-arg.scala index ccce4af660c7..7a45cb1d6199 100644 --- a/tests/explicit-nulls/neg/override-java-object-arg.scala +++ b/tests/explicit-nulls/neg/override-java-object-arg.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 } } diff --git a/tests/explicit-nulls/neg/override-java-object-arg2.scala b/tests/explicit-nulls/neg/override-java-object-arg2.scala index 9dae93be404f..1646ca37fc7c 100644 --- a/tests/explicit-nulls/neg/override-java-object-arg2.scala +++ b/tests/explicit-nulls/neg/override-java-object-arg2.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 } } diff --git a/tests/neg/abstract-givens.check b/tests/neg/abstract-givens.check index cd504320383c..03a5f718f86b 100644 --- a/tests/neg/abstract-givens.check +++ b/tests/neg/abstract-givens.check @@ -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 | ^ diff --git a/tests/neg/i10666.check b/tests/neg/i10666.check new file mode 100644 index 000000000000..65ac39e451af --- /dev/null +++ b/tests/neg/i10666.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.) diff --git a/tests/neg/i10666.scala b/tests/neg/i10666.scala new file mode 100644 index 000000000000..06ace72f40a3 --- /dev/null +++ b/tests/neg/i10666.scala @@ -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) + } +} diff --git a/tests/neg/i7597.scala b/tests/neg/i7597.scala index 9f98f8c54951..8b18b82b1db4 100644 --- a/tests/neg/i7597.scala +++ b/tests/neg/i7597.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 } diff --git a/tests/neg/i9329.check b/tests/neg/i9329.check index 72e5fa1a0ff9..8ddb0768a119 100644 --- a/tests/neg/i9329.check +++ b/tests/neg/i9329.check @@ -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) From f796c954ca1da5e6e3fb21c642a676dcc48057b5 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Wed, 17 Feb 2021 13:44:14 +0100 Subject: [PATCH 2/2] Fix #10599: Add testcase This was already fixed by #11361. --- tests/pos-java-interop/i10599/A.java | 4 ++++ tests/pos-java-interop/i10599/B.scala | 7 +++++++ 2 files changed, 11 insertions(+) create mode 100644 tests/pos-java-interop/i10599/A.java create mode 100644 tests/pos-java-interop/i10599/B.scala diff --git a/tests/pos-java-interop/i10599/A.java b/tests/pos-java-interop/i10599/A.java new file mode 100644 index 000000000000..f9d52c7a13e0 --- /dev/null +++ b/tests/pos-java-interop/i10599/A.java @@ -0,0 +1,4 @@ +public abstract class A { + public abstract void foo(T value); + public void foo(T value, Object obj) { return; } +} diff --git a/tests/pos-java-interop/i10599/B.scala b/tests/pos-java-interop/i10599/B.scala new file mode 100644 index 000000000000..6d39cbbc79f3 --- /dev/null +++ b/tests/pos-java-interop/i10599/B.scala @@ -0,0 +1,7 @@ +abstract class Base[M[_], T] extends A[M[T]] { + override def foo(value: M[T]): Unit = ??? +} + +class ArrayTest[T] extends Base[Array, T] + +class ListTest[T] extends Base[List, T]