From 68754650716cd86c5c070dab753f424752e11a1c Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Thu, 30 Nov 2023 17:42:46 +0000 Subject: [PATCH 1/3] Set errorsReported while running isHidden and forcing message --- .../src/dotty/tools/dotc/reporting/Reporter.scala | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/Reporter.scala b/compiler/src/dotty/tools/dotc/reporting/Reporter.scala index f567e094e831..b3e54d2e6734 100644 --- a/compiler/src/dotty/tools/dotc/reporting/Reporter.scala +++ b/compiler/src/dotty/tools/dotc/reporting/Reporter.scala @@ -154,7 +154,20 @@ abstract class Reporter extends interfaces.ReporterResult { val key = w.enablingOption.name addUnreported(key, 1) case _ => - if !isHidden(dia) then // avoid isHidden test for summarized warnings so that message is not forced + val hide = if !errorsReported && dia.isInstanceOf[Error] then + // We bump up errorCount so errorsReported is true while executing isHidden. + // We use errorsReported as a predicate for broken code. + // So now any code `isHidden` runs won't cause, for instance, + // assertion errors and thus compiler crashes. + // This normally amounts to forcing the message, which might run more code + // to generate useful hints for the user. + try + _errorCount += 1 + isHidden(dia) + finally + _errorCount -= 1 // decrease rather than set back to `oldErrorCount` so we only ever decrease by 1 + else isHidden(dia) + if !hide then // avoid isHidden test for summarized warnings so that message is not forced markReported(dia) withMode(Mode.Printing)(doReport(dia)) dia match { From 4fb8e7c9b1844085325876b4c2db7f5a517be691 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Thu, 30 Nov 2023 22:41:29 +0000 Subject: [PATCH 2/3] Move faking errors to HideNonSensicalMessages Because HideNonSensicalMessages looks at hasErrors, we need to narrow the faking to just the isNonSensical call --- .../reporting/HideNonSensicalMessages.scala | 13 ++++++++++++- .../dotty/tools/dotc/reporting/Reporter.scala | 19 +++---------------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/HideNonSensicalMessages.scala b/compiler/src/dotty/tools/dotc/reporting/HideNonSensicalMessages.scala index 5910d9b4d656..50b89d9fb393 100644 --- a/compiler/src/dotty/tools/dotc/reporting/HideNonSensicalMessages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/HideNonSensicalMessages.scala @@ -13,7 +13,18 @@ trait HideNonSensicalMessages extends Reporter { */ override def isHidden(dia: Diagnostic)(using Context): Boolean = super.isHidden(dia) || { - dia.msg.isNonSensical && + (if !errorsReported && dia.isInstanceOf[Diagnostic.Error] then + // Bump up errorCount so hasErrors is true while forcing the message. + // We use errorsReported as a predicate for broken code. + // So now any forcing won't cause, for instance, + // assertion errors and thus compiler crashes. + // Some messages, once forced, run more code + // to generate useful hints for the user. + try + _errorCount += 1 + dia.msg.isNonSensical + finally _errorCount -= 1 // decrease rather than reset the value so we only ever decrease by 1 + else dia.msg.isNonSensical) && hasErrors && // if there are no errors yet, report even if diagnostic is non-sensical !ctx.settings.YshowSuppressedErrors.value } diff --git a/compiler/src/dotty/tools/dotc/reporting/Reporter.scala b/compiler/src/dotty/tools/dotc/reporting/Reporter.scala index b3e54d2e6734..c24e8ba4f122 100644 --- a/compiler/src/dotty/tools/dotc/reporting/Reporter.scala +++ b/compiler/src/dotty/tools/dotc/reporting/Reporter.scala @@ -92,8 +92,8 @@ abstract class Reporter extends interfaces.ReporterResult { private def isIncompleteChecking = incompleteHandler ne defaultIncompleteHandler - private var _errorCount = 0 - private var _warningCount = 0 + protected var _errorCount = 0 + protected var _warningCount = 0 /** The number of errors reported by this reporter (ignoring outer reporters) */ def errorCount: Int = _errorCount @@ -154,20 +154,7 @@ abstract class Reporter extends interfaces.ReporterResult { val key = w.enablingOption.name addUnreported(key, 1) case _ => - val hide = if !errorsReported && dia.isInstanceOf[Error] then - // We bump up errorCount so errorsReported is true while executing isHidden. - // We use errorsReported as a predicate for broken code. - // So now any code `isHidden` runs won't cause, for instance, - // assertion errors and thus compiler crashes. - // This normally amounts to forcing the message, which might run more code - // to generate useful hints for the user. - try - _errorCount += 1 - isHidden(dia) - finally - _errorCount -= 1 // decrease rather than set back to `oldErrorCount` so we only ever decrease by 1 - else isHidden(dia) - if !hide then // avoid isHidden test for summarized warnings so that message is not forced + if !isHidden(dia) then // avoid isHidden test for summarized warnings so that message is not forced markReported(dia) withMode(Mode.Printing)(doReport(dia)) dia match { From 31165f276204404c5a9910d3308e09b71ebf1ca5 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Fri, 15 Dec 2023 12:13:46 +0000 Subject: [PATCH 3/3] Call hasErrors first but then bump before doReport We can switch to a more straightforward hasErrors check in isHidden, but then we need to bump the errorCount before calling doReport, as doReport will then, necessarily, force the message. For reference, the way I test this manually is by: 1. In ignoredConvertibleImplicits, changing back to `viewExists(imp, fail.expectedType)` 2. In adapt1, removing the `dummyTreeOfType.unapply(tree).isEmpty` guard 3. Compiling tests/neg/i18650.scala Also, while I'm here, instruct on the presence of -Yno-enrich-error-messages, like we do for -Yno-decode-stacktraces. --- compiler/src/dotty/tools/dotc/report.scala | 1 + .../reporting/HideNonSensicalMessages.scala | 17 +++-------------- .../dotty/tools/dotc/reporting/Reporter.scala | 8 ++++---- 3 files changed, 8 insertions(+), 18 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/report.scala b/compiler/src/dotty/tools/dotc/report.scala index 00b20b94ac87..8e39afdd6e7d 100644 --- a/compiler/src/dotty/tools/dotc/report.scala +++ b/compiler/src/dotty/tools/dotc/report.scala @@ -155,6 +155,7 @@ object report: | An unhandled exception was thrown in the compiler. | Please file a crash report here: | https://github.com/lampepfl/dotty/issues/new/choose + | For non-enriched exceptions, compile with -Yno-enrich-error-messages. | |$info1 |""".stripMargin diff --git a/compiler/src/dotty/tools/dotc/reporting/HideNonSensicalMessages.scala b/compiler/src/dotty/tools/dotc/reporting/HideNonSensicalMessages.scala index 50b89d9fb393..81e17c495d90 100644 --- a/compiler/src/dotty/tools/dotc/reporting/HideNonSensicalMessages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/HideNonSensicalMessages.scala @@ -13,19 +13,8 @@ trait HideNonSensicalMessages extends Reporter { */ override def isHidden(dia: Diagnostic)(using Context): Boolean = super.isHidden(dia) || { - (if !errorsReported && dia.isInstanceOf[Diagnostic.Error] then - // Bump up errorCount so hasErrors is true while forcing the message. - // We use errorsReported as a predicate for broken code. - // So now any forcing won't cause, for instance, - // assertion errors and thus compiler crashes. - // Some messages, once forced, run more code - // to generate useful hints for the user. - try - _errorCount += 1 - dia.msg.isNonSensical - finally _errorCount -= 1 // decrease rather than reset the value so we only ever decrease by 1 - else dia.msg.isNonSensical) && - hasErrors && // if there are no errors yet, report even if diagnostic is non-sensical - !ctx.settings.YshowSuppressedErrors.value + hasErrors // if there are no errors yet, report even if diagnostic is non-sensical + && dia.msg.isNonSensical // defer forcing the message by calling hasErrors first + && !ctx.settings.YshowSuppressedErrors.value } } diff --git a/compiler/src/dotty/tools/dotc/reporting/Reporter.scala b/compiler/src/dotty/tools/dotc/reporting/Reporter.scala index c24e8ba4f122..ca4114a82cdc 100644 --- a/compiler/src/dotty/tools/dotc/reporting/Reporter.scala +++ b/compiler/src/dotty/tools/dotc/reporting/Reporter.scala @@ -92,8 +92,8 @@ abstract class Reporter extends interfaces.ReporterResult { private def isIncompleteChecking = incompleteHandler ne defaultIncompleteHandler - protected var _errorCount = 0 - protected var _warningCount = 0 + private var _errorCount = 0 + private var _warningCount = 0 /** The number of errors reported by this reporter (ignoring outer reporters) */ def errorCount: Int = _errorCount @@ -155,8 +155,6 @@ 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 - markReported(dia) - withMode(Mode.Printing)(doReport(dia)) dia match { case w: Warning => warnings = w :: warnings @@ -169,6 +167,8 @@ abstract class Reporter extends interfaces.ReporterResult { case _: Info => // nothing to do here // match error if d is something else } + markReported(dia) + withMode(Mode.Printing)(doReport(dia)) end issueUnconfigured def issueIfNotSuppressed(dia: Diagnostic)(using Context): Unit =