Skip to content

Commit 65596ee

Browse files
committed
Be smarter about validity of owners
Previous scheme never went back once owner was taken to be invalid: all enclosed code was assumed to be with invalid owners. This is not true if the enclosed code contains a class or method. Also previous scheme looked at the owner, whereas the only thing that matters is the enclosing class. Therefore, by-name arguments are no longer considered to be regions with invalid owners. Also: run everything at thisTransform.next, except install forwarders at thisTransform. Previous scheme was weird in that it switched to thisTransform.next in an Apply node, but never switched back, even if said Apply node contains nested classes that need forwarders.
1 parent d726452 commit 65596ee

File tree

2 files changed

+30
-33
lines changed

2 files changed

+30
-33
lines changed

src/dotty/tools/dotc/transform/ForwardParamAccessors.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ trait ForwardParamAccessors extends DenotTransformer { thisTransformer: MacroTra
2525
def currentClass(implicit ctx: Context) = ctx.owner.enclosingClass.asClass
2626

2727
def forwardParamAccessors(impl: Template)(implicit ctx: Context): Template = {
28-
def fwd(stats: List[Tree]): List[Tree] = {
28+
def fwd(stats: List[Tree])(implicit ctx: Context): List[Tree] = {
2929
val (superArgs, superParamNames) = impl.parents match {
3030
case superCall @ Apply(fn, args) :: _ =>
3131
fn.tpe.widen match {
@@ -68,6 +68,6 @@ trait ForwardParamAccessors extends DenotTransformer { thisTransformer: MacroTra
6868
stats map forwardParamAccessor
6969
}
7070

71-
cpy.Template(impl)(body = fwd(impl.body))
71+
cpy.Template(impl)(body = fwd(impl.body)(ctx.withPhase(thisTransformer)))
7272
}
7373
}

src/dotty/tools/dotc/transform/SuperAccessors.scala

Lines changed: 28 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -55,18 +55,36 @@ class SuperAccessors extends MacroTransform
5555
/** the following two members override abstract members in Transform */
5656
override def phaseName: String = "superaccessors"
5757

58+
override def transformPhase(implicit ctx: Context) = thisTransformer.next
59+
5860
protected def newTransformer(implicit ctx: Context): Transformer =
5961
new SuperAccTransformer
6062

6163
class SuperAccTransformer extends Transformer {
6264

63-
/** validCurrentOwner arrives undocumented, but I reverse engineer it to be
64-
* a flag for needsProtectedAccessor which is false while transforming either
65-
* a by-name argument block or a closure. This excludes them from being
66-
* considered able to access protected members via subclassing (why?) which in turn
67-
* increases the frequency with which needsProtectedAccessor will be true.
65+
/** Some parts of trees will get a new owner in subsequent phases.
66+
* These are value class methods, which will become extension methods.
67+
* (By-name arguments used to be included also, but these
68+
* don't get a new class anymore, they are just wrapped in a new method).
69+
*
70+
* These regions will have to be treated specially for the purpose
71+
* of adding accessors. For instance, super calls from these regions
72+
* always have to go through an accessor.
73+
*
74+
* The `invalidOwner` field, if different from NoSymbol,
75+
* contains the symbol that is not a valid owner.
6876
*/
69-
private var validCurrentOwner = true
77+
private var invalidEnclClass: Symbol = NoSymbol
78+
79+
private def withInvalidCurrentClass[A](trans: => A)(implicit ctx: Context): A = {
80+
val saved = invalidEnclClass
81+
invalidEnclClass = ctx.owner
82+
try trans
83+
finally invalidEnclClass = saved
84+
}
85+
86+
private def validCurrentClass(implicit ctx: Context): Boolean =
87+
ctx.owner.enclosingClass != invalidEnclClass
7088

7189
private val accDefs = mutable.Map[Symbol, ListBuffer[Tree]]()
7290

@@ -92,15 +110,7 @@ class SuperAccessors extends MacroTransform
92110
This(clazz).select(superAcc).withPos(sel.pos)
93111
}
94112

95-
private def transformArgs(formals: List[Type], args: List[Tree])(implicit ctx: Context) =
96-
args.zipWithConserve(formals) {(arg, formal) =>
97-
formal match {
98-
case _: ExprType => withInvalidOwner(transform(arg))
99-
case _ => transform(arg)
100-
}
101-
}
102-
103-
private def transformSuperSelect(sel: Select)(implicit ctx: Context): Tree = {
113+
private def transformSuperSelect(sel: Select)(implicit ctx: Context): Tree = {
104114
val Select(sup @ Super(_, mix), name) = sel
105115
val sym = sel.symbol
106116
assert(sup.symbol.exists, s"missing symbol in $sel: ${sup.tpe}")
@@ -126,7 +136,7 @@ class SuperAccessors extends MacroTransform
126136

127137
}
128138
if (name.isTermName && mix == tpnme.EMPTY &&
129-
((clazz is Trait) || clazz != ctx.owner.enclosingClass || !validCurrentOwner))
139+
((clazz is Trait) || clazz != ctx.owner.enclosingClass || !validCurrentClass))
130140
ensureAccessor(sel)(ctx.withPhase(thisTransformer.next))
131141
else sel
132142
}
@@ -246,7 +256,7 @@ class SuperAccessors extends MacroTransform
246256

247257
case tree: DefDef =>
248258
cpy.DefDef(tree)(
249-
rhs = if (isMethodWithExtension(sym)) withInvalidOwner(transform(tree.rhs)) else transform(tree.rhs))
259+
rhs = if (isMethodWithExtension(sym)) withInvalidCurrentClass(transform(tree.rhs)) else transform(tree.rhs))
250260

251261
case TypeApply(sel @ Select(qual, name), args) =>
252262
mayNeedProtectedAccessor(sel, args, goToSuper = true)
@@ -265,12 +275,6 @@ class SuperAccessors extends MacroTransform
265275
}
266276
transformAssign
267277

268-
case Apply(fn, args) =>
269-
val MethodType(_, formals) = fn.tpe.widen
270-
ctx.atPhase(thisTransformer.next) { implicit ctx =>
271-
cpy.Apply(tree)(transform(fn), transformArgs(formals, args))
272-
}
273-
274278
case _ =>
275279
super.transform(tree)
276280
}
@@ -284,13 +288,6 @@ class SuperAccessors extends MacroTransform
284288
}
285289
}
286290

287-
private def withInvalidOwner[A](trans: => A): A = {
288-
val saved = validCurrentOwner
289-
validCurrentOwner = false
290-
try trans
291-
finally validCurrentOwner = saved
292-
}
293-
294291
/** Add a protected accessor, if needed, and return a tree that calls
295292
* the accessor and returns the same member. The result is already
296293
* typed.
@@ -382,7 +379,7 @@ class SuperAccessors extends MacroTransform
382379
val host = hostForAccessorOf(sym, clazz)
383380
val selfType = host.classInfo.selfType
384381
def accessibleThroughSubclassing =
385-
validCurrentOwner && (selfType <:< sym.owner.typeRef) && !clazz.is(Trait)
382+
validCurrentClass && (selfType <:< sym.owner.typeRef) && !clazz.is(Trait)
386383

387384
val isCandidate = (
388385
sym.is(Protected)

0 commit comments

Comments
 (0)