From 09b389b356f3baf6b6d8fa39bd50fb1a789874d5 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Wed, 1 Mar 2023 17:22:57 +0000 Subject: [PATCH 1/3] Change trytoShow's to not use raw string --- .../dotty/tools/dotc/core/Decorators.scala | 5 +---- .../tools/dotc/core/ShowDecoratorTest.scala | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) create mode 100644 compiler/test/dotty/tools/dotc/core/ShowDecoratorTest.scala diff --git a/compiler/src/dotty/tools/dotc/core/Decorators.scala b/compiler/src/dotty/tools/dotc/core/Decorators.scala index 9f55e29c0f59..4ef0dbc9a43b 100644 --- a/compiler/src/dotty/tools/dotc/core/Decorators.scala +++ b/compiler/src/dotty/tools/dotc/core/Decorators.scala @@ -280,10 +280,7 @@ object Decorators { case ex: CyclicReference => "... (caught cyclic reference) ..." case NonFatal(ex) if !ctx.mode.is(Mode.PrintShowExceptions) && !ctx.settings.YshowPrintErrors.value => - val msg = ex match - case te: TypeError => te.toMessage.message - case _ => ex.getMessage - s"[cannot display due to $msg, raw string = $x]" + s"... (cannot display due to ${ex.className} ${ex.getMessage}) ..." case _ => String.valueOf(x).nn /** Returns the simple class name of `x`. */ diff --git a/compiler/test/dotty/tools/dotc/core/ShowDecoratorTest.scala b/compiler/test/dotty/tools/dotc/core/ShowDecoratorTest.scala new file mode 100644 index 000000000000..acc9d1914bf6 --- /dev/null +++ b/compiler/test/dotty/tools/dotc/core/ShowDecoratorTest.scala @@ -0,0 +1,21 @@ +package dotty.tools +package dotc +package core + +import Contexts.*, Decorators.*, Denotations.*, SymDenotations.*, Symbols.*, Types.* +import printing.Formatting.Show + +import org.junit.Test +import org.junit.Assert.* + +class ShowDecoratorTest extends DottyTest: + import ShowDecoratorTest.* + + @Test def t1 = assertEquals("... (cannot display due to FooException boom) ...", Foo().tryToShow) +end ShowDecoratorTest + +object ShowDecoratorTest: + import printing.*, Texts.* + class FooException extends Exception("boom") + case class Foo() extends Showable: + def toText(printer: Printer): Text = throw new FooException From 378f5ff17f76c6370e9cba2f1c9775a055e0d377 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Wed, 1 Mar 2023 18:32:25 +0000 Subject: [PATCH 2/3] Add an enrichedErrorMessage on Run, so to only handle the first exception --- compiler/src/dotty/tools/dotc/Driver.scala | 10 +-- compiler/src/dotty/tools/dotc/Run.scala | 32 +++++++--- .../src/dotty/tools/dotc/core/Phases.scala | 4 +- compiler/src/dotty/tools/dotc/report.scala | 63 ++++++++++++++++++- .../src/dotty/tools/dotc/typer/ReTyper.scala | 10 ++- 5 files changed, 97 insertions(+), 22 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/Driver.scala b/compiler/src/dotty/tools/dotc/Driver.scala index 5a2c8b7be56e..e548cae55ddd 100644 --- a/compiler/src/dotty/tools/dotc/Driver.scala +++ b/compiler/src/dotty/tools/dotc/Driver.scala @@ -30,18 +30,20 @@ class Driver { protected def doCompile(compiler: Compiler, files: List[AbstractFile])(using Context): Reporter = if files.nonEmpty then + var runOrNull = ctx.run try val run = compiler.newRun + runOrNull = run run.compile(files) finish(compiler, run) catch case ex: FatalError => report.error(ex.getMessage.nn) // signals that we should fail compilation. - case ex: TypeError => - println(s"${ex.toMessage} while compiling ${files.map(_.path).mkString(", ")}") + case ex: TypeError if !runOrNull.enrichedErrorMessage => + println(runOrNull.enrichErrorMessage(s"${ex.toMessage} while compiling ${files.map(_.path).mkString(", ")}")) throw ex - case ex: Throwable => - println(s"$ex while compiling ${files.map(_.path).mkString(", ")}") + case ex: Throwable if !runOrNull.enrichedErrorMessage => + println(runOrNull.enrichErrorMessage(s"Exception while compiling ${files.map(_.path).mkString(", ")}")) throw ex ctx.reporter diff --git a/compiler/src/dotty/tools/dotc/Run.scala b/compiler/src/dotty/tools/dotc/Run.scala index 8cd1d160b42c..a46ded5d948b 100644 --- a/compiler/src/dotty/tools/dotc/Run.scala +++ b/compiler/src/dotty/tools/dotc/Run.scala @@ -173,15 +173,22 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint */ var ccImportEncountered = false + private var myEnrichedErrorMessage = false + + def enrichedErrorMessage: Boolean = myEnrichedErrorMessage + + def enrichErrorMessage(errorMessage: String)(using Context): String = + if enrichedErrorMessage then errorMessage + else + myEnrichedErrorMessage = true + report.enrichErrorMessage(errorMessage) + def compile(files: List[AbstractFile]): Unit = - try - val sources = files.map(runContext.getSource(_)) - compileSources(sources) - catch - case NonFatal(ex) => - if units.nonEmpty then report.echo(i"exception occurred while compiling $units%, %") - else report.echo(s"exception occurred while compiling ${files.map(_.name).mkString(", ")}") - throw ex + try compileSources(files.map(runContext.getSource(_))) + catch case NonFatal(ex) if !enrichedErrorMessage => + val files1 = if units.isEmpty then files else units.map(_.source.file) + report.echo(enrichErrorMessage(s"exception occurred while compiling ${files1.map(_.path)}")) + throw ex /** TODO: There's a fundamental design problem here: We assemble phases using `fusePhases` * when we first build the compiler. But we modify them with -Yskip, -Ystop @@ -398,3 +405,12 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint given runContext[Dummy_so_its_a_def]: Context = myCtx.nn assert(runContext.runId <= Periods.MaxPossibleRunId) } + +object Run { + extension (run: Run | Null) + def enrichedErrorMessage: Boolean = if run == null then false else run.enrichedErrorMessage + def enrichErrorMessage(errorMessage: String)(using Context): String = + if run == null + then report.enrichErrorMessage(errorMessage) + else run.enrichErrorMessage(errorMessage) +} diff --git a/compiler/src/dotty/tools/dotc/core/Phases.scala b/compiler/src/dotty/tools/dotc/core/Phases.scala index 205554e418ed..00e017430a5f 100644 --- a/compiler/src/dotty/tools/dotc/core/Phases.scala +++ b/compiler/src/dotty/tools/dotc/core/Phases.scala @@ -322,8 +322,8 @@ object Phases { units.map { unit => val unitCtx = ctx.fresh.setPhase(this.start).setCompilationUnit(unit).withRootImports try run(using unitCtx) - catch case ex: Throwable => - println(s"$ex while running $phaseName on $unit") + catch case ex: Throwable if !ctx.run.enrichedErrorMessage => + println(ctx.run.enrichErrorMessage(s"unhandled exception while running $phaseName on $unit")) throw ex unitCtx.compilationUnit } diff --git a/compiler/src/dotty/tools/dotc/report.scala b/compiler/src/dotty/tools/dotc/report.scala index c92fbe5daa56..38f2ab347c4c 100644 --- a/compiler/src/dotty/tools/dotc/report.scala +++ b/compiler/src/dotty/tools/dotc/report.scala @@ -4,13 +4,12 @@ import reporting._ import Diagnostic._ import util.{SourcePosition, NoSourcePosition, SrcPos} import core._ -import Contexts._, Symbols._, Decorators._ +import Contexts._, Flags.*, Symbols._, Decorators._ import config.SourceVersion import ast._ import config.Feature.sourceVersion import java.lang.System.currentTimeMillis - object report: /** For sending messages that are printed only if -verbose is set */ @@ -129,4 +128,64 @@ object report: case Nil => pos recur(pos.sourcePos, tpd.enclosingInlineds) + private object messageRendering extends MessageRendering + + // Should only be called from Run#enrichErrorMessage. + def enrichErrorMessage(errorMessage: String)(using Context): String = try { + def formatExplain(pairs: List[(String, Any)]) = pairs.map((k, v) => f"$k%20s: $v").mkString("\n") + + val settings = ctx.settings.userSetSettings(ctx.settingsState).sortBy(_.name) + val tree = ctx.tree + val sym = tree.symbol + val pos = tree.sourcePos + val path = pos.source.path + val site = ctx.outersIterator.map(_.owner).filter(sym => !sym.exists || sym.isClass || sym.is(Method)).next() + + import untpd.* + extension (tree: Tree) def summaryString: String = tree match + case Literal(const) => s"Literal($const)" + case Ident(name) => s"Ident(${name.decode})" + case Select(qual, name) => s"Select(${qual.summaryString}, ${name.decode})" + case tree: NameTree => (if tree.isType then "type " else "") + tree.name.decode + case tree => s"${tree.className}${if tree.symbol.exists then s"(${tree.symbol})" else ""}" + + val info1 = formatExplain(List( + "while compiling" -> ctx.compilationUnit, + "during phase" -> ctx.phase.prevMega, + "mode" -> ctx.mode, + "library version" -> scala.util.Properties.versionString, + "compiler version" -> dotty.tools.dotc.config.Properties.versionString, + "settings" -> settings.map(s => if s.value == "" then s"${s.name} \"\"" else s"${s.name} ${s.value}").mkString(" "), + )) + val symbolInfos = if sym eq NoSymbol then List("symbol" -> sym) else List( + "symbol" -> sym.showLocated, + "symbol definition" -> s"${sym.showDcl} (a ${sym.className})", + "symbol package" -> sym.enclosingPackageClass.fullName, + "symbol owners" -> sym.showExtendedLocation, + ) + val info2 = formatExplain(List( + "tree" -> tree.summaryString, + "tree position" -> (if pos.exists then s"$path:${pos.line + 1}:${pos.column}" else s"$path:"), + "tree type" -> tree.typeOpt.show, + ) ::: symbolInfos ::: List( + "call site" -> s"${site.showLocated} in ${site.enclosingPackageClass}" + )) + val context_s = try + s""" == Source file context for tree position == + | + |${messageRendering.messageAndPos(Diagnostic.Error("", pos))}""".stripMargin + catch case _: Exception => "" + s""" + | $errorMessage + | + | An unhandled exception was thrown in the compiler. + | Please file a crash report here: + | https://github.com/lampepfl/dotty/issues/new/choose + | + |$info1 + | + |$info2 + | + |$context_s""".stripMargin + } catch case _: Throwable => errorMessage // don't introduce new errors trying to report errors, so swallow exceptions end report diff --git a/compiler/src/dotty/tools/dotc/typer/ReTyper.scala b/compiler/src/dotty/tools/dotc/typer/ReTyper.scala index b53b2f9ec57a..c64f541fd811 100644 --- a/compiler/src/dotty/tools/dotc/typer/ReTyper.scala +++ b/compiler/src/dotty/tools/dotc/typer/ReTyper.scala @@ -124,12 +124,10 @@ class ReTyper(nestingLevel: Int = 0) extends Typer(nestingLevel) with ReChecking override def typedUnadapted(tree: untpd.Tree, pt: Type, locked: TypeVars)(using Context): Tree = try super.typedUnadapted(tree, pt, locked) - catch { - case NonFatal(ex) => - if ctx.phase != Phases.typerPhase && ctx.phase != Phases.inliningPhase then - println(i"exception while typing $tree of class ${tree.getClass} # ${tree.uniqueId}") - throw ex - } + catch case NonFatal(ex) if ctx.phase != Phases.typerPhase && ctx.phase != Phases.inliningPhase && !ctx.run.enrichedErrorMessage => + val treeStr = tree.show(using ctx.withPhase(ctx.phase.prevMega)) + println(ctx.run.enrichErrorMessage(s"exception while retyping $treeStr of class ${tree.className} # ${tree.uniqueId}")) + throw ex override def inlineExpansion(mdef: DefDef)(using Context): List[Tree] = mdef :: Nil From 099f9f2b4b84aeca7fd9ca5782e80e5ef8fa3912 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Mon, 6 Mar 2023 14:28:22 +0000 Subject: [PATCH 3/3] Unduplicate enrich{,ed}ErrorMessage Having a same-named enrichement will cause the original to be called if/when explicit nulls is switched off - which leads to the original method to be called, meaning an NPE will be thrown. So, instead, move all the functionality as an extension method, so we don't need mangled names. --- compiler/src/dotty/tools/dotc/Run.scala | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/Run.scala b/compiler/src/dotty/tools/dotc/Run.scala index a46ded5d948b..944ae794c94f 100644 --- a/compiler/src/dotty/tools/dotc/Run.scala +++ b/compiler/src/dotty/tools/dotc/Run.scala @@ -175,19 +175,11 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint private var myEnrichedErrorMessage = false - def enrichedErrorMessage: Boolean = myEnrichedErrorMessage - - def enrichErrorMessage(errorMessage: String)(using Context): String = - if enrichedErrorMessage then errorMessage - else - myEnrichedErrorMessage = true - report.enrichErrorMessage(errorMessage) - def compile(files: List[AbstractFile]): Unit = try compileSources(files.map(runContext.getSource(_))) - catch case NonFatal(ex) if !enrichedErrorMessage => + catch case NonFatal(ex) if !this.enrichedErrorMessage => val files1 = if units.isEmpty then files else units.map(_.source.file) - report.echo(enrichErrorMessage(s"exception occurred while compiling ${files1.map(_.path)}")) + report.echo(this.enrichErrorMessage(s"exception occurred while compiling ${files1.map(_.path)}")) throw ex /** TODO: There's a fundamental design problem here: We assemble phases using `fusePhases` @@ -408,9 +400,13 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint object Run { extension (run: Run | Null) - def enrichedErrorMessage: Boolean = if run == null then false else run.enrichedErrorMessage + def enrichedErrorMessage: Boolean = if run == null then false else run.myEnrichedErrorMessage def enrichErrorMessage(errorMessage: String)(using Context): String = - if run == null - then report.enrichErrorMessage(errorMessage) - else run.enrichErrorMessage(errorMessage) + if run == null then + report.enrichErrorMessage(errorMessage) + else if !run.enrichedErrorMessage then + run.myEnrichedErrorMessage = true + report.enrichErrorMessage(errorMessage) + else + errorMessage }