Skip to content

Commit 590d587

Browse files
committed
Fix overriding checks involving Scala and Java methods
To check which methods are in an overriding relationship, we rely on denotation matching which checks that the methods have matching signatures, but because Scala and Java have different erasure rules, seemingly valid overrides might not be seen as such because they end up with different signatures. To compensate for this we change SingleDenotation#matchesLoosely to use the Scala signatures of both methods when comparing a Scala and a Java method. This works even though the methods end up with different erased types because Erasure will insert bridges. Note that even after this change, we cannot simply override a Java generic `Array[T]` with a Scala `Array[T]`, because we intentionally parse these Java types as `Array[T & Object]`, see `TypeApplications.translateJavaArrayElementType` for details and tests/{neg,pos}/i1747.scala for examples.
1 parent 3fcc12f commit 590d587

File tree

5 files changed

+79
-28
lines changed

5 files changed

+79
-28
lines changed

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

Lines changed: 46 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -595,10 +595,17 @@ object Denotations {
595595
* SingleDenotations will have distinct signatures (cf #9050).
596596
*/
597597
final def signature(using Context): Signature =
598+
signature(isJava = !isType && symbol.is(JavaDefined))
599+
600+
/** Overload of `signature` which lets the caller pick between the Java and
601+
* Scala signature of the info. Useful to match denotations defined in
602+
* different classes (see `matchesLoosely`).
603+
*/
604+
def signature(isJava: Boolean)(using Context): Signature =
598605
if (isType) Signature.NotAMethod // don't force info if this is a type denotation
599606
else info match {
600607
case info: MethodOrPoly =>
601-
try info.signature(isJava = symbol.is(JavaDefined))
608+
try info.signature(isJava)
602609
catch { // !!! DEBUG
603610
case scala.util.control.NonFatal(ex) =>
604611
report.echo(s"cannot take signature of $info")
@@ -1004,34 +1011,45 @@ object Denotations {
10041011
symbol.hasTargetName(other.symbol.targetName)
10051012
&& matchesLoosely(other)
10061013

1007-
/** matches without a target name check */
1014+
/** `matches` without a target name check.
1015+
*
1016+
* We consider a Scala method and a Java method to match if they have
1017+
* matching Scala signatures. This allows us to override some Java
1018+
* definitions even if they have a different erasure (see i8615b,
1019+
* i9109b), Erasure takes care of adding any necessary bridge to make
1020+
* this work at runtime.
1021+
*/
10081022
def matchesLoosely(other: SingleDenotation)(using Context): Boolean =
1009-
val d = signature.matchDegree(other.signature)
1010-
d match
1011-
case FullMatch =>
1012-
true
1013-
case MethodNotAMethodMatch =>
1014-
!ctx.erasedTypes && {
1015-
val isJava = symbol.is(JavaDefined)
1016-
val otherIsJava = other.symbol.is(JavaDefined)
1017-
// A Scala zero-parameter method and a Scala non-method always match.
1018-
if !isJava && !otherIsJava then
1019-
true
1020-
// Java allows defining both a field and a zero-parameter method with the same name,
1021-
// so they must not match.
1022-
else if isJava && otherIsJava then
1023-
false
1024-
// A Java field never matches a Scala method.
1025-
else if isJava then
1026-
symbol.is(Method)
1027-
else // otherIsJava
1028-
other.symbol.is(Method)
1029-
}
1030-
case ParamMatch =>
1031-
// The signatures do not tell us enough to be sure about matching
1032-
!ctx.erasedTypes && info.matches(other.info)
1033-
case noMatch =>
1034-
false
1023+
if isType then true
1024+
else
1025+
val isJava = symbol.is(JavaDefined)
1026+
val otherIsJava = other.symbol.is(JavaDefined)
1027+
val useJavaSig = isJava && otherIsJava
1028+
val sig = signature(isJava = useJavaSig)
1029+
val otherSig = other.signature(isJava = useJavaSig)
1030+
sig.matchDegree(otherSig) match
1031+
case FullMatch =>
1032+
true
1033+
case MethodNotAMethodMatch =>
1034+
!ctx.erasedTypes && {
1035+
// A Scala zero-parameter method and a Scala non-method always match.
1036+
if !isJava && !otherIsJava then
1037+
true
1038+
// Java allows defining both a field and a zero-parameter method with the same name,
1039+
// so they must not match.
1040+
else if isJava && otherIsJava then
1041+
false
1042+
// A Java field never matches a Scala method.
1043+
else if isJava then
1044+
symbol.is(Method)
1045+
else // otherIsJava
1046+
other.symbol.is(Method)
1047+
}
1048+
case ParamMatch =>
1049+
// The signatures do not tell us enough to be sure about matching
1050+
!ctx.erasedTypes && info.matches(other.info)
1051+
case noMatch =>
1052+
false
10351053

10361054
def mapInherited(ownDenots: PreDenotation, prevDenots: PreDenotation, pre: Type)(using Context): SingleDenotation =
10371055
if hasUniqueSym && prevDenots.containsSym(symbol) then NoDenotation

tests/pos-java-interop/i9109b/A.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
public class A {
2+
public <T extends Object & Intf> void foo(T x) {}
3+
}

tests/pos-java-interop/i9109b/B.scala

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
class B extends A {
2+
override def foo[T <: Object with Intf](x: T): Unit = {}
3+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
public interface Intf {}

tests/pos/i8615b.scala

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
class ArrayOrdering[N] extends Comparable[Array[N]] {
2+
override def compareTo(x: Array[N]): Int = 0
3+
4+
compareTo(???)
5+
}
6+
7+
class ArrayIntOrdering extends Comparable[Array[Int]] {
8+
override def compareTo(x: Array[Int]): Int = 0
9+
10+
compareTo(???)
11+
}
12+
13+
class ArrayOrdering2[N] extends Comparable[Array[N]] {
14+
self: Serializable =>
15+
override def compareTo(x: Array[N]): Int = 0
16+
17+
compareTo(???)
18+
}
19+
20+
class ArrayIntOrdering2 extends Comparable[Array[Int]] {
21+
self: Serializable =>
22+
23+
override def compareTo(x: Array[Int]): Int = 0
24+
25+
compareTo(???)
26+
}

0 commit comments

Comments
 (0)