Skip to content

Commit 6a90e5e

Browse files
committed
Systematize treatement of trueMatch
Refchecks now decouples overriding from signature matching. A signature match is in essence a pre-check that two members might override, but if the two members are methods we also need to compare their parameter types. This has three consequences: 1. Before emitting an override error, test that parameter types match. If not, we have a false override and no error should be reported. 2. Before emitting a bridge, do the same test and omit the generation if the test fails 3. When checking whether a class is fully implemented, check that any implementation also agrees completely in its parameter types.
1 parent 8138eb4 commit 6a90e5e

File tree

7 files changed

+30
-20
lines changed

7 files changed

+30
-20
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1021,7 +1021,7 @@ object Denotations {
10211021
* erasure (see i8615b, i9109b), Erasure takes care of adding any necessary
10221022
* bridge to make this work at runtime.
10231023
*/
1024-
def matchesLoosely(other: SingleDenotation)(using Context): Boolean =
1024+
def matchesLoosely(other: SingleDenotation, alwaysCompareParams: Boolean = false)(using Context): Boolean =
10251025
if isType then true
10261026
else
10271027
val thisLanguage = SourceLanguage(symbol)
@@ -1031,7 +1031,7 @@ object Denotations {
10311031
val otherSig = other.signature(commonLanguage)
10321032
sig.matchDegree(otherSig) match
10331033
case FullMatch =>
1034-
true
1034+
!alwaysCompareParams || info.matches(other.info)
10351035
case MethodNotAMethodMatch =>
10361036
!ctx.erasedTypes && {
10371037
// A Scala zero-parameter method and a Scala non-method always match.

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

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -323,12 +323,13 @@ object RefChecks {
323323
overrideErrorMsg("no longer has compatible type"),
324324
(if (member.owner == clazz) member else clazz).srcPos))
325325

326-
/** Do types of `member` and `other` as seen from `self` match?
326+
/** Do types of term members `member` and `other` as seen from `self` match?
327327
* If not we treat them as not a real override and don't issue certain
328328
* error messages. Also, bridges are not generated in this case.
329+
* Type members are always assumed to match.
329330
*/
330331
def trueMatch: Boolean =
331-
memberTp(self).matches(otherTp(self))
332+
member.isType || memberTp(self).matches(otherTp(self))
332333

333334
def emitOverrideError(fullmsg: Message) =
334335
if (!(hasErrors && member.is(Synthetic) && member.is(Module))) {
@@ -340,7 +341,7 @@ object RefChecks {
340341
}
341342

342343
def overrideError(msg: String, compareTypes: Boolean = false) =
343-
if (noErrorType)
344+
if trueMatch && noErrorType then
344345
emitOverrideError(overrideErrorMsg(msg, compareTypes))
345346

346347
def autoOverride(sym: Symbol) =
@@ -401,7 +402,7 @@ object RefChecks {
401402
overrideError("cannot be used here - class definitions cannot be overridden")
402403
else if (!other.is(Deferred) && member.isClass)
403404
overrideError("cannot be used here - classes can only override abstract types")
404-
else if other.isEffectivelyFinal && trueMatch then // (1.2)
405+
else if other.isEffectivelyFinal then // (1.2)
405406
overrideError(i"cannot override final member ${other.showLocated}")
406407
else if (member.is(ExtensionMethod) && !other.is(ExtensionMethod)) // (1.3)
407408
overrideError("is an extension method, cannot override a normal method")
@@ -425,15 +426,14 @@ object RefChecks {
425426
else if member.owner != clazz
426427
&& other.owner != clazz
427428
&& !other.owner.derivesFrom(member.owner)
428-
&& trueMatch
429429
then
430-
emitOverrideError(
430+
overrideError(
431431
s"$clazz inherits conflicting members:\n "
432432
+ infoStringWithLocation(other) + " and\n " + infoStringWithLocation(member)
433433
+ "\n(Note: this can be resolved by declaring an override in " + clazz + ".)")
434434
else if member.is(Exported) then
435435
overrideError("cannot override since it comes from an export")
436-
else if trueMatch then
436+
else
437437
overrideError("needs `override` modifier")
438438
else if (other.is(AbsOverride) && other.isIncompleteIn(clazz) && !member.is(AbsOverride))
439439
overrideError("needs `abstract override` modifiers")
@@ -578,7 +578,8 @@ object RefChecks {
578578
def isConcrete(sym: Symbol) = sym.exists && !sym.isOneOf(NotConcrete)
579579
clazz.nonPrivateMembersNamed(mbr.name)
580580
.filterWithPredicate(
581-
impl => isConcrete(impl.symbol) && mbrDenot.matchesLoosely(impl))
581+
impl => isConcrete(impl.symbol)
582+
&& mbrDenot.matchesLoosely(impl, alwaysCompareParams = true))
582583
.exists
583584

584585
/** The term symbols in this class and its baseclasses that are

tests/neg/OpaqueEscape.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ def unwrap(i:Wrapped):Int
77
def wrap(i:Int):Wrapped
88
}
99
class Escaper extends EscaperBase{ // error: needs to be abstract
10-
override def unwrap(i:Int):Int = i // error overriding method unwrap
10+
override def unwrap(i:Int):Int = i // was error overriding method unwrap, now OK
1111
override def wrap(i:Int):Int = i // error overriding method wrap
1212
}
1313
val e = new Escaper:EscaperBase

tests/neg/i11828.check

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
-- Error: tests/neg/i12828.scala:7:7 -----------------------------------------------------------------------------------
2+
7 |object Baz extends Bar[Int] // error overriding foo: incompatible type
3+
| ^
4+
| object creation impossible, since def foo(x: A): Unit in trait Foo is not defined
5+
| (Note that
6+
| parameter A in def foo(x: A): Unit in trait Foo does not match
7+
| parameter Int & String in def foo(x: A & String): Unit in trait Bar
8+
| )

tests/neg/i12828.check

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
-- [E163] Declaration Error: tests/neg/i12828.scala:7:7 ----------------------------------------------------------------
2-
7 |object Baz extends Bar[Int] // error overriding foo: incompatible type
1+
-- Error: tests/neg/i12828.scala:7:7 -----------------------------------------------------------------------------------
2+
7 |object Baz extends Bar[Int] // error: not implemented
33
| ^
4-
| error overriding method foo in trait Foo of type (x: Int): Unit;
5-
| method foo in trait Bar of type (x: Int & String): Unit has incompatible type
6-
7-
longer explanation available when compiling with `-explain`
4+
| object creation impossible, since def foo(x: A): Unit in trait Foo is not defined
5+
| (Note that
6+
| parameter A in def foo(x: A): Unit in trait Foo does not match
7+
| parameter Int & String in def foo(x: A & String): Unit in trait Bar
8+
| )

tests/neg/i12828.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@ trait Foo[A]:
44
trait Bar[A] extends Foo[A]:
55
def foo(x: A & String): Unit = println(x.toUpperCase)
66

7-
object Baz extends Bar[Int] // error overriding foo: incompatible type
7+
object Baz extends Bar[Int] // error: not implemented
88

99
@main def run() = Baz.foo(42)

tests/neg/i7597.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ object Test extends App {
66
def apply(x: A): B
77
}
88

9-
class C[S <: String] extends Fn[String, Int] {
10-
def apply(s: S): Int = 0 // error
9+
class C[S <: String] extends Fn[String, Int] { // error
10+
def apply(s: S): Int = 0
1111
}
1212

1313
foo("")

0 commit comments

Comments
 (0)