Skip to content

Commit 51fb89d

Browse files
authored
Merge pull request #6120 from dotty-staging/fix-super-toString
Fix #6089: Handle super-accessors involving `()T` to `=> T` overrides
2 parents e8bde5e + 5d692bf commit 51fb89d

File tree

5 files changed

+26
-14
lines changed

5 files changed

+26
-14
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ class LinkScala2Impls extends MiniPhase with IdentityDenotTransformer { thisPhas
5757
)
5858
}
5959
for (sym <- mixin.info.decls) {
60-
if (needsForwarder(sym) || sym.isConstructor || sym.isGetter && sym.is(Lazy) || sym.is(Method, butNot = Deferred))
60+
if (needsMixinForwarder(sym) || sym.isConstructor || sym.isGetter && sym.is(Lazy) || sym.is(Method, butNot = Deferred))
6161
newImpl(sym.asTerm).enteredAfter(thisPhase)
6262
}
6363
// The trait is now fully augmented so the flag isn't needed anymore.

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase =>
247247
else
248248
initial
249249
// transformFollowing call is needed to make memoize & lazy vals run
250-
transformFollowing(DefDef(mkForwarder(getter.asTerm), rhs))
250+
transformFollowing(DefDef(mkForwarderSym(getter.asTerm), rhs))
251251
}
252252
else if (isScala2x || was(getter, ParamAccessor | Lazy)) EmptyTree
253253
else initial
@@ -256,13 +256,13 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase =>
256256

257257
def setters(mixin: ClassSymbol): List[Tree] =
258258
for (setter <- mixin.info.decls.filter(setr => setr.isSetter && !was(setr, Deferred)))
259-
yield transformFollowing(DefDef(mkForwarder(setter.asTerm), unitLiteral.withSpan(cls.span)))
259+
yield transformFollowing(DefDef(mkForwarderSym(setter.asTerm), unitLiteral.withSpan(cls.span)))
260260

261261
def mixinForwarders(mixin: ClassSymbol): List[Tree] =
262-
for (meth <- mixin.info.decls.toList if needsForwarder(meth))
262+
for (meth <- mixin.info.decls.toList if needsMixinForwarder(meth))
263263
yield {
264264
util.Stats.record("mixin forwarders")
265-
transformFollowing(polyDefDef(mkForwarder(meth.asTerm), forwarder(meth)))
265+
transformFollowing(polyDefDef(mkForwarderSym(meth.asTerm), forwarderRhsFn(meth)))
266266
}
267267

268268

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

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ class MixinOps(cls: ClassSymbol, thisPhase: DenotTransformer)(implicit ctx: Cont
1818
map(n => ctx.getClassIfDefined("org.junit." + n)).
1919
filter(_.exists)
2020

21-
def mkForwarder(member: TermSymbol): TermSymbol = {
21+
def mkForwarderSym(member: TermSymbol): TermSymbol = {
2222
val res = member.copy(
2323
owner = cls,
2424
name = member.name.stripScala2LocalSuffix,
@@ -52,7 +52,7 @@ class MixinOps(cls: ClassSymbol, thisPhase: DenotTransformer)(implicit ctx: Cont
5252
* - there's a class defining a method with same signature
5353
* - there are multiple traits defining method with same signature
5454
*/
55-
def needsForwarder(meth: Symbol): Boolean = {
55+
def needsMixinForwarder(meth: Symbol): Boolean = {
5656
lazy val competingMethods = competingMethodsIterator(meth).toList
5757

5858
def needsDisambiguation = competingMethods.exists(x=> !(x is Deferred)) // multiple implementations are available
@@ -71,9 +71,18 @@ class MixinOps(cls: ClassSymbol, thisPhase: DenotTransformer)(implicit ctx: Cont
7171
final val PrivateOrAccessor: FlagSet = Private | Accessor
7272
final val PrivateOrAccessorOrDeferred: FlagSet = Private | Accessor | Deferred
7373

74-
def forwarder(target: Symbol): List[Type] => List[List[Tree]] => Tree =
75-
targs => vrefss =>
76-
superRef(target).appliedToTypes(targs).appliedToArgss(vrefss)
74+
def forwarderRhsFn(target: Symbol): List[Type] => List[List[Tree]] => Tree =
75+
targs => vrefss => {
76+
val tapp = superRef(target).appliedToTypes(targs)
77+
vrefss match {
78+
case Nil | List(Nil) =>
79+
// Overriding is somewhat loose about `()T` vs `=> T`, so just pick
80+
// whichever makes sense for `target`
81+
tapp.ensureApplied
82+
case _ =>
83+
tapp.appliedToArgss(vrefss)
84+
}
85+
}
7786

7887
private def competingMethodsIterator(meth: Symbol): Iterator[Symbol] = {
7988
cls.baseClasses.iterator

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ class ResolveSuper extends MiniPhase with IdentityDenotTransformer { thisPhase =
3737
AugmentScala2Traits.name,
3838
PruneErasedDefs.name) // Erased decls make `isCurrent` work incorrectly
3939

40-
override def changesMembers: Boolean = true // the phase adds super accessors and method forwarders
40+
override def changesMembers: Boolean = true // the phase adds super accessors
4141

4242
override def transformTemplate(impl: Template)(implicit ctx: Context): Template = {
4343
val cls = impl.symbol.owner.asClass
@@ -48,7 +48,7 @@ class ResolveSuper extends MiniPhase with IdentityDenotTransformer { thisPhase =
4848
for (superAcc <- mixin.info.decls.filter(_.isSuperAccessor))
4949
yield {
5050
util.Stats.record("super accessors")
51-
polyDefDef(mkForwarder(superAcc.asTerm), forwarder(rebindSuper(cls, superAcc)))
51+
polyDefDef(mkForwarderSym(superAcc.asTerm), forwarderRhsFn(rebindSuper(cls, superAcc)))
5252
}
5353

5454
val overrides = mixins.flatMap(superAccessors)
@@ -63,7 +63,7 @@ class ResolveSuper extends MiniPhase with IdentityDenotTransformer { thisPhase =
6363
val cls = meth.owner.asClass
6464
val ops = new MixinOps(cls, thisPhase)
6565
import ops._
66-
polyDefDef(meth, forwarder(rebindSuper(cls, meth)))
66+
polyDefDef(meth, forwarderRhsFn(rebindSuper(cls, meth)))
6767
}
6868
else ddef
6969
}
@@ -92,7 +92,7 @@ object ResolveSuper {
9292
// of the superaccessor's type, see i5433.scala for an example where this matters
9393
val otherTp = other.asSeenFrom(base.typeRef).info
9494
val accTp = acc.asSeenFrom(base.typeRef).info
95-
if (!(otherTp <:< accTp))
95+
if (!(otherTp.overrides(accTp, matchLoosely = true)))
9696
ctx.error(IllegalSuperAccessor(base, memberName, acc, accTp, other.symbol, otherTp), base.sourcePos)
9797
}
9898

tests/pos/i6089.scala

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
trait X { override def toString = super.toString + " X" }
2+
trait Y { override def toString = super.toString + " Y" }
3+
class Z extends X with Y

0 commit comments

Comments
 (0)