-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Replace catching StackOverflowError with fuel #25937
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| package dotty.tools | ||
| package dotc | ||
| package core | ||
|
|
||
| import dotty.tools.dotc.core.Contexts.Context | ||
| import dotty.tools.dotc.core.Decorators.* | ||
| import dotty.tools.dotc.reporting.Message | ||
|
|
||
| /** | ||
| * Operation that may cause unbounded recursion depending on user input. | ||
| * @param title the operation title | ||
| * @param details the lazily-initialized operation details | ||
| * @param weight the operation weight, used to prioritize some operations when displaying error messages | ||
| */ | ||
| class RecursiveOperation(val title: String, details: => String, val weight: Int): | ||
| def explanation: String = s"$title $details" | ||
|
|
||
| /** | ||
| * Thrown when recursing too deep, as an alternative to triggering a stack overflow. | ||
| * | ||
| * @param ops the recursive operations, most recent first, i.e., in reverse order | ||
| */ | ||
| class RecursionOverflow(val ops: List[RecursiveOperation])(using Context) extends TypeError: | ||
| override def fillInStackTrace(): Throwable = | ||
| this | ||
|
|
||
| private def opsString(rs: List[RecursiveOperation]): String = { | ||
| val maxShown = 20 | ||
| if (rs.lengthCompare(maxShown) > 0) | ||
| i"""${opsString(rs.take(maxShown / 2))} | ||
| | ... | ||
| |${opsString(rs.takeRight(maxShown / 2))}""" | ||
| else | ||
| rs.map(_.explanation).mkString("\n ", "\n| ", "") | ||
| } | ||
|
|
||
| override def toMessage(using Context): Message = | ||
| val mostCommon = ops.groupBy(_.title).toList.maxBy(_._2.map(_.weight).sum)._2 | ||
| em"""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. | ||
| |For the unprocessed stack trace, compile with -Xno-enrich-error-messages. | ||
| |A recurring operation is (inner to outer): | ||
| |${opsString(mostCommon).stripMargin}""" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2361,7 +2361,7 @@ object SymDenotations { | |
| tp.derivedCapturingType(recur(parent), refs) | ||
|
|
||
| case tp: TypeProxy => | ||
| def computeTypeProxy = { | ||
| def computeTypeProxy = ctx.handleRecursive("type proxy of", i"$tp"): | ||
| val superTp = tp.superType | ||
| val baseTp = recur(superTp) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. each call to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah there are ~20 calls to |
||
| tp match { | ||
|
|
@@ -2370,7 +2370,6 @@ object SymDenotations { | |
| case _ => | ||
| } | ||
| baseTp | ||
| } | ||
| computeTypeProxy | ||
|
|
||
| case tp: AndOrType => | ||
|
|
@@ -2430,7 +2429,7 @@ object SymDenotations { | |
| def computeMemberNames(keepOnly: NameFilter)(implicit onBehalf: MemberNames, ctx: Context): Set[Name] = { | ||
| var names = Set[Name]() | ||
| def maybeAdd(name: Name) = if (keepOnly(thisType, name)) names += name | ||
| try { | ||
| ctx.handleRecursive("member names", i"of $this"): | ||
| for ptype <- parentTypes do | ||
| ptype.classSymbol match | ||
| case pcls: ClassSymbol => | ||
|
|
@@ -2449,10 +2448,6 @@ object SymDenotations { | |
| else info.decls.iterator | ||
| for (sym <- ownSyms) maybeAdd(sym.name) | ||
| names | ||
| } | ||
| catch { | ||
| case ex: Throwable => handleRecursive("member names", i"of $this", ex) | ||
| } | ||
| } | ||
|
|
||
| override final def fullNameSeparated(kind: QualifiedNameKind)(using Context): Name = { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this still relies on stack recursion no? (mutual recursion via traverseChildren)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this PR doesn't change the use of recursion, it changes how stack overflows are detected.