-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Refine hasCommonParent
criterion in OverridingPairs
#11741
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While this fixes the immediate problem, it's worth noting that Scala 2 completely removed the subparent logic in scala/scala@cc55bd9 because:
The testcase from that commit is: trait IO {
def c(x: Int): Int = ???
}
trait SO extends IO {
override final def c(x: Int): Int = ???
}
trait SOIO extends IO {
override def c(x: Int): Int = ???
}
trait SOSO extends SOIO with SO
abstract class AS extends SO
class L extends AS with SOSO // error expected: c definined in SOIO overrides final method c in SO Which currently compiles in Dotty, even though we end up overriding a final def with a non-final def. We already have an issue open to keep track of this (#7551) so we don't necessarily need to handle this now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK. We should evaluate the performance implications of subParents before removing them. Also bridge generation currently relies on the check being present, otherwise you would get duplicate bridges. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Indeed, but Scala 2 had the same issue so scala/scala@cc55bd9 also takes care of that: // Skip nextEntry if the (non-trait) class in `parents` is a subclass of the owners of both low and high.
// this means we'll come back to this entry when we consider `nonTraitParent`, and the linearisation order will be the same
// (this only works for non-trait classes, since subclassing on traits does not imply one's linearisation is contained in the other's)
// This is not just an optimization -- bridge generation relies on visiting each such class only once.
if (!isMatch || (nonTraitParent.isNonBottomSubClass(low.owner) && nonTraitParent.isNonBottomSubClass(high.owner))) |
||
|
||
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 = | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.