Skip to content

Revamp exception handling, from nsc #16685

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all 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
15 changes: 9 additions & 6 deletions compiler/src/dotty/tools/dotc/Driver.scala
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
package dotty.tools.dotc
package dotty.tools
package dotc

import dotty.tools.FatalError
import config.CompilerCommand
Expand Down Expand Up @@ -30,18 +31,20 @@ class Driver {

protected def doCompile(compiler: Compiler, files: List[AbstractFile])(using Context): Reporter =
if files.nonEmpty then
var run1 = ctx.run
try
val run = compiler.newRun
run1 = 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(", ")}")
report.error(run1.enrichErrorMessage(ex.getMessage.nn)) // signals that we should fail compilation.

Choose a reason for hiding this comment

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

It's too late to capture the context here. Unless I'm mistaken, the tree, symbol, position, even source file are all going to be wrong at this point, because the context they existed in got blown away many stack frames ago. see my last para here #16593 (comment)

@smarter's comment in the next reply seems to back this up

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, this case effectively handles when compiler.newRun fails. If it succeeds, then there's the handling in run.compile. I just applied the same strategy to these existing catch cases, to avoid it spamming, in particular I took out re-printing the exception message.

case ex: TypeError if !run1.enrichedErrorMessage =>
println(run1.enrichErrorMessage(s"TypeError while compiling ${files.map(_.path).mkString(", ")}"))
throw ex
case ex: Throwable =>
println(s"$ex while compiling ${files.map(_.path).mkString(", ")}")
case NonFatal(ex) if !run1.enrichedErrorMessage =>
println(run1.enrichErrorMessage(s"Exception while compiling ${files.map(_.path).mkString(", ")}"))
throw ex
ctx.reporter

Expand Down
32 changes: 24 additions & 8 deletions compiler/src/dotty/tools/dotc/Run.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
myEnrichedErrorMessage = true
report.enrichErrorMessage(errorMessage)
else 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.nonEmpty then units.map(_.source.file) else files
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
Expand Down Expand Up @@ -395,3 +402,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)
}
5 changes: 1 addition & 4 deletions compiler/src/dotty/tools/dotc/core/Decorators.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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"... (caught ${ex.className}) ..."
case _ => String.valueOf(x).nn

/** Returns the simple class name of `x`. */
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/core/Phases.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 NonFatal(ex) if !ctx.run.enrichedErrorMessage =>
println(ctx.run.enrichErrorMessage(s"unhandled exception while running $phaseName on $unit"))
throw ex
unitCtx.compilationUnit
}
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/core/TypeErasure.scala
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,8 @@ class TypeErasure(sourceLanguage: SourceLanguage, semiEraseVCs: Boolean, isConst
tp
case tp if (tp `eq` NoType) || (tp `eq` NoPrefix) =>
tp
case tp =>
unreachable(i"TypeErasure applied to $tp (of class ${tp.className})")
}

/** Like translucentSuperType, but issue a fatal error if it does not exist. */
Expand Down
62 changes: 60 additions & 2 deletions compiler/src/dotty/tools/dotc/report.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -129,4 +128,63 @@ 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:<unknown>"),
"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 => "<Cannot read source file>"
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
10 changes: 4 additions & 6 deletions compiler/src/dotty/tools/dotc/typer/ReTyper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
27 changes: 21 additions & 6 deletions compiler/src/dotty/tools/package.scala
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,27 @@ package object tools {
def unreachable(x: Any = "<< this case was declared unreachable >>"): Nothing =
throw new MatchError(x)

transparent inline def assertShort(inline assertion: Boolean, inline message: Any = null): Unit =
if !assertion then
val msg = message
val e = if msg == null then AssertionError() else AssertionError("assertion failed: " + msg)
e.setStackTrace(Array())
throw e
transparent inline def assertShort(inline assertion: Boolean, inline message: Any | Null = null): Unit =
if !assertion then throwAssertionShortError(message)

private def throwAssertionShortError(msg: Any): Nothing =
val e = AssertionError("assertion failed: " + String.valueOf(msg))
e.setStackTrace(Array())
throw e

import dotty.tools.dotc.core.Contexts.*

transparent inline def assert(inline assertion: Boolean, inline message: Any | Null = null)(using inline ctx: Context | Null = null): Unit =
if !assertion then throwAssertionError(message)

// extracted from `assert` to make it as small (and inlineable) as possible
private def throwAssertionError(message: Any | Null)(using ctx: Context | Null): Nothing =
val msg = String.valueOf(message).nn
if ctx == null then
throw AssertionError("assertion failed: " + msg)
else inContext(ctx) {
throw AssertionError("assertion failed: " + ctx.run.enrichErrorMessage(msg))
}

// Ensure this object is already classloaded, since it's only actually used
// when handling stack overflows and every operation (including class loading)
Expand Down