Skip to content

Better error diagnostics for cyclic references #19408

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/Run.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/config/ScalaSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
Expand Down Expand Up @@ -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.")
Expand Down
24 changes: 17 additions & 7 deletions compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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("compute the signature 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
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
39 changes: 30 additions & 9 deletions compiler/src/dotty/tools/dotc/core/TypeErrors.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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 _ =>
Expand All @@ -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:
Expand Down
15 changes: 10 additions & 5 deletions compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
48 changes: 33 additions & 15 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
9 changes: 7 additions & 2 deletions tests/neg-macros/i14772.check
Original file line number Diff line number Diff line change
@@ -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 compute the signature of method $anonfun
| which required to compute the signature of method impl
|
| Run with both -explain-cyclic and -Ydebug-cyclic to see full stack trace.
|
| longer explanation available when compiling with `-explain`
1 change: 1 addition & 0 deletions tests/neg-macros/i14772.scala
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//> using options -explain-cyclic
import scala.quoted.*

object A {
Expand Down
10 changes: 8 additions & 2 deletions tests/neg-macros/i16582.check
Original file line number Diff line number Diff line change
@@ -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 compute the signature of method test
| which required to compute the signature of value o2
| which required to compute the signature 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
Expand Down
1 change: 1 addition & 0 deletions tests/neg-macros/i16582/Test_2.scala
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//> using options -explain-cyclic
def test=
val o1 = ownerWorks(1)
println(o1)
Expand Down
14 changes: 14 additions & 0 deletions tests/neg/cyclic.check
Original file line number Diff line number Diff line change
@@ -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 compute the signature of method f
| which required to compute the signature of method g
| which required to compute the signature of method h
| which required to compute the signature of method i
| which required to compute the signature of method f
|
| Run with both -explain-cyclic and -Ydebug-cyclic to see full stack trace.
|
| longer explanation available when compiling with `-explain`
6 changes: 6 additions & 0 deletions tests/neg/cyclic.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
//> using options -explain-cyclic
object O:
def f() = g()
def g() = h()
def h() = i()
def i() = f() // error
2 changes: 2 additions & 0 deletions tests/neg/i10870.check
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Loading