Skip to content

Commit f026720

Browse files
authored
Better error diagnostics under -explain-cyclic (#20251)
Also report type-checked right hand sides and export expansions. Streamline trace-handling code using inline functions. Fixes #20245
2 parents 912d886 + 46e0d9d commit f026720

17 files changed

+173
-48
lines changed

compiler/src/dotty/tools/dotc/core/SymDenotations.scala

+4-16
Original file line numberDiff line numberDiff line change
@@ -168,16 +168,11 @@ object SymDenotations {
168168
}
169169
}
170170
else
171-
val traceCycles = CyclicReference.isTraced
172-
try
173-
if traceCycles then
174-
CyclicReference.pushTrace("compute the signature of ", symbol, "")
171+
CyclicReference.trace("compute the signature of ", symbol):
175172
if myFlags.is(Touched) then
176173
throw CyclicReference(this)(using ctx.withOwner(symbol))
177174
myFlags |= Touched
178175
atPhase(validFor.firstPhaseId)(completer.complete(this))
179-
finally
180-
if traceCycles then CyclicReference.popTrace()
181176

182177
protected[dotc] def info_=(tp: Type): Unit = {
183178
/* // DEBUG
@@ -2992,12 +2987,9 @@ object SymDenotations {
29922987
def apply(clsd: ClassDenotation)(implicit onBehalf: BaseData, ctx: Context)
29932988
: (List[ClassSymbol], BaseClassSet) = {
29942989
assert(isValid)
2995-
val traceCycles = CyclicReference.isTraced
2996-
try
2997-
if traceCycles then
2998-
CyclicReference.pushTrace("compute the base classes of ", clsd.symbol, "")
2999-
if (cache != null) cache.uncheckedNN
3000-
else {
2990+
CyclicReference.trace("compute the base classes of ", clsd.symbol):
2991+
if cache != null then cache.uncheckedNN
2992+
else
30012993
if (locked) throw CyclicReference(clsd)
30022994
locked = true
30032995
provisional = false
@@ -3007,10 +2999,6 @@ object SymDenotations {
30072999
if (!provisional) cache = computed
30083000
else onBehalf.signalProvisional()
30093001
computed
3010-
}
3011-
finally
3012-
if traceCycles then CyclicReference.popTrace()
3013-
addDependent(onBehalf)
30143002
}
30153003

30163004
def sameGroup(p1: Phase, p2: Phase) = p1.sameParentsStartId == p2.sameParentsStartId

compiler/src/dotty/tools/dotc/core/TypeErrors.scala

+15-4
Original file line numberDiff line numberDiff line change
@@ -198,20 +198,31 @@ object CyclicReference:
198198
cyclicErrors.println(elem.toString)
199199
ex
200200

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

205-
def isTraced(using Context) =
205+
private def isTraced(using Context) =
206206
ctx.property(CyclicReference.Trace).isDefined
207207

208-
def pushTrace(info: TraceElement)(using Context): Unit =
208+
private def pushTrace(info: TraceElement)(using Context): Unit =
209209
for buf <- ctx.property(CyclicReference.Trace) do
210210
buf += info
211211

212-
def popTrace()(using Context): Unit =
212+
private def popTrace()(using Context): Unit =
213213
for buf <- ctx.property(CyclicReference.Trace) do
214214
buf.dropRightInPlace(1)
215+
216+
inline def trace[T](info: TraceElement)(inline op: => T)(using Context): T =
217+
val traceCycles = isTraced
218+
try
219+
if traceCycles then pushTrace(info)
220+
op
221+
finally
222+
if traceCycles then popTrace()
223+
224+
inline def trace[T](prefix: String, sym: Symbol)(inline op: => T)(using Context): T =
225+
trace((ctx: Context) ?=> i"$prefix$sym")(op)
215226
end CyclicReference
216227

217228
class UnpicklingError(denot: Denotation, where: String, cause: Throwable)(using Context) extends TypeError:

compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala

+9-14
Original file line numberDiff line numberDiff line change
@@ -158,20 +158,15 @@ class TreeUnpickler(reader: TastyReader,
158158
if f == null then "" else s" in $f"
159159
def fail(ex: Throwable) = throw UnpicklingError(denot, where, ex)
160160
treeAtAddr(currentAddr) =
161-
val traceCycles = CyclicReference.isTraced
162-
try
163-
if traceCycles then
164-
CyclicReference.pushTrace("read the definition of ", denot.symbol, where)
165-
atPhaseBeforeTransforms {
166-
new TreeReader(reader).readIndexedDef()(
167-
using ctx.withOwner(owner).withModeBits(mode).withSource(source))
168-
}
169-
catch
170-
case ex: CyclicReference => throw ex
171-
case ex: AssertionError => fail(ex)
172-
case ex: Exception => fail(ex)
173-
finally
174-
if traceCycles then CyclicReference.popTrace()
161+
CyclicReference.trace(i"read the definition of ${denot.symbol}$where"):
162+
try
163+
atPhaseBeforeTransforms:
164+
new TreeReader(reader).readIndexedDef()(
165+
using ctx.withOwner(owner).withModeBits(mode).withSource(source))
166+
catch
167+
case ex: CyclicReference => throw ex
168+
case ex: AssertionError => fail(ex)
169+
case ex: Exception => fail(ex)
175170
}
176171

177172
class TreeReader(val reader: TastyReader) {

compiler/src/dotty/tools/dotc/reporting/messages.scala

+2-1
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,8 @@ abstract class CyclicMsg(errorId: ErrorMessageID)(using Context) extends Message
9595
protected def context: String = ex.optTrace match
9696
case Some(trace) =>
9797
s"\n\nThe error occurred while trying to ${
98-
trace.map((prefix, sym, suffix) => i"$prefix$sym$suffix").mkString("\n which required to ")
98+
trace.map(identity) // map with identity will turn Context ?=> String elements to String elements
99+
.mkString("\n which required to ")
99100
}$debugInfo"
100101
case None =>
101102
"\n\n Run with -explain-cyclic for more details."

compiler/src/dotty/tools/dotc/typer/Checking.scala

+1-6
Original file line numberDiff line numberDiff line change
@@ -349,10 +349,7 @@ object Checking {
349349
}
350350

351351
if isInteresting(pre) then
352-
val traceCycles = CyclicReference.isTraced
353-
try
354-
if traceCycles then
355-
CyclicReference.pushTrace("explore ", tp.symbol, " for cyclic references")
352+
CyclicReference.trace(i"explore ${tp.symbol} for cyclic references"):
356353
val pre1 = this(pre, false, false)
357354
if locked.contains(tp)
358355
|| tp.symbol.infoOrCompleter.isInstanceOf[NoCompleter]
@@ -367,8 +364,6 @@ object Checking {
367364
finally
368365
locked -= tp
369366
tp.withPrefix(pre1)
370-
finally
371-
if traceCycles then CyclicReference.popTrace()
372367
else tp
373368
}
374369
catch {

compiler/src/dotty/tools/dotc/typer/Implicits.scala

+9-4
Original file line numberDiff line numberDiff line change
@@ -1084,10 +1084,15 @@ trait Implicits:
10841084
(searchCtx.scope eq ctx.scope) && (searchCtx.owner eq ctx.owner.owner)
10851085
do ()
10861086

1087-
try ImplicitSearch(pt, argument, span)(using searchCtx).bestImplicit
1088-
catch case ce: CyclicReference =>
1089-
ce.inImplicitSearch = true
1090-
throw ce
1087+
def searchStr =
1088+
if argument.isEmpty then i"argument of type $pt"
1089+
else i"conversion from ${argument.tpe} to $pt"
1090+
1091+
CyclicReference.trace(i"searching for an implicit $searchStr"):
1092+
try ImplicitSearch(pt, argument, span)(using searchCtx).bestImplicit
1093+
catch case ce: CyclicReference =>
1094+
ce.inImplicitSearch = true
1095+
throw ce
10911096
else NoMatchingImplicitsFailure
10921097

10931098
val result =

compiler/src/dotty/tools/dotc/typer/Namer.scala

+5-3
Original file line numberDiff line numberDiff line change
@@ -1442,7 +1442,8 @@ class Namer { typer: Typer =>
14421442

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

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

19601962
def rhsType =
19611963
// For default getters, we use the corresponding parameter type as an

tests/neg-macros/i14772.check

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
|
66
| The error occurred while trying to compute the signature of method $anonfun
77
| which required to compute the signature of method impl
8+
| which required to type the right hand side of method impl since no explicit type was given
89
|
910
| Run with both -explain-cyclic and -Ydebug-cyclic to see full stack trace.
1011
|

tests/neg-macros/i16582.check

+2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
| dotty.tools.dotc.core.CyclicReference: Recursive value o2 needs type
77
|
88
| The error occurred while trying to compute the signature of method test
9+
| which required to type the right hand side of method test since no explicit type was given
910
| which required to compute the signature of value o2
11+
| which required to type the right hand side of value o2 since no explicit type was given
1012
| which required to compute the signature of value o2
1113
|
1214
| Run with both -explain-cyclic and -Ydebug-cyclic to see full stack trace.

tests/neg/cyclic.check

+4
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,13 @@
44
| Overloaded or recursive method f needs return type
55
|
66
| The error occurred while trying to compute the signature of method f
7+
| which required to type the right hand side of method f since no explicit type was given
78
| which required to compute the signature of method g
9+
| which required to type the right hand side of method g since no explicit type was given
810
| which required to compute the signature of method h
11+
| which required to type the right hand side of method h since no explicit type was given
912
| which required to compute the signature of method i
13+
| which required to type the right hand side of method i since no explicit type was given
1014
| which required to compute the signature of method f
1115
|
1216
| Run with both -explain-cyclic and -Ydebug-cyclic to see full stack trace.

tests/neg/i20245.check

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
2+
-- [E046] Cyclic Error: tests/neg/i20245/Typer_2.scala:16:57 -----------------------------------------------------------
3+
16 | private[typer] val unification = new Unification(using this) // error
4+
| ^
5+
| Cyclic reference involving class Context
6+
|
7+
| The error occurred while trying to compute the base classes of class Context
8+
| which required to compute the base classes of trait TyperOps
9+
| which required to compute the signature of trait TyperOps
10+
| which required to elaborate the export clause export unification.requireSubtype
11+
| which required to compute the signature of value unification
12+
| which required to type the right hand side of value unification since no explicit type was given
13+
| which required to compute the base classes of class Context
14+
|
15+
| Run with both -explain-cyclic and -Ydebug-cyclic to see full stack trace.
16+
|
17+
| longer explanation available when compiling with `-explain`

tests/neg/i20245/Context_1.scala

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package effekt
2+
package context
3+
4+
import effekt.typer.TyperOps
5+
6+
7+
abstract class Context extends TyperOps {
8+
9+
// bring the context itself in scope
10+
implicit val context: Context = this
11+
12+
}

tests/neg/i20245/Messages_1.scala

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
package effekt
2+
package util
3+
4+
object messages {
5+
trait ErrorReporter {
6+
7+
}
8+
}

tests/neg/i20245/Tree_1.scala

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package effekt
2+
package source
3+
4+
import effekt.context.Context
5+
6+
object Resolvable {
7+
8+
// There need to be two resolve extension methods for the error to show up
9+
// They also need to take an implicit Context
10+
extension (n: Int) {
11+
def resolve(using C: Context): Unit = ???
12+
}
13+
14+
extension (b: Boolean) {
15+
def resolve(using C: Context): Unit = ???
16+
}
17+
}
18+
export Resolvable.resolve

tests/neg/i20245/Typer_1.scala

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package effekt
2+
package typer
3+
4+
import effekt.util.messages.ErrorReporter
5+
6+
import effekt.context.{ Context }
7+
8+
// This import is also NECESSARY for the cyclic error
9+
import effekt.source.{ resolve }
10+
11+
12+
trait TyperOps extends ErrorReporter { self: Context =>
13+
14+
// passing `this` as ErrorReporter here is also NECESSARY for the cyclic error
15+
private[typer] val unification = new Unification(using this)
16+
17+
// this export is NECESSARY for the cyclic error
18+
export unification.{ requireSubtype }
19+
20+
println(1)
21+
22+
// vvvvvvvv insert a line here, save, and run `compile` again vvvvvvvvvv
23+
}
24+
25+
26+
27+
28+

tests/neg/i20245/Typer_2.scala

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
//> using options -explain-cyclic
2+
package effekt
3+
package typer
4+
5+
import effekt.util.messages.ErrorReporter
6+
7+
import effekt.context.{ Context }
8+
9+
// This import is also NECESSARY for the cyclic error
10+
import effekt.source.{ resolve }
11+
12+
13+
trait TyperOps extends ErrorReporter { self: Context =>
14+
15+
// passing `this` as ErrorReporter here is also NECESSARY for the cyclic error
16+
private[typer] val unification = new Unification(using this) // error
17+
18+
// this export is NECESSARY for the cyclic error
19+
export unification.{ requireSubtype }
20+
21+
// vvvvvvvv insert a line here, save, and run `compile` again vvvvvvvvvv
22+
}
23+
24+
25+
26+
27+

tests/neg/i20245/Unification_1.scala

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package effekt
2+
package typer
3+
4+
import effekt.util.messages.ErrorReporter
5+
6+
7+
class Unification(using C: ErrorReporter) {
8+
9+
def requireSubtype(): Unit = ()
10+
11+
}

0 commit comments

Comments
 (0)