-Vcyclic for CyclicRef trace locked symbols

Author: som-snytt

src/compiler/scala/tools/nsc/Global.scala

@@ -90,6 +90,8 @@ class Global(var currentSettings: Settings, reporter0: Reporter)
 
   override def settings = currentSettings
 
+  override def isSymbolLockTracingEnabled = settings.cyclic.value
+
   private[this] var currentReporter: FilteringReporter = null
   locally { reporter = reporter0 }
 

src/compiler/scala/tools/nsc/settings/ScalaSettings.scala

@@ -563,6 +563,7 @@ trait ScalaSettings extends StandardScalaSettings with Warnings { _: MutableSett
    */
   val Vhelp              = BooleanSetting("-V", "Print a synopsis of verbose options.")
   val browse          = PhasesSetting("-Vbrowse", "Browse the abstract syntax tree after") withAbbreviation "-Ybrowse"
+  val cyclic          = BooleanSetting("-Vcyclic", "Debug cyclic reference error.")
   val debug           = BooleanSetting("-Vdebug", "Increase the quantity of debugging output.") withAbbreviation "-Ydebug" withPostSetHook (s => if (s.value) StatisticsStatics.enableDebugAndDeoptimize())
   val YdebugTasty     = BooleanSetting("-Vdebug-tasty", "Increase the quantity of debugging output when unpickling tasty.") withAbbreviation "-Ydebug-tasty"
   val VdebugTypeError = BooleanSetting("-Vdebug-type-error", "Print the stack trace when any error is caught.") withAbbreviation "-Ydebug-type-error"

src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala

@@ -1340,7 +1340,7 @@ trait ContextErrors extends splain.SplainErrors {
 
       def TypeSigError(tree: Tree, ex: TypeError) = {
         ex match {
-          case CyclicReference(_, _) if tree.symbol.isTermMacro =>
+          case CyclicReference(_, _, _) if tree.symbol.isTermMacro =>
             // say, we have a macro def `foo` and its macro impl `impl`
             // if impl: 1) omits return type, 2) has anything implicit in its body, 3) sees foo
             //
@@ -1353,8 +1353,8 @@ trait ContextErrors extends splain.SplainErrors {
             // hence we (together with reportTypeError in TypeDiagnostics) make sure that this CyclicReference
             // evades all the handlers on its way and successfully reaches `isCyclicOrErroneous` in Implicits
             throw ex
-          case CyclicReference(sym, info: TypeCompleter) =>
-            issueNormalTypeError(tree, typer.cyclicReferenceMessage(sym, info.tree) getOrElse ex.getMessage)
+          case CyclicReference(sym, info: TypeCompleter, trace) =>
+            issueNormalTypeError(tree, typer.cyclicReferenceMessage(sym, info.tree, trace, tree.pos).getOrElse(ex.getMessage))
           case _ =>
             contextNamerErrorGen.issue(TypeErrorWithUnderlyingTree(tree, ex))
         }

src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala

@@ -840,19 +840,29 @@ trait TypeDiagnostics extends splain.SplainDiagnostics {
     def permanentlyHiddenWarning(pos: Position, hidden: Name, defn: Symbol) =
       context.warning(pos, "imported `%s` is permanently hidden by definition of %s".format(hidden, defn.fullLocationString), WarningCategory.OtherShadowing)
 
-    private def symWasOverloaded(sym: Symbol) = sym.owner.isClass && sym.owner.info.member(sym.name).isOverloaded
-    private def cyclicAdjective(sym: Symbol)  = if (symWasOverloaded(sym)) "overloaded" else "recursive"
-
     /** Returns Some(msg) if the given tree is untyped apparently due
      *  to a cyclic reference, and None otherwise.
      */
-    def cyclicReferenceMessage(sym: Symbol, tree: Tree) = condOpt(tree) {
-      case ValDef(_, _, TypeTree(), _)       => s"recursive $sym needs type"
-      case DefDef(_, _, _, _, TypeTree(), _) => s"${cyclicAdjective(sym)} $sym needs result type"
-      case Import(expr, selectors)           =>
-        """encountered unrecoverable cycle resolving import.
-          |Note: this is often due in part to a class depending on a definition nested within its companion.
-          |If applicable, you may wish to try moving some members into another object.""".stripMargin
+    def cyclicReferenceMessage(sym: Symbol, tree: Tree, trace: Array[Symbol], pos: Position) = {
+      def symWasOverloaded(sym: Symbol) = sym.owner.isClass && sym.owner.info.member(sym.name).isOverloaded
+      def cyclicAdjective(sym: Symbol)  = if (symWasOverloaded(sym)) "overloaded" else "recursive"
+
+      val badsym = if (!sym.isSynthetic) sym else {
+        val organics = trace.filter(!_.isSynthetic)
+        if (organics.length == 0) sym
+        else if (organics.length == 1) organics(0)
+        else organics.find(_.pos.focus == pos.focus).getOrElse(organics(0))
+      }
+      def help = if (!badsym.isSynthetic || settings.cyclic.value) "" else
+        s"; $badsym is synthetic; use -Vcyclic to find which definition needs an explicit type"
+      condOpt(tree) {
+        case ValDef(_, _, TypeTree(), _)       => s"recursive $badsym needs type$help"
+        case DefDef(_, _, _, _, TypeTree(), _) => s"${cyclicAdjective(badsym)} $badsym needs result type$help"
+        case Import(_, _)                      =>
+          sm"""encountered unrecoverable cycle resolving import.
+              |Note: this is often due in part to a class depending on a definition nested within its companion.
+              |If applicable, you may wish to try moving some members into another object."""
+      }
     }
 
     // warn about class/method/type-members' type parameters that shadow types already in scope
@@ -887,20 +897,16 @@ trait TypeDiagnostics extends splain.SplainDiagnostics {
       if (settings.isDebug) ex.printStackTrace()
 
       ex match {
-        case CyclicReference(sym, info: TypeCompleter) =>
-          if (context0.owner.isTermMacro) {
-            // see comments to TypeSigError for an explanation of this special case
-            throw ex
-          } else {
-            val pos = info.tree match {
-              case Import(expr, _)  => expr.pos
-              case _                => ex.pos
-            }
-            context0.error(pos, cyclicReferenceMessage(sym, info.tree) getOrElse ex.getMessage())
-
-            if (sym == ObjectClass)
-              throw new FatalError("cannot redefine root "+sym)
+        // see comments to TypeSigError for an explanation of this special case
+        case _: CyclicReference if context0.owner.isTermMacro => throw ex
+        case CyclicReference(sym, info: TypeCompleter, trace) =>
+          val pos = info.tree match {
+            case Import(expr, _)  => expr.pos
+            case _                => ex.pos
           }
+          context0.error(pos, cyclicReferenceMessage(sym, info.tree, trace, pos).getOrElse(ex.getMessage))
+
+          if (sym == ObjectClass) throw new FatalError(s"cannot redefine root $sym")
         case _ =>
           context0.error(ex.pos, ex.msg)
       }

src/compiler/scala/tools/nsc/typechecker/Typers.scala

@@ -6306,7 +6306,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
         case ex: TypeError =>
           tree.clearType()
           // The only problematic case are (recoverable) cyclic reference errors which can pop up almost anywhere.
-          typingStack.printTyping(tree, "caught %s: while typing %s".format(ex, tree)) //DEBUG
+          typingStack.printTyping(tree, s"caught $ex: while typing $tree")
           reportTypeError(context, tree.pos, ex)
           setError(tree)
         case ex: Exception =>

src/reflect/scala/reflect/internal/SymbolTable.scala

@@ -95,6 +95,8 @@ abstract class SymbolTable extends macros.Universe
 
   def settings: MutableSettings
 
+  def isSymbolLockTracingEnabled: Boolean = isDeveloper
+
   /** Override with final implementation for inlining. */
   def debuglog(msg:  => String): Unit = if (settings.isDebug) log(msg)
 

src/reflect/scala/reflect/internal/Symbols.scala

@@ -18,12 +18,12 @@ package scala
 package reflect
 package internal
 
-import scala.collection.immutable
-import scala.collection.mutable.ListBuffer
-import util.{ ReusableInstance, Statistics, shortClassOfInstance }
-import Flags._
 import scala.annotation.tailrec
+import scala.collection.mutable.{ArrayBuffer, ListBuffer}
 import scala.reflect.io.{AbstractFile, NoAbstractFile}
+
+import util.{ReusableInstance, Statistics, shortClassOfInstance}
+import Flags._
 import Variance._
 
 trait Symbols extends api.Symbols { self: SymbolTable =>
@@ -36,14 +36,16 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
   protected def nextId() = { ids += 1; ids }
 
   /** Used to keep track of the recursion depth on locked symbols */
-  private[this] var _recursionTable = immutable.Map.empty[Symbol, Int]
+  private[this] var _recursionTable = Map.empty[Symbol, Int]
   def recursionTable = _recursionTable
-  def recursionTable_=(value: immutable.Map[Symbol, Int]) = _recursionTable = value
+  def recursionTable_=(value: Map[Symbol, Int]) = _recursionTable = value
 
   private[this] var _lockedCount = 0
   def lockedCount = this._lockedCount
   def lockedCount_=(i: Int) = _lockedCount = i
 
+  private[this] val _lockingTrace = ArrayBuffer.empty[Symbol]
+  private[this] val lockTracing: Boolean = self.isSymbolLockTracingEnabled
 
   @deprecated("Global existential IDs no longer used", "2.12.1")
   private[this] var existentialIds = 0
@@ -553,19 +555,23 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     // True if the symbol is unlocked.
     // True if the symbol is locked but still below the allowed recursion depth.
     // False otherwise
-    private[scala] def lockOK: Boolean = {
-      ((_rawflags & LOCKED) == 0L) ||
-      ((settings.Yrecursion.value != 0) &&
-       (recursionTable get this match {
-         case Some(n) => (n <= settings.Yrecursion.value)
-         case None => true }))
-    }
+    private[scala] def lockOK: Boolean = (
+      (_rawflags & LOCKED) == 0L || {
+        val limit = settings.Yrecursion.value
+        limit != 0 && (
+        recursionTable.get(this) match {
+          case Some(n) => n <= limit
+          case None => true
+        })
+      }
+    )
 
     // Lock a symbol, using the handler if the recursion depth becomes too great.
     private[scala] def lock(handler: => Unit): Boolean = {
+      if (lockTracing) _lockingTrace.addOne(this)
       if ((_rawflags & LOCKED) != 0L) {
         if (settings.Yrecursion.value != 0) {
-          recursionTable get this match {
+          recursionTable.get(this) match {
             case Some(n) =>
               if (n > settings.Yrecursion.value) {
                 handler
@@ -578,21 +584,25 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
               recursionTable += (this -> 1)
               true
           }
-        } else { handler; false }
+        } else {
+          handler
+          false
+        }
       } else {
         _rawflags |= LOCKED
         true
       }
     }
 
     // Unlock a symbol
-    private[scala] def unlock() = {
+    private[scala] def unlock(): Unit =
       if ((_rawflags & LOCKED) != 0L) {
         _rawflags &= ~LOCKED
+        if (lockTracing && !_lockingTrace.isEmpty)
+          _lockingTrace.remove(index = _lockingTrace.size - 1, count = 1) // dropRightInPlace(1)
         if (settings.Yrecursion.value != 0)
           recursionTable -= this
       }
-    }
 
 // ----- tests ----------------------------------------------------------------------
 
@@ -1553,9 +1563,16 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
       if ((_rawflags & LOCKED) != 0L) { // rolled out once for performance
         lock {
           setInfo(ErrorType)
-          throw CyclicReference(this, tp)
+          val trace =
+            if (lockTracing) {
+              val t = _lockingTrace.toArray
+              _lockingTrace.clear()
+              t
+            } else CyclicReference.emptyTrace
+          throw CyclicReference(this, tp, trace)
         }
       } else {
+        if (lockTracing) _lockingTrace.addOne(this)
         _rawflags |= LOCKED
       }
       val current = phase
@@ -1568,18 +1585,15 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
           unlock()
           phase = current
         }
-      } else {
-          // In runtime reflection, there is only on phase, so don't mutate Global.phase which would lead to warnings
-        // of data races from when using TSAN to assess thread safety.
-        try {
-          tp.complete(this)
-        } finally {
-          unlock()
-        }
       }
+      else
+        // In runtime reflection, there is only one phase, so don't mutate Global.phase
+        // which would lead to warnings of data races from when using TSAN to assess thread safety.
+        try tp.complete(this)
+        finally unlock()
     } catch {
       case ex: CyclicReference =>
-        devWarning("... hit cycle trying to complete " + this.fullLocationString)
+        devWarning(s"... hit cycle trying to complete $fullLocationString")
         throw ex
     }
 
@@ -3839,10 +3853,13 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     else closestEnclMethod(from.owner)
 
   /** An exception for cyclic references of symbol definitions */
-  case class CyclicReference(sym: Symbol, info: Type)
-  extends TypeError("illegal cyclic reference involving " + sym) {
+  case class CyclicReference(sym: Symbol, info: Type, trace: Array[Symbol] = CyclicReference.emptyTrace)
+  extends TypeError(s"illegal cyclic reference involving $sym") {
     if (settings.isDebug) printStackTrace()
   }
+  object CyclicReference {
+    val emptyTrace: Array[Symbol] = Array.empty[Symbol]
+  }
 
   /** A class for type histories */
   private final case class TypeHistory protected (private var _validFrom: Period, private var _info: Type, private var _prev: TypeHistory) {

test/files/neg/t7808.check

@@ -0,0 +1,4 @@
+t7808.scala:5: error: recursive value ls needs type
+    val (ls, rs) = z match {
+         ^
+1 error

test/files/neg/t7808.scala

@@ -0,0 +1,18 @@
+//> using options -Vcyclic
+class C {
+  type OI = Option[Int]
+  def f(z: OI, ls: List[OI], rs: List[OI]): (List[OI], List[OI]) = {
+    val (ls, rs) = z match {
+      case Some(_) => (z::ls, rs)
+      case _       => (ls, z::rs)
+    }
+    (ls, rs)
+  }
+}
+
+/*
+t7808.scala:5: error: recursive value x$1 needs type
+    val (ls, rs) = z match {
+         ^
+1 error
+*/

test/files/neg/t7808b.check

@@ -0,0 +1,4 @@
+t7808b.scala:5: error: recursive value x$1 needs type; value x$1 is synthetic; use -Vcyclic to find which definition needs an explicit type
+    val (ls, rs) = z match {
+         ^
+1 error