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.

2 files changed, 30 insertions, 33 deletions

diff --git a/src/dotty/tools/dotc/transform/ForwardParamAccessors.scala b/src/dotty/tools/dotc/transform/ForwardParamAccessors.scala
index b65681a4b..ff4dc2652 100644
--- a/src/dotty/tools/dotc/transform/ForwardParamAccessors.scala
+++ b/src/dotty/tools/dotc/transform/ForwardParamAccessors.scala
@@ -25,7 +25,7 @@ trait ForwardParamAccessors extends DenotTransformer { thisTransformer: MacroTra
   def currentClass(implicit ctx: Context) = ctx.owner.enclosingClass.asClass
 
   def forwardParamAccessors(impl: Template)(implicit ctx: Context): Template = {
-    def fwd(stats: List[Tree]): List[Tree] = {
+    def fwd(stats: List[Tree])(implicit ctx: Context): List[Tree] = {
       val (superArgs, superParamNames) = impl.parents match {
         case superCall @ Apply(fn, args) :: _ =>
           fn.tpe.widen match {
@@ -68,6 +68,6 @@ trait ForwardParamAccessors extends DenotTransformer { thisTransformer: MacroTra
       stats map forwardParamAccessor
     }
 
-    cpy.Template(impl)(body = fwd(impl.body))
+    cpy.Template(impl)(body = fwd(impl.body)(ctx.withPhase(thisTransformer)))
   }
 }
\ No newline at end of file
diff --git a/src/dotty/tools/dotc/transform/SuperAccessors.scala b/src/dotty/tools/dotc/transform/SuperAccessors.scala
index cfd78a91d..ffad534b4 100644
--- a/src/dotty/tools/dotc/transform/SuperAccessors.scala
+++ b/src/dotty/tools/dotc/transform/SuperAccessors.scala
@@ -55,18 +55,36 @@ class SuperAccessors extends MacroTransform
   /** the following two members override abstract members in Transform */
   override def phaseName: String = "superaccessors"
 
+  override def transformPhase(implicit ctx: Context) = thisTransformer.next
+
   protected def newTransformer(implicit ctx: Context): Transformer =
     new SuperAccTransformer
 
   class SuperAccTransformer extends Transformer {
 
-    /** validCurrentOwner arrives undocumented, but I reverse engineer it to be
-     *  a flag for needsProtectedAccessor which is false while transforming either
-     *  a by-name argument block or a closure.  This excludes them from being
-     *  considered able to access protected members via subclassing (why?) which in turn
-     *  increases the frequency with which needsProtectedAccessor will be true.
+    /** Some parts of trees will get a new owner in subsequent phases.
+     *  These are value class methods, which will become extension methods.
+     *  (By-name arguments used to be included also, but these
+     *  don't get a new class anymore, they are just wrapped in a new method).
+     *  
+     *  These regions will have to be treated specially for the purpose
+     *  of adding accessors. For instance, super calls from these regions
+     *  always have to go through an accessor.
+     *  
+     *  The `invalidOwner` field, if different from NoSymbol,
+     *  contains the symbol that is not a valid owner.
      */
-    private var validCurrentOwner = true
+    private var invalidEnclClass: Symbol = NoSymbol
+
+    private def withInvalidCurrentClass[A](trans: => A)(implicit ctx: Context): A = {
+      val saved = invalidEnclClass
+      invalidEnclClass = ctx.owner
+      try trans
+      finally invalidEnclClass = saved
+    }
+    
+    private def validCurrentClass(implicit ctx: Context): Boolean = 
+      ctx.owner.enclosingClass != invalidEnclClass
 
     private val accDefs = mutable.Map[Symbol, ListBuffer[Tree]]()
 
@@ -92,15 +110,7 @@ class SuperAccessors extends MacroTransform
       This(clazz).select(superAcc).withPos(sel.pos)
     }
 
-    private def transformArgs(formals: List[Type], args: List[Tree])(implicit ctx: Context) =
-      args.zipWithConserve(formals) {(arg, formal) =>
-        formal match {
-          case _: ExprType => withInvalidOwner(transform(arg))
-          case _ => transform(arg)
-        }
-      }
-
-   private def transformSuperSelect(sel: Select)(implicit ctx: Context): Tree = {
+  private def transformSuperSelect(sel: Select)(implicit ctx: Context): Tree = {
       val Select(sup @ Super(_, mix), name) = sel
       val sym   = sel.symbol
       assert(sup.symbol.exists, s"missing symbol in $sel: ${sup.tpe}")
@@ -126,7 +136,7 @@ class SuperAccessors extends MacroTransform
 
         }
       if (name.isTermName && mix == tpnme.EMPTY &&
-          ((clazz is Trait) || clazz != ctx.owner.enclosingClass || !validCurrentOwner))
+          ((clazz is Trait) || clazz != ctx.owner.enclosingClass || !validCurrentClass))
         ensureAccessor(sel)(ctx.withPhase(thisTransformer.next))
       else sel
     }
@@ -246,7 +256,7 @@ class SuperAccessors extends MacroTransform
 
         case tree: DefDef =>
           cpy.DefDef(tree)(
-            rhs = if (isMethodWithExtension(sym)) withInvalidOwner(transform(tree.rhs)) else transform(tree.rhs))
+            rhs = if (isMethodWithExtension(sym)) withInvalidCurrentClass(transform(tree.rhs)) else transform(tree.rhs))
 
         case TypeApply(sel @ Select(qual, name), args) =>
           mayNeedProtectedAccessor(sel, args, goToSuper = true)
@@ -265,12 +275,6 @@ class SuperAccessors extends MacroTransform
           }
           transformAssign
 
-        case Apply(fn, args) =>
-          val MethodType(_, formals) = fn.tpe.widen
-          ctx.atPhase(thisTransformer.next) { implicit ctx =>
-            cpy.Apply(tree)(transform(fn), transformArgs(formals, args))
-          }
-
         case _ =>
           super.transform(tree)
       }
@@ -284,13 +288,6 @@ class SuperAccessors extends MacroTransform
       }
     }
 
-    private def withInvalidOwner[A](trans: => A): A = {
-      val saved = validCurrentOwner
-      validCurrentOwner = false
-      try trans
-      finally validCurrentOwner = saved
-    }
-
     /** Add a protected accessor, if needed, and return a tree that calls
      *  the accessor and returns the same member. The result is already
      *  typed.
@@ -382,7 +379,7 @@ class SuperAccessors extends MacroTransform
       val host = hostForAccessorOf(sym, clazz)
       val selfType = host.classInfo.selfType
       def accessibleThroughSubclassing =
-        validCurrentOwner && (selfType <:< sym.owner.typeRef) && !clazz.is(Trait)
+        validCurrentClass && (selfType <:< sym.owner.typeRef) && !clazz.is(Trait)
 
       val isCandidate = (
            sym.is(Protected)