Skip to content

Commit de8b794

Browse files
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 df8f79f commit de8b794

File tree

3 files changed

+88
-2
lines changed

3 files changed

+88
-2
lines changed

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

+44-2
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,52 @@ 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+
val superCall = i"super.$memberName"
116+
val accSuperCall = i"super[${acc.owner.name}].$memberName"
117+
val resolvedSuperCall = i"super[${cur.name}].$memberName"
118+
val overriddenOwnerName = {
119+
val overriddenIterator = other.symbol.allOverriddenSymbols
120+
if (overriddenIterator.hasNext)
121+
overriddenIterator.next.owner.name
122+
else
123+
"SomeParent"
124+
}
125+
val overriddenSuperCall = i"super[$overriddenOwnerName].$memberName"
126+
ctx.error(
127+
hl"""$base cannot be instantiated due to a conflict between its parents when
128+
|resolving a super-call:
129+
|
130+
|1. One of its parent (${acc.owner}) contains a call $superCall in its body.
131+
|2. Because ${cur.name} comes before ${acc.owner.name} in the linearization
132+
| order of ${base.name}, and because ${cur.name} overrides $memberName,
133+
| this is resolved as a call to $resolvedSuperCall in ${base.name}.
134+
|3. However:
135+
| ${otherTp.widenExpr} (the type of $resolvedSuperCall in ${base.name})
136+
| is not a subtype of
137+
| ${accTp.widenExpr} (the type of $accSuperCall in ${base.name})
138+
|
139+
|Here are two possible ways to resolve this:
140+
|
141+
|1. Change the linearization order of ${base.name} such that
142+
| ${acc.owner.name} comes before ${cur.name}.
143+
|2. Alternatively, replace $superCall in the body of ${acc.owner} by a
144+
| super-call to a specific parent, e.g. $overriddenSuperCall
145+
|""".stripMargin, base.sourcePos)
146+
}
147+
}
148+
107149
bcs = bcs.tail
108150
}
109151
assert(sym.exists)

tests/neg/i5433.check

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<298..298> in i5433.scala
2+
class Fail cannot be instantiated due to a conflict between its parents when
3+
resolving a super-call:
4+
5+
1. One of its parent (trait C) contains a call super.foo in its body.
6+
2. Because B comes before C in the linearization
7+
order of Fail, and because B overrides foo,
8+
this is resolved as a call to super[B].foo in Fail.
9+
3. However:
10+
X (the type of super[B].foo in Fail)
11+
is not a subtype of
12+
Y (the type of super[C].foo in Fail)
13+
14+
Here are two possible ways to resolve this:
15+
16+
1. Change the linearization order of Fail such that
17+
C comes before B.
18+
2. Alternatively, replace super.foo in the body of trait C by a
19+
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)