Replace catching StackOverflowError with fuel by SolalPirelli · Pull Request #25937 · scala/scala3

SolalPirelli · 2026-04-27T13:58:28Z

Part of #25799
Fixes #26159
Fixes #25724
Fixes #26458
Fixes #25718
Fixes #24683

Some neg tests no longer fail; all of them were failing with infinite recursions, I checked

Benchmarks within noise: https://lampepfl.github.io/scala3-benchmarks/#compare/3.9.0-RC1-bin-20260518-0ce4c47-NIGHTLY,3.9.0-RC1-bin-a8f3042310c66f22322ed9743c00872e8c6c5d21-BENCH

How much have you relied on LLM-based tools in this contribution?

Not at all

How was the solution tested?

Covered by existing tests (this is a refactoring)

SolalPirelli · 2026-04-27T13:59:37Z

    T#T#T#T#T#T#T#T#T#T#T#T#T#T#T#T#
    T#T#T#T#T#T#T#T#T#T#T#T#T#T#T#T#
    T#T#T#T#T#T#T#T#T#T#T#T#T#T#T#T#
-    T#T#T#T#T#T#T#T#T#T#T#T#T#T#T#T#


avoids a stack overflow; this test was introduced purely as a stress test in d3ac612 and the other elements of it were already reduced in 0ee31f4 so I think removing this line is fine

mbovel · 2026-05-20T04:34:54Z

Benchmarks completed. Overview.

bishabosha · 2026-05-20T06:34:07Z


+    /** Ensures recursive operations obey the fuel limit, and throws user-friendly errors when they do not. */
+    inline final def handleRecursive[T](name: String, details: => String, weight: Int = 1)(inline block: T): T =
+      val op = RecursiveOperation(name, details, weight)


also isnt this in the past this metadata only needed for the "outermost" operation, so perhaps another optimisation (rather than capturing a whole trace even in happy path)

or tune the level of tracing captured via settings?

we need the entire trace if something goes wrong, so we can show a message with the most frequent operations etc

but yeah the problem is fundamentally since we can't catch SOs we have to store info even in the happy path; I'll look at ways to lower the impact of this

given that you actually still use the stack to recurse, cant you only construct the metadata when you throw the exception and then catch the exception from each outer layer to add its trace in and rethrow?

yeah, I'm a little allergic to using exceptions for control flow, and it might negate the perf gains of not building the metadata, but we'll see if this latest benchmark run isn't good enough

mbovel · 2026-05-21T15:30:44Z

Benchmarks completed. Overview.

SolalPirelli · 2026-05-26T13:54:34Z


  /** Check type members inherited from different `parents` of `joint` type for cycles,
   *  unless a type with the same name already appears in `decls`.
-   *  @return    true iff no cycles were detected


unused return value (was already Unit, I guess it used to be a Boolean), noticed while working on this

SolalPirelli · 2026-05-26T13:55:55Z

+  |For the unprocessed stack overflow trace, compile with -Xno-enrich-error-messages.
+  |A recurring operation is (inner to outer):
  |
-  |         reduce type  t match ...


this changed message allows the details to be just an object instead of having to be computed to add a string afterwards

SolalPirelli · 2026-05-26T14:09:13Z

      withTyperState(typerState.uncommittedAncestor)

+    /** Ensures recursive operations obey the fuel limit, and throws user-friendly errors when they do not. */
+    final inline def handleRecursive[T](title: String, details: RecursiveOperationDetails, weight: Int = 1)(inline block: T): T =


the weight is only used in one place, not sure if it's that useful

mbovel · 2026-05-29T13:56:23Z

Benchmarks started. Workflow run.

SolalPirelli · 2026-05-29T13:57:21Z

@@ -0,0 +1,20 @@
+//> using options -Xmax-inlines:100000


this is #26159

SolalPirelli · 2026-05-29T13:58:18Z

 def combineAll[A <: SemigroupStructural[A]](
  i: A, l: List[A]  // error
-): A = l.foldLeft(i)(_.combine(_)) // error
+): A = l.foldLeft(i)(_.combine(_))


Since we stop at the first potentially-infinite recursion in order to make sure we have enough stack to display nice error messages (we need to unwind first), some errors no longer appear. I split tests that were testing multiple independent infinite recursions into multiple tests.

jchyb

Looks great, I just have a few questions for parts I don't understand yet

jchyb · 2026-08-12T09:51:01Z

    implicit val testGroup: TestGroup = TestGroup("compileNeg")
-
-    aggregateTests(
+    withCoverage(aggregateTests(


withCoverage? Does it expose any recursion overflows, or is it an unrelated change (probably fine either way, just wondering)?

It filters using the scoverage exclude list when running under coverage; there's one neg test I had to add in there

This is more of a bugfix for the testing harness, since all testing kinds should be going through withCoverage already. (I agree the name is... dubious. It doesn't actually run anything with coverage, it just does stuff if coverage is enabled)

ah, that makes sense, thank you!

jchyb · 2026-08-12T10:27:49Z

-    try
-      if isFullyDefined(tp, ForceDegree.all) then tp
-      else throw new Error(i"internal error: type of $what $tp is not fully defined, pos = $pos")
-    catch case ex: RecursionOverflow =>


Now that the RecursionOverflow exception is safe (and not tied to any undefined behaviors), it feels weird that we only catch it and report in Driver, only reporting the first encountered one, where previously we were able to report multiple different ones. Is there some specific reason for that? I see there are handlers added in typedUnadapted and other places, where there weren't any before - is it related to that?

Catching RecursionOverflow outside of the Driver means you probably don't have that much stack left yourself, and in the worst case, you can't even properly print what caused the recursion overflow because the printer can use a fair amount of stack. :(

…ave enough stack to display

anatoliykmetyuk · 2026-08-24T04:17:49Z

The first Scoverage nightly after this merged exposed coverage failures in 16463.scala, i10605.scala, i15158.scala, i15311.scala, i21015.scala, i2887b.scala, i5877.scala, matchtype-loop2.scala, recursive-lower-constraint.scala, and i20516.scala. The first nine stopped producing scoverage.coverage; i20516.scala exceeded the recursion limit. See the failing CI job. These tests are temporarily quarantined in #26894.

Fixes scoverage CI breakage by recent PRs. Also re-enable scoverage tests to run on every PR. The coverage invocation now runs only test suites explicitly marked as supporting coverage instrumentation, instead of also executing ordinary uninstrumented suites. It seems that the policy to only run scoverage tests nightly, intended to reduce friction on new semantic compiler changes, does not work so well, as the breaking changes fall between the cracks, ending up in the specialized Nightly CI Scoverage workflow. In August alone, the following were the breaking PRs for scoverage: - #26697 - #25937 - #26156 - #26813 Therefore, to prevent scoverage from drifting overtime, I believe running scoverage tests on CI should be done on each PR, while compiling subprojects with scoverage can stay in Nightly build. ## Have you relied on LLM-based tools in this contribution? Yes, and I checked the output by local testing, review, running CI (as this is the CI change). ## How was the solution tested? Non-code change, no tests needed

SolalPirelli commented Apr 27, 2026

View reviewed changes

SolalPirelli force-pushed the solal/fuel branch 2 times, most recently from 958aabd to 1e2b230 Compare April 29, 2026 11:17

SolalPirelli changed the title ~~no-review: Fuel~~ Replace catching StackOverflowError with fuel Apr 29, 2026

bishabosha reviewed Apr 29, 2026

View reviewed changes

Comment thread compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala Outdated

bishabosha reviewed Apr 29, 2026

View reviewed changes

Comment thread compiler/src/dotty/tools/dotc/core/SymDenotations.scala

SolalPirelli force-pushed the solal/fuel branch 3 times, most recently from e3ff246 to 44aa990 Compare May 18, 2026 13:00

SolalPirelli requested review from mbovel and sjrd May 18, 2026 14:23

This comment was marked as outdated.

Sign in to view

bishabosha reviewed May 20, 2026

View reviewed changes

Comment thread compiler/src/dotty/tools/dotc/core/Contexts.scala Outdated

bishabosha reviewed May 20, 2026

View reviewed changes

This comment was marked as outdated.

Sign in to view

SolalPirelli force-pushed the solal/fuel branch from b376520 to d69fca0 Compare May 20, 2026 14:25

SolalPirelli force-pushed the solal/fuel branch from d69fca0 to b4d7d23 Compare May 26, 2026 09:20

SolalPirelli commented May 26, 2026

View reviewed changes

SolalPirelli marked this pull request as ready for review May 26, 2026 13:56

SolalPirelli assigned sjrd May 26, 2026

SolalPirelli commented May 26, 2026

View reviewed changes

SolalPirelli mentioned this pull request May 29, 2026

Unexpected Stackoverflow when macro inlining #26159

Closed

SolalPirelli force-pushed the solal/fuel branch 2 times, most recently from 694009c to f1b787e Compare May 29, 2026 13:37

SolalPirelli commented May 29, 2026

View reviewed changes

Gedochao assigned jchyb Aug 10, 2026

SolalPirelli added needs-squashing PR whose commits should be squashed by the author or via the "Squash and Merge" button release-notes Should be mentioned in the release notes labels Aug 11, 2026

jchyb reviewed Aug 12, 2026

View reviewed changes

SolalPirelli force-pushed the solal/fuel branch from b271fa1 to e40f0fd Compare August 12, 2026 12:40

jchyb approved these changes Aug 12, 2026

View reviewed changes

SolalPirelli added 12 commits August 13, 2026 09:51

fuel

f79c29d

Sorry Toli

fb1114b

fix?

af3e082

fixes

f2acd06

Rework: only catch RecursionOverflow at module boundaries so errors h…

2418f90

…ave enough stack to display

Move catches to doCompile

ffe1eba

include neg in coverage exclude

037e154

Fix one more reported case

ac6b1a9

Fix this absurdity?

31cf33b

Catch one more case

d309a28

Fix and regression tests

36bf88b

Update check file

c845bcf

SolalPirelli force-pushed the solal/fuel branch from e40f0fd to c845bcf Compare August 13, 2026 08:09

SolalPirelli enabled auto-merge (squash) August 13, 2026 08:15

SolalPirelli merged commit a4e1b44 into scala:main Aug 13, 2026
78 of 79 checks passed

SolalPirelli deleted the solal/fuel branch August 13, 2026 09:18

anatoliykmetyuk mentioned this pull request Aug 24, 2026

Run scoverage tests on pull requests #26894

Merged

WojciechMazur added this to the 3.10.0 milestone Aug 26, 2026

Conversation

SolalPirelli commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How much have you relied on LLM-based tools in this contribution?

How was the solution tested?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

mbovel commented May 20, 2026

Uh oh!

Uh oh!

bishabosha May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

mbovel commented May 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mbovel commented May 29, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jchyb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SolalPirelli Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SolalPirelli Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

anatoliykmetyuk commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

SolalPirelli commented Apr 27, 2026 •

edited

Loading

bishabosha May 20, 2026 •

edited

Loading

SolalPirelli Aug 12, 2026 •

edited

Loading

SolalPirelli Aug 12, 2026 •

edited

Loading

anatoliykmetyuk commented Aug 24, 2026 •

edited

Loading