SI-9390 Emit local defs that don't capture this as static

This avoids unnecessary memory retention, and allows lambdas
that call the local methods to be serializable, regardless of
whether or not the enclosing class is serializable.

The second point is especially pressing, given that the enclosing
class for local methods defined in a used to be the (serializable)
anonymous function class, but as of Scala 2.12 will be the enclosing
class of the lambda.

This change is similar in spirit to SI-9408 / 93bee55.

Author: retronym

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

@@ -225,6 +225,7 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
       }
     }
 
+
     private def transformFunction(originalFunction: Function): Tree = {
       val target = targetMethod(originalFunction)
       assert(target.hasFlag(Flags.STATIC))
@@ -272,6 +273,12 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
           val Template(parents, self, body) = super.transform(deriveTemplate(tree)(_.mapConserve(pretransform)))
           Template(parents, self, body ++ boxingBridgeMethods)
         } finally boxingBridgeMethods.clear()
+      case dd: DefDef if dd.symbol.isLiftedMethod && !dd.symbol.isDelambdafyTarget =>
+        // SI-9390 emit lifted methods that don't require a `this` reference as STATIC
+        // delambdafy targets are excluded as they are made static by `transformFunction`.
+        if (!dd.symbol.hasFlag(STATIC) && !methodReferencesThis(dd.symbol))
+          dd.symbol.setFlag(STATIC)
+        super.transform(tree)
       case _ => super.transform(tree)
     }
   } // DelambdafyTransformer
@@ -326,19 +333,28 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
 
     // recursively find methods that refer to 'this' directly or indirectly via references to other methods
     // for each method found add it to the referrers set
-    private def refersToThis(symbol: Symbol): Boolean =
-      (thisReferringMethods contains symbol) ||
-        (liftedMethodReferences(symbol) exists refersToThis) && {
-          // add it early to memoize
-          debuglog(s"$symbol indirectly refers to 'this'")
-          thisReferringMethods += symbol
-          true
+    private def refersToThis(symbol: Symbol): Boolean = {
+      var seen = mutable.Set[Symbol]()
+      def loop(symbol: Symbol): Boolean = {
+        if (seen(symbol)) false
+        else {
+          seen += symbol
+          (thisReferringMethods contains symbol) ||
+            (liftedMethodReferences(symbol) exists loop) && {
+              // add it early to memoize
+              debuglog(s"$symbol indirectly refers to 'this'")
+              thisReferringMethods += symbol
+              true
+            }
         }
+      }
+      loop(symbol)
+    }
 
     private var currentMethod: Symbol = NoSymbol
 
     override def traverse(tree: Tree) = tree match {
-      case DefDef(_, _, _, _, _, _) if tree.symbol.isDelambdafyTarget =>
+      case DefDef(_, _, _, _, _, _) if tree.symbol.isDelambdafyTarget || tree.symbol.isLiftedMethod =>
         // we don't expect defs within defs. At this phase trees should be very flat
         if (currentMethod.exists) devWarning("Found a def within a def at a phase where defs are expected to be flattened out.")
         currentMethod = tree.symbol
@@ -349,6 +365,9 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
         // They'll be of the form {(args...) => this.anonfun(args...)}
         // but we do need to make note of the lifted body method in case it refers to 'this'
         if (currentMethod.exists) liftedMethodReferences(currentMethod) += targetMethod(fun)
+      case Apply(sel @ Select(This(_), _), args) if sel.symbol.isLiftedMethod =>
+        if (currentMethod.exists) liftedMethodReferences(currentMethod) += sel.symbol
+        super.traverseTrees(args)
       case This(_) =>
         if (currentMethod.exists && tree.symbol == currentMethod.enclClass) {
           debuglog(s"$currentMethod directly refers to 'this'")

test/files/run/t5652.check

@@ -1,7 +1,7 @@
-public default int T1.T1$$g$1()
 public default int T1.f0()
 public default void T1.$init$()
-public final int A1.A1$$g$2()
+public static int T1.T1$$g$1()
 public int A1.f1()
-public final int A2.A2$$g$1()
+public static final int A1.A1$$g$2()
 public int A2.f2()
+public static final int A2.A2$$g$1()

test/files/run/t5652b.check

@@ -1,4 +1,4 @@
-private final int A1.g$1()
+private static final int A1.g$1()
 public int A1.f1()
-private final int A2.g$1()
+private static final int A2.g$1()
 public int A2.f2()

test/files/run/t5652c.check

@@ -1,6 +1,6 @@
-public final int A1.A1$$g$1()
-public final int A1.A1$$g$2()
 public int A1.f1()
 public int A1.f2()
+public static final int A1.A1$$g$1()
+public static final int A1.A1$$g$2()
 1
 2

test/files/run/t9390.scala

@@ -0,0 +1,67 @@
+class C {
+  def methodLift1 = {
+    def isEven(c: Int) = c % 2 == 0
+    val f: Int => Boolean = isEven
+    f
+  }
+  def methodLift2 = {
+    def isEven(c: Int) = c % 2 == 0
+    def isEven0(c: Int) = isEven(c)
+    val f: Int => Boolean = isEven0
+    f
+  }
+
+  def methodLift3 = {
+    def isEven(c: Int) = {toString; c % 2 == 0}
+    def isEven0(c: Int) = isEven(c)
+    val f: Int => Boolean = isEven0
+    f
+  }
+}
+
+object Test {
+  def main(args: Array[String]): Unit = {
+    val c = new C
+
+    {
+      val f = c.methodLift1
+      assert(f(0))
+      assert(!f(1))
+      val f1 = serializeDeserialize(f)
+      assert(f1(0))
+      assert(!f1(1))
+    }
+
+
+    {
+      val f = c.methodLift2
+      assert(f(0))
+      assert(!f(1))
+      val f1 = serializeDeserialize(f)
+      assert(f1(0))
+      assert(!f1(1))
+    }
+
+    {
+      val f = c.methodLift3
+      assert(f(0))
+      assert(!f(1))
+      try {
+        serializeDeserialize(this)
+        assert(false)
+      } catch {
+        case _: java.io.NotSerializableException =>
+          // expected, the closure in methodLift3 must capture C which is not serializable
+      }
+    }
+  }
+
+  def serializeDeserialize[T <: AnyRef](obj: T): T = {
+    import java.io._
+    val buffer = new ByteArrayOutputStream
+    val out = new ObjectOutputStream(buffer)
+    out.writeObject(obj)
+    val in = new ObjectInputStream(new ByteArrayInputStream(buffer.toByteArray))
+    in.readObject.asInstanceOf[T]
+  }
+}

test/files/run/t9390b.scala

@@ -0,0 +1,31 @@
+class C { // C is not serializable
+  def foo = (x: Int) => (y: Int) => x + y
+  def bar = (x: Int) => (y: Int) => {toString; x + y}
+}
+
+object Test {
+  def main(args: Array[String]): Unit = {
+    val c = new C
+    val f = c.foo
+    assert(f(1)(2) == 3)
+    val f1 = serializeDeserialize(f)
+    assert(f1(1)(2) == 3)
+
+    try {
+      serializeDeserialize(c.bar)
+      assert(false)
+    } catch {
+      case _: java.io.NotSerializableException =>
+        // expected, lambda transitively refers to this
+    }
+  }
+
+  def serializeDeserialize[T <: AnyRef](obj: T): T = {
+    import java.io._
+    val buffer = new ByteArrayOutputStream
+    val out = new ObjectOutputStream(buffer)
+    out.writeObject(obj)
+    val in = new ObjectInputStream(new ByteArrayInputStream(buffer.toByteArray))
+    in.readObject.asInstanceOf[T]
+  }
+}

test/junit/scala/tools/nsc/backend/jvm/opt/InlineWarningTest.scala

@@ -113,7 +113,7 @@ class InlineWarningTest extends BytecodeTesting {
 
     val warn =
       """M::f()I is annotated @inline but could not be inlined:
-        |The callee M::f()I contains the instruction INVOKESPECIAL M.nested$1 ()I
+        |The callee M::f()I contains the instruction INVOKESTATIC M.nested$1 ()I
         |that would cause an IllegalAccessError when inlined into class N""".stripMargin
 
     var c = 0
@@ -140,7 +140,7 @@ class InlineWarningTest extends BytecodeTesting {
 
     val warn =
       """M::f(Lscala/Function1;)I could not be inlined:
-        |The callee M::f(Lscala/Function1;)I contains the instruction INVOKESPECIAL M.nested$1 ()I
+        |The callee M::f(Lscala/Function1;)I contains the instruction INVOKESTATIC M.nested$1 ()I
         |that would cause an IllegalAccessError when inlined into class N""".stripMargin
 
     var c = 0

test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala

@@ -1141,7 +1141,7 @@ class InlinerTest extends BytecodeTesting {
 
     val warn =
       """C::h()I is annotated @inline but could not be inlined:
-        |The callee C::h()I contains the instruction INVOKESPECIAL C.f$1 ()I
+        |The callee C::h()I contains the instruction INVOKESTATIC C.f$1 ()I
         |that would cause an IllegalAccessError when inlined into class D.""".stripMargin
 
     val List(c, d) = compile(code, allowMessage = _.msg contains warn)