Skip to content
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

Better error diagnostics under -explain-cyclic #20251

Merged
merged 3 commits into from
Apr 26, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
20 changes: 4 additions & 16 deletions compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -168,16 +168,11 @@ object SymDenotations {
}
}
else
val traceCycles = CyclicReference.isTraced
try
if traceCycles then
CyclicReference.pushTrace("compute the signature of ", symbol, "")
CyclicReference.trace("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 @@ -2994,12 +2989,9 @@ 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 {
CyclicReference.trace("compute the base classes of ", clsd.symbol):
if cache != null then cache.uncheckedNN
else
if (locked) throw CyclicReference(clsd)
locked = true
provisional = false
Expand All @@ -3009,10 +3001,6 @@ object SymDenotations {
if (!provisional) cache = computed
else onBehalf.signalProvisional()
computed
}
finally
if traceCycles then CyclicReference.popTrace()
addDependent(onBehalf)
}

def sameGroup(p1: Phase, p2: Phase) = p1.sameParentsStartId == p2.sameParentsStartId
Expand Down
19 changes: 15 additions & 4 deletions compiler/src/dotty/tools/dotc/core/TypeErrors.scala
Original file line number Diff line number Diff line change
Expand Up @@ -198,20 +198,31 @@ object CyclicReference:
cyclicErrors.println(elem.toString)
ex

type TraceElement = (/*prefix:*/ String, Symbol, /*suffix:*/ String)
type TraceElement = Context ?=> String
type Trace = mutable.ArrayBuffer[TraceElement]
val Trace = Property.Key[Trace]

def isTraced(using Context) =
private def isTraced(using Context) =
ctx.property(CyclicReference.Trace).isDefined

def pushTrace(info: TraceElement)(using Context): Unit =
private def pushTrace(info: Context ?=> String)(using Context): Unit =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't the type of info stay `TraceElement?

odersky marked this conversation as resolved.
Show resolved Hide resolved
for buf <- ctx.property(CyclicReference.Trace) do
buf += info

def popTrace()(using Context): Unit =
private def popTrace()(using Context): Unit =
for buf <- ctx.property(CyclicReference.Trace) do
buf.dropRightInPlace(1)

inline def trace[T](info: TraceElement)(inline op: => T)(using Context): T =
val traceCycles = isTraced
try
if traceCycles then pushTrace(info)
op
finally
if traceCycles then popTrace()

inline def trace[T](prefix: String, sym: Symbol)(inline op: => T)(using Context): T =
trace((ctx: Context) ?=> i"$prefix$sym")(op)
end CyclicReference

class UnpicklingError(denot: Denotation, where: String, cause: Throwable)(using Context) extends TypeError:
Expand Down
23 changes: 9 additions & 14 deletions compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -158,20 +158,15 @@ class TreeUnpickler(reader: TastyReader,
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()
CyclicReference.trace(i"read the definition of ${denot.symbol}$where"):
try
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)
}

class TreeReader(val reader: TastyReader) {
Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ abstract class CyclicMsg(errorId: ErrorMessageID)(using Context) extends Message
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 ")
trace.map(identity) // map with identity will turn Context ?=> String elements to String elements
.mkString("\n which required to ")
}$debugInfo"
case None =>
"\n\n Run with -explain-cyclic for more details."
Expand Down
7 changes: 1 addition & 6 deletions compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -349,10 +349,7 @@ object Checking {
}

if isInteresting(pre) then
val traceCycles = CyclicReference.isTraced
try
if traceCycles then
CyclicReference.pushTrace("explore ", tp.symbol, " for cyclic references")
CyclicReference.trace(i"explore ${tp.symbol} for cyclic references"):
val pre1 = this(pre, false, false)
if locked.contains(tp)
|| tp.symbol.infoOrCompleter.isInstanceOf[NoCompleter]
Expand All @@ -367,8 +364,6 @@ object Checking {
finally
locked -= tp
tp.withPrefix(pre1)
finally
if traceCycles then CyclicReference.popTrace()
else tp
}
catch {
Expand Down
13 changes: 9 additions & 4 deletions compiler/src/dotty/tools/dotc/typer/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1084,10 +1084,15 @@ trait Implicits:
(searchCtx.scope eq ctx.scope) && (searchCtx.owner eq ctx.owner.owner)
do ()

try ImplicitSearch(pt, argument, span)(using searchCtx).bestImplicit
catch case ce: CyclicReference =>
ce.inImplicitSearch = true
throw ce
def searchStr =
if argument.isEmpty then i"argument of type $pt"
else i"conversion from ${argument.tpe} to $pt"

CyclicReference.trace(i"searching for an implicit $searchStr"):
try ImplicitSearch(pt, argument, span)(using searchCtx).bestImplicit
catch case ce: CyclicReference =>
ce.inImplicitSearch = true
throw ce
else NoMatchingImplicitsFailure

val result =
Expand Down
8 changes: 5 additions & 3 deletions compiler/src/dotty/tools/dotc/typer/Namer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1442,7 +1442,8 @@ class Namer { typer: Typer =>

def process(stats: List[Tree])(using Context): Unit = stats match
case (stat: Export) :: stats1 =>
processExport(stat, NoSymbol)
CyclicReference.trace(i"elaborate the export clause $stat"):
processExport(stat, NoSymbol)
process(stats1)
case (stat: Import) :: stats1 =>
process(stats1)(using ctx.importContext(stat, symbolOfTree(stat)))
Expand Down Expand Up @@ -1954,8 +1955,9 @@ class Namer { typer: Typer =>
rhsCtx = prepareRhsCtx(rhsCtx, paramss)

def typedAheadRhs(pt: Type) =
PrepareInlineable.dropInlineIfError(sym,
typedAheadExpr(mdef.rhs, pt)(using rhsCtx))
CyclicReference.trace(i"type the right hand side of $sym since no explicit type was given"):
PrepareInlineable.dropInlineIfError(sym,
typedAheadExpr(mdef.rhs, pt)(using rhsCtx))

def rhsType =
// For default getters, we use the corresponding parameter type as an
Expand Down
1 change: 1 addition & 0 deletions tests/neg-macros/i14772.check
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
|
| The error occurred while trying to compute the signature of method $anonfun
| which required to compute the signature of method impl
| which required to type the right hand side of method impl since no explicit type was given
|
| Run with both -explain-cyclic and -Ydebug-cyclic to see full stack trace.
|
Expand Down
2 changes: 2 additions & 0 deletions tests/neg-macros/i16582.check
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
| 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 type the right hand side of method test since no explicit type was given
| which required to compute the signature of value o2
| which required to type the right hand side of value o2 since no explicit type was given
| which required to compute the signature of value o2
|
| Run with both -explain-cyclic and -Ydebug-cyclic to see full stack trace.
Expand Down
4 changes: 4 additions & 0 deletions tests/neg/cyclic.check
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,13 @@
| Overloaded or recursive method f needs return type
|
| The error occurred while trying to compute the signature of method f
| which required to type the right hand side of method f since no explicit type was given
| which required to compute the signature of method g
| which required to type the right hand side of method g since no explicit type was given
| which required to compute the signature of method h
| which required to type the right hand side of method h since no explicit type was given
| which required to compute the signature of method i
| which required to type the right hand side of method i since no explicit type was given
| which required to compute the signature of method f
|
| Run with both -explain-cyclic and -Ydebug-cyclic to see full stack trace.
Expand Down
17 changes: 17 additions & 0 deletions tests/neg/i20245.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@

-- [E046] Cyclic Error: tests/neg/i20245/Typer_2.scala:16:57 -----------------------------------------------------------
16 | private[typer] val unification = new Unification(using this) // error
| ^
| Cyclic reference involving class Context
|
| The error occurred while trying to compute the base classes of class Context
| which required to compute the base classes of trait TyperOps
| which required to compute the signature of trait TyperOps
| which required to elaborate the export clause export unification.requireSubtype
| which required to compute the signature of value unification
| which required to type the right hand side of value unification since no explicit type was given
| which required to compute the base classes of class Context
|
| Run with both -explain-cyclic and -Ydebug-cyclic to see full stack trace.
|
| longer explanation available when compiling with `-explain`
12 changes: 12 additions & 0 deletions tests/neg/i20245/Context_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package effekt
package context

import effekt.typer.TyperOps


abstract class Context extends TyperOps {

// bring the context itself in scope
implicit val context: Context = this

}
8 changes: 8 additions & 0 deletions tests/neg/i20245/Messages_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package effekt
package util

object messages {
trait ErrorReporter {

}
}
18 changes: 18 additions & 0 deletions tests/neg/i20245/Tree_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package effekt
package source

import effekt.context.Context

object Resolvable {

// There need to be two resolve extension methods for the error to show up
// They also need to take an implicit Context
extension (n: Int) {
def resolve(using C: Context): Unit = ???
}

extension (b: Boolean) {
def resolve(using C: Context): Unit = ???
}
}
export Resolvable.resolve
28 changes: 28 additions & 0 deletions tests/neg/i20245/Typer_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package effekt
package typer

import effekt.util.messages.ErrorReporter

import effekt.context.{ Context }

// This import is also NECESSARY for the cyclic error
import effekt.source.{ resolve }


trait TyperOps extends ErrorReporter { self: Context =>

// passing `this` as ErrorReporter here is also NECESSARY for the cyclic error
private[typer] val unification = new Unification(using this)

// this export is NECESSARY for the cyclic error
export unification.{ requireSubtype }

println(1)

// vvvvvvvv insert a line here, save, and run `compile` again vvvvvvvvvv
}





27 changes: 27 additions & 0 deletions tests/neg/i20245/Typer_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
//> using options -explain-cyclic
package effekt
package typer

import effekt.util.messages.ErrorReporter

import effekt.context.{ Context }

// This import is also NECESSARY for the cyclic error
import effekt.source.{ resolve }


trait TyperOps extends ErrorReporter { self: Context =>

// passing `this` as ErrorReporter here is also NECESSARY for the cyclic error
private[typer] val unification = new Unification(using this) // error

// this export is NECESSARY for the cyclic error
export unification.{ requireSubtype }

// vvvvvvvv insert a line here, save, and run `compile` again vvvvvvvvvv
}





11 changes: 11 additions & 0 deletions tests/neg/i20245/Unification_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package effekt
package typer

import effekt.util.messages.ErrorReporter


class Unification(using C: ErrorReporter) {

def requireSubtype(): Unit = ()

}
Loading