Skip to content

Fix #4368: Avoid stackoverflows for some cyclic definitions #4385

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

Merged
merged 21 commits into from
Jun 5, 2018
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
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
5 changes: 0 additions & 5 deletions compiler/src/dotty/tools/dotc/config/Config.scala
Original file line number Diff line number Diff line change
Expand Up @@ -184,11 +184,6 @@ object Config {
*/
final val LogPendingFindMemberThreshold = 9

/** Maximal number of outstanding recursive calls to findMember before backing out
* when findMemberLimit is set.
*/
final val PendingFindMemberLimit = LogPendingFindMemberThreshold * 4

/** When in IDE, turn StaleSymbol errors into warnings instead of crashing */
final val ignoreStaleInIDE = true
}
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/config/ScalaSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class ScalaSettings extends Settings.SettingGroup {
val Xhelp = BooleanSetting("-X", "Print a synopsis of advanced options.")
val XnoForwarders = BooleanSetting("-Xno-forwarders", "Do not generate static forwarders in mirror classes.")
val XminImplicitSearchDepth = IntSetting("-Xmin-implicit-search-depth", "Set number of levels of implicit searches undertaken before checking for divergence.", 5)
val xmaxInlines = IntSetting("-Xmax-inlines", "Maximal number of successive inlines", 32)
val XmaxInlines = IntSetting("-Xmax-inlines", "Maximal number of successive inlines", 32)
val XmaxClassfileName = IntSetting("-Xmax-classfile-name", "Maximum filename length for generated classes", 255, 72 to 255)
val Xmigration = VersionSetting("-Xmigration", "Warn about constructs whose behavior may have changed since version.")
val Xprint = PhasesSetting("-Xprint", "Print out program after")
Expand Down Expand Up @@ -146,6 +146,7 @@ class ScalaSettings extends Settings.SettingGroup {
val Xlink = BooleanSetting("-Xlink", "Recompile library code with the application.")
val YoptPhases = PhasesSetting("-Yopt-phases", "Restrict the optimisation phases to execute under -optimise.")
val YoptFuel = IntSetting("-Yopt-fuel", "Maximum number of optimisations performed under -optimise.", -1)
val YnoDecodeStacktraces = BooleanSetting("-Yno-decode-stacktraces", "Show operations that triggered StackOverflows instead of printing the raw stacktraces.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment is misleading. How I understand it: "if you pass the flag, you won't see original stack traces". I guess it is the opposite

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh right, fixing it


/** Dottydoc specific settings */
val siteRoot = StringSetting(
Expand Down
44 changes: 12 additions & 32 deletions compiler/src/dotty/tools/dotc/core/Denotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -332,21 +332,8 @@ object Denotations {
}

/** Handle merge conflict by throwing a `MergeError` exception */
private def mergeConflict(tp1: Type, tp2: Type, that: Denotation)(implicit ctx: Context): Type = {
def showSymbol(sym: Symbol): String = if (sym.exists) sym.showLocated else "[unknown]"
def showType(tp: Type) = tp match {
case ClassInfo(_, cls, _, _, _) => cls.showLocated
case bounds: TypeBounds => i"type bounds $bounds"
case _ => tp.show
}
val msg =
s"""cannot merge
| ${showSymbol(this.symbol)} of type ${showType(tp1)} and
| ${showSymbol(that.symbol)} of type ${showType(tp2)}
"""
if (true) throw new MergeError(msg, tp1, tp2)
else throw new Error(msg) // flip condition for debugging
}
private def mergeConflict(tp1: Type, tp2: Type, that: Denotation)(implicit ctx: Context): Type =
throw new MergeError(this.symbol, that.symbol, tp1, tp2, NoPrefix)

/** Merge parameter names of lambda types. If names in corresponding positions match, keep them,
* otherwise generate new synthetic names.
Expand Down Expand Up @@ -537,8 +524,7 @@ object Denotations {
else if (pre.widen.classSymbol.is(Scala2x) || ctx.scala2Mode)
info1 // follow Scala2 linearization -
// compare with way merge is performed in SymDenotation#computeMembersNamed
else
throw new MergeError(s"${ex.getMessage} as members of type ${pre.show}", ex.tp1, ex.tp2)
else throw new MergeError(ex.sym1, ex.sym2, ex.tp1, ex.tp2, pre)
}
new JointRefDenotation(sym, jointInfo, denot1.validFor & denot2.validFor)
}
Expand Down Expand Up @@ -1136,21 +1122,15 @@ object Denotations {
def doubleDefError(denot1: Denotation, denot2: Denotation, pre: Type = NoPrefix)(implicit ctx: Context): Nothing = {
val sym1 = denot1.symbol
val sym2 = denot2.symbol
def fromWhere = if (pre == NoPrefix) "" else i"\nwhen seen as members of $pre"
val msg =
if (denot1.isTerm)
i"""cannot merge
| $sym1: ${sym1.info} and
| $sym2: ${sym2.info};
|they are both defined in ${sym1.owner} but have matching signatures
| ${denot1.info} and
| ${denot2.info}$fromWhere"""
else
i"""cannot merge
| $sym1 ${denot1.info}
| $sym2 ${denot2.info}
|they are conflicting definitions$fromWhere"""
throw new MergeError(msg, denot2.info, denot2.info)
if (denot1.isTerm)
throw new MergeError(sym1, sym2, sym1.info, sym2.info, pre) {
override def addendum(implicit ctx: Context) =
i"""
|they are both defined in ${sym1.owner} but have matching signatures
| ${denot1.info} and
| ${denot2.info}${super.addendum}"""
}
else throw new MergeError(sym1, sym2, denot1.info, denot2.info, pre)
}

// --- Overloaded denotations and predenotations -------------------------------------------------
Expand Down
5 changes: 4 additions & 1 deletion compiler/src/dotty/tools/dotc/core/TypeApplications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ class TypeApplications(val self: Type) extends AnyVal {
* any type parameter that is-rebound by the refinement.
*/
final def typeParams(implicit ctx: Context): List[TypeParamInfo] = /*>|>*/ track("typeParams") /*<|<*/ {
self match {
try self match {
case self: TypeRef =>
val tsym = self.symbol
if (tsym.isClass) tsym.typeParams
Expand All @@ -193,6 +193,9 @@ class TypeApplications(val self: Type) extends AnyVal {
case _ =>
Nil
}
catch {
case ex: Throwable => handleRecursive("type parameters of", self.show, ex)
}
}

/** If `self` is a higher-kinded type, its type parameters, otherwise Nil */
Expand Down
9 changes: 7 additions & 2 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,11 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
protected def isSubType(tp1: Type, tp2: Type, a: ApproxState): Boolean = {
val saved = approx
this.approx = a
try recur(tp1, tp2) finally this.approx = saved
try recur(tp1, tp2)
catch {
case ex: Throwable => handleRecursive("subtype", i"$tp1 <:< $tp2", ex, weight = 2)
}
finally this.approx = saved
}

protected def isSubType(tp1: Type, tp2: Type): Boolean = isSubType(tp1, tp2, NoApprox)
Expand Down Expand Up @@ -161,7 +165,8 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
try {
pendingSubTypes += p
firstTry
} finally {
}
finally {
pendingSubTypes -= p
}
}
Expand Down
158 changes: 158 additions & 0 deletions compiler/src/dotty/tools/dotc/core/TypeErrors.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
package dotty.tools
package dotc
package core

import util.common._
import Types._
import Symbols._
import Flags._
import Names._
import Contexts._
import SymDenotations._
import Denotations._
import Decorators._
import reporting.diagnostic.Message
import reporting.diagnostic.messages._
import ast.untpd
import config.Printers.cyclicErrors

class TypeError(msg: String) extends Exception(msg) {
def this() = this("")
def toMessage(implicit ctx: Context): Message = getMessage
}

class MalformedType(pre: Type, denot: Denotation, absMembers: Set[Name]) extends TypeError {
override def toMessage(implicit ctx: Context): Message =
i"malformed type: $pre is not a legal prefix for $denot because it contains abstract type member${if (absMembers.size == 1) "" else "s"} ${absMembers.mkString(", ")}"
}

class MissingType(pre: Type, name: Name) extends TypeError {
private def otherReason(pre: Type)(implicit ctx: Context): String = pre match {
case pre: ThisType if pre.cls.givenSelfType.exists =>
i"\nor the self type of $pre might not contain all transitive dependencies"
case _ => ""
}

override def toMessage(implicit ctx: Context): Message = {
if (ctx.debug) printStackTrace()
i"""cannot resolve reference to type $pre.$name
|the classfile defining the type might be missing from the classpath${otherReason(pre)}"""
}
}

class RecursionOverflow(val op: String, details: => String, previous: Throwable, val weight: Int) extends TypeError {

def explanation = s"$op $details"

private def recursions: List[RecursionOverflow] = {
val nested = previous match {
case previous: RecursionOverflow => previous.recursions
case _ => Nil
}
this :: nested
}

def opsString(rs: List[RecursionOverflow])(implicit ctx: Context): String = {
val maxShown = 20
if (rs.lengthCompare(maxShown) > 0)
i"""${opsString(rs.take(maxShown / 2))}
| ...
|${opsString(rs.takeRight(maxShown / 2))}"""
else
(rs.map(_.explanation): List[String]).mkString("\n ", "\n| ", "")
}

override def toMessage(implicit ctx: Context): Message = {
val mostCommon = recursions.groupBy(_.op).toList.maxBy(_._2.map(_.weight).sum)._2.reverse
s"""Recursion limit exceeded.
|Maybe there is an illegal cyclic reference?
|If that's not the case, you could also try to increase the stacksize using the -Xss JVM option.
|A recurring operation is (inner to outer):
|${opsString(mostCommon)}""".stripMargin
}

override def fillInStackTrace(): Throwable = this
override def getStackTrace() = previous.getStackTrace()
}

/** Post-process exceptions that might result from StackOverflow to add
* tracing information while unwalking the stack.
*/
// Beware: Since this object is only used when handling a StackOverflow, this code
// cannot consume significant amounts of stack.
object handleRecursive {
def apply(op: String, details: => String, exc: Throwable, weight: Int = 1)(implicit ctx: Context): Nothing = {
if (ctx.settings.YnoDecodeStacktraces.value) {
throw exc
} else {
exc match {
case _: RecursionOverflow =>
throw new RecursionOverflow(op, details, exc, weight)
case _ =>
var e = exc
while (e != null && !e.isInstanceOf[StackOverflowError]) e = e.getCause
if (e != null) throw new RecursionOverflow(op, details, e, weight)
else throw exc
}
}
}
}

class CyclicReference private (val denot: SymDenotation) extends TypeError {

override def toMessage(implicit ctx: Context) = {

def errorMsg(cx: Context): Message =
if (cx.mode is Mode.InferringReturnType) {
cx.tree match {
case tree: untpd.DefDef if !tree.tpt.typeOpt.exists =>
OverloadedOrRecursiveMethodNeedsResultType(tree.name)
case tree: untpd.ValDef if !tree.tpt.typeOpt.exists =>
RecursiveValueNeedsResultType(tree.name)
case _ =>
errorMsg(cx.outer)
}
}
else CyclicReferenceInvolving(denot)

val cycleSym = denot.symbol
if (cycleSym.is(Implicit, butNot = Method) && cycleSym.owner.isTerm)
CyclicReferenceInvolvingImplicit(cycleSym)
else
errorMsg(ctx)
}
}

object CyclicReference {
def apply(denot: SymDenotation)(implicit ctx: Context): CyclicReference = {
val ex = new CyclicReference(denot)
if (!(ctx.mode is Mode.CheckCyclic)) {
cyclicErrors.println(ex.getMessage)
for (elem <- ex.getStackTrace take 200)
cyclicErrors.println(elem.toString)
}
ex
}
}

class MergeError(val sym1: Symbol, val sym2: Symbol, val tp1: Type, val tp2: Type, prefix: Type) extends TypeError {

private def showSymbol(sym: Symbol)(implicit ctx: Context): String =
if (sym.exists) sym.showLocated else "[unknown]"

private def showType(tp: Type)(implicit ctx: Context) = tp match {
case ClassInfo(_, cls, _, _, _) => cls.showLocated
case _ => tp.show
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this refactoring intentionally drop the case for type bounds here (search and compare with the removed showType)? But guess yes, the old output wasn’t ideal either.

}

protected def addendum(implicit ctx: Context) =
if (prefix `eq` NoPrefix) "" else i"\nas members of type $prefix"

override def toMessage(implicit ctx: Context): Message = {
if (ctx.debug) printStackTrace()
i"""cannot merge
| ${showSymbol(sym1)} of type ${showType(tp1)} and
| ${showSymbol(sym2)} of type ${showType(tp2)}$addendum
"""
}
}
6 changes: 0 additions & 6 deletions compiler/src/dotty/tools/dotc/core/TypeOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -326,10 +326,4 @@ trait TypeOps { this: Context => // TODO: Make standalone object.

object TypeOps {
@sharable var track = false // !!!DEBUG

/** When a property with this key is set in a context, it limits the number
* of recursive member searches. If the limit is reached, findMember returns
* NoDenotation.
*/
val findMemberLimit = new Property.Key[Unit]
}
Loading