Merge pull request #5157 from retronym/topic/lambda-statics

Lambda impl methods static and more stably named

9 files changed, 105 insertions, 23 deletions

diff --git a/src/compiler/scala/tools/nsc/ast/TreeGen.scala b/src/compiler/scala/tools/nsc/ast/TreeGen.scala
index 0786ceb7c2..14ee7d7a78 100644
--- a/src/compiler/scala/tools/nsc/ast/TreeGen.scala
+++ b/src/compiler/scala/tools/nsc/ast/TreeGen.scala
@@ -238,7 +238,8 @@ abstract class TreeGen extends scala.reflect.internal.TreeGen with TreeDSL {
    *  (outside the synchronized block).
    *
    *  The idiom works only if the condition is using a volatile field.
-   *  @see http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html
+    *
+    *  @see http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html
    */
   def mkSynchronizedCheck(clazz: Symbol, cond: Tree, syncBody: List[Tree], stats: List[Tree]): Tree =
     mkSynchronizedCheck(mkAttributedThis(clazz), cond, syncBody, stats)
@@ -274,8 +275,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: global.analyzer.Typer)(owner: global.Symbol, fun: global.Function) = {
+    def nonLocalEnclosingMember(sym: Symbol): Symbol = {
+      if (sym.isLocalDummy) sym.enclClass.primaryConstructor
+      else if (sym.isLocalToBlock) nonLocalEnclosingMember(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.dropLocal
+    }
+    val newName = nme.ANON_FUN_NAME.append(nme.NAME_JOIN_STRING).append(ownerName)
+    mkMethodForFunctionBody(localTyper)(owner, fun, newName)(additionalFlags = ARTIFACT)
+  }
 
 
   /**
@@ -310,6 +322,38 @@ abstract class TreeGen extends scala.reflect.internal.TreeGen with TreeDSL {
     newDefDef(methSym, moveToMethod(useMethodParams(fun.body)))(tpt = TypeTree(resTp))
   }
 
+  /**
+    * Create a new `DefDef` based on `orig` with an explicit self parameter.
+    *
+    * Details:
+    *   - Must by run after erasure
+    *   - If `maybeClone` is the identity function, this runs "in place"
+    *     and mutates the symbol of `orig`. `orig` should be discarded
+    *   - Symbol owners and returns are substituted, as are parameter symbols
+    *   - Recursive calls are not rewritten. This is correct if we assume
+    *     that we either:
+    *       - are in "in-place" mode, but can guarantee that no recursive calls exists
+    *       - are associating the RHS with a cloned symbol, but intend for the original
+    *         method to remain and for recursive calls to target it.
+    */
+  final def mkStatic(orig: DefDef, maybeClone: Symbol => Symbol): DefDef = {
+    assert(phase.erasedTypes, phase)
+    assert(!orig.symbol.hasFlag(SYNCHRONIZED), orig.symbol.defString)
+    val origSym = orig.symbol
+    val origParams = orig.symbol.info.params
+    val newSym = maybeClone(orig.symbol)
+    newSym.setFlag(STATIC)
+    // Add an explicit self parameter
+    val selfParamSym = newSym.newSyntheticValueParam(newSym.owner.typeConstructor, nme.SELF).setFlag(ARTIFACT)
+    newSym.updateInfo(newSym.info match {
+      case mt @ MethodType(params, res) => copyMethodType(mt, selfParamSym :: params, res)
+    })
+    val selfParam = ValDef(selfParamSym)
+    val rhs = orig.rhs.substituteThis(newSym.owner, atPos(newSym.pos)(gen.mkAttributedIdent(selfParamSym)))
+      .substituteSymbols(origParams, newSym.info.params.drop(1)).changeOwner(origSym -> newSym)
+    treeCopy.DefDef(orig, orig.mods, orig.name, orig.tparams, (selfParam :: orig.vparamss.head) :: Nil, orig.tpt, rhs).setSymbol(newSym)
+  }
+
   // TODO: the rewrite to AbstractFunction is superfluous once we compile FunctionN to a SAM type (aka functional interface)
   def functionClassType(fun: Function): Type =
     if (isFunctionType(fun.tpe)) abstractFunctionType(fun.vparams.map(_.symbol.tpe), fun.body.tpe.deconst)
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala b/src/compiler/scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala
index f94642389d..6d3c3f3863 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala
@@ -121,7 +121,7 @@ class BackendUtils[BT <: BTypes](val btypes: BT) {
 
   def getBoxedUnit: FieldInsnNode = new FieldInsnNode(GETSTATIC, srBoxedUnitRef.internalName, "UNIT", srBoxedUnitRef.descriptor)
 
-  private val anonfunAdaptedName = """.*\$anonfun\$\d+\$adapted""".r
+  private val anonfunAdaptedName = """.*\$anonfun\$.*\$\d+\$adapted""".r
   def hasAdaptedImplMethod(closureInit: ClosureInstantiation): Boolean = {
     isBuiltinFunctionType(Type.getReturnType(closureInit.lambdaMetaFactoryCall.indy.desc).getInternalName) &&
     anonfunAdaptedName.pattern.matcher(closureInit.lambdaMetaFactoryCall.implMethod.getName).matches
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/analysis/NullnessAnalyzer.scala b/src/compiler/scala/tools/nsc/backend/jvm/analysis/NullnessAnalyzer.scala
index 30e73f8ac2..01afd0d2ef 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/analysis/NullnessAnalyzer.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/analysis/NullnessAnalyzer.scala
@@ -5,8 +5,8 @@ package analysis
 import java.util
 
 import scala.annotation.switch
-import scala.tools.asm.{Type, Opcodes}
-import scala.tools.asm.tree.{MethodInsnNode, LdcInsnNode, AbstractInsnNode}
+import scala.tools.asm.{Opcodes, Type}
+import scala.tools.asm.tree.{AbstractInsnNode, LdcInsnNode, MethodInsnNode, MethodNode}
 import scala.tools.asm.tree.analysis._
 import scala.tools.nsc.backend.jvm.opt.BytecodeUtils
 import BytecodeUtils._
@@ -63,7 +63,7 @@ object NullnessValue {
   def unknown(insn: AbstractInsnNode) = if (BytecodeUtils.instructionResultSize(insn) == 2) UnknownValue2 else UnknownValue1
 }
 
-final class NullnessInterpreter(bTypes: BTypes) extends Interpreter[NullnessValue](Opcodes.ASM5) {
+final class NullnessInterpreter(bTypes: BTypes, method: MethodNode) extends Interpreter[NullnessValue](Opcodes.ASM5) {
   def newValue(tp: Type): NullnessValue = {
     // ASM loves giving semantics to null. The behavior here is the same as in SourceInterpreter,
     // which is provided by the framework.
@@ -80,7 +80,13 @@ final class NullnessInterpreter(bTypes: BTypes) extends Interpreter[NullnessValu
 
   override def newParameterValue(isInstanceMethod: Boolean, local: Int, tp: Type): NullnessValue = {
     // For instance methods, the `this` parameter is known to be not null.
-    if (isInstanceMethod && local == 0) NotNullValue
+    val isThis = local == 0 && (isInstanceMethod || {
+      method.parameters != null && !method.parameters.isEmpty && {
+        val p = method.parameters.get(0)
+        (p.access & Opcodes.ACC_SYNTHETIC) != 0 && p.name == "$this"
+      }
+    })
+    if (isThis) NotNullValue
     else super.newParameterValue(isInstanceMethod, local, tp)
   }
 
@@ -197,7 +203,7 @@ class NullnessFrame(nLocals: Int, nStack: Int) extends AliasingFrame[NullnessVal
  * This class is required to override the `newFrame` methods, which makes makes sure the analyzer
  * uses NullnessFrames.
  */
-class NullnessAnalyzer(bTypes: BTypes) extends Analyzer[NullnessValue](new NullnessInterpreter(bTypes)) {
+class NullnessAnalyzer(bTypes: BTypes, method: MethodNode) extends Analyzer[NullnessValue](new NullnessInterpreter(bTypes, method)) {
   override def newFrame(nLocals: Int, nStack: Int): NullnessFrame = new NullnessFrame(nLocals, nStack)
   override def newFrame(src: Frame[_ <: NullnessValue]): NullnessFrame = new NullnessFrame(src)
 }
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala
index e8d1bf203a..d4ff6493a3 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala
@@ -103,7 +103,7 @@ class CallGraph[BT <: BTypes](val btypes: BT) {
 
       val analyzer = {
         if (compilerSettings.optNullnessTracking && AsmAnalyzer.sizeOKForNullness(methodNode)) {
-          Some(new AsmAnalyzer(methodNode, definingClass.internalName, new NullnessAnalyzer(btypes)))
+          Some(new AsmAnalyzer(methodNode, definingClass.internalName, new NullnessAnalyzer(btypes, methodNode)))
         } else if (AsmAnalyzer.sizeOKForBasicValue(methodNode)) {
           Some(new AsmAnalyzer(methodNode, definingClass.internalName))
         } else None
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/LocalOpt.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/LocalOpt.scala
index 5ca0ad2773..447ee209b5 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/opt/LocalOpt.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/LocalOpt.scala
@@ -397,7 +397,7 @@ class LocalOpt[BT <: BTypes](val btypes: BT) {
    */
   def nullnessOptimizations(method: MethodNode, ownerClassName: InternalName): Boolean = {
     AsmAnalyzer.sizeOKForNullness(method) && {
-      lazy val nullnessAnalyzer = new AsmAnalyzer(method, ownerClassName, new NullnessAnalyzer(btypes))
+      lazy val nullnessAnalyzer = new AsmAnalyzer(method, ownerClassName, new NullnessAnalyzer(btypes, method))
 
       // When running nullness optimizations the method may still have unreachable code. Analyzer
       // frames of unreachable instructions are `null`.
diff --git a/src/compiler/scala/tools/nsc/transform/Delambdafy.scala b/src/compiler/scala/tools/nsc/transform/Delambdafy.scala
index d350ca8e17..1dfc1330c6 100644
--- a/src/compiler/scala/tools/nsc/transform/Delambdafy.scala
+++ b/src/compiler/scala/tools/nsc/transform/Delambdafy.scala
@@ -61,6 +61,9 @@ 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
@@ -68,7 +71,8 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
           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,22 @@ 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(STATIC) && methodReferencesThis(dd.symbol)) {
+              gen.mkStatic(dd, sym => sym)
+            } 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)
diff --git a/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala b/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala
index e894c58b1a..40ab8c0cf8 100644
--- a/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala
+++ b/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala
@@ -1329,6 +1329,7 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers {
   class SpecializationDuplicator(casts: Map[Symbol, Type]) extends Duplicator(casts) {
     override def retyped(context: Context, tree: Tree, oldThis: Symbol, newThis: Symbol, env: scala.collection.Map[Symbol, Type]): Tree =
       enteringSpecialize(super.retyped(context, tree, oldThis, newThis, env))
+
   }
 
   /** A tree symbol substituter that substitutes on type skolems.
diff --git a/src/compiler/scala/tools/nsc/transform/UnCurry.scala b/src/compiler/scala/tools/nsc/transform/UnCurry.scala
index e0b1543f24..374e8430d8 100644
--- a/src/compiler/scala/tools/nsc/transform/UnCurry.scala
+++ b/src/compiler/scala/tools/nsc/transform/UnCurry.scala
@@ -213,6 +213,7 @@ abstract class UnCurry extends InfoTransform
         // Expand the function body into an anonymous class
         gen.expandFunction(localTyper)(fun, inConstructorFlag)
       } else {
+        val mustExpand = mustExpandFunction(fun)
         // method definition with the same arguments, return type, and body as the original lambda
         val liftedMethod = gen.mkLiftedFunctionBodyMethod(localTyper)(fun.symbol.owner, fun)
 
@@ -221,11 +222,18 @@ abstract class UnCurry extends InfoTransform
           gen.mkForwarder(gen.mkAttributedRef(liftedMethod.symbol), (fun.vparams map (_.symbol)) :: Nil)
         ))
 
+        if (!mustExpand) {
+          liftedMethod.symbol.updateAttachment(DelambdafyTarget)
+          liftedMethod.updateAttachment(DelambdafyTarget)
+        }
+
         val typedNewFun = localTyper.typedPos(fun.pos)(Block(liftedMethod, super.transform(newFun)))
-        if (mustExpandFunction(fun)) {
+        if (mustExpand) {
           val Block(stats, expr : Function) = typedNewFun
           treeCopy.Block(typedNewFun, stats, gen.expandFunction(localTyper)(expr, inConstructorFlag))
-        } else typedNewFun
+        } else {
+          typedNewFun
+        }
       }
 
     def transformArgs(pos: Position, fun: Symbol, args: List[Tree], formals: List[Type]) = {
@@ -341,13 +349,18 @@ abstract class UnCurry extends InfoTransform
 
     private def isSelfSynchronized(ddef: DefDef) = ddef.rhs match {
       case Apply(fn @ TypeApply(Select(sel, _), _), _) =>
-        fn.symbol == Object_synchronized && sel.symbol == ddef.symbol.enclClass && !ddef.symbol.enclClass.isTrait
+        fn.symbol == Object_synchronized && sel.symbol == ddef.symbol.enclClass && !ddef.symbol.enclClass.isTrait &&
+          !ddef.symbol.isDelambdafyTarget /* these become static later, unsuitable for ACC_SYNCHRONIZED */
       case _ => false
     }
 
     /** If an eligible method is entirely wrapped in a call to synchronized
      *  locked on the same instance, remove the synchronized scaffolding and
      *  mark the method symbol SYNCHRONIZED for bytecode generation.
+     *
+     *  Delambdafy targets are deemed ineligible as the Delambdafy phase will
+     *  replace `this.synchronized` with `$this.synchronzed` now that it emits
+     *  all lambda impl methods as static.
      */
     private def translateSynchronized(tree: Tree) = tree match {
       case dd @ DefDef(_, _, _, _, _, Apply(fn, body :: Nil)) if isSelfSynchronized(dd) =>
diff --git a/src/compiler/scala/tools/nsc/typechecker/Duplicators.scala b/src/compiler/scala/tools/nsc/typechecker/Duplicators.scala
index 0c10242950..78e72cf771 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Duplicators.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Duplicators.scala
@@ -229,7 +229,12 @@ abstract class Duplicators extends Analyzer {
 
         case ddef @ DefDef(_, _, _, _, tpt, rhs) =>
           ddef.tpt modifyType fixType
-          super.typed(ddef.clearType(), mode, pt)
+          val result = super.typed(ddef.clearType(), mode, pt)
+          // TODO this is a hack, we really need a cleaner way to transport symbol attachments to duplicated methods
+          // bodies in specialized subclasses.
+          if (ddef.hasAttachment[DelambdafyTarget.type])
+            result.symbol.updateAttachment(DelambdafyTarget)
+          result
 
         case fun: Function =>
           debuglog("Clearing the type and retyping Function: " + fun)