SI-9105 Fix EnclosingMethod for classes defined in lambdas

This change fixes both GenASM and GenBCode, except for the change
to renaming in LamdaLift mentioned below.

The reason for an inconsistent EnclosingMethod attribute was the
symbol owner chain. Initially, closure class symbols don't exist, they
are only created in UnCurry (delambdafy:inline). So walking the
originalOwner of a definition does not yield closure classes.

The commit also fixes uses of isAnonymousClass, isAnonymousFunction
and isDelambdafyFunction in two ways:

  1. by phase-travelling to an early phase. after flatten, the name
     includes the name of outer classes, so the properties may become
     accidentally true (they check for a substring in the name)

  2. by ensuring that the (destructive) renames during LambdaLift
     don't make the above properties accidentally true. This was in
     fact the cause for SI-8900.

7 files changed, 119 insertions, 24 deletions

diff --git a/src/compiler/scala/tools/nsc/CompilationUnits.scala b/src/compiler/scala/tools/nsc/CompilationUnits.scala
index 1a6843a249..6be1fda1b5 100644
--- a/src/compiler/scala/tools/nsc/CompilationUnits.scala
+++ b/src/compiler/scala/tools/nsc/CompilationUnits.scala
@@ -25,9 +25,9 @@ trait CompilationUnits { global: Global =>
   class CompilationUnit(val source: SourceFile) extends CompilationUnitContextApi { self =>
 
     /** the fresh name creator */
-    implicit val fresh: FreshNameCreator     = new FreshNameCreator
-    def freshTermName(prefix: String = "x$") = global.freshTermName(prefix)
-    def freshTypeName(prefix: String)        = global.freshTypeName(prefix)
+    implicit val fresh: FreshNameCreator                           = new FreshNameCreator
+    def freshTermName(prefix: String = nme.FRESH_TERM_NAME_PREFIX) = global.freshTermName(prefix)
+    def freshTypeName(prefix: String)                              = global.freshTypeName(prefix)
 
     /** the content of the compilation unit in tree form */
     var body: Tree = EmptyTree
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeAsmCommon.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeAsmCommon.scala
index a5f33aa786..90d2fcf4dc 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeAsmCommon.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeAsmCommon.scala
@@ -22,6 +22,23 @@ final class BCodeAsmCommon[G <: Global](val global: G) {
   }
 
   /**
+   * Cache the value of delambdafy == "inline" for each run. We need to query this value many
+   * times, so caching makes sense.
+   */
+  object delambdafyInline {
+    private var runId = -1
+    private var value = false
+
+    def apply(): Boolean = {
+      if (runId != global.currentRunId) {
+        runId = global.currentRunId
+        value = settings.Ydelambdafy.value == "inline"
+      }
+      value
+    }
+  }
+
+  /**
    * True if `classSym` is an anonymous class or a local class. I.e., false if `classSym` is a
    * member class. This method is used to decide if we should emit an EnclosingMethod attribute.
    * It is also used to decide whether the "owner" field in the InnerClass attribute should be
@@ -29,10 +46,59 @@ final class BCodeAsmCommon[G <: Global](val global: G) {
    */
   def isAnonymousOrLocalClass(classSym: Symbol): Boolean = {
     assert(classSym.isClass, s"not a class: $classSym")
-    // Here used to be an `assert(!classSym.isDelambdafyFunction)`: delambdafy lambda classes are
-    // always top-level. However, SI-8900 shows an example where the weak name-based implementation
-    // of isDelambdafyFunction failed (for a function declared in a package named "lambda").
-    classSym.isAnonymousClass || !classSym.originalOwner.isClass
+    val r = exitingPickler(classSym.isAnonymousClass) || !classSym.originalOwner.isClass
+    if (r && settings.Ybackend.value == "GenBCode") {
+      // this assertion only holds in GenBCode. lambda lift renames symbols and may accidentally
+      // introduce `$lambda` into a class name, making `isDelambdafyFunction` true. under GenBCode
+      // we prevent this, see `nonAnon` in LambdaLift.
+      // phase travel necessary: after flatten, the name includes the name of outer classes.
+      // if some outer name contains $lambda, a non-lambda class is considered lambda.
+      assert(exitingPickler(!classSym.isDelambdafyFunction), classSym.name)
+    }
+    r
+  }
+
+  /**
+   * The next enclosing definition in the source structure. Includes anonymous function classes
+   * under delambdafy:inline, even though they are only generated during UnCurry.
+   */
+  def nextEnclosing(sym: Symbol): Symbol = {
+    val origOwner = sym.originalOwner
+    // phase travel necessary: after flatten, the name includes the name of outer classes.
+    // if some outer name contains $anon, a non-anon class is considered anon.
+    if (delambdafyInline() && sym.rawowner.isAnonymousFunction) {
+      // SI-9105: special handling for anonymous functions under delambdafy:inline.
+      //
+      //   class C { def t = () => { def f { class Z } } }
+      //
+      //   class C { def t = byNameMethod { def f { class Z } } }
+      //
+      // In both examples, the method f lambda-lifted into the anonfun class.
+      //
+      // In both examples, the enclosing method of Z is f, the enclosing class is the anonfun.
+      // So nextEnclosing needs to return the following chain:  Z - f - anonFunClassSym - ...
+      //
+      // In the first example, the initial owner of f is a TermSymbol named "$anonfun" (note: not the anonFunClassSym!)
+      // In the second, the initial owner of f is t (no anon fun term symbol for by-name args!).
+      //
+      // In both cases, the rawowner of class Z is the anonFunClassSym. So the check in the `if`
+      // above makes sure we don't jump over the anonymous function in the by-name argument case.
+      //
+      // However, we cannot directly return the rawowner: if `sym` is Z, we need to include method f
+      // in the result. This is done by comparing the rawowners (read: lambdalift-targets) of `sym`
+      // and `sym.originalOwner`: if they are the same, then the originalOwner is "in between", and
+      // we need to return it.
+      // If the rawowners are different, the symbol was not in between. In the first example, the
+      // originalOwner of `f` is the anonfun-term-symbol, whose rawowner is C. So the nextEnclosing
+      // of `f` is its rawowner, the anonFunClassSym.
+      //
+      // In delambdafy:method we don't have that problem. The f method is lambda-lifted into C,
+      // not into the anonymous function class. The originalOwner chain is Z - f - C.
+      if (sym.originalOwner.rawowner == sym.rawowner) sym.originalOwner
+      else sym.rawowner
+    } else {
+      origOwner
+    }
   }
 
   /**
@@ -63,9 +129,9 @@ final class BCodeAsmCommon[G <: Global](val global: G) {
     def enclosingMethod(sym: Symbol): Option[Symbol] = {
       if (sym.isClass || sym == NoSymbol) None
       else if (sym.isMethod) Some(sym)
-      else enclosingMethod(sym.originalOwner)
+      else enclosingMethod(nextEnclosing(sym))
     }
-    enclosingMethod(classSym.originalOwner)
+    enclosingMethod(nextEnclosing(classSym))
   }
 
   /**
@@ -76,9 +142,9 @@ final class BCodeAsmCommon[G <: Global](val global: G) {
     assert(classSym.isClass, classSym)
     def enclosingClass(sym: Symbol): Symbol = {
       if (sym.isClass) sym
-      else enclosingClass(sym.originalOwner)
+      else enclosingClass(nextEnclosing(sym))
     }
-    enclosingClass(classSym.originalOwner)
+    enclosingClass(nextEnclosing(classSym))
   }
 
   final case class EnclosingMethodEntry(owner: String, name: String, methodDescriptor: String)
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala
index a9bce82acd..7814ed858b 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala
@@ -527,7 +527,7 @@ abstract class BTypes {
    * local and anonymous classes, no matter if there is an enclosing method or not. Accordingly, the
    * "class" field (see below) must be always defined, while the "method" field may be null.
    *
-   * NOTE: When a EnclosingMethod attribute is requried (local and anonymous classes), the "outer"
+   * NOTE: When an EnclosingMethod attribute is requried (local and anonymous classes), the "outer"
    * field in the InnerClass table must be null.
    *
    * Fields:
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala
index 94f9b585d9..bc880e002e 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala
@@ -227,7 +227,9 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes {
       }
 
       val innerName: Option[String] = {
-        if (innerClassSym.isAnonymousClass || innerClassSym.isAnonymousFunction) None
+        // phase travel necessary: after flatten, the name includes the name of outer classes.
+        // if some outer name contains $anon, a non-anon class is considered anon.
+        if (exitingPickler(innerClassSym.isAnonymousClass || innerClassSym.isAnonymousFunction)) None
         else Some(innerClassSym.rawname + innerClassSym.moduleSuffix) // moduleSuffix for module classes
       }
 
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala
index abe3bc512c..c36afd018b 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala
@@ -692,11 +692,12 @@ abstract class GenASM extends SubComponent with BytecodeWriters { self =>
         }
       }
 
-      def innerName(innerSym: Symbol): String =
-        if (innerSym.isAnonymousClass || innerSym.isAnonymousFunction)
-          null
-        else
-          innerSym.rawname + innerSym.moduleSuffix
+      def innerName(innerSym: Symbol): String = {
+        // phase travel necessary: after flatten, the name includes the name of outer classes.
+        // if some outer name contains $anon, a non-anon class is considered anon.
+        if (exitingPickler(innerSym.isAnonymousClass || innerSym.isAnonymousFunction)) null
+        else innerSym.rawname + innerSym.moduleSuffix
+      }
 
       innerClassBuffer ++= {
         val members = exitingPickler(memberClassesOf(csym))
diff --git a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala
index fa0c1f797b..e9fd6e14a9 100644
--- a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala
+++ b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala
@@ -250,21 +250,30 @@ abstract class LambdaLift extends InfoTransform {
         debuglog("renaming in %s: %s => %s".format(sym.owner.fullLocationString, originalName, sym.name))
       }
 
+      // make sure that the name doesn't make the symbol accidentally `isAnonymousClass` (et.al) by
+      // introducing `$anon` in its name. to be cautious, we don't make this change in the default
+      // backend under 2.11.x, so only in GenBCode.
+      def nonAnon(s: String) = if (settings.Ybackend.value == "GenBCode") nme.ensureNonAnon(s) else s
+
       def newName(sym: Symbol): Name = {
         val originalName = sym.name
         def freshen(prefix: String): Name =
           if (originalName.isTypeName) unit.freshTypeName(prefix)
           else unit.freshTermName(prefix)
 
+        val join = nme.NAME_JOIN_STRING
         if (sym.isAnonymousFunction && sym.owner.isMethod) {
-          freshen(sym.name + nme.NAME_JOIN_STRING + sym.owner.name + nme.NAME_JOIN_STRING)
+          freshen(sym.name + join + nonAnon(sym.owner.name.toString) + join)
         } else {
+          val name = freshen(sym.name + join)
           // SI-5652 If the lifted symbol is accessed from an inner class, it will be made public. (where?)
-          //         Generating a unique name, mangled with the enclosing class name, avoids a VerifyError
-          //         in the case that a sub-class happens to lifts out a method with the *same* name.
-          val name = freshen("" + sym.name + nme.NAME_JOIN_STRING)
-          if (originalName.isTermName && !sym.enclClass.isImplClass && calledFromInner(sym)) nme.expandedName(name.toTermName, sym.enclClass)
-          else name
+          //         Generating a unique name, mangled with the enclosing full class name (including
+          //         package - subclass might have the same name), avoids a VerifyError in the case
+          //         that a sub-class happens to lifts out a method with the *same* name.
+          if (originalName.isTermName && !sym.enclClass.isImplClass && calledFromInner(sym))
+            newTermNameCached(nonAnon(sym.enclClass.fullName('$')) + nme.EXPAND_SEPARATOR_STRING + name)
+          else
+            name
         }
       }
 
diff --git a/src/reflect/scala/reflect/internal/StdNames.scala b/src/reflect/scala/reflect/internal/StdNames.scala
index c94ee996e4..f32b7326fe 100644
--- a/src/reflect/scala/reflect/internal/StdNames.scala
+++ b/src/reflect/scala/reflect/internal/StdNames.scala
@@ -112,6 +112,23 @@ trait StdNames {
     val ROOT: NameType                         = "<root>"
     val SPECIALIZED_SUFFIX: NameType           = "$sp"
 
+    val NESTED_IN: String                      = "$nestedIn"
+    val NESTED_IN_ANON_CLASS: String           = NESTED_IN + ANON_CLASS_NAME.toString.replace("$", "")
+    val NESTED_IN_ANON_FUN: String             = NESTED_IN + ANON_FUN_NAME.toString.replace("$", "")
+    val NESTED_IN_LAMBDA: String               = NESTED_IN + DELAMBDAFY_LAMBDA_CLASS_NAME.toString.replace("$", "")
+
+    /**
+     * Ensures that name mangling does not accidentally make a class respond `true` to any of
+     * isAnonymousClass, isAnonymousFunction, isDelambdafyFunction, e.g. by introducing "$anon".
+     */
+    def ensureNonAnon(name: String) = {
+      name
+        .replace(nme.ANON_CLASS_NAME.toString, NESTED_IN_ANON_CLASS)
+        .replace(nme.ANON_FUN_NAME.toString, NESTED_IN_ANON_FUN)
+        .replace(nme.DELAMBDAFY_LAMBDA_CLASS_NAME.toString, NESTED_IN_LAMBDA)
+    }
+
+
     // value types (and AnyRef) are all used as terms as well
     // as (at least) arguments to the @specialize annotation.
     final val Boolean: NameType = "Boolean"