From 74fd9d3f25219ad247ee0fca5652c5059c94da51 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Wed, 10 Apr 2024 14:13:27 +0200 Subject: [PATCH 1/3] Make error reporting resilient to exception thrown while reporting Previously the added test failed with `1 error reported` but no actual error message printed, because a stack overflow is thrown while reporting the original error. This is then caught and handled to emit a RecursionOverflow error, but that second error is non-sensical and non-sensical errors are only printed if `hasErrors` returns false. We fix this by deferring incrementing the error count (and therefore having `hasErrors` return true) until after having displayed the error. We also defer calling `markReported` otherwise the second error will also be suppressed. A similar change is necessary in our testing infrastructure to keep the error count is coherent. [Cherry-picked cd313fb0dcc2cebaf6d49f16d5e37ed696c3a0d3] --- .../dotty/tools/dotc/reporting/Reporter.scala | 2 +- .../tools/dotc/reporting/TestReporter.scala | 2 +- tests/neg/mt-deskolemize.scala | 16 ++++++++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 tests/neg/mt-deskolemize.scala diff --git a/compiler/src/dotty/tools/dotc/reporting/Reporter.scala b/compiler/src/dotty/tools/dotc/reporting/Reporter.scala index ca4114a82cdc..980954625723 100644 --- a/compiler/src/dotty/tools/dotc/reporting/Reporter.scala +++ b/compiler/src/dotty/tools/dotc/reporting/Reporter.scala @@ -155,6 +155,7 @@ abstract class Reporter extends interfaces.ReporterResult { addUnreported(key, 1) case _ => if !isHidden(dia) then // avoid isHidden test for summarized warnings so that message is not forced + withMode(Mode.Printing)(doReport(dia)) dia match { case w: Warning => warnings = w :: warnings @@ -168,7 +169,6 @@ abstract class Reporter extends interfaces.ReporterResult { // match error if d is something else } markReported(dia) - withMode(Mode.Printing)(doReport(dia)) end issueUnconfigured def issueIfNotSuppressed(dia: Diagnostic)(using Context): Unit = diff --git a/compiler/test/dotty/tools/dotc/reporting/TestReporter.scala b/compiler/test/dotty/tools/dotc/reporting/TestReporter.scala index 03b61c393d35..f68bc951a1df 100644 --- a/compiler/test/dotty/tools/dotc/reporting/TestReporter.scala +++ b/compiler/test/dotty/tools/dotc/reporting/TestReporter.scala @@ -70,8 +70,8 @@ extends Reporter with UniqueMessagePositions with HideNonSensicalMessages with M } if dia.level >= WARNING then - _diagnosticBuf.append(dia) _consoleReporter.doReport(dia) + _diagnosticBuf.append(dia) printMessageAndPos(dia, extra) } } diff --git a/tests/neg/mt-deskolemize.scala b/tests/neg/mt-deskolemize.scala new file mode 100644 index 000000000000..0a58d5db7bc4 --- /dev/null +++ b/tests/neg/mt-deskolemize.scala @@ -0,0 +1,16 @@ +trait Expr: + type Value +object Expr: + type Of[V] = Expr { type Value = V } + type ExtractValue[F <: Expr] = F match + case Expr.Of[v] => v +import Expr.ExtractValue + +class SimpleLoop1 extends Expr: + type Value = ExtractValue[SimpleLoop2] + +class SimpleLoop2 extends Expr: + type Value = ExtractValue[SimpleLoop1] + +object Test1: + val x: ExtractValue[SimpleLoop1] = 1 // error From 8d893450311758d016d9bd32bb3629a74ce5600f Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Wed, 10 Apr 2024 23:22:58 +0200 Subject: [PATCH 2/3] More explicit handling of exception in error reporting [Cherry-picked e7a1f7ba1504ba32b17ac0fbf36835b0fac629ae] --- compiler/src/dotty/tools/dotc/reporting/Reporter.scala | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/Reporter.scala b/compiler/src/dotty/tools/dotc/reporting/Reporter.scala index 980954625723..9ab1a0029277 100644 --- a/compiler/src/dotty/tools/dotc/reporting/Reporter.scala +++ b/compiler/src/dotty/tools/dotc/reporting/Reporter.scala @@ -15,6 +15,7 @@ import java.io.{BufferedReader, PrintWriter} import scala.annotation.internal.sharable import scala.collection.mutable import core.Decorators.em +import core.handleRecursive object Reporter { /** Convert a SimpleReporter into a real Reporter */ @@ -155,7 +156,12 @@ abstract class Reporter extends interfaces.ReporterResult { addUnreported(key, 1) case _ => if !isHidden(dia) then // avoid isHidden test for summarized warnings so that message is not forced - withMode(Mode.Printing)(doReport(dia)) + try + withMode(Mode.Printing)(doReport(dia)) + catch case ex: Throwable => + // #20158: Don't increment the error count, otherwise we might suppress + // the RecursiveOverflow error and not print any error at all. + handleRecursive("error reporting", dia.message, ex) dia match { case w: Warning => warnings = w :: warnings From cf749961fb163b6bf9bdda72cbe6fac3b844af26 Mon Sep 17 00:00:00 2001 From: Wojciech Mazur Date: Fri, 5 Jul 2024 19:42:59 +0200 Subject: [PATCH 3/3] Move neg/mt-deskolemize test to neg-deep-subtype - crashes due to unbackported changes --- tests/{neg => neg-deep-subtype}/mt-deskolemize.scala | 1 + 1 file changed, 1 insertion(+) rename tests/{neg => neg-deep-subtype}/mt-deskolemize.scala (81%) diff --git a/tests/neg/mt-deskolemize.scala b/tests/neg-deep-subtype/mt-deskolemize.scala similarity index 81% rename from tests/neg/mt-deskolemize.scala rename to tests/neg-deep-subtype/mt-deskolemize.scala index 0a58d5db7bc4..8b69af42e308 100644 --- a/tests/neg/mt-deskolemize.scala +++ b/tests/neg-deep-subtype/mt-deskolemize.scala @@ -1,3 +1,4 @@ +// LTS specific change: -Yno-deep-subtypes makes it crash, in Next is placed in neg/ trait Expr: type Value object Expr: