Fix #8033: Eliminate unused outer accessors

Author: odersky

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

@@ -109,8 +109,9 @@ class Compiler {
          new GetClass) ::            // Rewrites getClass calls on primitive types.
     List(new LinkScala2Impls,        // Redirect calls to trait methods defined by Scala 2.x, so that they now go to
          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
+                                     // 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 RenameLifted,           // Renames lifted classes to local numbering scheme
          new TransformWildcards,     // Replace wildcards with default values

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

@@ -1079,9 +1079,15 @@ object SymDenotations {
     final def lexicallyEnclosingClass(implicit ctx: Context): Symbol =
       if (!exists || isClass) symbol else owner.lexicallyEnclosingClass
 
+    /** A class is extensible if it is not final, nor a module class,
+     *  nor an anonymous class.
+     */
+    final def isExtensibleClass(using Context): Boolean =
+      isClass && !isOneOf(FinalOrModuleClass) && !isAnonymousClass
+
     /** A symbol is effectively final if it cannot be overridden in a subclass */
     final def isEffectivelyFinal(implicit ctx: Context): Boolean =
-      isOneOf(EffectivelyFinalFlags) || !owner.isClass || owner.isOneOf(FinalOrModuleClass) || owner.isAnonymousClass
+      isOneOf(EffectivelyFinalFlags) || !owner.isExtensibleClass
 
     /** A class is effectively sealed if has the `final` or `sealed` modifier, or it
      *  is defined in Scala 3 and is neither abstract nor open.

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

@@ -0,0 +1,100 @@
+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 DenotTransformers._
+import StdNames.nme
+import collection.mutable
+
+object DropOuterAccessors:
+  val name: String = "dropOuterAccessors"
+
+/** Drops outer accessors of final classes that are unused */
+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 changesMembers: Boolean = true // the phase drops outer accessors
+
+  def (sym: Symbol).isOuterParamAccessor(using Context) =
+    sym.is(ParamAccessor) && sym.name == nme.OUTER
+
+  private def mightBeDropped(sym: Symbol)(using Context) =
+    (sym.is(OuterAccessor) || sym.isOuterParamAccessor)
+    && !sym.owner.isExtensibleClass
+
+  /** 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 =
+
+    def dropOuterAccessor(stat: Tree): Boolean = stat match
+      case stat: DefDef
+      if stat.symbol.is(OuterAccessor)
+          && mightBeDropped(stat.symbol)
+          && accessCount(stat.symbol) == 0 =>
+        assert(stat.rhs.isInstanceOf[RefTree], stat)
+        assert(accessCount(stat.rhs.symbol) > 0)
+        accessCount(stat.rhs.symbol) -= 1
+        stat.symbol.dropAfter(thisPhase)
+        true
+      case _ =>
+        false
+
+    val droppedParamAccessors = mutable.Set[Symbol]()
+
+    def dropOuterParamAccessor(stat: Tree): Boolean = stat match
+      case stat: ValDef
+      if stat.symbol.isOuterParamAccessor
+          && mightBeDropped(stat.symbol)
+          && accessCount(stat.symbol) == 1 =>
+        droppedParamAccessors += stat.symbol
+        stat.symbol.dropAfter(thisPhase)
+        true
+      case _ =>
+        false
+
+    def dropOuterInit(stat: Tree): Boolean = stat match
+      case Assign(lhs, rhs) => droppedParamAccessors.remove(lhs.symbol)
+      case _ => false
+
+    val body1 = impl.body
+      .filterNot(dropOuterAccessor)
+      .filterNot(dropOuterParamAccessor)
+    val constr1 =
+      if droppedParamAccessors.isEmpty then impl.constr
+      else cpy.DefDef(impl.constr)(
+        rhs = impl.constr.rhs match {
+          case rhs @ Block(inits, expr) =>
+            cpy.Block(rhs)(inits.filterNot(dropOuterInit), expr)
+        })
+    assert(droppedParamAccessors.isEmpty,
+      i"""Failed to eliminate: $droppedParamAccessors
+          when dropping outer accessors for ${ctx.owner} with
+          $impl""")
+    cpy.Template(impl)(constr = constr1, body = body1)
+  end transformTemplate

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

@@ -23,6 +23,8 @@ object LambdaLift {
   import ast.tpd._
   private class NoPath extends Exception
 
+  val name: String = "lambdaLift"
+
   /** The core lambda lift functionality. */
   class Lifter(thisPhase: MiniPhase with DenotTransformer)(implicit ctx: Context) {
 
@@ -500,7 +502,7 @@ class LambdaLift extends MiniPhase with IdentityDenotTransformer { thisPhase =>
   import ast.tpd._
 
   /** the following two members override abstract members in Transform */
-  val phaseName: String = "lambdaLift"
+  val phaseName: String = LambdaLift.name
 
   override def relaxedTypingInGroup: Boolean = true
     // Because it adds free vars as additional proxy parameters

tests/run/i8033.scala

@@ -0,0 +1,36 @@
+trait Okay extends Serializable {
+  def okay: Okay
+}
+
+class Foo {
+  def okay1: Okay = new Okay() {
+    val okay: Okay = this
+    override def toString = "okay1"
+  }
+  def okay2: Okay = new Okay {
+    val okay: Okay = okay1
+    override def toString = "okay2"
+  }
+}
+
+object Test {
+  def main(args: Array[String]): Unit = {
+    val foo = new Foo
+    assert(roundTrip(foo.okay1).toString == "okay1")
+    assert(roundTrip(foo.okay2).toString == "okay2")
+  }
+
+  def roundTrip[A](a: A): A = {
+    import java.io._
+
+    val aos = new ByteArrayOutputStream()
+    val oos = new ObjectOutputStream(aos)
+    oos.writeObject(a)
+    oos.close()
+    val ais = new ByteArrayInputStream(aos.toByteArray())
+    val ois: ObjectInputStream = new ObjectInputStream(ais)
+    val newA = ois.readObject()
+    ois.close()
+    newA.asInstanceOf[A]
+  }
+}

tests/run/outer-accessors.scala

@@ -0,0 +1,18 @@
+class A:
+  val a = 2
+
+  class B:
+    val b = 3
+
+    trait T:
+      def t = a + b
+
+  val bb = B()
+
+  class C extends bb.T:
+    def result = a + t
+
+@main def Test =
+  val a = A()
+  val c = a.C()
+  assert(c.result == 7)