Skip to content

Fix #5433: check that the implemented super-accessor is valid #5604

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

Merged
merged 4 commits into from
Mar 5, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 59 additions & 2 deletions compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,67 @@ object ResolveSuper {
val SuperAccessorName(memberName) = acc.name.unexpandedName
ctx.debuglog(i"starting rebindsuper from $base of ${acc.showLocated}: ${acc.info} in $bcs, name = $memberName")
while (bcs.nonEmpty && sym == NoSymbol) {
val other = bcs.head.info.nonPrivateDecl(memberName)
val cur = bcs.head
val other = cur.info.nonPrivateDecl(memberName)
if (ctx.settings.Ydebug.value)
ctx.log(i"rebindsuper ${bcs.head} $other deferred = ${other.symbol.is(Deferred)}")
sym = other.matchingDenotation(base.thisType, base.thisType.memberInfo(acc)).symbol
val otherMember = other.matchingDenotation(base.thisType, base.thisType.memberInfo(acc))
if (otherMember.exists) {
sym = otherMember.symbol
// Having a matching denotation is not enough: it should also be a subtype
// of the superaccessor's type, see i5433.scala for an example where this matters
val otherTp = otherMember.asSeenFrom(base.typeRef).info
val accTp = acc.asSeenFrom(base.typeRef).info
if (!(otherTp <:< accTp)) {
// The mixin containing a super-call that requires a super-accessor
val mixin = acc.owner
// The super-call in `mixin`
val superCall = i"super.$memberName"
// The super-call that we end up trying to call
val resolvedSuperCall = i"super[${cur.name}].$memberName"
// The super-call that we would have called if `super` in traits behaved like it
// does in classes, i.e. followed the linearization of the trait itself.
val staticSuperCall = {
val staticSuper = mixin.asClass.info.parents.reverse
.find(_.nonPrivateMember(memberName).matchingDenotation(mixin.thisType, acc.info).exists)
val staticSuperName = staticSuper match {
case Some(parent) =>
parent.classSymbol.name.show
case None => // Might be reachable under separate compilation
"SomeParent"
}
i"super[$staticSuperName].$memberName"
}
ctx.error(
hl"""$base cannot be defined due to a conflict between its parents when
|implementing a super-accessor for $memberName in $mixin:
|
|1. One of its parent ($mixin) contains a call $superCall in its body,
| and when a super-call in a trait is written without an explicit parent
| listed in brackets, it is implemented by a generated super-accessor in
| the class that extends this trait based on the linearization order of
| the class.
|2. Because ${cur.name} comes before ${mixin.name} in the linearization
| order of ${base.name}, and because ${cur.name} overrides $memberName,
| the super-accessor in ${base.name} is implemented as a call to
| $resolvedSuperCall.
|3. However,
| ${otherTp.widenExpr} (the type of $resolvedSuperCall in ${base.name})
| is not a subtype of
| ${accTp.widenExpr} (the type of $memberName in $mixin).
| Hence, the super-accessor that needs to be generated in ${base.name}
| is illegal.
|
|Here are two possible ways to resolve this:
|
|1. Change the linearization order of ${base.name} such that
| ${mixin.name} comes before ${cur.name}.
|2. Alternatively, replace $superCall in the body of $mixin by a
| super-call to a specific parent, e.g. $staticSuperCall
|""".stripMargin, base.sourcePos)
}
}

bcs = bcs.tail
}
assert(sym.exists)
Expand Down
2 changes: 1 addition & 1 deletion compiler/test/dotty/tools/TestSources.scala
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ object TestSources {
val acc2 = files.foldLeft(acc)((acc1, file) => if (file.isFile && file.getPath.endsWith(".scala")) file.getPath :: acc1 else acc1)
files.foldLeft(acc2)((acc3, file) => if (file.isDirectory) collectAllFilesInDir(file, acc3) else acc3)
}
collectAllFilesInDir(new File(stdLibPath), Nil)
collectAllFilesInDir(new File(stdLibPath), Nil).sorted
}

// pos tests lists
Expand Down
26 changes: 26 additions & 0 deletions tests/neg/i5433.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<298..298> in i5433.scala
class Fail cannot be defined due to a conflict between its parents when
implementing a super-accessor for foo in trait C:

1. One of its parent (trait C) contains a call super.foo in its body,
and when a super-call in a trait is written without an explicit parent
listed in brackets, it is implemented by a generated super-accessor in
the class that extends this trait based on the linearization order of
the class.
2. Because B comes before C in the linearization
order of Fail, and because B overrides foo,
the super-accessor in Fail is implemented as a call to
super[B].foo.
3. However,
X (the type of super[B].foo in Fail)
is not a subtype of
Y (the type of foo in trait C).
Hence, the super-accessor that needs to be generated in Fail
is illegal.

Here are two possible ways to resolve this:

1. Change the linearization order of Fail such that
C comes before B.
2. Alternatively, replace super.foo in the body of trait C by a
super-call to a specific parent, e.g. super[A].foo
25 changes: 25 additions & 0 deletions tests/neg/i5433.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
class X
class Y extends X

trait A[+T] {
def foo: T = null.asInstanceOf[T]
}

trait B extends A[X] {
override def foo: X = new X
}

trait C extends A[Y] {
override def foo: Y = new Y
def superFoo: Y = super.foo // C will have an abstract `def C$$super$foo: Y` because of this call
}

class Fail extends B with C // error
// Generated `def C$$super$foo: Y = super[B].foo`

object Test {
def main(args: Array[String]): Unit = {
val y: Y = (new Fail).superFoo // Used to fail with a ClassCastException because of `Fail#C$$super$foo` being incorrect above
assert(y == null)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,11 @@ trait IndexedSeqOptimized[+A, +Repr] extends Any with IndexedSeqLike[A, Repr] {

override /*TraversableLike*/
def reduceLeft[B >: A](op: (B, A) => B): B =
if (length > 0) foldl(1, length, this(0), op) else super.reduceLeft(op)
if (length > 0) foldl(1, length, this(0), op) else super[IndexedSeqLike].reduceLeft(op)

override /*IterableLike*/
def reduceRight[B >: A](op: (A, B) => B): B =
if (length > 0) foldr(0, length - 1, this(length - 1), op) else super.reduceRight(op)
if (length > 0) foldr(0, length - 1, this(length - 1), op) else super[IndexedSeqLike].reduceRight(op)

override /*IterableLike*/
def zip[A1 >: A, B, That](that: GenIterable[B])(implicit bf: CanBuildFrom[Repr, (A1, B), That]): That = that match {
Expand All @@ -89,7 +89,7 @@ trait IndexedSeqOptimized[+A, +Repr] extends Any with IndexedSeqLike[A, Repr] {
}
b.result()
case _ =>
super.zip[A1, B, That](that)(bf)
super[IndexedSeqLike].zip[A1, B, That](that)(bf)
}

override /*IterableLike*/
Expand Down Expand Up @@ -122,16 +122,16 @@ trait IndexedSeqOptimized[+A, +Repr] extends Any with IndexedSeqLike[A, Repr] {
}

override /*IterableLike*/
def head: A = if (isEmpty) super.head else this(0)
def head: A = if (isEmpty) super[IndexedSeqLike].head else this(0)

override /*TraversableLike*/
def tail: Repr = if (isEmpty) super.tail else slice(1, length)
def tail: Repr = if (isEmpty) super[IndexedSeqLike].tail else slice(1, length)

override /*TraversableLike*/
def last: A = if (length > 0) this(length - 1) else super.last
def last: A = if (length > 0) this(length - 1) else super[IndexedSeqLike].last

override /*IterableLike*/
def init: Repr = if (length > 0) slice(0, length - 1) else super.init
def init: Repr = if (length > 0) slice(0, length - 1) else super[IndexedSeqLike].init

override /*TraversableLike*/
def take(n: Int): Repr = slice(0, n)
Expand Down Expand Up @@ -167,7 +167,7 @@ trait IndexedSeqOptimized[+A, +Repr] extends Any with IndexedSeqLike[A, Repr] {
i == len
}
case _ =>
super.sameElements(that)
super[IndexedSeqLike].sameElements(that)
}

override /*IterableLike*/
Expand Down Expand Up @@ -274,7 +274,7 @@ trait IndexedSeqOptimized[+A, +Repr] extends Any with IndexedSeqLike[A, Repr] {
true
}
case _ =>
super.endsWith(that)
super[IndexedSeqLike].endsWith(that)
}
}