Better error diagnostics for cyclic references

We now suggest to compile with -explain-cyclic, in which case we
give a trace of the forcings that led to the cycle.

The reason for the separate option is that maintaining a trace is not
free so we should not be doing it by default.

Author: odersky

compiler/src/dotty/tools/dotc/Run.scala

@@ -343,6 +343,8 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint
     runCtx.setProfiler(Profiler())
     unfusedPhases.foreach(_.initContext(runCtx))
     val fusedPhases = runCtx.base.allPhases
+    if ctx.settings.explainCyclic.value then
+      runCtx.setProperty(CyclicReference.Trace, new CyclicReference.Trace())
     runCtx.withProgressCallback: cb =>
       _progress = Progress(cb, this, fusedPhases.map(_.traversals).sum)
     runPhases(allPhases = fusedPhases)(using runCtx)

compiler/src/dotty/tools/dotc/config/ScalaSettings.scala

@@ -120,6 +120,7 @@ trait CommonScalaSettings:
   // -explain-types setting is necessary for cross compilation, since it is mentioned in sbt-tpolecat, for instance
   // it is otherwise subsumed by -explain, and should be dropped as soon as we can.
   val explainTypes: Setting[Boolean] = BooleanSetting("-explain-types", "Explain type errors in more detail (deprecated, use -explain instead).", aliases = List("--explain-types", "-explaintypes"))
+  val explainCyclic: Setting[Boolean] = BooleanSetting("-explain-cyclic", "Explain cyclic reference errors in more detail.", aliases = List("--explain-cyclic"))
   val unchecked: Setting[Boolean] = BooleanSetting("-unchecked", "Enable additional warnings where generated code depends on assumptions.", initialValue = true, aliases = List("--unchecked"))
   val language: Setting[List[String]] = MultiStringSetting("-language", "feature", "Enable one or more language features.", aliases = List("--language"))
   val experimental: Setting[Boolean] = BooleanSetting("-experimental", "Annotate all top-level definitions with @experimental. This enables the use of experimental features anywhere in the project.")
@@ -351,6 +352,7 @@ private sealed trait YSettings:
   val YdebugTypeError: Setting[Boolean] = BooleanSetting("-Ydebug-type-error", "Print the stack trace when a TypeError is caught", false)
   val YdebugError: Setting[Boolean] = BooleanSetting("-Ydebug-error", "Print the stack trace when any error is caught.", false)
   val YdebugUnpickling: Setting[Boolean] = BooleanSetting("-Ydebug-unpickling", "Print the stack trace when an error occurs when reading Tasty.", false)
+  val YdebugCyclic: Setting[Boolean] = BooleanSetting("-Ydebug-cyclic", "Print the stack trace when a cyclic reference error occurs.", false)
   val YtermConflict: Setting[String] = ChoiceSetting("-Yresolve-term-conflict", "strategy", "Resolve term conflicts", List("package", "object", "error"), "error")
   val Ylog: Setting[List[String]] = PhasesSetting("-Ylog", "Log operations during")
   val YlogClasspath: Setting[Boolean] = BooleanSetting("-Ylog-classpath", "Output information about what classpath is being applied.")

compiler/src/dotty/tools/dotc/core/SymDenotations.scala

@@ -167,12 +167,17 @@ object SymDenotations {
             println(i"${"  " * indent}completed $name in $owner")
           }
         }
-        else {
-          if (myFlags.is(Touched))
-            throw CyclicReference(this)(using ctx.withOwner(symbol))
-          myFlags |= Touched
-          atPhase(validFor.firstPhaseId)(completer.complete(this))
-        }
+        else
+          val traceCycles = CyclicReference.isTraced
+          try
+            if traceCycles then
+              CyclicReference.pushTrace("complete the info of ", symbol, "")
+            if myFlags.is(Touched) then
+              throw CyclicReference(this)(using ctx.withOwner(symbol))
+            myFlags |= Touched
+            atPhase(validFor.firstPhaseId)(completer.complete(this))
+          finally
+            if traceCycles then CyclicReference.popTrace()
 
     protected[dotc] def info_=(tp: Type): Unit = {
       /* // DEBUG
@@ -2971,7 +2976,10 @@ object SymDenotations {
     def apply(clsd: ClassDenotation)(implicit onBehalf: BaseData, ctx: Context)
         : (List[ClassSymbol], BaseClassSet) = {
       assert(isValid)
+      val traceCycles = CyclicReference.isTraced
       try
+        if traceCycles then
+          CyclicReference.pushTrace("compute the base classes of ", clsd.symbol, "")
         if (cache != null) cache.uncheckedNN
         else {
           if (locked) throw CyclicReference(clsd)
@@ -2984,7 +2992,9 @@ object SymDenotations {
           else onBehalf.signalProvisional()
           computed
         }
-      finally addDependent(onBehalf)
+      finally
+        if traceCycles then CyclicReference.popTrace()
+        addDependent(onBehalf)
     }
 
     def sameGroup(p1: Phase, p2: Phase) = p1.sameParentsStartId == p2.sameParentsStartId

compiler/src/dotty/tools/dotc/core/TypeErrors.scala

@@ -12,7 +12,9 @@ import Denotations.*
 import Decorators.*
 import reporting.*
 import ast.untpd
+import util.Property
 import config.Printers.{cyclicErrors, noPrinter}
+import collection.mutable
 
 import scala.annotation.constructorOnly
 
@@ -27,6 +29,7 @@ abstract class TypeError(using creationContext: Context) extends Exception(""):
     || ctx.settings.YdebugTypeError.value
     || ctx.settings.YdebugError.value
     || ctx.settings.YdebugUnpickling.value
+    || ctx.settings.YdebugCyclic.value
 
   override def fillInStackTrace(): Throwable =
     if computeStackTrace then super.fillInStackTrace().nn
@@ -72,8 +75,7 @@ extends TypeError:
   def explanation: String = s"$op $details"
 
   private def recursions: List[RecursionOverflow] = {
-    import scala.collection.mutable.ListBuffer
-    val result = ListBuffer.empty[RecursionOverflow]
+    val result = mutable.ListBuffer.empty[RecursionOverflow]
     @annotation.tailrec def loop(throwable: Throwable): List[RecursionOverflow] = throwable match {
       case ro: RecursionOverflow =>
         result += ro
@@ -135,7 +137,10 @@ end handleRecursive
  * so it requires knowing denot already.
  * @param denot
  */
-class CyclicReference(val denot: SymDenotation)(using Context) extends TypeError:
+class CyclicReference(
+    val denot: SymDenotation,
+    val optTrace: Option[Array[CyclicReference.TraceElement]])(using Context)
+extends TypeError:
   var inImplicitSearch: Boolean = false
 
   val cycleSym = denot.symbol
@@ -161,11 +166,11 @@ class CyclicReference(val denot: SymDenotation)(using Context) extends TypeError
         cx.tree match {
           case tree: untpd.ValOrDefDef if !tree.tpt.typeOpt.exists =>
             if (inImplicitSearch)
-              TermMemberNeedsResultTypeForImplicitSearch(cycleSym)
+              TermMemberNeedsResultTypeForImplicitSearch(this)
             else if (isMethod)
-              OverloadedOrRecursiveMethodNeedsResultType(cycleSym)
+              OverloadedOrRecursiveMethodNeedsResultType(this)
             else if (isVal)
-              RecursiveValueNeedsResultType(cycleSym)
+              RecursiveValueNeedsResultType(this)
             else
               errorMsg(cx.outer)
           case _ =>
@@ -174,22 +179,38 @@ class CyclicReference(val denot: SymDenotation)(using Context) extends TypeError
 
       // Give up and give generic errors.
       else if (cycleSym.isOneOf(GivenOrImplicitVal, butNot = Method) && cycleSym.owner.isTerm)
-        CyclicReferenceInvolvingImplicit(cycleSym)
+        CyclicReferenceInvolvingImplicit(this)
       else
-        CyclicReferenceInvolving(denot)
+        CyclicReferenceInvolving(this)
 
     errorMsg(ctx)
   end toMessage
 
 object CyclicReference:
+
   def apply(denot: SymDenotation)(using Context): CyclicReference =
-    val ex = new CyclicReference(denot)
+    val ex = new CyclicReference(denot, ctx.property(Trace).map(_.toArray))
     if ex.computeStackTrace then
       cyclicErrors.println(s"Cyclic reference involving $denot")
       val sts = ex.getStackTrace.asInstanceOf[Array[StackTraceElement]]
       for (elem <- sts take 200)
         cyclicErrors.println(elem.toString)
     ex
+
+  type TraceElement = (/*prefix:*/ String, Symbol, /*suffix:*/ String)
+  type Trace = mutable.ArrayBuffer[TraceElement]
+  val Trace = Property.Key[Trace]
+
+  def isTraced(using Context) =
+    ctx.property(CyclicReference.Trace).isDefined
+
+  def pushTrace(info: TraceElement)(using Context): Unit =
+    for buf <- ctx.property(CyclicReference.Trace) do
+      buf += info
+
+  def popTrace()(using Context): Unit =
+    for buf <- ctx.property(CyclicReference.Trace) do
+      buf.dropRightInPlace(1)
 end CyclicReference
 
 class UnpicklingError(denot: Denotation, where: String, cause: Throwable)(using Context) extends TypeError:

compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala

@@ -141,20 +141,25 @@ class TreeUnpickler(reader: TastyReader,
     val mode = ctx.mode
     val source = ctx.source
     def complete(denot: SymDenotation)(using Context): Unit =
-      def fail(ex: Throwable) =
-        def where =
-          val f = denot.symbol.associatedFile
-          if f == null then "" else s" in $f"
-        throw UnpicklingError(denot, where, ex)
+      def where =
+        val f = denot.symbol.associatedFile
+        if f == null then "" else s" in $f"
+      def fail(ex: Throwable) = throw UnpicklingError(denot, where, ex)
       treeAtAddr(currentAddr) =
+        val traceCycles = CyclicReference.isTraced
         try
+          if traceCycles then
+            CyclicReference.pushTrace("read the definition of ", denot.symbol, where)
           atPhaseBeforeTransforms {
             new TreeReader(reader).readIndexedDef()(
               using ctx.withOwner(owner).withModeBits(mode).withSource(source))
           }
         catch
+          case ex: CyclicReference => throw ex
           case ex: AssertionError => fail(ex)
           case ex: Exception => fail(ex)
+        finally
+          if traceCycles then CyclicReference.popTrace()
   }
 
   class TreeReader(val reader: TastyReader) {

compiler/src/dotty/tools/dotc/reporting/messages.scala

@@ -84,6 +84,23 @@ abstract class PatternMatchMsg(errorId: ErrorMessageID)(using Context) extends M
 abstract class CyclicMsg(errorId: ErrorMessageID)(using Context) extends Message(errorId):
   def kind = MessageKind.Cyclic
 
+  val ex: CyclicReference
+  protected def cycleSym = ex.denot.symbol
+
+  protected def debugInfo =
+    if ctx.settings.YdebugCyclic.value then
+      "\n\nStacktrace:" ++ ex.getStackTrace().nn.mkString("\n    ", "\n    ", "")
+    else "\n\n Run with both -explain-cyclic and -Ydebug-cyclic to see full stack trace."
+
+  protected def context: String = ex.optTrace match
+    case Some(trace) =>
+      s"\n\nThe error occurred while trying to ${
+        trace.map((prefix, sym, suffix) => i"$prefix$sym$suffix").mkString("\n  which required to ")
+      }$debugInfo"
+    case None =>
+      "\n\n Run with -explain-cyclic for more details."
+end CyclicMsg
+
 abstract class ReferenceMsg(errorId: ErrorMessageID)(using Context) extends Message(errorId):
   def kind = MessageKind.Reference
 
@@ -1249,9 +1266,9 @@ class UnreducibleApplication(tycon: Type)(using Context) extends TypeMsg(Unreduc
         |Such applications are equivalent to existential types, which are not
         |supported in Scala 3."""
 
-class OverloadedOrRecursiveMethodNeedsResultType(cycleSym: Symbol)(using Context)
+class OverloadedOrRecursiveMethodNeedsResultType(val ex: CyclicReference)(using Context)
 extends CyclicMsg(OverloadedOrRecursiveMethodNeedsResultTypeID) {
-  def msg(using Context) = i"""Overloaded or recursive $cycleSym needs return type"""
+  def msg(using Context) = i"""Overloaded or recursive $cycleSym needs return type$context"""
   def explain(using Context) =
     i"""Case 1: $cycleSym is overloaded
         |If there are multiple methods named $cycleSym and at least one definition of
@@ -1263,29 +1280,29 @@ extends CyclicMsg(OverloadedOrRecursiveMethodNeedsResultTypeID) {
         |"""
 }
 
-class RecursiveValueNeedsResultType(cycleSym: Symbol)(using Context)
+class RecursiveValueNeedsResultType(val ex: CyclicReference)(using Context)
 extends CyclicMsg(RecursiveValueNeedsResultTypeID) {
-  def msg(using Context) = i"""Recursive $cycleSym needs type"""
+  def msg(using Context) = i"""Recursive $cycleSym needs type$context"""
   def explain(using Context) =
     i"""The definition of $cycleSym is recursive and you need to specify its type.
         |"""
 }
 
-class CyclicReferenceInvolving(denot: SymDenotation)(using Context)
+class CyclicReferenceInvolving(val ex: CyclicReference)(using Context)
 extends CyclicMsg(CyclicReferenceInvolvingID) {
   def msg(using Context) =
-    val where = if denot.exists then s" involving $denot" else ""
-    i"Cyclic reference$where"
+    val where = if ex.denot.exists then s" involving ${ex.denot}" else ""
+    i"Cyclic reference$where$context"
   def explain(using Context) =
-    i"""|$denot is declared as part of a cycle which makes it impossible for the
-        |compiler to decide upon ${denot.name}'s type.
-        |To avoid this error, try giving ${denot.name} an explicit type.
+    i"""|${ex.denot} is declared as part of a cycle which makes it impossible for the
+        |compiler to decide upon ${ex.denot.name}'s type.
+        |To avoid this error, try giving ${ex.denot.name} an explicit type.
         |"""
 }
 
-class CyclicReferenceInvolvingImplicit(cycleSym: Symbol)(using Context)
+class CyclicReferenceInvolvingImplicit(val ex: CyclicReference)(using Context)
 extends CyclicMsg(CyclicReferenceInvolvingImplicitID) {
-  def msg(using Context) = i"""Cyclic reference involving implicit $cycleSym"""
+  def msg(using Context) = i"""Cyclic reference involving implicit $cycleSym$context"""
   def explain(using Context) =
     i"""|$cycleSym is declared as part of a cycle which makes it impossible for the
         |compiler to decide upon ${cycleSym.name}'s type.
@@ -2340,9 +2357,9 @@ class TypeTestAlwaysDiverges(scrutTp: Type, testTp: Type)(using Context) extends
 }
 
 // Relative of CyclicReferenceInvolvingImplicit and RecursiveValueNeedsResultType
-class TermMemberNeedsResultTypeForImplicitSearch(cycleSym: Symbol)(using Context)
+class TermMemberNeedsResultTypeForImplicitSearch(val ex: CyclicReference)(using Context)
   extends CyclicMsg(TermMemberNeedsNeedsResultTypeForImplicitSearchID) {
-  def msg(using Context) = i"""$cycleSym needs result type because its right-hand side attempts implicit search"""
+  def msg(using Context) = i"""$cycleSym needs result type because its right-hand side attempts implicit search$context"""
   def explain(using Context) =
     i"""|The right hand-side of $cycleSym's definition requires an implicit search at the highlighted position.
         |To avoid this error, give `$cycleSym` an explicit type.
@@ -2553,8 +2570,9 @@ class UnknownNamedEnclosingClassOrObject(name: TypeName)(using Context)
     """
   }
 
-class IllegalCyclicTypeReference(sym: Symbol, where: String, lastChecked: Type)(using Context)
+class IllegalCyclicTypeReference(val ex: CyclicReference, sym: Symbol, where: String, lastChecked: Type)(using Context)
   extends CyclicMsg(IllegalCyclicTypeReferenceID) {
+  override def context = ""
   def msg(using Context) =
     val lastCheckedStr =
       try lastChecked.show

compiler/src/dotty/tools/dotc/typer/Checking.scala

@@ -391,7 +391,7 @@ object Checking {
     catch {
       case ex: CyclicReference =>
         if (reportErrors)
-          errorType(IllegalCyclicTypeReference(sym, checker.where, checker.lastChecked), sym.srcPos)
+          errorType(IllegalCyclicTypeReference(ex, sym, checker.where, checker.lastChecked), sym.srcPos)
         else info
     }
   }

tests/neg-macros/i14772.check

@@ -1,6 +1,11 @@
--- [E044] Cyclic Error: tests/neg-macros/i14772.scala:7:7 --------------------------------------------------------------
-7 |    foo(a)    // error
+-- [E044] Cyclic Error: tests/neg-macros/i14772.scala:8:7 --------------------------------------------------------------
+8 |    foo(a)    // error
   |       ^
   |       Overloaded or recursive method impl needs return type
   |
+  |       The error occurred while trying to complete the info of method $anonfun
+  |         which required to complete the info of method impl
+  |
+  |        Run with both -explain-cyclic and -Ydebug-cyclic to see full stack trace.
+  |
   | longer explanation available when compiling with `-explain`

tests/neg-macros/i14772.scala

@@ -1,3 +1,4 @@
+//> using options -explain-cyclic
 import scala.quoted.*
 
 object A {

tests/neg-macros/i16582.check

@@ -1,10 +1,16 @@
 
--- Error: tests/neg-macros/i16582/Test_2.scala:5:27 --------------------------------------------------------------------
-5 |  val o2 = ownerDoesNotWork(2) // error
+-- Error: tests/neg-macros/i16582/Test_2.scala:6:27 --------------------------------------------------------------------
+6 |  val o2 = ownerDoesNotWork(2) // error
   |           ^^^^^^^^^^^^^^^^^^^
   |           Exception occurred while executing macro expansion.
   |           dotty.tools.dotc.core.CyclicReference: Recursive value o2 needs type
   |
+  |           The error occurred while trying to complete the info of method test
+  |             which required to complete the info of value o2
+  |             which required to complete the info of value o2
+  |
+  |            Run with both -explain-cyclic and -Ydebug-cyclic to see full stack trace.
+  |
   |           See full stack trace using -Ydebug
   |---------------------------------------------------------------------------------------------------------------------
   |Inline stack trace

tests/neg-macros/i16582/Test_2.scala

@@ -1,3 +1,4 @@
+//> using options -explain-cyclic
 def test=
   val o1 = ownerWorks(1)
   println(o1)

tests/neg/cyclic.check

@@ -0,0 +1,14 @@
+-- [E044] Cyclic Error: tests/neg/cyclic.scala:6:12 --------------------------------------------------------------------
+6 |  def i() = f()  // error
+  |            ^
+  |            Overloaded or recursive method f needs return type
+  |
+  |            The error occurred while trying to complete the info of method f
+  |              which required to complete the info of method g
+  |              which required to complete the info of method h
+  |              which required to complete the info of method i
+  |              which required to complete the info of method f
+  |
+  |             Run with both -explain-cyclic and -Ydebug-cyclic to see full stack trace.
+  |
+  | longer explanation available when compiling with `-explain`

tests/neg/cyclic.scala

@@ -0,0 +1,6 @@
+//> using options -explain-cyclic
+object O:
+  def f() = g()
+  def g() = h()
+  def h() = i()
+  def i() = f()  // error

tests/neg/i10870.check

@@ -5,3 +5,5 @@
    |            Extension methods were tried, but the search failed with:
    |
    |                Overloaded or recursive method x needs return type
+   |                
+   |                 Run with -explain-cyclic for more details.