Merge pull request scala#4652 from retronym/ticket/9408

SI-9408 Avoid capturing outer class in local classes.

Author: lrytz

src/compiler/scala/tools/nsc/transform/Constructors.scala

@@ -165,11 +165,19 @@ abstract class Constructors extends Statics with Transform with ast.TreeDSL {
         return
       }
 
+      // Note: elision of outer reference is based on a class-wise analysis, if a class might have subclasses,
+      //       it doesn't work. For example, `LocalParent` retains the outer reference in:
+      //
+      //   class Outer { def test = {class LocalParent; class LocalChild extends LocalParent } }
+      //
+      // See run/t9408.scala for related test cases.
+      val isEffectivelyFinal = clazz.isEffectivelyFinal
       def isParamCandidateForElision(sym: Symbol) = (sym.isParamAccessor && sym.isPrivateLocal)
-      def isOuterCandidateForElision(sym: Symbol) = (sym.isOuterAccessor && sym.owner.isEffectivelyFinal && !sym.isOverridingSymbol)
+      def isOuterCandidateForElision(sym: Symbol) = (sym.isOuterAccessor && isEffectivelyFinal && !sym.isOverridingSymbol)
 
-      val paramCandidatesForElision: Set[ /*Field*/  Symbol] = (clazz.info.decls.toSet filter isParamCandidateForElision)
-      val outerCandidatesForElision: Set[ /*Method*/ Symbol] = (clazz.info.decls.toSet filter isOuterCandidateForElision)
+      val decls = clazz.info.decls.toSet
+      val paramCandidatesForElision: Set[ /*Field*/  Symbol] = (decls filter isParamCandidateForElision)
+      val outerCandidatesForElision: Set[ /*Method*/ Symbol] = (decls filter isOuterCandidateForElision)
 
       omittables ++= paramCandidatesForElision
       omittables ++= outerCandidatesForElision

src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala

@@ -138,7 +138,7 @@ trait TreeAndTypeAnalysis extends Debugging {
 
           if(grouped) {
             def enumerateChildren(sym: Symbol) = {
-              sym.children.toList
+              sym.sealedChildren.toList
                 .sortBy(_.sealedSortName)
                 .filterNot(x => x.isSealed && x.isAbstractClass && !isPrimitiveValueClass(x))
             }

src/compiler/scala/tools/nsc/typechecker/Checkable.scala

@@ -212,8 +212,8 @@ trait Checkable {
     )
     /** Are all children of these symbols pairwise irreconcilable? */
     def allChildrenAreIrreconcilable(sym1: Symbol, sym2: Symbol) = (
-      sym1.children.toList forall (c1 =>
-        sym2.children.toList forall (c2 =>
+      sym1.sealedChildren.toList forall (c1 =>
+        sym2.sealedChildren.toList forall (c2 =>
           areIrreconcilableAsParents(c1, c2)
         )
       )

src/compiler/scala/tools/nsc/typechecker/Typers.scala

@@ -1694,6 +1694,8 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
               psym addChild context.owner
             else
               pending += ParentSealedInheritanceError(parent, psym)
+          if (psym.isLocalToBlock && !phase.erasedTypes)
+            psym addChild context.owner
           val parentTypeOfThis = parent.tpe.dealias.typeOfThis
 
           if (!(selfType <:< parentTypeOfThis) &&

src/reflect/scala/reflect/internal/Symbols.scala

@@ -980,7 +980,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     private def isNotOverridden = (
       owner.isClass && (
            owner.isEffectivelyFinal
-        || owner.isSealed && owner.children.forall(c => c.isEffectivelyFinal && (overridingSymbol(c) == NoSymbol))
+        || (owner.isSealed && owner.sealedChildren.forall(c => c.isEffectivelyFinal && (overridingSymbol(c) == NoSymbol)))
       )
     )
 
@@ -992,6 +992,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
              isPrivate
           || isLocalToBlock
          )
+      || isClass && originalOwner.isTerm && children.isEmpty // we track known subclasses of term-owned classes, use that infer finality
     )
     /** Is this symbol effectively final or a concrete term member of sealed class whose children do not override it */
     final def isEffectivelyFinalOrNotOverridden: Boolean = isEffectivelyFinal || (isTerm && !isDeferred && isNotOverridden)
@@ -2495,14 +2496,15 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     def associatedFile: AbstractFile = enclosingTopLevelClass.associatedFile
     def associatedFile_=(f: AbstractFile) { abort("associatedFile_= inapplicable for " + this) }
 
-    /** If this is a sealed class, its known direct subclasses.
+    /** If this is a sealed or local class, its known direct subclasses.
      *  Otherwise, the empty set.
      */
     def children: Set[Symbol] = Set()
+    final def sealedChildren: Set[Symbol] = if (!isSealed) Set.empty else children
 
     /** Recursively assemble all children of this symbol.
      */
-    def sealedDescendants: Set[Symbol] = children.flatMap(_.sealedDescendants) + this
+    final def sealedDescendants: Set[Symbol] = if (!isSealed) Set(this) else children.flatMap(_.sealedDescendants) + this
 
     @inline final def orElse(alt: => Symbol): Symbol = if (this ne NoSymbol) this else alt
     @inline final def andAlso(f: Symbol => Unit): Symbol = { if (this ne NoSymbol) f(this) ; this }

test/files/run/t9408.scala

@@ -0,0 +1,61 @@
+class Outer {
+  def assertNoFields(c: Class[_]) {
+    assert(c.getDeclaredFields.isEmpty)
+  }
+  def assertHasOuter(c: Class[_]) {
+    assert(c.getDeclaredFields.exists(_.getName.contains("outer")))
+  }
+  class Member
+  final class FinalMember
+
+  def test {
+    assertHasOuter(classOf[Member])
+    assertNoFields(classOf[FinalMember])
+    final class C
+    assertNoFields(classOf[C])
+    class D
+    assertNoFields(classOf[D])
+    (() => {class E; assertNoFields(classOf[E])}).apply()
+
+    // The outer reference elision currently runs on a class-by-class basis. If it cannot rule out that a class has
+    // subclasses, it will not remove the outer reference. A smarter analysis here could detect if no members of
+    // a sealed (or effectively sealed) hierarchy use the outer reference, the optimization could be performed.
+    class Parent
+    class Child extends Parent
+    assertHasOuter(classOf[Parent])
+
+    // Note: outer references (if they haven't been elided) are used in pattern matching as follows.
+    // This isn't relevant to term-owned classes, as you can't refer to them with a prefix that includes
+    // the outer class.
+    val outer1 = new Outer
+    val outer2 = new Outer
+    (new outer1.Member: Any) match {
+      case _: outer2.Member => sys.error("wrong match!")
+      case _: outer1.Member => // okay
+    }
+
+    // ... continuing on that theme, note that `Member` isn't considered as a local class, it is owned by a the class
+    // `LocalOuter`, which itself happens to be term-owned. So we expect that it has an outer reference, and that this
+    // is respected in type tests.
+    class LocalOuter {
+      class Member
+      final class FinalMember
+    }
+    assertNoFields(classOf[LocalOuter])
+    assertHasOuter(classOf[LocalOuter#Member])
+    val localOuter1 = new LocalOuter
+    val localOuter2 = new LocalOuter
+    (new localOuter1.Member: Any) match {
+      case _: localOuter2.Member => sys.error("wrong match!")
+      case _: localOuter1.Member => // okay
+    }
+    // Final member classes still lose the outer reference.
+    assertNoFields(classOf[LocalOuter#FinalMember])
+  }
+}
+
+object Test {
+  def main(args: Array[String]): Unit = {
+    new Outer().test
+  }
+}