Skip to content

Commit

Permalink
Use CyclicRef trace to pick sym in message
Browse files Browse the repository at this point in the history
  • Loading branch information
som-snytt committed Feb 3, 2024
1 parent e46dd02 commit f8708b2
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 51 deletions.
6 changes: 3 additions & 3 deletions src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala
Expand Up @@ -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
//
Expand All @@ -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))
}
Expand Down
49 changes: 26 additions & 23 deletions src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala
Expand Up @@ -805,19 +805,26 @@ 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: List[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 trace.filter(!_.isSynthetic) match {
case badsym :: Nil => badsym
case Nil => sym
case baddies => baddies.find(_.pos.focus == pos.focus).getOrElse(baddies.head)
}
condOpt(tree) {
case ValDef(_, _, TypeTree(), _) => s"recursive $badsym needs type"
case DefDef(_, _, _, _, TypeTree(), _) => s"${cyclicAdjective(badsym)} $badsym needs result type"
case Import(expr, selectors) =>
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
Expand Down Expand Up @@ -852,20 +859,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)
}
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/scala/tools/nsc/typechecker/Typers.scala
Expand Up @@ -6232,7 +6232,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 =>
Expand Down
58 changes: 34 additions & 24 deletions src/reflect/scala/reflect/internal/Symbols.scala
Expand Up @@ -19,7 +19,7 @@ package reflect
package internal

import scala.collection.immutable
import scala.collection.mutable.ListBuffer
import scala.collection.mutable.{ListBuffer, Stack}
import util.{ ReusableInstance, Statistics, shortClassOfInstance }
import Flags._
import scala.annotation.tailrec
Expand All @@ -44,6 +44,8 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
def lockedCount = this._lockedCount
def lockedCount_=(i: Int) = _lockedCount = i

private[this] val _lockingTrace = Stack.empty[Symbol]


@deprecated("Global existential IDs no longer used", "2.12.1")
private[this] var existentialIds = 0
Expand Down Expand Up @@ -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 = {
_lockingTrace.push(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
Expand All @@ -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 (!_lockingTrace.isEmpty)
_lockingTrace.remove(idx = 0, count = 1)
if (settings.Yrecursion.value != 0)
recursionTable -= this
}
}

// ----- tests ----------------------------------------------------------------------

Expand Down Expand Up @@ -1553,9 +1563,12 @@ 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 = _lockingTrace.toList
_lockingTrace.clear()
throw CyclicReference(this, tp, trace)
}
} else {
_lockingTrace.push(this)
_rawflags |= LOCKED
}
val current = phase
Expand All @@ -1568,18 +1581,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
}

Expand Down Expand Up @@ -3835,8 +3845,8 @@ 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: List[Symbol] = Nil)
extends TypeError(s"illegal cyclic reference involving $sym") {
if (settings.isDebug) printStackTrace()
}

Expand Down
4 changes: 4 additions & 0 deletions 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
18 changes: 18 additions & 0 deletions test/files/neg/t7808.scala
@@ -0,0 +1,18 @@

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
*/

0 comments on commit f8708b2

Please sign in to comment.