From 93bee55e68522a46e501229dd0d5f2af4b72ac4a Mon Sep 17 00:00:00 2001
From: Jason Zaugg <jzaugg@gmail.com>
Date: Thu, 23 Jul 2015 14:30:21 +1000
Subject: SI-9408 Avoid capturing outer class in local classes.

Previously, only local classes declared final would be candidates
for outer pointer elision in the constructor phase.

This commit infers finality of local classes to expand the scope
of this optimization.

== Background ==

This was brought to our attention when shapeless enabled
indylambda and found that a hitherto serializable
data structure started to capture the enclosing class and hence
lost its serializability.

    class NotSerializable {
      def test = () => {
        class C; assertSerializable(new C)
      }
    }

Under `-Ydelambdafy:inline`, it used to capture the enclosing anon
function class as its outer, which, being final, didn't in turn
capture the enclosing class.

    class NotSerializable {
      def test = new anonFun$1
    }
    class anonFun$1 {
      def apply = assertSerializable(new C(this))
    }
    class ...$C(outer$: anonFun)

indylambda perturbs the enclosing structure of the function body.

    class NotSerializable {
      def anonFun$1 = {class C; assertSerializable(new C()))
      def test = lambdaMetaFactory(<<anonFun$1>>)
    }

Which leads to:

    class NotSerializable$C(outer$: NotSerializable)
---
 .../scala/tools/nsc/transform/Constructors.scala   |  6 +++
 src/reflect/scala/reflect/internal/Symbols.scala   |  1 +
 test/files/run/t9408.scala                         | 61 ++++++++++++++++++++++
 3 files changed, 68 insertions(+)
 create mode 100644 test/files/run/t9408.scala

diff --git a/src/compiler/scala/tools/nsc/transform/Constructors.scala b/src/compiler/scala/tools/nsc/transform/Constructors.scala
index 03e39ee329..7c66bda46b 100644
--- a/src/compiler/scala/tools/nsc/transform/Constructors.scala
+++ b/src/compiler/scala/tools/nsc/transform/Constructors.scala
@@ -165,6 +165,12 @@ 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 && isEffectivelyFinal && !sym.isOverridingSymbol)
diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala
index 15ab97dc3f..ff26ce26ac 100644
--- a/src/reflect/scala/reflect/internal/Symbols.scala
+++ b/src/reflect/scala/reflect/internal/Symbols.scala
@@ -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)
diff --git a/test/files/run/t9408.scala b/test/files/run/t9408.scala
new file mode 100644
index 0000000000..231dca4ce7
--- /dev/null
+++ b/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
+  }
+}
-- 
cgit v1.2.3