From d4833c7a78e1a9595751af967ff5f0e81606b4ac Mon Sep 17 00:00:00 2001 From: odersky Date: Tue, 23 Apr 2024 19:37:15 +0200 Subject: [PATCH 1/4] Better error diagnostics under -explain-cyclic Also report type-checked right hand sides and export expansions. Streamline trace-handling code using inline functions. Fixes #20245 [Cherry-picked cfc46e4556a2a79bbe4d1d9daa31ad5842171fa7] --- .../tools/dotc/core/SymDenotations.scala | 20 +++---------- .../dotty/tools/dotc/core/TypeErrors.scala | 19 ++++++++++--- .../tools/dotc/core/tasty/TreeUnpickler.scala | 23 ++++++--------- .../dotty/tools/dotc/reporting/messages.scala | 3 +- .../src/dotty/tools/dotc/typer/Checking.scala | 7 +---- .../src/dotty/tools/dotc/typer/Namer.scala | 8 ++++-- tests/neg-macros/i14772.check | 1 + tests/neg-macros/i16582.check | 2 ++ tests/neg/cyclic.check | 4 +++ tests/neg/i20245.check | 17 +++++++++++ tests/neg/i20245/Context_1.scala | 12 ++++++++ tests/neg/i20245/Messages_1.scala | 8 ++++++ tests/neg/i20245/Tree_1.scala | 18 ++++++++++++ tests/neg/i20245/Typer_1.scala | 28 +++++++++++++++++++ tests/neg/i20245/Typer_2.scala | 27 ++++++++++++++++++ tests/neg/i20245/Unification_1.scala | 11 ++++++++ 16 files changed, 164 insertions(+), 44 deletions(-) create mode 100644 tests/neg/i20245.check create mode 100644 tests/neg/i20245/Context_1.scala create mode 100644 tests/neg/i20245/Messages_1.scala create mode 100644 tests/neg/i20245/Tree_1.scala create mode 100644 tests/neg/i20245/Typer_1.scala create mode 100644 tests/neg/i20245/Typer_2.scala create mode 100644 tests/neg/i20245/Unification_1.scala diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 71e54132f0ef..7e998b22a136 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -167,16 +167,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 @@ -2980,12 +2975,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 @@ -2995,10 +2987,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 diff --git a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala index a9b07869f6fc..81697d12bea5 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala @@ -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: diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala index 33b840ae2170..cf39fb956a58 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala @@ -138,20 +138,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) { diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index d6d9bd8bc868..56105bfe0c48 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -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." diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index 8ccb9c00dbfd..6bfbea0ace1a 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala @@ -336,10 +336,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] @@ -354,8 +351,6 @@ object Checking { finally locked -= tp tp.withPrefix(pre1) - finally - if traceCycles then CyclicReference.popTrace() else tp } catch { diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index 077b10526bac..3f5bd0637f3f 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -1440,7 +1440,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))) @@ -1949,8 +1950,9 @@ class Namer { typer: Typer => } 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 diff --git a/tests/neg-macros/i14772.check b/tests/neg-macros/i14772.check index 94b4a3445b01..5c1836811b03 100644 --- a/tests/neg-macros/i14772.check +++ b/tests/neg-macros/i14772.check @@ -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. | diff --git a/tests/neg-macros/i16582.check b/tests/neg-macros/i16582.check index 548a4491ed24..546d0b7efaf3 100644 --- a/tests/neg-macros/i16582.check +++ b/tests/neg-macros/i16582.check @@ -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. diff --git a/tests/neg/cyclic.check b/tests/neg/cyclic.check index 19eedac04f1c..d9afb91975f8 100644 --- a/tests/neg/cyclic.check +++ b/tests/neg/cyclic.check @@ -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. diff --git a/tests/neg/i20245.check b/tests/neg/i20245.check new file mode 100644 index 000000000000..565bde7678b7 --- /dev/null +++ b/tests/neg/i20245.check @@ -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` diff --git a/tests/neg/i20245/Context_1.scala b/tests/neg/i20245/Context_1.scala new file mode 100644 index 000000000000..a38d8fe7531d --- /dev/null +++ b/tests/neg/i20245/Context_1.scala @@ -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 + +} diff --git a/tests/neg/i20245/Messages_1.scala b/tests/neg/i20245/Messages_1.scala new file mode 100644 index 000000000000..c8cc8267d44c --- /dev/null +++ b/tests/neg/i20245/Messages_1.scala @@ -0,0 +1,8 @@ +package effekt +package util + +object messages { + trait ErrorReporter { + + } +} diff --git a/tests/neg/i20245/Tree_1.scala b/tests/neg/i20245/Tree_1.scala new file mode 100644 index 000000000000..54a2a5cc1a64 --- /dev/null +++ b/tests/neg/i20245/Tree_1.scala @@ -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 diff --git a/tests/neg/i20245/Typer_1.scala b/tests/neg/i20245/Typer_1.scala new file mode 100644 index 000000000000..0a61346ecaef --- /dev/null +++ b/tests/neg/i20245/Typer_1.scala @@ -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 +} + + + + + diff --git a/tests/neg/i20245/Typer_2.scala b/tests/neg/i20245/Typer_2.scala new file mode 100644 index 000000000000..ed7f05de80d0 --- /dev/null +++ b/tests/neg/i20245/Typer_2.scala @@ -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 +} + + + + + diff --git a/tests/neg/i20245/Unification_1.scala b/tests/neg/i20245/Unification_1.scala new file mode 100644 index 000000000000..406ab1b93b00 --- /dev/null +++ b/tests/neg/i20245/Unification_1.scala @@ -0,0 +1,11 @@ +package effekt +package typer + +import effekt.util.messages.ErrorReporter + + +class Unification(using C: ErrorReporter) { + + def requireSubtype(): Unit = () + +} From 72954ebe7993522f39e56e87012697f5b022cb88 Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 24 Apr 2024 12:28:18 +0200 Subject: [PATCH 2/4] Add implicit search to CyclicReference traces [Cherry-picked 5efacbe87d62c5e9a1f8fce07be2c3c8e5700ab7] --- compiler/src/dotty/tools/dotc/typer/Implicits.scala | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 85458a2fae7d..2e5acb1d217a 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1058,10 +1058,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 = From 2d51401e495f7844c3d38787f498bf1de4eb5d4a Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 25 Apr 2024 17:54:52 +0200 Subject: [PATCH 3/4] Update compiler/src/dotty/tools/dotc/core/TypeErrors.scala [Cherry-picked 46e0d9d433661525bd993ea78eee885674267de2] --- compiler/src/dotty/tools/dotc/core/TypeErrors.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala index 81697d12bea5..ae017a936818 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala @@ -205,7 +205,7 @@ object CyclicReference: private def isTraced(using Context) = ctx.property(CyclicReference.Trace).isDefined - private def pushTrace(info: Context ?=> String)(using Context): Unit = + private def pushTrace(info: TraceElement)(using Context): Unit = for buf <- ctx.property(CyclicReference.Trace) do buf += info From 900e74b9c7b349b63e00e1d207dacab99ebfc010 Mon Sep 17 00:00:00 2001 From: Wojciech Mazur Date: Sat, 6 Jul 2024 13:45:42 +0200 Subject: [PATCH 4/4] Adjust i11994.check for LTS outputs --- tests/neg/i11994.check | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/neg/i11994.check b/tests/neg/i11994.check index 76860af70a39..445e99b4aab8 100644 --- a/tests/neg/i11994.check +++ b/tests/neg/i11994.check @@ -8,6 +8,7 @@ | | The error occurred while trying to compute the signature of method foo | which required to compute the signature of type T + | which required to searching for an implicit conversion from Tuple.type to ?{ meow: ? } | which required to compute the signature of method foo | | Run with both -explain-cyclic and -Ydebug-cyclic to see full stack trace. @@ -21,6 +22,7 @@ | | The error occurred while trying to compute the signature of given instance given_Unit | which required to compute the signature of type T + | which required to searching for an implicit conversion from Tuple.type to ?{ meow: ? } | which required to compute the signature of given instance given_Unit | | Run with both -explain-cyclic and -Ydebug-cyclic to see full stack trace.