Merge pull request #5099 from retronym/ticket/9390

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

7 files changed, 49 insertions, 11 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/Constructors.scala b/src/compiler/scala/tools/nsc/transform/Constructors.scala
index 636fb08b89..971a55f763 100644
--- a/src/compiler/scala/tools/nsc/transform/Constructors.scala
+++ b/src/compiler/scala/tools/nsc/transform/Constructors.scala
@@ -715,6 +715,9 @@ abstract class Constructors extends Statics with Transform with ast.TreeDSL {
           primaryConstrBody.expr)
       })
 
+      if (omittableAccessor.exists(_.isOuterField) && !constructorStats.exists(_.exists { case i: Ident if i.symbol.isOuterParam => true; case _ => false}))
+        primaryConstructor.symbol.updateAttachment(OuterArgCanBeElided)
+
       val constructors = primaryConstructor :: auxConstructors
 
       // Unlink all fields that can be dropped from class scope
diff --git a/src/compiler/scala/tools/nsc/transform/Delambdafy.scala b/src/compiler/scala/tools/nsc/transform/Delambdafy.scala
index 1dfc1330c6..2dd8def53e 100644
--- a/src/compiler/scala/tools/nsc/transform/Delambdafy.scala
+++ b/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,10 +273,21 @@ 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 Apply(fun, outer :: rest) if shouldElideOuterArg(fun.symbol, outer) =>
+        val nullOuter = gen.mkZero(outer.tpe)
+        treeCopy.Apply(tree, transform(fun), nullOuter :: transformTrees(rest))
       case _ => super.transform(tree)
     }
   } // DelambdafyTransformer
 
+  private def shouldElideOuterArg(fun: Symbol, outerArg: Tree): Boolean =
+    fun.isConstructor && treeInfo.isQualifierSafeToElide(outerArg) && fun.hasAttachment[OuterArgCanBeElided.type]
 
   // A traverser that finds symbols used but not defined in the given Tree
   // TODO freeVarTraverser in LambdaLift does a very similar task. With some
@@ -326,19 +338,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 +370,12 @@ 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 Apply(fun, outer :: rest) if shouldElideOuterArg(fun.symbol, outer) =>
+        super.traverse(fun)
+        super.traverseTrees(rest)
       case This(_) =>
         if (currentMethod.exists && tree.symbol == currentMethod.enclClass) {
           debuglog(s"$currentMethod directly refers to 'this'")
diff --git a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala
index 3d6fad4238..411ff6b9be 100644
--- a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala
+++ b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala
@@ -67,8 +67,6 @@ abstract class ExplicitOuter extends InfoTransform
     result
   }
 
-  private val innerClassConstructorParamName: TermName = newTermName("arg" + nme.OUTER)
-
   class RemoveBindingsTransformer(toRemove: Set[Symbol]) extends Transformer {
     override def transform(tree: Tree) = tree match {
       case Bind(_, body) if toRemove(tree.symbol) => super.transform(body)
@@ -169,7 +167,7 @@ abstract class ExplicitOuter extends InfoTransform
 
       val paramsWithOuter =
         if (sym.isClassConstructor && isInner(sym.owner)) // 1
-          sym.newValueParameter(innerClassConstructorParamName, sym.pos).setInfo(sym.owner.outerClass.thisType) :: params
+          sym.newValueParameter(nme.OUTER_ARG, sym.pos).setInfo(sym.owner.outerClass.thisType) :: params
         else params
 
       if ((resTpTransformed ne resTp) || (paramsWithOuter ne params)) MethodType(paramsWithOuter, resTpTransformed)
diff --git a/src/reflect/scala/reflect/internal/StdAttachments.scala b/src/reflect/scala/reflect/internal/StdAttachments.scala
index ef95b38843..76e34153c9 100644
--- a/src/reflect/scala/reflect/internal/StdAttachments.scala
+++ b/src/reflect/scala/reflect/internal/StdAttachments.scala
@@ -71,4 +71,9 @@ trait StdAttachments {
   abstract class InlineAnnotatedAttachment
   case object NoInlineCallsiteAttachment extends InlineAnnotatedAttachment
   case object InlineCallsiteAttachment extends InlineAnnotatedAttachment
+
+  /** Attached to a local class that has its outer field elided. A `null` constant may be passed
+    * in place of the outer parameter, can help callers to avoid capturing the outer instance.
+    */
+  case object OuterArgCanBeElided extends PlainAttachment
 }
diff --git a/src/reflect/scala/reflect/internal/StdNames.scala b/src/reflect/scala/reflect/internal/StdNames.scala
index 20a485469d..4f5a545c95 100644
--- a/src/reflect/scala/reflect/internal/StdNames.scala
+++ b/src/reflect/scala/reflect/internal/StdNames.scala
@@ -365,6 +365,7 @@ trait StdNames {
     val MODULE_INSTANCE_FIELD: NameType    = NameTransformer.MODULE_INSTANCE_NAME  // "MODULE$"
     val OUTER: NameType                    = "$outer"
     val OUTER_LOCAL: NameType              = OUTER.localName
+    val OUTER_ARG: NameType                = "arg" + OUTER
     val OUTER_SYNTH: NameType              = "<outer>" // emitted by virtual pattern matcher, replaced by outer accessor in explicitouter
     val ROOTPKG: NameType                  = "_root_"
     val SELECTOR_DUMMY: NameType           = "<unapply-selector>"
diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala
index ba195363c1..6f2d8d802a 100644
--- a/src/reflect/scala/reflect/internal/Symbols.scala
+++ b/src/reflect/scala/reflect/internal/Symbols.scala
@@ -914,6 +914,9 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     /** Is this symbol an accessor method for outer? */
     final def isOuterField = isArtifact && (unexpandedName == nme.OUTER_LOCAL)
 
+    /** Is this symbol an outer parameter in a constructor */
+    final def isOuterParam = isParameter && owner.isConstructor && (name == nme.OUTER_ARG || name == nme.OUTER)
+
     /** Does this symbol denote a stable value, ignoring volatility?
      *
      * Stability and volatility are checked separately to allow volatile paths in patterns that amount to equality checks. SI-6815
diff --git a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala
index 28222cf9a7..0a90a141d3 100644
--- a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala
+++ b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala
@@ -45,6 +45,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse  =>
     this.SubpatternsAttachment
     this.NoInlineCallsiteAttachment
     this.InlineCallsiteAttachment
+    this.OuterArgCanBeElided
     this.noPrint
     this.typeDebug
     this.Range