Drop more outer accessors

We now also drop outer accessors of classes that are visible
only in one toplevel class. This requires counting accessors
in a complete traversal. We therefore need two phases in
separate groups to do the job. The first phase counts, the
second elimintaes accessors using the info gathered in the
first phase.

Author: odersky

compiler/src/dotty/tools/dotc/Compiler.scala

@@ -111,8 +111,9 @@ class Compiler {
          new LambdaLift,             // Lifts out nested functions to class scope, storing free variables in environments
                                      // Note: in this mini-phase block scopes are incorrect. No phases that rely on scopes should be here
          new ElimStaticThis,         // Replace `this` references to static objects by global identifiers
-         new DropOuterAccessors) ::  // Drop unused outer accessors
-    List(new Flatten,                // Lift all inner classes to package scope
+         new CountOuterAccesses) ::  // Identify outer accessors that can be dropped
+    List(new DropOuterAccessors,     // Drop unused outer accessors
+         new Flatten,                // Lift all inner classes to package scope
          new RenameLifted,           // Renames lifted classes to local numbering scheme
          new TransformWildcards,     // Replace wildcards with default values
          new MoveStatics,            // Move static methods from companion to the class itself

compiler/src/dotty/tools/dotc/core/Phases.scala

@@ -230,6 +230,7 @@ object Phases {
     private var myErasurePhase: Phase = _
     private var myElimErasedValueTypePhase: Phase = _
     private var myLambdaLiftPhase: Phase = _
+    private var myCountOuterAccessesPhase: Phase = _
     private var myFlattenPhase: Phase = _
     private var myGenBCodePhase: Phase = _
 
@@ -248,6 +249,7 @@ object Phases {
     final def erasurePhase: Phase = myErasurePhase
     final def elimErasedValueTypePhase: Phase = myElimErasedValueTypePhase
     final def lambdaLiftPhase: Phase = myLambdaLiftPhase
+    final def countOuterAccessesPhase = myCountOuterAccessesPhase
     final def flattenPhase: Phase = myFlattenPhase
     final def genBCodePhase: Phase = myGenBCodePhase
 
@@ -267,6 +269,7 @@ object Phases {
       myElimErasedValueTypePhase = phaseOfClass(classOf[ElimErasedValueType])
       myPatmatPhase = phaseOfClass(classOf[PatternMatcher])
       myLambdaLiftPhase = phaseOfClass(classOf[LambdaLift])
+      myCountOuterAccessesPhase = phaseOfClass(classOf[CountOuterAccesses])
       myFlattenPhase = phaseOfClass(classOf[Flatten])
       myExplicitOuterPhase = phaseOfClass(classOf[ExplicitOuter])
       myGettersPhase = phaseOfClass(classOf[Getters])

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

@@ -0,0 +1,58 @@
+package dotty.tools.dotc
+package transform
+
+import core._
+import MegaPhase.MiniPhase
+import dotty.tools.dotc.core.Contexts.Context
+import ast._
+import Trees._
+import Flags._
+import Symbols._
+import Decorators._
+import ExplicitOuter.isOuterParamAccessor
+
+import collection.mutable
+
+object CountOuterAccesses:
+  val name: String = "countOuterAccesses"
+
+  /** Characterizes outer accessors and outer fields that can be dropped
+   *  if there are no references to them from within the toplevel class
+   *  where they are defined.
+   */
+  def mightBeDropped(sym: Symbol)(using Context) =
+    def isLocal(cls: Symbol) =
+      cls.isAnonymousClass
+      || cls.owner.isTerm
+      || cls.accessBoundary(defn.RootClass).isContainedIn(cls.topLevelClass)
+    (sym.is(OuterAccessor) || sym.isOuterParamAccessor) && isLocal(sym.owner)
+
+/** Counts number of accesses to outer accessors and outer fields of
+ *  classes that are visible only within one source file. The info
+ *  is collected in `outerAccessCount` and used in the subsequent
+ *  DropOuterAccessors phase
+ */
+class CountOuterAccesses extends MiniPhase:
+  thisPhase =>
+  import tpd._
+
+  override def phaseName: String = CountOuterAccesses.name
+
+  override def runsAfter: Set[String] = Set(LambdaLift.name)
+    // LambdaLift can create outer paths. These need to be known in this phase.
+
+  /** The number of times an outer accessor that might be dropped is accessed */
+  val outerAccessCount = new mutable.HashMap[Symbol, Int] {
+    override def default(s: Symbol): Int = 0
+  }
+
+  private def markAccessed(tree: RefTree)(implicit ctx: Context): Tree =
+    val sym = tree.symbol
+    if CountOuterAccesses.mightBeDropped(sym) then outerAccessCount(sym) += 1
+    tree
+
+  override def transformIdent(tree: Ident)(using Context): Tree =
+    markAccessed(tree)
+
+  override def transformSelect(tree: Select)(using Context): Tree =
+    markAccessed(tree)

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

@@ -8,62 +8,44 @@ import ast._
 import Trees._
 import Flags._
 import Symbols._
+import Contexts._
 import Decorators._
 import DenotTransformers._
-import StdNames.nme
+import ExplicitOuter.isOuterParamAccessor
+import CountOuterAccesses.mightBeDropped
 import collection.mutable
+import annotation.threadUnsafe
 
 object DropOuterAccessors:
   val name: String = "dropOuterAccessors"
 
-/** Drops outer accessors of final classes that are unused */
+/** Drops unused outer accessors of inner classes that are visible only in one
+ *  toplevel class. For other classes, we can't tell whether an outer accessor
+ *  is used or not. It could for instance be used in a type test in some other source.
+ */
 class DropOuterAccessors extends MiniPhase with IdentityDenotTransformer:
   thisPhase =>
   import tpd._
 
   override def phaseName: String = DropOuterAccessors.name
 
-  override def runsAfter: Set[String] = Set(LambdaLift.name)
-    // LambdaLift can create outer paths. These need to be known in this phase
+  override def runsAfterGroupsOf: Set[String] = Set(CountOuterAccesses.name)
 
   override def changesMembers: Boolean = true // the phase drops outer accessors
 
-  def (sym: Symbol).isOuterParamAccessor(using Context) =
-    sym.is(ParamAccessor) && sym.name == nme.OUTER
-
-  /** Characterizes outer accessors and outer fields that can be dropped
-   *  if there are no references to them from within the class in which they
-   *  are defined.
-   */
-  private def mightBeDropped(sym: Symbol)(using Context) =
-    (sym.is(OuterAccessor) || sym.isOuterParamAccessor)
-    && sym.owner.isAnonymousClass
-
-  /** The number of times an outer accessor that might be dropped is accessed */
-  private val accessCount = new mutable.HashMap[Symbol, Int]:
-    override def default(key: Symbol): Int = 0
-
-  private def markAccessed(tree: RefTree)(implicit ctx: Context): Tree =
-    val sym = tree.symbol
-    if mightBeDropped(sym) then accessCount(sym) += 1
-    tree
-
-  override def transformIdent(tree: Ident)(using Context): Tree =
-    markAccessed(tree)
-
-  override def transformSelect(tree: Select)(using Context): Tree =
-    markAccessed(tree)
-
   override def transformTemplate(impl: Template)(using ctx: Context): Tree =
+    val outerAccessCount = ctx.base.countOuterAccessesPhase
+      .asInstanceOf[CountOuterAccesses]
+      .outerAccessCount
 
     def dropOuterAccessor(stat: Tree): Boolean = stat match
       case stat: DefDef
       if stat.symbol.is(OuterAccessor)
           && mightBeDropped(stat.symbol)
-          && accessCount(stat.symbol) == 0 =>
+          && outerAccessCount(stat.symbol) == 0 =>
         assert(stat.rhs.isInstanceOf[RefTree], stat)
-        assert(accessCount(stat.rhs.symbol) > 0)
-        accessCount(stat.rhs.symbol) -= 1
+        assert(outerAccessCount(stat.rhs.symbol) > 0)
+        outerAccessCount(stat.rhs.symbol) -= 1
         stat.symbol.dropAfter(thisPhase)
         true
       case _ =>
@@ -75,7 +57,7 @@ class DropOuterAccessors extends MiniPhase with IdentityDenotTransformer:
       case stat: ValDef
       if stat.symbol.isOuterParamAccessor
           && mightBeDropped(stat.symbol)
-          && accessCount(stat.symbol) == 1 =>
+          && outerAccessCount(stat.symbol) == 1 =>
         droppedParamAccessors += stat.symbol
         stat.symbol.dropAfter(thisPhase)
         true

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

@@ -323,6 +323,9 @@ object ExplicitOuter {
       tpe
   }
 
+  def (sym: Symbol).isOuterParamAccessor(using Context): Boolean =
+    sym.is(ParamAccessor) && sym.name == nme.OUTER
+
   def outer(implicit ctx: Context): OuterOps = new OuterOps(ctx)
 
   /** The operations in this class