Skip to content

Commit aaab389

Browse files
committed
Refine hasCommonParent criterion in OverridingPairs
Fixes #11719 `hasCommonParent` is used to avoid re-visiting overriding pairs that were already accounted for in a parent. But that only works if the base types of the classes forming an overriding pair are the same for the parent and the class itself. Otherwise we might miss an overriding relationship.
1 parent 66e180d commit aaab389

File tree

6 files changed

+58
-3
lines changed

6 files changed

+58
-3
lines changed

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ class Bridges(root: ClassSymbol, thisPhase: DenotTransformer)(using Context) {
2020

2121
private class BridgesCursor(using Context) extends OverridingPairs.Cursor(root) {
2222

23+
override def isSubParent(parent: Symbol, bc: Symbol)(using Context) =
24+
true
25+
// Never consider a bridge if there is a superclass that would contain it
26+
// See run/t2857.scala for a test that would break with a VerifyError otherwise.
27+
2328
/** Only use the superclass of `root` as a parent class. This means
2429
* overriding pairs that have a common implementation in a trait parent
2530
* are also counted. This is necessary because we generate bridge methods

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
package dotty.tools.dotc
1+
package dotty.tools
2+
package dotc
23
package transform
34

45
import ast.{Trees, tpd}
@@ -80,6 +81,10 @@ class ElimErasedValueType extends MiniPhase with InfoTransformer { thisPhase =>
8081
private def checkNoClashes(root: Symbol)(using Context) = {
8182
val opc = atPhase(thisPhase) {
8283
new OverridingPairs.Cursor(root) {
84+
override def isSubParent(parent: Symbol, bc: Symbol)(using Context) =
85+
// Need to compute suparents before erasure to not filter out parents
86+
// that are bypassed with different types. See neg/11719a.scala.
87+
atPhase(elimRepeatedPhase.next) { super.isSubParent(parent, bc) }
8388
override def exclude(sym: Symbol) =
8489
!sym.is(Method) || sym.is(Bridge) || super.exclude(sym)
8590
override def matches(sym1: Symbol, sym2: Symbol) =

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,25 @@ object OverridingPairs {
6464
decls
6565
}
6666

67+
/** Is `parent` a qualified sub-parent of `bc`?
68+
* @pre `parent` is a parent class of `base` and it derives from `bc`.
69+
* @return true if the `bc`-basetype of the parent's type is the same as
70+
* the `bc`-basetype of base. In that case, overriding checks
71+
* relative to `parent` already subsume overriding checks
72+
* relative to `base`. See neg/11719a.scala for where this makes
73+
* a difference.
74+
*/
75+
protected def isSubParent(parent: Symbol, bc: Symbol)(using Context) =
76+
bc.typeParams.isEmpty
77+
|| self.baseType(parent).baseType(bc) == self.baseType(bc)
78+
6779
private val subParents = MutableSymbolMap[BitSet]()
80+
6881
for bc <- base.info.baseClasses do
6982
var bits = BitSet.empty
7083
for i <- 0 until parents.length do
71-
if parents(i).derivesFrom(bc) then bits += i
84+
if parents(i).derivesFrom(bc) && isSubParent(parents(i), bc)
85+
then bits += i
7286
subParents(bc) = bits
7387

7488
private def hasCommonParentAsSubclass(cls1: Symbol, cls2: Symbol): Boolean =

tests/neg/i11130.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
trait PBar extends S[Bar] {
1010
override type T = Bar
1111
}
12-
class PFooBar extends PBar with PFoo {
12+
class PFooBar extends PBar with PFoo { // error
1313
override type T >: Bar // error
1414
}
1515

tests/neg/i11719.scala

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
class Base
2+
class Sub extends Base
3+
4+
trait A[+T] {
5+
def get: T = ???
6+
}
7+
8+
trait B extends A[Base] {
9+
override def get: Base = new Base
10+
}
11+
12+
class C extends B with A[Sub] // error: method get in trait B is not a legal implementation

tests/neg/i11719a.scala

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
class Base
2+
class Sub extends Base
3+
4+
trait A[+T] {
5+
def get(f: T => Boolean): Unit = {}
6+
}
7+
8+
trait B extends A[Base] {
9+
override def get(f: Base => Boolean): Unit = f(new Base)
10+
}
11+
12+
class C extends B with A[Sub] // error: Name clash between inherited members:
13+
14+
object Test {
15+
def main(args: Array[String]): Unit = {
16+
val c = new C
17+
c.get((x: Sub) => true) // ClassCastException: Base cannot be cast to Sub
18+
}
19+
}

0 commit comments

Comments
 (0)