Merge pull request #3149 from soc/SI-7974

 Fix broken 'Symbol-handling code in CleanUp

5 files changed, 53 insertions, 26 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/CleanUp.scala b/src/compiler/scala/tools/nsc/transform/CleanUp.scala
index 9738769db9..f14fce5de9 100644
--- a/src/compiler/scala/tools/nsc/transform/CleanUp.scala
+++ b/src/compiler/scala/tools/nsc/transform/CleanUp.scala
@@ -481,18 +481,33 @@ abstract class CleanUp extends Statics with Transform with ast.TreeDSL {
       * For instance, say we have a Scala class:
       *
       * class Cls {
-      *   // ...
-      *   def someSymbol = `symbolic
-      *   // ...
+      *   def someSymbol1 = 'Symbolic1
+      *   def someSymbol2 = 'Symbolic2
+      *   def sameSymbol1 = 'Symbolic1
+      *   val someSymbol3 = 'Symbolic3
       * }
       *
       * After transformation, this class looks like this:
       *
       * class Cls {
-      *   private "static" val <some_name>$symbolic = Symbol("symbolic")
-      *   // ...
-      *   def someSymbol = <some_name>$symbolic
-      *   // ...
+      *   private <static> var symbol$1: scala.Symbol
+      *   private <static> var symbol$2: scala.Symbol
+      *   private <static> var symbol$3: scala.Symbol
+      *   private          val someSymbol3: scala.Symbol
+      *
+      *   private <static> def <clinit> = {
+      *     symbol$1 = Symbol.apply("Symbolic1")
+      *     symbol$2 = Symbol.apply("Symbolic2")
+      *   }
+      *
+      *   private def <init> = {
+      *     someSymbol3 = symbol$3
+      *   }
+      *
+      *   def someSymbol1 = symbol$1
+      *   def someSymbol2 = symbol$2
+      *   def sameSymbol1 = symbol$1
+      *   val someSymbol3 = someSymbol3
       * }
       *
       * The reasoning behind this transformation is the following. Symbols get interned - they are stored
@@ -502,17 +517,17 @@ abstract class CleanUp extends Statics with Transform with ast.TreeDSL {
       * is accessed only once during class loading, and after that, the unique symbol is in the static
       * member. Hence, it is cheap to both reach the unique symbol and do equality checks on it.
       *
-      * And, finally, be advised - scala symbol literal and the Symbol class of the compiler
+      * And, finally, be advised - Scala's Symbol literal (scala.Symbol) and the Symbol class of the compiler
       * have little in common.
       */
       case Apply(fn, (arg @ Literal(Constant(symname: String))) :: Nil) if fn.symbol == Symbol_apply =>
         def transformApply = {
-        // add the symbol name to a map if it's not there already
-        val rhs = gen.mkMethodCall(Symbol_apply, arg :: Nil)
-        val staticFieldSym = getSymbolStaticField(tree.pos, symname, rhs, tree)
-        // create a reference to a static field
-        val ntree = typedWithPos(tree.pos)(REF(staticFieldSym))
-        super.transform(ntree)
+          // add the symbol name to a map if it's not there already
+          val rhs = gen.mkMethodCall(Symbol_apply, arg :: Nil)
+          val staticFieldSym = getSymbolStaticField(tree.pos, symname, rhs, tree)
+          // create a reference to a static field
+          val ntree = typedWithPos(tree.pos)(REF(staticFieldSym))
+          super.transform(ntree)
         }
         transformApply
 
diff --git a/src/reflect/scala/reflect/internal/Definitions.scala b/src/reflect/scala/reflect/internal/Definitions.scala
index 1fe6f249b8..38be6fcf56 100644
--- a/src/reflect/scala/reflect/internal/Definitions.scala
+++ b/src/reflect/scala/reflect/internal/Definitions.scala
@@ -1207,15 +1207,21 @@ trait Definitions extends api.StandardDefinitions {
     }
     def getMemberMethod(owner: Symbol, name: Name): TermSymbol = {
       getMember(owner, name.toTermName) match {
-        // todo. member symbol becomes a term symbol in cleanup. is this a bug?
-        // case x: MethodSymbol => x
         case x: TermSymbol => x
         case _             => fatalMissingSymbol(owner, name, "method")
       }
     }
 
+    private lazy val erasurePhase = findPhaseWithName("erasure")
     def getMemberIfDefined(owner: Symbol, name: Name): Symbol =
-      owner.info.nonPrivateMember(name)
+      // findMember considered harmful after erasure; e.g.
+      //
+      // scala> exitingErasure(Symbol_apply).isOverloaded
+      // res27: Boolean = true
+      //
+      enteringPhaseNotLaterThan(erasurePhase )(
+        owner.info.nonPrivateMember(name)
+      )
 
     /** Using getDecl rather than getMember may avoid issues with
      *  OverloadedTypes turning up when you don't want them, if you
diff --git a/src/reflect/scala/reflect/internal/SymbolTable.scala b/src/reflect/scala/reflect/internal/SymbolTable.scala
index 0ce5a0fbea..c46a559d6d 100644
--- a/src/reflect/scala/reflect/internal/SymbolTable.scala
+++ b/src/reflect/scala/reflect/internal/SymbolTable.scala
@@ -244,6 +244,18 @@ abstract class SymbolTable extends macros.Universe
     finally popPhase(saved)
   }
 
+  final def findPhaseWithName(phaseName: String): Phase = {
+    var ph = phase
+    while (ph != NoPhase && ph.name != phaseName) {
+      ph = ph.prev
+    }
+    if (ph eq NoPhase) phase else ph
+  }
+  final def enteringPhaseWithName[T](phaseName: String)(body: => T): T = {
+    val phase = findPhaseWithName(phaseName)
+    enteringPhase(phase)(body)
+  }
+
   def slowButSafeEnteringPhase[T](ph: Phase)(op: => T): T = {
     if (isCompilerUniverse) enteringPhase(ph)(op)
     else op
diff --git a/src/reflect/scala/reflect/internal/util/TraceSymbolActivity.scala b/src/reflect/scala/reflect/internal/util/TraceSymbolActivity.scala
index f61c1f3c50..e4a6503184 100644
--- a/src/reflect/scala/reflect/internal/util/TraceSymbolActivity.scala
+++ b/src/reflect/scala/reflect/internal/util/TraceSymbolActivity.scala
@@ -41,7 +41,8 @@ trait TraceSymbolActivity {
     }
   }
 
-  private def signature(id: Int) = runBeforeErasure(allSymbols(id).defString)
+  private lazy val erasurePhase = findPhaseWithName("erasure")
+  private def signature(id: Int) = enteringPhase(erasurePhase)(allSymbols(id).defString)
 
   private def dashes(s: Any): String = ("" + s) map (_ => '-')
   private def show(s1: Any, ss: Any*) {
@@ -87,14 +88,6 @@ trait TraceSymbolActivity {
   private def showFreq[T, U](xs: Traversable[T])(groupFn: T => U, showFn: U => String) = {
     showMapFreq(xs.toList groupBy groupFn)(showFn)
   }
-  private lazy val findErasurePhase: Phase = {
-    var ph = phase
-    while (ph != NoPhase && ph.name != "erasure") {
-      ph = ph.prev
-    }
-    if (ph eq NoPhase) phase else ph
-  }
-  private def runBeforeErasure[T](body: => T): T = enteringPhase(findErasurePhase)(body)
 
   def showAllSymbols() {
     if (!enabled) return
diff --git a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala
index 9c4a3a5fe1..c87995275f 100644
--- a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala
+++ b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala
@@ -425,6 +425,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse  =>
     definitions.languageFeatureModule
     definitions.metaAnnotations
     definitions.AnnotationDefaultAttr
+    // inaccessible: definitions.erasurePhase
     definitions.isPhantomClass
     definitions.syntheticCoreClasses
     definitions.syntheticCoreMethods