Make lambda body public rather than using static accessor

Under -Ydelambdafy:method (the basis of the upcoming "indylambda"
translation for -target:jvm-1.8), an anonymous function is currently
encoded as:

 1. a private method containing the lambda's code
 2. a public, static accessor method that allows access to 1 from
    other classes, namely:
 3. an anonymous class capturing free variables and implementing
    the suitable FunctionN interface.

In our prototypes of indylambda, we do away with 3, instead deferring
creating of this class to JDK8's LambdaMetafactory by way of an
invokedynamic instruction at the point of lambda capture. This
facility can use a private method as the lambda target; access is
mediated by the runtime-provided instance of
`java.lang.invoke.MethodHandles.Lookup` that confers the privelages
of the lambda capture call site to the generated implementation class.
Indeed, The java compiler uses this to emit private lambda body
methods.

However, there are two Scala specific factors that make this
a little troublesome.

First, Scala's optimizer may want to inline the lambda capture
call site into a new enclosing class. In general, this isn't a
safe optimization, as `invokedynamic` instructions have call-site
specific semantics. But we will rely the ability to inline like
this in order to eliminate closures altogether where possible.

Second, to support lambda deserialization, the Java compiler creates
a synthetic method `$dersializeLamda$` in each class that captures
them, just to be able to get the suitable access to spin up an
anoymous class with access to the private method. It only needs
to do this for functional interfaces that extends Serializable,
which is the exception rather than the rule. But in Scala, *every*
function must support serialization, so blindly copying the Java
approach will lead to some code bloat.

I have prototyped a hybrid approach to these challenges: use the
private method directly where it is allowed, but fall back to
using the accessor method in a generic lambda deserializer or
in call sites that have been inlined into a new enclosing class.

However, the most straight forward approach is to simply emit the
lambda bodies as public (with an mangled name and with the SYHTNETIC
flag) and use them in all cases. That is what is done in this commit.

This does moves us one step backwards from the goals of SI-7085,
but it doesn't seem sensible to incur the inconvenience from locking
down one small part of our house when we don't have a plan or the
budget to complete that job.

The REPL has some fancy logic to decompile the bodys of lambdas
(`:javap -fun C#m`) which needed tweaking to accomodate this change.
I haven't tried to make this backwards compatible with the old
encoding as `-Ydelambdafy:method` is still experimental.

3 files changed, 49 insertions, 102 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/Delambdafy.scala b/src/compiler/scala/tools/nsc/transform/Delambdafy.scala
index 94e88589f5..2d33b35241 100644
--- a/src/compiler/scala/tools/nsc/transform/Delambdafy.scala
+++ b/src/compiler/scala/tools/nsc/transform/Delambdafy.scala
@@ -15,13 +15,12 @@ import scala.collection.mutable.LinkedHashMap
  * Currently Uncurry is responsible for that transformation.
  *
  * From a lambda, Delambdafy will create
- * 1) a static forwarder at the top level of the class that contained the lambda
- * 2) a new top level class that
+ * 1) a new top level class that
       a) has fields and a constructor taking the captured environment (including possibly the "this"
  *       reference)
- *    b) an apply method that calls the static forwarder
+ *    b) an apply method that calls the target method
  *    c) if needed a bridge method for the apply method
- *  3) an instantiation of the newly created class which replaces the lambda
+ * 2) an instantiation of the newly created class which replaces the lambda
  *
  *  TODO the main work left to be done is to plug into specialization. Primarily that means choosing a
  * specialized FunctionN trait instead of the generic FunctionN trait as a parent and creating the
@@ -76,36 +75,25 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
       referrers
     }
 
-    val accessorMethods = mutable.ArrayBuffer[Tree]()
-
-    // the result of the transformFunction method. A class definition for the lambda, an expression
-    // insantiating the lambda class, and an accessor method for the lambda class to be able to
-    // call the implementation
-    case class TransformedFunction(lambdaClassDef: ClassDef, newExpr: Tree, accessorMethod: Tree)
+    // the result of the transformFunction method.
+    sealed abstract class TransformedFunction
+    // A class definition for the lambda, an expression insantiating the lambda class
+    case class DelambdafyAnonClass(lambdaClassDef: ClassDef, newExpr: Tree) extends TransformedFunction
 
     // here's the main entry point of the transform
     override def transform(tree: Tree): Tree = tree match {
       // the main thing we care about is lambdas
       case fun @ Function(_, _) =>
-        // a lambda beccomes a new class, an instantiation expression, and an
-        // accessor method
-        val TransformedFunction(lambdaClassDef, newExpr, accessorMethod) = transformFunction(fun)
-        // we'll add accessor methods to the current template later
-        accessorMethods += accessorMethod
-        val pkg = lambdaClassDef.symbol.owner
-
-        // we'll add the lambda class to the package later
-        lambdaClassDefs(pkg) = lambdaClassDef :: lambdaClassDefs(pkg)
-
-        super.transform(newExpr)
-      // when we encounter a template (basically the thing that holds body of a class/trait)
-      // we need to updated it to include newly created accessor methods after transforming it
-      case Template(_, _, _) =>
-        try {
-          // during this call accessorMethods will be populated from the Function case
-          val Template(parents, self, body) = super.transform(tree)
-          Template(parents, self, body ++ accessorMethods)
-        } finally accessorMethods.clear()
+        transformFunction(fun) match {
+          case DelambdafyAnonClass(lambdaClassDef, newExpr) =>
+            // a lambda beccomes a new class, an instantiation expression
+            val pkg = lambdaClassDef.symbol.owner
+
+            // we'll add the lambda class to the package later
+            lambdaClassDefs(pkg) = lambdaClassDef :: lambdaClassDefs(pkg)
+
+            super.transform(newExpr)
+        }
       case _ => super.transform(tree)
     }
 
@@ -120,8 +108,7 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
 
     private def optionSymbol(sym: Symbol): Option[Symbol] = if (sym.exists) Some(sym) else None
 
-    // turns a lambda into a new class def, a New expression instantiating that class, and an
-    // accessor method fo the body of the lambda
+    // turns a lambda into a new class def, a New expression instantiating that class
     private def transformFunction(originalFunction: Function): TransformedFunction = {
       val functionTpe = originalFunction.tpe
       val targs = functionTpe.typeArgs
@@ -132,46 +119,16 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
       // passed into the constructor of the anonymous function class
       val captures = FreeVarTraverser.freeVarsOf(originalFunction)
 
-      /**
-       * Creates the apply method for the anonymous subclass of FunctionN
-       */
-      def createAccessorMethod(thisProxy: Symbol, fun: Function): DefDef = {
-        val target = targetMethod(fun)
-        if (!thisProxy.exists) {
-          target setFlag STATIC
-        }
-        val params = ((optionSymbol(thisProxy) map {proxy:Symbol => ValDef(proxy)}) ++ (target.paramss.flatten map ValDef.apply)).toList
-
-        val methSym = oldClass.newMethod(unit.freshTermName(nme.accessor.toString() + "$"), target.pos, FINAL | BRIDGE | SYNTHETIC | PROTECTED | STATIC)
+      val target = targetMethod(originalFunction)
+      target.makeNotPrivate(target.owner)
+      if (!thisReferringMethods.contains(target))
+        target setFlag STATIC
 
-        val paramSyms = params map {param => methSym.newSyntheticValueParam(param.symbol.tpe, param.name) }
-
-        params zip paramSyms foreach { case (valdef, sym) => valdef.symbol = sym }
-        params foreach (_.symbol.owner = methSym)
-
-        val methodType = MethodType(paramSyms, restpe)
-        methSym setInfo methodType
-
-        oldClass.info.decls enter methSym
-
-        val body = localTyper.typed {
-          val newTarget = Select(if (thisProxy.exists) gen.mkAttributedRef(paramSyms(0)) else gen.mkAttributedThis(oldClass), target)
-          val newParams = paramSyms drop (if (thisProxy.exists) 1 else 0) map Ident
-          Apply(newTarget, newParams)
-        } setPos fun.pos
-        val methDef = DefDef(methSym, List(params), body)
-
-        // Have to repack the type to avoid mismatches when existentials
-        // appear in the result - see SI-4869.
-        // TODO probably don't need packedType
-        methDef.tpt setType localTyper.packedType(body, methSym)
-        methDef
-      }
 
       /**
        * Creates the apply method for the anonymous subclass of FunctionN
        */
-      def createApplyMethod(newClass: Symbol, fun: Function, accessor: DefDef, thisProxy: Symbol): DefDef = {
+      def createApplyMethod(newClass: Symbol, fun: Function, thisProxy: Symbol): DefDef = {
         val methSym = newClass.newMethod(nme.apply, fun.pos, FINAL | SYNTHETIC)
         val params = fun.vparams map (_.duplicate)
 
@@ -187,8 +144,12 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
         newClass.info.decls enter methSym
 
         val Apply(_, oldParams) = fun.body
+        val qual = if (thisProxy.exists)
+          Select(gen.mkAttributedThis(newClass), thisProxy)
+        else
+          gen.mkAttributedThis(oldClass) // sort of a lie, EmptyTree.<static method> would be more honest, but the backend chokes on that.
 
-        val body = localTyper typed Apply(Select(gen.mkAttributedThis(oldClass), accessor.symbol), (optionSymbol(thisProxy) map {tp => Select(gen.mkAttributedThis(newClass), tp)}).toList ++ oldParams)
+        val body = localTyper typed Apply(Select(qual, target), oldParams)
         body.substituteSymbols(fun.vparams map (_.symbol), params map (_.symbol))
         body changeOwner (fun.symbol -> methSym)
 
@@ -271,18 +232,16 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
         // the Optional proxy that will hold a reference to the 'this'
         // object used by the lambda, if any. NoSymbol if there is no this proxy
         val thisProxy = {
-          val target = targetMethod(originalFunction)
-          if (thisReferringMethods contains target) {
+          if (target.hasFlag(STATIC))
+            NoSymbol
+          else {
             val sym = lambdaClass.newVariable(nme.FAKE_LOCAL_THIS, originalFunction.pos, SYNTHETIC)
-            sym.info = oldClass.tpe
-            sym
-          } else NoSymbol
+            sym.setInfo(oldClass.tpe)
+          }
         }
 
         val decapturify = new DeCapturifyTransformer(captureProxies2, unit, oldClass, lambdaClass, originalFunction.symbol.pos, thisProxy)
 
-        val accessorMethod = createAccessorMethod(thisProxy, originalFunction)
-
         val decapturedFunction = decapturify.transform(originalFunction).asInstanceOf[Function]
 
         val members = (optionSymbol(thisProxy).toList ++ (captureProxies2 map (_._2))) map {member =>
@@ -294,7 +253,7 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
         val constr = createConstructor(lambdaClass, members)
 
         // apply method with same arguments and return type as original lambda.
-        val applyMethodDef = createApplyMethod(lambdaClass, decapturedFunction, accessorMethod, thisProxy)
+        val applyMethodDef = createApplyMethod(lambdaClass, decapturedFunction, thisProxy)
 
         val bridgeMethod = createBridgeMethod(lambdaClass, originalFunction, applyMethodDef)
 
@@ -312,10 +271,10 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
         val body = members ++ List(constr, applyMethodDef) ++ bridgeMethod
 
         // TODO if member fields are private this complains that they're not accessible
-        (localTyper.typedPos(decapturedFunction.pos)(ClassDef(lambdaClass, body)).asInstanceOf[ClassDef], thisProxy, accessorMethod)
+        (localTyper.typedPos(decapturedFunction.pos)(ClassDef(lambdaClass, body)).asInstanceOf[ClassDef], thisProxy)
       }
 
-      val (anonymousClassDef, thisProxy, accessorMethod) = makeAnonymousClass
+      val (anonymousClassDef, thisProxy) = makeAnonymousClass
 
       pkg.info.decls enter anonymousClassDef.symbol
 
@@ -327,7 +286,7 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
 
       val typedNewStat = localTyper.typedPos(originalFunction.pos)(newStat)
 
-      TransformedFunction(anonymousClassDef, typedNewStat, accessorMethod)
+      DelambdafyAnonClass(anonymousClassDef, typedNewStat)
     }
 
     /**
diff --git a/src/repl/scala/tools/nsc/interpreter/JavapClass.scala b/src/repl/scala/tools/nsc/interpreter/JavapClass.scala
index c80b94bf89..1ccade2172 100644
--- a/src/repl/scala/tools/nsc/interpreter/JavapClass.scala
+++ b/src/repl/scala/tools/nsc/interpreter/JavapClass.scala
@@ -8,6 +8,7 @@ package tools.nsc
 package interpreter
 
 import java.lang.{ ClassLoader => JavaClassLoader, Iterable => JIterable }
+import scala.tools.asm.Opcodes
 import scala.tools.nsc.util.ScalaClassLoader
 import java.io.{ ByteArrayInputStream, CharArrayWriter, FileNotFoundException, PrintWriter, StringWriter, Writer }
 import java.util.{ Locale }
@@ -758,32 +759,19 @@ object JavapClass {
       import scala.tools.asm.ClassReader
       import scala.tools.asm.Opcodes.INVOKESTATIC
       import scala.tools.asm.tree.{ ClassNode, MethodInsnNode }
-      // the accessor methods invoked statically by the apply of the given closure class
-      def accesses(s: String): Seq[(String, String)] = {
-        val accessor = """accessor\$\d+""".r
+      def callees(s: String): List[(String, String)] = {
         loader classReader s withMethods { ms =>
-          ms filter (_.name == "apply") flatMap (_.instructions.toArray.collect {
-            case i: MethodInsnNode if i.getOpcode == INVOKESTATIC && when(i.name) { case accessor(_*) => true } => (i.owner, i.name)
-          })
+          val nonBridgeApplyMethods = ms filter (_.name == "apply") filter (n => (n.access & Opcodes.ACC_BRIDGE) == 0)
+          val instructions = nonBridgeApplyMethods flatMap (_.instructions.toArray)
+          instructions.collect {
+            case i: MethodInsnNode => (i.owner, i.name)
+          }.toList
         }
       }
-      // get the k.$anonfun for the accessor k.m
-      def anonOf(k: String, m: String): String = {
-        val res = 
-          loader classReader k withMethods { ms =>
-            ms filter (_.name == m) flatMap (_.instructions.toArray.collect {
-              case i: MethodInsnNode if i.getOpcode == INVOKESTATIC && i.name.startsWith("$anonfun") => i.name
-            })
-          }
-        assert(res.size == 1)
-        res.head
-      }
-      // the lambdas invoke accessors that call the anonfuns of interest. Filter k on the k#$anonfuns.
-      val ack = accesses(lambda)
-      assert(ack.size == 1)  // There can be only one.
-      ack.head match {
-        case (k, _) if target.isModule && !(k endsWith "$") => None
-        case (k, m)                                         => Some(s"${k}#${anonOf(k, m)}")
+      callees(lambda) match {
+        case (k, _) :: Nil if target.isModule && !(k endsWith "$") => None
+        case (k, m) :: _ => Some(s"${k}#${m}")
+        case _ => None
       }
     }
     /** Translate the supplied targets to patterns for anonfuns.
diff --git a/test/files/run/repl-javap-lambdas.scala b/test/files/run/repl-javap-lambdas.scala
index c503c99d66..76a6ec8450 100644
--- a/test/files/run/repl-javap-lambdas.scala
+++ b/test/files/run/repl-javap-lambdas.scala
@@ -16,7 +16,7 @@ object Test extends JavapTest {
   // three anonfuns of Betty#g
   override def yah(res: Seq[String]) = {
     import PartialFunction.{ cond => when }
-    val r = """\s*private static final .* \$anonfun\$\d+\(.*""".r
+    val r = """.*final .* .*\$anonfun\$\d+\(.*""".r
     def filtered = res filter (when(_) { case r(_*) => true })
     3 == filtered.size
   }