Lambda impl methods static and more stably named

The body of lambdas is compiled into a synthetic method
in the enclosing class. Previously, this method was a public
virtual method named `fully$qualified$Class$$anonfun$n`.
For lambdas that didn't capture a `this` reference, a static
method was used.

This commit changes two aspects.

Firstly, all lambda impl methods are not emitted static.
For those that require a this reference, an extra parameter
is added.

This is an improvement as it:

  - allows, shorter, more readable names for the lambda impl method
  - avoids pollution of the vtable of the class. Note that javac uses
    private instance methods, rather than public static methods. If
    we followed its lead, we would be unable to support important use
    cases in our inliner

Secondly, the name of the enclosing method has been included in
the name of the lambda impl method to improve debuggability and
to improve serialization compatibility. The serialization improvement
comes from the way that fresh names for the impl methods are
allocated: adding or removing lambdas in methods not named "foo" won't
change the numbering of the `anonfun$foo$n` impl methods from methods
named "foo". This is in line with user expectations about anonymous
class and lambda serialization stability. Brian Goetz has described
this tricky area will in:

  http://cr.openjdk.java.net/~briangoetz/eg-attachments/lambda-serialization.html

This commit doesn't go as far a Javac, we don't use the hash of the
lambda type info, param names, etc to map to a lambda impl method name.
As such, we are more prone to the type-1 and -2 failures described there.
However, our Scala 2.11.8 has similar characteristics, so we aren't going
backwards.

Special case in the naming: Use "new" rather than "<init>" for constructor enclosed
lambdas, as javac does.

Author: retronym

src/compiler/scala/tools/nsc/ast/TreeGen.scala

@@ -274,8 +274,19 @@ abstract class TreeGen extends scala.reflect.internal.TreeGen with TreeDSL {
   }
 
   // used to create the lifted method that holds a function's body
-  def mkLiftedFunctionBodyMethod(localTyper: analyzer.Typer)(owner: Symbol, fun: Function) =
-    mkMethodForFunctionBody(localTyper)(owner, fun, nme.ANON_FUN_NAME)(additionalFlags = ARTIFACT)
+  def mkLiftedFunctionBodyMethod(localTyper: analyzer.Typer)(owner: Symbol, fun: Function) = {
+    def nonLocalEnclosingMember(sym: Symbol) = {
+      if (sym.isLocalDummy) sym.enclClass.primaryConstructor else {
+        if (sym.isLocalToBlock) sym.originalOwner else sym
+      }
+    }
+    val ownerName = nonLocalEnclosingMember(fun.symbol.originalOwner).name match {
+      case nme.CONSTRUCTOR => nme.NEWkw // do as javac does for the suffix, prefer "new" to "$lessinit$greater$1"
+      case x => x
+    }
+    val newName = nme.ANON_FUN_NAME.append(nme.NAME_JOIN_STRING).append(ownerName)
+    mkMethodForFunctionBody(localTyper)(owner, fun, newName)(additionalFlags = ARTIFACT)
+  }
 
 
   /**

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

@@ -61,14 +61,18 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
 
     private def mkLambdaMetaFactoryCall(fun: Function, target: Symbol, functionalInterface: Symbol, samUserDefined: Symbol, isSpecialized: Boolean): Tree = {
       val pos = fun.pos
+      def isSelfParam(p: Symbol) = p.isSynthetic && p.name == nme.SELF
+      val hasSelfParam = isSelfParam(target.firstParam)
+
       val allCapturedArgRefs = {
         // find which variables are free in the lambda because those are captures that need to be
         // passed into the constructor of the anonymous function class
         val captureArgs = FreeVarTraverser.freeVarsOf(fun).iterator.map(capture =>
           gen.mkAttributedRef(capture) setPos pos
         ).toList
 
-        if (target hasFlag STATIC) captureArgs  // no `this` reference needed
+        if (!hasSelfParam) captureArgs.filterNot(arg => isSelfParam(arg.symbol))
+        else if (currentMethod.hasFlag(Flags.STATIC)) captureArgs
         else (gen.mkAttributedThis(fun.symbol.enclClass) setPos pos) :: captureArgs
       }
 
@@ -179,7 +183,7 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
         val numCaptures  = targetParams.length - functionParamTypes.length
         val (targetCapturedParams, targetFunctionParams) = targetParams.splitAt(numCaptures)
 
-        val methSym = oldClass.newMethod(target.name.append("$adapted").toTermName, target.pos, target.flags | FINAL | ARTIFACT)
+        val methSym = oldClass.newMethod(target.name.append("$adapted").toTermName, target.pos, target.flags | FINAL | ARTIFACT | STATIC)
         val bridgeCapturedParams = targetCapturedParams.map(param => methSym.newSyntheticValueParam(param.tpe, param.name.toTermName))
         val bridgeFunctionParams =
           map2(targetFunctionParams, bridgeParamTypes)((param, tp) => methSym.newSyntheticValueParam(tp, param.name.toTermName))
@@ -223,10 +227,8 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
 
     private def transformFunction(originalFunction: Function): Tree = {
       val target = targetMethod(originalFunction)
-      target.makeNotPrivate(target.owner)
-
-      // must be done before calling createBoxingBridgeMethod and mkLambdaMetaFactoryCall
-      if (!(target hasFlag STATIC) && !methodReferencesThis(target)) target setFlag STATIC
+      assert(target.hasFlag(Flags.STATIC))
+      target.setFlag(notPRIVATE)
 
       val funSym = originalFunction.tpe.typeSymbolDirect
       // The functional interface that can be used to adapt the lambda target method `target` to the given function type.
@@ -252,11 +254,30 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
     // 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 => super.transform(transformFunction(fun))
+      case fun: Function =>
+        super.transform(transformFunction(fun))
       case Template(_, _, _) =>
+        def pretransform(tree: Tree): Tree = tree match {
+          case dd: DefDef if dd.symbol.isDelambdafyTarget =>
+            if (!dd.symbol.hasFlag(Flags.STATIC) && methodReferencesThis(dd.symbol)) {
+                dd.symbol.setFlag(Flags.STATIC)
+                val selfParamSym = dd.symbol.newSyntheticValueParam(dd.symbol.owner.typeConstructor, nme.SELF)
+                dd.symbol.updateInfo(dd.symbol.info match {
+                  case mt @ MethodType(params, res) => copyMethodType(mt, selfParamSym :: params, res)
+                })
+                val selfParam = ValDef(selfParamSym)
+                val rhs = dd.rhs.substituteThis(dd.symbol.owner, atPos(dd.symbol.pos)(gen.mkAttributedIdent(selfParamSym)))
+                val result = treeCopy.DefDef(dd, dd.mods, dd.name, dd.tparams, (selfParam :: dd.vparamss.head) :: Nil, dd.tpt, rhs)
+                result
+            } else {
+              dd.symbol setFlag STATIC
+              dd
+            }
+          case t => t
+        }
         try {
           // during this call boxingBridgeMethods will be populated from the Function case
-          val Template(parents, self, body) = super.transform(tree)
+          val Template(parents, self, body) = super.transform(deriveTemplate(tree)(_.mapConserve(pretransform)))
           Template(parents, self, body ++ boxingBridgeMethods)
         } finally boxingBridgeMethods.clear()
       case _ => super.transform(tree)

test/files/run/delambdafy_t6028.check

@@ -11,7 +11,7 @@ package <empty> {
     def foo(methodParam: String): Function0 = {
       val methodLocal: String = "";
       {
-        (() => T.this.$anonfun$1(methodParam, methodLocal))
+        (() => T.this.$anonfun$foo$1(methodParam, methodLocal))
       }
     };
     def bar(barParam: String): Object = {
@@ -21,10 +21,10 @@ package <empty> {
     def tryy(tryyParam: String): Function0 = {
       var tryyLocal: runtime.ObjectRef = scala.runtime.ObjectRef.create("");
       {
-        (() => T.this.$anonfun$2(tryyParam, tryyLocal))
+        (() => T.this.$anonfun$tryy$1(tryyParam, tryyLocal))
       }
     };
-    final <artifact> private[this] def $anonfun$1(methodParam$1: String, methodLocal$1: String): String = T.this.classParam.+(T.this.field()).+(methodParam$1).+(methodLocal$1);
+    final <artifact> private[this] def $anonfun$foo$1(methodParam$1: String, methodLocal$1: String): String = T.this.classParam.+(T.this.field()).+(methodParam$1).+(methodLocal$1);
     abstract trait MethodLocalTrait$1 extends Object {
       def /*MethodLocalTrait$1*/$init$(barParam$1: String): Unit = {
         ()
@@ -54,7 +54,7 @@ package <empty> {
       T.this.MethodLocalObject$lzycompute$1(barParam$1, MethodLocalObject$module$1)
     else
       MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]();
-    final <artifact> private[this] def $anonfun$2(tryyParam$1: String, tryyLocal$1: runtime.ObjectRef): Unit = try {
+    final <artifact> private[this] def $anonfun$tryy$1(tryyParam$1: String, tryyLocal$1: runtime.ObjectRef): Unit = try {
       tryyLocal$1.elem = tryyParam$1
     } finally ()
   }

test/files/run/delambdafy_t6555.check

@@ -6,8 +6,8 @@ package <empty> {
       ()
     };
     private[this] val f: String => String = {
-      final <artifact> def $anonfun(param: String): String = param;
-      ((param: String) => $anonfun(param))
+      final <artifact> def $anonfun$f(param: String): String = param;
+      ((param: String) => $anonfun$f(param))
     };
     <stable> <accessor> def f(): String => String = Foo.this.f
   }

test/files/run/delambdafy_uncurry_byname_method.check

@@ -7,8 +7,8 @@ package <empty> {
     };
     def bar(x: () => String): String = x.apply();
     def foo(): String = Foo.this.bar({
-      final <artifact> def $anonfun(): String = "";
-      (() => $anonfun())
+      final <artifact> def $anonfun$foo(): String = "";
+      (() => $anonfun$foo())
     })
   }
 }

test/files/run/delambdafy_uncurry_method.check

@@ -7,8 +7,8 @@ package <empty> {
     };
     def bar(): Unit = {
       val f: Int => Int = {
-        final <artifact> def $anonfun(x: Int): Int = x.+(1);
-        ((x: Int) => $anonfun(x))
+        final <artifact> def $anonfun|(x: Int): Int = x.+(1);
+        ((x: Int) => $anonfun|(x))
       };
       ()
     }

test/files/run/mixin-signatures.check

@@ -60,6 +60,7 @@ interface Foo1 {
     generic: public abstract R Base.f(T)
   public default java.lang.String Foo1.f(java.lang.Object)
     generic: public default java.lang.String Foo1.f(T)
+  public static java.lang.String Foo1.f(Foo1,java.lang.Object) <synthetic>
   public abstract java.lang.Object Base.g(java.lang.Object)
     generic: public abstract R Base.g(T)
   public abstract java.lang.String Foo1.g(java.lang.Object)
@@ -73,6 +74,7 @@ interface Foo2 {
     generic: public abstract R Base.f(T)
   public default java.lang.Object Foo2.f(java.lang.String)
     generic: public default R Foo2.f(java.lang.String)
+  public static java.lang.Object Foo2.f(Foo2,java.lang.String) <synthetic>
   public abstract java.lang.Object Base.g(java.lang.Object)
     generic: public abstract R Base.g(T)
   public abstract java.lang.Object Foo2.g(java.lang.String)

test/files/run/t9097.scala

@@ -28,6 +28,6 @@ object Test extends StoreReporterDirectTest {
     assert(!storeReporter.hasErrors, message = filteredInfos map (_.msg) mkString "; ")
     val out = baos.toString("UTF-8")
     // was 2 before the fix, the two PackageDefs for a would both contain the ClassDef for the closure
-    assert(out.lines.count(_ contains "def $anonfun$1(x$1: Int): String") == 1, out)
+    assert(out.lines.count(_ contains "def $anonfun$hihi$1(x$1: Int): String") == 1, out)
   }
 }