Skip to content
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

Improved crash handling #16593

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/backend/jvm/BTypes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ abstract class BTypes {
*/
final def maxValueType(other: BType): BType = {

def uncomparable: Nothing = throw new AssertionError(s"Cannot compute maxValueType: $this, $other")
def uncomparable: Nothing = ctx.implode(s"Cannot compute maxValueType: $this, $other")

if (!other.isPrimitive && !other.isNothingType) uncomparable

Expand Down
20 changes: 10 additions & 10 deletions compiler/src/dotty/tools/backend/sjs/JSCodeGen.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2368,10 +2368,10 @@ class JSCodeGen()(using genCtx: Context) {
// Construct inline class definition

val jsClassCaptures = originalClassDef.jsClassCaptures.getOrElse {
throw new AssertionError(s"no class captures for anonymous JS class at $pos")
ctx.implode(s"no class captures for anonymous JS class at $pos")
}
val js.JSConstructorDef(_, ctorParams, ctorRestParam, ctorBody) = constructor.getOrElse {
throw new AssertionError("No ctor found")
ctx.implode("No ctor found")
}
assert(ctorParams.isEmpty && ctorRestParam.isEmpty,
s"non-empty constructor params for anonymous JS class at $pos")
Expand All @@ -2396,17 +2396,17 @@ class JSCodeGen()(using genCtx: Context) {

val memberDefinitions0 = instanceMembers.toList.map {
case fdef: js.FieldDef =>
throw new AssertionError("unexpected FieldDef")
ctx.implode("unexpected FieldDef")

case fdef: js.JSFieldDef =>
implicit val pos = fdef.pos
js.Assign(js.JSSelect(selfRef, fdef.name), jstpe.zeroOf(fdef.ftpe))

case mdef: js.MethodDef =>
throw new AssertionError("unexpected MethodDef")
ctx.implode("unexpected MethodDef")

case cdef: js.JSConstructorDef =>
throw new AssertionError("unexpected JSConstructorDef")
ctx.implode("unexpected JSConstructorDef")

case mdef: js.JSMethodDef =>
implicit val pos = mdef.pos
Expand Down Expand Up @@ -2773,7 +2773,7 @@ class JSCodeGen()(using genCtx: Context) {
if (from == to || from == jstpe.NothingType) {
value
} else if (from == jstpe.BooleanType || to == jstpe.BooleanType) {
throw new AssertionError(s"Invalid genConversion from $from to $to")
ctx.implode(s"Invalid genConversion from $from to $to")
} else {
def intValue = (from: @unchecked) match {
case jstpe.IntType => value
Expand Down Expand Up @@ -3134,7 +3134,7 @@ class JSCodeGen()(using genCtx: Context) {
case value :: Nil =>
genSelectSet(genExpr(jsName), value)
case _ =>
throw new AssertionError(s"property methods should have 0 or 1 non-varargs arguments at $pos")
ctx.implode(s"property methods should have 0 or 1 non-varargs arguments at $pos")
}

case JSCallingConvention.BracketAccess =>
Expand All @@ -3144,7 +3144,7 @@ class JSCodeGen()(using genCtx: Context) {
case keyArg :: valueArg :: Nil =>
genSelectSet(keyArg, valueArg)
case _ =>
throw new AssertionError(s"@JSBracketAccess methods should have 1 or 2 non-varargs arguments at $pos")
ctx.implode(s"@JSBracketAccess methods should have 1 or 2 non-varargs arguments at $pos")
}

case JSCallingConvention.BracketCall =>
Expand Down Expand Up @@ -3238,7 +3238,7 @@ class JSCodeGen()(using genCtx: Context) {
s"Trying to call the super constructor of Object in a non-native JS class at $pos")
genApplyMethod(genReceiver, sym, genScalaArgs)
} else if (sym.isClassConstructor) {
throw new AssertionError(
ctx.implode(
s"calling a JS super constructor should have happened in genPrimaryJSClassCtor at $pos")
} else if (sym.owner.isNonNativeJSClass && !sym.isJSExposed) {
// Reroute to the static method
Expand Down Expand Up @@ -3358,7 +3358,7 @@ class JSCodeGen()(using genCtx: Context) {

clauses = clauses.reverse
val defaultClause = optDefaultClause.getOrElse {
throw new AssertionError("No elseClause in pattern match")
ctx.implode("No elseClause in pattern match")
}

/* Builds a `js.Match`, but simplifies it to a `js.If` if there is only
Expand Down
6 changes: 3 additions & 3 deletions compiler/src/dotty/tools/backend/sjs/JSExportsGen.scala
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ final class JSExportsGen(jsCodeGen: JSCodeGen)(using Context) {
js.TopLevelMethodExportDef(info.moduleID, methodDef)

case Property =>
throw new AssertionError("found top-level exported property")
ctx.implode("found top-level exported property")

case Field =>
val sym = checkSingleField(tups)
Expand Down Expand Up @@ -222,7 +222,7 @@ final class JSExportsGen(jsCodeGen: JSCodeGen)(using Context) {
js.JSFieldDef(flags, name, irTpe)

case kind =>
throw new AssertionError(s"unexpected static export kind: $kind")
ctx.implode(s"unexpected static export kind: $kind")
}
}).toList
}
Expand Down Expand Up @@ -743,7 +743,7 @@ final class JSExportsGen(jsCodeGen: JSCodeGen)(using Context) {
val modAccessor = outer.info.allMembers.find { denot =>
denot.symbol.is(Module) && denot.name.unexpandedName == name
}.getOrElse {
throw new AssertionError(i"could not find module accessor for ${targetSym.fullName} at $pos")
ctx.implode(i"could not find module accessor for ${targetSym.fullName} at $pos")
}.symbol

val receiver = captures.head
Expand Down
12 changes: 5 additions & 7 deletions compiler/src/dotty/tools/dotc/Driver.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import dotty.tools.FatalError
import config.CompilerCommand
import core.Comments.{ContextDoc, ContextDocstrings}
import core.Contexts._
import util.Implosion
import core.{MacroClassLoader, TypeError}
import dotty.tools.dotc.ast.Positioned
import dotty.tools.io.AbstractFile
Expand Down Expand Up @@ -35,14 +36,11 @@ class Driver {
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(", ")}")
throw ex
case ex: Implosion =>
// All handling related to the Implosion is done during creation, so we can swallow this
case ex: Throwable =>
println(s"$ex while compiling ${files.map(_.path).mkString(", ")}")
throw ex
ctx.lateImplode(ex) //this should never happen except in the case of `FatalError`s

ctx.reporter

protected def finish(compiler: Compiler, run: Run)(using Context): Unit =
Expand Down
13 changes: 9 additions & 4 deletions compiler/src/dotty/tools/dotc/Run.scala
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,15 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint
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
if units.nonEmpty then ctx.lateImplode(i"exception occurred while compiling $units%, %")
else ctx.lateImplode(s"exception occurred while compiling ${files.map(_.name).mkString(", ")}")
catch
case _: Implosion =>

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
* on each run. That modification needs to either transform the tree structure,
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/cc/CaptureSet.scala
Original file line number Diff line number Diff line change
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
1 change: 0 additions & 1 deletion compiler/src/dotty/tools/dotc/core/ContextOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import ast.untpd

/** Extension methods for contexts where we want to keep the ctx.<methodName> syntax */
object ContextOps:

extension (ctx: Context)

/** Enter symbol into current class, if current class is owner of current context,
Expand Down
33 changes: 32 additions & 1 deletion compiler/src/dotty/tools/dotc/core/Contexts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import Scopes._
import Uniques._
import ast.Trees._
import ast.untpd
import util.{NoSource, SimpleIdentityMap, SourceFile, HashSet, ReusableInstance}
import util.{NoSource, SimpleIdentityMap, SourceFile, HashSet, ReusableInstance, Implosion}
import typer.{Implicits, ImportInfo, SearchHistory, SearchRoot, TypeAssigner, Typer, Nullables}
import inlines.Inliner
import Nullables._
Expand Down Expand Up @@ -454,6 +454,37 @@ object Contexts {
def freshOver(outer: Context): FreshContext =
FreshContext(base).init(outer, this).setTyperState(this.typerState)

/** 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 =
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
inline def implode(inline cause: Throwable): Nothing =
def implode(inline cause: Throwable): Nothing =

Copy link
Contributor

Choose a reason for hiding this comment

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

We will not gain anything from inlining this definition. We might actually end up generating more code.

Implosion(cause)(using thiscontext)

/**
* Crash the compiler with a message in a controlled way
*/
inline def implode(inline msg: Any): Nothing =
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
inline def implode(inline msg: Any): Nothing =
def implode(inline msg: Any): Nothing =

implode(AssertionError(msg))

/**
* A fallback for when an exception has been thrown without using `implode`
* This will capture context from this context which may be a parent of the context the actual exception was thrown in.
*/
def lateImplode(cause: Throwable): Nothing =
Implosion(Exception("Context was thrown away, this crash report may not be accurate", (cause)))(using ctx)

def lateImplode(msg: Any): Nothing =
lateImplode(AssertionError(msg))

final def withOwner(owner: Symbol): Context =
if (owner ne this.owner) fresh.setOwner(owner) else this

Expand Down
9 changes: 6 additions & 3 deletions compiler/src/dotty/tools/dotc/core/Phases.scala
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import ast.{tpd, untpd}
import scala.annotation.internal.sharable
import scala.util.control.NonFatal



object Phases {

inline def phaseOf(id: PhaseId)(using Context): Phase =
Expand Down Expand Up @@ -322,9 +324,10 @@ 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")
throw 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/core/TypeOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ object TypeOps:
}

def mergeRefinedOrApplied(tp1: Type, tp2: Type): Type = {
def fail = throw new AssertionError(i"Failure to join alternatives $tp1 and $tp2")
def fail = ctx.implode(i"Failure to join alternatives $tp1 and $tp2")
def fallback = tp2 match
case AndType(tp21, tp22) =>
mergeRefinedOrApplied(tp1, tp21) & mergeRefinedOrApplied(tp1, tp22)
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3087,8 +3087,8 @@ object Types {

override def underlying(using Context): Type = parent

private def badInst =
throw new AssertionError(s"bad instantiation: $this")
private def badInst(using Context) =
ctx.implode(s"bad instantiation: $this")

def checkInst(using Context): this.type = this // debug hook

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -863,7 +863,7 @@ class ClassfileParser(
// create a new class member for immediate inner classes
if entry.outer.name == currentClassName then
val file = ctx.platform.classPath.findClassFile(entry.externalName.toString) getOrElse {
throw new AssertionError(entry.externalName)
ctx.implode(entry.externalName)
}
enterClassAndModule(entry, file, entry.jflags)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ trait MessageRendering {
sb.toString
}

private def hl(str: String)(using Context, Level): String =
private def hl(str: String)(using Context, Level): String =
summon[Level].value match
case interfaces.Diagnostic.ERROR => Red(str).show
case interfaces.Diagnostic.WARNING => Yellow(str).show
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/Memoize.scala
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class Memoize extends MiniPhase with IdentityDenotTransformer { thisPhase =>
override def checkPostCondition(tree: Tree)(using Context): Unit = {
def errorLackImplementation(t: Tree) = {
val definingPhase = phaseOf(t.symbol.initial.validFor.firstPhaseId)
throw new AssertionError(
ctx.implode(
i"Non-deferred definition introduced by $definingPhase lacks implementation: $t")
}
tree match {
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
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/TypeUtils.scala
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ object TypeUtils {
def toNestedPairs(using Context): Type =
tupleElementTypes match
case Some(types) => TypeOps.nestedPairs(types)
case None => throw new AssertionError("not a tuple")
case None => ctx.implode("not a tuple")

def refinedWith(name: Name, info: Type)(using Context) = RefinedType(self, name, info)

Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/patmat/Space.scala
Original file line number Diff line number Diff line change
Expand Up @@ -774,7 +774,7 @@ class SpaceEngine(using Context) extends SpaceLogic {

/** Whether the counterexample is satisfiable. The space is flattened and non-empty. */
def satisfiable(sp: Space): Boolean = {
def impossible: Nothing = throw new AssertionError("`satisfiable` only accepts flattened space.")
def impossible: Nothing = ctx.implode("`satisfiable` only accepts flattened space.")

def genConstraint(space: Space): List[(Type, Type)] = space match {
case Prod(tp, unappTp, ss) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ object PrepJSExports {
}

case _: ExportDestination.TopLevel =>
throw new AssertionError(
ctx.implode(
em"Found a top-level export without an explicit name at ${exportPos.sourcePos}")

case ExportDestination.Static =>
Expand Down
9 changes: 5 additions & 4 deletions compiler/src/dotty/tools/dotc/typer/ReTyper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,11 @@ 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
case NonFatal(ex) if ctx.phase != Phases.typerPhase && ctx.phase != Phases.inliningPhase =>
ctx.lateImplode(
Exception(i"exception while typing $tree of class ${tree.getClass} # ${tree.uniqueId}", ex)
)

}

override def inlineExpansion(mdef: DefDef)(using Context): List[Tree] = mdef :: Nil
Expand Down