Disable generation of protected accessors in SuperAccessors phase

Fix remaining problems, mostly having to do with choosing between
super accessors and protected accessors.

Author: odersky

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

@@ -21,6 +21,7 @@ import config.Printers.transforms
  */
 abstract class AccessProxies {
   import ast.tpd._
+  import AccessProxies._
 
   def getterName: ClassifiedNameKind
   def setterName: ClassifiedNameKind
@@ -76,17 +77,13 @@ abstract class AccessProxies {
 
       def refersToAccessed(sym: Symbol) = accessedBy.get(sym) == Some(accessed)
 
-      val accessorClass = {
-        def owningClass(start: Symbol) =
-          start.ownersIterator.findSymbol(_.derivesFrom(accessed.owner))
+      var accessorClass = hostForAccessorOf(accessed: Symbol)
+      if (!accessorClass.exists) {
         val curCls = ctx.owner.enclosingClass
-        var owner = owningClass(curCls) //`orElse` owningClass(curCls.linkedClass)
-        if (!owner.exists) {
-          transforms.println(i"${curCls.ownersIterator.toList}%, %")
-          ctx.error(i"illegal access to protected ${accessed.showLocated} from $curCls", reference.pos)
-          owner = curCls
-        }
-        owner
+        transforms.println(i"${curCls.ownersIterator.toList}%, %")
+        ctx.error(i"illegal access to protected ${accessed.showLocated} from $curCls",
+          reference.pos)
+        accessorClass = curCls
       }
 
       val accessorRawInfo =
@@ -125,4 +122,8 @@ abstract class AccessProxies {
         tree
     }
   }
+}
+object AccessProxies {
+  def hostForAccessorOf(accessed: Symbol)(implicit ctx: Context): Symbol =
+    ctx.owner.ownersIterator.findSymbol(_.derivesFrom(accessed.owner))
 }

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

@@ -216,12 +216,10 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase
             case sel: Select =>
               val args1 = transform(args)
               val sel1 = transformSelect(sel, args1)
-              if (superAcc.isProtectedAccessor(sel1)) sel1 else cpy.TypeApply(tree1)(sel1, args1)
+              cpy.TypeApply(tree1)(sel1, args1)
             case _ =>
               super.transform(tree1)
           }
-        case tree @ Assign(sel: Select, _) =>
-          super.transform(superAcc.transformAssign(tree))
         case Inlined(call, bindings, expansion) =>
           // Leave only a call trace consisting of
           //  - a reference to the top-level class from which the call was inlined,

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

@@ -8,14 +8,50 @@ import core.Flags._
 import core.Decorators._
 import MegaPhase.MiniPhase
 import ast.Trees._
+import util.Property
 
 /** Add accessors for all protected accesses. An accessor is needed if
  *  according to the rules of the JVM a protected class member is not accesissible
  *  from the point of access, but is accessible if the access is from an enclosing
  *  class. In this point a public access method is placed in that enclosing class.
  */
+object ProtectedAccessors {
+  val name = "protectedAccessors"
+
+  private val LHS = new Property.StickyKey[Unit]
+
+  /** Is the current context's owner inside the access boundary established by `sym`? */
+  def insideBoundaryOf(sym: Symbol)(implicit ctx: Context): Boolean = {
+    if (sym.is(JavaDefined)) {
+      sym.is(JavaStatic) ||  // Java's static protected definitions are treated as public
+      ctx.owner.enclosingPackageClass == sym.enclosingPackageClass
+    }
+    else {
+      // For Scala-defined symbols we currently allow private and protected accesses
+      // from inner packages, and compensate by widening accessibility of such symbols to public.
+      // It would be good if we could revisit this at some point.
+      val boundary = sym.accessBoundary(sym.enclosingPackageClass)
+      ctx.owner.isContainedIn(boundary) || ctx.owner.isContainedIn(boundary.linkedClass)
+    }
+  }
+
+  /** Do we need a protected accessor if the current context's owner
+   *  is not in a subclass or subtrait of `sym`?
+   */
+  def needsAccessorIfNotInSubclass(sym: Symbol)(implicit ctx: Context): Boolean =
+    sym.isTerm && sym.is(Protected) &&
+    !sym.owner.is(Trait) && // trait methods need to be handled specially, are currently always public
+    !insideBoundaryOf(sym)
+
+   /** Do we need a protected accessor for accessing sym from the current context's owner? */
+   def needsAccessor(sym: Symbol)(implicit ctx: Context): Boolean =
+    needsAccessorIfNotInSubclass(sym) &&
+    !ctx.owner.enclosingClass.derivesFrom(sym.owner)
+}
+
 class ProtectedAccessors extends MiniPhase {
   import ast.tpd._
+  import ProtectedAccessors._
 
   override def phaseName = ProtectedAccessors.name
 
@@ -24,39 +60,19 @@ class ProtectedAccessors extends MiniPhase {
     def setterName = ProtectedSetterName
 
     val insert = new Insert {
-      def needsAccessor(sym: Symbol)(implicit ctx: Context): Boolean = {
-        def insideBoundary = {
-          if (sym.is(JavaDefined)) {
-            sym.is(JavaStatic) ||  // Java's static protected definitions are treated as public
-            ctx.owner.enclosingPackageClass == sym.enclosingPackageClass
-          }
-          else {
-            // For Scala-defined symbols we currently allow private and protected accesses
-            // from inner packages, and compensate by widening accessibility of such symbols to public.
-            // It would be good if we could revisit this at some point.
-            val boundary = sym.accessBoundary(sym.enclosingPackageClass)
-            ctx.owner.isContainedIn(boundary) || ctx.owner.isContainedIn(boundary.linkedClass)
-          }
-        }
-        sym.isTerm && sym.is(Protected) &&
-        !sym.owner.is(Trait) && // trait methods need to be handled specially, are currently always public
-        !insideBoundary &&
-        !ctx.owner.enclosingClass.derivesFrom(sym.owner)
-      }
+      def needsAccessor(sym: Symbol)(implicit ctx: Context) = ProtectedAccessors.needsAccessor(sym)
     }
   }
 
-  override def prepareForAssign(tree: Assign)(implicit ctx: Context) = tree.lhs match {
-    case tree: RefTree if Accessors.insert.needsAccessor(tree.symbol) =>
-      ctx.fresh.setTree(tree)
-    case _ =>
-      ctx
+  override def prepareForAssign(tree: Assign)(implicit ctx: Context) = {
+    tree.lhs match {
+      case lhs: RefTree if needsAccessor(lhs.symbol) => lhs.putAttachment(LHS, ())
+      case _ =>
+    }
+    ctx
   }
 
-  def isLHS(tree: RefTree)(implicit ctx: Context) = ctx.tree match {
-    case Assign(lhs, _) => tree.symbol == lhs.symbol
-    case _ => false
-  }
+  private def isLHS(tree: RefTree) = tree.removeAttachment(LHS).isDefined
 
   override def transformIdent(tree: Ident)(implicit ctx: Context): Tree =
     if (isLHS(tree)) tree else Accessors.insert.accessorIfNeeded(tree)
@@ -70,5 +86,3 @@ class ProtectedAccessors extends MiniPhase {
   override def transformTemplate(tree: Template)(implicit ctx: Context): Tree =
     cpy.Template(tree)(body = Accessors.addAccessorDefs(tree.symbol.owner, tree.body))
 }
-object ProtectedAccessors { val name = "protectedAccessors" }
-