Skip to content

Commit

Permalink
Better error diagnostics under -explain-cyclic
Browse files Browse the repository at this point in the history
Also report type-checked right hand sides and export expansions.

Streamline trace-handling code using inline functions.

Fixes #20245
  • Loading branch information
odersky committed Apr 23, 2024
1 parent fef6576 commit cfc46e4
Show file tree
Hide file tree
Showing 16 changed files with 164 additions and 44 deletions.
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 =
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
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 = ()

}

0 comments on commit cfc46e4

Please sign in to comment.