From aaab38918200e2f8d742b94e36353d8c04436c75 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 15 Mar 2021 10:52:20 +0100 Subject: [PATCH] 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. --- .../dotty/tools/dotc/transform/Bridges.scala | 5 +++++ .../dotc/transform/ElimErasedValueType.scala | 7 ++++++- .../dotc/transform/OverridingPairs.scala | 16 +++++++++++++++- tests/neg/i11130.scala | 2 +- tests/neg/i11719.scala | 12 ++++++++++++ tests/neg/i11719a.scala | 19 +++++++++++++++++++ 6 files changed, 58 insertions(+), 3 deletions(-) create mode 100644 tests/neg/i11719.scala create mode 100644 tests/neg/i11719a.scala diff --git a/compiler/src/dotty/tools/dotc/transform/Bridges.scala b/compiler/src/dotty/tools/dotc/transform/Bridges.scala index 038201d0afe6..8cb78c258704 100644 --- a/compiler/src/dotty/tools/dotc/transform/Bridges.scala +++ b/compiler/src/dotty/tools/dotc/transform/Bridges.scala @@ -20,6 +20,11 @@ class Bridges(root: ClassSymbol, thisPhase: DenotTransformer)(using Context) { private class BridgesCursor(using Context) extends OverridingPairs.Cursor(root) { + override def isSubParent(parent: Symbol, bc: Symbol)(using Context) = + true + // Never consider a bridge if there is a superclass that would contain it + // See run/t2857.scala for a test that would break with a VerifyError otherwise. + /** Only use the superclass of `root` as a parent class. This means * overriding pairs that have a common implementation in a trait parent * are also counted. This is necessary because we generate bridge methods diff --git a/compiler/src/dotty/tools/dotc/transform/ElimErasedValueType.scala b/compiler/src/dotty/tools/dotc/transform/ElimErasedValueType.scala index b49cacb19ecd..6545d7653c6b 100644 --- a/compiler/src/dotty/tools/dotc/transform/ElimErasedValueType.scala +++ b/compiler/src/dotty/tools/dotc/transform/ElimErasedValueType.scala @@ -1,4 +1,5 @@ -package dotty.tools.dotc +package dotty.tools +package dotc package transform import ast.{Trees, tpd} @@ -80,6 +81,10 @@ class ElimErasedValueType extends MiniPhase with InfoTransformer { thisPhase => private def checkNoClashes(root: Symbol)(using Context) = { val opc = atPhase(thisPhase) { new OverridingPairs.Cursor(root) { + override def isSubParent(parent: Symbol, bc: Symbol)(using Context) = + // Need to compute suparents before erasure to not filter out parents + // that are bypassed with different types. See neg/11719a.scala. + atPhase(elimRepeatedPhase.next) { super.isSubParent(parent, bc) } override def exclude(sym: Symbol) = !sym.is(Method) || sym.is(Bridge) || super.exclude(sym) override def matches(sym1: Symbol, sym2: Symbol) = diff --git a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala index 40be0694dc1e..b2851fd92619 100644 --- a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala +++ b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala @@ -64,11 +64,25 @@ object OverridingPairs { decls } + /** Is `parent` a qualified sub-parent of `bc`? + * @pre `parent` is a parent class of `base` and it derives from `bc`. + * @return true if the `bc`-basetype of the parent's type is the same as + * the `bc`-basetype of base. In that case, overriding checks + * relative to `parent` already subsume overriding checks + * relative to `base`. See neg/11719a.scala for where this makes + * a difference. + */ + protected def isSubParent(parent: Symbol, bc: Symbol)(using Context) = + bc.typeParams.isEmpty + || self.baseType(parent).baseType(bc) == self.baseType(bc) + private val subParents = MutableSymbolMap[BitSet]() + for bc <- base.info.baseClasses do var bits = BitSet.empty for i <- 0 until parents.length do - if parents(i).derivesFrom(bc) then bits += i + if parents(i).derivesFrom(bc) && isSubParent(parents(i), bc) + then bits += i subParents(bc) = bits private def hasCommonParentAsSubclass(cls1: Symbol, cls2: Symbol): Boolean = diff --git a/tests/neg/i11130.scala b/tests/neg/i11130.scala index df179cc60a41..fcf9c9e84ee7 100644 --- a/tests/neg/i11130.scala +++ b/tests/neg/i11130.scala @@ -9,7 +9,7 @@ trait PBar extends S[Bar] { override type T = Bar } - class PFooBar extends PBar with PFoo { + class PFooBar extends PBar with PFoo { // error override type T >: Bar // error } diff --git a/tests/neg/i11719.scala b/tests/neg/i11719.scala new file mode 100644 index 000000000000..b7251fc78594 --- /dev/null +++ b/tests/neg/i11719.scala @@ -0,0 +1,12 @@ +class Base +class Sub extends Base + +trait A[+T] { + def get: T = ??? +} + +trait B extends A[Base] { + override def get: Base = new Base +} + +class C extends B with A[Sub] // error: method get in trait B is not a legal implementation \ No newline at end of file diff --git a/tests/neg/i11719a.scala b/tests/neg/i11719a.scala new file mode 100644 index 000000000000..5f59f14d0a02 --- /dev/null +++ b/tests/neg/i11719a.scala @@ -0,0 +1,19 @@ +class Base +class Sub extends Base + +trait A[+T] { + def get(f: T => Boolean): Unit = {} +} + +trait B extends A[Base] { + override def get(f: Base => Boolean): Unit = f(new Base) +} + +class C extends B with A[Sub] // error: Name clash between inherited members: + +object Test { + def main(args: Array[String]): Unit = { + val c = new C + c.get((x: Sub) => true) // ClassCastException: Base cannot be cast to Sub + } +} \ No newline at end of file