SI-9390 Avoid needless outer capture with local classes

An existing optimization in `Constructors` elides the outer
field in member and local classes, if the class doesn't use
the outer reference. (Member classes also need to be final,
which is a secret handshake to say we're also happy to weaken
prefix matching in the pattern matcher.)

That optimization leaves the constructor signature as is: the
constructor still accepts the outer instance, but does not store
it. For member classes, this means that we can separately compile
code that calls the constructor.

Local classes need not be hampered by this constraint, we could
remove the outer instance from the constructor call too.

Why would we want to do this?

Let's look at the case before and after this commit.

Before:
```
class C extends Object {
  def foo(): Function1 = $anonfun();
  final <static> <artifact> def $anonfun$foo$1($this: C, x: Object): Object = new <$anon: Object>($this);
  def <init>(): C = {
    C.super.<init>();
    ()
  }
};
final class anon$1 extends Object {
  def <init>($outer: C): <$anon: Object> = {
    anon$1.super.<init>();
    ()
  }
}
```

After:

```
class C extends Object {
  def foo(): Function1 = $anonfun();
  final <static> <artifact> def $anonfun$foo$1(x: Object): Object = new <$anon: Object>(null);
  def <init>(): C = {
    C.super.<init>();
    ()
  }
};
final class anon$1 extends Object {
  def <init>($outer: C): <$anon: Object> = {
    anon$1.super.<init>();
    ()
  }
}

```

However, the status quo means that a lambda that
This in turn makes lambdas that refer to such classes serializable
even when the outer class is not itself serialiable.

I have not attempted to extend this to calls to secondary constructors.

Author: retronym

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

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

@@ -279,10 +279,15 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
         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
@@ -368,6 +373,9 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
       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'")

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)

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
 }

src/reflect/scala/reflect/internal/StdNames.scala

@@ -364,6 +364,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>"

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

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

test/files/jvm/t9105.check

@@ -1,18 +1,8 @@
-#partest -Ydelambdafy:inline
-(class C$$anonfun$1$A$1,class C$$anonfun$1,null)
-(class C$$anonfun$1$B$1,class C$$anonfun$1,private final java.lang.Object C$$anonfun$1.m$1())
-(class C$$anonfun$1$C$1,class C$$anonfun$1,null)
-(class C$$anonfun$1$$anonfun$2$D$1,class C$$anonfun$1$$anonfun$2,null)
-(class C$$anonfun$met$1$E$1,class C$$anonfun$met$1,null)
-(class C$$anonfun$met$1$F$1,class C$$anonfun$met$1,private final java.lang.Object C$$anonfun$met$1.m$2())
-(class C$$anonfun$met$1$G$1,class C$$anonfun$met$1,null)
-(class C$$anonfun$met$1$$anonfun$3$H$1,class C$$anonfun$met$1$$anonfun$3,null)
-#partest !-Ydelambdafy:inline
 (class C$A$1,class C,null)
-(class C$B$1,class C,private final java.lang.Object C.m$1())
+(class C$B$1,class C,private static final java.lang.Object C.m$1())
 (class C$C$1,class C,null)
 (class C$D$1,class C,null)
 (class C$E$1,class C,public scala.Function0 C.met())
-(class C$F$1,class C,private final java.lang.Object C.m$2())
+(class C$F$1,class C,private static final java.lang.Object C.m$2())
 (class C$G$1,class C,public scala.Function0 C.met())
 (class C$H$1,class C,public scala.Function0 C.met())

test/files/run/t9390c.scala

@@ -0,0 +1,21 @@
+class C { // C is not serializable
+  def foo = {
+    { (x: Any) => new Object {} }
+  }
+}
+object Test {
+  def main(args: Array[String]): Unit = {
+    val c = new C
+    val f = c.foo
+    val f1 = serializeDeserialize(f)
+  }
+
+  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/t9390d.scala

@@ -0,0 +1,12 @@
+class C { // C is not serializable
+  def foo: () => Any = {
+    { () => class UseOuterInConstructor { C.this.toString }; new UseOuterInConstructor : Any}
+  }
+}
+object Test {
+  def main(args: Array[String]): Unit = {
+    val c = new C
+    val f = c.foo
+    f() // Doesn't NPE, as we didn't elide the outer instance in the constructor call.
+  }
+}