Change of TERMREFsymbol/TYPEREFsymbol semantics

Used to be "with fixed sym". Now it is: With initial symbol
as given in the serialized info.

It turns out the only previous uses of (TERM|TYPE)REFsymbol were
types that were made symbolic in self types. But exactly that caused
that problems in unpickling which we tried to workaround by changing
WithFixedSym#newLikeThis. And these fixes became less and less intuitive and
still could not solve the problem for good. Last hurdle was pickle-testing
all files in ast together.

It's much simpler to reserve (TERM|TYPE)REFsymbol for NamedTypes that have
an initial symbol. Like the previous "WithFixedSym" pickling, this avoids
inifinite recursions in pickling/extmethods.scala. But it also avoids problens
with unpickling ast/*.scala. The reason it is better is that it more accurately
models that types that exist before pickling.

This change also undoes previous changes to WithFixedSym#newlikeThis. Better to keep
the more restrictive contract for these.

4 files changed, 51 insertions, 56 deletions

diff --git a/src/dotty/tools/dotc/core/Types.scala b/src/dotty/tools/dotc/core/Types.scala
index 363e3006a..f8ae06a94 100644
--- a/src/dotty/tools/dotc/core/Types.scala
+++ b/src/dotty/tools/dotc/core/Types.scala
@@ -480,7 +480,7 @@ object Types {
       case ex: MergeError =>
         throw new MergeError(s"${ex.getMessage} as members of type ${pre.show}")
       case ex: Throwable =>
-        println(i"findMember exception for $this: ${this.widen} member $name")
+        println(s"findMember exception for $this member $name")
         throw ex // DEBUG
     } finally {
       recCount -= 1
@@ -1159,6 +1159,11 @@ object Types {
     private[this] var lastSymbol: Symbol = _
     private[this] var checkedPeriod = Nowhere
 
+    /** Provided for debugging, can be used to intercept assignments of uncommon denotations
+     */  
+    private def checkDenot(d: Denotation)(implicit ctx: Context) = {
+    }
+    
     // Invariants:
     // (1) checkedPeriod != Nowhere  =>  lastDenotation != null
     // (2) lastDenotation != null    =>  lastSymbol != null
@@ -1238,6 +1243,7 @@ object Types {
           // phase but a defined denotation earlier (e.g. a TypeRef to an abstract type
           // is undefined after erasure.) We need to be able to do time travel back and
           // forth also in these cases.
+          checkDenot(d)
           lastDenotation = d
           lastSymbol = d.symbol
           checkedPeriod = ctx.period
@@ -1285,6 +1291,7 @@ object Types {
       if (Config.checkNoDoubleBindings)
         if (ctx.settings.YnoDoubleBindings.value)
           checkSymAssign(denot.symbol)
+      checkDenot(denot)
       lastDenotation = denot
       lastSymbol = denot.symbol
     }
@@ -1511,11 +1518,8 @@ object Types {
     override def withSym(sym: Symbol, signature: Signature)(implicit ctx: Context): ThisType =
       unsupported("withSym")
 
-    override def newLikeThis(prefix: Type)(implicit ctx: Context): NamedType = {
-      var newSym = prefix.member(fixedSym.name).symbol
-      if (!newSym.exists) newSym = fixedSym
-      NamedType.withFixedSym(prefix, newSym)
-    }
+    override def newLikeThis(prefix: Type)(implicit ctx: Context): NamedType =
+      NamedType.withFixedSym(prefix, fixedSym)
 
     override def equals(that: Any) = that match {
       case that: WithFixedSym => this.prefix == that.prefix && (this.fixedSym eq that.fixedSym)
diff --git a/src/dotty/tools/dotc/core/pickling/PickleFormat.scala b/src/dotty/tools/dotc/core/pickling/PickleFormat.scala
index d37a3673d..872265e2d 100644
--- a/src/dotty/tools/dotc/core/pickling/PickleFormat.scala
+++ b/src/dotty/tools/dotc/core/pickling/PickleFormat.scala
@@ -269,7 +269,7 @@ object PickleFormat {
   final val DOUBLEconst = 76
   final val STRINGconst = 77
   final val IMPORTED = 78
-  
+
   final val THIS = 96
   final val CLASSconst = 97
   final val ENUMconst = 98
diff --git a/src/dotty/tools/dotc/core/pickling/TreePickler.scala b/src/dotty/tools/dotc/core/pickling/TreePickler.scala
index 14a92e8e6..85addc563 100644
--- a/src/dotty/tools/dotc/core/pickling/TreePickler.scala
+++ b/src/dotty/tools/dotc/core/pickling/TreePickler.scala
@@ -32,20 +32,6 @@ class TreePickler(pickler: TastyPickler) {
     symRefs.get(sym)
   }
 
-  private var makeSymbolicRefsTo: Symbol = NoSymbol
-
-  /** All references to members of class `sym` are pickled
-   *  as symbolic references. Used to pickle the self info of a class.
-   *  Without this precaution we get an infinite cycle when unpickling pos/extmethods.scala
-   *  The problem arises when a self type of a trait is a type parameter of the same trait.
-   */
-  private def withSymbolicRefsTo[T](sym: Symbol)(op: => T): T = {
-    val saved = makeSymbolicRefsTo
-    makeSymbolicRefsTo = sym
-    try op
-    finally makeSymbolicRefsTo = saved
-  }
-  
   def preRegister(tree: Tree)(implicit ctx: Context): Unit = tree match {
     case tree: MemberDef => 
       if (!symRefs.contains(tree.symbol)) symRefs(tree.symbol) = NoAddr
@@ -88,6 +74,11 @@ class TreePickler(pickler: TastyPickler) {
     assert(!sym.is(Flags.Package), sym)
     forwardSymRefs(sym) = ref :: forwardSymRefs.getOrElse(sym, Nil)  
   }
+  
+  private def isLocallyDefined(sym: Symbol)(implicit ctx: Context) = symRefs.get(sym) match {
+    case Some(label) => assert(sym.exists); label != NoAddr
+    case None => false
+  }
 
   def pickle(trees: List[Tree])(implicit ctx: Context) = {
     
@@ -162,7 +153,8 @@ class TreePickler(pickler: TastyPickler) {
           writeByte(if (tpe.isType) TYPEREFpkg else TERMREFpkg)
           pickleName(qualifiedName(sym))
         } 
-        else if (tpe.prefix == NoPrefix) {
+        else {
+          assert(tpe.prefix == NoPrefix)
           def pickleRef() = {
             writeByte(if (tpe.isType) TYPEREFdirect else TERMREFdirect)
             pickleSymRef(sym)        
@@ -178,10 +170,6 @@ class TreePickler(pickler: TastyPickler) {
           }
           else pickleRef()
         }
-        else {
-          writeByte(if (tpe.isType) TYPEREFsymbol else TERMREFsymbol)
-          pickleSymRef(sym); pickleType(tpe.prefix)
-        }
       case tpe: TermRefWithSignature =>
         writeByte(TERMREF)
         pickleNameAndSig(tpe.name, tpe.signature); pickleType(tpe.prefix)
@@ -190,12 +178,13 @@ class TreePickler(pickler: TastyPickler) {
           // instantiated lambdas are pickled as APPLIEDTYPE; #Apply will 
           // be reconstituted when unpickling.
           pickleType(tpe.prefix)
-        else tpe.prefix match {
-          case prefix: ThisType if prefix.cls == makeSymbolicRefsTo =>
-            pickleType(NamedType.withFixedSym(tpe.prefix, tpe.symbol))
-          case _ =>
-            writeByte(if (tpe.isType) TYPEREF else TERMREF)
-            pickleName(tpe.name); pickleType(tpe.prefix)
+        else if (isLocallyDefined(tpe.symbol)) {
+          writeByte(if (tpe.isType) TYPEREFsymbol else TERMREFsymbol)
+          pickleSymRef(tpe.symbol); pickleType(tpe.prefix)
+        }
+        else { 
+          writeByte(if (tpe.isType) TYPEREF else TERMREF)
+          pickleName(tpe.name); pickleType(tpe.prefix)
         }
       case tpe: ThisType =>
         writeByte(THIS)
@@ -432,12 +421,10 @@ class TreePickler(pickler: TastyPickler) {
           if ((selfInfo ne NoType) || !tree.self.isEmpty) {
             writeByte(SELFDEF)
             pickleName(tree.self.name)
-            withSymbolicRefsTo(tree.symbol.owner) {
-              pickleType {
-                cinfo.selfInfo match {
-                  case sym: Symbol => sym.info
-                  case tp: Type => tp
-                }
+            pickleType {
+              cinfo.selfInfo match {
+                case sym: Symbol => sym.info
+                case tp: Type => tp
               }
             }
           }
diff --git a/src/dotty/tools/dotc/core/pickling/TreeUnpickler.scala b/src/dotty/tools/dotc/core/pickling/TreeUnpickler.scala
index cfee3851a..a1a630d92 100644
--- a/src/dotty/tools/dotc/core/pickling/TreeUnpickler.scala
+++ b/src/dotty/tools/dotc/core/pickling/TreeUnpickler.scala
@@ -246,26 +246,11 @@ class TreeUnpickler(reader: TastyReader, tastyName: TastyName.Table) {
         case TYPEREFdirect | TERMREFdirect => 
           NamedType.withFixedSym(NoPrefix, readSymRef())
         case TYPEREFsymbol | TERMREFsymbol =>
-          val sym = readSymRef()
-          val prefix = readType()
-          val res = NamedType.withFixedSym(prefix, sym)
-          if (prefix.isInstanceOf[ThisType]) res.withDenot(sym.denot) else res
-            // without this precaution we get an infinite cycle when unpickling pos/extmethods.scala
-            // the problem arises when a self type of a trait is a type parameter of the same trait.
+          readSymNameRef()
         case TYPEREFpkg =>
-          val name = readName()
-          val pkg = 
-            if (name == nme.ROOT) defn.RootClass
-            else if (name == nme.EMPTY_PACKAGE) defn.EmptyPackageClass
-            else ctx.requiredPackage(name).moduleClass
-          pkg.typeRef
+          readPackageRef().moduleClass.typeRef
         case TERMREFpkg =>
-          val name = readName()
-          val pkg = 
-            if (name == nme.ROOT) defn.RootPackage
-            else if (name == nme.EMPTY_PACKAGE) defn.EmptyPackageVal
-            else ctx.requiredPackage(name)
-          pkg.termRef
+          readPackageRef().termRef
         case TYPEREF =>
           val name =  readName().toTypeName
           TypeRef(readType(), name)
@@ -312,6 +297,25 @@ class TreeUnpickler(reader: TastyReader, tastyName: TastyName.Table) {
       if (tag < firstLengthTreeTag) readSimpleType() else readLengthType()
     }
     
+    private def readSymNameRef()(implicit ctx: Context): Type = {
+      val sym = readSymRef()
+      val prefix = readType()
+      val res = NamedType.withSymAndName(prefix, sym, sym.name)
+      prefix match {
+        case prefix: ThisType if prefix.cls eq sym.owner => res.withDenot(sym.denot)
+          // without this precaution we get an infinite cycle when unpickling pos/extmethods.scala
+          // the problem arises when a self type of a trait is a type parameter of the same trait.
+        case _ => res
+      }      
+    }
+    
+    private def readPackageRef()(implicit ctx: Context): TermSymbol = {
+      val name = readName()
+      if (name == nme.ROOT) defn.RootPackage
+      else if (name == nme.EMPTY_PACKAGE) defn.EmptyPackageVal
+      else ctx.requiredPackage(name)
+    }
+    
     def readTypeRef(): Type = 
       typeAtAddr(readAddr())