Skip to content

Commit

Permalink
address some feedback, borrow a lot of scala#16685
Browse files Browse the repository at this point in the history
  • Loading branch information
robmwalsh committed Jan 15, 2023
1 parent ec2e8dc commit 5ccfd52
Show file tree
Hide file tree
Showing 11 changed files with 175 additions and 82 deletions.
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/CompilationUnit.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import core.Decorators._
import config.{SourceVersion, Feature}
import StdNames.nme
import scala.annotation.internal.sharable
import scala.util.control.ControlThrowable
import transform.MacroAnnotations

class CompilationUnit protected (val source: SourceFile) {
Expand Down Expand Up @@ -105,7 +106,7 @@ class CompilationUnit protected (val source: SourceFile) {

object CompilationUnit {

class SuspendException extends Exception
class SuspendException extends ControlThrowable

/** Make a compilation unit for top class `clsd` with the contents of the `unpickled` tree */
def apply(clsd: ClassDenotation, unpickled: Tree, forceTrees: Boolean)(using Context): CompilationUnit =
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/Run.scala
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,8 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint
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
case _: Implosion =>
// All handling related to the Implosion is done during creation, so we can swallow this

/** 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
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/cc/CaptureSet.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package dotc
package cc

import core.*
import Types.*, Symbols.*, Flags.*, Contexts.*, Decorators.*
import Types.*, Symbols.*, Flags.*, Contexts.*, ContextOps.*, Decorators.*
import config.Printers.capt
import Annotations.Annotation
import annotation.threadUnsafe
Expand Down Expand Up @@ -690,7 +690,7 @@ object CaptureSet:
upper.isAlwaysEmpty || upper.isConst && upper.elems.size == 1 && upper.elems.contains(r1)
if variance > 0 || isExact then upper
else if variance < 0 then CaptureSet.empty
else assert(false, i"trying to add $upper from $r via ${tm.getClass} in a non-variant setting")
else ctx.implode( i"trying to add $upper from $r via ${tm.getClass} in a non-variant setting")

/** Apply `f` to each element in `xs`, and join result sets with `++` */
def mapRefs(xs: Refs, f: CaptureRef => CaptureSet)(using Context): CaptureSet =
Expand Down
60 changes: 26 additions & 34 deletions compiler/src/dotty/tools/dotc/core/ContextOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,45 +5,37 @@ import Contexts._, Symbols._, Types._, Flags._
import Denotations._, SymDenotations._
import Names.Name, StdNames.nme
import ast.untpd
import util.*
import scala.util.control.NonFatal

/** Extension methods for contexts where we want to keep the ctx.<methodName> syntax */
object ContextOps:
case class Implosion(ex: Throwable) extends Exception(ex)

extension (ctx: Context)
/** Crash somewhat gracefully, reporting useful info.*/
def implode(message: => Any, ex: Throwable): Nothing = {
ex match
case NonFatal(Implosion(_)) => throw ex // avoid implosions all the way down, ideally we should avoid catching Implosions in the first place
case NonFatal(ex) =>
given Context = ctx
report.error(
s"""|An unhandled exception was thrown in the compiler. Please file a crash
|report here: https://github.com/lampepfl/dotty/issues/new/choose
|
|Exception: ${ex.getClass}
| message: ${ex.getMessage()}
| cause: ${ex.getCause()}
| (truncated) stack trace:
| ${ex.getStackTrace().nn.take(10).nn.mkString("\n ")}
|Sources:
| ${ctx.base.sources.keysIterator.mkString(":")}
|Files:
| ${ctx.base.files.keysIterator.mkString(":")}
|Context:
| phase: ${ctx.phase}
| owner: ${ctx.owner}
| unit: ${ctx.compilationUnit}
| mode: ${ctx.mode}
| source: ${ctx.source}:${ctx.tree.sourcePos.line + 1}:${ctx.tree.sourcePos.column + 1}
| ${ctx.tree}
|
|""".stripMargin,
ctx.tree.sourcePos)

throw Implosion(ex)
case _ => throw ex
}
/** Crash the compiler with a `Throwable` in a controlled way, reporting useful info
* and asking users to submit a crash report.
* This helps users by providing information they can use to work around the crash
* and helps compiler maintainers by capturing important information about the crash.
* With the exception of `ControlThrowable`s, this method should be used instead of
* throwing exceptions whenever a context is available.
*
* instead of:
* `throw Exception("foo")`
* use:
* `ctx.implode(Exception("foo"))`
*/
inline def implode(inline cause: Throwable): Nothing =
Implosion(cause)(using ctx)

inline def implode(inline msg: String): Nothing =
implode(AssertionError(msg))

/**
* A fallback for when an exception has been thrown without using `implode`
*
*/
def lateImplode(cause: Throwable): Nothing =
Implosion(Exception("Context was thrown away, this crash report may not be accurate", (cause)))(using ctx)

/** Enter symbol into current class, if current class is owner of current context,
* or into current scope, if not. Should always be called instead of scope.enter
Expand Down
5 changes: 3 additions & 2 deletions compiler/src/dotty/tools/dotc/core/Phases.scala
Original file line number Diff line number Diff line change
Expand Up @@ -325,8 +325,9 @@ object Phases {
units.map { unit =>
val unitCtx = ctx.fresh.setPhase(this.start).setCompilationUnit(unit).withRootImports
try run(using unitCtx)
catch case ex: Throwable if !(ex.isInstanceOf[Implosion] || ex.isInstanceOf[SuspendException]) =>
unitCtx.implode(i"$ex while running $phaseName on $unit", ex)
catch
case NonFatal(ex) =>
unitCtx.lateImplode(Exception(i"$ex while running $phaseName on $unit", ex))

unitCtx.compilationUnit
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/Erasure.scala
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import ContextFunctionResults._
import ExplicitOuter._
import core.Mode
import util.Property
import util.Assert.assert

import reporting._

class Erasure extends Phase with DenotTransformer {
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/Recheck.scala
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ abstract class Recheck extends Phase, SymTransformer:
constFold(tree, instantiate(fntpe, argTypes, tree.fun.symbol))
//.showing(i"typed app $tree : $fntpe with ${tree.args}%, % : $argTypes%, % = $result")
case tp =>
assert(false, i"unexpected type of ${tree.fun}: $funtpe")
ctx.implode(i"unexpected type of ${tree.fun}: $funtpe")

def recheckTypeApply(tree: TypeApply, pt: Type)(using Context): Type =
recheck(tree.fun).widen match
Expand Down
5 changes: 2 additions & 3 deletions compiler/src/dotty/tools/dotc/typer/ReTyper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,8 @@ class ReTyper(nestingLevel: Int = 0) extends Typer(nestingLevel) with ReChecking
try super.typedUnadapted(tree, pt, locked)
catch {
case NonFatal(ex) if ctx.phase != Phases.typerPhase && ctx.phase != Phases.inliningPhase =>
ctx.implode(
i"exception while typing $tree of class ${tree.getClass} # ${tree.uniqueId}",
ex
ctx.lateImplode(
Exception(i"exception while typing $tree of class ${tree.getClass} # ${tree.uniqueId}", ex)
)

}
Expand Down
32 changes: 0 additions & 32 deletions compiler/src/dotty/tools/dotc/util/Assertion.scala

This file was deleted.

119 changes: 119 additions & 0 deletions compiler/src/dotty/tools/dotc/util/Implosion.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
package dotty.tools.dotc
package util

import core.*
import Contexts.*, Flags.*, ContextOps.*, Symbols.*, Decorators.*
import reporting.*
import util.{SourcePosition, NoSourcePosition, SrcPos}
import ast.*


import scala.util.control.ControlThrowable

/**
* An `Error` that indicates that the compiler has crashed.
* This is used to capture and report important information from the context when the compiler crashes.
* An `Implosion` should only be caught at the edge of the compiler to prevent the crash propagating to the caller of the compiler.
*
* No handling is required
*
* @param cause The exception that caused the crash.
*/
class Implosion private (message: String, val cause: Throwable) extends ControlThrowable:
override def getCause(): Throwable = cause
override def stackTrace(): Array[StackTraceElement] = cause.getStackTrace()
cause.setStackTrace(Array()) //Avoid printing the stack trace twice


object Implosion :

def apply(cause: Throwable)(using ctx: Context): Nothing =
if cause.isInstanceOf[ControlThrowable] then
//not rethrown because `implode` should always crash the compiler
report.error("A `ControlThrowable` was used to crash the compiler")
val message = enrichErrorMessage(cause.getMessage())
report.error(message, ctx.tree.sourcePos)
throw new Implosion(message, cause)

private object messageRendering extends MessageRendering

private def enrichErrorMessage(errorMessage: String)(using Context): String =
// record state as we go along so we can report something more than the errorMessage if there's an error while reporting
val blackBox = new StringBuilder()

extension [A](a: A)
transparent inline def record: A =
blackBox.append(a).append("\n")
a


"""|Error during error reporting, black box recording:
|An unhandled exception was thrown in the compiler. Please file a crash
|report here: https://github.com/lampepfl/dotty/issues/new/choose)""".stripMargin.record

errorMessage.record

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).record
val tree = ctx.tree.record
val sym = tree.symbol.record
val pos = tree.sourcePos.record
val path = pos.source.path.record
val site = ctx.outersIterator.map(_.owner).filter(sym => !sym.exists || sym.isClass || sym.is(Method)).next().record

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 ""}"

"info1:".record
val info1 = formatExplain(List(
"while compiling".record -> ctx.compilationUnit.record,
"during phase".record -> ctx.phase.prevMega.record,
"mode".record -> ctx.mode.record,
"library version".record -> scala.util.Properties.versionString.record,
"compiler version".record -> dotty.tools.dotc.config.Properties.versionString.record,
"settings".record -> settings.map(s => if s.value == "" then s"${s.name} \"\"" else s"${s.name} ${s.value}").mkString(" ").record,
))
"symbolInfos:".record
val symbolInfos = if sym eq NoSymbol then List("symbol".record -> sym.record) else List(
"symbol".record -> sym.showLocated.record,
"symbol definition".record -> s"${sym.showDcl} (a ${sym.className})".record,
"symbol package".record -> sym.enclosingPackageClass.fullName.record,
"symbol owners".record -> sym.showExtendedLocation.record,
)
"info2:".record
val info2 = formatExplain(List(
"tree".record -> tree.summaryString,
"tree position".record -> (if pos.exists then s"$path:${pos.line + 1}:${pos.column}" else s"$path:<unknown>"),
"tree type".record -> tree.typeOpt.show.record,
) ::: symbolInfos.record ::: List(
"call site".record -> s"${site.showLocated} in ${site.enclosingPackageClass}".record
))
"context_s:".record
val context_s = try
s""" == Source file context for tree position ==
|
|${messageRendering.messageAndPos(Diagnostic.Error("", pos))}""".stripMargin.record
catch case _: Exception => "<Cannot read source file>".record
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 =>
// don't introduce new errors trying to report errors, so swallow exceptions and fall back to the blackBox recording
blackBox.toString()
23 changes: 17 additions & 6 deletions compiler/src/dotty/tools/package.scala
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,24 @@ package object tools {

def unreachable(x: Any = "<< this case was declared unreachable >>"): Nothing =
throw new MatchError(x)

import dotty.tools.dotc.core.Contexts.Context
transparent inline def assertShort(inline assertion: Boolean, inline message: Any = null)(using inline ctx: Context | Null = null): Unit =
if !assertion then throwAssertionError(message, truncateStack = true)

//todo get location of assertion and add to message
transparent inline def assert(inline assertion: Boolean, inline message: Any | Null = null)(using inline ctx: Context | Null = null): Unit =
if !assertion then throwAssertionError(message)

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
// extracted from `assert` to make it as small (and inlineable) as possible
private def throwAssertionError(message: Any | Null, truncateStack: Boolean = false)(using ctx: Context | Null): Unit =
import dotty.tools.dotc.core.ContextOps.implode
val assertionError = AssertionError("assertion failed: " + String.valueOf(message).nn)
if truncateStack then assertionError.setStackTrace(Array())
if ctx == null then
throw assertionError
else
ctx.implode(assertionError)

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

0 comments on commit 5ccfd52

Please sign in to comment.