Skip to content

Commit 5cebc76

Browse files
smartersmarter
committed
Fix #5433: check that the implemented super-accessor is valid
Relying on `matchingDenotation` is not enough as demonstrated by i5433.scala: in `Fail`, `B#foo` matches `C$$super$foo` but it cannot implement it since `X` is a supertype of `Y` Note that scalac seems had the same bug (but at least in Dotty this is detected by -Ycheck:all), now fixed based on the logic in this PR by scala/scala#7641. For reference, here's what the spec says on resolving super accessors: A reference super.m refers statically to a method or type m in the least proper supertype of the innermost template containing the reference. It evaluates to the member m′ in the actual supertype of that template which is equal to m or which overrides m.
1 parent 4c4d5a1 commit 5cebc76

File tree

3 files changed

+110
-2
lines changed

3 files changed

+110
-2
lines changed

‎compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala

+59-2
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,67 @@ object ResolveSuper {
100100
val SuperAccessorName(memberName) = acc.name.unexpandedName
101101
ctx.debuglog(i"starting rebindsuper from $base of ${acc.showLocated}: ${acc.info} in $bcs, name = $memberName")
102102
while (bcs.nonEmpty && sym == NoSymbol) {
103-
val other = bcs.head.info.nonPrivateDecl(memberName)
103+
val cur = bcs.head
104+
val other = cur.info.nonPrivateDecl(memberName)
104105
if (ctx.settings.Ydebug.value)
105106
ctx.log(i"rebindsuper ${bcs.head} $other deferred = ${other.symbol.is(Deferred)}")
106-
sym = other.matchingDenotation(base.thisType, base.thisType.memberInfo(acc)).symbol
107+
val otherMember = other.matchingDenotation(base.thisType, base.thisType.memberInfo(acc))
108+
if (otherMember.exists) {
109+
sym = otherMember.symbol
110+
// Having a matching denotation is not enough: it should also be a subtype
111+
// of the superaccessor's type, see i5433.scala for an example where this matters
112+
val otherTp = otherMember.asSeenFrom(base.typeRef).info
113+
val accTp = acc.asSeenFrom(base.typeRef).info
114+
if (!(otherTp <:< accTp)) {
115+
// The mixin containing a super-call that requires a super-accessor
116+
val mixin = acc.owner
117+
// The super-call in `mixin`
118+
val superCall = i"super.$memberName"
119+
// The super-call that we end up trying to call
120+
val resolvedSuperCall = i"super[${cur.name}].$memberName"
121+
// The super-call that we would have called if `super` in traits behaved like it
122+
// does in classes, i.e. followed the linearization of the trait itself.
123+
val staticSuperCall = {
124+
val staticSuper = mixin.asClass.info.parents.reverse
125+
.find(_.nonPrivateMember(memberName).matchingDenotation(mixin.thisType, acc.info).exists)
126+
val staticSuperName = staticSuper match {
127+
case Some(parent) =>
128+
parent.classSymbol.name.show
129+
case None => // Might be reachable under separate compilation
130+
"SomeParent"
131+
}
132+
i"super[$staticSuperName].$memberName"
133+
}
134+
ctx.error(
135+
hl"""$base cannot be defined due to a conflict between its parents when
136+
|implementing a super-accessor for $memberName in $mixin:
137+
|
138+
|1. One of its parent ($mixin) contains a call $superCall in its body,
139+
| and when a super-call in a trait is written without an explicit parent
140+
| listed in brackets, it is implemented by a generated super-accessor in
141+
| the class that extends this trait based on the linearization order of
142+
| the class.
143+
|2. Because ${cur.name} comes before ${mixin.name} in the linearization
144+
| order of ${base.name}, and because ${cur.name} overrides $memberName,
145+
| the super-accessor in ${base.name} is implemented as a call to
146+
| $resolvedSuperCall.
147+
|3. However,
148+
| ${otherTp.widenExpr} (the type of $resolvedSuperCall in ${base.name})
149+
| is not a subtype of
150+
| ${accTp.widenExpr} (the type of $memberName in $mixin).
151+
| Hence, the super-accessor that needs to be generated in ${base.name}
152+
| is illegal.
153+
|
154+
|Here are two possible ways to resolve this:
155+
|
156+
|1. Change the linearization order of ${base.name} such that
157+
| ${mixin.name} comes before ${cur.name}.
158+
|2. Alternatively, replace $superCall in the body of $mixin by a
159+
| super-call to a specific parent, e.g. $staticSuperCall
160+
|""".stripMargin, base.sourcePos)
161+
}
162+
}
163+
107164
bcs = bcs.tail
108165
}
109166
assert(sym.exists)

‎tests/neg/i5433.check

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<298..298> in i5433.scala
2+
class Fail cannot be defined due to a conflict between its parents when
3+
implementing a super-accessor for foo in trait C:
4+
5+
1. One of its parent (trait C) contains a call super.foo in its body,
6+
and when a super-call in a trait is written without an explicit parent
7+
listed in brackets, it is implemented by a generated super-accessor in
8+
the class that extends this trait based on the linearization order of
9+
the class.
10+
2. Because B comes before C in the linearization
11+
order of Fail, and because B overrides foo,
12+
the super-accessor in Fail is implemented as a call to
13+
super[B].foo.
14+
3. However,
15+
X (the type of super[B].foo in Fail)
16+
is not a subtype of
17+
Y (the type of foo in trait C).
18+
Hence, the super-accessor that needs to be generated in Fail
19+
is illegal.
20+
21+
Here are two possible ways to resolve this:
22+
23+
1. Change the linearization order of Fail such that
24+
C comes before B.
25+
2. Alternatively, replace super.foo in the body of trait C by a
26+
super-call to a specific parent, e.g. super[A].foo

‎tests/neg/i5433.scala

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
class X
2+
class Y extends X
3+
4+
trait A[+T] {
5+
def foo: T = null.asInstanceOf[T]
6+
}
7+
8+
trait B extends A[X] {
9+
override def foo: X = new X
10+
}
11+
12+
trait C extends A[Y] {
13+
override def foo: Y = new Y
14+
def superFoo: Y = super.foo // C will have an abstract `def C$$super$foo: Y` because of this call
15+
}
16+
17+
class Fail extends B with C // error
18+
// Generated `def C$$super$foo: Y = super[B].foo`
19+
20+
object Test {
21+
def main(args: Array[String]): Unit = {
22+
val y: Y = (new Fail).superFoo // Used to fail with a ClassCastException because of `Fail#C$$super$foo` being incorrect above
23+
assert(y == null)
24+
}
25+
}

0 commit comments

Comments
 (0)