LazyRefs break cycles for unpickled types

Insert LazyRefs to break cycles for F-bounded types that
are unpickled or read from Java signatures.

7 files changed, 82 insertions, 31 deletions

diff --git a/src/dotty/tools/dotc/core/SymDenotations.scala b/src/dotty/tools/dotc/core/SymDenotations.scala
index fd47ee4ec..c543a5a0c 100644
--- a/src/dotty/tools/dotc/core/SymDenotations.scala
+++ b/src/dotty/tools/dotc/core/SymDenotations.scala
@@ -147,7 +147,7 @@ object SymDenotations {
     }
 
     private def completeFrom(completer: LazyType)(implicit ctx: Context): Unit = {
-      if (myFlags is Touched) throw new CyclicReference(this)
+      if (myFlags is Touched) throw CyclicReference(this)
       myFlags |= Touched
 
       // completions.println(s"completing ${this.debugString}")
@@ -1034,7 +1034,7 @@ object SymDenotations {
       }
 
     private def computeBases(implicit ctx: Context): Unit = {
-      if (myBaseClasses eq Nil) throw new CyclicReference(this)
+      if (myBaseClasses eq Nil) throw CyclicReference(this)
       myBaseClasses = Nil
       val seen = new mutable.BitSet
       val locked = new mutable.BitSet
@@ -1294,7 +1294,7 @@ object SymDenotations {
               basetp = computeBaseTypeRefOf(tp)
               baseTypeRefCache.put(tp, basetp)
             } else if (basetp == NoPrefix) {
-              throw new CyclicReference(this)
+              throw CyclicReference(this)
             }
             basetp
           case _ =>
diff --git a/src/dotty/tools/dotc/core/Types.scala b/src/dotty/tools/dotc/core/Types.scala
index a81d200d3..8ec5c7295 100644
--- a/src/dotty/tools/dotc/core/Types.scala
+++ b/src/dotty/tools/dotc/core/Types.scala
@@ -1236,7 +1236,7 @@ object Types {
       if (ctx.underlyingRecursions < LogPendingUnderlyingThreshold)
         op
       else if (ctx.pendingUnderlying contains this)
-        throw new CyclicReference(symbol)
+        throw CyclicReference(symbol)
       else
         try {
           ctx.pendingUnderlying += this
@@ -1487,7 +1487,7 @@ object Types {
       unique(new CachedConstantType(value))
   }
 
-  case class LazyRef(refFn: () => Type) extends UncachedProxyType with TermType {
+  case class LazyRef(refFn: () => Type) extends UncachedProxyType with ValueType {
     lazy val ref = refFn()
     override def underlying(implicit ctx: Context) = ref
     override def toString = s"LazyRef($ref)"
@@ -2689,10 +2689,17 @@ object Types {
     extends FatalTypeError(
       s"""malformed type: $pre is not a legal prefix for $denot because it contains abstract type member${if (absMembers.size == 1) "" else "s"} ${absMembers.mkString(", ")}""")
 
-  class CyclicReference(val denot: SymDenotation)
+  class CyclicReference private (val denot: SymDenotation)
     extends FatalTypeError(s"cyclic reference involving $denot") {
     def show(implicit ctx: Context) = s"cyclic reference involving ${denot.show}"
-    printStackTrace()
+  }
+
+  object CyclicReference {
+    def apply(denot: SymDenotation)(implicit ctx: Context): CyclicReference = {
+      val ex = new CyclicReference(denot)
+      if (!(ctx.mode is typer.Mode.CheckCyclic)) ex.printStackTrace()
+      ex
+    }
   }
 
   class MergeError(msg: String) extends FatalTypeError(msg)
diff --git a/src/dotty/tools/dotc/core/pickling/ClassfileParser.scala b/src/dotty/tools/dotc/core/pickling/ClassfileParser.scala
index 59658c9c1..193c872f1 100644
--- a/src/dotty/tools/dotc/core/pickling/ClassfileParser.scala
+++ b/src/dotty/tools/dotc/core/pickling/ClassfileParser.scala
@@ -11,6 +11,7 @@ import java.lang.Integer.toHexString
 import scala.collection.{ mutable, immutable }
 import scala.collection.mutable.{ ListBuffer, ArrayBuffer }
 import scala.annotation.switch
+import typer.Checking.checkNonCyclic
 import io.AbstractFile
 
 class ClassfileParser(
@@ -337,7 +338,11 @@ class ClassfileParser(
         val savedIndex = index
         try {
           index = start
-          denot.info = sig2typeBounds(tparams, skiptvs = false)
+          denot.info =
+            checkNonCyclic( // we need the checkNonCyclic call to insert LazyRefs for F-bounded cycles
+                denot.symbol,
+                sig2typeBounds(tparams, skiptvs = false),
+                reportErrors = false)
         } finally {
           index = savedIndex
         }
diff --git a/src/dotty/tools/dotc/core/pickling/UnPickler.scala b/src/dotty/tools/dotc/core/pickling/UnPickler.scala
index de3f626da..2e21358e4 100644
--- a/src/dotty/tools/dotc/core/pickling/UnPickler.scala
+++ b/src/dotty/tools/dotc/core/pickling/UnPickler.scala
@@ -15,6 +15,7 @@ import printing.Texts._
 import printing.Printer
 import io.AbstractFile
 import util.common._
+import typer.Checking.checkNonCyclic
 import PickleBuffer._
 import scala.reflect.internal.pickling.PickleFormat._
 import Decorators._
@@ -516,7 +517,11 @@ class UnPickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClassRoot:
             denot setFlag Scala2x
           case denot =>
             val tp1 = depoly(tp, denot)
-            denot.info = if (tag == ALIASsym) TypeAlias(tp1) else tp1
+            denot.info =
+              if (tag == ALIASsym) TypeAlias(tp1)
+              else if (denot.isType) checkNonCyclic(denot.symbol, tp1, reportErrors = false)
+                // we need the checkNonCyclic call to insert LazyRefs for F-bounded cycles
+              else tp1
             if (denot.isConstructor) addConstructorTypeParams(denot)
             if (atEnd) {
               assert(!(denot is SuperAccessor), denot)
diff --git a/src/dotty/tools/dotc/typer/Checking.scala b/src/dotty/tools/dotc/typer/Checking.scala
index cd2e9b55f..eb202cc15 100644
--- a/src/dotty/tools/dotc/typer/Checking.scala
+++ b/src/dotty/tools/dotc/typer/Checking.scala
@@ -58,7 +58,7 @@ object Checking {
   /** A type map which checks that the only cycles in a type are F-bounds
    *  and that protects all F-bounded references by LazyRefs.
    */
-  class CheckNonCyclicMap(implicit ctx: Context) extends TypeMap {
+  class CheckNonCyclicMap(sym: Symbol, reportErrors: Boolean)(implicit ctx: Context) extends TypeMap {
 
     /** Are cycles allowed within nested refinedInfos of currently checked type? */
     private var nestedCycleOK = false
@@ -72,6 +72,9 @@ object Checking {
      */
     var where: String = ""
 
+    /** The last type top-level type checked when a CyclicReference occurs. */
+    var lastChecked: Type = NoType
+
     /** Check info `tp` for cycles. Throw CyclicReference for illegal cycles,
      *  break direct cycle with a LazyRef for legal, F-bounded cycles.
      */
@@ -79,15 +82,22 @@ object Checking {
       case tp @ TypeBounds(lo, hi) =>
         if (lo eq hi)
           try tp.derivedTypeAlias(apply(lo))
-          finally where = "alias"
+          finally {
+            where = "alias"
+            lastChecked = lo
+          }
         else {
-          val lo1 = try apply(lo) finally where = "lower bound"
+          val lo1 = try apply(lo) finally {
+            where = "lower bound"
+            lastChecked = lo
+          }
           val saved = nestedCycleOK
           nestedCycleOK = true
           try tp.derivedTypeBounds(lo1, apply(hi))
           finally {
             nestedCycleOK = saved
             where = "upper bound"
+            lastChecked = hi
           }
         }
       case _ =>
@@ -103,44 +113,66 @@ object Checking {
         finally cycleOK = saved
       case tp @ TypeRef(pre, name) =>
         try {
-          // Check info of typeref recursively, marking the referred symbol
+          // A prefix is interesting if it might contain (transitively) a reference
+          // to symbol `sym` itself. We only check references with interesting
+          // prefixes for cycles. This pruning is done in order not to force
+          // global symbols when doing the cyclicity check.
+          def isInteresting(prefix: Type): Boolean = prefix.stripTypeVar match {
+            case NoPrefix => true
+            case ThisType(cls) => sym.owner.isClass && cls.isContainedIn(sym.owner)
+            case prefix: NamedType => !prefix.symbol.isStaticOwner && isInteresting(prefix.prefix)
+            case SuperType(thistp, _) => isInteresting(thistp)
+            case AndType(tp1, tp2) => isInteresting(tp1) || isInteresting(tp2)
+            case OrType(tp1, tp2) => isInteresting(tp1) && isInteresting(tp2)
+            case _ => false
+          }
+          // If prefix is interesting, check info of typeref recursively, marking the referred symbol
           // with NoCompleter. This provokes a CyclicReference when the symbol
           // is hit again. Without this precaution we could stackoverflow here.
-          val info = tp.info
-          val symInfo = tp.symbol.info
-          if (tp.symbol.exists) tp.symbol.info = SymDenotations.NoCompleter
-          try checkInfo(info)
-          finally if (tp.symbol.exists) tp.symbol.info = symInfo
+          if (isInteresting(pre)) {
+            val info = tp.info
+            val symInfo = tp.symbol.info
+            if (tp.symbol.exists) tp.symbol.info = SymDenotations.NoCompleter
+            try checkInfo(info)
+            finally if (tp.symbol.exists) tp.symbol.info = symInfo
+          }
           tp
         } catch {
           case ex: CyclicReference =>
             ctx.debuglog(i"cycle detected for $tp, $nestedCycleOK, $cycleOK")
-            if (cycleOK) LazyRef(() => tp) else throw ex
+            if (cycleOK) LazyRef(() => tp)
+            else if (reportErrors) throw ex
+            else tp
         }
       case _ => mapOver(tp)
     }
   }
-}
-
-trait Checking {
-
-  import tpd._
-  import Checking._
 
   /** Check that `info` of symbol `sym` is not cyclic.
    *  @pre     sym is not yet initialized (i.e. its type is a Completer).
    *  @return  `info` where every legal F-bounded reference is proctected
    *                  by a `LazyRef`, or `ErrorType` if a cycle was detected and reported.
    */
-  def checkNonCyclic(sym: Symbol, info: TypeBounds)(implicit ctx: Context): Type = {
-    val checker = new CheckNonCyclicMap
+  def checkNonCyclic(sym: Symbol, info: Type, reportErrors: Boolean)(implicit ctx: Context): Type = {
+    val checker = new CheckNonCyclicMap(sym, reportErrors)(ctx.withMode(Mode.CheckCyclic))
     try checker.checkInfo(info)
     catch {
       case ex: CyclicReference =>
-        ctx.error(i"illegal cyclic reference: ${checker.where} $info of $sym refers back to the type itself", sym.pos)
-        ErrorType
-      }
+        if (reportErrors) {
+          ctx.error(i"illegal cyclic reference: ${checker.where} ${checker.lastChecked} of $sym refers back to the type itself", sym.pos)
+          ErrorType
+        }
+        else info
+    }
   }
+}
+
+trait Checking {
+
+  import tpd._
+
+  def checkNonCyclic(sym: Symbol, info: TypeBounds, reportErrors: Boolean)(implicit ctx: Context): Type =
+    Checking.checkNonCyclic(sym, info, reportErrors)
 
   /** Check that Java statics and packages can only be used in selections.
    */
@@ -252,6 +284,7 @@ trait Checking {
 
 trait NoChecking extends Checking {
   import tpd._
+  override def checkNonCyclic(sym: Symbol, info: TypeBounds, reportErrors: Boolean)(implicit ctx: Context): Type = info
   override def checkValue(tree: Tree, proto: Type)(implicit ctx: Context): tree.type = tree
   override def checkBounds(args: List[tpd.Tree], poly: PolyType, pos: Position)(implicit ctx: Context): Unit = ()
   override def checkStable(tp: Type, pos: Position)(implicit ctx: Context): Unit = ()
diff --git a/src/dotty/tools/dotc/typer/Mode.scala b/src/dotty/tools/dotc/typer/Mode.scala
index 55baa6bc5..54487722a 100644
--- a/src/dotty/tools/dotc/typer/Mode.scala
+++ b/src/dotty/tools/dotc/typer/Mode.scala
@@ -33,6 +33,7 @@ object Mode {
 
   val TypevarsMissContext = newMode(4, "TypevarsMissContext")
   val InSuperCall = newMode(5, "InSuperCall")
+  val CheckCyclic = newMode(6, "CheckCyclic")
 
   val PatternOrType = Pattern | Type
 }
\ No newline at end of file
diff --git a/src/dotty/tools/dotc/typer/Namer.scala b/src/dotty/tools/dotc/typer/Namer.scala
index afa270d46..1002abe4d 100644
--- a/src/dotty/tools/dotc/typer/Namer.scala
+++ b/src/dotty/tools/dotc/typer/Namer.scala
@@ -679,6 +679,6 @@ class Namer { typer: Typer =>
       case _ => TypeAlias(abstractedRhsType, if (sym is Local) sym.variance else 0)
     }
     sym.info = NoCompleter
-    checkNonCyclic(sym, unsafeInfo)
+    checkNonCyclic(sym, unsafeInfo, reportErrors = true)
   }
 }
\ No newline at end of file